Repository: pre-commit/pre-commit (python)

Issue:
Support tags as well as shas
They kind-of work as-is unless tags collide across different hooks.

The simplest solution is to make the repository symlinks `repo + ref` instead of just `ref` as they are right now.

Then anything that git understands as `git checkout {0}` will work.


Pull request description:
Migrate sha -> rev
Resolves #106 

The ticket mentions "ref", but I think "rev" is perhaps a better name?  Would love to get input here

This will be a slow migration, and during that time both `sha` and `rev` will be supported (but not both!)

I was able to leverage a [custom `cfgv` validator](https://github.com/asottile/cfgv#writing-your-own-validator) to accomplish this cleanly.

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 11f0ac2..e4888a0 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -13,16 +13,16 @@ repos:
     -   id: requirements-txt-fixer
     -   id: flake8
 -   repo: https://github.com/pre-commit/pre-commit.git
-    sha: v0.16.3
+    rev: v0.16.3
     hooks:
     -   id: validate_manifest
 -   repo: https://github.com/asottile/reorder_python_imports.git
-    sha: v0.3.5
+    rev: v0.3.5
     hooks:
     -   id: reorder-python-imports
         language_version: python2.7
 -   repo: https://github.com/asottile/add-trailing-comma
-    sha: v0.6.4
+    rev: v0.6.4
     hooks:
     -   id: add-trailing-comma
 -   repo: meta
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
index bb77234..f6f8619 100644
--- a/pre_commit/clientlib.py
+++ b/pre_commit/clientlib.py
@@ -93,6 +93,36 @@ def validate_manifest_main(argv=None):
 _LOCAL_SENTINEL = 'local'
 _META_SENTINEL = 'meta'
 
+
+class MigrateShaToRev(object):
+    @staticmethod
+    def _cond(key):
+        return cfgv.Conditional(
+            key, cfgv.check_string,
+            condition_key='repo',
+            condition_value=cfgv.NotIn(_LOCAL_SENTINEL, _META_SENTINEL),
+            ensure_absent=True,
+        )
+
+    def check(self, dct):
+        if dct.get('repo') in {_LOCAL_SENTINEL, _META_SENTINEL}:
+            self._cond('rev').check(dct)
+            self._cond('sha').check(dct)
+        elif 'sha' in dct and 'rev' in dct:
+            raise cfgv.ValidationError('Cannot specify both sha and rev')
+        elif 'sha' in dct:
+            self._cond('sha').check(dct)
+        else:
+            self._cond('rev').check(dct)
+
+    def apply_default(self, dct):
+        if 'sha' in dct:
+            dct['rev'] = dct.pop('sha')
+
+    def remove_default(self, dct):
+        pass
+
+
 CONFIG_HOOK_DICT = cfgv.Map(
     'Hook', 'id',
 
@@ -114,12 +144,7 @@ CONFIG_REPO_DICT = cfgv.Map(
     cfgv.Required('repo', cfgv.check_string),
     cfgv.RequiredRecurse('hooks', cfgv.Array(CONFIG_HOOK_DICT)),
 
-    cfgv.Conditional(
-        'sha', cfgv.check_string,
-        condition_key='repo',
-        condition_value=cfgv.NotIn(_LOCAL_SENTINEL, _META_SENTINEL),
-        ensure_absent=True,
-    ),
+    MigrateShaToRev(),
 )
 CONFIG_SCHEMA = cfgv.Map(
     'Config', None,
diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py
index 9da4027..cdaccfc 100644
--- a/pre_commit/commands/autoupdate.py
+++ b/pre_commit/commands/autoupdate.py
@@ -32,7 +32,7 @@ def _update_repo(repo_config, runner, tags_only):
     Args:
         repo_config - A config for a repository
     """
-    repo_path = runner.store.clone(repo_config['repo'], repo_config['sha'])
+    repo_path = runner.store.clone(repo_config['repo'], repo_config['rev'])
 
     cmd_output('git', '-C', repo_path, 'fetch')
     tag_cmd = ('git', '-C', repo_path, 'describe', 'origin/master', '--tags')
@@ -46,13 +46,13 @@ def _update_repo(repo_config, runner, tags_only):
         tag_cmd = ('git', '-C', repo_path, 'rev-parse', 'origin/master')
         rev = cmd_output(*tag_cmd)[1].strip()
 
-    # Don't bother trying to update if our sha is the same
-    if rev == repo_config['sha']:
+    # Don't bother trying to update if our rev is the same
+    if rev == repo_config['rev']:
         return repo_config
 
-    # Construct a new config with the head sha
+    # Construct a new config with the head rev
     new_config = OrderedDict(repo_config)
-    new_config['sha'] = rev
+    new_config['rev'] = rev
     new_repo = Repository.create(new_config, runner.store)
 
     # See if any of our hooks were deleted with the new commits
@@ -67,8 +67,8 @@ def _update_repo(repo_config, runner, tags_only):
     return new_config
 
 
-SHA_LINE_RE = re.compile(r'^(\s+)sha:(\s*)([^\s#]+)(.*)$', re.DOTALL)
-SHA_LINE_FMT = '{}sha:{}{}{}'
+REV_LINE_RE = re.compile(r'^(\s+)rev:(\s*)([^\s#]+)(.*)$', re.DOTALL)
+REV_LINE_FMT = '{}rev:{}{}{}'
 
 
 def _write_new_config_file(path, output):
@@ -77,25 +77,25 @@ def _write_new_config_file(path, output):
     new_contents = ordered_dump(output, **C.YAML_DUMP_KWARGS)
 
     lines = original_contents.splitlines(True)
-    sha_line_indices_rev = list(reversed([
-        i for i, line in enumerate(lines) if SHA_LINE_RE.match(line)
+    rev_line_indices_reversed = list(reversed([
+        i for i, line in enumerate(lines) if REV_LINE_RE.match(line)
     ]))
 
     for line in new_contents.splitlines(True):
-        if SHA_LINE_RE.match(line):
-            # It's possible we didn't identify the sha lines in the original
-            if not sha_line_indices_rev:
+        if REV_LINE_RE.match(line):
+            # It's possible we didn't identify the rev lines in the original
+            if not rev_line_indices_reversed:
                 break
-            line_index = sha_line_indices_rev.pop()
+            line_index = rev_line_indices_reversed.pop()
             original_line = lines[line_index]
-            orig_match = SHA_LINE_RE.match(original_line)
-            new_match = SHA_LINE_RE.match(line)
-            lines[line_index] = SHA_LINE_FMT.format(
+            orig_match = REV_LINE_RE.match(original_line)
+            new_match = REV_LINE_RE.match(line)
+            lines[line_index] = REV_LINE_FMT.format(
                 orig_match.group(1), orig_match.group(2),
                 new_match.group(3), orig_match.group(4),
             )
 
-    # If we failed to intelligently rewrite the sha lines, fall back to the
+    # If we failed to intelligently rewrite the rev lines, fall back to the
     # pretty-formatted yaml output
     to_write = ''.join(lines)
     if remove_defaults(ordered_load(to_write), CONFIG_SCHEMA) != output:
@@ -132,10 +132,10 @@ def autoupdate(runner, tags_only, repos=()):
             retv = 1
             continue
 
-        if new_repo_config['sha'] != repo_config['sha']:
+        if new_repo_config['rev'] != repo_config['rev']:
             changed = True
             output.write_line('updating {} -> {}.'.format(
-                repo_config['sha'], new_repo_config['sha'],
+                repo_config['rev'], new_repo_config['rev'],
             ))
             output_repos.append(new_repo_config)
         else:
diff --git a/pre_commit/commands/migrate_config.py b/pre_commit/commands/migrate_config.py
index 50f0c2d..193a002 100644
--- a/pre_commit/commands/migrate_config.py
+++ b/pre_commit/commands/migrate_config.py
@@ -2,6 +2,7 @@ from __future__ import print_function
 from __future__ import unicode_literals
 
 import io
+import re
 
 import yaml
 from aspy.yaml import ordered_load
@@ -16,10 +17,7 @@ def _is_header_line(line):
     return (line.startswith(('#', '---')) or not line.strip())
 
 
-def migrate_config(runner, quiet=False):
-    with io.open(runner.config_file_path) as f:
-        contents = f.read()
-
+def _migrate_map(contents):
     # Find the first non-header line
     lines = contents.splitlines(True)
     i = 0
@@ -39,6 +37,22 @@ def migrate_config(runner, quiet=False):
         except yaml.YAMLError:
             contents = header + 'repos:\n' + _indent(rest)
 
+    return contents
+
+
+def _migrate_sha_to_rev(contents):
+    reg = re.compile(r'(\n\s+)sha:')
+    return reg.sub(r'\1rev:', contents)
+
+
+def migrate_config(runner, quiet=False):
+    with io.open(runner.config_file_path) as f:
+        orig_contents = contents = f.read()
+
+    contents = _migrate_map(contents)
+    contents = _migrate_sha_to_rev(contents)
+
+    if contents != orig_contents:
         with io.open(runner.config_file_path, 'w') as f:
             f.write(contents)
 
diff --git a/pre_commit/commands/sample_config.py b/pre_commit/commands/sample_config.py
index ae59468..aef0107 100644
--- a/pre_commit/commands/sample_config.py
+++ b/pre_commit/commands/sample_config.py
@@ -12,7 +12,7 @@ SAMPLE_CONFIG = '''\
 # See https://pre-commit.com/hooks.html for more hooks
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    sha: v0.9.2
+    rev: v1.2.1-1
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
diff --git a/pre_commit/commands/try_repo.py b/pre_commit/commands/try_repo.py
index 4c82582..6815431 100644
--- a/pre_commit/commands/try_repo.py
+++ b/pre_commit/commands/try_repo.py
@@ -17,7 +17,7 @@ from pre_commit.util import tmpdir
 
 
 def try_repo(args):
-    ref = args.ref or git.head_sha(args.repo)
+    ref = args.ref or git.head_rev(args.repo)
 
     with tmpdir() as tempdir:
         if args.hook:
@@ -28,7 +28,7 @@ def try_repo(args):
             manifest = sorted(manifest, key=lambda hook: hook['id'])
             hooks = [{'id': hook['id']} for hook in manifest]
 
-        items = (('repo', args.repo), ('sha', ref), ('hooks', hooks))
+        items = (('repo', args.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 c38b83a..4fb2e65 100644
--- a/pre_commit/git.py
+++ b/pre_commit/git.py
@@ -97,7 +97,7 @@ def get_changed_files(new, old):
     )[1])
 
 
-def head_sha(remote):
+def head_rev(remote):
     _, out, _ = cmd_output('git', 'ls-remote', '--exit-code', remote, 'HEAD')
     return out.split()[0]
 
diff --git a/pre_commit/main.py b/pre_commit/main.py
index e2f48ed..9267714 100644
--- a/pre_commit/main.py
+++ b/pre_commit/main.py
@@ -200,9 +200,9 @@ def main(argv=None):
         'repo', help='Repository to source hooks from.',
     )
     try_repo_parser.add_argument(
-        '--ref',
+        '--ref', '--rev',
         help=(
-            'Manually select a ref to run against, otherwise the `HEAD` '
+            'Manually select a rev to run against, otherwise the `HEAD` '
             'revision will be used.'
         ),
     )
diff --git a/pre_commit/repository.py b/pre_commit/repository.py
index 624ccd0..0647d9d 100644
--- a/pre_commit/repository.py
+++ b/pre_commit/repository.py
@@ -150,8 +150,8 @@ class Repository(object):
 
     @cached_property
     def manifest_hooks(self):
-        repo, sha = self.repo_config['repo'], self.repo_config['sha']
-        repo_path = self.store.clone(repo, sha)
+        repo, rev = self.repo_config['repo'], self.repo_config['rev']
+        repo_path = self.store.clone(repo, rev)
         manifest_path = os.path.join(repo_path, C.MANIFEST_FILE)
         return {hook['id']: hook for hook in load_manifest(manifest_path)}
 
@@ -174,8 +174,8 @@ class Repository(object):
         )
 
     def _prefix_from_deps(self, language_name, deps):
-        repo, sha = self.repo_config['repo'], self.repo_config['sha']
-        return Prefix(self.store.clone(repo, sha, deps))
+        repo, rev = self.repo_config['repo'], self.repo_config['rev']
+        return Prefix(self.store.clone(repo, rev, deps))
 
     def _venvs(self):
         ret = []


Test patch:
diff --git a/testing/fixtures.py b/testing/fixtures.py
index 3537ca7..15c06df 100644
--- a/testing/fixtures.py
+++ b/testing/fixtures.py
@@ -78,11 +78,11 @@ def config_with_local_hooks():
     ))
 
 
-def make_config_from_repo(repo_path, sha=None, hooks=None, check=True):
+def make_config_from_repo(repo_path, rev=None, hooks=None, check=True):
     manifest = load_manifest(os.path.join(repo_path, C.MANIFEST_FILE))
     config = OrderedDict((
         ('repo', 'file://{}'.format(repo_path)),
-        ('sha', sha or git.head_sha(repo_path)),
+        ('rev', rev or git.head_rev(repo_path)),
         (
             'hooks',
             hooks or [OrderedDict((('id', hook['id']),)) for hook in manifest],
diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py
index 2f0b6fc..fcd34dc 100644
--- a/tests/clientlib_test.py
+++ b/tests/clientlib_test.py
@@ -8,6 +8,7 @@ from pre_commit.clientlib import CONFIG_HOOK_DICT
 from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import is_local_repo
 from pre_commit.clientlib import MANIFEST_SCHEMA
+from pre_commit.clientlib import MigrateShaToRev
 from pre_commit.clientlib import validate_config_main
 from pre_commit.clientlib import validate_manifest_main
 from testing.util import get_resource_path
@@ -49,7 +50,7 @@ def test_validate_config_main(args, expected_output):
         (
             {'repos': [{
                 'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-                'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
                 'hooks': [{'id': 'pyflakes', 'files': '\\.py$'}],
             }]},
             True,
@@ -57,7 +58,7 @@ def test_validate_config_main(args, expected_output):
         (
             {'repos': [{
                 'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-                'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
                 'hooks': [
                     {
                         'id': 'pyflakes',
@@ -71,7 +72,7 @@ def test_validate_config_main(args, expected_output):
         (
             {'repos': [{
                 'repo': 'git@github.com:pre-commit/pre-commit-hooks',
-                'sha': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
+                'rev': 'cd74dc150c142c3be70b24eaf0b02cae9d235f37',
                 'hooks': [
                     {
                         'id': 'pyflakes',
@@ -94,7 +95,7 @@ def test_config_valid(config_obj, expected):
 def test_config_with_local_hooks_definition_fails():
     config_obj = {'repos': [{
         'repo': 'local',
-        'sha': 'foo',
+        'rev': 'foo',
         'hooks': [{
             'id': 'do_not_commit',
             'name': 'Block if "DO NOT COMMIT" is found',
@@ -201,3 +202,45 @@ def test_validate_manifest_main(args, expected_output):
 def test_valid_manifests(manifest_obj, expected):
     ret = is_valid_according_to_schema(manifest_obj, MANIFEST_SCHEMA)
     assert ret is expected
+
+
+@pytest.mark.parametrize(
+    'dct',
+    (
+        {'repo': 'local'}, {'repo': 'meta'},
+        {'repo': 'wat', 'sha': 'wat'}, {'repo': 'wat', 'rev': 'wat'},
+    ),
+)
+def test_migrate_sha_to_rev_ok(dct):
+    MigrateShaToRev().check(dct)
+
+
+def test_migrate_sha_to_rev_dont_specify_both():
+    with pytest.raises(cfgv.ValidationError) as excinfo:
+        MigrateShaToRev().check({'repo': 'a', 'sha': 'b', 'rev': 'c'})
+    msg, = excinfo.value.args
+    assert msg == 'Cannot specify both sha and rev'
+
+
+@pytest.mark.parametrize(
+    'dct',
+    (
+        {'repo': 'a'},
+        {'repo': 'meta', 'sha': 'a'}, {'repo': 'meta', 'rev': 'a'},
+    ),
+)
+def test_migrate_sha_to_rev_conditional_check_failures(dct):
+    with pytest.raises(cfgv.ValidationError):
+        MigrateShaToRev().check(dct)
+
+
+def test_migrate_to_sha_apply_default():
+    dct = {'repo': 'a', 'sha': 'b'}
+    MigrateShaToRev().apply_default(dct)
+    assert dct == {'repo': 'a', 'rev': 'b'}
+
+
+def test_migrate_to_sha_ok():
+    dct = {'repo': 'a', 'rev': 'b'}
+    MigrateShaToRev().apply_default(dct)
+    assert dct == {'repo': 'a', 'rev': 'b'}
diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py
index e7fa666..0c6ffba 100644
--- a/tests/commands/autoupdate_test.py
+++ b/tests/commands/autoupdate_test.py
@@ -32,9 +32,9 @@ def up_to_date_repo(tempdir_factory):
 
 def test_up_to_date_repo(up_to_date_repo, runner_with_mocked_store):
     config = make_config_from_repo(up_to_date_repo)
-    input_sha = config['sha']
+    input_rev = config['rev']
     ret = _update_repo(config, runner_with_mocked_store, tags_only=False)
-    assert ret['sha'] == input_sha
+    assert ret['rev'] == input_rev
 
 
 def test_autoupdate_up_to_date_repo(
@@ -65,12 +65,12 @@ def test_autoupdate_old_revision_broken(
     cmd_output('git', '-C', path, 'mv', C.MANIFEST_FILE, 'nope.yaml')
     cmd_output('git', '-C', path, 'commit', '-m', 'simulate old repo')
     # Assume this is the revision the user's old repository was at
-    rev = git.head_sha(path)
+    rev = git.head_rev(path)
     cmd_output('git', '-C', path, 'mv', 'nope.yaml', C.MANIFEST_FILE)
     cmd_output('git', '-C', path, 'commit', '-m', 'move hooks file')
-    update_rev = git.head_sha(path)
+    update_rev = git.head_rev(path)
 
-    config['sha'] = rev
+    config['rev'] = rev
     write_config('.', config)
     before = open(C.CONFIG_FILE).read()
     ret = autoupdate(Runner('.', C.CONFIG_FILE), tags_only=False)
@@ -83,24 +83,24 @@ def test_autoupdate_old_revision_broken(
 @pytest.fixture
 def out_of_date_repo(tempdir_factory):
     path = make_repo(tempdir_factory, 'python_hooks_repo')
-    original_sha = git.head_sha(path)
+    original_rev = git.head_rev(path)
 
     # Make a commit
     cmd_output('git', '-C', path, 'commit', '--allow-empty', '-m', 'foo')
-    head_sha = git.head_sha(path)
+    head_rev = git.head_rev(path)
 
     yield auto_namedtuple(
-        path=path, original_sha=original_sha, head_sha=head_sha,
+        path=path, original_rev=original_rev, head_rev=head_rev,
     )
 
 
 def test_out_of_date_repo(out_of_date_repo, runner_with_mocked_store):
     config = make_config_from_repo(
-        out_of_date_repo.path, sha=out_of_date_repo.original_sha,
+        out_of_date_repo.path, rev=out_of_date_repo.original_rev,
     )
     ret = _update_repo(config, runner_with_mocked_store, tags_only=False)
-    assert ret['sha'] != out_of_date_repo.original_sha
-    assert ret['sha'] == out_of_date_repo.head_sha
+    assert ret['rev'] != out_of_date_repo.original_rev
+    assert ret['rev'] == out_of_date_repo.head_rev
 
 
 def test_autoupdate_out_of_date_repo(
@@ -108,7 +108,7 @@ def test_autoupdate_out_of_date_repo(
 ):
     # Write out the config
     config = make_config_from_repo(
-        out_of_date_repo.path, sha=out_of_date_repo.original_sha, check=False,
+        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
     )
     write_config('.', config)
 
@@ -119,14 +119,14 @@ def test_autoupdate_out_of_date_repo(
     assert before != after
     # Make sure we don't add defaults
     assert 'exclude' not in after
-    assert out_of_date_repo.head_sha in after
+    assert out_of_date_repo.head_rev in after
 
 
 def test_autoupdate_out_of_date_repo_with_correct_repo_name(
         out_of_date_repo, in_tmpdir, mock_out_store_directory,
 ):
     stale_config = make_config_from_repo(
-        out_of_date_repo.path, sha=out_of_date_repo.original_sha, check=False,
+        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
     )
     local_config = config_with_local_hooks()
     config = {'repos': [stale_config, local_config]}
@@ -140,7 +140,7 @@ def test_autoupdate_out_of_date_repo_with_correct_repo_name(
     after = open(C.CONFIG_FILE).read()
     assert ret == 0
     assert before != after
-    assert out_of_date_repo.head_sha in after
+    assert out_of_date_repo.head_rev in after
     assert local_config['repo'] in after
 
 
@@ -149,7 +149,7 @@ def test_autoupdate_out_of_date_repo_with_wrong_repo_name(
 ):
     # Write out the config
     config = make_config_from_repo(
-        out_of_date_repo.path, sha=out_of_date_repo.original_sha, check=False,
+        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
     )
     write_config('.', config)
 
@@ -168,40 +168,40 @@ def test_does_not_reformat(
     fmt = (
         'repos:\n'
         '-   repo: {}\n'
-        '    sha: {}  # definitely the version I want!\n'
+        '    rev: {}  # definitely the version I want!\n'
         '    hooks:\n'
         '    -   id: foo\n'
         '        # These args are because reasons!\n'
         '        args: [foo, bar, baz]\n'
     )
-    config = fmt.format(out_of_date_repo.path, out_of_date_repo.original_sha)
+    config = fmt.format(out_of_date_repo.path, out_of_date_repo.original_rev)
     with open(C.CONFIG_FILE, 'w') as f:
         f.write(config)
 
     autoupdate(Runner('.', C.CONFIG_FILE), tags_only=False)
     after = open(C.CONFIG_FILE).read()
-    expected = fmt.format(out_of_date_repo.path, out_of_date_repo.head_sha)
+    expected = fmt.format(out_of_date_repo.path, out_of_date_repo.head_rev)
     assert after == expected
 
 
 def test_loses_formatting_when_not_detectable(
         out_of_date_repo, mock_out_store_directory, in_tmpdir,
 ):
-    """A best-effort attempt is made at updating sha without rewriting
+    """A best-effort attempt is made at updating rev without rewriting
     formatting.  When the original formatting cannot be detected, this
     is abandoned.
     """
     config = (
         'repos: [\n'
         '    {{\n'
-        '        repo: {}, sha: {},\n'
+        '        repo: {}, rev: {},\n'
         '        hooks: [\n'
         '            # A comment!\n'
         '            {{id: foo}},\n'
         '        ],\n'
         '    }}\n'
         ']\n'.format(
-            pipes.quote(out_of_date_repo.path), out_of_date_repo.original_sha,
+            pipes.quote(out_of_date_repo.path), out_of_date_repo.original_rev,
         )
     )
     with open(C.CONFIG_FILE, 'w') as f:
@@ -212,10 +212,10 @@ def test_loses_formatting_when_not_detectable(
     expected = (
         'repos:\n'
         '-   repo: {}\n'
-        '    sha: {}\n'
+        '    rev: {}\n'
         '    hooks:\n'
         '    -   id: foo\n'
-    ).format(out_of_date_repo.path, out_of_date_repo.head_sha)
+    ).format(out_of_date_repo.path, out_of_date_repo.head_rev)
     assert after == expected
 
 
@@ -229,7 +229,7 @@ def test_autoupdate_tagged_repo(
         tagged_repo, in_tmpdir, mock_out_store_directory,
 ):
     config = make_config_from_repo(
-        tagged_repo.path, sha=tagged_repo.original_sha,
+        tagged_repo.path, rev=tagged_repo.original_rev,
     )
     write_config('.', config)
 
@@ -250,7 +250,7 @@ def test_autoupdate_tags_only(
 ):
     config = make_config_from_repo(
         tagged_repo_with_more_commits.path,
-        sha=tagged_repo_with_more_commits.original_sha,
+        rev=tagged_repo_with_more_commits.original_rev,
     )
     write_config('.', config)
 
@@ -262,7 +262,7 @@ def test_autoupdate_tags_only(
 @pytest.fixture
 def hook_disappearing_repo(tempdir_factory):
     path = make_repo(tempdir_factory, 'python_hooks_repo')
-    original_sha = git.head_sha(path)
+    original_rev = git.head_rev(path)
 
     shutil.copy(
         get_resource_path('manifest_without_foo.yaml'),
@@ -271,7 +271,7 @@ def hook_disappearing_repo(tempdir_factory):
     cmd_output('git', '-C', path, 'add', '.')
     cmd_output('git', '-C', path, 'commit', '-m', 'Remove foo')
 
-    yield auto_namedtuple(path=path, original_sha=original_sha)
+    yield auto_namedtuple(path=path, original_rev=original_rev)
 
 
 def test_hook_disppearing_repo_raises(
@@ -279,7 +279,7 @@ def test_hook_disppearing_repo_raises(
 ):
     config = make_config_from_repo(
         hook_disappearing_repo.path,
-        sha=hook_disappearing_repo.original_sha,
+        rev=hook_disappearing_repo.original_rev,
         hooks=[OrderedDict((('id', 'foo'),))],
     )
     with pytest.raises(RepositoryCannotBeUpdatedError):
@@ -291,7 +291,7 @@ def test_autoupdate_hook_disappearing_repo(
 ):
     config = make_config_from_repo(
         hook_disappearing_repo.path,
-        sha=hook_disappearing_repo.original_sha,
+        rev=hook_disappearing_repo.original_rev,
         hooks=[OrderedDict((('id', 'foo'),))],
         check=False,
     )
@@ -319,7 +319,7 @@ def test_autoupdate_local_hooks_with_out_of_date_repo(
         out_of_date_repo, in_tmpdir, mock_out_store_directory,
 ):
     stale_config = make_config_from_repo(
-        out_of_date_repo.path, sha=out_of_date_repo.original_sha, check=False,
+        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
     )
     local_config = config_with_local_hooks()
     config = {'repos': [local_config, stale_config]}
diff --git a/tests/commands/migrate_config_test.py b/tests/commands/migrate_config_test.py
index 7b43098..a2a34b6 100644
--- a/tests/commands/migrate_config_test.py
+++ b/tests/commands/migrate_config_test.py
@@ -118,3 +118,30 @@ def test_already_migrated_configuration_noop(tmpdir, capsys):
     out, _ = capsys.readouterr()
     assert out == 'Configuration is already migrated.\n'
     assert cfg.read() == contents
+
+
+def test_migrate_config_sha_to_rev(tmpdir):
+    contents = (
+        'repos:\n'
+        '-   repo: https://github.com/pre-commit/pre-commit-hooks\n'
+        '    sha: v1.2.0\n'
+        '    hooks: []\n'
+        'repos:\n'
+        '-   repo: https://github.com/pre-commit/pre-commit-hooks\n'
+        '    sha: v1.2.0\n'
+        '    hooks: []\n'
+    )
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(contents)
+    assert not migrate_config(Runner(tmpdir.strpath, C.CONFIG_FILE))
+    contents = cfg.read()
+    assert contents == (
+        'repos:\n'
+        '-   repo: https://github.com/pre-commit/pre-commit-hooks\n'
+        '    rev: v1.2.0\n'
+        '    hooks: []\n'
+        'repos:\n'
+        '-   repo: https://github.com/pre-commit/pre-commit-hooks\n'
+        '    rev: v1.2.0\n'
+        '    hooks: []\n'
+    )
diff --git a/tests/commands/sample_config_test.py b/tests/commands/sample_config_test.py
index 1dca98b..7c4e88d 100644
--- a/tests/commands/sample_config_test.py
+++ b/tests/commands/sample_config_test.py
@@ -13,7 +13,7 @@ def test_sample_config(capsys):
 # See https://pre-commit.com/hooks.html for more hooks
 repos:
 -   repo: https://github.com/pre-commit/pre-commit-hooks
-    sha: v0.9.2
+    rev: v1.2.1-1
     hooks:
     -   id: trailing-whitespace
     -   id: end-of-file-fixer
diff --git a/tests/commands/try_repo_test.py b/tests/commands/try_repo_test.py
index a29181b..4fb0755 100644
--- a/tests/commands/try_repo_test.py
+++ b/tests/commands/try_repo_test.py
@@ -39,7 +39,7 @@ def test_try_repo_repo_only(cap_out, tempdir_factory):
     assert re.match(
         '^repos:\n'
         '-   repo: .+\n'
-        '    sha: .+\n'
+        '    rev: .+\n'
         '    hooks:\n'
         '    -   id: bash_hook\n'
         '    -   id: bash_hook2\n'
@@ -63,7 +63,7 @@ def test_try_repo_with_specific_hook(cap_out, tempdir_factory):
     assert re.match(
         '^repos:\n'
         '-   repo: .+\n'
-        '    sha: .+\n'
+        '    rev: .+\n'
         '    hooks:\n'
         '    -   id: bash_hook\n$',
         config,
diff --git a/tests/make_archives_test.py b/tests/make_archives_test.py
index 414f853..65715ac 100644
--- a/tests/make_archives_test.py
+++ b/tests/make_archives_test.py
@@ -19,8 +19,8 @@ def test_make_archive(tempdir_factory):
     open(os.path.join(git_path, 'foo'), 'a').close()
     cmd_output('git', '-C', git_path, 'add', '.')
     cmd_output('git', '-C', git_path, 'commit', '-m', 'foo')
-    # We'll use this sha
-    head_sha = git.head_sha(git_path)
+    # We'll use this rev
+    head_rev = git.head_rev(git_path)
     # And check that this file doesn't exist
     open(os.path.join(git_path, 'bar'), 'a').close()
     cmd_output('git', '-C', git_path, 'add', '.')
@@ -28,7 +28,7 @@ def test_make_archive(tempdir_factory):
 
     # Do the thing
     archive_path = make_archives.make_archive(
-        'foo', git_path, head_sha, output_dir,
+        'foo', git_path, head_rev, output_dir,
     )
 
     assert archive_path == os.path.join(output_dir, 'foo.tar.gz')
diff --git a/tests/repository_test.py b/tests/repository_test.py
index 7f0593b..63b9f1c 100644
--- a/tests/repository_test.py
+++ b/tests/repository_test.py
@@ -660,14 +660,14 @@ def test_tags_on_repositories(in_tmpdir, tempdir_factory, store):
     )
 
     repo_1 = Repository.create(
-        make_config_from_repo(git_dir_1, sha=tag), store,
+        make_config_from_repo(git_dir_1, rev=tag), store,
     )
     ret = repo_1.run_hook(repo_1.hooks[0][1], ['-L'])
     assert ret[0] == 0
     assert ret[1].strip() == _norm_pwd(in_tmpdir)
 
     repo_2 = Repository.create(
-        make_config_from_repo(git_dir_2, sha=tag), store,
+        make_config_from_repo(git_dir_2, rev=tag), store,
     )
     ret = repo_2.run_hook(repo_2.hooks[0][1], ['bar'])
     assert ret[0] == 0
diff --git a/tests/staged_files_only_test.py b/tests/staged_files_only_test.py
index 481a288..932ee4b 100644
--- a/tests/staged_files_only_test.py
+++ b/tests/staged_files_only_test.py
@@ -192,15 +192,14 @@ def submodule_with_commits(tempdir_factory):
     path = git_dir(tempdir_factory)
     with cwd(path):
         cmd_output('git', 'commit', '--allow-empty', '-m', 'foo')
-        sha1 = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
+        rev1 = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
         cmd_output('git', 'commit', '--allow-empty', '-m', 'bar')
-        sha2 = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
-        yield auto_namedtuple(path=path, sha1=sha1, sha2=sha2)
+        rev2 = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
+        yield auto_namedtuple(path=path, rev1=rev1, rev2=rev2)
 
 
-def checkout_submodule(sha):
-    with cwd('sub'):
-        cmd_output('git', 'checkout', sha)
+def checkout_submodule(rev):
+    cmd_output('git', '-C', 'sub', 'checkout', rev)
 
 
 @pytest.fixture
@@ -210,7 +209,7 @@ def sub_staged(submodule_with_commits, tempdir_factory):
         cmd_output(
             'git', 'submodule', 'add', submodule_with_commits.path, 'sub',
         )
-        checkout_submodule(submodule_with_commits.sha1)
+        checkout_submodule(submodule_with_commits.rev1)
         cmd_output('git', 'add', 'sub')
         yield auto_namedtuple(
             path=path,
@@ -219,11 +218,11 @@ def sub_staged(submodule_with_commits, tempdir_factory):
         )
 
 
-def _test_sub_state(path, sha='sha1', status='A'):
+def _test_sub_state(path, rev='rev1', status='A'):
     assert os.path.exists(path.sub_path)
     with cwd(path.sub_path):
-        actual_sha = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
-    assert actual_sha == getattr(path.submodule, sha)
+        actual_rev = cmd_output('git', 'rev-parse', 'HEAD')[1].strip()
+    assert actual_rev == getattr(path.submodule, rev)
     actual_status = get_short_git_status()['sub']
     assert actual_status == status
 
@@ -239,15 +238,15 @@ def test_sub_nothing_unstaged(sub_staged, patch_dir):
 
 
 def test_sub_something_unstaged(sub_staged, patch_dir):
-    checkout_submodule(sub_staged.submodule.sha2)
+    checkout_submodule(sub_staged.submodule.rev2)
 
-    _test_sub_state(sub_staged, 'sha2', 'AM')
+    _test_sub_state(sub_staged, 'rev2', 'AM')
 
     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, 'rev2', 'AM')
 
-    _test_sub_state(sub_staged, 'sha2', 'AM')
+    _test_sub_state(sub_staged, 'rev2', 'AM')
 
 
 def test_stage_utf8_changes(foo_staged, patch_dir):
diff --git a/tests/store_test.py b/tests/store_test.py
index 86c3ec4..4e80f05 100644
--- a/tests/store_test.py
+++ b/tests/store_test.py
@@ -91,10 +91,10 @@ def test_clone(store, tempdir_factory, log_info_mock):
     path = git_dir(tempdir_factory)
     with cwd(path):
         cmd_output('git', 'commit', '--allow-empty', '-m', 'foo')
-        sha = git.head_sha(path)
+        rev = git.head_rev(path)
         cmd_output('git', 'commit', '--allow-empty', '-m', 'bar')
 
-    ret = store.clone(path, sha)
+    ret = store.clone(path, rev)
     # Should have printed some stuff
     assert log_info_mock.call_args_list[0][0][0].startswith(
         'Initializing environment for ',
@@ -106,14 +106,14 @@ def test_clone(store, tempdir_factory, log_info_mock):
     # Directory should start with `repo`
     _, dirname = os.path.split(ret)
     assert dirname.startswith('repo')
-    # Should be checked out to the sha we specified
-    assert git.head_sha(ret) == sha
+    # Should be checked out to the rev we specified
+    assert git.head_rev(ret) == rev
 
     # Assert there's an entry in the sqlite db for this
     with sqlite3.connect(store.db_path) as db:
         path, = db.execute(
             'SELECT path from repos WHERE repo = ? and ref = ?',
-            [path, sha],
+            (path, rev),
         ).fetchone()
         assert path == ret
 
@@ -122,7 +122,7 @@ def test_clone_cleans_up_on_checkout_failure(store):
     try:
         # This raises an exception because you can't clone something that
         # doesn't exist!
-        store.clone('/i_dont_exist_lol', 'fake_sha')
+        store.clone('/i_dont_exist_lol', 'fake_rev')
     except Exception as e:
         assert '/i_dont_exist_lol' in six.text_type(e)