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

From: Jonathan Tan <jonathantanmy@google.com>
Date: Sep 27, 2018, 19:41 UTC
Message-ID: <20180927194125.8380-1-jonathantanmy@google.com>
In-Reply-To: <87in4tu6wb.fsf@evledraar.gmail.com>

Show 22 quoted lines

> > If you wanted to do this, it seems better to me to just declare a "null"
> > negotiation algorithm that does not perform any negotiation at all.
> 
> I think such an algorithm is a good idea in general, especially for
> testing, and yeah, maybe that's the best way out of this, i.e. to do:
> 
>     if git rev-parse {}/HEAD 2>/dev/null
>     then
>         git fetch --negotiation-tip={}/HEAD {}
>     else
>         git -c fetch.negotiationAlgorithm=null fetch {}
>     fi
> 
> Would such an algorithm be added by overriding default.c's add_tip
> function to never add anything by calling default_negotiator_init()
> followed by null_negotiator_init(), which would only override add_tip?
> (yay C OO)
> 
> If so from fetch-pack.c it looks like there may be the limitation on the
> interface that the negotiator can't exit early (in
> fetch-pack.c:mark_tips). But I've just skimmed this, so maybe I've
> missed something.

(I was reminded to reply to this offlist - sorry for the late reply.)

I think too many things need to be replaced (known_common, add_tip, and ack all need to do nothing), so it's best to start from scratch. That way, we also don't need to deal with the subtleties of C OO :-)

> Also, looks like because of the current interface =null and
> --negotiation-tip=* would (somewhat confusingly) do a "real" negotiation
> if done that way, since it'll bypass the API and insert tips for it to
> negotiate, but it looks like overriding next() will get around that.

If you do it as I suggest (in particular, add_tip doing nothing) then there is the opposite problem that it won't be easy to inform the user that --negotiation-tip does nothing in this case. Maybe there needs to be an "accepts_tips" field in struct fetch_negotiator that, if false, means that custom tips (or any tips) are not accepted, allowing the caller of the negotiator to print a warning message in this case.