Repository: iterative/dvc

Issue:
post-checkout hook breaks checkout and rebase when adopting DVC in a branch
When adopting DVC in an existing repository with PR code-review workflow, it will be landing in a branch that isn't master. This branch then has problematic interactions where work continues on master before the dvc adoption lands there.

Specifically, once you've done `dvc init`, committed that, and then `dvc install`, the post-checkout hook breaks `git checkout` and `git rebase` between the master branch and the adopting-dvc branch.

To reproduce:

```bash
mkdir dvc-test
cd dvc-test
git init
echo "Testing" > README.md
git add README.md
git commit -m "Initial commit"
git checkout -b start-using-dvc
dvc init
git commit -m "dvc init"
dvc install
```

I go on experimenting with dvc, but then I have to pause the dvc adoption to do some work on the master branch.:

```bash
git stash save "experimenting with dvc"
git checkout master
```

This last command succeeds but outputs:

```
Switched to branch 'master'
Adding '.dvc/state' to '.dvc/.gitignore'.
Adding '.dvc/lock' to '.dvc/.gitignore'.
Adding '.dvc/config.local' to '.dvc/.gitignore'.
Adding '.dvc/updater' to '.dvc/.gitignore'.
Adding '.dvc/updater.lock' to '.dvc/.gitignore'.
Adding '.dvc/repos' to '.dvc/.gitignore'.
Adding '.dvc/state-journal' to '.dvc/.gitignore'.
Adding '.dvc/state-wal' to '.dvc/.gitignore'.
Adding '.dvc/cache' to '.dvc/.gitignore'.
```

... uh-oh. I'm on master where dvc hasn't been taken into use, so I (or my tooling) shouldn't be putting anything in `.dvc`.

I continue with the master work and then return to DVC:

```bash
echo "Progress" > README.md
git commit README.md -m "Progress"
git checkout start-using-dvc
```

That last command fails with:

```
error: The following untracked working tree files would be overwritten by checkout:
	.dvc/.gitignore
Please move or remove them before you switch branches.
Aborting
```

... which is not normal in git; with a clean working tree, `git checkout` to switch branches should always work.

I workaround this and want to keep going, rebasing my dvc experiment branch on current tip of master:

```bash
rm .dvc/.gitignore
git checkout start-using-dvc
git rebase master
```

That rebase fails with:

```
First, rewinding head to replay your work on top of it...
Adding '.dvc/state' to '.dvc/.gitignore'.
Adding '.dvc/lock' to '.dvc/.gitignore'.
Adding '.dvc/config.local' to '.dvc/.gitignore'.
Adding '.dvc/updater' to '.dvc/.gitignore'.
Adding '.dvc/updater.lock' to '.dvc/.gitignore'.
Adding '.dvc/repos' to '.dvc/.gitignore'.
Adding '.dvc/state-journal' to '.dvc/.gitignore'.
Adding '.dvc/state-wal' to '.dvc/.gitignore'.
Adding '.dvc/cache' to '.dvc/.gitignore'.
Applying: dvc init
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
error: The following untracked working tree files would be overwritten by merge:
	.dvc/.gitignore
Please move or remove them before you merge.
Aborting
error: Failed to merge in the changes.
Patch failed at 0001 dvc init
hint: Use 'git am --show-current-patch' to see the failed patch
Resolve all conflicts manually, mark them as resolved with
"git add/rm <conflicted_files>", then run "git rebase --continue".
You can instead skip this commit: run "git rebase --skip".
To abort and get back to the state before "git rebase", run "git rebase --abort".
```

Deleting the post-checkout hook makes the rebase work just fine:

```bash
rm .git/hooks/post-checkout
git rebase master
```

So probably the post-checkout hook should abort with an informative message if you're on a branch where dvc _hasn't_ been adopted. Not sure whether that check is as simple as checking existence of `.dvc` or whether that breaks other legitimate uses of the hook.

My `dvc --version` is 0.50.1, installed with homebrew (`brew cask`).

Discussion:

Patch:
diff --git a/dvc/command/version.py b/dvc/command/version.py
index d964eaa8b..770d9ad3c 100644
--- a/dvc/command/version.py
+++ b/dvc/command/version.py
@@ -42,22 +42,22 @@ class CmdVersion(CmdBaseNoRepo):
             binary=binary,
         )
 
-        if psutil:
-            try:
-                repo = Repo()
-                root_directory = repo.root_dir
+        try:
+            repo = Repo()
+            root_directory = repo.root_dir
+
+            info += "Cache: {cache}\n".format(
+                cache=self.get_linktype_support_info(repo)
+            )
 
+            if psutil:
                 info += (
-                    "Cache: {cache}\n"
                     "Filesystem type (cache directory): {fs_cache}\n"
-                ).format(
-                    cache=self.get_linktype_support_info(repo),
-                    fs_cache=self.get_fs_type(repo.cache.local.cache_dir),
-                )
-
-            except NotDvcRepoError:
-                root_directory = os.getcwd()
+                ).format(fs_cache=self.get_fs_type(repo.cache.local.cache_dir))
+        except NotDvcRepoError:
+            root_directory = os.getcwd()
 
