Support for (un)installing hooks in git worktrees

Gitlint no longer hardcodes the path of the commit-msg hook, but takes the
correct path from git itself. This fixes issues with using git worktrees.

This fixes #68.

7 files changed, 122 insertions, 42 deletions

diff --git a/gitlint/git.py b/gitlint/git.py
index 76fe1b9..8602746 100644
--- a/gitlint/git.py
+++ b/gitlint/git.py
@@ -1,3 +1,5 @@
+import os
+
 import arrow
 import sh
 # import exceptions separately, this makes it a little easier to mock them out in the unit tests
@@ -56,6 +58,13 @@ def git_commentchar(repository_path=None):
     return ustr(commentchar).replace(u"\n", u"")
 
 
+def git_hooks_dir(repository_path):
+    """ Determine hooks directory for a given target dir """
+    hooks_dir = _git("rev-parse", "--git-path", "hooks", _cwd=repository_path)
+    hooks_dir = ustr(hooks_dir).replace(u"\n", u"")
+    return os.path.realpath(os.path.join(repository_path, hooks_dir))
+
+
 class GitCommitMessage(object):
     """ Class representing a git commit message. A commit message consists of the following:
       - context: The `GitContext` this commit message is part of
diff --git a/gitlint/hooks.py b/gitlint/hooks.py
index e5748e8..fc4dc4e 100644
--- a/gitlint/hooks.py
+++ b/gitlint/hooks.py
@@ -4,10 +4,10 @@ import os
 import stat
 
 from gitlint.utils import DEFAULT_ENCODING
+from gitlint.git import git_hooks_dir
 
-COMMIT_MSG_HOOK_SRC_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "files/commit-msg")
-COMMIT_MSG_HOOK_DST_PATH = os.path.join(".git", "hooks", "commit-msg")
-HOOKS_DIR_PATH = os.path.join(".git", "hooks")
+COMMIT_MSG_HOOK_SRC_PATH = os.path.join(os.path.dirname(os.path.realpath(__file__)), "files", "commit-msg")
+COMMIT_MSG_HOOK_DST_PATH = "commit-msg"
 GITLINT_HOOK_IDENTIFIER = "### gitlint commit-msg hook start ###\n"
 
 
@@ -20,12 +20,12 @@ class GitHookInstaller(object):
 
     @staticmethod
     def commit_msg_hook_path(lint_config):
-        return os.path.realpath(os.path.join(lint_config.target, COMMIT_MSG_HOOK_DST_PATH))
+        return os.path.join(git_hooks_dir(lint_config.target), COMMIT_MSG_HOOK_DST_PATH)
 
     @staticmethod
     def _assert_git_repo(target):
         """ Asserts that a given target directory is a git repository """
-        hooks_dir = os.path.realpath(os.path.join(target, HOOKS_DIR_PATH))
+        hooks_dir = git_hooks_dir(target)
         if not os.path.isdir(hooks_dir):
             raise GitHookInstallerError(u"{0} is not a git repository.".format(target))
 
diff --git a/gitlint/tests/test_cli.py b/gitlint/tests/test_cli.py
index a92a13d..319532f 100644
--- a/gitlint/tests/test_cli.py
+++ b/gitlint/tests/test_cli.py
@@ -447,11 +447,12 @@ class CLITests(BaseTestCase):
         self.assertEqual(result.exit_code, 0)
 
     @patch('gitlint.hooks.GitHookInstaller.install_commit_msg_hook')
-    def test_install_hook(self, install_hook):
+    @patch('gitlint.hooks.git_hooks_dir', return_value=os.path.join(u"/hür", u"dur"))
+    def test_install_hook(self, _, install_hook):
         """ Test for install-hook subcommand """
         result = self.cli.invoke(cli.cli, ["install-hook"])
-        expected_path = os.path.join(os.getcwd(), hooks.COMMIT_MSG_HOOK_DST_PATH)
-        expected = "Successfully installed gitlint commit-msg hook in {0}\n".format(expected_path)
+        expected_path = os.path.join(u"/hür", u"dur", hooks.COMMIT_MSG_HOOK_DST_PATH)
+        expected = u"Successfully installed gitlint commit-msg hook in {0}\n".format(expected_path)
         self.assertEqual(result.exit_code, 0)
         self.assertEqual(result.output, expected)
         expected_config = config.LintConfig()
@@ -459,11 +460,12 @@ class CLITests(BaseTestCase):
         install_hook.assert_called_once_with(expected_config)
 
     @patch('gitlint.hooks.GitHookInstaller.install_commit_msg_hook')
-    def test_install_hook_target(self, install_hook):
+    @patch('gitlint.hooks.git_hooks_dir', return_value=os.path.join(u"/hür", u"dur"))
+    def test_install_hook_target(self, _, install_hook):
         """  Test for install-hook subcommand with a specific --target option specified """
         # Specified target
         result = self.cli.invoke(cli.cli, ["--target", self.SAMPLES_DIR, "install-hook"])
-        expected_path = os.path.realpath(os.path.join(self.SAMPLES_DIR, hooks.COMMIT_MSG_HOOK_DST_PATH))
+        expected_path = os.path.join(u"/hür", u"dur", hooks.COMMIT_MSG_HOOK_DST_PATH)
         expected = "Successfully installed gitlint commit-msg hook in %s\n" % expected_path
         self.assertEqual(result.exit_code, 0)
         self.assertEqual(result.output, expected)
@@ -483,11 +485,12 @@ class CLITests(BaseTestCase):
         install_hook.assert_called_once_with(expected_config)
 
     @patch('gitlint.hooks.GitHookInstaller.uninstall_commit_msg_hook')
-    def test_uninstall_hook(self, uninstall_hook):
+    @patch('gitlint.hooks.git_hooks_dir', return_value=os.path.join(u"/hür", u"dur"))
+    def test_uninstall_hook(self, _, uninstall_hook):
         """ Test for uninstall-hook subcommand """
         result = self.cli.invoke(cli.cli, ["uninstall-hook"])
-        expected_path = os.path.realpath(os.path.join(os.getcwd(), hooks.COMMIT_MSG_HOOK_DST_PATH))
-        expected = "Successfully uninstalled gitlint commit-msg hook from {0}\n".format(expected_path)
+        expected_path = os.path.join(u"/hür", u"dur", hooks.COMMIT_MSG_HOOK_DST_PATH)
+        expected = u"Successfully uninstalled gitlint commit-msg hook from {0}\n".format(expected_path)
         self.assertEqual(result.exit_code, 0)
         self.assertEqual(result.output, expected)
         expected_config = config.LintConfig()
diff --git a/gitlint/tests/test_git.py b/gitlint/tests/test_git.py
index bb1ac2b..f64776f 100644
--- a/gitlint/tests/test_git.py
+++ b/gitlint/tests/test_git.py
@@ -1,6 +1,8 @@
 # -*- coding: utf-8 -*-
 
 import datetime
+import os
+
 import dateutil
 
 try:
@@ -13,7 +15,8 @@ except ImportError:
 from sh import ErrorReturnCode, CommandNotFound
 
 from gitlint.tests.base import BaseTestCase
-from gitlint.git import GitContext, GitCommit, GitCommitMessage, GitContextError, GitNotInstalledError, git_commentchar
+from gitlint.git import GitContext, GitCommit, GitCommitMessage, GitContextError, \
+    GitNotInstalledError, git_commentchar, git_hooks_dir
 
 
 class GitTests(BaseTestCase):
@@ -375,7 +378,19 @@ class GitTests(BaseTestCase):
         self.assertEqual(git_commentchar(), u"ä")
 
         git.return_value = ';\n'
-        self.assertEqual(git_commentchar(), ';')
+        self.assertEqual(git_commentchar(os.path.join(u"/föo", u"bar")), ';')
+
+        git.assert_called_with("config", "--get", "core.commentchar", _ok_code=[0, 1],
+                               _cwd=os.path.join(u"/föo", u"bar"))
+
+    @patch("gitlint.git._git")
+    def test_git_hooks_dir(self, git):
+        hooks_dir = os.path.join(u"föo", ".git", "hooks")
+        git.return_value.__str__ = lambda _: hooks_dir + "\n"
+        git.return_value.__unicode__ = lambda _: hooks_dir + "\n"
+        self.assertEqual(git_hooks_dir(u"/blä"), os.path.join(u"/blä", hooks_dir))
+
+        git.assert_called_once_with("rev-parse", "--git-path", "hooks", _cwd=u"/blä")
 
     @patch("gitlint.git.git_commentchar")
     def test_commit_msg_custom_commentchar(self, patched):
diff --git a/gitlint/tests/test_hooks.py b/gitlint/tests/test_hooks.py
index ca487c9..4fd5836 100644
--- a/gitlint/tests/test_hooks.py
+++ b/gitlint/tests/test_hooks.py
@@ -16,11 +16,16 @@ from gitlint.hooks import GitHookInstaller, GitHookInstallerError, COMMIT_MSG_HO
 
 
 class HookTests(BaseTestCase):
-    def test_commit_msg_hook_path(self):
+
+    @patch('gitlint.hooks.git_hooks_dir')
+    def test_commit_msg_hook_path(self, git_hooks_dir):
+        git_hooks_dir.return_value = os.path.join(u"/föo", u"bar")
         lint_config = LintConfig()
         lint_config.target = self.SAMPLES_DIR
-        expected_path = os.path.join(self.SAMPLES_DIR, COMMIT_MSG_HOOK_DST_PATH)
+        expected_path = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
         path = GitHookInstaller.commit_msg_hook_path(lint_config)
+
+        git_hooks_dir.assert_called_once_with(self.SAMPLES_DIR)
         self.assertEqual(path, expected_path)
 
     @staticmethod
@@ -29,36 +34,41 @@ class HookTests(BaseTestCase):
     @patch('gitlint.hooks.shutil.copy')
     @patch('os.path.exists', return_value=False)
     @patch('os.path.isdir', return_value=True)
-    def test_install_commit_msg_hook(isdir, path_exists, copy, stat, chmod):
+    @patch('gitlint.hooks.git_hooks_dir')
+    def test_install_commit_msg_hook(git_hooks_dir, isdir, path_exists, copy, stat, chmod):
         lint_config = LintConfig()
-        lint_config.target = os.path.join(u"/foo", u"bår")
-        expected_dst = os.path.join(u"/foo", u"bår", COMMIT_MSG_HOOK_DST_PATH)
+        lint_config.target = os.path.join(u"/hür", u"dur")
+        git_hooks_dir.return_value = os.path.join(u"/föo", u"bar", ".git", "hooks")
+        expected_dst = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
         GitHookInstaller.install_commit_msg_hook(lint_config)
-        isdir.assert_called_with(os.path.join(u"/foo", u"bår", ".git", "hooks"))
+        isdir.assert_called_with(git_hooks_dir.return_value)
         path_exists.assert_called_once_with(expected_dst)
         copy.assert_called_once_with(COMMIT_MSG_HOOK_SRC_PATH, expected_dst)
         stat.assert_called_once_with(expected_dst)
         chmod.assert_called_once_with(expected_dst, ANY)
+        git_hooks_dir.assert_called_with(lint_config.target)
 
     @patch('gitlint.hooks.shutil.copy')
     @patch('os.path.exists', return_value=False)
     @patch('os.path.isdir', return_value=True)
-    def test_install_commit_msg_hook_negative(self, isdir, path_exists, copy):
+    @patch('gitlint.hooks.git_hooks_dir')
+    def test_install_commit_msg_hook_negative(self, git_hooks_dir, isdir, path_exists, copy):
         lint_config = LintConfig()
-        lint_config.target = os.path.join(u"/foo", u"bår")
+        lint_config.target = os.path.join(u"/hür", u"dur")
+        git_hooks_dir.return_value = os.path.join(u"/föo", u"bar", ".git", "hooks")
         # mock that current dir is not a git repo
         isdir.return_value = False
-        expected_msg = u"{0} is not a git repository".format(os.path.join(u"/foo", u"bår"))
+        expected_msg = u"{0} is not a git repository".format(lint_config.target)
         with self.assertRaisesRegex(GitHookInstallerError, expected_msg):
             GitHookInstaller.install_commit_msg_hook(lint_config)
-            isdir.assert_called_with(os.path.join(u"/foo", u"bår", ".git", "hooks"))
+            isdir.assert_called_with(git_hooks_dir.return_value)
             path_exists.assert_not_called()
             copy.assert_not_called()
 
         # mock that there is already a commit hook present
         isdir.return_value = True
         path_exists.return_value = True
-        expected_dst = os.path.join(u"/foo", u"bår", COMMIT_MSG_HOOK_DST_PATH)
+        expected_dst = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
         expected_msg = u"There is already a commit-msg hook file present in {0}.\n".format(expected_dst) + \
                        "gitlint currently does not support appending to an existing commit-msg file."
         with self.assertRaisesRegex(GitHookInstallerError, expected_msg):
@@ -68,41 +78,47 @@ class HookTests(BaseTestCase):
     @patch('os.remove')
     @patch('os.path.exists', return_value=True)
     @patch('os.path.isdir', return_value=True)
-    def test_uninstall_commit_msg_hook(isdir, path_exists, remove):
+    @patch('gitlint.hooks.git_hooks_dir')
+    def test_uninstall_commit_msg_hook(git_hooks_dir, isdir, path_exists, remove):
         lint_config = LintConfig()
-        lint_config.target = os.path.join(u"/foo", u"bår")
+        git_hooks_dir.return_value = os.path.join(u"/föo", u"bar", ".git", "hooks")
+        lint_config.target = os.path.join(u"/hür", u"dur")
         read_data = "#!/bin/sh\n" + GITLINT_HOOK_IDENTIFIER
         with patch('gitlint.hooks.io.open', mock_open(read_data=read_data), create=True):
             GitHookInstaller.uninstall_commit_msg_hook(lint_config)
 
-        expected_dst = os.path.join(u"/foo", u"bår", COMMIT_MSG_HOOK_DST_PATH)
-        isdir.assert_called_with(os.path.join(u"/foo", u"bår", ".git", "hooks"))
+        expected_dst = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
+        isdir.assert_called_with(git_hooks_dir.return_value)
         path_exists.assert_called_once_with(expected_dst)
         remove.assert_called_with(expected_dst)
+        git_hooks_dir.assert_called_with(lint_config.target)
 
     @patch('os.remove')
     @patch('os.path.exists', return_value=True)
     @patch('os.path.isdir', return_value=True)
-    def test_uninstall_commit_msg_hook_negative(self, isdir, path_exists, remove):
+    @patch('gitlint.hooks.git_hooks_dir')
+    def test_uninstall_commit_msg_hook_negative(self, git_hooks_dir, isdir, path_exists, remove):
         lint_config = LintConfig()
-        lint_config.target = os.path.join(u"/foo", u"bår")
+        lint_config.target = os.path.join(u"/hür", u"dur")
+        git_hooks_dir.return_value = os.path.join(u"/föo", u"bar", ".git", "hooks")
+
         # mock that the current directory is not a git repo
         isdir.return_value = False
-        expected_msg = u"{0} is not a git repository".format(os.path.join(u"/foo", u"bår"))
+        expected_msg = u"{0} is not a git repository".format(lint_config.target)
         with self.assertRaisesRegex(GitHookInstallerError, expected_msg):
             GitHookInstaller.uninstall_commit_msg_hook(lint_config)
-            isdir.assert_called_with(os.path.join(u"/foo", u"bår", '.git/hooks'))
+            isdir.assert_called_with(git_hooks_dir.return_value)
             path_exists.assert_not_called()
             remove.assert_not_called()
 
         # mock that there is no commit hook present
         isdir.return_value = True
         path_exists.return_value = False
-        expected_dst = os.path.join(u"/foo", u"bår", COMMIT_MSG_HOOK_DST_PATH)
+        expected_dst = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
         expected_msg = u"There is no commit-msg hook present in {0}.".format(expected_dst)
         with self.assertRaisesRegex(GitHookInstallerError, expected_msg):
             GitHookInstaller.uninstall_commit_msg_hook(lint_config)
-            isdir.assert_called_with(os.path.join(u"/foo", u"bår", '.git/hooks'))
+            isdir.assert_called_with(git_hooks_dir.return_value)
             path_exists.assert_called_once_with(expected_dst)
             remove.assert_not_called()
 
@@ -110,7 +126,7 @@ class HookTests(BaseTestCase):
         isdir.return_value = True
         path_exists.return_value = True
         read_data = "#!/bin/sh\nfoo"
-        expected_dst = os.path.join(u"/foo", u"bår", COMMIT_MSG_HOOK_DST_PATH)
+        expected_dst = os.path.join(git_hooks_dir.return_value, COMMIT_MSG_HOOK_DST_PATH)
         expected_msg = u"The commit-msg hook in {0} was not installed by gitlint ".format(expected_dst) + \
                        r"\(or it was modified\).\nUninstallation of 3th party or modified gitlint hooks " + \
                        "is not supported."
diff --git a/qa/base.py b/qa/base.py
index 511ab8c..24039b6 100644
--- a/qa/base.py
+++ b/qa/base.py
@@ -62,20 +62,25 @@ class BaseTestCase(TestCase):
     @classmethod
     def setUpClass(cls):
         """ Sets up the integration tests by creating a new temporary git repository """
-        cls._tmp_git_repos = [cls.create_tmp_git_repo()]
-        cls.tmp_git_repo = cls._tmp_git_repos[0]
+        cls.tmp_git_repos = []
+        cls.tmp_git_repo = cls.create_tmp_git_repo()
 
     @classmethod
     def tearDownClass(cls):
         """ Cleans up the temporary git repositories """
-        for repo in cls._tmp_git_repos:
+        for repo in cls.tmp_git_repos:
             rm("-rf", repo)
 
     @classmethod
+    def generate_temp_path(cls):
+        return os.path.realpath("/tmp/gitlint-test-{0}".format(datetime.now().strftime("%Y%m%d-%H%M%S-%f")))
+
+    @classmethod
     def create_tmp_git_repo(cls):
         """ Creates a temporary git repository and returns its directory path """
-        tmp_git_repo = os.path.realpath("/tmp/gitlint-test-{0}".format(
-            datetime.now().strftime("%Y%m%d-%H%M%S-%f")))
+        tmp_git_repo = cls.generate_temp_path()
+        cls.tmp_git_repos.append(tmp_git_repo)
+
         git("init", tmp_git_repo)
         # configuring name and email is required in every git repot
         git("config", "user.name", "gitlint-test-user", _cwd=tmp_git_repo)
@@ -89,6 +94,7 @@ class BaseTestCase(TestCase):
         # Git on mac doesn't like unicode characters by default, so we need to set this option
         # http://stackoverflow.com/questions/5581857/git-and-the-umlaut-problem-on-mac-os-x
         git("config", "core.precomposeunicode", "true", _cwd=tmp_git_repo)
+
         return tmp_git_repo
 
     def _create_simple_commit(self, message, out=None, ok_code=None, env=None, git_repo=None, tty_in=False):
diff --git a/qa/test_hooks.py b/qa/test_hooks.py
index b36cde6..29080fc 100644
--- a/qa/test_hooks.py
+++ b/qa/test_hooks.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # pylint: disable=too-many-function-args,unexpected-keyword-arg
+import os
 from sh import git, gitlint  # pylint: disable=no-name-in-module
 from qa.base import BaseTestCase
 
@@ -115,3 +116,33 @@ class HookTests(BaseTestCase):
             self.assertMultiLineEqual(
                 output.replace('\r', ''),
                 expected.replace('\r', ''))
+
+    def test_commit_hook_worktree(self):
+        """ Tests that hook installation and un-installation also work in git worktrees.
+        Test steps:
+            ```sh
+            git init <tmpdir>
+            cd <tmpdir>
+            git worktree add <worktree-tempdir>
+            cd <worktree-tempdir>
+            gitlint install-hook
+            gitlint uninstall-hook
+            ```
+        """
+        tmp_git_repo = self.create_tmp_git_repo()
+        self._create_simple_commit(u"Simple title\n\nContënt in the body", git_repo=tmp_git_repo)
+
+        worktree_dir = self.generate_temp_path()
+        self.tmp_git_repos.append(worktree_dir)  # make sure we clean up the worktree afterwards
+
+        git("worktree", "add", worktree_dir, _cwd=tmp_git_repo, _tty_in=True)
+
+        output_installed = gitlint("install-hook", _cwd=worktree_dir)
+        expected_hook_path = os.path.join(tmp_git_repo, ".git", "hooks", "commit-msg")
+        expected_msg = "Successfully installed gitlint commit-msg hook in {0}\n".format(expected_hook_path)
+        self.assertEqual(output_installed, expected_msg)
+
+        output_uninstalled = gitlint("uninstall-hook", _cwd=worktree_dir)
+        expected_hook_path = os.path.join(tmp_git_repo, ".git", "hooks", "commit-msg")
+        expected_msg = "Successfully uninstalled gitlint commit-msg hook from {0}\n".format(expected_hook_path)
+        self.assertEqual(output_uninstalled, expected_msg)