Merge pull request #497 from pre-commit/future_versions

Make hook-tmpl resilient to future changes

pre_commit/commands/install_uninstall.py

@@ -12,29 +12,21 @@ from pre_commit.util import resource_filename
 
 
 # This is used to identify the hook file we install
-PREVIOUS_IDENTIFYING_HASHES = (
+PRIOR_HASHES = (
     '4d9958c90bc262f47553e2c073f14cfe',
     'd8ee923c46731b42cd95cc869add4062',
     '49fd668cb42069aa1b6048464be5d395',
     '79f09a650522a87b0da915d0d983b2de',
     'e358c9dae00eac5d06b38dfdb1e33a8c',
 )
+CURRENT_HASH = '138fd403232d2ddd5efb44317e38bf03'
 
 
-IDENTIFYING_HASH = '138fd403232d2ddd5efb44317e38bf03'
+def is_our_script(filename):
-
-
-def is_our_pre_commit(filename):
-    if not os.path.exists(filename):
-        return False
-    return IDENTIFYING_HASH in io.open(filename).read()
-
-
-def is_previous_pre_commit(filename):
     if not os.path.exists(filename):
         return False
     contents = io.open(filename).read()
-    return any(hash in contents for hash in PREVIOUS_IDENTIFYING_HASHES)
+    return any(h in contents for h in (CURRENT_HASH,) + PRIOR_HASHES)
 
 
 def install(
@@ -48,11 +40,7 @@ def install(
     mkdirp(os.path.dirname(hook_path))
 
     # If we have an existing hook, move it to pre-commit.legacy
-    if (
+    if os.path.lexists(hook_path) and not is_our_script(hook_path):
-        os.path.lexists(hook_path) and
-        not is_our_pre_commit(hook_path) and
-        not is_previous_pre_commit(hook_path)
-    ):
         os.rename(hook_path, legacy_path)
 
     # If we specify overwrite, we simply delete the legacy file
@@ -102,12 +90,7 @@ def uninstall(runner, hook_type='pre-commit'):
     hook_path = runner.get_hook_path(hook_type)
     legacy_path = hook_path + '.legacy'
     # If our file doesn't exist or it isn't ours, gtfo.
-    if (
+    if not os.path.exists(hook_path) or not is_our_script(hook_path):
-        not os.path.exists(hook_path) or (
-            not is_our_pre_commit(hook_path) and
-            not is_previous_pre_commit(hook_path)
-        )
-    ):
         return 0
 
     os.remove(hook_path)

requirements-dev.txt

@@ -4,6 +4,7 @@ coverage
 flake8
 mock
 pytest
+pytest-env
 
 # setuptools breaks pypy3 with extraneous output
 setuptools<18.5

tests/commands/install_uninstall_test.py

@@ -11,12 +11,11 @@ import sys
 import mock
 
 import pre_commit.constants as C
-from pre_commit.commands.install_uninstall import IDENTIFYING_HASH
+from pre_commit.commands.install_uninstall import CURRENT_HASH
 from pre_commit.commands.install_uninstall import install
 from pre_commit.commands.install_uninstall import install_hooks
-from pre_commit.commands.install_uninstall import is_our_pre_commit
+from pre_commit.commands.install_uninstall import is_our_script
-from pre_commit.commands.install_uninstall import is_previous_pre_commit
+from pre_commit.commands.install_uninstall import PRIOR_HASHES
-from pre_commit.commands.install_uninstall import PREVIOUS_IDENTIFYING_HASHES
 from pre_commit.commands.install_uninstall import uninstall
 from pre_commit.runner import Runner
 from pre_commit.util import cmd_output
@@ -31,27 +30,18 @@ from testing.util import cmd_output_mocked_pre_commit_home
 from testing.util import xfailif_no_symlink
 
 
-def test_is_not_our_pre_commit():
+def test_is_not_script():
-    assert is_our_pre_commit('setup.py') is False
+    assert is_our_script('setup.py') is False
 
 
-def test_is_our_pre_commit():
+def test_is_script():
-    assert is_our_pre_commit(resource_filename('hook-tmpl'))
+    assert is_our_script(resource_filename('hook-tmpl'))
 
 
-def test_is_not_previous_pre_commit():
+def test_is_previous_pre_commit(tmpdir):
-    assert is_previous_pre_commit('setup.py') is False
+    f = tmpdir.join('foo')
-
+    f.write(PRIOR_HASHES[0] + '\n')
-
+    assert is_our_script(f.strpath)
-def test_is_also_not_previous_pre_commit():
-    assert not is_previous_pre_commit(resource_filename('hook-tmpl'))
-
-
-def test_is_previous_pre_commit(in_tmpdir):
-    with io.open('foo', 'w') as foo_file:
-        foo_file.write(PREVIOUS_IDENTIFYING_HASHES[0])
-
-    assert is_previous_pre_commit('foo')
 
 
 def test_install_pre_commit(tempdir_factory):
@@ -411,7 +401,7 @@ def test_replace_old_commit_script(tempdir_factory):
             resource_filename('hook-tmpl'),
         ).read()
         new_contents = pre_commit_contents.replace(
-            IDENTIFYING_HASH, PREVIOUS_IDENTIFYING_HASHES[-1],
+            CURRENT_HASH, PRIOR_HASHES[-1],
         )
 
         mkdirp(os.path.dirname(runner.pre_commit_path))

tox.ini

@@ -6,12 +6,6 @@ envlist = py27,py34,py35,pypy
 [testenv]
 deps = -rrequirements-dev.txt
 passenv = GOROOT HOME HOMEPATH PROGRAMDATA TERM
-setenv =
-    VIRTUALENV_NO_DOWNLOAD = 1
-    GIT_AUTHOR_NAME = "test"
-    GIT_COMMITTER_NAME = "test"
-    GIT_AUTHOR_EMAIL = "test@example.com"
-    GIT_COMMITTER_EMAIL = "test@example.com"
 commands =
     coverage erase
     coverage run -m pytest {posargs:tests}
@@ -25,3 +19,11 @@ commands =
 
 [pep8]
 ignore = E265,E309,E501
+
+[pytest]
+env =
+    GIT_AUTHOR_NAME=test
+    GIT_COMMITTER_NAME=test
+    GIT_AUTHOR_EMAIL=test@example.com
+    GIT_COMMITTER_EMAIL=test@example.com
+    VIRTUALENV_NO_DOWNLOAD=1