Subject: Re: [PATCH v2] builtin/fetch: skip unnecessary tasks when using --negotiate-only

From: Glen Choo <[email-masked]>
Date: Tue, 21 Dec 2021 15:07:48 -0800

Glen Choo <[email-masked]> writes:

>>>  	prepare_repo_settings(the_repository);
>>
>> This is existing code, but I wonder why it can be done _SO_ late in
>> the sequence.  We've already called the transport API for the
>> negotiate-only communication at this point, but a call to this
>> function is the only thing that gives fetch_negotiation_algorithm
>> member in the_repository its default value, isn't it?
>
> That's right, this looks like it could be a bug. Maybe Jonathan knows
> more.

It seems that fetch negotiation always calls prepare_repo_settings().
Fetch negotiation uses negotiate_using_fetch(), which calls
fetch_negotiator_init(), which calls prepare_repo_settings():

  void fetch_negotiator_init(struct repository *r,
          struct fetch_negotiator *negotiator)
  {
    prepare_repo_settings(r);
    switch(r->settings.fetch_negotiation_algorithm) {
    case FETCH_NEGOTIATION_SKIPPING:
      skipping_negotiator_init(negotiator);
      return;

    case FETCH_NEGOTIATION_NOOP:
      noop_negotiator_init(negotiator);
      return;

    case FETCH_NEGOTIATION_DEFAULT:
      default_negotiator_init(negotiator);
      return;
    }
  }

If anything, this seems safer than calling prepare_repo_settings() in
cmd_fetch() :)