From b8f814f695ff52e6365c7129b9899d9321d65644 Mon Sep 17 00:00:00 2001 From: Philipp Joram Date: Sun, 20 Sep 2020 12:37:34 +0200 Subject: [PATCH 1/3] Fix get_versioned_files() failing when GIT_INDEX_FILE is set Instead of running `git ls-files` in each git submodule, run $ git ls-file --recurse-submodules from the git root directory and list all files under version control in one go. This fixes a bug when check-manifest would fail with ['git', 'ls-files', '-z'] failed (status 128): fatal: .git/index: index file open failed: Not a directory when run from a pre-commit hook. When running from a pre-commit hook, git sets `GIT_INDEX_FILE=.git/index`, which should be interpreted relative to the git root directory. When changing directories into a submodule directory (say `./submodule/`) and running `git ls-files` there, git expects to find a git index file at `./submodule/.git/index`, which fails since `./submodule/.git` is a regular file in a submodule. The easy solution is to just pass `--recurse-submodules` when listing files under version control and remove all logic listing submodules files one-by-one. A test is added that sets `GIT_INDEX_FILE=.git/index` and checks that tests dealing with submodules still pass. This simulates the "running from git-hook" setting --- check_manifest.py | 29 +++++------------------------ tests.py | 4 ++++ 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/check_manifest.py b/check_manifest.py index 32ca497..8bb67f9 100755 --- a/check_manifest.py +++ b/check_manifest.py @@ -406,33 +406,14 @@ def detect(cls, location): def get_versioned_files(self): """List all files versioned by git in the current directory.""" - files = self._git_ls_files() - submodules = self._list_submodules() - for subdir in submodules: - subdir = os.path.relpath(subdir).replace(os.path.sep, '/') - files += add_prefix_to_each(subdir, self._git_ls_files(subdir)) - return files - - @classmethod - def _git_ls_files(cls, cwd=None): - output = run(['git', 'ls-files', '-z'], encoding=cls._encoding, cwd=cwd) + output = run( + ["git", "ls-files", "-z", "--recurse-submodules"], + encoding=self._encoding, + ) # -z tells git to use \0 as a line terminator; split() treats it as a # line separator, so we always get one empty line at the end, which we # drop with the [:-1] slice - return output.split('\0')[:-1] - - @classmethod - def _list_submodules(cls): - # This is incredibly expensive on my Jenkins instance (50 seconds for - # each invocation, even when there are no submodules whatsoever). - # Curiously, I cannot reproduce that in Appveyor, or even on the same - # Jenkins machine but when I run the tests manually. Still, 2-hour - # Jenkins runs are bad, so let's avoid running 'git submodule' when - # there's no .gitmodules file. - if not os.path.exists('.gitmodules'): - return [] - return run(['git', 'submodule', '--quiet', 'foreach', '--recursive', - 'printf "%s/%s\\n" $toplevel $path'], encoding=cls._encoding).splitlines() + return output.split("\0")[:-1] class Mercurial(VCS): diff --git a/tests.py b/tests.py index b55ba96..951a4f2 100644 --- a/tests.py +++ b/tests.py @@ -1102,6 +1102,10 @@ def test_get_versioned_files_with_git_submodules(self): 'subdir/sub2/sub3/file4', ]) + def test_get_versioned_files_with_git_submodules_with_git_index_file_set(self): + with mock.patch.dict(os.environ, {"GIT_INDEX_FILE": ".git/index"}): + self.test_get_versioned_files_with_git_submodules() + class BzrHelper(VCSHelper): From db1481966007ff6021da601accfcfb46632b1de0 Mon Sep 17 00:00:00 2001 From: Philipp Joram Date: Mon, 21 Sep 2020 14:01:53 +0200 Subject: [PATCH 2/3] Remove add_prefix_to_each() This is not needed anymore since b8f814f. --- check_manifest.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/check_manifest.py b/check_manifest.py index 8bb67f9..8a9cd79 100755 --- a/check_manifest.py +++ b/check_manifest.py @@ -369,16 +369,6 @@ def strip_toplevel_name(filelist): return [name[len(prefix):] for name in names if name != prefix] -def add_prefix_to_each(prefix, filelist): - """Add a prefix to each name in a file list. - - >>> add_prefix_to_each('foo/bar', ['a', 'b', 'c/d']) - ['foo/bar/a', 'foo/bar/b', 'foo/bar/c/d'] - - """ - return [posixpath.join(prefix, name) for name in filelist] - - class VCS: def __init__(self, ui): From 4319e5030331cff45d31d31272d771269af0f2bb Mon Sep 17 00:00:00 2001 From: Philipp Joram Date: Mon, 21 Sep 2020 14:32:10 +0200 Subject: [PATCH 3/3] Add bugfix of #122 to changelog --- CHANGES.rst | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index dd10525..44373f4 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,11 @@ Changelog 0.43 (unreleased) ----------------- -- Nothing changed yet. +- Fix collecting files versioned by ``git`` when a project has submodules and + ``GIT_INDEX_FILE`` is set. This bug was triggered when ``check-manifest`` + was run as part of a git hook ( + `#122 `__, + `#123 `__). 0.42 (2020-05-03)