From b707cbba06c3d08c8aac31c460ffc7156765e25c Mon Sep 17 00:00:00 2001 From: dongweiming Date: Sun, 11 Jan 2015 22:40:35 +0800 Subject: [PATCH 1/4] 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 From 931c69b3faccffda8c18233c743cda6493737dcd Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 14 Jan 2015 19:21:32 -0800 Subject: [PATCH 2/4] Simplify a few things --- pre_commit/commands/install_uninstall.py | 21 +++++++-------------- pre_commit/commands/run.py | 13 +++++-------- pre_commit/main.py | 4 ++-- pre_commit/runner.py | 23 ----------------------- tests/commands/install_uninstall_test.py | 22 ++++------------------ tests/commands/run_test.py | 20 +++++++++++--------- tests/runner_test.py | 23 ----------------------- 7 files changed, 29 insertions(+), 97 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index 61c8e062..b2af14d4 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -9,6 +9,7 @@ import stat import sys from pre_commit.logging_handler import LoggingHandler +from pre_commit.util import resource_filename logger = logging.getLogger('pre_commit') @@ -41,19 +42,10 @@ def make_executable(filename): ) -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.""" - hook_path, legacy_path = get_hook_path(runner, hook_type) + hook_path = runner.get_hook_path(hook_type) + legacy_path = hook_path + '.legacy' # If we have an existing hook, move it to pre-commit.legacy if ( @@ -76,12 +68,12 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): 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: + with io.open(resource_filename('pre-push-tmpl')) as fp: pre_push_contents = fp.read() else: pre_push_contents = '' - contents = io.open(runner.pre_template).read().format( + contents = io.open(resource_filename('hook-tmpl')).read().format( sys_executable=sys.executable, hook_type=hook_type, pre_push=pre_push_contents, @@ -104,7 +96,8 @@ def install(runner, overwrite=False, hooks=False, hook_type='pre-commit'): def uninstall(runner, hook_type='pre-commit'): """Uninstall the pre-commit hooks.""" - hook_path, legacy_path = get_hook_path(runner, hook_type) + hook_path = runner.get_hook_path(hook_type) + legacy_path = hook_path + '.legacy' # If our file doesn't exist or it isn't ours, gtfo. if ( not os.path.exists(hook_path) or ( diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 2817a533..a9e59d55 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -50,18 +50,16 @@ def _print_user_skipped(hook, write, args): def get_changed_files(new, old): - changed_files = cmd_output( + return 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.origin and args.source: get_filenames = git.get_files_matching( - lambda: get_changed_files(args.origin, args.source)) + lambda: get_changed_files(args.origin, args.source), + ) elif args.files: get_filenames = git.get_files_matching(lambda: args.files) elif args.all_files: @@ -150,9 +148,8 @@ 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.') + if bool(args.source) != bool(args.origin): + logger.error('Specify both --origin and --source.') return 1 # Don't stash if specified or files are specified diff --git a/pre_commit/main.py b/pre_commit/main.py index 6cf6bd33..64c04047 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -79,11 +79,11 @@ def main(argv=None): ) run_parser.add_argument( - '--origin', '-o', default='', + '--origin', '-o', help='The origin branch"s commit_id when using `git push`', ) run_parser.add_argument( - '--source', '-s', default='', + '--source', '-s', help='The remote branch"s commit_id when using `git push`', ) run_mutex_group = run_parser.add_mutually_exclusive_group(required=False) diff --git a/pre_commit/runner.py b/pre_commit/runner.py index 302bacb0..ae720d05 100644 --- a/pre_commit/runner.py +++ b/pre_commit/runner.py @@ -10,7 +10,6 @@ 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): @@ -55,28 +54,6 @@ class Runner(object): 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/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 9733b175..658e8966 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,7 +22,6 @@ 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 @@ -71,11 +70,12 @@ def test_install_pre_commit(tmpdir_factory): 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() + pre_push_tmpl = resource_filename('pre-push-tmpl') + pre_push_template_contents = io.open(pre_push_tmpl).read() expected_contents = io.open(pre_commit_script).read().format( sys_executable=sys.executable, hook_type='pre-push', - pre_push=pre_push_template_contents + pre_push=pre_push_template_contents, ) assert pre_push_contents == expected_contents @@ -406,17 +406,3 @@ 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 1a1a2fb9..e67c02bd 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -132,20 +132,21 @@ def test_run( @pytest.mark.parametrize( - ('origin', 'source', 'expect_stash'), + ('origin', 'source', 'expect_failure'), ( ('master', 'master', False), ('master', '', True), ('', 'master', True), ) ) -def test_origin_source_define( - repo_with_passing_hook, origin, source, expect_stash, - mock_out_store_directory): +def test_origin_source_error_msg( + repo_with_passing_hook, origin, source, expect_failure, + 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: + warning_msg = 'Specify both --origin and --source.' + if expect_failure: assert ret == 1 assert warning_msg in printed else: @@ -297,7 +298,8 @@ def test_stdout_write_bug_py26( def test_get_changed_files(): - files = list(get_changed_files('78c682a1d13ba20e7cb735313b9314a74365cd3a', - '3387edbb1288a580b37fe25225aa0b856b18ad1a' - )) + files = get_changed_files( + '78c682a1d13ba20e7cb735313b9314a74365cd3a', + '3387edbb1288a580b37fe25225aa0b856b18ad1a', + ) assert files == ['CHANGELOG.md', 'setup.py'] diff --git a/tests/runner_test.py b/tests/runner_test.py index 41bffa1c..249cc2c4 100644 --- a/tests/runner_test.py +++ b/tests/runner_test.py @@ -7,7 +7,6 @@ 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 @@ -65,28 +64,6 @@ def test_pre_push_path(): 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 From b7141f32c00c40e0b2b481f9f3715bf2eea837fa Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 14 Jan 2015 20:19:49 -0800 Subject: [PATCH 3/4] Add integration test demonstrating hooks --- tests/commands/install_uninstall_test.py | 43 ++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 658e8966..d83d6ec3 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -406,3 +406,46 @@ def test_installed_from_venv(tmpdir_factory): ) assert ret == 0 assert NORMAL_PRE_COMMIT_RUN.match(output) + + +def _get_push_output(tmpdir_factory): + # Don't want to write to home directory + home = tmpdir_factory.get() + env = dict(os.environ, **{'PRE_COMMIT_HOME': home}) + return cmd_output( + 'git', 'push', 'origin', 'HEAD:new_branch', + # git commit puts pre-commit to stderr + stderr=subprocess.STDOUT, + env=env, + retcode=None, + )[:2] + + +def test_pre_push_integration_failing(tmpdir_factory): + upstream = make_consuming_repo(tmpdir_factory, 'failing_hook_repo') + path = tmpdir_factory.get() + cmd_output('git', 'clone', upstream, path) + with cwd(path): + install(Runner(path), hook_type='pre-push') + # commit succeeds because pre-commit is only installed for pre-push + assert _get_commit_output(tmpdir_factory)[0] == 0 + + retc, output = _get_push_output(tmpdir_factory) + assert retc == 1 + assert 'Failing hook' in output + assert 'Failed' in output + assert 'hookid: failing_hook' in output + + +def test_pre_push_integration_accepted(tmpdir_factory): + upstream = make_consuming_repo(tmpdir_factory, 'script_hooks_repo') + path = tmpdir_factory.get() + cmd_output('git', 'clone', upstream, path) + with cwd(path): + install(Runner(path), hook_type='pre-push') + assert _get_commit_output(tmpdir_factory)[0] == 0 + + retc, output = _get_push_output(tmpdir_factory) + assert retc == 0 + assert 'Bash hook' in output + assert 'Passed' in output From febb270afe307af73e3d94c101e005459cafdb6c Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Wed, 14 Jan 2015 20:27:37 -0800 Subject: [PATCH 4/4] Bump magic numbers --- pre_commit/commands/install_uninstall.py | 3 ++- pre_commit/resources/hook-tmpl | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index b2af14d4..c84d29b4 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -19,10 +19,11 @@ logger = logging.getLogger('pre_commit') PREVIOUS_IDENTIFYING_HASHES = ( '4d9958c90bc262f47553e2c073f14cfe', 'd8ee923c46731b42cd95cc869add4062', + '49fd668cb42069aa1b6048464be5d395', ) -IDENTIFYING_HASH = '49fd668cb42069aa1b6048464be5d395' +IDENTIFYING_HASH = '79f09a650522a87b0da915d0d983b2de' def is_our_pre_commit(filename): diff --git a/pre_commit/resources/hook-tmpl b/pre_commit/resources/hook-tmpl index 9583f893..e65f60e6 100755 --- a/pre_commit/resources/hook-tmpl +++ b/pre_commit/resources/hook-tmpl @@ -1,6 +1,6 @@ #!/usr/bin/env bash # This is a randomish md5 to identify this script -# 49fd668cb42069aa1b6048464be5d395 +# 79f09a650522a87b0da915d0d983b2de pushd `dirname $0` > /dev/null HERE=`pwd`