Restyle WIP: Use pre-commit for Git hooks

dvc/command/install.py

@@ -11,7 +11,7 @@
 class CmdInstall(CmdBase):
     def run(self):
         try:
-            self.repo.install()
+            self.repo.install(self.args.use_pre_commit_tool)
         except Exception:
             logger.exception("failed to install DVC Git hooks")
             return 1
@@ -27,4 +27,11 @@ def add_parser(subparsers, parent_parser):
         help=INSTALL_HELP,
         formatter_class=argparse.RawDescriptionHelpFormatter,
     )
+    install_parser.add_argument(
+        "--use-pre-commit-tool",
+        action="store_true",
+        default=False,
+        help="Install DVC hooks using pre-commit "
+        "(https://pre-commit.com) if it is installed.",
+    )
     install_parser.set_defaults(func=CmdInstall)

dvc/repo/install.py

@@ -1,2 +1,2 @@
-def install(self):
-    self.scm.install()
+def install(self, use_pre_commit_tool):
+    self.scm.install(use_pre_commit_tool)

dvc/scm/base.py

@@ -114,8 +114,13 @@ def list_all_commits(self):  # pylint: disable=no-self-use
         """Returns a list of commits in the repo."""
         return []
 
-    def install(self):
-        """Adds dvc commands to SCM hooks for the repo."""
+    def install(self, use_pre_commit_tool):
+        """
+        Adds dvc commands to SCM hooks for the repo.
+
+        If use_pre_commit_tool is set and pre-commit is
+        installed it will be used to install the hooks.
+        """
 
     def cleanup_ignores(self):
         """

dvc/scm/git/__init__.py

@@ -2,13 +2,20 @@
 
 import logging
 import os
+import yaml
+from shutil import which
+from subprocess import check_call
+from pathlib import Path
 
 from funcy import cached_property
 from pathspec.patterns import GitWildMatchPattern
 
+from dvc.exceptions import DvcException
 from dvc.exceptions import GitHookAlreadyExistsError
 from dvc.scm.base import Base
 from dvc.scm.base import CloneError, FileNotInRepoError, RevError, SCMError
+from dvc.scm.git.pre_commit_tool import pre_commit_tool_conf
+from dvc.scm.git.pre_commit_tool import merge_pre_commit_tool_confs
 from dvc.scm.git.tree import GitTree
 from dvc.utils import fix_env, is_binary, relpath
 from dvc.utils.fs import path_isin
@@ -253,15 +260,15 @@ def list_tags(self):
     def list_all_commits(self):
         return [c.hexsha for c in self.repo.iter_commits("--all")]
 
-    def _install_hook(self, name, preconditions, cmd):
+    def _install_hook(self, name, preconditions, cmd, hook_path_fn):
         # only run in dvc repo
         in_dvc_repo = '[ -n "$(git ls-files --full-name .dvc)" ]'
 
         command = "if {}; then exec dvc {}; fi".format(
             " && ".join([in_dvc_repo] + preconditions), cmd
         )
 
-        hook = self._hook_path(name)
+        hook = hook_path_fn(name)
 
         if os.path.isfile(hook):
             with open(hook, "r+") as fobj:
@@ -273,9 +280,16 @@ def _install_hook(self, name, preconditions, cmd):
 
         os.chmod(hook, 0o777)
 
-    def install(self):
+    def install(self, use_pre_commit_tool):
         self._verify_dvc_hooks()
 
+        hook_path_fn = self._hook_path
+
+        if use_pre_commit_tool:
+            hook_path_fn = self._pre_commit_tool_hook_path
+            path = Path(self._pre_commit_tool_hooks_home)
+            path.mkdir(parents=True, exist_ok=True)
+
         self._install_hook(
             "post-checkout",
             [
@@ -287,9 +301,32 @@ def install(self):
                 "[ ! -d .git/rebase-merge ]",
             ],
             "checkout",
+            hook_path_fn,
         )
-        self._install_hook("pre-commit", [], "status")
-        self._install_hook("pre-push", [], "push")
+        self._install_hook("pre-commit", [], "status", hook_path_fn)
+        self._install_hook("pre-push", [], "push", hook_path_fn)
+
+        if use_pre_commit_tool:
+            self._integrate_pre_commit_tool()
+
+    def _integrate_pre_commit_tool(self):
+        if not which("pre-commit"):
+            raise DvcException("pre-commit is not installed")
+
+        conf = pre_commit_tool_conf(
+            self._pre_commit_tool_hook_path("pre-commit"),
+            self._pre_commit_tool_hook_path("push"),
+            self._pre_commit_tool_hook_path("post-checkout"),
+        )
+
+        conf_yaml = os.path.join(self.root_dir, ".pre-commit-config.yaml")
+        if not os.path.isfile(conf_yaml):
+            check_call("pre-commit install", shell=True)
+
+        with open(conf_yaml, "w+") as conf_yaml_f:
+            existing_conf = yaml.safe_load(conf_yaml_f)
+            conf = merge_pre_commit_tool_confs(existing_conf, conf)
+            yaml.dump(conf, conf_yaml_f)
 
     def cleanup_ignores(self):
         for path in self.ignored_paths:
@@ -372,9 +409,17 @@ def close(self):
     def _hooks_home(self):
         return os.path.join(self.root_dir, self.GIT_DIR, "hooks")
 
+    @cached_property
+    def _pre_commit_tool_hooks_home(self):
+        # TODO(andrewhare): Is there a const somewhere for ".dvc/tmp"?
+        return os.path.join(".dvc", "tmp", "hooks")
+
     def _hook_path(self, name):
         return os.path.join(self._hooks_home, name)
 
+    def _pre_commit_tool_hook_path(self, name):
+        return os.path.join(self._pre_commit_tool_hooks_home, name)
+
     def _verify_hook(self, name):
         if os.path.exists(self._hook_path(name)):
             raise GitHookAlreadyExistsError(name)
@@ -383,3 +428,9 @@ def _verify_dvc_hooks(self):
         self._verify_hook("post-checkout")
         self._verify_hook("pre-commit")
         self._verify_hook("pre-push")
+
+    def _verify_pre_commit_tool(self):
+        if not which("pre-commit"):
+            raise DvcException("pre-commit is not installed")
+
+        check_call("pre-commit install", shell=True)

dvc/scm/git/pre_commit_tool.py

@@ -0,0 +1,39 @@
+def pre_commit_tool_conf(pre_commit_path, push_path, post_checkout_path):
+    return {
+        "repos": [
+            {
+                "repo": "local",
+                "hooks": [
+                    {
+                        "id": "dvc-pre-commit",
+                        "name": "DVC Pre Commit",
+                        "entry": pre_commit_path,
+                        "language": "script",
+                        "stages": ["commit"],
+                    },
+                    {
+                        "id": "dvc-pre-push",
+                        "name": "DVC Pre Push",
+                        "entry": push_path,
+                        "language": "script",
+                        "stages": ["push"],
+                    },
+                    {
+                        "id": "dvc-post-checkout",
+                        "name": "DVC Post Checkout",
+                        "entry": post_checkout_path,
+                        "language": "script",
+                        "stages": ["checkout"],
+                    },
+                ],
+            }
+        ]
+    }
+
+
+def merge_pre_commit_tool_confs(existing_conf, conf):
+    if not existing_conf or not "repos" in existing_conf:
+        return conf
+
+    existing_conf["repos"].append(conf["repos"][0])
+    return existing_conf

tests/unit/scm/test_pre_commit_tool.py

@@ -0,0 +1,31 @@
+from dvc.scm.git.pre_commit_tool import pre_commit_tool_conf
+from dvc.scm.git.pre_commit_tool import merge_pre_commit_tool_confs
+
+from unittest import TestCase
+
+
+class TestPreCommitTool(TestCase):
+    def setUp(self):
+        self.conf = pre_commit_tool_conf("a", "b", "c")
+
+    def test_merge_pre_commit_tool_confs_empty(self):
+        existing_conf = None
+        merged_conf = merge_pre_commit_tool_confs(existing_conf, self.conf)
+        self.assertEqual(self.conf, merged_conf)
+
+    def test_merge_pre_commit_tool_confs_invalid_yaml(self):
+        existing_conf = "some invalid yaml"
+        merged_conf = merge_pre_commit_tool_confs(existing_conf, self.conf)
+        self.assertEqual(self.conf, merged_conf)
+
+    def test_merge_pre_commit_tool_confs_no_repos(self):
+        existing_conf = {"foo": [1, 2, 3]}
+        merged_conf = merge_pre_commit_tool_confs(existing_conf, self.conf)
+        self.assertEqual(self.conf, merged_conf)
+
+    def test_merge_pre_commit_tool_confs(self):
+        existing_conf = {"repos": [{}]}
+        merged_conf = merge_pre_commit_tool_confs(existing_conf, self.conf)
+        # Merging the new conf in should append the new repo to the end of
+        # the existing repos array on the existing conf.
+        self.assertEqual(self.conf["repos"][0], merged_conf["repos"][1])