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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]