Re: [PATCH] git-remote-testgit: avoid process substitution

[Junio C Hamano <gitster@pobox.com>]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, Apr 25, 2013 at 1:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>>>> ...
>>>>> +             git for-each-ref --format='%(refname) %(objectname)' |
>>>>> +             while read ref a
>>>>>               do
>>>>> -                     test $a == $b && continue
>>>>> +                     case "$before" in
>>>>> +                     *"$ref $a"*)
>>>>> +                             continue
>>>>
>>> I wonder if we should bother with this at all. The purpose of the code
>>> was mainly to show to users that they should report the success only
>>> if the refs have been updated, but the code is becoming more
>>> obfuscated, a comment should do the trick. And then, we can just
>>> report success for all the refs (and explain in the comment why).
>>
>> Are you proposing to say "ok $ref" to everything we see in the
>> resulting repository, even the ones the caller of remote-testgit did
>> not ask us to do anything with?
>>
>> Wouldn't the caller be surprised if we did so?
>
> Why would it?  The only effective difference is what you'll see
> reported in the UI, but there's no user here.

You are correct that it affects only the UI, but isn't that because
the current implementation of push_update_refs_status() blindly
accepts whatever 'ok' response says without checking the ref
mentioned against what it tried to push out?  I was wondering if
that codepath should stay that way forever, or if we may want add
sanity checks later.  If the latter, I suspect this would manifest
as an ancient bug to say 'ok' for everything we have instead of what
we actually pushed out (of course, the explanation in the comment
would help).

So I am OK with that simpler approach.  Care to roll the conclusion
of your idea into a patch?

By the way, I noticed that Documentation/gitremote-helpers.txt does
not even mention these 'ok' responses for 'export' command, but they
should be the same as responses to 'push'. Perhaps we can share some
text between the two?