Re: [PATCH v6 0/4] fetch: avoid fetching every branch of a new remote in a shallow repo

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

Thanks. I think this is becoming much better, but I see one glitch and one design question, for which I do not yet know the right answer.

Before going there, since one of the test scripts added by this series is called 'fetch refmap', we should have a test or two to check its more basic use.

When the user configures remote.origin.refmap, the command should behave as if --refmap were given on the command line, even when the repository does not yet have a local branch that builds on anything from the remote. Attached is my attempt to do so. It does multiple things in a single block, which we may want to split up, but I am sending it here to illustrate what we might want to test and, more importantly, to present a scenario that exposes both the design question and the glitch.

The early part of the scenario goes like this:

 * We create a new repository and add ".." as a remote.
 * We remove remote.origin.fetch and set up remote.origin.refmap.
 * When we run "git fetch origin", nothing is fetched because
   nothing yet builds on what we would fetch from them.

If you try to run this with [1/4] alone, however, it errors out with "fatal: --refmap option is only meaningful with command-line refspec", which is suboptimal when triggered by a configuration variable. Even though our design says that remote.*.refmap makes the command behave as if the user gave '--refmap' on the command line, applying that rule here is a bit too strict.

Fortunately, this is rectified later in the series when we begin tracking which of our local branches build on what we get from them. Even when the number of branches to fetch is zero, meaning we should pretend no command-line refspec was given with --refmap, we no longer get the same error, which is good.

The second part of the scenario explicitly specifies what to fetch on the command line and verifies that we fetch exactly that.

Then there is the last part, where the desired behavior is unclear. What should happen if the remote.origin.* configuration defines both fetch and refmap? How would we explain our choice to the users? I do not have a good answer to this design question.

As for the glitch, the last part of the test below dies with the "fatal: --refmap option is only meaningful..." message when run with the current patchset. We might decide to error out if both are set. Alternatively, we could ignore .refmap and use .fetch, or ignore .fetch and use .refmap. Whatever we decide, the "fatal: --refmap option is only meaningful..." error is not the right message to show in this situation.

Thoughts?

 t/t5586-fetch-refmap.sh | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)