Hi,
On Sat, 16 Sep 2023, Mark Levedahl wrote:
> 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.
Sounds good. FWIW I ran a couple experiments here, too:
% file pathtype "C:/foo"
absolute
% file pathtype ".git/hooks"
relative
% file pathtype ".git\\hooks"
relative
% file pathtype "/foo"
volumerelative
% file pathtype "foo"
relative
The problem, therefore, is that `file pathtype` does not discern between a
bare file name and a relative path. The proposed patch looks correct to
me.
Thank you,
Johannes
>
> Signed-off-by: Mark Levedahl <mlevedahl@xxxxxxxxx>
> ---
> 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"
> --
> 2.41.0.99.19
>
>
