prati0100 / git-gui

Tcl/Tk based UI for Git. I am currently acting as the project's maintainer.
160 stars 87 forks source link

git-gui - re-enable use of hook scripts #100

Open gitster opened 9 months ago

gitster commented 9 months ago

Earlier, commit aae9560a introduced search in $PATH to find executables before running them, avoiding an issue where on Windows a same named file in the current directory can be executed in preference to anything in a directory in $PATH. This search is intended to find an absolute path for a bare executable ( e.g, a function "foo") by finding the first instance of "foo" in a directory given in $PATH, and this search works correctly. The search is explicitly avoided for an executable named with an absolute path (e.g., /bin/sh), and that works as well.

Unfortunately, the search is also applied to commands named with a relative path. A hook script (or executable) $HOOK is usually located relative to the project directory as .git/hooks/$HOOK. The search for this will generally fail as that relative path will (probably) not exist on any directory in $PATH. This means that git hooks in general now fail to run. Considerable mayhem could occur should a directory on $PATH be git controlled. If such a directory includes .git/hooks/$HOOK, that repository's $HOOK will be substituted for the one in the current project, with unknown consequences.

This lookup failure also occurs in worktrees linked to a remote .git directory using git-new-workdir. However, a worktree using a .git file pointing to a separate git directory apparently avoids this: in that case the hook command is resolved to an absolute path before being passed down to the code introduced in aae9560a.

Fix this by replacing the test for an "absolute" pathname to a check for a command name having more than one pathname component. This limits the search and absolute pathname resolution to bare commands. The new test uses tcl's "file split" command. Experiments on Linux and Windows, using tclsh, show that command names with relative and absolute paths always give at least two components, while a bare command gives only one.

  Linux:   puts [file split {foo}]       ==>  foo
  Linux:   puts [file split {/foo}]      ==>  / foo
  Linux:   puts [file split {.git/foo}]  ==> .git foo
  Windows: puts [file split {foo}]       ==>  foo
  Windows: puts [file split {c:\foo}]    ==>  c:/ foo
  Windows: puts [file split {.git\foo}]  ==> .git foo

The above results show the new test limits search and replacement to bare commands on both Linux and Windows.

gitster commented 9 months ago

I am planning to merge this directly to my tree (via -Xsubtree=git-gui/ option), and thought that it would make it easier to maintain your tree up to date by simply fast-forwarding to it (the commit is built directly on top of your 'master').

Thanks.

prati0100 commented 9 months ago

Hi @gitster

I fetched your tree just now and I see that you have not yet merged it. My master branch has moved since you opened this PR. If it works for you, I can apply the patch and send you a pull request today. I am looking at it now.

gitster commented 9 months ago

My copy already has these in 'next', but they are identical so it is not a huge deal.