Thread (4 messages) flat view 4 messages, 2 authors, 2016-06-15

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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help