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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to