Re: [PATCH v2 2/8] upload-pack: implement ref-in-want

[Brandon Williams <bmwill@google.com>]

On 06/15, Junio C Hamano wrote:
> The story would be different if your request were 
> 
> 	git fetch refs/heads/*:refs/remotes/origin/*
> 
> in which case, you are not even saying "I want this and that ref";
> you are saying "all refs in refs/heads/* whoever ends up serving me
> happens to have".  You may initially contact one of my friends and
> learn that there are 'master' and 'bo' branches (and probably
> others), and after conversation end up talking with me who is stale
> and lack 'bo'.  In such a case, I agree that it is not sensible for
> me to fail the request as a whole and instead serve you whatever
> branches I happen to have.  I may lack 'bo' branch due to mirroring
> lag, but I may also have 'extra' branch that others no longer have
> due to mirroring lag of deletion of that branch!
> 
> But then I think your "git fetch refs/heads/*:refs/remotes/origin/*"
> should not fail not just because I do not have 'bo', but you also
> should grab other old branches I have, which you didn't hear about
> when you made the initial contact with my friend in the mirror pool.
> 
> So, given that, would it make sense for 'want-ref <ref>' request to
> name "a particular ref" as the above document says?  I have a
> feeling that it should allow a pattern to be matched at the server
> side (and it is not an error if the pattern did not match anything),
> in addition to asking for a particular ref (in which case, lack of
> that ref should be a hard failure, at least for the mirror that ends
> up serving the packfile and the final "here are the refs your
> request ended up fetching, with their values").

After some more in-office discussion about this I think I should revert
back to making it a hard failure when a client requests a ref which
doesn't exist on the server.  This makes things more consistent with
what happens today if I request a non-existent ref (Although that error
is purely on the client).  Its no worse than we have today and even with
this solution not solving the issues of new/deleted refs (which are rare
operations wrt updates) we still can get the benefit of not failing due
to a ref updating.  This is also very valuable for the servers which
have to to ACL checks on individual ref names.

I also think that we should keep this first implementation of
ref-in-want simple and *not* include patterns, even if that's what we
may want someday down the road.  Adding a new capability in the future
for support of such patterns would be relatively simple and easy.  The
reason why I don't think we should add pattern support just yet is due
to a request for "want-ref refs/tags/*" or a like request resulting in a
larger than expected packfile every time "fetch --tags" is run.  The
issue being that in a fetch request "refs/tags/*" is too broad of a
request and could be requesting 100s of tags when all we really wanted
was to get the one or two new tags which are present on the remote
(because we already have all the other tags present locally).

So I think the best way is to limit these patterns to the ls-refs
request where we can then discover the few tags we're missing and then
request just those tags explicitly, keeping the resulting packfile
smaller.

Thoughts?

-- 
Brandon Williams