Re: [PATCH/RFC] remote-helper: allow fetch to discover sha1 later

On Thu, Sep 13, 2012 at 11:10:26PM -0700, Junio C Hamano wrote:

I do not think it is a good idea to allow such a helper to claim that
it supports "fetch" capability, for at least two reasons:

 * Being able to "list" is essential for "fetch" based helpers.
   think, far from "arbitrary".
   ...

Oh, I don't mean to change the semantics of the list command at all.
What I thought seemed arbitrary was limiting the *fetch* command to refs
with pre-known sha1s.  Any list-time optimization in place or possible
for such refs wouldn't be affected.

(In light of this, specifying a new sha1 for a ref which already had one
in the list should probably be an error rather than a warning.  I'll
update this in the next version and make it clear that a "ref" message
must only be issued for refs reported without a sha1.)

 * Existing versions of "git" that can drive remote helpers that
   claim to have "fetch" capability are not prepared to accept an
   unknown "refs" protocol message from the helper, so a helper
   written for your new semantics of "fetch" capability will not
   work with them.
   ...

I see.  I didn't realize that new helper + old Git client is a supported
combination.  Still, hear me out.

1. A new helper must only send a "ref" message when git asks for a ref
   without a known sha1.
2. Asking for a ref without a known sha1 is unsupported, according to
   git-remote-helpers.txt: "Only objects which were reported in the ref
   list with a sha1 may be fetched [with fetch]."
3. Furthermore, asking for a ref without a known sha1 *already* breaks
   in existing versions of git with current handlers:

    $ git fetch testfetch::../git1 foo
    fatal: bad object 0000000000000000000000000000000000000000
    error: testfetch::../git1 did not send all necessary objects

   Compare - existing version of git, with a new handler:

    $ git fetch testref::../git1 foo
    warning: testref unexpectedly said: 'ref 0f31<snip> refs/heads/foo'
    fatal: bad object 0000000000000000000000000000000000000000
    error: testref::../git1 did not send all necessary objects

So the proposed change doesn't break anything that isn't already broken.
:)

That said, if you're still not sold, a new capability is fine.  Though I
think it can be done without it, I'm certainly not opposed to adding
one.