Subject: Re: [PATCH v4 18/35] fetch: pass ref patterns when fetching

From: Brandon Williams <[email-masked]>
Date: Mon, 12 Mar 2018 15:18:55 -0700

On 03/02, Junio C Hamano wrote:
> Brandon Williams <[email-masked]> writes:
> 
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index 850382f55..695fafe06 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -332,11 +332,25 @@ static struct ref *get_ref_map(struct transport *transport,
> >  	struct ref *rm;
> >  	struct ref *ref_map = NULL;
> >  	struct ref **tail = &ref_map;
> > +	struct argv_array ref_patterns = ARGV_ARRAY_INIT;
> >  
> >  	/* opportunistically-updated references: */
> >  	struct ref *orefs = NULL, **oref_tail = &orefs;
> >  
> > -	const struct ref *remote_refs = transport_get_remote_refs(transport, NULL);
> > +	const struct ref *remote_refs;
> > +
> > +	for (i = 0; i < refspec_count; i++) {
> > +		if (!refspecs[i].exact_sha1) {
> > +			if (refspecs[i].pattern)
> > +				argv_array_push(&ref_patterns, refspecs[i].src);
> > +			else
> > +				expand_ref_pattern(&ref_patterns, refspecs[i].src);
> > +		}
> > +	}
> > +
> > +	remote_refs = transport_get_remote_refs(transport, &ref_patterns);
> > +
> > +	argv_array_clear(&ref_patterns);
> 
> Is the idea here, which is shared with 17/35 about ls-remote, that
> we used to grab literally everything they have in remote_refs, but
> we have code in place to filter that set using refspecs given in the
> remote.*.fetch configuration, so it is OK as long as we grab everything
> that would match the remote.*.fetch pattern?  That is, grabbing too
> much is acceptable, but if we populated ref_patterns[] with too few
> patterns and fail to ask refs that would match our refspec it would
> be a bug?

Yes that's the idea.  Right now we're in the state where we ask for
everything (since there is no server side filtering) and the client just
does its own filtering after the fact using the refspec.  So if we end
up not sending enough ref patterns to match what the refspec is, it
would be a bug.

> 
> The reason behind this question is that I am wondering if/how we can
> take advantage of this remote-side pre-filtering while doing "fetch
> --prune".

Hmm maybe, assuming prune then means "get rid of all remote-tracking
branches that don't match the user provided refspec"

> 
> Thanks.
> 
> >  
> >  	if (refspec_count) {
> >  		struct refspec *fetch_refspec;

-- 
Brandon Williams