{"thread":{"id":"9807","subject":"rebase from ambiguous ref discards changes","startedAt":"2007-09-06T21:48:28Z","lastAt":"2007-09-08T22:20:59Z","messageCount":17,"participants":["Keith Packard","Pierre Habouzit","Junio C Hamano","Johannes Sixt","Nicolas Pitre","Carl Worth","Alex Riesen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"52805","messageId":"1189115308.30308.9.camel@koto.keithp.com","threadId":"9807","inReplyTo":null,"subject":"rebase from ambiguous ref discards changes","fromName":"Keith Packard","fromEmail":"keithp@keithp.com","sentAt":"2007-09-06T21:48:28Z","receivedAt":"2007-09-06T21:48:28Z","isPatch":false,"sender":{"key":"keithp@keithp.com","avatar":"https://gravatar.com/avatar/fa1f479cdd51322fe86215c955a81d296bbf66a1fe625f8a12d87a8ec7faf648?d=mp&s=160"},"body":"So, I started with a very simple repository\n\n---*--- master\n    \\\n     -- origin/master\n\nFrom master, I did\n\n$ git-rebase origin/master\nwarning: refname 'master' is ambiguous.\nFirst, rewinding head to replay your work on top of it...\nHEAD is now at 2a8592f... Fix G33 GTT stolen mem range\nFast-forwarded master to origin/master.\n$\n\nNote the lack of the usual 'Applying <patch>' messages.\n\nchecking the tree, I now had\n\n---*\n    \\\n     -- origin/master\n        master\n\nwith my patch lost.\n\nrecovering my patch (having the ID in my terminal window from the\ncommit), I named it 'master-with-fix'\n\n---*--- master-with-fix\n    \\\n     -- origin/master\n        master\n\nNow the rebase from 'master-with-fix worked as expected:\n\n$ git-rebase origin/master\nFirst, rewinding head to replay your work on top of it...\nHEAD is now at 2a8592f... Fix G33 GTT stolen mem range\n\nApplying Switch to pci_device_map_range/pci_device_unmap_range APIs.\n\nAdds trailing whitespace.\n.dotest/patch:225:      \nAdds trailing whitespace.\n.dotest/patch:226:      if (IS_I965G(pI830)) \nAdds trailing whitespace.\n.dotest/patch:446:\ndev->regions[mmio_bar].size, \nAdds trailing whitespace.\n.dotest/patch:449:    \nwarning: 4 lines add whitespace errors.\nWrote tree cd373666254d56a137d282deeb15a2ccaf8da22b\nCommitted: 286f5df0b62f571cbb4dbf120679d3af029b8775\n$ \n\nAnd the tree looks right too:\n\n--- origin/master --- master-with-fix\n    master\n       \nSeems like there's something going on when 'master' is ambiguous, or\nperhaps some other problem.\n\nThis is all from version 1.5.3, but I think I've seen this on 1.5.2 as\nwell.\n\nGit made me sad today; I'm not sure it's ever disappointed like this\nbefore.\n\n-- \nkeith.packard@intel.com\n"},{"id":"52810","messageId":"20070906223721.GC26924@artemis.corp","threadId":"9807","inReplyTo":"1189115308.30308.9.camel@koto.keithp.com","subject":"Re: rebase from ambiguous ref discards changes","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-06T22:37:21Z","receivedAt":"2007-09-06T22:37:21Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Sep 06, 2007 at 09:48:28PM +0000, Keith Packard wrote:\n> recovering my patch (having the ID in my terminal window from the\n> commit), I named it 'master-with-fix'\n\n  Note that your patch was not lost, even if you had pruned, because it\nwould still be accessible from the reflog of your master branch. Not\nthat it's supposed to soften your disappointment but well, at least it\nshould make you a bit less uncomfortable, maybe :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"52817","messageId":"7vsl5r8jer.fsf@gitster.siamese.dyndns.org","threadId":"9807","inReplyTo":"1189115308.30308.9.camel@koto.keithp.com","subject":"Re: rebase from ambiguous ref discards changes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-06T23:26:52Z","receivedAt":"2007-09-06T23:26:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Keith Packard <keithp@keithp.com> writes:\n\n> So, I started with a very simple repository\n>\n> ---*--- master\n>     \\\n>      -- origin/master\n>\n> From master, I did\n>\n> $ git-rebase origin/master\n> warning: refname 'master' is ambiguous.\n> First, rewinding head to replay your work on top of it...\n> HEAD is now at 2a8592f... Fix G33 GTT stolen mem range\n> Fast-forwarded master to origin/master.\n> ...\n> Seems like there's something going on when 'master' is ambiguous, or\n> perhaps some other problem.\n>\n> This is all from version 1.5.3, but I think I've seen this on 1.5.2 as\n> well.\n\nWe haven't touched this area for a long time (like \"v1.3.0\" or\n\"since March 2006\").  I wish you told us about this earlier.\n\nI'd like to reproduce this, but I need to be sure what your\n\"ambiguous\" situation is really like.  What ambiguous \"master\"s\ndo you have?  IOW, what does:\n\n\tgit show-ref | grep master\n\nsay?  I have 11 lines of output from the above and I have never\nseen a problem like this.\n\nPerhaps you have \".git/master\" by mistake?\n"},{"id":"52837","messageId":"1189133898.30308.58.camel@koto.keithp.com","threadId":"9807","inReplyTo":"7vsl5r8jer.fsf@gitster.siamese.dyndns.org","subject":"Re: rebase from ambiguous ref discards changes","fromName":"Keith Packard","fromEmail":"keithp@keithp.com","sentAt":"2007-09-07T02:58:18Z","receivedAt":"2007-09-07T02:58:18Z","isPatch":false,"sender":{"key":"keithp@keithp.com","avatar":"https://gravatar.com/avatar/fa1f479cdd51322fe86215c955a81d296bbf66a1fe625f8a12d87a8ec7faf648?d=mp&s=160"},"body":"On Thu, 2007-09-06 at 16:26 -0700, Junio C Hamano wrote:\n\n> We haven't touched this area for a long time (like \"v1.3.0\" or\n> \"since March 2006\").  I wish you told us about this earlier.\n\nI would have were I certain that it wasn't user-error last time. I took\na few minutes this time to verify precisely what I saw and reproduce it.\n\n> I'd like to reproduce this, but I need to be sure what your\n> \"ambiguous\" situation is really like.  What ambiguous \"master\"s\n> do you have?  IOW, what does:\n> \n> \tgit show-ref | grep master\n\n$ git show-ref | grep master\n286f5df0b62f571cbb4dbf120679d3af029b8775 refs/heads/master\n1feb733eb8b09a8b07b7a6987add5149c53b0157 refs/heads/master-guitar\nd957c6b8e1dde8e11c1db3431e0ff58c5d984880 refs/heads/master-i830\n0fd3ba0518b3cde9ca0e4e2fc1854c00d8a43d5c refs/remotes/kyle/master\ne25f8e145f4f73b62c32389b922291fd561af9d2 refs/remotes/origin/lg3d-master\n286f5df0b62f571cbb4dbf120679d3af029b8775 refs/remotes/origin/master\ne25f8e145f4f73b62c32389b922291fd561af9d2 refs/remotes/otc/lg3d-master\n6781575f734f05547d7d5ceef4116fc157bba44d refs/remotes/otc/master\n\nso, nothing obviously amiss here.\n\n> Perhaps you have \".git/master\" by mistake?\n\noops.\n\n$ find .git -name master\n.git/master\n.git/refs/heads/master\n.git/refs/remotes/kyle/master\n.git/refs/remotes/origin/master\n.git/refs/remotes/otc/master\n.git/logs/refs/heads/master\n.git/logs/refs/remotes/fdo/master\n.git/logs/refs/remotes/kyle/master\n.git/logs/refs/remotes/origin/master\n.git/logs/refs/remotes/otc/master\n\nSo, I think that explains where the ambiguous master came from.  Seems\nlike rebase should be able to bail out before breaking things though.\n\n-- \nkeith.packard@intel.com\n"},{"id":"52863","messageId":"20070907065546.GA21418@artemis.corp","threadId":"9807","inReplyTo":"1189133898.30308.58.camel@koto.keithp.com","subject":"Re: rebase from ambiguous ref discards changes","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-07T06:55:46Z","receivedAt":"2007-09-07T06:55:46Z","isPatch":false,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Sep 07, 2007 at 02:58:18AM +0000, Keith Packard wrote:\n> On Thu, 2007-09-06 at 16:26 -0700, Junio C Hamano wrote:\n>\n> > Perhaps you have \".git/master\" by mistake?\n>\n> oops.\n>\n> $ find .git -name master\n> ..git/master\n> ..git/refs/heads/master\n> ..git/refs/remotes/kyle/master\n> ..git/refs/remotes/origin/master\n> ..git/refs/remotes/otc/master\n> ..git/logs/refs/heads/master\n> ..git/logs/refs/remotes/fdo/master\n> ..git/logs/refs/remotes/kyle/master\n> ..git/logs/refs/remotes/origin/master\n> ..git/logs/refs/remotes/otc/master\n> \n> So, I think that explains where the ambiguous master came from.  Seems\n> like rebase should be able to bail out before breaking things though.\n\n  Actually if I get this right, it didn't broke anything, it just\nrebased your \".git/master\" :) It just chose the wrong desambiguation for\nsome reason.\n\n\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"52896","messageId":"7vd4wu67qs.fsf_-_@gitster.siamese.dyndns.org","threadId":"9807","inReplyTo":"1189133898.30308.58.camel@koto.keithp.com","subject":"[PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-07T11:21:47Z","receivedAt":"2007-09-07T11:21:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Keith Packard <keithp@keithp.com> writes:\n\n> On Thu, 2007-09-06 at 16:26 -0700, Junio C Hamano wrote:\n> ...\n>> Perhaps you have \".git/master\" by mistake?\n>\n> oops.\n\nOk, that explains it; git is doing exactly what the user asked\nit to do.\n\n              5---6 side\n             /\n\t1---2---3---4 master\n\nThe user has this history.  But he has a stray .git/master file,\nperhaps created by hand by mistake (it would be very interesting\nto find how that file got there in the first place), that points\nat commit \"3\".  From a side branch, he says \"git rebase master\".\n\nThe first parameter to \"git rebase\" in a single parameter form\nis \"the commit on which to replay the changes my current branch\nhas\".  It is not limited to a branch name.  IOW, it can be an\narbitrary object name, and one of the rules to translate a user\nstring that is supposed to mean an arbitrary object name goes\nthrough this table:\n\n        \"%s\", \"refs/%s\", \"refs/tags/%s\", \"refs/heads/%s\",\n        \"refs/remotes/%s\", \"refs/remotes/%s/HEAD\"\n\nFor each entry in the above table, \"%s\" part of the entry is\nreplaced by the user string, and resulting string is used to see\nif there is such a file under .git/ directory, and the first\nmatch is used (this explanation is simplifying things a bit).\n\nThis rule has been there almost from the beginning.  The first\nentry allowed you to have a tag A and a branch A at the same\ntime, while giving you a way to disambiguate them by spelling\nthem out as \"refs/tags/A\" and \"refs/heads/A\", respectively.\nAlso the first rule covered special \"ref\" names such as HEAD and\nORIG_HEAD.\n\nAlas, there is one drawback of this rule.  His .git/master hides\nrefs/heads/master.  So essentially his command line said \"I've\nbuilt a few commits since I forked from the history that leads\nto commit 3; I want to replay my changes on top of that commit\".\nHe meant to say 4, but said 3, and as a unfortunate consequence,\ncommit 4 is lost.\n\nThe above explanation is solely for understanding the situation\nand, not meant to defend the current behaviour.  I think the\ncurrent behaviour is inviting mistakes and confusion.\n\nThis patch brings in a new world order by introducing a backward\nincompatible change.  When the string the user gave us does not\ncontain any slash, we do not apply the first entry (i.e.\ndirectly underneath .git/ without any \"refs/***\") unless the\nname consists solely of uppercase letters or an underscore,\nthereby ignoring .git/master.  The ones we often use, such as\nHEAD and ORIG_HEAD are not affected by this change.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n sha1_name.c                |   35 ++++++++++++++++++++++++++++++++++\n t/t3407-rebase-confused.sh |   45 ++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 80 insertions(+), 0 deletions(-)\n create mode 100755 t/t3407-rebase-confused.sh\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 2d727d5..1b980ca 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -239,6 +239,35 @@ static int ambiguous_path(const char *path, int len)\n \treturn slash;\n }\n \n+static int confused_ref(const char *str)\n+{\n+\tchar ch;\n+\tint seen_non_uppercase = 0;\n+\n+\t/*\n+\t * People create .git/master by mistake using hand-rolled\n+\t * scripts and confuse themselves utterly.  Make sure this\n+\t * does not match such a path.\n+\t */\n+\twhile ((ch = *str++) != '\\0') {\n+\t\tif (ch == '/')\n+\t\t\treturn 0;\n+\t\tif (!((ch == '_') ||\n+\t\t      ('A' <= ch && ch <= 'Z') ||\n+\t\t      ('0' <= ch && ch <= '9')))\n+\t\t\tseen_non_uppercase = 1;\n+\t}\n+\n+\t/*\n+\t * str did not have any slash and we are checking when\n+\t * it hangs directly underneath .git/; the only valid\n+\t * cases we currently have are HEAD, ORIG_HEAD, MERGE_HEAD and\n+\t * FETCH_HEAD.  If we saw any non uppercase, non underscore,\n+\t * we should ignore this string.\n+\t */\n+\treturn seen_non_uppercase;\n+}\n+\n static const char *ref_fmt[] = {\n \t\"%.*s\",\n \t\"refs/%.*s\",\n@@ -259,6 +288,9 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)\n \t\tunsigned char sha1_from_ref[20];\n \t\tunsigned char *this_result;\n \n+\t\tif ((p == ref_fmt) && confused_ref(str))\n+\t\t\tcontinue;\n+\n \t\tthis_result = refs_found ? sha1_from_ref : sha1;\n \t\tr = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);\n \t\tif (r) {\n@@ -283,6 +315,9 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)\n \t\tchar path[PATH_MAX];\n \t\tconst char *ref, *it;\n \n+\t\tif (p == ref_fmt && confused_ref(str))\n+\t\t\tcontinue;\n+\n \t\tstrcpy(path, mkpath(*p, len, str));\n \t\tref = resolve_ref(path, hash, 0, NULL);\n \t\tif (!ref)\ndiff --git a/t/t3407-rebase-confused.sh b/t/t3407-rebase-confused.sh\nnew file mode 100755\nindex 0000000..5254da6\n--- /dev/null\n+++ b/t/t3407-rebase-confused.sh\n@@ -0,0 +1,45 @@\n+#!/bin/sh\n+\n+test_description=\"Keithp's .git/master problem\n+\n+              5---6 side\n+             /\n+\t1---2---3---4 master\n+\n+\"\n+\n+. ./test-lib.sh\n+\n+build () {\n+\techo \"$1\" >\"$1\" && git add \"$1\" && test_tick && git commit -m \"$1\"\n+}\n+\n+test_expect_success setup '\n+\n+\tbuild one &&\n+\tbuild two &&\n+\tgit branch side &&\n+\tbuild three &&\n+\tgit tag anchor &&\n+\tbuild four &&\n+\tgit-checkout side &&\n+\tbuild five &&\n+\tbuild six &&\n+\tgit update-ref master anchor &&\n+\tgit tag -d anchor\n+\n+'\n+\n+test_expect_success rebase '\n+\n+\tgit rebase master\n+\n+'\n+\n+test_expect_success 'everybody is still there' '\n+\n+\ttest 6 = $(git log --pretty=oneline HEAD | wc -l)\n+\n+'\n+\n+test_done\n-- \n1.5.3.1.879.g4d83f\n"},{"id":"52905","messageId":"46E145BF.4070403@eudaptics.com","threadId":"9807","inReplyTo":"7vd4wu67qs.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-09-07T12:36:15Z","receivedAt":"2007-09-07T12:36:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Junio C Hamano schrieb:\n> But he has a stray .git/master file,\n> perhaps created by hand by mistake (it would be very interesting\n> to find how that file got there in the first place),\n\nIt is easy to get one there if, in a brave moment, you try\n\n    git update-ref master $some_other_ref\n\ninstead of the correct\n\n    git update-ref refs/heads/master $some_other_ref\n\n-- Hannes\n"},{"id":"52906","messageId":"20070907124253.GB27754@artemis.corp","threadId":"9807","inReplyTo":"46E145BF.4070403@eudaptics.com","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-07T12:42:53Z","receivedAt":"2007-09-07T12:42:53Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Sep 07, 2007 at 12:36:15PM +0000, Johannes Sixt wrote:\n> Junio C Hamano schrieb:\n> >But he has a stray .git/master file,\n> >perhaps created by hand by mistake (it would be very interesting\n> >to find how that file got there in the first place),\n> \n> It is easy to get one there if, in a brave moment, you try\n> \n>    git update-ref master $some_other_ref\n> \n> instead of the correct\n> \n>    git update-ref refs/heads/master $some_other_ref\n\n  I was about to say the same :)\n  I'd have added though that maybe update-ref should print a warning for\nthe references that do not match the restriction Junio added. This could\nbe done using the function Junio proposed un update_ref() in refs.c\n\n  note that it's a sane thing to do anyways, I would not bet a lot of\nmoney on what happens if you ask git to:\n\n  git update-ref $foo $sha\n\n  for foo in (completely random :P): index config packed-refs ...\n\n  If a tool needs a new reference namespace, it can create a\nsubdirectory under refs/ so it does not really causes harm IMHO.\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"52914","messageId":"1189181019.30308.91.camel@koto.keithp.com","threadId":"9807","inReplyTo":"46E145BF.4070403@eudaptics.com","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Keith Packard","fromEmail":"keithp@keithp.com","sentAt":"2007-09-07T16:03:39Z","receivedAt":"2007-09-07T16:03:39Z","isPatch":true,"sender":{"key":"keithp@keithp.com","avatar":"https://gravatar.com/avatar/fa1f479cdd51322fe86215c955a81d296bbf66a1fe625f8a12d87a8ec7faf648?d=mp&s=160"},"body":"On Fri, 2007-09-07 at 14:36 +0200, Johannes Sixt wrote:\n\n>     git update-ref master $some_other_ref\n\nI'm sure that's how it was created; I was using update-ref on a regular\nbasis for a few weeks with this repository before the 1.5 new world\norder for remotes came about.\n\n-- \nkeith.packard@intel.com\n"},{"id":"52915","messageId":"1189181313.30308.97.camel@koto.keithp.com","threadId":"9807","inReplyTo":"7vd4wu67qs.fsf_-_@gitster.siamese.dyndns.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Keith Packard","fromEmail":"keithp@keithp.com","sentAt":"2007-09-07T16:08:33Z","receivedAt":"2007-09-07T16:08:33Z","isPatch":true,"sender":{"key":"keithp@keithp.com","avatar":"https://gravatar.com/avatar/fa1f479cdd51322fe86215c955a81d296bbf66a1fe625f8a12d87a8ec7faf648?d=mp&s=160"},"body":"On Fri, 2007-09-07 at 04:21 -0700, Junio C Hamano wrote:\n\n> This patch brings in a new world order by introducing a backward\n> incompatible change.  When the string the user gave us does not\n> contain any slash, we do not apply the first entry (i.e.\n> directly underneath .git/ without any \"refs/***\") unless the\n> name consists solely of uppercase letters or an underscore,\n> thereby ignoring .git/master.  The ones we often use, such as\n> HEAD and ORIG_HEAD are not affected by this change.\n\nIt seems to me that instead of introducing an incompatible (but probably\nuseful) change, a sensible option would be to have the ambiguous\nreference be an error instead of a warning. One shouldn't be encouraged\nto use names in .git that conflict with stuff in refs/heads anyway.\n\n-- \nkeith.packard@intel.com\n"},{"id":"52919","messageId":"alpine.LFD.0.9999.0709071222270.21186@xanadu.home","threadId":"9807","inReplyTo":"1189181313.30308.97.camel@koto.keithp.com","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-09-07T16:29:01Z","receivedAt":"2007-09-07T16:29:01Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 7 Sep 2007, Keith Packard wrote:\n\n> On Fri, 2007-09-07 at 04:21 -0700, Junio C Hamano wrote:\n> \n> > This patch brings in a new world order by introducing a backward\n> > incompatible change.  When the string the user gave us does not\n> > contain any slash, we do not apply the first entry (i.e.\n> > directly underneath .git/ without any \"refs/***\") unless the\n> > name consists solely of uppercase letters or an underscore,\n> > thereby ignoring .git/master.  The ones we often use, such as\n> > HEAD and ORIG_HEAD are not affected by this change.\n> \n> It seems to me that instead of introducing an incompatible (but probably\n> useful) change, a sensible option would be to have the ambiguous\n> reference be an error instead of a warning. One shouldn't be encouraged\n> to use names in .git that conflict with stuff in refs/heads anyway.\n\nI agree.  IMHO the sensible thing to do is to always warn, and error out \nby default.  I see no advantage for core.warnAmbiguousRefs=false other \nthan allow the user to shoot himself in the foot someday.  Instead, we \nshould have core.allowAmbiguousRefs set to off by default.\n\n\nNicolas\n"},{"id":"52939","messageId":"7vejha43oh.fsf@gitster.siamese.dyndns.org","threadId":"9807","inReplyTo":"alpine.LFD.0.9999.0709071222270.21186@xanadu.home","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-07T20:32:30Z","receivedAt":"2007-09-07T20:32:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre <nico@cam.org> writes:\n\n>> It seems to me that instead of introducing an incompatible (but probably\n>> useful) change, a sensible option would be to have the ambiguous\n>> reference be an error instead of a warning. One shouldn't be encouraged\n>> to use names in .git that conflict with stuff in refs/heads anyway.\n>\n> I agree.  IMHO the sensible thing to do is to always warn, and error out \n> by default.  I see no advantage for core.warnAmbiguousRefs=false other \n> than allow the user to shoot himself in the foot someday.  Instead, we \n> should have core.allowAmbiguousRefs set to off by default.\n\nWell, for about three weeks late November to early December\n2005, we did make this an error.  Since mid December 2005, we\nreverted that change to the original \"take first match, without\neven attempting to detect ambiguity\".  I do not recall what the\ndiscussion that led to that change was about, but it could have\nbeen the issue Len had that confused \"git merge\" with a tag and\na branch named after bugzilla bug number.  In any case, this\nchange most likely was because some people were actually using\nthe same name and the change to make it an error hurted them.\n\nWe then reintroduced the ambiguity detection late March 2006,\nbut only as a warning, again fearing that erroring out would\nbreak people's existing setups.  I think we also rewrote\nexamples in our documentation that said \"create your own branch\nv2.6.13 that fork from v2.6.13 tag\" to read \"create your own\nbrancy my-2.6.13...\" to avoid encouraging the use of same name\nto people.\n\nI think the warning has been with us for a long time and by now\npeople know better not to confuse themselves.\n\nSo I am all for making an ambiguous refname an error in 1.5.4.\n\nAt the same time, I think it makes sense to forbid update-ref\noutside refs/ if the refname is not special (say, with any\nlowercase letters), and ignore names immediately below .git that\nare not all-uppercase+underscore (e.g. \"FETCH_HEAD\" we read,\n\"description\" we ignore).\n\nPlease make it so.\n"},{"id":"52940","messageId":"7vabry43cg.fsf@gitster.siamese.dyndns.org","threadId":"9807","inReplyTo":"20070907124253.GB27754@artemis.corp","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-07T20:39:43Z","receivedAt":"2007-09-07T20:39:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> I'd have added though that maybe update-ref should print a warning for\n> the references that do not match the restriction Junio added. This could\n> be done using the function Junio proposed un update_ref() in refs.c\n\nI would even suggest making it into an error, even if we do not\nerror out on the reading side (being liberal when reading but\nmore strict when creating, that is).\n\nThat confused_ref() needs to be tightened further, by the way.\nIt is called only when we are considering to tack the user\nstring immediately below $GIT_DIR/ so the only valid cases are\n(1) the string begins with \"refs/\", or (2) the string is all\nuppercase (or underscore), especially without slash.  The one in\nthe proposed patch is not strict enough and does not enforce the\nformer.\n"},{"id":"52946","messageId":"20070907210433.GD23483@artemis.corp","threadId":"9807","inReplyTo":"7vabry43cg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-09-07T21:04:33Z","receivedAt":"2007-09-07T21:04:33Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Sep 07, 2007 at 08:39:43PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > I'd have added though that maybe update-ref should print a warning for\n> > the references that do not match the restriction Junio added. This could\n> > be done using the function Junio proposed un update_ref() in refs.c\n> \n> I would even suggest making it into an error, even if we do not\n> error out on the reading side (being liberal when reading but\n> more strict when creating, that is).\n> \n> That confused_ref() needs to be tightened further, by the way.\n> It is called only when we are considering to tack the user\n> string immediately below $GIT_DIR/ so the only valid cases are\n> (1) the string begins with \"refs/\", or (2) the string is all\n> uppercase (or underscore), especially without slash.  The one in\n> the proposed patch is not strict enough and does not enforce the\n> former.\n\n  I reckon I didn't checked what the function did in detail, just the\ncode layout :) And I agree an error is event better, I just don't have\nenough knowledge of the scripts that used refs in git for a long time\nthat such a change could break. I mean, I only use git since the 1.2\n(maybe even 1.3) series :)\n\n  I'm always all for refusing dangerous layouts rather than trying too\nhard to support cumbersome things that are 99% of the times issues :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"52953","messageId":"87r6lab0rw.wl%cworth@cworth.org","threadId":"9807","inReplyTo":"7vejha43oh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Carl Worth","fromEmail":"cworth@cworth.org","sentAt":"2007-09-07T21:53:23Z","receivedAt":"2007-09-07T21:53:23Z","isPatch":true,"sender":{"key":"cworth@cworth.org","avatar":"https://gravatar.com/avatar/3746dc28cde609bdbd7f939058356e7e2bbd16d21e32274df0725eb3d998bc5b?d=mp&s=160"},"body":"On Fri, 07 Sep 2007 13:32:30 -0700, Junio C Hamano wrote:\n> So I am all for making an ambiguous refname an error in 1.5.4.\n\nIf you do, then please also make it an error to create an ambiguous\nrefname as well.\n\nFor example, look at how late the warning comes out in this case, and\nhow changing it to an error at that point would not help anything:\n\n\t$ git branch tmp\n\t$ ...\n\t$ git tag tmp\t# No warning here!\n\t$ git show tmp\n\twarning: refname 'tmp' is ambiguous.\n\n-Carl\n"},{"id":"52954","messageId":"7vzlzy162s.fsf@gitster.siamese.dyndns.org","threadId":"9807","inReplyTo":"87r6lab0rw.wl%cworth@cworth.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-07T22:08:59Z","receivedAt":"2007-09-07T22:08:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carl Worth <cworth@cworth.org> writes:\n\n> On Fri, 07 Sep 2007 13:32:30 -0700, Junio C Hamano wrote:\n>> So I am all for making an ambiguous refname an error in 1.5.4.\n>\n> If you do, then please also make it an error to create an ambiguous\n> refname as well.\n\nYeah, didn't I also suggest that already?\n"},{"id":"53011","messageId":"20070908222059.GA5035@steel.home","threadId":"9807","inReplyTo":"7vabry43cg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] HEAD, ORIG_HEAD and FETCH_HEAD are really special.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2007-09-08T22:20:59Z","receivedAt":"2007-09-08T22:20:59Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Junio C Hamano, Fri, Sep 07, 2007 22:39:43 +0200:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > I'd have added though that maybe update-ref should print a warning for\n> > the references that do not match the restriction Junio added. This could\n> > be done using the function Junio proposed un update_ref() in refs.c\n> \n> I would even suggest making it into an error, even if we do not\n> error out on the reading side (being liberal when reading but\n> more strict when creating, that is).\n\nI agree (and suggest failing even on reading), but see below\n\n> That confused_ref() needs to be tightened further, by the way.\n> It is called only when we are considering to tack the user\n> string immediately below $GIT_DIR/ so the only valid cases are\n> (1) the string begins with \"refs/\",\n\nIf that will be the case git-p4-import.bat (yes, just a script of\nmine) will break because it has its namespace directly in $GIT_DIR\n(i.e. .git/p4/*) and stores there backup references. It is just a\nsomeones (ok, it is mine) script, but maybe there are others, who\nexpect that plumbing level git-update-ref just do what its told.\n\n> or (2) the string is all uppercase (or underscore), especially\n> without slash.\n\nI'd suggest just check for uppercase+underscore _or_ slash. It is\nplumbing after all.\n"}]}