done feature in remote-helpers (was Re: [PATCH 2/2] Add a remote helper to interact with mediawiki (fetch & push))

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Aug 31, 2011, 12:05 UTC
Message-ID: <vpq1uw13hx3.fsf_-_@bauges.imag.fr>
In-Reply-To: <CAGdFq_jwLGZ+tLKramRrLJmwyY_uDtj7JXUwYBO9pSqOmZ20xQ@mail.gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> writes:

Show 11 quoted lines

> Heya,
>
> On Mon, Aug 29, 2011 at 08:05, Junio C Hamano <gitster@pobox.com> wrote:
>> Does this exchange suggest that at least we would need an update to
>> documentation around "done", as Matthieu's "why is 'done' needed even
>> though I am not calling with --done?" sounds like a very fair question.
>
> No I think the documentation for fast-import is correct. If you pass
> --use-done-feature or print 'feature done' in the stream the use of
> 'done' is required, otherwise it isn't. We did recently changed git to
> pass '--use-done-feature' to the fast-import process though :).

That doesn't help much someone writting a remote helper.

The documentation for remote-helpers neither talks about "done" nor about "--use-done-feature" or whatever way Git uses this feature when using remote-helpers.

The current state is particularly confusing: git seems to expect one and only one "done" feature, even when multiple "import" commands are issued. That's very strange, and I'm not sure whether it's the expected behavior (I can try a documentation patch, but I need to understand better what's expected and what's not).

It would be natural to write remote-helpers like

while ($cmd = <read command>) {
    if ($cmd eq "import") {
       <write fast-import stream>
       print "done\n";
    } ...
}

but in the current state, it doesn't work since we'll get a first "import HEAD", issue a "done", then get a "import refs/heads/master" and write to a dead pipe.

Then, it would be very tempting to write it like

while ($cmd = <read command>) {
    if ($cmd eq "import") {
       <write fast-import stream>
    } ...
}
print "done\n";

but this doesn't work either, because when calling "git push", no "import" command is involved, no fast-import is started, and the "done" breaks everything.

That's why I had to make it like

my $import_started;
while ($cmd = <read command>) {
    if ($cmd eq "import") {
       $import_started = 1;
       <write fast-import stream>
    } ...
}
if ($import_started) {
    print "done\n";
}

and I really had the feeling I was working around a mis-feature of Git here.

So, is this the expected behavior? Wouldn't it be more sensible to allow the remote-helper to issue a "done" after each "import" command? Right now, my understanding is that after an "import" command is issued, it's no longer possible to output anything other than fast-import stream on stdout, and I guess it'd be more future-proof to allow closing the fast-import with a "done", and allow any dialog between git and the remote helper afterwards.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/