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

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:45

"Devin J. Pohly" [off-list ref] writes:
From: "Devin J. Pohly" <redacted>

Allow a fetch-style remote helper to issue the notification
  ref <sha1> <name>
to specify a ref's value during the fetch operation.

Currently, a remote helper with the "fetch" capability cannot fetch a
ref unless its sha1 was known when the "list" command was executed.
This limitation is arbitrary, as the helper may eventually be able to
determine the correct sha1 as it fetches objects.
While I am not fundamentally against supporting a remote helper that
is not capable of saying what the object names of the tip of its
histories are before doing a lot of work, 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, I
   think, far from "arbitrary".  When running a "git fetch" against
   such a remote, we can first issue "list" to see what it has and
   avoid asking for the refs that point at the histories we already
   have (or the refs that are the same as the last time we fetched
   from the remote).  I do not offhand know if we do this kind of
   optimization, but if we don't, we should.

 * 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.  The point of capability mechanism is to prevent
   such a compatibility issue from breaking the system, and this
   patch goes against that spirit.

Such a remote helper should not advertise to support "fetch"
capability, because it does not support it.  It can advertise that
it supports "something else" and it is OK to make an updated Git
that knows how to drive a remote helper that lack "fetch" but
support that "something else" work with it.

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

From: Devin J. Pohly <hidden>
Date: 2016-06-15 22:54:45

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help