mirror/pre-commit
1
0
Fork 0
mirror of https://github.com/pre-commit/pre-commit.git synced 2026-01-14 04:50:20 -06:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #608 from pre-commit/nix_runner_cmd_runner

Browse Source 
Remove Runner.cmd_runner and Store.cmd_runner
This commit is contained in:
Anthony Sottile
2017-09-05 15:11:11 -07:00
committed by GitHub
parent 95c3afacda 6141c419ee
commit 3e507bf8bd
8 changed files with 65 additions and 120 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
pre_commit/commands/run.py
View File

@@ -192,17 +192,14 @@ def get_repo_hooks(runner):
yield (repo, hook)
def _has_unmerged_paths(runner):
_, stdout, _ = runner.cmd_runner.run(['git', 'ls-files', '--unmerged'])
def _has_unmerged_paths():
_, stdout, _ = cmd_output('git', 'ls-files', '--unmerged')
return bool(stdout.strip())
def _has_unstaged_config(runner):
retcode, _, _ = runner.cmd_runner.run(
(
'git', 'diff', '--no-ext-diff', '--exit-code',
runner.config_file_path,
),
retcode, _, _ = cmd_output(
'git', 'diff', '--no-ext-diff', '--exit-code', runner.config_file_path,
retcode=None,
)
# be explicit, other git errors don't mean it has an unstaged config.
@@ -213,7 +210,7 @@ def run(runner, args, environ=os.environ):
no_stash = args.all_files or bool(args.files)
# Check if we have unresolved merge conflict files and fail fast.
if _has_unmerged_paths(runner):
if _has_unmerged_paths():
logger.error('Unmerged files. Resolve before committing.')
return 1
if bool(args.source) != bool(args.origin):
@@ -234,7 +231,7 @@ def run(runner, args, environ=os.environ):
if no_stash:
ctx = noop_context()
else:
ctx = staged_files_only(runner.cmd_runner)
ctx = staged_files_only(runner.store.directory)
with ctx:
repo_hooks = list(get_repo_hooks(runner))

5
pre_commit/runner.py
View File

@@ -57,11 +57,6 @@ class Runner(object):
def pre_push_path(self):
return self.get_hook_path('pre-push')
@cached_property
def cmd_runner(self):
# TODO: remove this and inline runner.store.cmd_runner
return self.store.cmd_runner
@cached_property
def store(self):
return Store()

35
pre_commit/staged_files_only.py
View File

@@ -3,44 +3,41 @@ from __future__ import unicode_literals
import contextlib
import io
import logging
import os.path
import time
from pre_commit.util import CalledProcessError
from pre_commit.util import cmd_output
logger = logging.getLogger('pre_commit')
def _git_apply(cmd_runner, patch):
def _git_apply(patch):
args = ('apply', '--whitespace=nowarn', patch)
try:
cmd_runner.run(('git',) + args, encoding=None)
cmd_output('git', *args, encoding=None)
except CalledProcessError:
# Retry with autocrlf=false -- see #570
cmd = ('git', '-c', 'core.autocrlf=false') + args
cmd_runner.run(cmd, encoding=None)
cmd_output('git', '-c', 'core.autocrlf=false', *args, encoding=None)
@contextlib.contextmanager
def staged_files_only(cmd_runner):
def staged_files_only(patch_dir):
"""Clear any unstaged changes from the git working directory inside this
context.
Args:
cmd_runner - PrefixedCommandRunner
"""
# Determine if there are unstaged files
tree = cmd_runner.run(('git', 'write-tree'))[1].strip()
retcode, diff_stdout_binary, _ = cmd_runner.run(
(
'git', 'diff-index', '--ignore-submodules', '--binary',
'--exit-code', '--no-color', '--no-ext-diff', tree, '--',
),
tree = cmd_output('git', 'write-tree')[1].strip()
retcode, diff_stdout_binary, _ = cmd_output(
'git', 'diff-index', '--ignore-submodules', '--binary',
'--exit-code', '--no-color', '--no-ext-diff', tree, '--',
retcode=None,
encoding=None,
)
if retcode and diff_stdout_binary.strip():
patch_filename = cmd_runner.path('patch{}'.format(int(time.time())))
patch_filename = 'patch{}'.format(int(time.time()))
patch_filename = os.path.join(patch_dir, patch_filename)
logger.warning('Unstaged files detected.')
logger.info(
'Stashing unstaged files to {}.'.format(patch_filename),
@@ -50,13 +47,13 @@ def staged_files_only(cmd_runner):
patch_file.write(diff_stdout_binary)
# Clear the working directory of unstaged changes
cmd_runner.run(('git', 'checkout', '--', '.'))
cmd_output('git', 'checkout', '--', '.')
try:
yield
finally:
# Try to apply the patch we saved
try:
_git_apply(cmd_runner, patch_filename)
_git_apply(patch_filename)
except CalledProcessError:
logger.warning(
'Stashed changes conflicted with hook auto-fixes... '
@@ -65,8 +62,8 @@ def staged_files_only(cmd_runner):
# We failed to apply the patch, presumably due to fixes made
# by hooks.
# Roll back the changes made by hooks.
cmd_runner.run(('git', 'checkout', '--', '.'))
_git_apply(cmd_runner, patch_filename)
cmd_output('git', 'checkout', '--', '.')
_git_apply(patch_filename)
logger.info('Restored changes from {}.'.format(patch_filename))
else:
# There weren't any staged files so we don't need to do anything

5
pre_commit/store.py
View File

@@ -11,7 +11,6 @@ from cached_property import cached_property
import pre_commit.constants as C
from pre_commit import file_lock
from pre_commit.prefixed_command_runner import PrefixedCommandRunner
from pre_commit.util import clean_path_on_failure
from pre_commit.util import cmd_output
from pre_commit.util import copy_tree_to_path
@@ -162,10 +161,6 @@ class Store(object):
make_local_strategy,
)
@cached_property
def cmd_runner(self):
return PrefixedCommandRunner(self.directory)
@cached_property
def db_path(self):
return os.path.join(self.directory, 'db.db')

10
tests/commands/run_test.py
View File

@@ -7,7 +7,6 @@ import subprocess
import sys
from collections import OrderedDict
import mock
import pytest
import pre_commit.constants as C
@@ -327,11 +326,10 @@ def test_origin_source_error_msg(
assert warning_msg not in printed
@pytest.mark.parametrize(('output', 'expected'), (('some', True), ('', False)))
def test_has_unmerged_paths(output, expected):
mock_runner = mock.Mock()
mock_runner.cmd_runner.run.return_value = (1, output, '')
assert _has_unmerged_paths(mock_runner) is expected
def test_has_unmerged_paths(in_merge_conflict):
assert _has_unmerged_paths() is True
cmd_output('git', 'add', '.')
assert _has_unmerged_paths() is False
def test_merge_conflict(cap_out, in_merge_conflict, mock_out_store_directory):

6
tests/runner_test.py
View File

@@ -133,9 +133,3 @@ def test_pre_push_path(in_tmpdir):
runner = Runner(path, C.CONFIG_FILE)
expected_path = os.path.join(path, '.git', 'hooks', 'pre-push')
assert runner.pre_push_path == expected_path
def test_cmd_runner(mock_out_store_directory):
runner = Runner(os.path.join('foo', 'bar'), C.CONFIG_FILE)
ret = runner.cmd_runner
assert ret.prefix_dir == os.path.join(mock_out_store_directory) + os.sep

104
tests/staged_files_only_test.py
View File

@@ -4,11 +4,9 @@ from __future__ import unicode_literals
import io
import itertools
import logging
import os.path
import shutil
import mock
import pytest
from pre_commit.staged_files_only import staged_files_only
@@ -22,6 +20,11 @@ from testing.util import get_resource_path
FOO_CONTENTS = '\n'.join(('1', '2', '3', '4', '5', '6', '7', '8', ''))
@pytest.fixture
def patch_dir(tempdir_factory):
return tempdir_factory.get()
def get_short_git_status():
git_status = cmd_output('git', 'status', '-s')[1]
return dict(reversed(line.split()) for line in git_status.splitlines())
@@ -54,43 +57,43 @@ def test_foo_staged(foo_staged):
_test_foo_state(foo_staged)
def test_foo_nothing_unstaged(foo_staged, cmd_runner):
with staged_files_only(cmd_runner):
def test_foo_nothing_unstaged(foo_staged, patch_dir):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
_test_foo_state(foo_staged)
def test_foo_something_unstaged(foo_staged, cmd_runner):
def test_foo_something_unstaged(foo_staged, patch_dir):
with io.open(foo_staged.foo_filename, 'w') as foo_file:
foo_file.write('herp\nderp\n')
_test_foo_state(foo_staged, 'herp\nderp\n', 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
_test_foo_state(foo_staged, 'herp\nderp\n', 'AM')
def test_something_unstaged_ext_diff_tool(foo_staged, cmd_runner, tmpdir):
def test_something_unstaged_ext_diff_tool(foo_staged, patch_dir, tmpdir):
diff_tool = tmpdir.join('diff-tool.sh')
diff_tool.write('#!/usr/bin/env bash\necho "$@"\n')
cmd_output('git', 'config', 'diff.external', diff_tool.strpath)
test_foo_something_unstaged(foo_staged, cmd_runner)
test_foo_something_unstaged(foo_staged, patch_dir)
def test_foo_something_unstaged_diff_color_always(foo_staged, cmd_runner):
def test_foo_something_unstaged_diff_color_always(foo_staged, patch_dir):
cmd_output('git', 'config', '--local', 'color.diff', 'always')
test_foo_something_unstaged(foo_staged, cmd_runner)
test_foo_something_unstaged(foo_staged, patch_dir)
def test_foo_both_modify_non_conflicting(foo_staged, cmd_runner):
def test_foo_both_modify_non_conflicting(foo_staged, patch_dir):
with io.open(foo_staged.foo_filename, 'w') as foo_file:
foo_file.write(FOO_CONTENTS + '9\n')
_test_foo_state(foo_staged, FOO_CONTENTS + '9\n', 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
# Modify the file as part of the "pre-commit"
@@ -102,13 +105,13 @@ def test_foo_both_modify_non_conflicting(foo_staged, cmd_runner):
_test_foo_state(foo_staged, FOO_CONTENTS.replace('1', 'a') + '9\n', 'AM')
def test_foo_both_modify_conflicting(foo_staged, cmd_runner):
def test_foo_both_modify_conflicting(foo_staged, patch_dir):
with io.open(foo_staged.foo_filename, 'w') as foo_file:
foo_file.write(FOO_CONTENTS.replace('1', 'a'))
_test_foo_state(foo_staged, FOO_CONTENTS.replace('1', 'a'), 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
# Modify in the same place as the stashed diff
@@ -144,30 +147,30 @@ def test_img_staged(img_staged):
_test_img_state(img_staged)
def test_img_nothing_unstaged(img_staged, cmd_runner):
with staged_files_only(cmd_runner):
def test_img_nothing_unstaged(img_staged, patch_dir):
with staged_files_only(patch_dir):
_test_img_state(img_staged)
_test_img_state(img_staged)
def test_img_something_unstaged(img_staged, cmd_runner):
def test_img_something_unstaged(img_staged, patch_dir):
shutil.copy(get_resource_path('img2.jpg'), img_staged.img_filename)
_test_img_state(img_staged, 'img2.jpg', 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_img_state(img_staged)
_test_img_state(img_staged, 'img2.jpg', 'AM')
def test_img_conflict(img_staged, cmd_runner):
def test_img_conflict(img_staged, patch_dir):
"""Admittedly, this shouldn't happen, but just in case."""
shutil.copy(get_resource_path('img2.jpg'), img_staged.img_filename)
_test_img_state(img_staged, 'img2.jpg', 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_img_state(img_staged)
shutil.copy(get_resource_path('img3.jpg'), img_staged.img_filename)
_test_img_state(img_staged, 'img3.jpg', 'AM')
@@ -220,77 +223,48 @@ def test_sub_staged(sub_staged):
_test_sub_state(sub_staged)
def test_sub_nothing_unstaged(sub_staged, cmd_runner):
with staged_files_only(cmd_runner):
def test_sub_nothing_unstaged(sub_staged, patch_dir):
with staged_files_only(patch_dir):
_test_sub_state(sub_staged)
_test_sub_state(sub_staged)
def test_sub_something_unstaged(sub_staged, cmd_runner):
def test_sub_something_unstaged(sub_staged, patch_dir):
checkout_submodule(sub_staged.submodule.sha2)
_test_sub_state(sub_staged, 'sha2', 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
# This is different from others, we don't want to touch subs
_test_sub_state(sub_staged, 'sha2', 'AM')
_test_sub_state(sub_staged, 'sha2', 'AM')
@pytest.yield_fixture
def fake_logging_handler():
class FakeHandler(logging.Handler):
def __init__(self):
logging.Handler.__init__(self)
self.logs = []
def emit(self, record):
self.logs.append(record) # pragma: no cover (only hit in failure)
pre_commit_logger = logging.getLogger('pre_commit')
original_level = pre_commit_logger.getEffectiveLevel()
handler = FakeHandler()
pre_commit_logger.addHandler(handler)
pre_commit_logger.setLevel(logging.WARNING)
yield handler
pre_commit_logger.setLevel(original_level)
pre_commit_logger.removeHandler(handler)
def test_diff_returns_1_no_diff_though(fake_logging_handler, foo_staged):
cmd_runner = mock.Mock()
cmd_runner.run.return_value = (1, '', '')
cmd_runner.path.return_value = '.pre-commit-files_patch'
with staged_files_only(cmd_runner):
pass
assert not fake_logging_handler.logs
def test_stage_utf8_changes(foo_staged, cmd_runner):
def test_stage_utf8_changes(foo_staged, patch_dir):
contents = '\u2603'
with io.open('foo', 'w', encoding='UTF-8') as foo_file:
foo_file.write(contents)
_test_foo_state(foo_staged, contents, 'AM')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
_test_foo_state(foo_staged, contents, 'AM')
def test_stage_non_utf8_changes(foo_staged, cmd_runner):
def test_stage_non_utf8_changes(foo_staged, patch_dir):
contents = 'ú'
# Produce a latin-1 diff
with io.open('foo', 'w', encoding='latin-1') as foo_file:
foo_file.write(contents)
_test_foo_state(foo_staged, contents, 'AM', encoding='latin-1')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
_test_foo_state(foo_staged, contents, 'AM', encoding='latin-1')
def test_non_utf8_conflicting_diff(foo_staged, cmd_runner):
def test_non_utf8_conflicting_diff(foo_staged, patch_dir):
"""Regression test for #397"""
# The trailing whitespace is important here, this triggers git to produce
# an error message which looks like:
@@ -307,7 +281,7 @@ def test_non_utf8_conflicting_diff(foo_staged, cmd_runner):
foo_file.write(contents)
_test_foo_state(foo_staged, contents, 'AM', encoding='latin-1')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
_test_foo_state(foo_staged)
# Create a conflicting diff that will need to be rolled back
with io.open('foo', 'w') as foo_file:
@@ -337,7 +311,7 @@ bool_product = tuple(itertools.product((True, False), repeat=2))
@pytest.mark.parametrize(('crlf_before', 'crlf_after'), bool_product)
@pytest.mark.parametrize('autocrlf', ('true', 'false', 'input'))
def test_crlf(in_git_dir, cmd_runner, crlf_before, crlf_after, autocrlf):
def test_crlf(in_git_dir, patch_dir, crlf_before, crlf_after, autocrlf):
cmd_output('git', 'config', '--local', 'core.autocrlf', autocrlf)
before, after = b'1\n2\n', b'3\n4\n\n'
@@ -347,16 +321,16 @@ def test_crlf(in_git_dir, cmd_runner, crlf_before, crlf_after, autocrlf):
_write(before)
cmd_output('git', 'add', 'foo')
_write(after)
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
assert_no_diff()
def test_whitespace_errors(in_git_dir, cmd_runner):
def test_whitespace_errors(in_git_dir, patch_dir):
cmd_output('git', 'config', '--local', 'apply.whitespace', 'error')
test_crlf(in_git_dir, cmd_runner, True, True, 'true')
test_crlf(in_git_dir, patch_dir, True, True, 'true')
def test_autocrlf_commited_crlf(in_git_dir, cmd_runner):
def test_autocrlf_commited_crlf(in_git_dir, patch_dir):
"""Regression test for #570"""
cmd_output('git', 'config', '--local', 'core.autocrlf', 'false')
_write(b'1\r\n2\r\n')
@@ -366,5 +340,5 @@ def test_autocrlf_commited_crlf(in_git_dir, cmd_runner):
cmd_output('git', 'config', '--local', 'core.autocrlf', 'true')
_write(b'1\r\n2\r\n\r\n\r\n\r\n')
with staged_files_only(cmd_runner):
with staged_files_only(patch_dir):
assert_no_diff()

5
tests/store_test.py
View File

@@ -125,11 +125,6 @@ def test_clone_cleans_up_on_checkout_failure(store):
assert things_starting_with_repo == []
def test_has_cmd_runner_at_directory(store):
ret = store.cmd_runner
assert ret.prefix_dir == store.directory + os.sep
def test_clone_when_repo_already_exists(store):
# Create an entry in the sqlite db that makes it look like the repo has
# been cloned.