Re: [RFC 4/4] Add cat-blob report pipe from fast-import to remote-helper.
- From
Jeff King <peff@peff.net>
- Date
- Jun 5, 2012, 08:14 UTC
- Message-ID
- <20120605081402.GF25809@sigill.intra.peff.net>
- In-Reply-To
- <CAFfmPPP1koMnYBFbgHt0MGr77okjL5OdAh-TMxFTevj+mDbOZQ@mail.gmail.com>
On Tue, Jun 05, 2012 at 05:07:53PM +1000, David Michael Barr wrote:
Show 16 quoted lines
> On Tue, Jun 5, 2012 at 4:56 PM, Jeff King <peff@peff.net> wrote:
> > On Mon, Jun 04, 2012 at 07:20:55PM +0200, Florian Achleitner wrote:
> >> @@ -427,6 +469,11 @@ static int fetch_with_import(struct transport *transport,
> >> if (get_importer(transport, &fastimport))
> >> die("Couldn't run fast-import");
> >>
> >> +
> >> + /* in the parent process we close both pipe ends. */
> >> + close(data->fast_import_backchannel_pipe[0]);
> >> + close(data->fast_import_backchannel_pipe[1]);
> >
> > I'm confused. We close both ends? Who is actually reading and writing to
> > this pipe, then?
>
> One child, git-fast-import writes to one end.
> The other child, git-remote-* reads from the other end.Ah, thanks. I missed where the write end was going, but now I see it. Overall, the point of the patch makes sense to me (it would have been nice if the commit message described the rationale a bit more completely).
Is there a reason that the patch unconditionally creates the pipe in get_helper? I.e., isn't it specific to the get_importer code path? It feels a little hacky to have it infect the other code paths.
-Peff