Merge pull request #904 from asottile/clone_uncommitted

Teach pre-commit try-repo to clone uncommitted changes

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 (
+    if retval and args.show_diff_on_failure and git.has_diff():
-            retval and
-            args.show_diff_on_failure and
-            subprocess.call(('git', 'diff', '--quiet', '--no-ext-diff')) != 0
-    ):
         output.write_line('All changes made by hooks:')
         subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff'))
     return retval

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)
 

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()
-        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?',
-        )
 
 
 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)

pre_commit/logging_handler.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import contextlib
 import logging
 
 from pre_commit import color
@@ -34,6 +35,12 @@ class LoggingHandler(logging.Handler):
         )
 
 
-def add_logging_handler(*args, **kwargs):
+@contextlib.contextmanager
-    logger.addHandler(LoggingHandler(*args, **kwargs))
+def logging_handler(*args, **kwargs):
+    handler = LoggingHandler(*args, **kwargs)
+    logger.addHandler(handler)
     logger.setLevel(logging.INFO)
+    try:
+        yield
+    finally:
+        logger.removeHandler(handler)

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.logging_handler import add_logging_handler
+from pre_commit.error_handler import FatalError
+from pre_commit.logging_handler import 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'}:
@@ -240,9 +248,7 @@ def main(argv=None):
     elif args.command == 'help':
         parser.parse_args(['--help'])
 
-    with error_handler():
+    with error_handler(), logging_handler(args.color):
-        add_logging_handler(args.color)
-
         _adjust_args_and_chdir(args)
 
         store = Store()

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()
+            env = git.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
 
             # 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', '<<unknown>>')
             _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,

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

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

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()
+        _add_test_file()
-        cmd_output('git', 'add', '.')
         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_hook] Bash hook................................(no files to check)Skipped\n'  # noqa: E501
-        '[bash_hook2] Bash hook...................................................Passed\n'  # noqa
+        '[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()
+        _add_test_file()
-        cmd_output('git', 'add', '.')
         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

tests/conftest.py

@@ -11,7 +11,7 @@ import pytest
 import six
 
 from pre_commit import output
-from pre_commit.logging_handler import add_logging_handler
+from pre_commit.logging_handler import logging_handler
 from pre_commit.store import Store
 from pre_commit.util import cmd_output
 from testing.fixtures import git_dir
@@ -155,7 +155,8 @@ def dont_write_to_home_directory():
 
 @pytest.fixture(autouse=True, scope='session')
 def configure_logging():
-    add_logging_handler(use_color=False)
+    with logging_handler(use_color=False):
+        yield
 
 
 @pytest.fixture

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')

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)