Use pre-commit for Git hooks (iterative#3406)

* Initial design of using pre-commit for Git hooks

* Refactor to reuse existing code and provide merge capabilities

* Linter fixes

* More cleanup

* scm: make use_precommit_tool optional

* dvc: install our own hooks

* install: fix typo

* install: fix bugs in pre-commit integration

* install: don't forget 'always_run' for post-checkout

* install: no need to run pre-commit install if conf file doesn't exist

* install: fix bugs

* install: use python hooks

* add .pre-commit-hooks.yaml

Co-authored-by: Andrew Hare <[email protected]>
Co-authored-by: Ruslan Kuprieiev <[email protected]>

.pre-commit-config.yaml

@@ -1,17 +1,35 @@
 repos:
--   repo: https://github.com/ambv/black
-    rev: 19.10b0
-    hooks:
-    - id: black
-      language_version: python3
--   repo: https://gitlab.com/pycqa/flake8
-    rev: master
-    hooks:
-    - id: flake8
-      language_version: python3
--   repo: https://github.com/lovesegfault/beautysh
-    rev: master
-    hooks:
-    - id: beautysh
-      language_version: python3
-      args: [-i, '2']  # 2-space indentaion
+- hooks:
+  - id: black
+    language_version: python3
+  repo: https://github.com/ambv/black
+  rev: 19.10b0
+- hooks:
+  - id: flake8
+    language_version: python3
+  repo: https://gitlab.com/pycqa/flake8
+  rev: master
+- hooks:
+  - args:
+    - -i
+    - '2'
+    id: beautysh
+    language_version: python3
+  repo: https://github.com/lovesegfault/beautysh
+  rev: master
+- hooks:
+  - id: dvc-pre-commit
+    language_version: python3
+    stages:
+    - commit
+  - id: dvc-pre-push
+    language_version: python3
+    stages:
+    - push
+  - always_run: true
+    id: dvc-post-checkout
+    language_version: python3
+    stages:
+    - post-checkout
+  repo: https://github.com/andrewhare/dvc
+  rev: WIP-pre-commit-tool

.pre-commit-hooks.yaml

@@ -0,0 +1,32 @@
+- args:
+  - git-hook
+  - pre-commit
+  entry: dvc
+  id: dvc-pre-commit
+  language: python
+  language_version: python3
+  name: DVC pre-commit
+  stages:
+  - commit
+- args:
+  - git-hook
+  - pre-push
+  entry: dvc
+  id: dvc-pre-push
+  language: python
+  language_version: python3
+  name: DVC pre-push
+  stages:
+  - push
+- always_run: true
+  args:
+  - git-hook
+  - post-checkout
+  entry: dvc
+  id: dvc-post-checkout
+  language: python
+  language_version: python3
+  minimum_pre_commit_version: 2.2.0
+  name: DVC post-checkout
+  stages:
+  - post-checkout

dvc/cli.py

@@ -34,6 +34,7 @@
     unprotect,
     update,
     version,
+    git_hook,
 )
 from .command.base import fix_subparsers
 from .exceptions import DvcParserError
@@ -72,6 +73,7 @@
     diff,
     version,
     update,
+    git_hook,
 ]
 
 

dvc/command/git_hook.py

