Re: git-gui ignores core.hooksPath

Hi,

On Fri, 2 Jun 2017, Ævar Arnfjörð Bjarmason wrote:

On Fri, Jun 2, 2017 at 3:41 PM, Philipp Gortan [off-list ref] wrote:

quoted

I've been unhappy for quite a while that I had to configure the hooks
manually for each of my repos - until I found out recently that there
is the core.hooksPath config variable that (when set globally) allows
me to specify a hooks directory to be used for all my repositories.

Now I was happy - for a few minutes, until I tested this feature in
git-gui, and realized that it doesn't work there.

This seems to be caused by "proc githook_read", which says "set pchook
[gitdir hooks $hook_name]" instead of querying "git config
core.hooksPath" first - cf
https://github.com/git/git/blob/2cc2e70264e0fcba04f9ef791d144bbc8b501206/git-gui/git-gui.sh#L627

Would be great if this could get fixed...

Indeed. Why don't you give it a try?

This indeed is something that should be fixed, but git-gui development
is managed outside of git.git, it's just occasionally pulled in. I'm
not what the best place to contact is, but I've CC'd
Philip Oakley who's been making recent commits to git-gui.git at
http://repo.or.cz/git-gui.git/

Philip is in the same spot as I am: we both worked on Git GUI to improve
it for Git for Windows users, but Pat has been silent for over half a year
on all of our PRs.

In the meantime, I managed to get a couple of changes into git.git via
Junio, but the situation is far from ideal.

So what I settled on is to carry a couple of Git GUI patches in Git for
Windows' fork, until the time when the patches finally get accepted into
https://github.com/patthoyts/git-gui.

In this particular instance, the only question is whether to use `git
rev-parse --git-path hooks` or re-roll the core.hookspath logic in
git-gui.

Both approaches have their downsides:

- rev-parse --git-path was broken in subdirectories for a *really* long
  time. Since Git GUI is supposed to be relatively independent from the
  version of the installed git executable, that would imply a couple of
  ugly extra code just to make sure that it works correctly.

- duplicating the core.hookspath logic is prone to become stale over time,
  as Git may change the behavior (as it did with the core.hookspath
  setting).

The subdirectory problem of --git-path is actually not that bad, as Git
GUI cd's to the top-level directory anyway. So that bug does not affect
us. The only caveat is that --git-path was only introduced into v2.5.0,
and Git GUI has conditional code to even support pre-1.6.3 versions.

Happily, pre-1.6.3 versions are not supposed to handle core.hookspath in
the way v2.9.0 and later handle it.

So something like this *may* work:

-- snip --

diff --git a/git-gui/git-gui.sh b/git-gui/git-gui.sh
index 37c1c5d227b..3067a3b000a 100755
--- a/git-gui/git-gui.sh
+++ b/git-gui/git-gui.sh

@@ -624,7 +624,11 @@ proc git_write {args} {
 }
 
 proc githook_read {hook_name args} {
-	set pchook [gitdir hooks $hook_name]
+	if {[package vcompare $::_git_version 2.5.0] >= 0} {
+		set pchook [git rev-parse --git-path "hooks/$hook_name"]
+	} else {
+		set pchook [gitdir hooks $hook_name]
+	}
 	lappend args 2>@1
 
 	# On Windows [file executable] might lie so we need to ask
-- snap --

Philipp, this is as far as I will go with this. If you truly desire this
to be fixed, please take it from here (read: test, fix and submit with a
good commit message).

Ciao,
Johannes