The fetch command should "always" honor remote.<name>.fetch

[Lewis Diamond <git@lewisdiamond.com>]

Hi all,
//for tldr; see EOM
I recently setup git for my team and we ran into a small annoyance. 
Though this may be solved with a simple alias, I decided to spend this 
Saturday afternoon exploring the Git source to find out how easy an 
actual patch could be. Unfortunately, the fix isn't trivial and I'm not 
familiar with the Git code base. I'd definitely prefer fixing issue 
properly in the source since the behaviour is, in my opinion, 
inconsistent, so I'll need some insights from experienced git developers.

Let me first describe the issue. Within a single repository, multiple 
ref name spaces can be defined to hold, for example, user branches. For 
example, the main development branches can be located under 
refs/heads/[master | develop] while users push their personal branches 
to refs/user/[bob | alice]/heads/* in order to save their work or 
collaborate with one another. The problem arises when fetching from 
configured fetch refspecs. The behaviour isn't consistent with the push 
behaviour. Let's look at an example:

==
Alice has a repository with a configured remote named foo:
[remote "foo"]
     url = ...
     fetch = +refs/heads/*:refs/remotes/foo/*
     fetch = +refs/users/bob/heads/*:refs/remotes/foo/bob/
     p
     push = refs/heads/*:refs/users/alice/heads/*

Let's say the following refs exist on the remote:
refs/heads/master
refs/users/bob/heads/master
refs/users/bob/heads/develop

If Alice has a master and feature_a branch, 'git push foo' would result in:
master -> refs/users/alice/heads/master
feature_a -> refs/users/alice/heads/feature/a

'git push foo feature/a' would result in:
feature/a -> refs/users/alice/heads/feature/a

'git fetch foo' would result in:
[new ref] refs/users/bob/heads/master -> foo/bob/master //OK
[new ref] refs/users/bob/heads/develop -> foo/bob/develop //OK
[new ref] refs/heads/master -> foo/master //OK

'git fetch foo refs/users/bob/heads/develop' would result in (FETCH_HEAD 
omitted):
[new ref] refs/users/bob/heads/develop -> foo/bob/develop //OK

'git fetch foo refs/heads/master' would result in (FETCH_HEAD omitted):
[new ref] refs/heads/master -> foo/master //OK

'git fetch foo develop' would result in:
fatal: Couldn't find remote ref test2 //Not OK, (case 1)

'git fetch foo master' would result in (FETCH_HEAD omitted):
[new ref] refs/heads/master -> foo/master //OK, but missing another ref! 
(case 2)
//It should also fetch refs/users/bob/heads/master -> foo/bob/master

If you remove this configuration line: fetch = 
+refs/heads/*:refs/remotes/foo/*
Then you run 'git fetch foo master', this would result in:
  * branch master -> FETCH_HEAD //Debatable whether this is OK or not, 
but it's definitely missing bob's master! (case 3)
==

case 1: The ref abbreviation is received in builtin/fetch.c: get_ref_map 
and passed to remote.c:get_fetch_map with missing_ok = 0. The 
abbreviated ref is then evaluated through remote.c:get_remote_ref --> 
remote.c:find_ref_by_name_abbrev to finally end up being matched in 
refs.c:ref_rev_parse_rules. Since in this case, develop doesn't exist 
under 'develop', 'refs/develop', 'refs/tags/develop', 
'refs/heads/develop', 'refs/remotes/develop' or 
'refs/remotes/develop/HEAD', no ref is matched and the command dies (due 
to missing_ok=0). All of this happens without ever looking at the 
configured fetch refspec.

case 2: Starts like case 1, except that the ref_rev_parse_rules get a 
match and only this match is used. The destination is then mapped in a 
later call to get_fetch_map, from builtin/fetch.c@311

case 3: Same as 2, but it maps to HEAD.

tldr;
In my opinion, these issues can be resolved with the following rules:
1. Explicit refs (starts with refs/) are fetched 'as-is' and put into 
FETCH_HEAD and into every configured refspec->dst for which refspec->src 
matches the input ref. (The push command should be made the same for 
consistency. It currently only pushes to the first matching refspec.)
2. Abbreviated refs (e.g. 'master') are matched only against the 
remote_refs matching remote->fetch_refspec (I think remote.c 
get_expanded_map can provide this). If multiple matches exist, all of 
them are fetched to their corresponding refspec->dst (and FETCH_HEAD). 
If multiple matches exist and have conflicting refspec->dst, error out 
(or should we honour the first matching refspec?).

Note that for the 2nd rule, if remote->fetch_refspec doesn't include 
refs/heads/*, it means I went through the trouble of removing it from 
the configuration. Therefore, I think it shouldn't even be fetched. 
However, we need to make sure to handle 'git fetch <URL> master', such 
that it is still evaluated by ref_rev_parse_rules.

Any input is be appreciated.
Thanks,
Lewis Diamond