git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2] git-gui - re-enable use of hook scripts

From
Pratyush Yadav <me@yadavpratyush.com>
Date
Sep 20, 2023, 13:27 UTC
Message-ID
<mafs0wmwlotya.fsf@yadavpratyush.com>
In-Reply-To
<20230916210131.78593-1-mlevedahl@gmail.com>
Hi,
On Sat, Sep 16 2023, Mark Levedahl wrote:
Show 43 quoted lines
> 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.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
Looks good. Thanks.
Reviewed-by: Pratyush Yadav <me@yadavpratyush.com>
Show 17 quoted lines
> ---
>  git-gui.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8bc8892..8603437 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -118,7 +118,7 @@ proc sanitize_command_line {command_line from_index} {
>  	set i $from_index
>  	while {$i < [llength $command_line]} {
>  		set cmd [lindex $command_line $i]
> -		if {[file pathtype $cmd] ne "absolute"} {
> +		if {[llength [file split $cmd]] < 2} {
>  			set fullpath [_which $cmd]
>  			if {$fullpath eq ""} {
>  				throw {NOT-FOUND} "$cmd not found in PATH"
-- 
Regards,
Pratyush Yadav
Previous: Junio C HamanoNext: Junio C Hamano
Message 21 of 25 in “BUG: git-gui no longer executes hook scripts”
  1. Mark LevedahlSep 15, 2023
  2. Junio C HamanoSep 15, 2023
  3. Junio C HamanoSep 15, 2023
  4. Mark LevedahlSep 15, 2023
  5. git-gui - re-enable use of hook scriptsMark Levedahl, Sep 16, 2023
  6. Junio C HamanoSep 16, 2023
  7. git-gui - re-enable use of hook scriptsMark Levedahl, Sep 16, 2023
  8. Junio C HamanoSep 16, 2023
  9. Mark LevedahlSep 17, 2023
  10. git-gui - use git-hook, honor core.hooksPathMark Levedahl, Sep 17, 2023
  11. Johannes SchindelinSep 18, 2023
  12. Junio C HamanoSep 18, 2023
  13. Mark LevedahlSep 18, 2023
  14. Junio C HamanoSep 18, 2023
  15. Pratyush YadavSep 20, 2023
  16. Mark LevedahlSep 20, 2023
  17. Junio C HamanoSep 20, 2023
  18. Junio C HamanoSep 20, 2023
  19. Johannes SchindelinSep 18, 2023
  20. Junio C HamanoSep 18, 2023
  21. Pratyush YadavSep 20, 2023
  22. Junio C HamanoSep 16, 2023
  23. Mark LevedahlSep 16, 2023
  24. Mark LevedahlSep 16, 2023
  25. Junio C HamanoSep 16, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.