diff --git a/gclient.py b/gclient.py index 659fdb482..6a47709a9 100755 --- a/gclient.py +++ b/gclient.py @@ -128,6 +128,8 @@ PREVIOUS_SYNC_COMMITS = 'GCLIENT_PREVIOUS_SYNC_COMMITS' NO_SYNC_EXPERIMENT = 'no-sync' +PRECOMMIT_HOOK_VAR = 'GCLIENT_PRECOMMIT' + class GNException(Exception): pass @@ -1982,6 +1984,46 @@ it or fix the checkout. patch_refs[patch_repo] = patch_ref return patch_refs, target_branches + def _InstallPreCommitHook(self): + # On Windows, this path is written to the file as + # "dir\hooks\pre-commit.py" but it gets interpreted as + # "dirhookspre-commit.py". + gclient_hook_path = os.path.join(DEPOT_TOOLS_DIR, 'hooks', + 'pre-commit.py').replace('\\', '\\\\') + gclient_hook_content = '\n'.join(( + f'{PRECOMMIT_HOOK_VAR}={gclient_hook_path}', + f'if [ -f "${PRECOMMIT_HOOK_VAR}" ]; then', + f' python3 "${PRECOMMIT_HOOK_VAR}" || exit 1', + 'fi', + )) + + soln = gclient_paths.GetPrimarySolutionPath() + if not soln: + print('Could not find gclient solution.') + return + + git_dir = os.path.join(soln, '.git') + if not os.path.exists(git_dir): + return + + hook = os.path.join(git_dir, 'hooks', 'pre-commit') + if os.path.exists(hook): + with open(hook, 'r') as f: + content = f.read() + if PRECOMMIT_HOOK_VAR in content: + print(f'{hook} already contains the gclient pre-commit hook.') + else: + print(f'A pre-commit hook already exists at {hook}.\n' + f'Please append the following lines to the hook:\n\n' + f'{gclient_hook_content}') + return + + print(f'Creating a pre-commit hook at {hook}') + with open(hook, 'w') as f: + f.write('#!/bin/sh\n') + f.write(f'{gclient_hook_content}\n') + os.chmod(hook, 0o755) + def _RemoveUnversionedGitDirs(self): """Remove directories that are no longer part of the checkout. @@ -3552,6 +3594,23 @@ def CMDrunhooks(parser, args): return client.RunOnDeps('runhooks', args) +# TODO(crbug.com/1481266): Collect merics for installhooks. +def CMDinstallhooks(parser, args): + """Installs gclient git hooks. + + Currently only installs a pre-commit hook to drop staged gitlinks. To + bypass this pre-commit hook once it's installed, set the environment + variable SKIP_GITLINK_PRECOMMIT=1. + """ + (options, args) = parser.parse_args(args) + client = GClient.LoadCurrentConfig(options) + if not client: + raise gclient_utils.Error( + 'client not configured; see \'gclient config\'') + client._InstallPreCommitHook() + return 0 + + @metrics.collector.collect_metrics('gclient revinfo') def CMDrevinfo(parser, args): """Outputs revision info mapping for the client and its dependencies. diff --git a/hooks/pre-commit.py b/hooks/pre-commit.py new file mode 100644 index 000000000..b3970ef2d --- /dev/null +++ b/hooks/pre-commit.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. +"""A git pre-commit hook to drop staged gitlink changes. + +To bypass this hook, set SKIP_GITLINK_PRECOMMIT=1. +""" + +import os +import sys + +ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, ROOT_DIR) + +import git_common +from gclient_eval import SYNC + +SKIP_VAR = 'SKIP_GITLINK_PRECOMMIT' + + +def main(): + if os.getenv(SKIP_VAR) == '1': + print(f'{SKIP_VAR} is set. Committing gitlinks, if any.') + exit(0) + + has_deps_diff = False + staged_gitlinks = [] + diff = git_common.run('diff-index', '--cached', 'HEAD') + for line in diff.splitlines(): + path = line.split()[-1] + if path == 'DEPS': + has_deps_diff = True + continue + if line.startswith(':160000 160000'): + staged_gitlinks.append(path) + + if not staged_gitlinks or has_deps_diff: + exit(0) + + # There are staged gitlinks and DEPS wasn't changed. Get git_dependencies + # migration state in DEPS. + state = None + try: + with open('DEPS', 'r') as f: + for l in f.readlines(): + if l.startswith('git_dependencies'): + state = l.split()[-1].strip(' "\'') + break + except OSError: + # Don't abort the commit if DEPS wasn't found. + exit(0) + + if state != SYNC: + # 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) + + 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) + print(disable_msg) + + +if __name__ == "__main__": + main() diff --git a/tests/gclient_git_smoketest.py b/tests/gclient_git_smoketest.py index 7abbcb42c..e4c8edf79 100755 --- a/tests/gclient_git_smoketest.py +++ b/tests/gclient_git_smoketest.py @@ -387,6 +387,41 @@ class GClientSmokeGIT(gclient_smoketest_base.GClientSmokeBase): self.githash('repo_2', 1), self.gitrevparse(os.path.join(self.root_dir, 'src/repo2'))) + def testInstallHooks_no_existing_hook(self): + repo = os.path.join(self.git_base, 'repo_18') + self.gclient(['config', repo, '--name', 'src'], cwd=repo) + self.gclient(['sync'], cwd=repo) + self.gclient(['installhooks'], cwd=repo) + + hook = os.path.join(repo, 'src', '.git', 'hooks', 'pre-commit') + with open(hook) as f: + contents = f.read() + self.assertIn('python3 "$GCLIENT_PRECOMMIT"', contents) + + # Later runs should only inform that the hook was already installed. + stdout, _, _ = self.gclient(['installhooks'], cwd=repo) + self.assertIn(f'{hook} already contains the gclient pre-commit hook.', + stdout) + + def testInstallHooks_existing_hook(self): + repo = os.path.join(self.git_base, 'repo_19') + self.gclient(['config', repo, '--name', 'src'], cwd=repo) + self.gclient(['sync'], cwd=repo) + + # Create an existing pre-commit hook. + hook = os.path.join(repo, 'src', '.git', 'hooks', 'pre-commit') + hook_contents = ['#!/bin/sh', 'echo hook'] + with open(hook, 'w') as f: + f.write('\n'.join(hook_contents)) + + stdout, _, _ = self.gclient(['installhooks'], cwd=repo) + self.assertIn('Please append the following lines to the hook', stdout) + + # The orignal hook is left unchanged. + with open(hook) as f: + contents = f.read().splitlines() + self.assertEqual(hook_contents, contents) + def testRunHooks(self): self.gclient(['config', self.git_base + 'repo_1', '--name', 'src']) self.gclient(['sync', '--deps', 'mac']) diff --git a/tests/hooks_test.py b/tests/hooks_test.py new file mode 100755 index 000000000..e2f9d3fa3 --- /dev/null +++ b/tests/hooks_test.py @@ -0,0 +1,275 @@ +#!/usr/bin/env python3 +# Copyright (c) 2023 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import os +import os.path +import sys +import tempfile +import unittest +import unittest.mock + +ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +sys.path.insert(0, ROOT_DIR) + +from gclient import PRECOMMIT_HOOK_VAR +import gclient_utils +from gclient_eval import SYNC, SUBMODULES +import git_common as git + + +class HooksTest(unittest.TestCase): + def setUp(self): + super(HooksTest, self).setUp() + self.repo = tempfile.mkdtemp() + self.env = os.environ.copy() + self.env['SKIP_GITLINK_PRECOMMIT'] = '0' + self.populate() + + def tearDown(self): + gclient_utils.rmtree(self.repo) + + def write(self, repo, path, content): + with open(os.path.join(repo, path), 'w') as f: + f.write(content) + + def populate(self): + git.run('init', cwd=self.repo) + deps_content = '\n'.join(( + f'git_dependencies = "{SYNC}"', + 'deps = {', + f' "dep_a": "host://dep_a@{"a"*40}",', + f' "dep_b": "host://dep_b@{"b"*40}",', + '}', + )) + self.write(self.repo, 'DEPS', deps_content) + + self.dep_a_repo = os.path.join(self.repo, 'dep_a') + os.mkdir(self.dep_a_repo) + git.run('init', cwd=self.dep_a_repo) + os.mkdir(os.path.join(self.repo, 'dep_b')) + gitmodules_content = '\n'.join(( + '[submodule "dep_a"]' + '\tpath = dep_a', + '\turl = host://dep_a', + '[submodule "dep_b"]' + '\tpath = dep_b', + '\turl = host://dep_b', + )) + self.write(self.repo, '.gitmodules', gitmodules_content) + git.run('update-index', + '--add', + '--cacheinfo', + f'160000,{"a"*40},dep_a', + cwd=self.repo) + git.run('update-index', + '--add', + '--cacheinfo', + f'160000,{"b"*40},dep_b', + cwd=self.repo) + + git.run('add', '.', cwd=self.repo) + git.run('commit', '-m', 'init', cwd=self.repo) + + # On Windows, this path is written to the file as + # "root_dir\hooks\pre-commit.py", but it gets interpreted as + # "root_dirhookspre-commit.py". + precommit_path = os.path.join(ROOT_DIR, 'hooks', + 'pre-commit.py').replace('\\', '\\\\') + precommit_content = '\n'.join(( + '#!/bin/sh', + f'{PRECOMMIT_HOOK_VAR}={precommit_path}', + f'if [ -f "${PRECOMMIT_HOOK_VAR}" ]; then', + f' python3 "${PRECOMMIT_HOOK_VAR}" || exit 1', + 'fi', + )) + self.write(self.repo, os.path.join('.git', 'hooks', 'pre-commit'), + precommit_content) + os.chmod(os.path.join(self.repo, '.git', 'hooks', 'pre-commit'), 0o755) + + def testPreCommit_NoGitlinkOrDEPS(self): + # Sanity check. Neither gitlinks nor DEPS are touched. + self.write(self.repo, 'foo', 'foo') + git.run('add', '.', cwd=self.repo) + expected_diff = git.run('diff', '--cached', cwd=self.repo) + git.run('commit', '-m', 'foo', cwd=self.repo) + self.assertEqual(expected_diff, + git.run('diff', 'HEAD^', 'HEAD', cwd=self.repo)) + + def testPreCommit_GitlinkWithoutDEPS(self): + # Gitlink changes were staged without a corresponding 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) + _, stderr = git.run_with_stderr('commit', + '-m', + 'regular file and gitlinks', + cwd=self.repo) + + 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) + diff_after_commit = git.run('diff', + '--name-only', + 'HEAD^', + 'HEAD', + cwd=self.repo) + self.assertNotIn('dep_a', diff_after_commit) + self.assertNotIn('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. + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"b"*40},dep_a', + cwd=self.repo) + diff_before_commit = git.run('diff', + '--cached', + '--name-only', + cwd=self.repo) + ret = git.run_with_retcode('commit', + '-m', + 'gitlink only', + cwd=self.repo) + + self.assertIn('dep_a', diff_before_commit) + # Gitlinks should be droppped and the empty commit should be aborted. + self.assertEqual(ret, 1) + diff_after_commit = git.run('diff', + '--cached', + '--name-only', + cwd=self.repo) + self.assertNotIn('dep_a', diff_after_commit) + + def testPreCommit_CommitAll(self): + self.write(self.repo, 'foo', 'foo') + git.run('add', '.', cwd=self.repo) + git.run('commit', '-m', 'add foo', cwd=self.repo) + self.write(self.repo, 'foo', 'foo2') + + # Create a new commit in dep_a. + self.write(self.dep_a_repo, 'sub_foo', 'sub_foo') + git.run('add', '.', cwd=self.dep_a_repo) + git.run('commit', '-m', 'sub_foo', cwd=self.dep_a_repo) + + diff_before_commit = git.run('status', + cwd=self.repo) + self.assertIn('foo', diff_before_commit) + self.assertIn('dep_a', diff_before_commit) + ret = git.run_with_retcode('commit', + '--all', + '-m', + 'commit all', + cwd=self.repo) + + self.assertIn('dep_a', diff_before_commit) + self.assertEqual(ret, 0) + diff_after_commit = git.run('diff', + '--cached', + '--name-only', + cwd=self.repo) + self.assertNotIn('dep_a', diff_after_commit) + diff_from_commit = git.run('diff', + '--name-only', + 'HEAD^', + 'HEAD', + cwd=self.repo) + self.assertIn('foo', diff_from_commit) + + def testPreCommit_GitlinkWithDEPS(self): + # A gitlink was staged with a corresponding DEPS change. + updated_deps = '\n'.join(( + f'git_dependencies = "{SYNC}"', + 'deps = {', + f' "dep_a": "host://dep_a@{"b"*40}",', + f' "dep_b": "host://dep_b@{"b"*40}",', + '}', + )) + self.write(self.repo, 'DEPS', updated_deps) + git.run('add', '.', cwd=self.repo) + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"b"*40},dep_a', + cwd=self.repo) + diff_before_commit = git.run('diff', '--cached', cwd=self.repo) + git.run('commit', '-m', 'gitlink and DEPS', cwd=self.repo) + + # There should be no changes to the commit. + diff_after_commit = git.run('diff', 'HEAD^', 'HEAD', cwd=self.repo) + self.assertEqual(diff_before_commit, diff_after_commit) + + def testPreCommit_SkipPrecommit(self): + # A gitlink was staged without a corresponding DEPS change but the + # SKIP_GITLINK_PRECOMMIT envvar was set. + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"b"*40},dep_a', + cwd=self.repo) + diff_before_commit = git.run('diff', + '--cached', + '--name-only', + cwd=self.repo) + self.env['SKIP_GITLINK_PRECOMMIT'] = '1' + git.run('commit', + '-m', + 'gitlink only, skipping precommit', + cwd=self.repo, + env=self.env) + + # Gitlink should be kept. + self.assertIn('dep_a', diff_before_commit) + diff_after_commit = git.run('diff', + '--name-only', + 'HEAD^', + 'HEAD', + cwd=self.repo) + self.assertIn('dep_a', diff_after_commit) + + def testPreCommit_OtherDEPSState(self): + # DEPS is set to a git_dependencies state other than SYNC. + deps_content = '\n'.join(( + f'git_dependencies = \'{SUBMODULES}\'', + 'deps = {', + f' "dep_a": "host://dep_a@{"a"*40}",', + f' "dep_b": "host://dep_b@{"b"*40}",', + '}', + )) + self.write(self.repo, 'DEPS', deps_content) + git.run('add', '.', cwd=self.repo) + git.run('commit', '-m', 'change git_dependencies', cwd=self.repo) + + git.run('update-index', + '--replace', + '--cacheinfo', + f'160000,{"b"*40},dep_a', + cwd=self.repo) + diff_before_commit = git.run('diff', '--cached', cwd=self.repo) + git.run('commit', '-m', 'update dep_a', cwd=self.repo) + + # There should be no changes to the commit. + diff_after_commit = git.run('diff', 'HEAD^', 'HEAD', cwd=self.repo) + self.assertEqual(diff_before_commit, diff_after_commit) + + +if __name__ == '__main__': + unittest.main()