+        if psutil:
             info += ("Filesystem type (workspace): {fs_root}").format(
                 fs_root=self.get_fs_type(os.path.abspath(root_directory))
             )
diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py
index c0592e896..883c75afd 100644
--- a/dvc/scm/git/__init__.py
+++ b/dvc/scm/git/__init__.py
@@ -216,19 +216,17 @@ class Git(Base):
         return [t.name for t in self.git.tags]
 
     def _install_hook(self, name, cmd):
-        command = "dvc {}".format(cmd)
+        command = '[ -z "$(git ls-files .dvc)" ] || exec dvc {}'.format(cmd)
 
         hook = os.path.join(self.root_dir, self.GIT_DIR, "hooks", name)
 
         if os.path.isfile(hook):
             with open(hook, "r+") as fobj:
                 if command not in fobj.read():
-                    fobj.write("exec {command}\n".format(command=command))
+                    fobj.write("{command}\n".format(command=command))
         else:
             with open(hook, "w+") as fobj:
-                fobj.write(
-                    "#!/bin/sh\n" "exec {command}\n".format(command=command)
-                )
+                fobj.write("#!/bin/sh\n" "{command}\n".format(command=command))
 
         os.chmod(hook, 0o777)
 


Test patch:
diff --git a/tests/func/test_install.py b/tests/func/test_install.py
index 5e5efee29..1e3c7b8ba 100644
--- a/tests/func/test_install.py
+++ b/tests/func/test_install.py
@@ -36,7 +36,11 @@ class TestInstall(object):
 
         assert main(["install"]) == 0
 
-        expected_script = "#!/bin/sh\n" "echo hello\n" "exec dvc checkout\n"
+        expected_script = (
+            "#!/bin/sh\n"
+            "echo hello\n"
+            '[ -z "$(git ls-files .dvc)" ] || exec dvc checkout\n'
+        )
 
         with open(self._hook("post-checkout"), "r") as fobj:
             assert fobj.read() == expected_script
@@ -45,7 +49,10 @@ class TestInstall(object):
         assert main(["install"]) == 0
         assert main(["install"]) == 0
 
-        expected_script = "#!/bin/sh\n" "exec dvc checkout\n"
+        expected_script = (
+            "#!/bin/sh\n"
+            '[ -z "$(git ls-files .dvc)" ] || exec dvc checkout\n'
+        )
 
         with open(self._hook("post-checkout"), "r") as fobj:
             assert fobj.read() == expected_script
diff --git a/tests/func/test_version.py b/tests/func/test_version.py
index a040c1e14..d0fe6f2eb 100644
--- a/tests/func/test_version.py
+++ b/tests/func/test_version.py
@@ -1,6 +1,8 @@
 import re
+import pytest
 
 from dvc.main import main
+from dvc.command.version import psutil
 
 
 def test_info_in_repo(dvc_repo, caplog):
@@ -11,13 +13,19 @@ def test_info_in_repo(dvc_repo, caplog):
     assert re.search(re.compile(r"Platform: .*"), caplog.text)
     assert re.search(re.compile(r"Binary: (True|False)"), caplog.text)
     assert re.search(
-        re.compile(r"Filesystem type \(cache directory\): .*"), caplog.text
+        re.compile(r"(Cache: (.*link - (True|False)(,\s)?){3})"), caplog.text
     )
+
+
+@pytest.mark.skipif(psutil is None, reason="No psutil.")
+def test_fs_info_in_repo(dvc_repo, caplog):
+    assert main(["version"]) == 0
+
     assert re.search(
-        re.compile(r"Filesystem type \(workspace\): .*"), caplog.text
+        re.compile(r"Filesystem type \(cache directory\): .*"), caplog.text
     )
     assert re.search(
-        re.compile(r"(Cache: (.*link - (True|False)(,\s)?){3})"), caplog.text
+        re.compile(r"Filesystem type \(workspace\): .*"), caplog.text
     )
 
 
@@ -28,12 +36,18 @@ def test_info_outside_of_repo(repo_dir, caplog):
     assert re.search(re.compile(r"Python version: \d\.\d\.\d"), caplog.text)
     assert re.search(re.compile(r"Platform: .*"), caplog.text)
     assert re.search(re.compile(r"Binary: (True|False)"), caplog.text)
+    assert not re.search(
+        re.compile(r"(Cache: (.*link - (True|False)(,\s)?){3})"), caplog.text
+    )
+
+
+@pytest.mark.skipif(psutil is None, reason="No psutil.")
+def test_fs_info_outside_of_repo(repo_dir, caplog):
+    assert main(["version"]) == 0
+
     assert re.search(
         re.compile(r"Filesystem type \(workspace\): .*"), caplog.text
     )
     assert not re.search(
         re.compile(r"Filesystem type \(cache directory\): .*"), caplog.text
     )
-    assert not re.search(
-        re.compile(r"(Cache: (.*link - (True|False)(,\s)?){3})"), caplog.text
-    )