git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: git clone silently aborts if stdout gets a broken pipe
Date: Thu, 19 Sep 2013 04:35:30 -0400	[thread overview]
Message-ID: <20130919083530.GA12597@sigill.intra.peff.net> (raw)
In-Reply-To: <A612847CFE53224C91B23E3A5B48BAC798CD91DBA7@xmail3.se.axis.com>

On Thu, Sep 19, 2013 at 09:54:38AM +0200, Peter Kjellerstedt wrote:

> > I think your perl script is somewhat questionable, as it is making
> > assumptions about the output of git-clone, and you would do better to
> > accept arbitrary-sized output 
> 
> Well, the whole idea of using Git::command_oneline() is that we 
> are only interested in the first line of output, similar to using 
> "| head -1". If we had wanted all of the output we would have used 
> Git::command() instead. Since the Git Perl module is released as a 
> part of Git, I would expect it to work as documented regardless of 
> which Git command is used with Git::command_oneline().

I think command_oneline is exactly like "| head -1" in this case. Doing
"git clone | head -1" would also fail, and should not be used. In
general, you do not want to put a limiting pipe on a command with side
effects beyond output. The design of unix pipes and SIGPIPE is such that
you can do "generate_output | head", and "generate_output" will get
SIGPIPE and die after realizing that its writer no longer cares about
the output. But if your command is doing something besides output, that
assumption doesn't hold.

Arguably, "git clone" should be taking the initiative to ignore SIGPIPE
itself.  Its primary function is not output, but doing the clone. If
output fails, we would want to continue the clone, not die.

By the way, did you actually want to capture the stdout of git-clone, or
were you just trying to suppress it? Because the eventual patch I posted
sends it to stderr, under the assumption that what used to go to stdout
should not be captured and parsed (because it is localized and subject
to change).

> However, what surprised me most was that git clone failed silently 
> when it got a broken pipe.

It's not "git clone" that is doing this, I think, but rather the design
of command_oneline. If I do:

  (sleep 1; git clone ...; echo >&2 exit=$?) | false

then I see:

  exit=141

That is, clone dies from SIGPIPE trying to write "Cloning into...". But
command_oneline is specifically designed to ignore SIGPIPE death,
because you would want something like:

  command_oneline("git", "rev-list", "$A..$B");

to give you the first line, and then you do not care if the rest of the
rev-list dies due to SIGPIPE (it is a good thing, because by closing the
pipe you are telling it that its output is not needed). It may be that
the documentation for command_oneline can be improved to mention this
subtlety.

-Peff

  reply	other threads:[~2013-09-19  8:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-18 16:52 git clone silently aborts if stdout gets a broken pipe Peter Kjellerstedt
2013-09-18 18:45 ` Jeff King
2013-09-18 19:04   ` Jeff King
2013-09-18 19:31     ` Junio C Hamano
2013-09-18 20:01       ` Jeff King
2013-09-18 20:05         ` [PATCH 1/2] clone: send diagnostic messages to stderr Jeff King
2013-09-18 20:06         ` [PATCH 2/2] clone: treat "checking connectivity" like other progress Jeff King
2013-09-18 20:35           ` [PATCH 3/2] clone: always set transport options Jeff King
2013-09-19  7:54   ` git clone silently aborts if stdout gets a broken pipe Peter Kjellerstedt
2013-09-19  8:35     ` Jeff King [this message]
2013-09-19 15:48       ` Peter Kjellerstedt

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

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

  git send-email \
    --in-reply-to=20130919083530.GA12597@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peter.kjellerstedt@axis.com \
    /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.
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 NNTP newsgroup(s).