From e60f541559f19b858c499cbe182a9cf1d35c2f53 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sun, 21 Apr 2019 21:07:13 +0100 Subject: [PATCH 1/3] Adds support for prepare-commit-msg hooks Adds a prepare-commit-msg hook stage which allows for hooks which add dynamic suggested/placeholder text to commit messages that an author can use as a starting point for writing a commit message --- pre_commit/commands/run.py | 2 +- pre_commit/constants.py | 2 +- pre_commit/main.py | 4 +- pre_commit/resources/hook-tmpl | 1 + tests/commands/install_uninstall_test.py | 62 +++++++++++++++++++++++- tests/commands/run_test.py | 28 ++++++++++- tests/conftest.py | 49 +++++++++++++++++++ 7 files changed, 142 insertions(+), 6 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index cfa62ee2..95488b52 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -190,7 +190,7 @@ def _compute_cols(hooks, verbose): def _all_filenames(args): if args.origin and args.source: return git.get_changed_files(args.origin, args.source) - elif args.hook_stage == 'commit-msg': + elif args.hook_stage in ['prepare-commit-msg', 'commit-msg']: return (args.commit_msg_filename,) elif args.files: return args.files diff --git a/pre_commit/constants.py b/pre_commit/constants.py index 996480a9..307b09a4 100644 --- a/pre_commit/constants.py +++ b/pre_commit/constants.py @@ -21,6 +21,6 @@ LOCAL_REPO_VERSION = '1' VERSION = importlib_metadata.version('pre_commit') # `manual` is not invoked by any installed git hook. See #719 -STAGES = ('commit', 'commit-msg', 'manual', 'push') +STAGES = ('commit', 'prepare-commit-msg', 'commit-msg', 'manual', 'push') DEFAULT = 'default' diff --git a/pre_commit/main.py b/pre_commit/main.py index a935cf1c..aa7ff2a7 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -52,7 +52,9 @@ def _add_config_option(parser): def _add_hook_type_option(parser): parser.add_argument( - '-t', '--hook-type', choices=('pre-commit', 'pre-push', 'commit-msg'), + '-t', '--hook-type', choices=( + 'pre-commit', 'pre-push', 'prepare-commit-msg', 'commit-msg', + ), default='pre-commit', ) diff --git a/pre_commit/resources/hook-tmpl b/pre_commit/resources/hook-tmpl index 76123d3c..19d0e726 100755 --- a/pre_commit/resources/hook-tmpl +++ b/pre_commit/resources/hook-tmpl @@ -161,6 +161,7 @@ def _pre_push(stdin): def _opts(stdin): fns = { + 'prepare-commit-msg': lambda _: ('--commit-msg-filename', sys.argv[1]), 'commit-msg': lambda _: ('--commit-msg-filename', sys.argv[1]), 'pre-commit': lambda _: (), 'pre-push': _pre_push, diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index c19aaa44..a216bd5a 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -655,7 +655,65 @@ def test_commit_msg_legacy(commit_msg_repo, tempdir_factory, store): assert second_line.startswith('Must have "Signed off by:"...') -def test_install_disallow_mising_config(tempdir_factory, store): +def test_prepare_commit_msg_integration_failing( + failing_prepare_commit_msg_repo, tempdir_factory, store, +): + install(C.CONFIG_FILE, store, hook_type='prepare-commit-msg') + retc, out = _get_commit_output(tempdir_factory) + assert retc == 1 + assert out.startswith('Add "Signed off by:"...') + assert out.strip().endswith('...Failed') + + +def test_prepare_commit_msg_integration_passing( + prepare_commit_msg_repo, tempdir_factory, store, +): + install(C.CONFIG_FILE, store, hook_type='prepare-commit-msg') + msg = 'Hi' + retc, out = _get_commit_output(tempdir_factory, msg=msg) + assert retc == 0 + first_line = out.splitlines()[0] + assert first_line.startswith('Add "Signed off by:"...') + assert first_line.endswith('...Passed') + commit_msg_path = os.path.join( + prepare_commit_msg_repo, '.git/COMMIT_EDITMSG', + ) + with io.open(commit_msg_path, 'rt') as f: + assert 'Signed off by: ' in f.read() + + +def test_prepare_commit_msg_legacy( + prepare_commit_msg_repo, tempdir_factory, store, +): + hook_path = os.path.join( + prepare_commit_msg_repo, '.git/hooks/prepare-commit-msg', + ) + mkdirp(os.path.dirname(hook_path)) + with io.open(hook_path, 'w') as hook_file: + hook_file.write( + '#!/usr/bin/env bash\n' + 'set -eu\n' + 'test -e "$1"\n' + 'echo legacy\n', + ) + make_executable(hook_path) + + install(C.CONFIG_FILE, store, hook_type='prepare-commit-msg') + + msg = 'Hi' + retc, out = _get_commit_output(tempdir_factory, msg=msg) + assert retc == 0 + first_line, second_line = out.splitlines()[:2] + assert first_line == 'legacy' + assert second_line.startswith('Add "Signed off by:"...') + commit_msg_path = os.path.join( + prepare_commit_msg_repo, '.git/COMMIT_EDITMSG', + ) + with io.open(commit_msg_path, 'rt') as f: + assert 'Signed off by: ' in f.read() + + +def test_install_disallow_missing_config(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): remove_config_from_repo(path) @@ -668,7 +726,7 @@ def test_install_disallow_mising_config(tempdir_factory, store): assert ret == 1 -def test_install_allow_mising_config(tempdir_factory, store): +def test_install_allow_missing_config(tempdir_factory, store): path = make_consuming_repo(tempdir_factory, 'script_hooks_repo') with cwd(path): remove_config_from_repo(path) diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 11a8eea1..29534648 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -557,7 +557,12 @@ def test_stages(cap_out, store, repo_with_passing_hook): 'language': 'pygrep', 'stages': [stage], } - for i, stage in enumerate(('commit', 'push', 'manual'), 1) + for i, stage in enumerate( + ( + 'commit', 'push', 'manual', 'prepare-commit-msg', + 'commit-msg', + ), 1, + ) ], } add_config_to_repo(repo_with_passing_hook, config) @@ -575,6 +580,8 @@ def test_stages(cap_out, store, repo_with_passing_hook): assert _run_for_stage('commit').startswith(b'hook 1...') assert _run_for_stage('push').startswith(b'hook 2...') assert _run_for_stage('manual').startswith(b'hook 3...') + assert _run_for_stage('prepare-commit-msg').startswith(b'hook 4...') + assert _run_for_stage('commit-msg').startswith(b'hook 5...') def test_commit_msg_hook(cap_out, store, commit_msg_repo): @@ -593,6 +600,25 @@ def test_commit_msg_hook(cap_out, store, commit_msg_repo): ) +def test_prepare_commit_msg_hook(cap_out, store, prepare_commit_msg_repo): + filename = '.git/COMMIT_EDITMSG' + with io.open(filename, 'w') as f: + f.write('This is the commit message') + + _test_run( + cap_out, + store, + prepare_commit_msg_repo, + {'hook_stage': 'prepare-commit-msg', 'commit_msg_filename': filename}, + expected_outputs=[b'Add "Signed off by:"', b'Passed'], + expected_ret=0, + stage=False, + ) + + with io.open(filename, 'rt') as f: + assert 'Signed off by: ' in f.read() + + def test_local_hook_passes(cap_out, store, repo_with_passing_hook): config = { 'repo': 'local', diff --git a/tests/conftest.py b/tests/conftest.py index 50ad76ed..e6d7777e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,6 +14,7 @@ from pre_commit import output from pre_commit.logging_handler import logging_handler from pre_commit.store import Store from pre_commit.util import cmd_output +from pre_commit.util import make_executable from testing.fixtures import git_dir from testing.fixtures import make_consuming_repo from testing.fixtures import write_config @@ -134,6 +135,54 @@ def commit_msg_repo(tempdir_factory): yield path +@pytest.fixture +def prepare_commit_msg_repo(tempdir_factory): + path = git_dir(tempdir_factory) + script_name = 'add_sign_off.sh' + config = { + 'repo': 'local', + 'hooks': [{ + 'id': 'add-signoff', + 'name': 'Add "Signed off by:"', + 'entry': './{}'.format(script_name), + 'language': 'script', + 'stages': ['prepare-commit-msg'], + }], + } + write_config(path, config) + with cwd(path): + with io.open(script_name, 'w') as script_file: + script_file.write( + '#!/usr/bin/env bash\n' + 'set -eu\n' + 'echo "\nSigned off by: " >> "$1"\n', + ) + make_executable(script_name) + cmd_output('git', 'add', '.') + git_commit(msg=prepare_commit_msg_repo.__name__) + yield path + + +@pytest.fixture +def failing_prepare_commit_msg_repo(tempdir_factory): + path = git_dir(tempdir_factory) + config = { + 'repo': 'local', + 'hooks': [{ + 'id': 'add-signoff', + 'name': 'Add "Signed off by:"', + 'entry': '/usr/bin/env bash -c "exit 1"', + 'language': 'system', + 'stages': ['prepare-commit-msg'], + }], + } + write_config(path, config) + with cwd(path): + cmd_output('git', 'add', '.') + git_commit(msg=failing_prepare_commit_msg_repo.__name__) + yield path + + @pytest.fixture(autouse=True, scope='session') def dont_write_to_home_directory(): """pre_commit.store.Store will by default write to the home directory From 64467f6ab9bcffb6ade2d631e32270cc248750e8 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sun, 21 Apr 2019 21:54:23 +0100 Subject: [PATCH 2/3] Fix broken test_manifest_hooks test --- tests/repository_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/repository_test.py b/tests/repository_test.py index d8bfde30..a2a9bb57 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -824,7 +824,9 @@ def test_manifest_hooks(tempdir_factory, store): name='Bash hook', pass_filenames=True, require_serial=False, - stages=('commit', 'commit-msg', 'manual', 'push'), + stages=( + 'commit', 'prepare-commit-msg', 'commit-msg', 'manual', 'push', + ), types=['file'], verbose=False, ) From 82969e4ba3623d6e0205a14a13411ff0aae1e197 Mon Sep 17 00:00:00 2001 From: Marc Jay Date: Sun, 21 Apr 2019 21:58:01 +0100 Subject: [PATCH 3/3] Use set rather than list for commit message related stages, remove default file open modes, tidy up bash call for failing hook test --- pre_commit/commands/run.py | 2 +- tests/commands/install_uninstall_test.py | 4 ++-- tests/commands/run_test.py | 2 +- tests/conftest.py | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 95488b52..d060e186 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -190,7 +190,7 @@ def _compute_cols(hooks, verbose): def _all_filenames(args): if args.origin and args.source: return git.get_changed_files(args.origin, args.source) - elif args.hook_stage in ['prepare-commit-msg', 'commit-msg']: + elif args.hook_stage in {'prepare-commit-msg', 'commit-msg'}: return (args.commit_msg_filename,) elif args.files: return args.files diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index a216bd5a..e253dd4b 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -678,7 +678,7 @@ def test_prepare_commit_msg_integration_passing( commit_msg_path = os.path.join( prepare_commit_msg_repo, '.git/COMMIT_EDITMSG', ) - with io.open(commit_msg_path, 'rt') as f: + with io.open(commit_msg_path) as f: assert 'Signed off by: ' in f.read() @@ -709,7 +709,7 @@ def test_prepare_commit_msg_legacy( commit_msg_path = os.path.join( prepare_commit_msg_repo, '.git/COMMIT_EDITMSG', ) - with io.open(commit_msg_path, 'rt') as f: + with io.open(commit_msg_path) as f: assert 'Signed off by: ' in f.read() diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index 29534648..b465cae6 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -615,7 +615,7 @@ def test_prepare_commit_msg_hook(cap_out, store, prepare_commit_msg_repo): stage=False, ) - with io.open(filename, 'rt') as f: + with io.open(filename) as f: assert 'Signed off by: ' in f.read() diff --git a/tests/conftest.py b/tests/conftest.py index e6d7777e..23ff7460 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -171,7 +171,7 @@ def failing_prepare_commit_msg_repo(tempdir_factory): 'hooks': [{ 'id': 'add-signoff', 'name': 'Add "Signed off by:"', - 'entry': '/usr/bin/env bash -c "exit 1"', + 'entry': 'bash -c "exit 1"', 'language': 'system', 'stages': ['prepare-commit-msg'], }],