Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH] run-command: don't try to execute directories

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:55

Carlos Martín Nieto [off-list ref] writes:
When looking through $PATH to try to find an external command,
locate_in_PATH doesn't check that it's trying to execute a file. Add a
check to make sure we won't try to execute a directory.

This also stops us from looking further and maybe finding that the
user meant an alias, as in the case where the user has
/home/user/bin/git-foo/git-foo.pl and an alias

    [alias] foo = !/home/user/bin/git-foo/git-foo.pl

Running 'git foo' will currently will try to execute ~/bin/git-foo and
fail because you can't execute a directory. By making sure we don't do
that, we realise that it's an alias and do the right thing

Signed-off-by: Carlos Martín Nieto <redacted>

---

This comes from a case in #git. Not sure if this is worth it, or the
better solution is just to say no to dirs in $PATH.

After writing all of that, I thought to check the shell, and indeed

    % git-foo
    zsh: permission denied: git-foo

so if the shell doesn't do it, the benefits probably don't outweigh
having a dozen stat instead of access calls. strace reveals that zsh
does what git currently does. bash uses stat and says 'command not
found'.
Hrm, I do not use zsh but it does not seem to reproduce for me.

	$ mkdir -p /var/tmp/xx/git
        $ zsh
        % PATH=/var/tmp/xx:$PATH
        % type git
        git is /home/junio/bin/git
        % git version
        git version 1.8.0.rc0.45.g7ce8dc5
	% zsh --version
	zsh 4.3.10 (x86_64-unknown-linux-gnu)
quoted hunk
@@ -101,8 +102,9 @@ static char *locate_in_PATH(const char *file)
 		}
 		strbuf_addstr(&buf, file);
 
-		if (!access(buf.buf, F_OK))
+		if (!stat(buf.buf, &st) && !S_ISDIR(st.st_mode)) {
 			return strbuf_detach(&buf, NULL);
+		}
So we used to say "if it exists and accessible, return that".  Now
we say "if it exists and is not a directory, return that".

I have to wonder what would happen if it exists as a non-directory
but we cannot access it.  Is that a regression?

 		if (!*end)
 			break;
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help