On Mon, Apr 1, 2013 at 8:30 PM, Jeff King <p...@peff.net> wrote:
> On Mon, Apr 01, 2013 at 06:12:45PM -0600, Felipe Contreras wrote:
>> > Checking asynchronously for death like this is subject to a rac
>> > condition; the helper may be about to die but not have died yet. In
>> > practice we may catch some cases, but this seems like an indication that
>> > the protocol is not well thought-out. Usually we would wait for a
>> > confirmation over the read pipe from a child, and know that the child
>> > failed when either:
>> > 1. It tells us so on the pipe.
>> > 2. The pipe closes (at which point we know it is time to reap the
>> > child).
>> > Why doesn't that scheme work here? I am not doubting you that it does
>> > not; the import helper protocol is a bit of a mess, and I can easily
>> > believe it has such a problem. But I'm wondering if it's possible to
>> > improve it in a more robust way.
>> The pipe is between fast-export and the remote-helper, "we"
>> (transport-helper) are not part of the pipe any more. That's the
> So in fetch_with_import, we have a remote-helper, and we have a
> bidirectional pipe to it. We then call get_importer, which starts
> fast-import, whose stdin is connected to the stdout of the remote
> helper. We tell the remote-helper to run the import, then we wait for
> fast-import to finish (and complain if it fails).
> Then what? We seem to do some more work, which I think is what causes
> the errors you see; but should we instead be reaping the helper at this
> point unconditionally? Its stdout has presumably been flushed out to
> fast-import; is there anything else for us to get from it besides its
> exit code?
The problem is not with import, since fast-import would generally wait
properly for a 'done' status, the problem is with export. Also, the
design is such that the remote-helper stays alive, even after
fast-export has finished.
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