[PATCH 0/2] negotiator: improve recent behavior + docs

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Aug 1, 2018, 15:18 UTC
Message-ID: <20180801151835.9182-1-avarab@gmail.com>
In-Reply-To: <20180731180248.42787-1-jonathantanmy@google.com>

On Tue, Jul 31 2018, Jonathan Tan wrote:

Show 23 quoted lines

>> > +fetch.negotiationAlgorithm::
>> > +  Control how information about the commits in the local repository is
>> > +  sent when negotiating the contents of the packfile to be sent by the
>> > +  server. Set to "skipping" to use an algorithm that skips commits in an
>> > +  effort to converge faster, but may result in a larger-than-necessary
>> > +  packfile; any other value instructs Git to use the default algorithm
>> > +  that never skips commits (unless the server has acknowledged it or one
>> > +  of its descendants).
>> > +
>> 
>> ...let's instead document that there's just the values "skipping" and
>> "default", and say "default" is provided by default, and perhaps change
>> the code to warn about anything that isn't those two.
>> 
>> Then we're not painting ourselves into a corner by needing to break a
>> promise in the docs ("any other value instructs Git to use the default")
>> if we add a new one of these, and aren't silently falling back on the
>> default if we add new-fancy-algo the user's version doesn't know about.
>
> My intention was to allow future versions of Git to introduce more
> algorithms, but have older versions of Git still work even if a
> repository is configured to use a newer algorithm. But your suggestion
> is reasonable too.

I think 01/02 in this patch series implements something that's better & more future-proof.

Show 17 quoted lines

>> Now, running that "git fetch --all" takes ages, and I know why. It's
>> because the in the negotiation for "git fetch some/small-repo" I'm
>> emitting hundreds of thousands of "have" lines for SHA1s found in other
>> unrelated repos, only to get a NAK for all of them.
>> 
>> One way to fix that with this facility would be to have some way to pass
>> in arguments, similar to what we have for merge drivers, so I can say
>> "just emit 'have' lines for stuff found in this branch". The most
>> pathological cases are when I'm fetching a remote that has one commit,
>> and I'm desperately trying to find something in common by asking if the
>> remote has hundreds of K of commits it has no chance of having.
>> 
>> Or there may be some smarter way to do this, what do you think?
>
> Well, there is already a commit in "next" that does this :-)
>
> 3390e42adb ("fetch-pack: support negotiation tip whitelist", 2018-07-03)

That's awesome. This is exactly what I wanted, this patch series also fixes another small issue in 02/02; which is that the docs for the two really should cross-link to make these discoverable from one another.

Another thing I noticed, which I have not fixed, is that there's no way to say "I don't want to you to say that anything is in common". Currently I'm hacking around in my script by doing:

    parallel --eta --verbose --progress '
        if git rev-parse {}/HEAD 2>/dev/null
        then
            git fetch --negotiation-tip={}/HEAD {}
        else
            git fetch --negotiation-tip=ref-i-have-with-one-commit {}
        fi
    ' ::: $(git remote)

I.e. the way I'm doing this is I add all the remotes first, then I fetch them all in parallel, but because the first time around I don't have anything for that remote (and they don't share any commits) I need to fake it up and pretend to be fetching from a repo that has just one commit.

It would be better if I could somehow say that I don't mind that the ref doesn't exist, but currently you either error out with this, or ignore the glob, depending on the mode.

So I want this, but can't think of a less shitty UI than:

    git fetch --negotiation-tip=$REF --negotiation-tip-error-handling=missing-ref-means-no-want

Or something equally atrocious, do you have any better ideas?

Ævar Arnfjörð Bjarmason (2):
  negotiator: unknown fetch.negotiationAlgorithm should error out
  fetch doc: cross-link two new negotiation options

 Documentation/config.txt             |  5 ++++-
 Documentation/fetch-options.txt      |  3 +++
 fetch-negotiator.c                   | 12 +++++++++---
 t/t5552-skipping-fetch-negotiator.sh | 23 +++++++++++++++++++++++
 4 files changed, 39 insertions(+), 4 deletions(-)

-- 
2.18.0.345.g5c9ce644c3