From 40c83283d600058a1217628a18c2d765de3cceb2 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Fri, 1 Mar 2024 15:35:57 -0800 Subject: [PATCH] [rfc] Support Sapling (alternative SCM) when using precommit When using [Sapling SCM](https://sapling-scm.com/) with a Git repository, precommit expectedly fails today because it shells out to `git` to get the list of modified files in the working copy. In a Sapling clone of a Git repository, these calls will fail. While most of the logic in precommit that deals with talking to Git today is encapsulated in `git.py`, calls to `git` are sprinkled throughout the project and are called directly. In order to support alternative SCMs, such as Sapling, ideally there would be a more generic "SCM" interface that business logic would call into with both Git and Sapling implementations. Though this PR does not attempt to introduce such an interface. Instead, it is just a basic PoC to provide evidence that something like this could be made to work. For the moment, it takes a shortcut and simply sprinkles "if Sapling" checks in `git.py` in enough places that I could get precommit to run in a Sapling working copy in the simple case where I had only one file modified locally. My higher-level question is whether the precommit project would be open to accepting changes to support Sapling, either via the introduction of an SCM-agnostic interface or some other means. --- pre_commit/commands/run.py | 4 ++ pre_commit/git.py | 81 +++++++++++++++++++++++++++++++-- pre_commit/staged_files_only.py | 6 ++- 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 076f16d8..75fd5e57 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -322,6 +322,10 @@ def _run_hooks( def _has_unmerged_paths() -> bool: + if git.is_sapling(): + # FIXME: Implement this properly. + return False + _, stdout, _ = cmd_output_b('git', 'ls-files', '--unmerged') return bool(stdout.strip()) diff --git a/pre_commit/git.py b/pre_commit/git.py index 19aac387..3743599d 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -1,9 +1,11 @@ from __future__ import annotations import logging +import os import os.path import sys from collections.abc import Mapping +from enum import Flag, auto from pre_commit.errors import FatalError from pre_commit.util import CalledProcessError @@ -54,13 +56,15 @@ def get_root() -> str: # "rev-parse --show-cdup" to get the appropriate path, but must perform # an extra check to see if we are in the .git directory. try: - root = os.path.abspath( - cmd_output('git', 'rev-parse', '--show-cdup')[1].strip(), - ) inside_git_dir = cmd_output( 'git', 'rev-parse', '--is-inside-git-dir', )[1].strip() except CalledProcessError: + # Check if `git rev-parse` failed because we are in a Sapling repo. + sapling_root = get_sapling_root() + if sapling_root: + return sapling_root + raise FatalError( 'git failed. Is it installed, and are you in a Git repository ' 'directory?', @@ -70,9 +74,68 @@ def get_root() -> str: 'git toplevel unexpectedly empty! make sure you are not ' 'inside the `.git` directory of your repository.', ) + + try: + root = os.path.abspath( + cmd_output('git', 'rev-parse', '--show-cdup')[1].strip(), + ) + except CalledProcessError: + raise FatalError( + 'git failed. Is it installed, and are you in a Git repository ' + 'directory?', + ) + return root +SAPLING_CLI = 'sl' + +def get_sapling_root() -> str | None: + try: + sapling_root = sapling_output('root')[1].strip() + except CalledProcessError: + return None + return sapling_root + + +def is_sapling() -> bool: + return get_sapling_root() is not None + + +class SaplingStatus(Flag): + MODIFIED = auto() + ADDED = auto() + REMOVED = auto() + DELETED = auto() + UNKNOWN = auto() + + +def sapling_status(status_flags: SaplingStatus) -> list[str]: + args = ["status", "--print0", "--root-relative", "--no-status"] + for enum_value, flag in [ + (SaplingStatus.MODIFIED, '--modified'), + (SaplingStatus.ADDED, '--added'), + (SaplingStatus.REMOVED, '--removed'), + (SaplingStatus.DELETED, '--deleted'), + (SaplingStatus.UNKNOWN, '--unknown'), + ]: + if enum_value & status_flags: + args.append(flag) + + return zsplit(sapling_output(*args)[1]) + + +def sapling_output(*cmd: str, **kwargs) -> tuple[int, str, str | None]: + """cmd should NOT include the initial `sl` argument: that is implied""" + env = kwargs.get('env') + if env is None: + env = os.environ.copy() + kwargs['env'] = env + env['SL_AUTOMATION'] = '1' # Ensure SL_AUTOMATION is set. + args = SAPLING_CLI, *cmd + return cmd_output(*args, **kwargs) + + def get_git_dir(git_root: str = '.') -> str: opt = '--git-dir' _, out, _ = cmd_output('git', 'rev-parse', opt, cwd=git_root) @@ -94,6 +157,10 @@ def get_git_common_dir(git_root: str = '.') -> str: def is_in_merge_conflict() -> bool: + if is_sapling(): + # FIXME + return False + git_dir = get_git_dir('.') return ( os.path.exists(os.path.join(git_dir, 'MERGE_MSG')) and @@ -113,6 +180,10 @@ def parse_merge_msg_for_conflicts(merge_msg: bytes) -> list[str]: def get_conflicted_files() -> set[str]: logger.info('Checking merge-conflict files only.') + if is_sapling(): + # FIXME + return set() + # Need to get the conflicted files from the MERGE_MSG because they could # have resolved the conflict by choosing one side or the other with open(os.path.join(get_git_dir('.'), 'MERGE_MSG'), 'rb') as f: @@ -133,6 +204,10 @@ def get_conflicted_files() -> set[str]: def get_staged_files(cwd: str | None = None) -> list[str]: + if is_sapling(): + flags = SaplingStatus.MODIFIED | SaplingStatus.ADDED + return sapling_status(flags) + return zsplit( cmd_output( 'git', 'diff', '--staged', '--name-only', '--no-ext-diff', '-z', diff --git a/pre_commit/staged_files_only.py b/pre_commit/staged_files_only.py index e1f81ba9..5c9faef0 100644 --- a/pre_commit/staged_files_only.py +++ b/pre_commit/staged_files_only.py @@ -109,5 +109,9 @@ def staged_files_only(patch_dir: str) -> Generator[None, None, None]: """Clear any unstaged changes from the git working directory inside this context. """ - with _intent_to_add_cleared(), _unstaged_changes_cleared(patch_dir): + if git.is_sapling(): + # FIXME: what is the intention here? yield + else: + with _intent_to_add_cleared(), _unstaged_changes_cleared(patch_dir): + yield