From c39c2f087aaa04b36769cc5b29ecfca7f4b2da97 Mon Sep 17 00:00:00 2001
From: "Mr. Outis" <mroutis@protonmail.com>
Date: Fri, 10 May 2019 16:19:41 -0700
Subject: [PATCH] install: append commands if hook already exists

Close #1964
---
 dvc/scm/git/__init__.py    | 16 +++++++++----
 tests/func/test_install.py | 47 +++++++++++++++++++++++++++++---------
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py
index 85b66db81..04e14f336 100644
--- a/dvc/scm/git/__init__.py
+++ b/dvc/scm/git/__init__.py
@@ -207,12 +207,20 @@ class Git(Base):
         return [t.name for t in self.git.tags]
 
     def _install_hook(self, name, cmd):
+        command = "dvc {}".format(cmd)
+
         hook = os.path.join(self.root_dir, self.GIT_DIR, "hooks", name)
+
         if os.path.isfile(hook):
-            msg = "git hook '{}' already exists."
-            raise SCMError(msg.format(os.path.relpath(hook)))
-        with open(hook, "w+") as fobj:
-            fobj.write("#!/bin/sh\nexec dvc {}\n".format(cmd))
+            with open(hook, "r+") as fobj:
+                if command not in fobj.read():
+                    fobj.write("exec {command}\n".format(command=command))
+        else:
+            with open(hook, "w+") as fobj:
+                fobj.write(
+                    "#!/bin/sh\n" "exec {command}\n".format(command=command)
+                )
+
         os.chmod(hook, 0o777)
 
     def install(self):
diff --git a/tests/func/test_install.py b/tests/func/test_install.py
index 47ff748a3..58146b12c 100644
--- a/tests/func/test_install.py
+++ b/tests/func/test_install.py
@@ -15,21 +15,44 @@ class TestInstall(object):
     def _hook(self, name):
         return os.path.join(".git", "hooks", name)
 
-    @pytest.fixture(autouse=True)
-    def setUp(self, dvc):
-        ret = main(["install"])
-        assert ret == 0
+    def test_should_create_hooks(self, dvc):
+        assert main(["install"]) == 0
 
-    def test_should_not_install_twice(self, dvc):
-        ret = main(["install"])
-        assert ret != 0
+        hooks_with_commands = [
+            ("post-checkout", "exec dvc checkout"),
+            ("pre-commit", "exec dvc status"),
+            ("pre-push", "exec dvc push"),
+        ]
 
-    def test_should_create_hooks(self):
-        assert os.path.isfile(self._hook("post-checkout"))
-        assert os.path.isfile(self._hook("pre-commit"))
-        assert os.path.isfile(self._hook("pre-push"))
+        for fname, command in hooks_with_commands:
+            assert os.path.isfile(self._hook(fname))
+
+            with open(self._hook(fname), "r") as fobj:
+                assert command in fobj.read()
+
+    def test_should_append_hooks_if_file_already_exists(self, dvc):
+        with open(self._hook("post-checkout"), "w") as fobj:
+            fobj.write("#!/bin/sh\n" "echo hello\n")
+
+        assert main(["install"]) == 0
+
+        expected_script = "#!/bin/sh\n" "echo hello\n" "exec dvc checkout\n"
+
+        with open(self._hook("post-checkout"), "r") as fobj:
+            assert fobj.read() == expected_script
+
+    def test_should_be_idempotent(self, dvc):
+        assert main(["install"]) == 0
+        assert main(["install"]) == 0
+
+        expected_script = "#!/bin/sh\n" "exec dvc checkout\n"
+
+        with open(self._hook("post-checkout"), "r") as fobj:
+            assert fobj.read() == expected_script
 
     def test_should_post_checkout_hook_checkout(self, repo_dir, dvc):
+        assert main(["install"]) == 0
+
         stage_file = repo_dir.FOO + Stage.STAGE_FILE_SUFFIX
 
         dvc.add(repo_dir.FOO)
@@ -42,6 +65,8 @@ class TestInstall(object):
         assert os.path.isfile(repo_dir.FOO)
 
     def test_should_pre_push_hook_push(self, repo_dir, dvc):
+        assert main(["install"]) == 0
+
         temp = repo_dir.mkdtemp()
         git_remote = os.path.join(temp, "project.git")
         storage_path = os.path.join(temp, "dvc_storage")