From ac735e85e28a9af1622aac7841dc1fbf719ceafd Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 16 Jun 2014 17:33:25 -0700 Subject: [PATCH] Make install -f / --overwrite work. --- pre_commit/commands/install.py | 16 +++++++++-- pre_commit/main.py | 10 +++++-- pre_commit/runner.py | 9 +++++- tests/commands/install_test.py | 52 ++++++++++++++++++++++++++++++++-- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/pre_commit/commands/install.py b/pre_commit/commands/install.py index 606cfd25..242d2e83 100644 --- a/pre_commit/commands/install.py +++ b/pre_commit/commands/install.py @@ -7,6 +7,7 @@ import os.path import pkg_resources import stat + # This is used to identify the hook file we install IDENTIFYING_HASH = 'd8ee923c46731b42cd95cc869add4062' @@ -23,7 +24,7 @@ def make_executable(filename): ) -def install(runner): +def install(runner, overwrite=False): """Install the pre-commit hooks.""" pre_commit_file = pkg_resources.resource_filename( 'pre_commit', 'resources/pre-commit-hook', @@ -34,7 +35,18 @@ def install(runner): os.path.exists(runner.pre_commit_path) and not is_our_pre_commit(runner.pre_commit_path) ): - os.rename(runner.pre_commit_path, runner.pre_commit_path + '.legacy') + os.rename(runner.pre_commit_path, runner.pre_commit_legacy_path) + + # If we specify overwrite, we simply delete the legacy file + if overwrite and os.path.exists(runner.pre_commit_legacy_path): + os.remove(runner.pre_commit_legacy_path) + elif os.path.exists(runner.pre_commit_legacy_path): + print( + 'Running in migration mode with existing hooks at {0}\n' + 'Use -f to use only pre-commit.'.format( + runner.pre_commit_legacy_path, + ) + ) with open(runner.pre_commit_path, 'w') as pre_commit_file_obj: pre_commit_file_obj.write(open(pre_commit_file).read()) diff --git a/pre_commit/main.py b/pre_commit/main.py index e1e8ff73..af8f2a13 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -28,7 +28,13 @@ def main(argv): subparsers = parser.add_subparsers(dest='command') - subparsers.add_parser('install', help='Intall the pre-commit script.') + install_parser = subparsers.add_parser( + 'install', help='Intall the pre-commit script.', + ) + install_parser.add_argument( + '-f', '--overwrite', action='store_true', + help='Overwrite existing hooks / remove migration mode.', + ) subparsers.add_parser('uninstall', help='Uninstall the pre-commit script.') @@ -67,7 +73,7 @@ def main(argv): runner = Runner.create() if args.command == 'install': - return install(runner) + return install(runner, overwrite=args.overwrite) elif args.command == 'uninstall': return uninstall(runner) elif args.command == 'clean': diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 1768a336..bbba62c7 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -41,7 +41,14 @@ class Runner(object): @cached_property def pre_commit_path(self): - return os.path.join(self.git_root, '.git/hooks/pre-commit') + return os.path.join(self.git_root, '.git', 'hooks', 'pre-commit') + + @cached_property + def pre_commit_legacy_path(self): + """The path in the 'hooks' directory representing the temporary + storage for existing pre-commit hooks. + """ + return self.pre_commit_path + '.legacy' @cached_property def cmd_runner(self): diff --git a/tests/commands/install_test.py b/tests/commands/install_test.py index 00bca826..44842774 100644 --- a/tests/commands/install_test.py +++ b/tests/commands/install_test.py @@ -156,7 +156,28 @@ def test_install_existing_hooks_no_overwrite(tmpdir_factory): assert EXISTING_COMMIT_RUN.match(output) # Now install pre-commit (no-overwrite) - assert install(Runner(path)) == 0 + assert install(runner) == 0 + + # We should run both the legacy and pre-commit hooks + ret, output = _get_commit_output(tmpdir_factory) + assert ret == 0 + assert output.startswith('legacy hook\n') + assert NORMAL_PRE_COMMIT_RUN.match(output[len('legacy hook\n'):]) + + +def test_install_existing_hook_no_overwrite_idempotent(tmpdir_factory): + path = make_consuming_repo(tmpdir_factory, 'script_hooks_repo') + with local.cwd(path): + runner = Runner(path) + + # Write out an "old" hook + with io.open(runner.pre_commit_path, 'w') as hook_file: + hook_file.write('#!/usr/bin/env bash\necho "legacy hook"\n') + make_executable(runner.pre_commit_path) + + # Install twice + assert install(runner) == 0 + assert install(runner) == 0 # We should run both the legacy and pre-commit hooks ret, output = _get_commit_output(tmpdir_factory) @@ -184,9 +205,36 @@ def test_failing_existing_hook_returns_1(tmpdir_factory): hook_file.write('#!/usr/bin/env bash\necho "fail!"\nexit 1\n') make_executable(runner.pre_commit_path) - assert install(Runner(path)) == 0 + assert install(runner) == 0 # We should get a failure from the legacy hook ret, output = _get_commit_output(tmpdir_factory) assert ret == 1 assert FAIL_OLD_HOOK.match(output) + + +def test_install_overwrite_no_existing_hooks(tmpdir_factory): + path = make_consuming_repo(tmpdir_factory, 'script_hooks_repo') + with local.cwd(path): + assert install(Runner(path), overwrite=True) == 0 + + ret, output = _get_commit_output(tmpdir_factory) + assert ret == 0 + assert NORMAL_PRE_COMMIT_RUN.match(output) + + +def test_install_overwrite(tmpdir_factory): + path = make_consuming_repo(tmpdir_factory, 'script_hooks_repo') + with local.cwd(path): + runner = Runner(path) + + # Write out the "old" hook + with io.open(runner.pre_commit_path, 'w') as hook_file: + hook_file.write('#!/usr/bin/env bash\necho "legacy hook"\n') + make_executable(runner.pre_commit_path) + + assert install(runner, overwrite=True) == 0 + + ret, output = _get_commit_output(tmpdir_factory) + assert ret == 0 + assert NORMAL_PRE_COMMIT_RUN.match(output)