From b707cbba06c3d08c8aac31c460ffc7156765e25c Mon Sep 17 00:00:00 2001 From: dongweiming Date: Sun, 11 Jan 2015 22:40:35 +0800 Subject: [PATCH] Make pre_commit also support pre-push hook --- pre_commit/commands/install_uninstall.py | 66 ++++++++++++------- pre_commit/commands/run.py | 19 +++++- pre_commit/main.py | 24 ++++++- .../resources/{pre-commit-hook => hook-tmpl} | 14 ++-- pre_commit/resources/pre-push-tmpl | 12 ++++ pre_commit/runner.py | 29 +++++++- setup.py | 3 +- tests/commands/install_uninstall_test.py | 39 +++++++++-- tests/commands/run_test.py | 34 ++++++++++ tests/runner_test.py | 29 ++++++++ 10 files changed, 227 insertions(+), 42 deletions(-) rename pre_commit/resources/{pre-commit-hook => hook-tmpl} (75%) create mode 100755 pre_commit/resources/pre-push-tmpl diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 1c23739d..61c8e062 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -9,7 +9,6 @@ import stat import sys from pre_commit.logging_handler import LoggingHandler -from pre_commit.util import resource_filename logger = logging.getLogger('pre_commit') @@ -42,37 +41,55 @@ def make_executable(filename): ) -def install(runner, overwrite=False, hooks=False): +def get_hook_path(runner, hook_type): + if hook_type == 'pre-commit': + hook_path = runner.pre_commit_path + legacy_path = runner.pre_commit_legacy_path + else: + hook_path = runner.pre_push_path + legacy_path = runner.pre_push_legacy_path + return hook_path, legacy_path + + +def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): """Install the pre-commit hooks.""" - pre_commit_file = resource_filename('pre-commit-hook') + hook_path, legacy_path = get_hook_path(runner, hook_type) # If we have an existing hook, move it to pre-commit.legacy if ( - os.path.exists(runner.pre_commit_path) and - not is_our_pre_commit(runner.pre_commit_path) and - not is_previous_pre_commit(runner.pre_commit_path) + os.path.exists(hook_path) and + not is_our_pre_commit(hook_path) and + not is_previous_pre_commit(hook_path) ): - os.rename(runner.pre_commit_path, runner.pre_commit_legacy_path) + os.rename(hook_path, 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): + if overwrite and os.path.exists(legacy_path): + os.remove(legacy_path) + elif os.path.exists(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, + legacy_path, ) ) - with io.open(runner.pre_commit_path, 'w') as pre_commit_file_obj: - contents = io.open(pre_commit_file).read().format( + with io.open(hook_path, 'w') as pre_commit_file_obj: + if hook_type == 'pre-push': + with io.open(runner.pre_push_template) as fp: + pre_push_contents = fp.read() + else: + pre_push_contents = '' + + contents = io.open(runner.pre_template).read().format( sys_executable=sys.executable, + hook_type=hook_type, + pre_push=pre_push_contents, ) pre_commit_file_obj.write(contents) - make_executable(runner.pre_commit_path) + make_executable(hook_path) - print('pre-commit installed at {0}'.format(runner.pre_commit_path)) + print('pre-commit installed at {0}'.format(hook_path)) # If they requested we install all of the hooks, do so. if hooks: @@ -85,22 +102,23 @@ def install(runner, overwrite=False, hooks=False): return 0 -def uninstall(runner): +def uninstall(runner, hook_type='pre-commit'): """Uninstall the pre-commit hooks.""" + hook_path, legacy_path = get_hook_path(runner, hook_type) # If our file doesn't exist or it isn't ours, gtfo. if ( - not os.path.exists(runner.pre_commit_path) or ( - not is_our_pre_commit(runner.pre_commit_path) and - not is_previous_pre_commit(runner.pre_commit_path) + not os.path.exists(hook_path) or ( + not is_our_pre_commit(hook_path) and + not is_previous_pre_commit(hook_path) ) ): return 0 - os.remove(runner.pre_commit_path) - print('pre-commit uninstalled') + os.remove(hook_path) + print('{0} uninstalled'.format(hook_type)) - if os.path.exists(runner.pre_commit_legacy_path): - os.rename(runner.pre_commit_legacy_path, runner.pre_commit_path) - print('Restored previous hooks to {0}'.format(runner.pre_commit_path)) + if os.path.exists(legacy_path): + os.rename(legacy_path, hook_path) + print('Restored previous hooks to {0}'.format(hook_path)) return 0 diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 6f046e1a..2817a533 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -11,6 +11,7 @@ from pre_commit.logging_handler import LoggingHandler from pre_commit.output import get_hook_message from pre_commit.output import sys_stdout_write_wrapper from pre_commit.staged_files_only import staged_files_only +from pre_commit.util import cmd_output from pre_commit.util import noop_context @@ -48,8 +49,20 @@ def _print_user_skipped(hook, write, args): )) +def get_changed_files(new, old): + changed_files = cmd_output( + 'git', 'diff', '--name-only', '{0}..{1}'.format(old, new), + )[1].splitlines() + for f in changed_files: + if f: + yield f + + def _run_single_hook(runner, repository, hook, args, write, skips=set()): - if args.files: + if args.origin and args.source: + get_filenames = git.get_files_matching( + lambda: get_changed_files(args.origin, args.source)) + elif args.files: get_filenames = git.get_files_matching(lambda: args.files) elif args.all_files: get_filenames = git.get_all_files_matching @@ -137,6 +150,10 @@ def run(runner, args, write=sys_stdout_write_wrapper, environ=os.environ): if _has_unmerged_paths(runner): logger.error('Unmerged files. Resolve before committing.') return 1 + if (args.source and not args.origin) or \ + (args.origin and not args.source): + logger.error('--origin and --source depend on each other.') + return 1 # Don't stash if specified or files are specified if args.no_stash or args.all_files or args.files: diff --git a/pre_commit/main.py b/pre_commit/main.py index e4bae2b9..6cf6bd33 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -44,8 +44,18 @@ def main(argv=None): 'in the config file.' ), ) + install_parser.add_argument( + '-t', '--hook-type', choices=('pre-commit', 'pre-push'), + default='pre-commit', + ) - subparsers.add_parser('uninstall', help='Uninstall the pre-commit script.') + uninstall_parser = subparsers.add_parser( + 'uninstall', help='Uninstall the pre-commit script.', + ) + uninstall_parser.add_argument( + '-t', '--hook-type', choices=('pre-commit', 'pre-push'), + default='pre-commit', + ) subparsers.add_parser('clean', help='Clean out pre-commit files.') @@ -67,6 +77,15 @@ def main(argv=None): run_parser.add_argument( '--verbose', '-v', action='store_true', default=False, ) + + run_parser.add_argument( + '--origin', '-o', default='', + help='The origin branch"s commit_id when using `git push`', + ) + run_parser.add_argument( + '--source', '-s', default='', + help='The remote branch"s commit_id when using `git push`', + ) run_mutex_group = run_parser.add_mutually_exclusive_group(required=False) run_mutex_group.add_argument( '--all-files', '-a', action='store_true', default=False, @@ -98,9 +117,10 @@ def main(argv=None): if args.command == 'install': return install( runner, overwrite=args.overwrite, hooks=args.install_hooks, + hook_type=args.hook_type, ) elif args.command == 'uninstall': - return uninstall(runner) + return uninstall(runner, hook_type=args.hook_type) elif args.command == 'clean': return clean(runner) elif args.command == 'autoupdate': diff --git a/pre_commit/resources/pre-commit-hook b/pre_commit/resources/hook-tmpl similarity index 75% rename from pre_commit/resources/pre-commit-hook rename to pre_commit/resources/hook-tmpl index b004d29a..9583f893 100755 --- a/pre_commit/resources/pre-commit-hook +++ b/pre_commit/resources/hook-tmpl @@ -7,6 +7,7 @@ HERE=`pwd` popd > /dev/null retv=0 +args="" ENV_PYTHON='{sys_executable}' @@ -23,29 +24,30 @@ if (( (ENV_PYTHON_RETV != 0) && (PYTHON_RETV != 0) )); then - echo '`pre-commit` not found. Did you forget to activate your virtualenv?' + echo '`{hook_type}` not found. Did you forget to activate your virtualenv?' exit 1 fi # Run the legacy pre-commit if it exists -if [ -x "$HERE"/pre-commit.legacy ]; then - "$HERE"/pre-commit.legacy +if [ -x "$HERE"/{hook_type}.legacy ]; then + "$HERE"/{hook_type}.legacy if [ $? -ne 0 ]; then retv=1 fi fi +{pre_push} # Run pre-commit if ((WHICH_RETV == 0)); then - pre-commit + pre-commit $args PRE_COMMIT_RETV=$? elif ((ENV_PYTHON_RETV == 0)); then - "$ENV_PYTHON" -m pre_commit.main + "$ENV_PYTHON" -m pre_commit.main $args PRE_COMMIT_RETV=$? else - python -m pre_commit.main + python -m pre_commit.main $args PRE_COMMIT_RETV=$? fi diff --git a/pre_commit/resources/pre-push-tmpl b/pre_commit/resources/pre-push-tmpl new file mode 100755 index 00000000..cfbba996 --- /dev/null +++ b/pre_commit/resources/pre-push-tmpl @@ -0,0 +1,12 @@ +z40=0000000000000000000000000000000000000000 +while read local_ref local_sha remote_ref remote_sha +do + if [ "$local_sha" != $z40 ]; then + if [ "$remote_sha" = $z40 ]; + then + args="run --all-files" + else + args="run --origin $local_sha --source $remote_sha" + fi + fi +done diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 9e9ac216..302bacb0 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -10,6 +10,7 @@ from pre_commit import git from pre_commit.clientlib.validate_config import load_config from pre_commit.repository import Repository from pre_commit.store import Store +from pre_commit.util import resource_filename class Runner(object): @@ -43,17 +44,39 @@ class Runner(object): repository.require_installed() return repositories - @cached_property - def pre_commit_path(self): - return os.path.join(self.git_root, '.git', 'hooks', 'pre-commit') + def get_hook_path(self, hook_type): + return os.path.join(self.git_root, '.git', 'hooks', hook_type) @cached_property + def pre_commit_path(self): + return self.get_hook_path('pre-commit') + + @cached_property + def pre_push_path(self): + return self.get_hook_path('pre-push') + + @cached_property + def pre_template(self): + return resource_filename('hook-tmpl') + + @cached_property + def pre_push_template(self): + return resource_filename('pre-push-tmpl') + + @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' + @property + def pre_push_legacy_path(self): + """The path in the 'hooks' directory representing the temporary + storage for existing pre-push hooks. + """ + return self.pre_push_path + '.legacy' + @cached_property def cmd_runner(self): # TODO: remove this and inline runner.store.cmd_runner diff --git a/setup.py b/setup.py index af171031..90af5bb5 100644 --- a/setup.py +++ b/setup.py @@ -30,7 +30,8 @@ setup( packages=find_packages('.', exclude=('tests*', 'testing*')), package_data={ 'pre_commit': [ - 'resources/pre-commit-hook', + 'resources/hook-tmpl', + 'resources/pre-push-tmpl', 'resources/rbenv.tar.gz', 'resources/ruby-build.tar.gz', 'resources/ruby-download.tar.gz', diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 4e41f727..9733b175 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -10,7 +10,7 @@ import subprocess import sys import mock - +from pre_commit.commands.install_uninstall import get_hook_path from pre_commit.commands.install_uninstall import IDENTIFYING_HASH from pre_commit.commands.install_uninstall import install from pre_commit.commands.install_uninstall import is_our_pre_commit @@ -22,6 +22,7 @@ from pre_commit.runner import Runner from pre_commit.util import cmd_output from pre_commit.util import cwd from pre_commit.util import resource_filename + from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo @@ -31,7 +32,7 @@ def test_is_not_our_pre_commit(): def test_is_our_pre_commit(): - assert is_our_pre_commit(resource_filename('pre-commit-hook')) + assert is_our_pre_commit(resource_filename('hook-tmpl')) def test_is_not_previous_pre_commit(): @@ -39,7 +40,7 @@ def test_is_not_previous_pre_commit(): def test_is_also_not_previous_pre_commit(): - assert not is_previous_pre_commit(resource_filename('pre-commit-hook')) + assert not is_previous_pre_commit(resource_filename('hook-tmpl')) def test_is_previous_pre_commit(in_tmpdir): @@ -56,14 +57,28 @@ def test_install_pre_commit(tmpdir_factory): assert ret == 0 assert os.path.exists(runner.pre_commit_path) pre_commit_contents = io.open(runner.pre_commit_path).read() - pre_commit_script = resource_filename('pre-commit-hook') + pre_commit_script = resource_filename('hook-tmpl') expected_contents = io.open(pre_commit_script).read().format( sys_executable=sys.executable, + hook_type='pre-commit', + pre_push='' ) assert pre_commit_contents == expected_contents stat_result = os.stat(runner.pre_commit_path) assert stat_result.st_mode & (stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + ret = install(runner, hook_type='pre-push') + assert ret == 0 + assert os.path.exists(runner.pre_push_path) + pre_push_contents = io.open(runner.pre_push_path).read() + pre_push_template_contents = io.open(runner.pre_push_template).read() + expected_contents = io.open(pre_commit_script).read().format( + sys_executable=sys.executable, + hook_type='pre-push', + pre_push=pre_push_template_contents + ) + assert pre_push_contents == expected_contents + def test_uninstall_does_not_blow_up_when_not_there(tmpdir_factory): path = git_dir(tmpdir_factory) @@ -322,7 +337,7 @@ def test_replace_old_commit_script(tmpdir_factory): # Install a script that looks like our old script pre_commit_contents = io.open( - resource_filename('pre-commit-hook'), + resource_filename('hook-tmpl'), ).read() new_contents = pre_commit_contents.replace( IDENTIFYING_HASH, PREVIOUS_IDENTIFYING_HASHES[-1], @@ -391,3 +406,17 @@ def test_installed_from_venv(tmpdir_factory): ) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) + + +def test_get_hook_path(tmpdir_factory): + path = git_dir(tmpdir_factory) + with cwd(path): + runner = Runner(path) + expected_paths = (os.path.join(path, '.git/hooks/pre-commit'), + os.path.join(path, '.git/hooks/pre-commit.legacy') + ) + assert expected_paths == get_hook_path(runner, 'pre-commit') + expected_paths = (os.path.join(path, '.git/hooks/pre-push'), + os.path.join(path, '.git/hooks/pre-push.legacy') + ) + assert expected_paths == get_hook_path(runner, 'pre-push') diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 4bf3347e..1a1a2fb9 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -12,6 +12,7 @@ import pytest from pre_commit.commands.install_uninstall import install from pre_commit.commands.run import _get_skips from pre_commit.commands.run import _has_unmerged_paths +from pre_commit.commands.run import get_changed_files from pre_commit.commands.run import run from pre_commit.runner import Runner from pre_commit.util import cmd_output @@ -50,6 +51,8 @@ def _get_opts( verbose=False, hook=None, no_stash=False, + origin='', + source='', ): # These are mutually exclusive assert not (all_files and files) @@ -60,6 +63,8 @@ def _get_opts( verbose=verbose, hook=hook, no_stash=no_stash, + origin=origin, + source=source, ) @@ -126,6 +131,28 @@ def test_run( _test_run(repo_with_passing_hook, options, outputs, expected_ret, stage) +@pytest.mark.parametrize( + ('origin', 'source', 'expect_stash'), + ( + ('master', 'master', False), + ('master', '', True), + ('', 'master', True), + ) +) +def test_origin_source_define( + repo_with_passing_hook, origin, source, expect_stash, + mock_out_store_directory): + args = _get_opts(origin=origin, source=source) + ret, printed = _do_run(repo_with_passing_hook, args) + warning_msg = '--origin and --source depend on each other.' + if expect_stash: + assert ret == 1 + assert warning_msg in printed + else: + assert ret == 0 + assert warning_msg not in printed + + @pytest.mark.parametrize( ('no_stash', 'all_files', 'expect_stash'), ( @@ -267,3 +294,10 @@ def test_stdout_write_bug_py26( assert 'UnicodeEncodeError' not in stdout # Doesn't actually happen, but a reasonable assertion assert 'UnicodeDecodeError' not in stdout + + +def test_get_changed_files(): + files = list(get_changed_files('78c682a1d13ba20e7cb735313b9314a74365cd3a', + '3387edbb1288a580b37fe25225aa0b856b18ad1a' + )) + assert files == ['CHANGELOG.md', 'setup.py'] diff --git a/tests/runner_test.py b/tests/runner_test.py index cc4c816a..41bffa1c 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -7,6 +7,7 @@ import os.path import pre_commit.constants as C from pre_commit.runner import Runner from pre_commit.util import cwd +from pre_commit.util import resource_filename from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo @@ -58,6 +59,34 @@ def test_pre_commit_path(): assert runner.pre_commit_path == expected_path +def test_pre_push_path(): + runner = Runner('foo/bar') + expected_path = os.path.join('foo/bar', '.git/hooks/pre-push') + assert runner.pre_push_path == expected_path + + +def test_pre_commit_legacy_path(): + runner = Runner('foo/bar') + expected_path = os.path.join('foo/bar', '.git/hooks/pre-commit.legacy') + assert runner.pre_commit_legacy_path == expected_path + + +def test_pre_push_legacy_path(): + runner = Runner('foo/bar') + expected_path = os.path.join('foo/bar', '.git/hooks/pre-push.legacy') + assert runner.pre_push_legacy_path == expected_path + + +def test_pre_template(): + runner = Runner('foo/bar') + assert runner.pre_template == resource_filename('hook-tmpl') + + +def test_pre_push_template(): + runner = Runner('foo/bar') + assert runner.pre_push_template == resource_filename('pre-push-tmpl') + + def test_cmd_runner(mock_out_store_directory): runner = Runner('foo/bar') ret = runner.cmd_runner