Merge pull request #193 from pre-commit/control_c_during_install_more_atmoic

Make ^C^C during install not cause subsequent runs to fail.  Resolves #186.

pre_commit/repository.py

@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+import shutil
+
 from cached_property import cached_property
 
 from pre_commit.languages.all import languages
@@ -64,11 +66,21 @@ class Repository(object):
             language = languages[language_name]
             if (
                 language.ENVIRONMENT_DIR is None or
-                self.cmd_runner.exists(language.ENVIRONMENT_DIR)
+                self.cmd_runner.exists(language.ENVIRONMENT_DIR, '.installed')
             ):
                 # The language is already installed
                 continue
+            # There's potentially incomplete cleanup from previous runs
+            # Clean it up!
+            if self.cmd_runner.exists(language.ENVIRONMENT_DIR):
+                shutil.rmtree(self.cmd_runner.path(language.ENVIRONMENT_DIR))
+
             language.install_environment(self.cmd_runner, language_version)
+            # Touch the .installed file (atomic) to indicate we've installed
+            open(
+                self.cmd_runner.path(language.ENVIRONMENT_DIR, '.installed'),
+                'w',
+            ).close()
 
     def run_hook(self, hook, file_args):
         """Run a hook.

tests/repository_test.py

@@ -3,6 +3,7 @@ from __future__ import unicode_literals
 
 import io
 import os.path
+import shutil
 
 import mock
 import pytest
@@ -10,6 +11,7 @@ import pytest
 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.languages.python import PythonEnv
 from pre_commit.repository import Repository
 from pre_commit.util import cmd_output
 from pre_commit.util import cwd
@@ -266,6 +268,35 @@ def test_reinstall(tmpdir_factory, store):
     repo.require_installed()
 
 
+def test_control_c_control_c_on_install(tmpdir_factory, store):
+    """Regression test for #186."""
+    path = make_repo(tmpdir_factory, 'python_hooks_repo')
+    config = make_config_from_repo(path)
+    repo = Repository.create(config, store)
+    hook = repo.hooks[0][1]
+
+    class MyKeyboardInterrupt(KeyboardInterrupt):
+        pass
+
+    # To simulate a killed install, we'll make PythonEnv.run raise ^C
+    # and then to simulate a second ^C during cleanup, we'll make shutil.rmtree
+    # raise as well.
+    with pytest.raises(MyKeyboardInterrupt):
+        with mock.patch.object(
+            PythonEnv, 'run', side_effect=MyKeyboardInterrupt,
+        ):
+            with mock.patch.object(shutil, 'rmtree', MyKeyboardInterrupt):
+                repo.run_hook(hook, [])
+
+    # Should have made an environment, however this environment is broken!
+    assert os.path.exists(repo.cmd_runner.path('py_env'))
+
+    # However, it should be perfectly runnable (reinstall after botched
+    # install)
+    retv, stdout, stderr = repo.run_hook(hook, [])
+    assert retv == 0
+
+
 @pytest.mark.integration
 def test_really_long_file_paths(tmpdir_factory, store):
     base_path = tmpdir_factory.get()