Re: [REGRESSION] git-wrapper to run-commands codepath regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> There appears to be a regression in the codepath between git wrapper and
> run_commands API.
>
> 	$ T=/var/tmp/test-commands
> 	$ mkdir $T
> 	$ cat >$T/git-hello <<\-EOF
> 	#!/bin/sh
> 	echo hello
> 	EOF
> 	$ chmod +x $T/git-hello
> 	$ oPATH=$PATH
> 	$ PATH=$T:$PATH
> 	$ export PATH
> 	$ git hello
> 	hello
>
> So far, I added a "hello" subcommand to "git", and it runs correctly.
>
> ...
> Note that we can observe the same regression if you instead make $T
> unreadable with:
>
> 	$ chmod 755 $T/git-hello ;# make it executable again
> 	$ chmod a-rwx $T ;# but that directory cannot be read
> 	$ git hello
>
> So that is the "regression" part.
>
> The following is a tangent that was brought up at $work.

And that is the topic of this follow-up.

> Some people might argue that we should skip $T/git-hello in the last case
> and try to find git-hello in a later directory listed in $PATH, but I do
> not personally think that is a right thing to do.  It would make the
> problem harder to diagnose, and more importantly, the fact that the user
> listed $T earlier in the $PATH is a strong indication that the user wants
> the scripts in $T override the scripts with the same name in directories
> that appear later in the $PATH, and we should report when that is not
> happening, either
>
>  (1) when $T/git-hello was found but was not executable; or
>
>  (2) when we cannot read $T and we cannot even tell $T/git-hello exists or
>      not.
>
> So I think it is Ok to be silent only when we see ENOENT like the current
> code does.

Unfortunately, as we are giving "git-hello" to underlying execvp(3),
instead of splitting $PATH, prefixing each element of it in front of
"git-hello" and trying execv() in a loop, when the call fails, we do not
know which component of $PATH caused execvp() to fail, and end up saying
"fatal: cannot exec 'git-hello': Permission denied".  So in that sense, we
are not helping the end user by making it easier to diagnose the problem.

We would need to emulate what execvp() does ourselves (i.e. split $PATH,
prefix each component and try execv(), ignoring ENOENT or EACCES while
trying next component in $PATH), plus note the first path that got EACCES
so that we can report which script (including its leading directories) had
trouble executing.  Perhaps a simple enough task for beginners.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]