diff --git a/CHANGES.rst b/CHANGES.rst index 92c84fff..74ed0f39 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -21,6 +21,7 @@ Fixed ``--stdout`` option. This makes the option compatible with the `new Black extension for VSCode`__. - Badge links in the README on GitHub. +- Handling of relative paths and running from elsewhere than repository root. __ https://github.com/microsoft/vscode-black-formatter diff --git a/src/darker/__main__.py b/src/darker/__main__.py index 93734312..f9c77d46 100644 --- a/src/darker/__main__.py +++ b/src/darker/__main__.py @@ -516,10 +516,14 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen if args.skip_magic_trailing_comma is not None: black_config["skip_magic_trailing_comma"] = args.skip_magic_trailing_comma - paths, root = resolve_paths(args.stdin_filename, args.src) + paths, common_root = resolve_paths(args.stdin_filename, args.src) + # `common_root` is now the common root of given paths, + # not necessarily the repository root. + # `paths` are the unmodified paths from `--stdin-filename` or `SRC`, + # so either relative to the current working directory or absolute paths. revrange = RevisionRange.parse_with_common_ancestor( - args.revision, root, args.stdin_filename is not None + args.revision, common_root, args.stdin_filename is not None ) output_mode = OutputMode.from_args(args) write_modified_files = not args.check and output_mode == OutputMode.NOTHING @@ -539,7 +543,7 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen ) if revrange.rev2 != STDIN: - missing = get_missing_at_revision(paths, revrange.rev2, root) + missing = get_missing_at_revision(paths, revrange.rev2, common_root) if missing: missing_reprs = " ".join(repr(str(path)) for path in missing) rev2_repr = ( @@ -550,9 +554,9 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen f"Error: Path(s) {missing_reprs} do not exist in {rev2_repr}", ) - # These paths are relative to `root`: - files_to_process = filter_python_files(paths, root, {}) - files_to_blacken = filter_python_files(paths, root, black_config) + # These paths are relative to `common_root`: + files_to_process = filter_python_files(paths, common_root, {}) + files_to_blacken = filter_python_files(paths, common_root, black_config) # Now decide which files to reformat (Black & isort). Note that this doesn't apply # to linting. if output_mode == OutputMode.CONTENT or revrange.rev2 == STDIN: @@ -563,11 +567,11 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen black_exclude = set() else: # In other modes, only reformat files which have been modified. - if git_is_repository(root): + if git_is_repository(common_root): # Get the modified files only. - repo_root = find_project_root((str(root),)) + repo_root = find_project_root((str(common_root),)) changed_files = { - (repo_root / file).relative_to(root) + (repo_root / file).relative_to(common_root) for file in git_get_modified_python_files(paths, revrange, repo_root) } # Filter out changed files that are not supposed to be processed @@ -584,7 +588,7 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen formatting_failures_on_modified_lines = False for path, old, new in sorted( format_edited_parts( - root, + common_root, changed_files_to_reformat, Exclusions( black=black_exclude, @@ -602,16 +606,16 @@ def main( # pylint: disable=too-many-locals,too-many-branches,too-many-statemen # there were any changes to the original formatting_failures_on_modified_lines = True if output_mode == OutputMode.DIFF: - print_diff(path, old, new, root, use_color) + print_diff(path, old, new, common_root, use_color) elif output_mode == OutputMode.CONTENT: print_source(new, use_color) if write_modified_files: modify_file(path, new) linter_failures_on_modified_lines = run_linters( args.lint, - root, + common_root, # paths to lint are not limited to modified files or just Python files: - {p.resolve().relative_to(root) for p in paths}, + {p.resolve().relative_to(common_root) for p in paths}, revrange, use_color, ) diff --git a/src/darker/git.py b/src/darker/git.py index fff2eb1d..e89ca863 100644 --- a/src/darker/git.py +++ b/src/darker/git.py @@ -76,25 +76,33 @@ def should_reformat_file(path: Path) -> bool: return path.exists() and get_path_in_repo(path).suffix == ".py" -def _git_exists_in_revision(path: Path, rev2: str, cwd: Path) -> bool: +def _git_exists_in_revision(path: Path, rev2: str, git_cwd: Path) -> bool: """Return ``True`` if the given path exists in the given Git revision - :param path: The path of the file or directory to check + :param path: The path of the file or directory to check, either relative to current + working directory or absolute :param rev2: The Git revision to look at - :param cwd: The Git repository root + :param git_cwd: The working directory to use when invoking Git. This has to be + either the root of the working tree, or another directory inside it. :return: ``True`` if the file or directory exists at the revision, or ``False`` if it doesn't. """ - if (cwd / path).resolve() == cwd.resolve(): - return True + while not git_cwd.exists(): + # The working directory for running Git doesn't exist. Walk up the directory + # tree until we find an existing directory. This is necessary because `git + # cat-file` doesn't work if the current working directory doesn't exist. + git_cwd = git_cwd.parent + relative_path = (Path.cwd() / path).relative_to(git_cwd.resolve()) # Surprise: On Windows, `git cat-file` doesn't work with backslash directory # separators in paths. We need to use Posix paths and forward slashes instead. - cmd = ["git", "cat-file", "-e", f"{rev2}:{path.as_posix()}"] - logger.debug("[%s]$ %s", cwd, " ".join(cmd)) + # Surprise #2: `git cat-file` assumes paths are relative to the repository root. + # We need to prepend `./` to paths relative to the working directory. + cmd = ["git", "cat-file", "-e", f"{rev2}:./{relative_path.as_posix()}"] + logger.debug("[%s]$ %s", git_cwd, " ".join(cmd)) result = run( # nosec cmd, - cwd=str(cwd), + cwd=str(git_cwd), check=False, stderr=DEVNULL, env=make_git_env(), @@ -108,9 +116,10 @@ def get_missing_at_revision(paths: Iterable[Path], rev2: str, cwd: Path) -> Set[ In case of ``WORKTREE``, just check if the files exist on the filesystem instead of asking Git. - :param paths: Paths to check + :param paths: Paths to check, relative to the current working directory or absolute :param rev2: The Git revision to look at, or ``WORKTREE`` for the working tree - :param cwd: The Git repository root + :param cwd: The working directory to use when invoking Git. This has to be either + the root of the working tree, or another directory inside it. :return: The set of file or directory paths which are missing in the revision """ @@ -120,15 +129,15 @@ def get_missing_at_revision(paths: Iterable[Path], rev2: str, cwd: Path) -> Set[ def _git_diff_name_only( - rev1: str, rev2: str, relative_paths: Iterable[Path], cwd: Path + rev1: str, rev2: str, relative_paths: Iterable[Path], repo_root: Path ) -> Set[Path]: """Collect names of changed files between commits from Git :param rev1: The old commit to compare to :param rev2: The new commit to compare, or the string ``":WORKTREE:"`` to compare current working tree to ``rev1`` - :param relative_paths: Relative paths to the files to compare - :param cwd: The Git repository root + :param relative_paths: Relative paths from repository root to the files to compare + :param repo_root: The Git repository root :return: Relative paths of changed files """ @@ -143,7 +152,7 @@ def _git_diff_name_only( ] if rev2 != WORKTREE: diff_cmd.insert(diff_cmd.index("--"), rev2) - lines = git_check_output_lines(diff_cmd, cwd) + lines = git_check_output_lines(diff_cmd, repo_root) return {Path(line) for line in lines} @@ -153,7 +162,7 @@ def _git_ls_files_others(relative_paths: Iterable[Path], cwd: Path) -> Set[Path] This will return those files in ``relative_paths`` which are untracked and not excluded by ``.gitignore`` or other Git's exclusion mechanisms. - :param relative_paths: Relative paths to the files to consider + :param relative_paths: Relative paths from repository root to the files to consider :param cwd: The Git repository root :return: Relative paths of untracked files @@ -170,23 +179,26 @@ def _git_ls_files_others(relative_paths: Iterable[Path], cwd: Path) -> Set[Path] def git_get_modified_python_files( - paths: Iterable[Path], revrange: RevisionRange, cwd: Path + paths: Iterable[Path], revrange: RevisionRange, repo_root: Path ) -> Set[Path]: """Ask Git for modified and untracked ``*.py`` files - ``git diff --name-only --relative -- `` - ``git ls-files --others --exclude-standard -- `` - :param paths: Relative paths to the files to diff + :param paths: Files to diff, either relative to the current working dir or absolute :param revrange: Git revision range to compare - :param cwd: The Git repository root + :param repo_root: The Git repository root :return: File names relative to the Git repository root """ - changed_paths = _git_diff_name_only(revrange.rev1, revrange.rev2, paths, cwd) + repo_paths = [path.resolve().relative_to(repo_root) for path in paths] + changed_paths = _git_diff_name_only( + revrange.rev1, revrange.rev2, repo_paths, repo_root + ) if revrange.rev2 == WORKTREE: - changed_paths.update(_git_ls_files_others(paths, cwd)) - return {path for path in changed_paths if should_reformat_file(cwd / path)} + changed_paths.update(_git_ls_files_others(repo_paths, repo_root)) + return {path for path in changed_paths if should_reformat_file(repo_root / path)} def _revision_vs_lines( diff --git a/src/darker/tests/test_git.py b/src/darker/tests/test_git.py index 8601faff..8b65ed85 100644 --- a/src/darker/tests/test_git.py +++ b/src/darker/tests/test_git.py @@ -69,7 +69,7 @@ def test_git_exists_in_revision_git_call(retval, expect): result = git._git_exists_in_revision(Path("path.py"), "rev2", Path(".")) run.assert_called_once_with( - ["git", "cat-file", "-e", "rev2:path.py"], + ["git", "cat-file", "-e", "rev2:./path.py"], cwd=".", check=False, stderr=DEVNULL, @@ -79,54 +79,125 @@ def test_git_exists_in_revision_git_call(retval, expect): @pytest.mark.kwparametrize( - dict(rev2="{add}", path="dir/a.py", expect=True), - dict(rev2="{add}", path="dir/b.py", expect=True), - dict(rev2="{add}", path="dir/", expect=True), - dict(rev2="{add}", path="dir", expect=True), - dict(rev2="{del_a}", path="dir/a.py", expect=False), - dict(rev2="{del_a}", path="dir/b.py", expect=True), - dict(rev2="{del_a}", path="dir/", expect=True), - dict(rev2="{del_a}", path="dir", expect=True), - dict(rev2="HEAD", path="dir/a.py", expect=False), - dict(rev2="HEAD", path="dir/b.py", expect=False), - dict(rev2="HEAD", path="dir/", expect=False), - dict(rev2="HEAD", path="dir", expect=False), + dict(cwd=".", rev2="{add}", path="x/dir/a.py", expect=True), + dict(cwd=".", rev2="{add}", path="x/dir/sub/b.py", expect=True), + dict(cwd=".", rev2="{add}", path="x/dir/", expect=True), + dict(cwd=".", rev2="{add}", path="x/dir", expect=True), + dict(cwd=".", rev2="{add}", path="x/dir/sub", expect=True), + dict(cwd=".", rev2="{del_a}", path="x/dir/a.py", expect=False), + dict(cwd=".", rev2="{del_a}", path="x/dir/sub/b.py", expect=True), + dict(cwd=".", rev2="{del_a}", path="x/dir/", expect=True), + dict(cwd=".", rev2="{del_a}", path="x/dir", expect=True), + dict(cwd=".", rev2="{del_a}", path="x/dir/sub", expect=True), + dict(cwd=".", rev2="HEAD", path="x/dir/a.py", expect=False), + dict(cwd=".", rev2="HEAD", path="x/dir/sub/b.py", expect=False), + dict(cwd=".", rev2="HEAD", path="x/dir/", expect=False), + dict(cwd=".", rev2="HEAD", path="x/dir", expect=False), + dict(cwd=".", rev2="HEAD", path="x/dir/sub", expect=False), + dict(cwd="x", rev2="{add}", path="dir/a.py", expect=True), + dict(cwd="x", rev2="{add}", path="dir/sub/b.py", expect=True), + dict(cwd="x", rev2="{add}", path="dir/", expect=True), + dict(cwd="x", rev2="{add}", path="dir", expect=True), + dict(cwd="x", rev2="{add}", path="dir/sub", expect=True), + dict(cwd="x", rev2="{del_a}", path="dir/a.py", expect=False), + dict(cwd="x", rev2="{del_a}", path="dir/sub/b.py", expect=True), + dict(cwd="x", rev2="{del_a}", path="dir/", expect=True), + dict(cwd="x", rev2="{del_a}", path="dir", expect=True), + dict(cwd="x", rev2="{del_a}", path="dir/sub", expect=True), + dict(cwd="x", rev2="HEAD", path="dir/a.py", expect=False), + dict(cwd="x", rev2="HEAD", path="dir/sub/b.py", expect=False), + dict(cwd="x", rev2="HEAD", path="dir/", expect=False), + dict(cwd="x", rev2="HEAD", path="dir", expect=False), + dict(cwd="x", rev2="HEAD", path="dir/sub", expect=False), ) -def test_git_exists_in_revision(git_repo, rev2, path, expect): +def test_git_exists_in_revision(git_repo, monkeypatch, cwd, rev2, path, expect): """``_get_exists_in_revision()`` detects file/dir existence correctly""" - git_repo.add({"dir/a.py": "", "dir/b.py": ""}, commit="Add dir/*.py") + git_repo.add( + {"x/README": "", "x/dir/a.py": "", "x/dir/sub/b.py": ""}, + commit="Add x/dir/*.py", + ) add = git_repo.get_hash() - git_repo.add({"dir/a.py": None}, commit="Delete dir/a.py") + git_repo.add({"x/dir/a.py": None}, commit="Delete x/dir/a.py") del_a = git_repo.get_hash() - git_repo.add({"dir/b.py": None}, commit="Delete dir/b.py") + git_repo.add({"x/dir/sub/b.py": None}, commit="Delete x/dir/b.py") + monkeypatch.chdir(cwd) result = git._git_exists_in_revision( - Path(path), rev2.format(add=add, del_a=del_a), git_repo.root + Path(path), rev2.format(add=add, del_a=del_a), git_repo.root / "x/dir/sub" ) assert result == expect @pytest.mark.kwparametrize( - dict(rev2="{add}", expect=set()), - dict(rev2="{del_a}", expect={Path("dir/a.py")}), - dict(rev2="HEAD", expect={Path("dir"), Path("dir/a.py"), Path("dir/b.py")}), + dict( + paths={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + rev2="{add}", + expect=set(), + ), + dict( + paths={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + rev2="{del_a}", + expect={"x/dir/a.py"}, + ), + dict( + paths={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + rev2="HEAD", + expect={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + ), + dict( + paths={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + rev2=":WORKTREE:", + expect={"x/dir", "x/dir/a.py", "x/dir/sub", "x/dir/sub/b.py"}, + ), + dict( + paths={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + cwd="x", + rev2="{add}", + expect=set(), + ), + dict( + paths={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + cwd="x", + rev2="{del_a}", + expect={"dir/a.py"}, + ), + dict( + paths={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + cwd="x", + rev2="HEAD", + expect={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + ), + dict( + paths={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + cwd="x", + rev2=":WORKTREE:", + expect={"dir", "dir/a.py", "dir/sub", "dir/sub/b.py"}, + ), + cwd=".", + git_cwd=".", ) -def test_get_missing_at_revision(git_repo, rev2, expect): +def test_get_missing_at_revision( + git_repo, monkeypatch, paths, cwd, git_cwd, rev2, expect +): """``get_missing_at_revision()`` returns missing files/directories correctly""" - git_repo.add({"dir/a.py": "", "dir/b.py": ""}, commit="Add dir/*.py") + git_repo.add( + {"x/README": "", "x/dir/a.py": "", "x/dir/sub/b.py": ""}, + commit="Add x/dir/**/*.py", + ) add = git_repo.get_hash() - git_repo.add({"dir/a.py": None}, commit="Delete dir/a.py") + git_repo.add({"x/dir/a.py": None}, commit="Delete x/dir/a.py") del_a = git_repo.get_hash() - git_repo.add({"dir/b.py": None}, commit="Delete dir/b.py") + git_repo.add({"x/dir/sub/b.py": None}, commit="Delete x/dir/sub/b.py") + monkeypatch.chdir(git_repo.root / cwd) result = git.get_missing_at_revision( - {Path("dir"), Path("dir/a.py"), Path("dir/b.py")}, + {Path(p) for p in paths}, rev2.format(add=add, del_a=del_a), - git_repo.root, + git_repo.root / git_cwd, ) - assert result == expect + assert result == {Path(p) for p in expect} def test_get_missing_at_revision_worktree(git_repo): @@ -224,7 +295,7 @@ def test_git_get_modified_python_files(git_repo, modify_paths, paths, expect): revrange = RevisionRange("HEAD", ":WORKTREE:") result = git.git_get_modified_python_files( - {root / p for p in paths}, revrange, cwd=root + {root / p for p in paths}, revrange, repo_root=root ) assert result == {Path(p) for p in expect}