From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,RP_MATCHES_RCVD shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 8188A201A7 for ; Tue, 16 May 2017 03:37:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750765AbdEPDhj (ORCPT ); Mon, 15 May 2017 23:37:39 -0400 Received: from cloud.peff.net ([104.130.231.41]:52372 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750711AbdEPDhj (ORCPT ); Mon, 15 May 2017 23:37:39 -0400 Received: (qmail 28084 invoked by uid 109); 16 May 2017 03:37:39 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.84) with SMTP; Tue, 16 May 2017 03:37:39 +0000 Received: (qmail 24073 invoked by uid 111); 16 May 2017 03:38:11 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.84) with SMTP; Mon, 15 May 2017 23:38:11 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Mon, 15 May 2017 23:37:36 -0400 Date: Mon, 15 May 2017 23:37:36 -0400 From: Jeff King To: Eric Rannaud Cc: git@vger.kernel.org, Johannes Schindelin , Jeremy Serror Subject: Re: git rebase regression: cannot pass a shell expression directly to --exec Message-ID: <20170516033736.23cfkouus3p67vhc@sigill.intra.peff.net> References: <20170516032503.bzkxmtqpmppxgi75@sigill.intra.peff.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170516032503.bzkxmtqpmppxgi75@sigill.intra.peff.net> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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