git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: git clone silently aborts if stdout gets a broken pipe

From
Jeff King <peff@peff.net>
Date
Sep 18, 2013, 20:01 UTC
Message-ID
<20130918200152.GA17074@sigill.intra.peff.net>
In-Reply-To
<xmqqmwnaudtg.fsf@gitster.dls.corp.google.com>
On Wed, Sep 18, 2013 at 12:31:23PM -0700, Junio C Hamano wrote:
> > Hrm, this actually breaks t5701, which expects "clone 2>err" to print
> > nothing to stderr.
> 
> Hmm, where in t5701?  Ah, you meant t5702 and possibly t5601.
Yes, sorry, I meant t5702.
Show 7 quoted lines
> I actually think "it is long and not meant to be seen sequentially"
> is a bad classifier; these new messages are also progress report in
> that it reports "we are now in this phase".  So if I were to vote, I
> would say we should apply the same progress-silencing criteria,
> preferrably by not checking isatty() again, but by recording the
> decision we have already made when squelching the progress during
> the transfer in order to make sure they stay consistent.

Unfortunately that decision is made in the transport code, not by clone itself. We can cheat and peek at "transport->progress" after initializing the transport. That would require some refactoring, though; we print "Cloning into" before setting up the transport. And we do not even tell the transport about our progress options if we are doing a local clone.

If we wanted to _just_ suppress "Checking connectivity" (and not "Cloning into..."), that's a bit easier. And I could see an argument that the former is the only one that falls into the "progress report" category.

Show 7 quoted lines
> > Also, we should arguably give the "Cloning into..." message the same
> > treatment. We have printed that to stdout for a very long time, so there
> > is a slim chance that somebody actually tries to parse it. But I think
> > they are wrong to do so; we already changed it once (in 28ba96a), and
> > these days it is internationalized, anyway.
> 
> Good thinking.  Please make it so ;-)

OK. I've squashed the "use stderr" patches into one, and added a patch on top to correctly check the progress flag.

  [1/2]: clone: send diagnostic messages to stderr
  [2/2]: clone: treat "checking connectivity" like other progress
-Peff
Previous: Junio C HamanoNext: Jeff King
Message 5 of 11 in “git clone silently aborts if stdout gets a broken pipe”
  1. Peter KjellerstedtSep 18, 2013
  2. Jeff KingSep 18, 2013
  3. Jeff KingSep 18, 2013
  4. Junio C HamanoSep 18, 2013
  5. Jeff KingSep 18, 2013
  6. 1/2 clone: send diagnostic messages to stderrJeff King, Sep 18, 2013
  7. 2/2 clone: treat "checking connectivity" like other progressJeff King, Sep 18, 2013
  8. 3/2 clone: always set transport optionsJeff King, Sep 18, 2013
  9. Peter KjellerstedtSep 19, 2013
  10. Jeff KingSep 19, 2013
  11. Peter KjellerstedtSep 19, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.