Re: [PATCH v3 8/8] fetch-pack: implement ref-in-want

[Brandon Williams <bmwill@google.com>]

On 06/22, Jonathan Nieder wrote:
> Hi,
> 
> Brandon Williams wrote:
> 
> > Implement ref-in-want on the client side so that when a server supports
> > the "ref-in-want" feature, a client will send "want-ref" lines for each
> > reference the client wants to fetch.
> >
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  fetch-pack.c                       | 35 +++++++++++++++++++++++++++---
> >  remote.c                           |  1 +
> >  remote.h                           |  1 +
> >  t/t5703-upload-pack-ref-in-want.sh |  4 ++--
> >  4 files changed, 36 insertions(+), 5 deletions(-)
> 
> This commit message doesn't tell me what ref-in-want is or is for.  Could
> it include
> 
>  A. a pointer to Documentation/technical/protocol-v2.txt, or
>  B. an example illustrating the effect e.g. using GIT_TRACE_PACKET
> 
> or both?

Yeah I can imporve the message here.

> > +
> > +		for (r = refs; r; r = r->next) {
> > +			if (!strcmp(end, r->name)) {
> > +				oidcpy(&r->old_oid, &oid);
> > +				break;
> > +			}
> 
> Stefan mentioned that the spec says
> 
>         * The server MUST NOT send any refs which were not requested
>           using 'want-ref' lines.
> 
> Can client enforce that?  If not, can the spec say SHOULD NOT for the
> server and add a MUST describing appropriate client behavior?

Yeah I can update the docs in an earlier patch.

> 
> > +		}
> > +	}
> > +
> > +	if (reader->status != PACKET_READ_DELIM)
> 
> The spec says
> 
>         * This section is only included if the client has requested a
>           ref using a 'want-ref' line and if a packfile section is also
>           included in the response.
> 
> What should happen if the client already has all the relevant objects
> (or in other words if there is no packfile to send in the packfile
> section)?  Is the idea that the client should already have known that
> based on the ref advertisement?  What if ref values change to put us
> in that state between the ls-refs and fetch steps?

I believe the current functionality is that if all wants are already
satisfied by all haves then an empty packfile is sent, so that would
fall under that case.


-- 
Brandon Williams