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

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I've just read through the commit messages and documentation changes and 
> I think it is a mistake to use a single config variable to configure 
> where fetched refs are stored and which refs are fetched. The problem 
> this series tries to solve is that the default refspec fetches all the 
> branches from the remote and it's not possible to use that mapping and 
> also restrict which branches are fetched. To fix that it introduces a 
> new config setting to configure where the refs are stored which is good, 
> but it then uses the presence of that setting to implicitly configure 
> the default set of refs to fetch. To me that repeats the mistake of the 
> existing fetch config.
>
> It would be much more flexible if we had a separate config setting 
> (maybe remote.<name>.fetchRefs) to describe which refs are fetched.

Good point.

When I wrote the refmap feature (without configuration variable)
long time ago, one of the goals was to allow me to say

    $ git fetch cabal \
        --refmap="+refs/heads/*:refs/remotes/cabal/*" \
	maint master next

That is, "here is the place I grab from there are stored" and
"maint master next are the things I grab from there" are specified
separately.  With the configuration variable added in this series,

    $ git config set remote.cabal.refmap "+refs/heads/*:refs/remotes/cabal/*"
    $ git fetch cabal master next seen

should become possible with the same behaviour.  I am not sure
offhand if this series does that, but I would consider it a bug if
it does not.

In any case, this series, since it lacks the separate setting, takes
the lack of "what to fetch" on the command line (and perhaps lack of
remote.cabal.fetch???) as a sign to trigger this new feature to
guess what to fetch based on what is tracked by the local branches.

For now, as this "what is tracked by the local branches" is the only
kind of smart that was designed to decide what to fetch, it may be OK
to use "you gave me nothing to specify what to fetch, so I'll guess
using the smart".  But when we introduce more kinds of smart later,
this becomes a problem.  So I do agree with you that we should have
something to specifically say "when I do not explicitly give you
what to fetch, decide based on what local branches track".

> That 
> setting could accept something like "*@{upstream}" to "mean fetch the 
> upstream branches of my local branches that live on this remote" which 
> would cover what this series wants. I think it would be useful to 
> support "+" and "^" prefixes on the refs to fetch as well, so that the 
> refmap config controls the mapping and the "what to fetch" config 
> handles negation and forced updates on a per-ref basis.

The reasoning here is slightly wrong.  "^" (not) may be good thing
to consider as it is to decide what to fetch, but "+" (overwrite)
should be on the refmap side, i.e., where to store.  If you are not
storing, then "overwrite what I have with non-fast-forward" becomes
a nonsense request, and that is why it does not belong to "what to
fetch".