Re: On fetch refspecs and wildcards
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:23
Daniel Barkalow [off-list ref] writes:
On Sun, 16 Mar 2008, Junio C Hamano wrote: ...quoted
Fortunately or unfortunately, Documentation/pull-fetch-param.txt does not talk about wildcard refspecs (not even the syntax, let alone the semantics), so we can define whatever we want right now, and I think both (1) allow duplicated destinations, including wildcard matches; and (2) refuse duplicated destinations for explicit ones, and more than one wildcard patterns that match the same ref, but omit explicitly specified ones from wildcard matches; are viable options. I suspect the current code does not do either. We should pick one semantics, make sure the implementation matches that, and document it.Actually, I think the current code is close to (2). get_fetch_map() returns everything, ref_remove_duplicates() removes any exact matches and gives errors if there's the same destination for two different sources. (Upon further consideration, there's one slight issue: [remote "origin"] fetch = refs/heads/*:refs/remotes/origin/* fetch = +refs/heads/pu:refs/remotes/origin/pu is not quite the same as: [remote "origin"] fetch = +refs/heads/pu:refs/remotes/origin/pu fetch = refs/heads/*:refs/remotes/origin/* in whether pu will be forced; the forcing flag on the first matching refspec is what matters.)
Ok. As I said, I think either one is valid, and I only mentioned (1) because I thought refusing duplicates might be more work to get it right. So if your code does _most of_ (2), that is good. Please document what it is meant to do in Documentation/pull-fetch-param.txt, so that others can report deviation from the defined semantics, if any, in the implementation for us to fix.
quoted
The issues are: (1) get_fetch_map() currently insists on refname to be check_ref_format() clean; it even rejects CHECK_REF_FORMAT_ONELEVEL, which means that refs/stash would not be considered Ok and the code will die().Yes, that's probably wrong. We probably do want to reject people whose servers send us "refs/heads/../../heads/master", but not "refs/stash".
The feeler patch I sent out would be Ok, then. Can you test it, after updating it with the die() -> error() and message rewording we discussed in the other message, and send the result in?
quoted
(2) "git remote prune" seems to cull refs/remotes/one/HEAD if exists. Currently we do not have a way to determine where HEAD at the remote points at at the protocol level (I've sent a patch to the list earlier for the necessary protocol extension on the upload-pack side, but receiver side never got implemented in remotes.c). So we cannot propagate refs/HEAD information correctly right now, but when we accept the protocol extension to do so, issue (1) will matter also for HEAD.There's the issue that "HEAD" isn't "refs/HEAD". I'm not at all sure how the user should communicate the desire to update things to match the remote HEAD. FWIW, I was considering moving the code to guess where the remote HEAD points from builtin-clone to remotes.c, until I realized that it's not clear what configuration should control this.. I think it'd be necessary to have a special option to say "write HEAD here", but I may be wrong.
I tend to agree. I'd propose the semantics for refs/remotes/<name>/HEAD symref to be like this: * It is under _local_ control. That means fetch should not update it, and "remote prune" should not prune it, nor even mention it is prunable. * It is the means for the user (i.e. the owner of the local repository) to express which branch from the remote he is most interested in. I.e., it exists solely to make "<name>" => "refs/remotes/<name>/HEAD" ref dwimming work as expected. * It is set up by "git clone" to point at the branch the remote had its HEAD pointing at when clone happened but that is merely a convenience feature. * We would probably want an explicit convenience subcommand "git remote something <name> <branch>" that switches refs/remotes/<name>/HEAD to point at a specific remote tracking branch, although you can do that yourself with symbolic-ref. * We may want to teach "git remote add <name>" to do the same HEAD discovery as done by "git clone" (earlier JBF had a patch for it to the scripted version), to have the same convenience feature as "git clone" has. * If we teach "git remote add" to set refs/remotes/<name>/HEAD, we may also want to teach it an explicit way to let the user say "I want <name> to mean refs/remotes/<name>/this", not whatever the remote side currently points at with its HEAD. * If we teach "git remote add" to do the HEAD discovery, we may also want to teach "git remote update" a way to let the user request "my refs/remotes/<name>/HEAD may not be pointing at the branch the remote currently points at with its HEAD. Please update mine to match theirs". When true mirroring configuration "refs/*:refs/*" is employed, neither "refs/HEAD" nor "refs/heads/HEAD" is needed nor desired on the local side.