* git remote --mirror bug? @ 2008-03-14 13:05 Joakim Tjernlund 2008-03-15 18:08 ` Joakim Tjernlund 0 siblings, 1 reply; 16+ messages in thread From: Joakim Tjernlund @ 2008-03-14 13:05 UTC (permalink / raw) To: git Created a mirror like so: git --bare init git remote add --mirror os2kernel /usr/local/src/os2kernel Git fetch errors out git fetch os2kernel fatal: * refusing to create funny ref 'refs/stash' locally Also git remote show os2kernel * remote os2kernel URL: /usr/local/src/os2kernel Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* git --version git version 1.5.4.3 Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: git remote --mirror bug? 2008-03-14 13:05 git remote --mirror bug? Joakim Tjernlund @ 2008-03-15 18:08 ` Joakim Tjernlund 2008-03-16 10:21 ` Re* " Junio C Hamano 2008-03-28 6:16 ` [PATCH/RFC] Allow "git remote --mirror" to mirror stashes Junio C Hamano 0 siblings, 2 replies; 16+ messages in thread From: Joakim Tjernlund @ 2008-03-15 18:08 UTC (permalink / raw) To: git On Fri, 2008-03-14 at 14:05 +0100, Joakim Tjernlund wrote: > Created a mirror like so: > git --bare init > git remote add --mirror os2kernel /usr/local/src/os2kernel > > Git fetch errors out > git fetch os2kernel > fatal: * refusing to create funny ref 'refs/stash' locally > > Also > git remote show os2kernel > * remote os2kernel > URL: /usr/local/src/os2kernel > Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* > > git --version > git version 1.5.4.3 > > Jocke Forgot to mention that clearing the stash with "git stash clear" deletes the refs/stash file and then above commands succeeds. This is a rather harmless bug, but if you are running the fetch command in a cron job to backup your repo, it becomes more serious as one will not see the failure. Jocke ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re* git remote --mirror bug? 2008-03-15 18:08 ` Joakim Tjernlund @ 2008-03-16 10:21 ` Junio C Hamano 2008-03-16 17:21 ` remote/clone bug: Stale tracking branch HEAD Teemu Likonen 2008-03-18 14:04 ` Re* git remote --mirror bug? Johannes Schindelin 2008-03-28 6:16 ` [PATCH/RFC] Allow "git remote --mirror" to mirror stashes Junio C Hamano 1 sibling, 2 replies; 16+ messages in thread From: Junio C Hamano @ 2008-03-16 10:21 UTC (permalink / raw) To: joakim.tjernlund; +Cc: git, Johannes Schindelin Joakim Tjernlund <joakim.tjernlund@transmode.se> writes: >> git remote show os2kernel >> * remote os2kernel >> URL: /usr/local/src/os2kernel >> Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* This is very unfortunate. What's more unfortunate is that, adding insult to injury, "git remote" reimplemented in C gives even worse nonsense: $ git remote show git.git fatal: * refusing to create funny ref 'refs/stash' locally This is with my test repository sitting next to my primary git.git repository with config entries like this: [remote "git.git"] url = ../git.git/ fetch = refs/*:refs/* The thing is that we obviously are _not_ attempting to create anything when we say "git remote show". Now, it happens that "stash" is purely a local matter, and propagating it with fetch (even with mirroring) may not make much sense. Even if the mirroring is for backup purposes, its value is dubious (if you are backing up because you are afraid of disk failure, you should back up properly with disk back-up tools). So in that sense, it could be argued that it is Ok not to propagate refs/stash and that was partly the reason why we create the "stash" ref directly underneath refs/ namespace. Everything else that is propagatable has at least two level refnames, e.g. heads/master, tags/v1.5.0, and the plumbing layer has a way to enforce this, and "git remote" uses it. But that is not an excuse to fail "git fetch" (which is the underlying mechanism "git remote" uses). I think this patch may work the issue around for recent enough git, but I have to say that this is an interim fix. With this, now you get a less nonsense output, but it still is nonsense nevertheless: $ git remote show git.git * remote git.git URL: ../git.git/ New remote branch (next fetch will store in remotes/git.git) refs/stash Tracked remote branches ar/sgid-bsd cb/mergetool cc/help cr/reset-parseopt db/diff... Notice that it talks about storing refs/stash in remotes/git.git? It won't, and it shouldn't, as we told it to mirror refs/stash to refs/stash. As a side note, I think remote, when doing "show" that does _not_ update the local side of tracking refs, should tweak the error behaviour that it can trigger when it calls get_fetch_map(). The callee may have originally been written for "fetch" and had a hardcoded "we are fetching and storing, so our behaviour when seeing a funny ref is to error out with "refusing to create" message and that is fine" mentality. Calling such a function when it is not actually fetching, without modifying the callee's assumption to suit the error behaviour for its needs, is sloppy and needs to be fixed. The scripted version if git-parse-remote, which is being phased out (it still is used somewhat by git-pull and git-clone) has similar logic to parse fetch refspec mapping, but it does not understand the mirroring layout (refs/*:refs/*) as you saw, so it fails way before it triggers these issues. What the C reimplementation does (or at least tries to do) is an improvement in that sense, but because it is relatively a new feature, bugs in there are to be expected. We are going into 1.5.5-rc feature freeze tonight, and squashing these bugs now are of the highest priority. Please keep the bug reports and fixes flowing. Thanks. --- builtin-check-ref-format.c | 2 +- git-parse-remote.sh | 9 +++++++-- remote.c | 16 +++++++++++++--- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c index fe04be7..e2b4eef 100644 --- a/builtin-check-ref-format.c +++ b/builtin-check-ref-format.c @@ -10,5 +10,5 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix) { if (argc != 2) usage("git-check-ref-format refname"); - return !!check_ref_format(argv[1]); + return (-check_ref_format(argv[1])); } diff --git a/git-parse-remote.sh b/git-parse-remote.sh index 695a409..19d00e7 100755 --- a/git-parse-remote.sh +++ b/git-parse-remote.sh @@ -156,8 +156,13 @@ canon_refs_list_for_fetch () { if local_ref_name=$(expr "z$local" : 'zrefs/\(.*\)') then - git check-ref-format "$local_ref_name" || - die "* refusing to create funny ref '$local_ref_name' locally" + git check-ref-format "$local_ref_name" + case "$?" in + 0 | 2) ;; + *) + die "* refusing to create funny ref '$local_ref_name' locally" + ;; + esac fi echo "${dot_prefix}${force}${remote}:${local}" done diff --git a/remote.c b/remote.c index f3f7375..6a95b1e 100644 --- a/remote.c +++ b/remote.c @@ -1007,9 +1007,19 @@ int get_fetch_map(const struct ref *remote_refs, } for (rm = ref_map; rm; rm = rm->next) { - if (rm->peer_ref && check_ref_format(rm->peer_ref->name + 5)) - die("* refusing to create funny ref '%s' locally", - rm->peer_ref->name); + if (rm->peer_ref) { + switch (check_ref_format(rm->peer_ref->name + 5)) { + default: + die("* refusing to create funny ref '%s' locally", + rm->peer_ref->name); + break; + case CHECK_REF_FORMAT_ONELEVEL: + /* allow "refs/stash" to be propagated */ + break; + case CHECK_REF_FORMAT_OK: + break; + } + } } if (ref_map) ^ permalink raw reply related [flat|nested] 16+ messages in thread
* remote/clone bug: Stale tracking branch HEAD 2008-03-16 10:21 ` Re* " Junio C Hamano @ 2008-03-16 17:21 ` Teemu Likonen 2008-03-16 22:24 ` On fetch refspecs and wildcards Junio C Hamano 2008-03-18 14:04 ` Re* git remote --mirror bug? Johannes Schindelin 1 sibling, 1 reply; 16+ messages in thread From: Teemu Likonen @ 2008-03-16 17:21 UTC (permalink / raw) To: git; +Cc: Junio C Hamano, Johannes Schindelin Junio C Hamano kirjoitti: > We are going into 1.5.5-rc feature freeze tonight, and squashing > these bugs now are of the highest priority. Please keep the bug > reports and fixes flowing. I hope this bug won't find it's way to the release: http://article.gmane.org/gmane.comp.version-control.git/77188 In short: After 'git clone' the command 'git remote show origin' shows a stale tracking branch HEAD: Stale tracking branch (use 'git remote prune') HEAD 'git remote prune origin' removes branch 'master'; you can see this with "git remote show origin": New remote branch (next fetch will store in remotes/origin) master 'git fetch' fetches 'master' again but the stale tracking branch HEAD appears again. ^ permalink raw reply [flat|nested] 16+ messages in thread
* On fetch refspecs and wildcards 2008-03-16 17:21 ` remote/clone bug: Stale tracking branch HEAD Teemu Likonen @ 2008-03-16 22:24 ` Junio C Hamano 2008-03-16 22:33 ` Junio C Hamano 2008-03-16 23:03 ` Daniel Barkalow 0 siblings, 2 replies; 16+ messages in thread From: Junio C Hamano @ 2008-03-16 22:24 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Daniel Barkalow, git, Teemu Likonen I was looking at t5505 tests and noticed something funny. This is a design level question, so I am cc'ing Daniel whose remote.c is heavily involved in the new implementation. What should this config do: [remote "origin"] url = ../one/.git fetch = +refs/heads/*:refs/remotes/origin/* fetch = refs/heads/master:refs/heads/upstream when the other repository (../one/.git) has branches "master" and "side2"? I am not sure if the original implementation used to copy master to both refs/remotes/origin/master and refs/heads/upstream, but I think that is what the users would expect. I think the current one excludes any source that has an explicit destination from the wildcard matches. It is probably Ok as long as we reject if the same source has more than one destinations (or matches more than one wildcards, for that matter) like this as a configuration error: [remote "origin"] url = ../one/.git fetch = refs/heads/master:refs/heads/upstream fetch = refs/heads/master:refs/heads/another If it doesn't, it does feel somewhat inconsistent. 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. Another topic is what the semantics should be for mirroring configuration, like this: [remote "origin"] url = ../one/.git fetch = refs/*:refs/* or [remote "origin"] url = ../one/.git fetch = refs/*:refs/remotes/one/* 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(). (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. I think there is no design level question on this one. I think both are bugs we currently have (and they may probably not be regressions). ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: On fetch refspecs and wildcards 2008-03-16 22:24 ` On fetch refspecs and wildcards Junio C Hamano @ 2008-03-16 22:33 ` Junio C Hamano 2008-03-16 23:03 ` Daniel Barkalow 1 sibling, 0 replies; 16+ messages in thread From: Junio C Hamano @ 2008-03-16 22:33 UTC (permalink / raw) To: Johannes Schindelin; +Cc: Daniel Barkalow, git, Teemu Likonen Junio C Hamano <gitster@pobox.com> writes: > ... > 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 Clarification. "s/;/, but do honor requests to copy one thing to multiple destinations;/" is what I meant here. > (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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: On fetch refspecs and wildcards 2008-03-16 22:24 ` On fetch refspecs and wildcards Junio C Hamano 2008-03-16 22:33 ` Junio C Hamano @ 2008-03-16 23:03 ` Daniel Barkalow 2008-03-17 0:14 ` Junio C Hamano 1 sibling, 1 reply; 16+ messages in thread From: Daniel Barkalow @ 2008-03-16 23:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git, Teemu Likonen On Sun, 16 Mar 2008, Junio C Hamano wrote: > I was looking at t5505 tests and noticed something funny. > > This is a design level question, so I am cc'ing Daniel whose remote.c is > heavily involved in the new implementation. > > What should this config do: > > [remote "origin"] > url = ../one/.git > fetch = +refs/heads/*:refs/remotes/origin/* > fetch = refs/heads/master:refs/heads/upstream > > when the other repository (../one/.git) has branches "master" and "side2"? > > I am not sure if the original implementation used to copy master to both > refs/remotes/origin/master and refs/heads/upstream, but I think that is > what the users would expect. > > I think the current one excludes any source that has an explicit > destination from the wildcard matches. It is probably Ok as long as we > reject if the same source has more than one destinations (or matches more > than one wildcards, for that matter) like this as a configuration error: > > [remote "origin"] > url = ../one/.git > fetch = refs/heads/master:refs/heads/upstream > fetch = refs/heads/master:refs/heads/another > > If it doesn't, it does feel somewhat inconsistent. > > 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.) In any case, the implementation should be easy enough with any of these combinations. > Another topic is what the semantics should be for mirroring configuration, > like this: > > [remote "origin"] > url = ../one/.git > fetch = refs/*:refs/* > > or > > [remote "origin"] > url = ../one/.git > fetch = refs/*:refs/remotes/one/* > > 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". > (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. The implementation of whatever handles it is slightly non-trivial, since struct ref can't currently represent a symref, but that shouldn't be a major issue. -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: On fetch refspecs and wildcards 2008-03-16 23:03 ` Daniel Barkalow @ 2008-03-17 0:14 ` Junio C Hamano 2008-03-17 2:14 ` Daniel Barkalow 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2008-03-17 0:14 UTC (permalink / raw) To: Daniel Barkalow; +Cc: Johannes Schindelin, git, Teemu Likonen Daniel Barkalow <barkalow@iabervon.org> writes: > On Sun, 16 Mar 2008, Junio C Hamano wrote: > ... >> 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. >> 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? >> (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. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: On fetch refspecs and wildcards 2008-03-17 0:14 ` Junio C Hamano @ 2008-03-17 2:14 ` Daniel Barkalow 0 siblings, 0 replies; 16+ messages in thread From: Daniel Barkalow @ 2008-03-17 2:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Johannes Schindelin, git, Teemu Likonen On Sun, 16 Mar 2008, Junio C Hamano wrote: > Daniel Barkalow <barkalow@iabervon.org> writes: > > > On Sun, 16 Mar 2008, Junio C Hamano wrote: > > ... > >> 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. Oh, sorry, I got the cases backwards. The current code does (1): for each refspec, it collects all of the matches for that refspec into one big list, and then it (a) removes items where src and dst match and (b) gives errors if dst matches but src doesn't (which is obviously a problem: two refspecs will try to write different things to the same place). > >> 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? Sure. > >> (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. Ah, interesting. Not at all what I'd expected, but much more useful, I think, than having it controlled by the remote. > * 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. In other words, a reasonable default since you haven't yet configured anything. > * 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. The command would mainly be useful to users as a way of making it clear that it's theirs to control, rather than as a method for controlling it. > * 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. Right. > * 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. And likewise clone, which is probably more relevant, actually, because clone will often also create a local branch based on it and check that out, which can waste some time if you actually want to use some other branch. > * 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". Right. > When true mirroring configuration "refs/*:refs/*" is employed, neither > "refs/HEAD" nor "refs/heads/HEAD" is needed nor desired on the local side. Sure. -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re* git remote --mirror bug? 2008-03-16 10:21 ` Re* " Junio C Hamano 2008-03-16 17:21 ` remote/clone bug: Stale tracking branch HEAD Teemu Likonen @ 2008-03-18 14:04 ` Johannes Schindelin 2008-03-18 19:02 ` Junio C Hamano 1 sibling, 1 reply; 16+ messages in thread From: Johannes Schindelin @ 2008-03-18 14:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: joakim.tjernlund, git Hi, On Sun, 16 Mar 2008, Junio C Hamano wrote: > Joakim Tjernlund <joakim.tjernlund@transmode.se> writes: > > >> git remote show os2kernel > >> * remote os2kernel > >> URL: /usr/local/src/os2kernel > >> Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* > > This is very unfortunate. > > [...] > > builtin-check-ref-format.c | 2 +- > git-parse-remote.sh | 9 +++++++-- > remote.c | 16 +++++++++++++--- > 3 files changed, 21 insertions(+), 6 deletions(-) Thanks for the fix, and sorry for not being available to fix it myself. Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re* git remote --mirror bug? 2008-03-18 14:04 ` Re* git remote --mirror bug? Johannes Schindelin @ 2008-03-18 19:02 ` Junio C Hamano 2008-03-19 0:35 ` Johannes Schindelin 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2008-03-18 19:02 UTC (permalink / raw) To: Johannes Schindelin; +Cc: joakim.tjernlund, git Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Sun, 16 Mar 2008, Junio C Hamano wrote: > >> Joakim Tjernlund <joakim.tjernlund@transmode.se> writes: >> >> >> git remote show os2kernel >> >> * remote os2kernel >> >> URL: /usr/local/src/os2kernel >> >> Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* >> >> This is very unfortunate. >> >> [...] >> >> builtin-check-ref-format.c | 2 +- >> git-parse-remote.sh | 9 +++++++-- >> remote.c | 16 +++++++++++++--- >> 3 files changed, 21 insertions(+), 6 deletions(-) > > Thanks for the fix,... As I alluded to in the message, I do not think this was a fix. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: Re* git remote --mirror bug? 2008-03-18 19:02 ` Junio C Hamano @ 2008-03-19 0:35 ` Johannes Schindelin 0 siblings, 0 replies; 16+ messages in thread From: Johannes Schindelin @ 2008-03-19 0:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: joakim.tjernlund, git Hi, On Tue, 18 Mar 2008, Junio C Hamano wrote: > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Sun, 16 Mar 2008, Junio C Hamano wrote: > > > >> Joakim Tjernlund <joakim.tjernlund@transmode.se> writes: > >> > >> >> git remote show os2kernel > >> >> * remote os2kernel > >> >> URL: /usr/local/src/os2kernel > >> >> Warning: unrecognized mapping in remotes.os2kernel.fetch: +refs/*:refs/* > >> > >> This is very unfortunate. > >> > >> [...] > >> > >> builtin-check-ref-format.c | 2 +- > >> git-parse-remote.sh | 9 +++++++-- > >> remote.c | 16 +++++++++++++--- > >> 3 files changed, 21 insertions(+), 6 deletions(-) > > > > Thanks for the fix,... > > As I alluded to in the message, I do not think this was a fix. I am very sorry, as I read the mail under extreme time pressure, this must have slipped by. I hope that my time management reverts to normal beginning tomorrow. I looked into this issue, and I seem not to be able to reproduce with my current git (which is based on next). Ciao, Dscho ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH/RFC] Allow "git remote --mirror" to mirror stashes 2008-03-15 18:08 ` Joakim Tjernlund 2008-03-16 10:21 ` Re* " Junio C Hamano @ 2008-03-28 6:16 ` Junio C Hamano 2008-03-28 15:45 ` Daniel Barkalow 1 sibling, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2008-03-28 6:16 UTC (permalink / raw) To: git; +Cc: joakim.tjernlund, Daniel Barkalow When you have "remote.$there.fetch = refs/*:refs/*" and the remote has a ref directly under refs/ (e.g. "stash"), "git fetch" still errored out even with fixes in -rc1. This should hopefully fix it. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- * Rather than failing, it would be better to allow "git fetch" to succeed by doing this, but on the other hand, stash is purely a local matter, so it might make more sense to avoid exposing it from the uploader. builtin-fetch-pack.c | 13 ++++++++++--- 1 files changed, 10 insertions(+), 3 deletions(-) diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c index 65350ca..472bad5 100644 --- a/builtin-fetch-pack.c +++ b/builtin-fetch-pack.c @@ -363,10 +363,17 @@ static void filter_refs(struct ref **refs, int nr_match, char **match) return_refs = NULL; for (ref = *refs; ref; ref = next) { + int trash = 0; + next = ref->next; - if (!memcmp(ref->name, "refs/", 5) && - check_ref_format(ref->name + 5)) - ; /* trash */ + if (!memcmp(ref->name, "refs/", 5)) { + trash = check_ref_format(ref->name + 5); + if (trash == CHECK_REF_FORMAT_ONELEVEL) + trash = 0; + } + + if (trash) + ; /* this is trash */ else if (args.fetch_all && (!args.depth || prefixcmp(ref->name, "refs/tags/") )) { *newtail = ref; ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] Allow "git remote --mirror" to mirror stashes 2008-03-28 6:16 ` [PATCH/RFC] Allow "git remote --mirror" to mirror stashes Junio C Hamano @ 2008-03-28 15:45 ` Daniel Barkalow 2008-03-31 0:19 ` Junio C Hamano 0 siblings, 1 reply; 16+ messages in thread From: Daniel Barkalow @ 2008-03-28 15:45 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, joakim.tjernlund On Thu, 27 Mar 2008, Junio C Hamano wrote: > When you have "remote.$there.fetch = refs/*:refs/*" and the remote has a > ref directly under refs/ (e.g. "stash"), "git fetch" still errored out > even with fixes in -rc1. In particular, it would fail to request "refs/stash", and then be surprised that it didn't get the object that points to. (This would be a helpful thing to mention in the commit message) > This should hopefully fix it. Maybe it shouldn't do any filtering here, and instead do it in cmd_fetch_pack? If the transport code gets to this point and anything gets filtered out by this function, the transport code or builtin-fetch will have to be terribly confused and fail with a mysterious error message, AFAICT. > * Rather than failing, it would be better to allow "git fetch" to succeed > by doing this, but on the other hand, stash is purely a local matter, > so it might make more sense to avoid exposing it from the uploader. This is also true, although I'm not too sure that we won't want to do things like having "refs/default" in a public repository be the repository's suggestion for the default branch (to replace "HEAD", because, in a world where people use lots of branches, the "current branch" idea and the "default branch" idea aren't really the same idea, although there's no technical conflict since only one of these ideas is really important in any given repository). So we probably want a whitelist or blacklist for refs to serve when we avoid exposing things in the uploader, rather than using the level, in which case it's definitely important to have fetch-pack just ignore stuff. -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] Allow "git remote --mirror" to mirror stashes 2008-03-28 15:45 ` Daniel Barkalow @ 2008-03-31 0:19 ` Junio C Hamano 2008-03-31 3:03 ` Daniel Barkalow 0 siblings, 1 reply; 16+ messages in thread From: Junio C Hamano @ 2008-03-31 0:19 UTC (permalink / raw) To: Daniel Barkalow; +Cc: git, joakim.tjernlund Daniel Barkalow <barkalow@iabervon.org> writes: > On Thu, 27 Mar 2008, Junio C Hamano wrote: > > Maybe it shouldn't do any filtering here, and instead do it in > cmd_fetch_pack? I dunno. How would the code look like? > This is also true, although I'm not too sure that we won't want to do > things like having "refs/default" in a public repository be the > repository's suggestion for the default branch (to replace "HEAD", > because, in a world where people use lots of branches, the "current > branch" idea and the "default branch" idea aren't really the same idea, In a public repository with many branches to serve people with different interests, I do not think a single refs/default in addition to HEAD would help that much. We would _not_ want to have more magic refs like HEAD. Quite the opposite. In such a repository, HEAD means even less, and instead of giving an extra layer of indirection, you tell people which branches are what in your repository. "If you are interested in only the bugfixes without any new features since the last feature lease no matter how solid and tested they are, use 'maint' branch. If you want solid and tested features, and do not mind new features, use 'master'. Etc.". And just like a good API names its functions sensibly, you give meaningful names to your branches, so that you do not _need_ that extra layer of indirection refs/default would incur. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH/RFC] Allow "git remote --mirror" to mirror stashes 2008-03-31 0:19 ` Junio C Hamano @ 2008-03-31 3:03 ` Daniel Barkalow 0 siblings, 0 replies; 16+ messages in thread From: Daniel Barkalow @ 2008-03-31 3:03 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, joakim.tjernlund On Sun, 30 Mar 2008, Junio C Hamano wrote: > Daniel Barkalow <barkalow@iabervon.org> writes: > > > On Thu, 27 Mar 2008, Junio C Hamano wrote: > > > > Maybe it shouldn't do any filtering here, and instead do it in > > cmd_fetch_pack? > > I dunno. How would the code look like? Actually, I don't see any reason to call check_ref_format. The point of filter_refs is to make sure that we don't fetch anything we didn't ask for. We shouldn't care at all about the name of the refs we're considering except whether there's in the list to fetch, and if the user requests the objects for a ref named 'refs/*^&+' and the server offers such a ref, there's no reason for us not to get the objects. (Sure, we shouldn't create the ref with that name, but this code path doesn't go on the create refs based on these names, except when it's already checking their format for that purpose anyway.) So I'd say just drop the first "if" in that sequence entirely. The only thing that could be a problem we'd want to stop here is something that would break the packet protocol, and we've already gotten these values over the packet protocol anyway. > > This is also true, although I'm not too sure that we won't want to do > > things like having "refs/default" in a public repository be the > > repository's suggestion for the default branch (to replace "HEAD", > > because, in a world where people use lots of branches, the "current > > branch" idea and the "default branch" idea aren't really the same idea, > > In a public repository with many branches to serve people with different > interests, I do not think a single refs/default in addition to HEAD would > help that much. We would _not_ want to have more magic refs like HEAD. > > Quite the opposite. In such a repository, HEAD means even less, and > instead of giving an extra layer of indirection, you tell people which > branches are what in your repository. "If you are interested in only the > bugfixes without any new features since the last feature lease no matter > how solid and tested they are, use 'maint' branch. If you want solid and > tested features, and do not mind new features, use 'master'. Etc.". It's not a particularly *useful* default, but "git-clone" presumably should initially check out *something* given a repository with multiple branches and no local user guidance. And, if this is the git.git repository, and you briefly check out each branch in turn to build it when you push new changes, then HEAD is usually master but briefly, rarely, and irrelevantly other things. I'm not convinced that it's something worth actually implementing. But I think it's a plausible enough idea that we shouldn't exclude the possibility of one-level public refs. There are various usues people find for this sort of low-semantics pointer on FTP sites, so it could be useful in git as well. -Daniel *This .sig left intentionally blank* ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2008-03-31 3:04 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2008-03-14 13:05 git remote --mirror bug? Joakim Tjernlund 2008-03-15 18:08 ` Joakim Tjernlund 2008-03-16 10:21 ` Re* " Junio C Hamano 2008-03-16 17:21 ` remote/clone bug: Stale tracking branch HEAD Teemu Likonen 2008-03-16 22:24 ` On fetch refspecs and wildcards Junio C Hamano 2008-03-16 22:33 ` Junio C Hamano 2008-03-16 23:03 ` Daniel Barkalow 2008-03-17 0:14 ` Junio C Hamano 2008-03-17 2:14 ` Daniel Barkalow 2008-03-18 14:04 ` Re* git remote --mirror bug? Johannes Schindelin 2008-03-18 19:02 ` Junio C Hamano 2008-03-19 0:35 ` Johannes Schindelin 2008-03-28 6:16 ` [PATCH/RFC] Allow "git remote --mirror" to mirror stashes Junio C Hamano 2008-03-28 15:45 ` Daniel Barkalow 2008-03-31 0:19 ` Junio C Hamano 2008-03-31 3:03 ` Daniel Barkalow
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).