@@ -0,0 +1,112 @@
+import logging
+import os
+
+from dvc.command.base import CmdBaseNoRepo, fix_subparsers
+from dvc.exceptions import NotDvcRepoError
+
+logger = logging.getLogger(__name__)
+
+
+class CmdHookBase(CmdBaseNoRepo):
+    def run(self):
+        from dvc.repo import Repo
+
+        try:
+            repo = Repo()
+            repo.close()
+        except NotDvcRepoError:
+            return 0
+
+        return self._run()
+
+
+class CmdPreCommit(CmdHookBase):
+    def _run(self):
+        from dvc.main import main
+
+        return main(["status"])
+
+
+class CmdPostCheckout(CmdHookBase):
+    def run(self):
+        # when we are running from pre-commit tool, it doesn't provide CLI
+        # flags, but instead provides respective env vars that we could use.
+        flag = os.environ.get("PRE_COMMIT_CHECKOUT_TYPE")
+        if flag is None and len(self.args.args) >= 3:
+            # see https://git-scm.com/docs/githooks#_post_checkout
+            flag = self.args.args[2]
+
+        # checking out some reference and not specific file.
+        if flag != "1":
+            return 0
+
+        # make sure we are not in the middle of a rebase/merge, so we
+        # don't accidentally break it with an unsuccessful checkout.
+        # Note that git hooks are always running in repo root.
+        if os.path.isdir(os.path.join(".git", "rebase-merge")):
+            return 0
+
+        from dvc.main import main
+
+        return main(["checkout"])
+
+
+class CmdPrePush(CmdHookBase):
+    def run(self):
+        from dvc.main import main
+
+        return main(["push"])
+
+
+def add_parser(subparsers, parent_parser):
+    GIT_HOOK_HELP = "Run GIT hook."
+
+    git_hook_parser = subparsers.add_parser(
+        "git-hook",
+        parents=[parent_parser],
+        description=GIT_HOOK_HELP,
+        add_help=False,
+    )
+
+    git_hook_subparsers = git_hook_parser.add_subparsers(
+        dest="cmd",
+        help="Use `dvc daemon CMD --help` for command-specific help.",
+    )
+
+    fix_subparsers(git_hook_subparsers)
+
+    PRE_COMMIT_HELP = "Run pre-commit GIT hook."
+    pre_commit_parser = git_hook_subparsers.add_parser(
+        "pre-commit",
+        parents=[parent_parser],
+        description=PRE_COMMIT_HELP,
+        help=PRE_COMMIT_HELP,
+    )
+    pre_commit_parser.add_argument(
+        "args", nargs="*", help="Arguments passed by GIT or pre-commit tool.",
+    )
+    pre_commit_parser.set_defaults(func=CmdPreCommit)
+
+    POST_CHECKOUT_HELP = "Run post-checkout GIT hook."
+    post_checkout_parser = git_hook_subparsers.add_parser(
+        "post-checkout",
+        parents=[parent_parser],
+        description=POST_CHECKOUT_HELP,
+        help=POST_CHECKOUT_HELP,
+    )
+    post_checkout_parser.add_argument(
+        "args", nargs="*", help="Arguments passed by GIT or pre-commit tool.",
+    )
+    post_checkout_parser.set_defaults(func=CmdPostCheckout)
+
+    PRE_PUSH_HELP = "Run pre-push GIT hook."
+    pre_push_parser = git_hook_subparsers.add_parser(
+        "pre-push",
+        parents=[parent_parser],
+        description=PRE_PUSH_HELP,
+        help=PRE_PUSH_HELP,
+    )
+    pre_push_parser.add_argument(
+        "args", nargs="*", help="Arguments passed by GIT or pre-commit tool.",
+    )
+    pre_push_parser.set_defaults(func=CmdPrePush)

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=False):
+        """
+        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,6 +2,7 @@
 
 import logging
 import os
+import yaml
 
 from funcy import cached_property
 from pathspec.patterns import GitWildMatchPattern
@@ -253,43 +254,57 @@ 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):
-        # 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
-        )
-
+    def _install_hook(self, name):
         hook = self._hook_path(name)
-
-        if os.path.isfile(hook):
-            with open(hook, "r+") as fobj:
-                if command not in fobj.read():
-                    fobj.write("{command}\n".format(command=command))
-        else:
-            with open(hook, "w+") as fobj:
-                fobj.write("#!/bin/sh\n" "{command}\n".format(command=command))
+        with open(hook, "w+") as fobj:
+            fobj.write("#!/bin/sh\nexec dvc git-hook {} $@\n".format(name))
 
         os.chmod(hook, 0o777)
 
-    def install(self):
-        self._verify_dvc_hooks()
-
-        self._install_hook(
-            "post-checkout",
-            [
-                # checking out some reference and not specific file.
-                '[ "$3" = "1" ]',
-                # make sure we are not in the middle of a rebase/merge, so we
-                # don't accidentally break it with an unsuccessful checkout.
-                # Note that git hooks are always running in repo root.
-                "[ ! -d .git/rebase-merge ]",
+    def install(self, use_pre_commit_tool=False):
+        if not use_pre_commit_tool:
+            self._verify_dvc_hooks()
+            self._install_hook("post-checkout")
+            self._install_hook("pre-commit")
+            self._install_hook("pre-push")
+            return
+
+        config_path = os.path.join(self.root_dir, ".pre-commit-config.yaml")
+
+        config = {}
+        if os.path.exists(config_path):
+            with open(config_path, "r") as fobj:
+                config = yaml.safe_load(fobj)
+
+        entry = {
+            "repo": "https://github.com/andrewhare/dvc",  # FIXME
+            "rev": "WIP-pre-commit-tool",
+            "hooks": [
+                {
+                    "id": "dvc-pre-commit",
+                    "language_version": "python3",
+                    "stages": ["commit"],
+                },
+                {
+                    "id": "dvc-pre-push",
+                    "language_version": "python3",
+                    "stages": ["push"],
+                },
+                {
+                    "id": "dvc-post-checkout",
+                    "language_version": "python3",
+                    "stages": ["post-checkout"],
+                    "always_run": True,
+                },
             ],
-            "checkout",
-        )
-        self._install_hook("pre-commit", [], "status")
-        self._install_hook("pre-push", [], "push")
+        }
+
+        if entry in config["repos"]:
+            return
+
+        config["repos"].append(entry)
+        with open(config_path, "w+") as fobj:
+            yaml.dump(config, fobj)
 
     def cleanup_ignores(self):
         for path in self.ignored_paths:

setup.py

@@ -8,7 +8,10 @@
 # Prevents pkg_resources import in entry point script,
 # see https://github.com/ninjaaron/fast-entry_points.
 # This saves about 200 ms on startup time for non-wheel installs.
-import fastentrypoints  # noqa: F401
+try:
+    import fastentrypoints  # noqa: F401
+except ImportError:
+    pass  # not able to import when installing through pre-commit
 
 
 # Read package meta-data from version.py

tests/func/test_install.py

@@ -19,9 +19,9 @@ def test_create_hooks(self, scm, dvc):
         scm.install()
 
         hooks_with_commands = [
-            ("post-checkout", "exec dvc checkout"),
-            ("pre-commit", "exec dvc status"),
-            ("pre-push", "exec dvc push"),
+            ("post-checkout", "exec dvc git-hook post-checkout"),
+            ("pre-commit", "exec dvc git-hook pre-commit"),
+            ("pre-push", "exec dvc git-hook pre-push"),
         ]
 
         for fname, command in hooks_with_commands: