Re: [PATCH v3] fetch-pack: support negotiation tip whitelist

[Brandon Williams <bmwill@google.com>]

On 06/28, Jonathan Tan wrote:
> During negotiation, fetch-pack eventually reports as "have" lines all
> commits reachable from all refs. Allow the user to restrict the commits
> sent in this way by providing a whitelist of tips; only the tips
> themselves and their ancestors will be sent.
> 
> Both globs and single objects are supported.
> 
> This feature is only supported for protocols that support connect or
> stateless-connect (such as HTTP with protocol v2).
> 
> This will speed up negotiation when the repository has multiple
> relatively independent branches (for example, when a repository
> interacts with multiple repositories, such as with linux-next [1] and
> torvalds/linux [2]), and the user knows which local branch is likely to
> have commits in common with the upstream branch they are fetching.
> 
> [1] https://kernel.googlesource.com/pub/scm/linux/kernel/git/next/linux-next/
> [2] https://kernel.googlesource.com/pub/scm/linux/kernel/git/torvalds/linux/
> 
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> This is on jt/fetch-pack-negotiator.
> 
> > I don't think that would be strange at all, and no where in git do we
> > handle heads/* but we do already handle refs/heads/* as well as DWIM
> > master.
> >
> > > and (2) I can't think of anywhere in Git
> > > where you can provide either one - it's either SHA-1 and DWIM name, or
> > > SHA-1 and refspec, but not all three.
> >
> > fetch is a perfect example of supporting all three.  I can do
> >
> >   git fetch origin SHA1
> >   git fetch origin master
> >   git fetch origin refs/heads/*:refs/heads/*
> 
> OK, Brandon managed to convince me that this is fine. I've included glob
> support, supporting the same globs that git notes supports.
> ---
>  Documentation/fetch-options.txt | 16 +++++++
>  builtin/fetch.c                 | 41 +++++++++++++++++
>  fetch-pack.c                    | 19 +++++++-
>  fetch-pack.h                    |  7 +++
>  t/t5510-fetch.sh                | 78 +++++++++++++++++++++++++++++++++
>  transport-helper.c              |  3 ++
>  transport.c                     |  1 +
>  transport.h                     | 10 +++++
>  8 files changed, 173 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
> index 97d3217df..6e4db1738 100644
> --- a/Documentation/fetch-options.txt
> +++ b/Documentation/fetch-options.txt
> @@ -42,6 +42,22 @@ the current repository has the same history as the source repository.
>  	.git/shallow. This option updates .git/shallow and accept such
>  	refs.
>  
> +--negotiation-tip=<commit|glob>::
> +	By default, Git will report, to the server, commits reachable
> +	from all local refs to find common commits in an attempt to
> +	reduce the size of the to-be-received packfile. If specified,
> +	Git will only report commits reachable from the given tips.
> +	This is useful to speed up fetches when the user knows which
> +	local ref is likely to have commits in common with the
> +	upstream ref being fetched.
> ++
> +This option may be specified more than once; if so, Git will report
> +commits reachable from any of the given commits.
> ++
> +The argument to this option may be a glob on ref names, a ref, or the (possibly
> +abbreviated SHA-1 of a commit. Specifying a glob is equivalent to specifying
> +this option multiple times, one for each matching ref name.

I think you're missing a closing ')'

Aside from that nit this patch looks good, thanks!


-- 
Brandon Williams