diff --git a/hooks/pre-commit.py b/hooks/pre-commit.py index b3970ef2d7..742f20fff1 100644 --- a/hooks/pre-commit.py +++ b/hooks/pre-commit.py @@ -17,6 +17,7 @@ import git_common from gclient_eval import SYNC SKIP_VAR = 'SKIP_GITLINK_PRECOMMIT' +TESTING_ANSWER = 'TESTING_ANSWER' def main(): @@ -55,15 +56,39 @@ def main(): # DEPS only has to be in sync with gitlinks when state is SYNC. exit(0) - print(f'Found no change to DEPS, unstaging {len(staged_gitlinks)} ' - f'staged gitlink(s) found in diff:\n{diff}') - git_common.run('restore', '--staged', '--', *staged_gitlinks) + prompt = ( + f'Found no change to DEPS, but found staged gitlink(s) in diff:\n{diff}\n' + 'Press Enter/Return if you intended to include them or "n" to unstage ' + '(exclude from commit) the gitlink(s): ') + print(prompt) + + if os.getenv(TESTING_ANSWER) is not None: + answer = os.getenv(TESTING_ANSWER) + else: + try: + sys.stdin = open("/dev/tty", "r") + except (FileNotFoundError, OSError): + try: + sys.stdin = open('CON') + except: + print( + 'Unable to acquire input handle, proceeding without modifications' + ) + exit(0) + answer = input() disable_msg = f'To disable this hook, set {SKIP_VAR}=1' - if len(staged_gitlinks) == len(diff.splitlines()): - print('Found no changes after unstaging gitlinks, aborting commit.') - print(disable_msg) - exit(1) + if answer.lower() == 'n': + print( + f'\nUnstaging {len(staged_gitlinks)} staged gitlink(s) found in diff' + ) + git_common.run('restore', '--staged', '--', *staged_gitlinks) + if len(staged_gitlinks) == len(diff.splitlines()): + print( + '\nFound no changes after unstaging gitlinks, aborting commit.') + print(disable_msg) + exit(1) + print(disable_msg) diff --git a/tests/hooks_test.py b/tests/hooks_test.py index e2f9d3fa3d..1fc7e774e6 100755 --- a/tests/hooks_test.py +++ b/tests/hooks_test.py @@ -9,6 +9,7 @@ import sys import tempfile import unittest import unittest.mock +from unittest.mock import patch ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) sys.path.insert(0, ROOT_DIR) @@ -25,6 +26,7 @@ class HooksTest(unittest.TestCase): self.repo = tempfile.mkdtemp() self.env = os.environ.copy() self.env['SKIP_GITLINK_PRECOMMIT'] = '0' + self.env['TESTING_ANSWER'] = 'n' self.populate() def tearDown(self): @@ -118,12 +120,15 @@ class HooksTest(unittest.TestCase): _, stderr = git.run_with_stderr('commit', '-m', 'regular file and gitlinks', - cwd=self.repo) + cwd=self.repo, + env=self.env) self.assertIn('dep_a', diff_before_commit) self.assertIn('dep_b', diff_before_commit) # Gitlinks should be dropped. - self.assertIn('unstaging 2 staged gitlink(s)', stderr) + self.assertIn( + 'Found no change to DEPS, but found staged gitlink(s) in diff', + stderr) diff_after_commit = git.run('diff', '--name-only', 'HEAD^', @@ -133,6 +138,46 @@ class HooksTest(unittest.TestCase): self.assertNotIn('dep_b', diff_after_commit) self.assertIn('foo', diff_after_commit) + def testPreCommit_IntentionalGitlinkWithoutDEPS(self): + # Intentional Gitlink changes staged without a DEPS change. + self.write(self.repo, 'foo', 'foo') + git.run('add', '.', cwd=self.repo) + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"b"*40},dep_a', + cwd=self.repo) + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"a"*40},dep_b', + cwd=self.repo) + diff_before_commit = git.run('diff', + '--cached', + '--name-only', + cwd=self.repo) + self.env['TESTING_ANSWER'] = '' + _, stderr = git.run_with_stderr('commit', + '-m', + 'regular file and gitlinks', + cwd=self.repo, + env=self.env) + + self.assertIn('dep_a', diff_before_commit) + self.assertIn('dep_b', diff_before_commit) + # Gitlinks should be dropped. + self.assertIn( + 'Found no change to DEPS, but found staged gitlink(s) in diff', + stderr) + diff_after_commit = git.run('diff', + '--name-only', + 'HEAD^', + 'HEAD', + cwd=self.repo) + self.assertIn('dep_a', diff_after_commit) + self.assertIn('dep_b', diff_after_commit) + self.assertIn('foo', diff_after_commit) + def testPreCommit_OnlyGitlinkWithoutDEPS(self): # Gitlink changes were staged without a corresponding DEPS change but # no other files were included in the commit. @@ -148,7 +193,8 @@ class HooksTest(unittest.TestCase): ret = git.run_with_retcode('commit', '-m', 'gitlink only', - cwd=self.repo) + cwd=self.repo, + env=self.env) self.assertIn('dep_a', diff_before_commit) # Gitlinks should be droppped and the empty commit should be aborted. @@ -178,7 +224,8 @@ class HooksTest(unittest.TestCase): '--all', '-m', 'commit all', - cwd=self.repo) + cwd=self.repo, + env=self.env) self.assertIn('dep_a', diff_before_commit) self.assertEqual(ret, 0)