opendev/git-review
Code Issues Proposed changes

Support the Git "core.hooksPath" option when dealing with hook scripts

Browse Source 
Previously, git-review would assume that the Git repository's hook
directory is .git/hooks, relative to the root of the checkout. This
assumption breaks if the user has set the core.hooksPath option on the
repository (or, for that matter, in ~/.gitconfig or /etc/gitconfig).

core.hooksPath can either be set to an absolute path, in which case it
is to be interpreted as-is, or to a relative path, in which case it
should be interpreted as relative to the root of the checkout.

Introduce a new convenience function to suss out the correct path, and
use it in places where the reference to .git/hooks was previously
hard-coded.

Reference:
https: //git-scm.com/docs/git-config#Documentation/git-config.txt-corehooksPath

Depends-on: I0f0f44e57a100420d8e6d2eaec7dbb5d77b654af
Change-Id: Id8a3ac464ff75e6d8207f198089f018cc790eca5
This commit is contained in:
Florian Haas 2021-06-16 20:51:59 +02:00
parent d0d3da82f5
commit c40eb491e6
3 changed files with 81 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
git_review/cmd.py
View File

@ -247,7 +247,7 @@ def run_custom_script(action):
script_file = "%s-review" % (action)
(top_dir, git_dir) = git_directories()
paths = [os.path.join(CONFIGDIR, "hooks", script_file),
os.path.join(git_dir, "hooks", script_file)]
os.path.join(git_get_hooks_path(top_dir, git_dir), script_file)]
for fpath in paths:
if os.path.isfile(fpath) and os.access(fpath, os.X_OK):
status, output = run_command_status(fpath)
@ -285,7 +285,26 @@ def git_config_get_value(section, option, default=None, as_bool=False):
raise
def git_get_hooks_path(top_dir, git_dir):
"""Get the path where we need to store and retrieve Git hooks.
Normally hooks go into .git/hooks, but users can override with the
core.hooksPath option. This can either be an absolute path, in
which case we use it as-is, or a relative path, in which case we
must interpret it as relative to top_dir.
"""
hook_dir = os.path.join(git_dir, "hooks")
hooks_path_option = git_config_get_value('core', 'hooksPath')
if hooks_path_option:
if os.path.isabs(hooks_path_option):
hook_dir = hooks_path_option
else:
hook_dir = os.path.join(top_dir, hooks_path_option)
return hook_dir
class Config(object):
"""Expose as dictionary configuration options."""
def __init__(self, config_file=None):
@ -365,6 +384,9 @@ def set_hooks_commit_msg(remote, target_file):
run_command_exc(CannotInstallHook, *cmd)
# If there are submodules, the hook needs to be installed into
# each of them.
# Here, we don't check for any nonstandard hooks path, because
# it should be safe to assume that very few users are inclined
# to set the core.hooksPath option in a submodule checkout.
run_command_exc(
CannotInstallHook,
"git", "submodule", "foreach",
@ -1686,7 +1708,8 @@ additional information:
if options.custom_script:
run_custom_script("pre")
hook_file = os.path.join(git_dir, "hooks", "commit-msg")
hook_dir = git_get_hooks_path(top_dir, git_dir)
hook_file = os.path.join(hook_dir, "commit-msg")
have_hook = os.path.exists(hook_file) and os.access(hook_file, os.X_OK)
if not have_hook:

48
git_review/tests/test_git_review.py
View File

@ -18,6 +18,7 @@
import json
import os
import shutil
import tempfile
import testtools
from git_review import tests
@ -102,6 +103,53 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
self._run_git_review(
'-s', chdir=os.path.join(self.test_dir, 'subdirectory'))
def test_git_review_s_without_core_hooks_path_option(self):
"""Test whether git-review -s correctly creates the commit-msg hook,
with the Git core.hooksPath option unset.
"""
hooks_subdir = ".git/hooks"
self.reset_remote()
# There really isn't a good way to ensure that
# "core.hooksPath" option is presently unset. "git config
# --unset" is not idempotent; if you try to unset a config
# option that isn't defined it fails with an exit code of
# 5. Running "git config core.hooksPath" to retrieve the value
# returns 1 if unset, but we can't use self.assertRaises here
# either because run_cmd raises a generic Exception and that's
# too broad. So instead, we don't check core.hooksPath at all
# here, and instead rely on the next two tests to unset the
# option after they've set it.
self._run_git_review('-s')
self.assertTrue(os.path.exists(os.path.join(self.test_dir,
hooks_subdir,
'commit-msg')))
def test_git_review_s_with_core_hooks_path_option_relative(self):
"""Test whether git-review -s correctly creates the commit-msg hook,
with the Git core.hooksPath option set to a relative path.
"""
hooks_subdir = "foo"
self.reset_remote()
self._run_git("config", "core.hooksPath", hooks_subdir)
self._run_git_review('-s')
self.assertTrue(os.path.exists(os.path.join(self.test_dir,
hooks_subdir,
'commit-msg')))
self._run_git("config", "--unset", "core.hooksPath")
def test_git_review_s_with_core_hooks_path_option_absolute(self):
"""Test whether git-review -s correctly creates the commit-msg hook,
with the Git core.hooksPath option set to an absolute path.
"""
self.reset_remote()
with tempfile.TemporaryDirectory() as hooks_dir:
self._run_git("config", "core.hooksPath", hooks_dir)
self._run_git_review('-s')
self.assertTrue(os.path.exists(os.path.join(hooks_dir,
'commit-msg')))
self._run_git("config", "--unset", "core.hooksPath")
def test_git_review_d(self):
"""Test git-review -d."""
self._run_git_review('-s')

8
releasenotes/notes/support-core-hooks-path-ea5402d5d6204f70.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
git-review now handles the Git "core.hooksPath" configuration
option correctly. Thus, it installs the commit-msg hook into the
core.hooksPath directory, if that option is set. Otherwise, it
continues to install the hook into .git/hooks, relative to the
root of the checkout.