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

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

From
Junio C Hamano <gitster@pobox.com>
Date
Oct 2, 2012, 17:35 UTC
Message-ID
<7vvces93qj.fsf@alter.siamese.dyndns.org>
In-Reply-To
<1349189193-25497-1-git-send-email-cmn@elego.de>
Carlos Martín Nieto <cmn@elego.de> writes:
Show 30 quoted lines
> 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 <cmn@elego.de>
>
> ---
>
> 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)
Show 8 quoted lines
> @@ -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;
Previous: Carlos Martín NietoNext: Carlos Martín Nieto
Message 2 of 5 in “run-command: don't try to execute directories”
  1. run-command: don't try to execute directoriesCarlos Martín Nieto, Oct 2, 2012
  2. Junio C HamanoOct 2, 2012
  3. Carlos Martín NietoOct 2, 2012
  4. Junio C HamanoOct 2, 2012
  5. Jeff KingOct 2, 2012

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.