git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Rannaud <eric.rannaud@gmail.com>
Cc: git@vger.kernel.org,
	Johannes Schindelin <johannes.schindelin@gmx.de>,
	Jeremy Serror <jeremy.serror@gmail.com>
Subject: Re: git rebase regression: cannot pass a shell expression directly to --exec
Date: Mon, 15 May 2017 23:37:36 -0400	[thread overview]
Message-ID: <20170516033736.23cfkouus3p67vhc@sigill.intra.peff.net> (raw)
In-Reply-To: <20170516032503.bzkxmtqpmppxgi75@sigill.intra.peff.net>

On Mon, May 15, 2017 at 11:25:03PM -0400, Jeff King wrote:

> One hack would be to look for BASH_FUNC_* in the environment and disable
> the optimization in that case. I think that would make your case Just
> Work. It doesn't help other oddball cases, like:
> 
>   - you're trying to run a shell builtin that behaves differently than
>     its exec-able counterpart
> 
>   - your shell has some other mechanism for defining commands that we
>     would not find via exec. I don't know of one offhand. Obviously $ENV
>     could point to a file which defines some, but for most shells would
>     not read any startup files for a non-interactive "sh -c" invocation.

So I was thinking something like the patch below, though I guess
technically you could look for BASH_FUNC_$argv[0]%%, which seems to be
bash's magic variable name. I hate to get too intimate with those
details, though.

Another option is to speculatively run "foo" without the shell, and if
execve fails to find it, then fall back to running the shell. That would
catch any number of cases where the shell "somehow" finds a command that
we can't.

You'd still have confusing behavior if your shell builtin behaved
differently than the exec-able version, though (because we'd quietly use
the exec-able one), but I would imagine that's exceedingly rare.

I dunno. Maybe the whole thing is a fool's errand.

diff --git a/run-command.c b/run-command.c
index 1c02bfb2e..8328d27fb 100644
--- a/run-command.c
+++ b/run-command.c
@@ -240,12 +240,24 @@ int sane_execvp(const char *file, char * const argv[])
 	return -1;
 }
 
+static int env_has_bash_functions(void)
+{
+	extern char **environ;
+	char **key;
+
+	for (key = environ; *key; key++)
+		if (starts_with(*key, "BASH_FUNC_"))
+			return 1;
+	return 0;
+}
+
 static const char **prepare_shell_cmd(struct argv_array *out, const char **argv)
 {
 	if (!argv[0])
 		die("BUG: shell command is empty");
 
-	if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0])) {
+	if (strcspn(argv[0], "|&;<>()$`\\\"' \t\n*?[#~=%") != strlen(argv[0]) ||
+	    env_has_bash_functions()) {
 #ifndef GIT_WINDOWS_NATIVE
 		argv_array_push(out, SHELL_PATH);
 #else

  reply	other threads:[~2017-05-16  3:37 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-15 18:08 git rebase regression: cannot pass a shell expression directly to --exec Eric Rannaud
2017-05-16  3:25 ` Jeff King
2017-05-16  3:37   ` Jeff King [this message]
2017-05-16 16:41     ` Jonathan Nieder
2017-05-16 16:47       ` Jeff King
2017-05-16 17:11         ` Eric Rannaud
2017-05-16 17:15         ` Brandon Williams
2017-05-16 17:23           ` Jeff King
2017-05-16 17:30             ` Brandon Williams
2017-05-16 19:14             ` Linus Torvalds
2017-05-16 19:35               ` [TANGENT] run-command: use vfork instead of fork Eric Wong
2017-05-16 20:47                 ` Linus Torvalds
2017-05-16 21:11                   ` Brandon Williams
2017-05-16 20:12               ` git rebase regression: cannot pass a shell expression directly to --exec Johannes Schindelin
2017-05-16 20:27                 ` Linus Torvalds
2017-05-16 17:37         ` Jonathan Nieder
2017-05-16  3:40   ` Junio C Hamano
2017-05-16  3:53     ` Jeff King
2017-05-16  4:08       ` Jeff King
2017-05-16 16:45       ` Eric Rannaud
2017-05-16 10:46     ` Johannes Schindelin
2017-05-16 10:23 ` Johannes Schindelin
2017-05-16 16:18   ` Jeff King
2017-05-16 16:59     ` Eric Rannaud
2017-05-16 17:14       ` Kevin Daudt
2017-05-16 17:29         ` Eric Rannaud
2017-05-16 17:41           ` Jeff King
2017-05-16 17:21       ` Eric Rannaud
2017-05-16 17:37         ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170516033736.23cfkouus3p67vhc@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=eric.rannaud@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jeremy.serror@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).