[PATCH 0/5] consistent setup code for external commands

From: Jeff King <peff@peff.net>
Date: Jul 1, 2016, 05:55 UTC
Message-ID: <20160701055532.GA4488@sigill.intra.peff.net>
In-Reply-To: <20160701040715.GB4832@sigill.intra.peff.net>

On Fri, Jul 01, 2016 at 12:07:15AM -0400, Jeff King wrote:

Show 13 quoted lines

> Interesting. It's failing on the assert(argv0_path) in system_path().
> 
> That's part of the RUNTIME_PREFIX code which is built only on Windows,
> so this is a Windows-specific issue.
> 
> I can guess the reason that argv0_path is not set is that
> credential-store has its own main() function and does not call
> git_extract_argv0_path(). It never mattered before, but as of v2.8.0,
> one of the library functions it calls wants to use system_path(), which
> assumes that the argv0 stuff has been set up.
> 
> I'm preparing some patches to fix this case (and some other related
> ones).

Here they are:

  [1/5]: add an extra level of indirection to main()
  [2/5]: common-main: call git_extract_argv0_path()
  [3/5]: common-main: call sanitize_stdfds()
  [4/5]: common-main: call restore_sigpipe_to_default()
  [5/5]: common-main: call git_setup_gettext()

The first patch is refactoring so we can fix this problem once, rather than in the dozens of programs that need it.

The second should fix the problem you saw. It's unfortunate that the tests didn't detect it; there's some discussion of that in the commit message.

Patches 3-5 are other places where we can use the refactoring to be more consistent and remove boilerplate code.

I almost did a patch 6 moving trace_command_performance(), but I'm not sure if people would care or not. Running "git foo" already covers that, even for an external command, because the git wrapper waits until the sub-process finishes. Setting it up in each sub-program would mean:

  - you get it for dashed externals you run directly, too

  - for external programs run as "git foo", you get _two_ times. One for
    the time the actual sub-program ran, and one with the overhead of
    the git wrapper process.

    I'm not sure if people actually care about that distinction, or
    whether the extra number would simply be annoying.

So I punted. It's easy to move it over later if anybody cares.

-Peff