From e9ed248a15b89faeb0389877b02d1f78ffe24243 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 1 Oct 2021 18:45:36 -0400 Subject: [PATCH] make sure to not discard changes even if submodule.recurse=1 --- pre_commit/staged_files_only.py | 10 ++++++-- tests/staged_files_only_test.py | 43 +++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/pre_commit/staged_files_only.py b/pre_commit/staged_files_only.py index 48cc10299..bad004cd1 100644 --- a/pre_commit/staged_files_only.py +++ b/pre_commit/staged_files_only.py @@ -13,6 +13,12 @@ logger = logging.getLogger('pre_commit') +# without forcing submodule.recurse=0, changes in nested submodules will be +# discarded if `submodule.recurse=1` is configured +# we choose this instead of `--no-recurse-submodules` because it works on +# versions of git before that option was added to `git checkout` +_CHECKOUT_CMD = ('git', '-c', 'submodule.recurse=0', 'checkout', '--', '.') + def _git_apply(patch: str) -> None: args = ('apply', '--whitespace=nowarn', patch) @@ -58,7 +64,7 @@ def _unstaged_changes_cleared(patch_dir: str) -> Generator[None, None, None]: # prevent recursive post-checkout hooks (#1418) no_checkout_env = dict(os.environ, _PRE_COMMIT_SKIP_POST_CHECKOUT='1') - cmd_output_b('git', 'checkout', '--', '.', env=no_checkout_env) + cmd_output_b(*_CHECKOUT_CMD, env=no_checkout_env) try: yield @@ -74,7 +80,7 @@ def _unstaged_changes_cleared(patch_dir: str) -> Generator[None, None, None]: # We failed to apply the patch, presumably due to fixes made # by hooks. # Roll back the changes made by hooks. - cmd_output_b('git', 'checkout', '--', '.', env=no_checkout_env) + cmd_output_b(*_CHECKOUT_CMD, env=no_checkout_env) _git_apply(patch_filename) logger.info(f'Restored changes from {patch_filename}.') diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py index ddb957435..2e3f62091 100644 --- a/tests/staged_files_only_test.py +++ b/tests/staged_files_only_test.py @@ -181,9 +181,11 @@ def test_img_conflict(img_staged, patch_dir): @pytest.fixture -def submodule_with_commits(tempdir_factory): +def repo_with_commits(tempdir_factory): path = git_dir(tempdir_factory) with cwd(path): + open('foo', 'a+').close() + cmd_output('git', 'add', 'foo') git_commit() rev1 = cmd_output('git', 'rev-parse', 'HEAD')[1].strip() git_commit() @@ -196,18 +198,21 @@ def checkout_submodule(rev): @pytest.fixture -def sub_staged(submodule_with_commits, tempdir_factory): +def sub_staged(repo_with_commits, tempdir_factory): path = git_dir(tempdir_factory) with cwd(path): + open('bar', 'a+').close() + cmd_output('git', 'add', 'bar') + git_commit() cmd_output( - 'git', 'submodule', 'add', submodule_with_commits.path, 'sub', + 'git', 'submodule', 'add', repo_with_commits.path, 'sub', ) - checkout_submodule(submodule_with_commits.rev1) + checkout_submodule(repo_with_commits.rev1) cmd_output('git', 'add', 'sub') yield auto_namedtuple( path=path, sub_path=os.path.join(path, 'sub'), - submodule=submodule_with_commits, + submodule=repo_with_commits, ) @@ -242,6 +247,34 @@ def test_sub_something_unstaged(sub_staged, patch_dir): _test_sub_state(sub_staged, 'rev2', 'AM') +def test_submodule_does_not_discard_changes(sub_staged, patch_dir): + with open('bar', 'w') as f: + f.write('unstaged changes') + + foo_path = os.path.join(sub_staged.sub_path, 'foo') + with open(foo_path, 'w') as f: + f.write('foo contents') + + with staged_files_only(patch_dir): + with open('bar') as f: + assert f.read() == '' + + with open(foo_path) as f: + assert f.read() == 'foo contents' + + with open('bar') as f: + assert f.read() == 'unstaged changes' + + with open(foo_path) as f: + assert f.read() == 'foo contents' + + +def test_submodule_does_not_discard_changes_recurse(sub_staged, patch_dir): + cmd_output('git', 'config', 'submodule.recurse', '1', cwd=sub_staged.path) + + test_submodule_does_not_discard_changes(sub_staged, patch_dir) + + def test_stage_utf8_changes(foo_staged, patch_dir): contents = '\u2603' with open('foo', 'w', encoding='UTF-8') as foo_file: