git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "dmh@ucar.edu" <dmh@ucar.edu>
Cc: git@vger.kernel.org
Subject: [PATCH 0/5] consistent setup code for external commands
Date: Fri, 1 Jul 2016 01:55:32 -0400	[thread overview]
Message-ID: <20160701055532.GA4488@sigill.intra.peff.net> (raw)
In-Reply-To: <20160701040715.GB4832@sigill.intra.peff.net>

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

> 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

  reply	other threads:[~2016-07-01  5:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-30 21:24 git-credentials-store.exe crash dmh
2016-07-01  4:07 ` Jeff King
2016-07-01  5:55   ` Jeff King [this message]
2016-07-01  5:58     ` [PATCH 1/5] add an extra level of indirection to main() Jeff King
2016-07-01  8:04       ` Johannes Schindelin
2016-07-01  8:19         ` Jeff King
2016-07-01 13:39           ` Johannes Schindelin
2016-07-01 22:38             ` Jeff King
2016-07-02  6:52               ` Johannes Schindelin
2016-07-01  6:04     ` [PATCH 2/5] common-main: call git_extract_argv0_path() Jeff King
2016-07-01  8:05       ` Johannes Schindelin
2016-07-01  6:06     ` [PATCH 3/5] common-main: call sanitize_stdfds() Jeff King
2016-07-01  6:06     ` [PATCH 4/5] common-main: call restore_sigpipe_to_default() Jeff King
2016-07-01  6:07     ` [PATCH 5/5] common-main: call git_setup_gettext() Jeff King
2016-07-01  7:45     ` [PATCH 0/5] consistent setup code for external commands Johannes Schindelin
2016-07-01 22:19     ` Junio C Hamano
2016-07-01 22:39       ` Jeff King
2016-07-06 15:17         ` Junio C Hamano
2016-07-06 15:36           ` Johannes Schindelin
2016-07-01  7:38   ` git-credentials-store.exe crash Johannes Schindelin
2016-07-01  7:43     ` 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=20160701055532.GA4488@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dmh@ucar.edu \
    --cc=git@vger.kernel.org \
    /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).