Re: [PATCH 0/8] ref-in-want

From: Jonathan Tan <jonathantanmy@google.com>
Date: Jun 15, 2018, 19:04 UTC
Message-ID: <20180615190458.147775-1-jonathantanmy@google.com>
In-Reply-To: <20180605175144.4225-1-bmwill@google.com>

(replying to the original since my e-mail is about design)

> This version of ref-in-want is a bit more restrictive than what Jonathan
> originally proposed (only full ref names are allowed instead of globs
> and OIDs), but it is meant to accomplish the same goal (solve the issues
> of refs changing during negotiation).

One question remains: are we planning to expand this feature (e.g. to support patterns ending in *, or to support any pattern that can appear on the LHS of a refspec), and if yes, are we OK with having 2 or more versions of the service in the wild, each having different pattern support?

Supporting patterns would mean that we would possibly be able to eliminate the ls-refs step, thus saving at least a RTT. (Originally I thought that supporting patterns would also allow us to tolerate refs being removed during the fetch process, but I see that this is already handled by the server ignoring "want-ref <ref>" wherein <ref> doesn't exist on the server.)

However, after some in-office discussion, I see that eliminating the ls-refs step means that we lose some optimizations that can only be done when we see that we already have a sought remote ref. For example, in a repo like this:

 A
 |
 O
 |
 O B C
 |/ /
 O O
 |/
 O

in which we have rarely-updated branches that we still want to fetch (e.g. an annotated tag when we fetch refs/tags/* or a Gerrit refs/changes/* branch), having the ref advertisement first means that we can omit them from our "want" or "want-ref" list. But not having them means that we send "want-ref refs/tags/*" to the server, and during negotiation inform the server of our master branch (A), and since the server knows of a common ancestor of all our wants (A, B, C), it will terminate the negotiation and send the objects specific to branches B and C even though it didn't need to.

So maybe we still need to keep the ls-refs step around, and thus, this design of only accepting exact refs is perhaps good enough for now.