From 94d626691f5a9479adfbef94382c1c6c74619076 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 14 Apr 2014 09:57:38 -0700 Subject: [PATCH 1/3] Move get_head_sha into testing since it is only used by tests. --- pre_commit/git.py | 5 ----- testing/util.py | 6 ++++++ tests/commands_test.py | 11 +++++------ tests/conftest.py | 4 ++-- tests/repository_test.py | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/pre_commit/git.py b/pre_commit/git.py index 6c359b66..bd6eb3d7 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -22,11 +22,6 @@ def get_root(): return _get_root_new() -def get_head_sha(git_repo_path): - with local.cwd(git_repo_path): - return local['git']['rev-parse', 'HEAD']().strip() - - @memoize_by_cwd def get_staged_files(): return local['git']['diff', '--staged', '--name-only']().splitlines() diff --git a/testing/util.py b/testing/util.py index 50390cbe..7af18e71 100644 --- a/testing/util.py +++ b/testing/util.py @@ -2,6 +2,7 @@ import jsonschema import os import os.path import shutil +from plumbum import local TESTING_DIR = os.path.abspath(os.path.dirname(__file__)) @@ -29,6 +30,11 @@ def copy_tree_to_path(src_dir, dest_dir): shutil.copy(srcname, destname) +def get_head_sha(dir): + with local.cwd(dir): + return local['git']['rev-parse', 'HEAD']().strip() + + def is_valid_according_to_schema(obj, schema): try: jsonschema.validate(obj, schema) diff --git a/tests/commands_test.py b/tests/commands_test.py index e3e6eb17..55cd3f15 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -9,15 +9,14 @@ from asottile.ordereddict import OrderedDict from asottile.yaml import ordered_dump from plumbum import local - import pre_commit.constants as C from pre_commit import commands -from pre_commit import git from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.runner import Runner from testing.auto_namedtuple import auto_namedtuple +from testing.util import get_head_sha from testing.util import get_resource_path @@ -53,7 +52,7 @@ def test_uninstall(empty_git_dir): def up_to_date_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), - ('sha', git.get_head_sha(python_hooks_repo)), + ('sha', get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) wrapped_config = apply_defaults([config], CONFIG_JSON_SCHEMA) @@ -90,14 +89,14 @@ def test_autoupdate_up_to_date_repo(up_to_date_repo): def out_of_date_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), - ('sha', git.get_head_sha(python_hooks_repo)), + ('sha', get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) validate_config_extra(config_wrapped) config = config_wrapped[0] local['git']['commit', '--allow-empty', '-m', 'foo']() - head_sha = git.get_head_sha(python_hooks_repo) + head_sha = get_head_sha(python_hooks_repo) with open(os.path.join(python_hooks_repo, C.CONFIG_FILE), 'w') as file_obj: file_obj.write( @@ -136,7 +135,7 @@ def test_autoupdate_out_of_date_repo(out_of_date_repo): def hook_disappearing_repo(python_hooks_repo): config = OrderedDict(( ('repo', python_hooks_repo), - ('sha', git.get_head_sha(python_hooks_repo)), + ('sha', get_head_sha(python_hooks_repo)), ('hooks', [OrderedDict((('id', 'foo'), ('files', '')))]), )) config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) diff --git a/tests/conftest.py b/tests/conftest.py index e7be74a0..27f5d2f6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,11 +6,11 @@ import yaml from plumbum import local import pre_commit.constants as C -from pre_commit import git from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.jsonschema_extensions import apply_defaults from testing.util import copy_tree_to_path +from testing.util import get_head_sha from testing.util import get_resource_path @@ -78,7 +78,7 @@ def failing_hook_repo(dummy_git_repo): def _make_config(path, hook_id, file_regex): config = { 'repo': path, - 'sha': git.get_head_sha(path), + 'sha': get_head_sha(path), 'hooks': [{'id': hook_id, 'files': file_regex}], } config_wrapped = apply_defaults([config], CONFIG_JSON_SCHEMA) diff --git a/tests/repository_test.py b/tests/repository_test.py index 4c93f354..ab3ea71f 100644 --- a/tests/repository_test.py +++ b/tests/repository_test.py @@ -3,13 +3,13 @@ import os import pytest import pre_commit.constants as C -from pre_commit import git from pre_commit import repository from pre_commit.clientlib.validate_config import CONFIG_JSON_SCHEMA from pre_commit.clientlib.validate_config import validate_config_extra from pre_commit.jsonschema_extensions import apply_defaults from pre_commit.prefixed_command_runner import PrefixedCommandRunner from pre_commit.repository import Repository +from testing.util import get_head_sha @pytest.fixture @@ -17,7 +17,7 @@ def dummy_repo_config(dummy_git_repo): # This is not a valid config, but it is pretty close return { 'repo': dummy_git_repo, - 'sha': git.get_head_sha(dummy_git_repo), + 'sha': get_head_sha(dummy_git_repo), 'hooks': [], } From 5810ee43152fcdf1d09a967f87bd79b700512621 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 14 Apr 2014 22:50:16 -0700 Subject: [PATCH 2/3] Implement merge-files only using @bukzor's method. --- pre_commit/commands.py | 2 ++ pre_commit/git.py | 35 +++++++++++++++++++++++++--- tests/commands_test.py | 20 ---------------- tests/conftest.py | 28 ++++++++++++++++++++++ tests/git_test.py | 53 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 23 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 33e36879..d9cb0d4e 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -147,6 +147,8 @@ def clean(runner): def _run_single_hook(runner, repository, hook_id, args, write): if args.all_files: get_filenames = git.get_all_files_matching + elif git.is_in_merge_conflict(): + get_filenames = git.get_conflicted_files_matching else: get_filenames = git.get_staged_files_matching diff --git a/pre_commit/git.py b/pre_commit/git.py index bd6eb3d7..79507680 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -1,4 +1,5 @@ import functools +import logging import os import os.path import re @@ -7,7 +8,11 @@ from plumbum import local from pre_commit.util import memoize_by_cwd -def _get_root_new(): +logger = logging.getLogger('pre_commit') + + +@memoize_by_cwd +def get_root(): path = os.getcwd() while len(path) > 1: if os.path.exists(os.path.join(path, '.git')): @@ -17,9 +22,32 @@ def _get_root_new(): raise AssertionError('called from outside of the gits') +def is_in_merge_conflict(): + return os.path.exists(os.path.join('.git', 'MERGE_MSG')) + + +def parse_merge_msg_for_conflicts(merge_msg): + # Conflicted files start with tabs + return [ + line.strip() for line in merge_msg.splitlines() if line.startswith('\t') + ] + + @memoize_by_cwd -def get_root(): - return _get_root_new() +def get_conflicted_files(): + # Need to get the conflicted files from the MERGE_MSG because they could + # have resolved the conflict by choosing one side or the other + merge_msg = open(os.path.join('.git', 'MERGE_MSG')).read() + merge_conflict_filenames = parse_merge_msg_for_conflicts(merge_msg) + + # This will get the rest of the changes made after the merge. + # If they resolved the merge conflict by choosing a mesh of both sides + # this will also include the conflicted files + tree_hash = local['git']['write-tree']().strip() + merge_diff_filenames = local['git'][ + 'diff', '-m', tree_hash, 'HEAD', 'MERGE_HEAD', '--name-only', + ]().splitlines() + return set(merge_conflict_filenames) | set(merge_diff_filenames) @memoize_by_cwd @@ -52,3 +80,4 @@ def get_files_matching(all_file_list_strategy): get_staged_files_matching = get_files_matching(get_staged_files) get_all_files_matching = get_files_matching(get_all_files) +get_conflicted_files_matching = get_files_matching(get_conflicted_files) diff --git a/tests/commands_test.py b/tests/commands_test.py index 55cd3f15..c9f553c3 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -283,26 +283,6 @@ def test_has_unmerged_paths(output, expected): assert commands._has_unmerged_paths(mock_runner) is expected -@pytest.yield_fixture -def in_merge_conflict(repo_with_passing_hook): - local['git']['add', C.CONFIG_FILE]() - local['git']['commit', '-m' 'add hooks file']() - local['git']['clone', '.', 'foo']() - with local.cwd('foo'): - local['git']['checkout', 'origin/master', '-b', 'foo']() - with open('conflict_file', 'w') as conflict_file: - conflict_file.write('herp\nderp\n') - local['git']['add', 'conflict_file']() - local['git']['commit', '-m', 'conflict_file']() - local['git']['checkout', 'origin/master', '-b', 'bar']() - with open('conflict_file', 'w') as conflict_file: - conflict_file.write('harp\nddrp\n') - local['git']['add', 'conflict_file']() - local['git']['commit', '-m', 'conflict_file']() - local['git']['merge', 'foo'](retcode=None) - yield os.path.join(repo_with_passing_hook, 'foo') - - def test_merge_conflict(in_merge_conflict): ret, printed = _do_run(in_merge_conflict, _get_opts()) assert ret == 1 diff --git a/tests/conftest.py b/tests/conftest.py index 27f5d2f6..9c513eee 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,5 +1,7 @@ from __future__ import absolute_import +import os +import os.path import pytest import time import yaml @@ -126,3 +128,29 @@ def repo_with_passing_hook(config_for_script_hooks_repo, empty_git_dir): def repo_with_failing_hook(failing_hook_repo, empty_git_dir): _make_repo_from_configs(_make_config(failing_hook_repo, 'failing_hook', '')) yield empty_git_dir + + +@pytest.yield_fixture +def in_merge_conflict(repo_with_passing_hook): + local['git']['add', C.CONFIG_FILE]() + local['git']['commit', '-m' 'add hooks file']() + local['git']['clone', '.', 'foo']() + with local.cwd('foo'): + local['git']['checkout', 'origin/master', '-b', 'foo']() + with open('conflict_file', 'w') as conflict_file: + conflict_file.write('herp\nderp\n') + local['git']['add', 'conflict_file']() + with open('foo_only_file', 'w') as foo_only_file: + foo_only_file.write('foo') + local['git']['add', 'foo_only_file']() + local['git']['commit', '-m', 'conflict_file']() + local['git']['checkout', 'origin/master', '-b', 'bar']() + with open('conflict_file', 'w') as conflict_file: + conflict_file.write('harp\nddrp\n') + local['git']['add', 'conflict_file']() + with open('bar_only_file', 'w') as bar_only_file: + bar_only_file.write('bar') + local['git']['add', 'bar_only_file']() + local['git']['commit', '-m', 'conflict_file']() + local['git']['merge', 'foo'](retcode=None) + yield os.path.join(repo_with_passing_hook, 'foo') diff --git a/tests/git_test.py b/tests/git_test.py index 4dd01f99..63cb1c61 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -14,6 +14,14 @@ def test_get_root(empty_git_dir): assert git.get_root() == empty_git_dir +def test_is_in_merge_conflict(empty_git_dir): + assert git.is_in_merge_conflict() is False + + +def test_is_not_in_merge_conflict(in_merge_conflict): + assert git.is_in_merge_conflict() is True + + @pytest.fixture def get_files_matching_func(): def get_filenames(): @@ -57,3 +65,48 @@ def test_does_not_include_deleted_fileS(get_files_matching_func): def test_exclude_removes_files(get_files_matching_func): ret = get_files_matching_func('', '\\.py$') assert ret == set(['hooks.yaml']) + + +def resolve_conflict(): + with open('conflict_file', 'w') as conflicted_file: + conflicted_file.write('herp\nderp\n') + local['git']['add', 'conflict_file']() + + +def test_get_conflicted_files(in_merge_conflict): + resolve_conflict() + with open('other_file', 'w') as other_file: + other_file.write('oh hai') + local['git']['add', 'other_file']() + + ret = set(git.get_conflicted_files()) + assert ret == set(('conflict_file', 'other_file')) + + +def test_get_conflicted_files_unstaged_files(in_merge_conflict): + # If they for whatever reason did pre-commit run --no-stash during a + # conflict + resolve_conflict() + + # Make unstaged file. + with open('bar_only_file', 'w') as bar_only_file: + bar_only_file.write('new contents!\n') + + ret = set(git.get_conflicted_files()) + assert ret == set(('conflict_file',)) + + +MERGE_MSG = "Merge branch 'foo' into bar\n\nConflicts:\n\tconflict_file\n" +OTHER_MERGE_MSG = MERGE_MSG + '\tother_conflict_file\n' + + +@pytest.mark.parametrize( + ('input', 'expected_output'), + ( + (MERGE_MSG, ['conflict_file']), + (OTHER_MERGE_MSG, ['conflict_file', 'other_conflict_file']), + ), +) +def test_parse_merge_msg_for_conflicts(input, expected_output): + ret = git.parse_merge_msg_for_conflicts(input) + assert ret == expected_output From 3ebf976afbadc74882e51c27475594d9f24444ce Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 15 Apr 2014 09:31:40 -0700 Subject: [PATCH 3/3] Add logging message. --- pre_commit/git.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pre_commit/git.py b/pre_commit/git.py index 79507680..847aedf4 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -35,6 +35,7 @@ def parse_merge_msg_for_conflicts(merge_msg): @memoize_by_cwd def get_conflicted_files(): + logger.info('Checking merge-conflict files only.') # Need to get the conflicted files from the MERGE_MSG because they could # have resolved the conflict by choosing one side or the other merge_msg = open(os.path.join('.git', 'MERGE_MSG')).read()