diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 2b90e44e..f38b25c7 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -199,11 +199,7 @@ def _run_hooks(config, hooks, args, environ): retval |= _run_single_hook(filenames, hook, args, skips, cols) if retval and config['fail_fast']: break - if ( - retval and - args.show_diff_on_failure and - subprocess.call(('git', 'diff', '--quiet', '--no-ext-diff')) != 0 - ): + if retval and args.show_diff_on_failure and git.has_diff(): output.write_line('All changes made by hooks:') subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff')) return retval diff --git a/pre_commit/commands/try_repo.py b/pre_commit/commands/try_repo.py index e964987c..c9849ea4 100644 --- a/pre_commit/commands/try_repo.py +++ b/pre_commit/commands/try_repo.py @@ -2,6 +2,7 @@ from __future__ import absolute_import from __future__ import unicode_literals import collections +import logging import os.path from aspy.yaml import ordered_dump @@ -12,23 +13,50 @@ from pre_commit import output from pre_commit.clientlib import load_manifest from pre_commit.commands.run import run from pre_commit.store import Store +from pre_commit.util import cmd_output from pre_commit.util import tmpdir +logger = logging.getLogger(__name__) + + +def _repo_ref(tmpdir, repo, ref): + # if `ref` is explicitly passed, use it + if ref: + return repo, ref + + ref = git.head_rev(repo) + # if it exists on disk, we'll try and clone it with the local changes + if os.path.exists(repo) and git.has_diff('HEAD', repo=repo): + logger.warning('Creating temporary repo with uncommitted changes...') + + shadow = os.path.join(tmpdir, 'shadow-repo') + cmd_output('git', 'clone', repo, shadow) + cmd_output('git', 'checkout', ref, '-b', '_pc_tmp', cwd=shadow) + idx = git.git_path('index', repo=shadow) + objs = git.git_path('objects', repo=shadow) + env = dict(os.environ, GIT_INDEX_FILE=idx, GIT_OBJECT_DIRECTORY=objs) + cmd_output('git', 'add', '-u', cwd=repo, env=env) + git.commit(repo=shadow) + + return shadow, git.head_rev(shadow) + else: + return repo, ref + def try_repo(args): - ref = args.ref or git.head_rev(args.repo) - with tmpdir() as tempdir: + repo, ref = _repo_ref(tempdir, args.repo, args.ref) + store = Store(tempdir) if args.hook: hooks = [{'id': args.hook}] else: - repo_path = store.clone(args.repo, ref) + repo_path = store.clone(repo, ref) manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE)) manifest = sorted(manifest, key=lambda hook: hook['id']) hooks = [{'id': hook['id']} for hook in manifest] - items = (('repo', args.repo), ('rev', ref), ('hooks', hooks)) + items = (('repo', repo), ('rev', ref), ('hooks', hooks)) config = {'repos': [collections.OrderedDict(items)]} config_s = ordered_dump(config, **C.YAML_DUMP_KWARGS) diff --git a/pre_commit/git.py b/pre_commit/git.py index 84db66ea..ccdd1856 100644 --- a/pre_commit/git.py +++ b/pre_commit/git.py @@ -4,12 +4,10 @@ import logging import os.path import sys -from pre_commit.error_handler import FatalError -from pre_commit.util import CalledProcessError from pre_commit.util import cmd_output -logger = logging.getLogger('pre_commit') +logger = logging.getLogger(__name__) def zsplit(s): @@ -20,14 +18,23 @@ def zsplit(s): return [] +def no_git_env(): + # Too many bugs dealing with environment variables and GIT: + # https://github.com/pre-commit/pre-commit/issues/300 + # In git 2.6.3 (maybe others), git exports GIT_WORK_TREE while running + # pre-commit hooks + # In git 1.9.1 (maybe others), git exports GIT_DIR and GIT_INDEX_FILE + # while running pre-commit hooks in submodules. + # GIT_DIR: Causes git clone to clone wrong thing + # GIT_INDEX_FILE: Causes 'error invalid object ...' during commit + return { + k: v for k, v in os.environ.items() + if not k.startswith('GIT_') or k in {'GIT_SSH'} + } + + def get_root(): - try: - return cmd_output('git', 'rev-parse', '--show-toplevel')[1].strip() - except CalledProcessError: - raise FatalError( - 'git failed. Is it installed, and are you in a Git repository ' - 'directory?', - ) + return cmd_output('git', 'rev-parse', '--show-toplevel')[1].strip() def get_git_dir(git_root='.'): @@ -106,6 +113,27 @@ def head_rev(remote): return out.split()[0] +def has_diff(*args, **kwargs): + repo = kwargs.pop('repo', '.') + assert not kwargs, kwargs + cmd = ('git', 'diff', '--quiet', '--no-ext-diff') + args + return cmd_output(*cmd, cwd=repo, retcode=None)[0] + + +def commit(repo='.'): + env = no_git_env() + name, email = 'pre-commit', 'asottile+pre-commit@umich.edu' + env['GIT_AUTHOR_NAME'] = env['GIT_COMMITTER_NAME'] = name + env['GIT_AUTHOR_EMAIL'] = env['GIT_COMMITTER_EMAIL'] = email + cmd = ('git', 'commit', '--no-edit', '--no-gpg-sign', '-n', '-minit') + cmd_output(*cmd, cwd=repo, env=env) + + +def git_path(name, repo='.'): + _, out, _ = cmd_output('git', 'rev-parse', '--git-path', name, cwd=repo) + return os.path.join(repo, out.strip()) + + def check_for_cygwin_mismatch(): """See https://github.com/pre-commit/pre-commit/issues/354""" if sys.platform in ('cygwin', 'win32'): # pragma: no cover (windows) diff --git a/pre_commit/main.py b/pre_commit/main.py index 99f34070..71995f15 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -19,8 +19,10 @@ from pre_commit.commands.run import run from pre_commit.commands.sample_config import sample_config from pre_commit.commands.try_repo import try_repo from pre_commit.error_handler import error_handler +from pre_commit.error_handler import FatalError from pre_commit.logging_handler import add_logging_handler from pre_commit.store import Store +from pre_commit.util import CalledProcessError logger = logging.getLogger('pre_commit') @@ -97,7 +99,13 @@ def _adjust_args_and_chdir(args): if args.command == 'try-repo' and os.path.exists(args.repo): args.repo = os.path.abspath(args.repo) - os.chdir(git.get_root()) + try: + os.chdir(git.get_root()) + except CalledProcessError: + raise FatalError( + 'git failed. Is it installed, and are you in a Git repository ' + 'directory?', + ) args.config = os.path.relpath(args.config) if args.command in {'run', 'try-repo'}: diff --git a/pre_commit/store.py b/pre_commit/store.py index f3096fcd..3200a567 100644 --- a/pre_commit/store.py +++ b/pre_commit/store.py @@ -9,9 +9,9 @@ import tempfile import pre_commit.constants as C from pre_commit import file_lock +from pre_commit import git from pre_commit.util import clean_path_on_failure from pre_commit.util import cmd_output -from pre_commit.util import no_git_env from pre_commit.util import resource_text @@ -135,7 +135,7 @@ class Store(object): def clone(self, repo, ref, deps=()): """Clone the given url and checkout the specific ref.""" def clone_strategy(directory): - env = no_git_env() + env = git.no_git_env() cmd = ('git', 'clone', '--no-checkout', repo, directory) cmd_output(*cmd, env=env) @@ -160,10 +160,7 @@ class Store(object): with io.open(os.path.join(directory, resource), 'w') as f: f.write(contents) - env = no_git_env() - name, email = 'pre-commit', 'asottile+pre-commit@umich.edu' - env['GIT_AUTHOR_NAME'] = env['GIT_COMMITTER_NAME'] = name - env['GIT_AUTHOR_EMAIL'] = env['GIT_COMMITTER_EMAIL'] = email + env = git.no_git_env() # initialize the git repository so it looks more like cloned repos def _git_cmd(*args): @@ -172,7 +169,7 @@ class Store(object): _git_cmd('init', '.') _git_cmd('config', 'remote.origin.url', '<>') _git_cmd('add', '.') - _git_cmd('commit', '--no-edit', '--no-gpg-sign', '-n', '-minit') + git.commit(repo=directory) return self._new_repo( 'local', C.LOCAL_REPO_VERSION, deps, make_local_strategy, diff --git a/pre_commit/util.py b/pre_commit/util.py index 963461d1..c38af5a2 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -64,21 +64,6 @@ def noop_context(): yield -def no_git_env(): - # Too many bugs dealing with environment variables and GIT: - # https://github.com/pre-commit/pre-commit/issues/300 - # In git 2.6.3 (maybe others), git exports GIT_WORK_TREE while running - # pre-commit hooks - # In git 1.9.1 (maybe others), git exports GIT_DIR and GIT_INDEX_FILE - # while running pre-commit hooks in submodules. - # GIT_DIR: Causes git clone to clone wrong thing - # GIT_INDEX_FILE: Causes 'error invalid object ...' during commit - return { - k: v for k, v in os.environ.items() - if not k.startswith('GIT_') or k in {'GIT_SSH'} - } - - @contextlib.contextmanager def tmpdir(): """Contextmanager to create a temporary directory. It will be cleaned up diff --git a/testing/fixtures.py b/testing/fixtures.py index 74fe517b..b0606ee4 100644 --- a/testing/fixtures.py +++ b/testing/fixtures.py @@ -53,7 +53,7 @@ def make_repo(tempdir_factory, repo_source): @contextlib.contextmanager -def modify_manifest(path): +def modify_manifest(path, commit=True): """Modify the manifest yielded by this context to write to .pre-commit-hooks.yaml. """ @@ -63,7 +63,8 @@ def modify_manifest(path): yield manifest with io.open(manifest_path, 'w') as manifest_file: manifest_file.write(ordered_dump(manifest, **C.YAML_DUMP_KWARGS)) - git_commit(msg=modify_manifest.__name__, cwd=path) + if commit: + git_commit(msg=modify_manifest.__name__, cwd=path) @contextlib.contextmanager diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py index 66d1642d..5b50f420 100644 --- a/tests/commands/try_repo_test.py +++ b/tests/commands/try_repo_test.py @@ -4,12 +4,15 @@ from __future__ import unicode_literals import os.path import re +from pre_commit import git from pre_commit.commands.try_repo import try_repo from pre_commit.util import cmd_output from testing.auto_namedtuple import auto_namedtuple from testing.fixtures import git_dir from testing.fixtures import make_repo +from testing.fixtures import modify_manifest from testing.util import cwd +from testing.util import git_commit from testing.util import run_opts @@ -21,22 +24,26 @@ def _get_out(cap_out): out = cap_out.get().replace('\r\n', '\n') out = re.sub(r'\[INFO\].+\n', '', out) start, using_config, config, rest = out.split('=' * 79 + '\n') - assert start == '' assert using_config == 'Using config:\n' - return config, rest + return start, config, rest + + +def _add_test_file(): + open('test-file', 'a').close() + cmd_output('git', 'add', '.') def _run_try_repo(tempdir_factory, **kwargs): repo = make_repo(tempdir_factory, 'modified_file_returns_zero_repo') with cwd(git_dir(tempdir_factory)): - open('test-file', 'a').close() - cmd_output('git', 'add', '.') + _add_test_file() assert not try_repo(try_repo_opts(repo, **kwargs)) def test_try_repo_repo_only(cap_out, tempdir_factory): _run_try_repo(tempdir_factory, verbose=True) - config, rest = _get_out(cap_out) + start, config, rest = _get_out(cap_out) + assert start == '' assert re.match( '^repos:\n' '- repo: .+\n' @@ -48,19 +55,20 @@ def test_try_repo_repo_only(cap_out, tempdir_factory): config, ) assert rest == ( - '[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa - '[bash_hook2] Bash hook...................................................Passed\n' # noqa + '[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa: E501 + '[bash_hook2] Bash hook...................................................Passed\n' # noqa: E501 'hookid: bash_hook2\n' '\n' 'test-file\n' '\n' - '[bash_hook3] Bash hook...............................(no files to check)Skipped\n' # noqa + '[bash_hook3] Bash hook...............................(no files to check)Skipped\n' # noqa: E501 ) def test_try_repo_with_specific_hook(cap_out, tempdir_factory): _run_try_repo(tempdir_factory, hook='bash_hook', verbose=True) - config, rest = _get_out(cap_out) + start, config, rest = _get_out(cap_out) + assert start == '' assert re.match( '^repos:\n' '- repo: .+\n' @@ -69,14 +77,49 @@ def test_try_repo_with_specific_hook(cap_out, tempdir_factory): ' - id: bash_hook\n$', config, ) - assert rest == '[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa + assert rest == '[bash_hook] Bash hook................................(no files to check)Skipped\n' # noqa: E501 def test_try_repo_relative_path(cap_out, tempdir_factory): repo = make_repo(tempdir_factory, 'modified_file_returns_zero_repo') with cwd(git_dir(tempdir_factory)): - open('test-file', 'a').close() - cmd_output('git', 'add', '.') + _add_test_file() relative_repo = os.path.relpath(repo, '.') # previously crashed on cloning a relative path assert not try_repo(try_repo_opts(relative_repo, hook='bash_hook')) + + +def test_try_repo_specific_revision(cap_out, tempdir_factory): + repo = make_repo(tempdir_factory, 'script_hooks_repo') + ref = git.head_rev(repo) + git_commit(cwd=repo) + with cwd(git_dir(tempdir_factory)): + _add_test_file() + assert not try_repo(try_repo_opts(repo, ref=ref)) + + _, config, _ = _get_out(cap_out) + assert ref in config + + +def test_try_repo_uncommitted_changes(cap_out, tempdir_factory): + repo = make_repo(tempdir_factory, 'script_hooks_repo') + # make an uncommitted change + with modify_manifest(repo, commit=False) as manifest: + manifest[0]['name'] = 'modified name!' + + with cwd(git_dir(tempdir_factory)): + open('test-fie', 'a').close() + cmd_output('git', 'add', '.') + assert not try_repo(try_repo_opts(repo)) + + start, config, rest = _get_out(cap_out) + assert start == '[WARNING] Creating temporary repo with uncommitted changes...\n' # noqa: E501 + assert re.match( + '^repos:\n' + '- repo: .+shadow-repo\n' + ' rev: .+\n' + ' hooks:\n' + ' - id: bash_hook\n$', + config, + ) + assert rest == 'modified name!...........................................................Passed\n' # noqa: E501 diff --git a/tests/git_test.py b/tests/git_test.py index cb8a2bf1..a78b7458 100644 --- a/tests/git_test.py +++ b/tests/git_test.py @@ -7,7 +7,6 @@ import os.path import pytest from pre_commit import git -from pre_commit.error_handler import FatalError from pre_commit.util import cmd_output from testing.util import git_commit @@ -23,11 +22,6 @@ def test_get_root_deeper(in_git_dir): assert os.path.normcase(git.get_root()) == expected -def test_get_root_not_git_dir(in_tmpdir): - with pytest.raises(FatalError): - git.get_root() - - def test_get_staged_files_deleted(in_git_dir): in_git_dir.join('test').ensure() cmd_output('git', 'add', 'test') diff --git a/tests/main_test.py b/tests/main_test.py index 83758bf4..c5db3da1 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -9,6 +9,7 @@ import pytest import pre_commit.constants as C from pre_commit import main +from pre_commit.error_handler import FatalError from testing.auto_namedtuple import auto_namedtuple @@ -19,6 +20,11 @@ class Args(object): self.__dict__.update(kwargs) +def test_adjust_args_and_chdir_not_in_git_dir(in_tmpdir): + with pytest.raises(FatalError): + main._adjust_args_and_chdir(Args()) + + def test_adjust_args_and_chdir_noop(in_git_dir): args = Args(command='run', files=['f1', 'f2']) main._adjust_args_and_chdir(args)