{"thread":{"id":"17740","subject":"[topgit] tg update error","startedAt":"2009-02-12T08:09:06Z","lastAt":"2009-02-14T02:24:36Z","messageCount":20,"participants":["Aneesh Kumar","martin f krafft","Aneesh Kumar K.V","Bert Wesarg","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"104349","messageId":"cc723f590902120009w432f5f61xd6550409835cdbb7@mail.gmail.com","threadId":"17740","inReplyTo":null,"subject":"[topgit] tg update error","fromName":"Aneesh Kumar","fromEmail":"aneesh.kumar@gmail.com","sentAt":"2009-02-12T08:09:06Z","receivedAt":"2009-02-12T08:09:06Z","isPatch":false,"sender":{"key":"aneesh.kumar@gmail.com","avatar":"https://gravatar.com/avatar/0621fc0b2f14ead1e9024382f16053a808c148596da30c1b92572fa075621f68?d=mp&s=160"},"body":"doing a tg update with latest git gives the below error\n\n[extent_validate@linux-2.6]$ tg update\nfatal: Refusing to point HEAD outside of refs/heads/\n[extent_validate@linux-2.6]$\n\n-aneesh\n"},{"id":"104354","messageId":"20090212084811.GA14261@piper.oerlikon.madduck.net","threadId":"17740","inReplyTo":"cc723f590902120009w432f5f61xd6550409835cdbb7@mail.gmail.com","subject":"Re: [topgit] tg update error","fromName":"martin f krafft","fromEmail":"madduck@debian.org","sentAt":"2009-02-12T08:48:11Z","receivedAt":"2009-02-12T08:48:11Z","isPatch":false,"sender":{"key":"madduck@debian.org","avatar":null},"body":"also sprach Aneesh Kumar [2009.02.12.0909 +0100]:\n> doing a tg update with latest git gives the below error\n> \n> [extent_validate@linux-2.6]$ tg update\n> fatal: Refusing to point HEAD outside of refs/heads/\n> [extent_validate@linux-2.6]$\n\nWhich version? And could you please provide (a lot) more information\nabout your repository or make it available?\n\n-- \n .''`. martin f. krafft Related projects:\n: :' : proud Debian developer http://debiansystem.info\n`. `'` http://people.debian.org/~madduck http://vcs-pkg.org\n `- Debian - when you have better things to do than fixing systems\n \n\"to me, vi is zen. to use vi is to practice zen. every command is\n a koan. profound to the user, unintelligible to the uninitiated.\n you discover truth everytime you use it.\"\n -- reddy ät lion.austin.ibm.com\n"},{"id":"104356","messageId":"20090212092558.GB21074@skywalker","threadId":"17740","inReplyTo":"20090212084811.GA14261@piper.oerlikon.madduck.net","subject":"Re: [topgit] tg update error","fromName":"Aneesh Kumar K.V","fromEmail":"aneesh.kumar@linux.vnet.ibm.com","sentAt":"2009-02-12T09:25:58Z","receivedAt":"2009-02-12T09:25:58Z","isPatch":false,"sender":{"key":"aneesh.kumar@linux.vnet.ibm.com","avatar":"https://gravatar.com/avatar/a32cbcc7e70c8eb5a4cdbd1ea0b6c2ead382f2529cc43f51fd350767f079d92d?d=mp&s=160"},"body":"On Thu, Feb 12, 2009 at 09:48:11AM +0100, martin f krafft wrote:\n> also sprach Aneesh Kumar [2009.02.12.0909 +0100]:\n> > doing a tg update with latest git gives the below error\n> > \n> > [extent_validate@linux-2.6]$ tg update\n> > fatal: Refusing to point HEAD outside of refs/heads/\n> > [extent_validate@linux-2.6]$\n> \n> Which version? And could you please provide (a lot) more information\n> about your repository or make it available?\n> \n\nLatest git and topgit. Moving to git version v1.6.1.3 fixed the issue.\nI can reproduce the problem on any test repo. Just do a tg update after\ncommitting something in the dependent branch.\n\n-aneesh\n"},{"id":"104358","messageId":"20090212093227.GC20248@piper.oerlikon.madduck.net","threadId":"17740","inReplyTo":"20090212092558.GB21074@skywalker","subject":"Re: [topgit] tg update error","fromName":"martin f krafft","fromEmail":"madduck@madduck.net","sentAt":"2009-02-12T09:32:27Z","receivedAt":"2009-02-12T09:32:27Z","isPatch":false,"sender":{"key":"madduck@madduck.net","avatar":null},"body":"also sprach Aneesh Kumar K.V [2009.02.12.1025 +0100]:\n> Latest git and topgit. Moving to git version v1.6.1.3 fixed the issue.\n> I can reproduce the problem on any test repo. Just do a tg update after\n> committing something in the dependent branch.\n\nThis is not helpful. Please provide a complete transcript of\na session reproducing the problem.\n\nI can't:\n\npiper:~|master|.tmp/cdt.GydvBgiR% echo foo > bar #10002\npiper:~|master|.tmp/cdt.GydvBgiR% giti #10003\nInitialized empty Git repository in /home/madduck/.tmp/cdt.GydvBgiR/.git/\nCreated initial commit 0f189f3: initial checkin\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 bar\npiper:~/.tmp/cdt.GydvBgiR|master|% tg create test #10004\ntg: Automatically marking dependency on master\ntg: Creating test base from master...\nSwitched to a new branch \"test\"\ntg: Topic branch test set up. Please fill .topmsg now and make initial commit.\ntg: To abort: git rm -f .top* && git checkout master && tg delete test\ncached/staged changes:\n .topdeps | 1 +\n .topmsg | 6 ++++++\npiper:~/.tmp/cdt.GydvBgiR|master|% git commit -minit #10005\nCreated commit d49ea41: init\n 2 files changed, 7 insertions(+), 0 deletions(-)\n create mode 100644 .topdeps\n create mode 100644 .topmsg\npiper:~/.tmp/cdt.GydvBgiR|test|% echo bar >> bar #10006\nchanges on filesystem:\n bar | 1 +\npiper:~/.tmp/cdt.GydvBgiR|test|% git add bar #10007\ncached/staged changes:\n bar | 1 +\npiper:~/.tmp/cdt.GydvBgiR|test|% git commit -m'append' #10008\nCreated commit e85457e: append\n 1 files changed, 1 insertions(+), 0 deletions(-)\npiper:~/.tmp/cdt.GydvBgiR|test|% tg update #10009\ntg: The base is up-to-date.\ntg: The test head is up-to-date wrt. the base.\npiper:~/.tmp/cdt.GydvBgiR|test|% git --version #10010\ngit version 1.6.0.2\npiper:~/.tmp/cdt.GydvBgiR|test|% tg --version #10011\nUnknown subcommand: --version\nTopGit v0.5 - A different patch queue manager\nUsage: tg [-r REMOTE] (create|delete|depend|export|import|info|mail|patch|remote|summary|update|help) ...\n\n-- \nmartin | http://madduck.net/ | http://two.sentenc.es/\n \nthis space intentionally left blank.\n \nspamtraps: madduck.bogus@madduck.net\n"},{"id":"104363","messageId":"20090212101243.GC21074@skywalker","threadId":"17740","inReplyTo":"20090212093227.GC20248@piper.oerlikon.madduck.net","subject":"Re: [topgit] tg update error","fromName":"Aneesh Kumar K.V","fromEmail":"aneesh.kumar@linux.vnet.ibm.com","sentAt":"2009-02-12T10:12:43Z","receivedAt":"2009-02-12T10:12:43Z","isPatch":false,"sender":{"key":"aneesh.kumar@linux.vnet.ibm.com","avatar":"https://gravatar.com/avatar/a32cbcc7e70c8eb5a4cdbd1ea0b6c2ead382f2529cc43f51fd350767f079d92d?d=mp&s=160"},"body":"On Thu, Feb 12, 2009 at 10:32:27AM +0100, martin f krafft wrote:\n> also sprach Aneesh Kumar K.V [2009.02.12.1025 +0100]:\n> > Latest git and topgit. Moving to git version v1.6.1.3 fixed the issue.\n> > I can reproduce the problem on any test repo. Just do a tg update after\n> > committing something in the dependent branch.\n> \n> This is not helpful. Please provide a complete transcript of\n> a session reproducing the problem.\n> \n> I can't:\n> \n> piper:~/.tmp/cdt.GydvBgiR|test|% git --version #10010\n> git version 1.6.0.2\n\nThe git version that failed for me is the latest git. As I mentioned\nabove git version 1.6.1.3 works fine.\n\nCan you test with\n$git --version\ngit version 1.6.2.rc0.55.g30aa4f\n\n-aneesh\n"},{"id":"104373","messageId":"36ca99e90902120329v2351174cg4597e4995c4e4274@mail.gmail.com","threadId":"17740","inReplyTo":"20090212093227.GC20248@piper.oerlikon.madduck.net","subject":"Re: [topgit] tg update error","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2009-02-12T11:29:54Z","receivedAt":"2009-02-12T11:29:54Z","isPatch":false,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Thu, Feb 12, 2009 at 10:32, martin f krafft wrote:\n> also sprach Aneesh Kumar K.V [2009.02.12.1025 +0100]:\n>> Latest git and topgit. Moving to git version v1.6.1.3 fixed the issue.\n>> I can reproduce the problem on any test repo. Just do a tg update after\n>> committing something in the dependent branch.\n>\n> This is not helpful. Please provide a complete transcript of\n> a session reproducing the problem.\n>\n> I can't:\n>\n> piper:~|master|.tmp/cdt.GydvBgiR% echo foo > bar #10002\n> piper:~|master|.tmp/cdt.GydvBgiR% giti #10003\n> Initialized empty Git repository in /home/madduck/.tmp/cdt.GydvBgiR/.git/\n> Created initial commit 0f189f3: initial checkin\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> create mode 100644 bar\n> piper:~/.tmp/cdt.GydvBgiR|master|% tg create test #10004\n> tg: Automatically marking dependency on master\n> tg: Creating test base from master...\n> Switched to a new branch \"test\"\n> tg: Topic branch test set up. Please fill .topmsg now and make initial commit.\n> tg: To abort: git rm -f .top* && git checkout master && tg delete test\n> cached/staged changes:\n> .topdeps | 1 +\n> .topmsg | 6 ++++++\n> piper:~/.tmp/cdt.GydvBgiR|master|% git commit -minit #10005\n> Created commit d49ea41: init\n> 2 files changed, 7 insertions(+), 0 deletions(-)\n> create mode 100644 .topdeps\n> create mode 100644 .topmsg\nIf I interpret the 'how to reproduce' right, you have to switch to the\nmaster branch here.\n\n> piper:~/.tmp/cdt.GydvBgiR|test|% echo bar >> bar #10006\n> changes on filesystem:\n> bar | 1 +\n> piper:~/.tmp/cdt.GydvBgiR|test|% git add bar #10007\n> cached/staged changes:\n> bar | 1 +\n> piper:~/.tmp/cdt.GydvBgiR|test|% git commit -m'append' #10008\n> Created commit e85457e: append\n> 1 files changed, 1 insertions(+), 0 deletions(-)\nAnd switch back to branch test here.\n\n> piper:~/.tmp/cdt.GydvBgiR|test|% tg update #10009\n> tg: The base is up-to-date.\n> tg: The test head is up-to-date wrt. the base.\n> piper:~/.tmp/cdt.GydvBgiR|test|% git --version #10010\n> git version 1.6.0.2\n> piper:~/.tmp/cdt.GydvBgiR|test|% tg --version #10011\n> Unknown subcommand: --version\n> TopGit v0.5 - A different patch queue manager\n> Usage: tg [-r REMOTE] (create|delete|depend|export|import|info|mail|patch|remote|summary|update|help) ...\n>\nBert\n"},{"id":"104380","messageId":"20090212125621.GB5397@sigill.intra.peff.net","threadId":"17740","inReplyTo":"20090212092558.GB21074@skywalker","subject":"Re: [topgit] tg update error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-12T12:56:21Z","receivedAt":"2009-02-12T12:56:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 12, 2009 at 02:55:58PM +0530, Aneesh Kumar K.V wrote:\n\n> On Thu, Feb 12, 2009 at 09:48:11AM +0100, martin f krafft wrote:\n> > also sprach Aneesh Kumar [2009.02.12.0909 +0100]:\n> > > doing a tg update with latest git gives the below error\n> > >\n> > > [extent_validate@linux-2.6]$ tg update\n> > > fatal: Refusing to point HEAD outside of refs/heads/\n> > > [extent_validate@linux-2.6]$\n> >\n> > Which version? And could you please provide (a lot) more information\n> > about your repository or make it available?\n> >\n>\n> Latest git and topgit. Moving to git version v1.6.1.3 fixed the issue.\n> I can reproduce the problem on any test repo. Just do a tg update after\n> committing something in the dependent branch.\n\nThis error message and safety valve are not in any released version of\ngit yet. So by moving back to 1.6.1.3, you are just predating the\naddition of that message. :)\n\nI think I know what is going on. A safety valve was added in afe5d3d to\ndisallow setting HEAD to anything that would violate git's \"is this a\ngit directory\" detector:\n\n symbolic ref: refuse non-ref targets in HEAD\n\n When calling \"git symbolic-ref\" it is easy to forget that\n the target must be a fully qualified ref. E.g., you might\n accidentally do:\n\n $ git symbolic-ref HEAD master\n\n Unfortunately, this is very difficult to recover from,\n because the bogus contents of HEAD make git believe we are\n no longer in a git repository (as is_git_dir explicitly\n checks for \"^refs/heads/\" in the HEAD target). So\n immediately trying to fix the situation doesn't work:\n\n $ git symbolic-ref HEAD refs/heads/master\n fatal: Not a git repository\n\n and one is left editing the .git/HEAD file manually.\n\nReleased versions of git just check \"refs/\" in HEAD. _But_ as part of\nthis patch series, b229d18 also tightened the \"refs/\" check to\n\"refs/heads/\".\n\nSo what I suspect is happening is that topgit is trying to set HEAD to\n\"refs/top-bases/whatever\". Aneesh, can you confirm by running your test\nwith GIT_TRACE=1? I suspect you will see a call like \"git symbolic-ref\nHEAD refs/top-bases/foo\".\n\nJunio, I think we should probably revert b229d18 (and loosen\nsymbolic-ref's check to just \"refs/\"). Even if you want to argue that\ntopgit should be changed to handle this differently, we are still\nbreaking existing topgit installations, and who knows what other scripts\nwhich might have relied on doing something like this.\n\n-Peff\n"},{"id":"104381","messageId":"20090212125946.GC5397@sigill.intra.peff.net","threadId":"17740","inReplyTo":"20090212125621.GB5397@sigill.intra.peff.net","subject":"Re: [topgit] tg update error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-12T12:59:46Z","receivedAt":"2009-02-12T12:59:46Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 12, 2009 at 07:56:21AM -0500, Jeff King wrote:\n\n> So what I suspect is happening is that topgit is trying to set HEAD to\n> \"refs/top-bases/whatever\". Aneesh, can you confirm by running your test\n> with GIT_TRACE=1? I suspect you will see a call like \"git symbolic-ref\n> HEAD refs/top-bases/foo\".\n\nActually, I was able to reproduce with the recipe from Martin and Bert\nelsewhere in the thread. And that is indeed what is happening:\n\n trace: built-in: git 'symbolic-ref' 'HEAD' 'refs/top-bases/test'\n\n-Peff\n"},{"id":"104419","messageId":"20090212210135.GA24687@piper.oerlikon.madduck.net","threadId":"17740","inReplyTo":"20090212125946.GC5397@sigill.intra.peff.net","subject":"Re: [topgit] tg update error","fromName":"martin f krafft","fromEmail":"madduck@debian.org","sentAt":"2009-02-12T21:01:35Z","receivedAt":"2009-02-12T21:01:35Z","isPatch":false,"sender":{"key":"madduck@debian.org","avatar":null},"body":"also sprach Jeff King [2009.02.12.1359 +0100]:\n> > So what I suspect is happening is that topgit is trying to set HEAD to\n> > \"refs/top-bases/whatever\". Aneesh, can you confirm by running your test\n> > with GIT_TRACE=1? I suspect you will see a call like \"git symbolic-ref\n> > HEAD refs/top-bases/foo\".\n> \n> Actually, I was able to reproduce with the recipe from Martin and Bert\n> elsewhere in the thread. And that is indeed what is happening:\n> \n> trace: built-in: git 'symbolic-ref' 'HEAD'\n> 'refs/top-bases/test'\\\n\nThanks, Jeff, for the accurate analysis. Since I do not see a way\nfor topgit to do things differently -- top-bases are *not* heads and\nthus warrant a different namespace -- can I assume that this is to\nbe fixed in Git and not in TopGit?\n\n-- \n .''`. martin f. krafft Related projects:\n: :' : proud Debian developer http://debiansystem.info\n`. `'` http://people.debian.org/~madduck http://vcs-pkg.org\n `- Debian - when you have better things to do than fixing systems\n \n\"it is only the modern that ever becomes old-fashioned.\"\n -- oscar wilde\n"},{"id":"104418","messageId":"7veiy3l689.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"20090212125621.GB5397@sigill.intra.peff.net","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-12T21:01:42Z","receivedAt":"2009-02-12T21:01:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Junio, I think we should probably revert b229d18 (and loosen\n> symbolic-ref's check to just \"refs/\"). Even if you want to argue that\n> topgit should be changed to handle this differently, we are still\n> breaking existing topgit installations, and who knows what other scripts\n> which might have relied on doing something like this.\n\nI'm Ok with the revert (and I agree it is absolutely the right thing to do\nat least for the short term).\n\nBut I still do agree with the reasoning for the change stated in its\ncommit log message:\n\n commit b229d18a809c169314b7f0d048dc5a7632e8f916\n Author: Jeff King \n Date: Thu Jan 29 03:30:16 2009 -0500\n\n validate_headref: tighten ref-matching to just branches\n\n When we are trying to determine whether a directory contains\n a git repository, one of the tests we do is to check whether\n HEAD is either a symlink or a symref into the \"refs/\"\n hierarchy, or a detached HEAD.\n\n We can tighten this a little more, though: a non-detached\n HEAD should always point to a branch (since checking out\n anything else should result in detachment), so it is safe to\n check for \"refs/heads/\".\n\n Signed-off-by: Jeff King \n Signed-off-by: Junio C Hamano \n\nIt would be nice to hear TopGit people defend why setting HEAD to outside\nrefs/heads/ is justified, why doing so should not break other things, and\nwhy it was needed.\n\nThe last one is particularly important to avoid this kind of issue in the\nfuture. Perhaps they _knew_ some things refuse to work on refs outside\nrefs/heads/ and wanted to take advantage of that fact to protect their own\nrefs from vanilla git tools, but if that really is the case, the rules\nthey want have to be spelled out.\n\n\"git checkout\" would refuse to switch to \"refs/top-bases/frotz\", because\nit currently considers HEAD pointing outside refs/heads/ is insane, for\nexample. But the revert of the above commit *means* that it is not\ninsane, and somebody may add an option to switch to any refs inside refs/\nhierarchy. If the reason TopGit points HEAD outside refs/heads hierarchy\nwere because they assume \"git checkout\" would never do so, such a change\nwould break them again (I am not seriously suggesting to add such an\noption to \"git checkout\", but I am just using it to illustrate the point.\nWe would not know what other assumption, warranted or unwarranted, it is\nmaking).\n"},{"id":"104433","messageId":"20090212214106.GC26573@piper.oerlikon.madduck.net","threadId":"17740","inReplyTo":"7veiy3l689.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"martin f krafft","fromEmail":"madduck@debian.org","sentAt":"2009-02-12T21:41:06Z","receivedAt":"2009-02-12T21:41:06Z","isPatch":false,"sender":{"key":"madduck@debian.org","avatar":null},"body":"also sprach Junio C Hamano [2009.02.12.2201 +0100]:\n> It would be nice to hear TopGit people defend why setting HEAD to outside\n> refs/heads/ is justified, why doing so should not break other things, and\n> why it was needed.\n\nAs far as I understand it, TopGit does not /need/ to set HEAD to\nrefs/top-bases/foo, but it currently does so as part of its\nalgorithm:\n\nWhen tg-update updates a depending branch, it first merges the\ndependent branch into the base of the topic branch, which is pointed\nto by the corresponding top-base (refs/top-bases/foo). It then\nmerges the top-base into the topic branch, \"foo\" in this case.\n\nThe result is the same as if the base branch had been merged into\n\"foo\", and refs/top-bases/foo updated to point to the head of the\nbase branch.\n\nThis stops working, however, as soon as you have a topic branch\ndepending on more than one base branches. Since you need to track\nthe base of a topic branch (e.g. in order to be able to get the diff\nrepresented by the TopGit branch), you now have a problem: which of\nthe base branches is the base to diff against?\n\nTopGit addresses this requirement by creating a \"virtual\" branch\ninto which it merges all the bases (into the top-base) first, and\nthen merging this \"virtual\" branch into the topic branch. The result\nis a merge commit combining all bases, which is a parent of the\nmerge commit into the topic branch, and can thus serve as the origin\nof a diff calculation.\n\nTopGit right now does all of this while HEAD is detached: it points\ninto the refs/top-bases/* namespace -- the \"virtual\" branch. Here,\nit does the merges of the bases, and then checks out the topic\nbranch to merge this combined (\"virtual\") base.\n\nTo work around the new restriction in Git, TopGit would need to make\na proper branch, merge the bases into it, merge that branch into the\ntopic branch, and the probably delete the branch pointer, as it's no\nlonger needed and would only pollute the refs/heads/* namespace. It\ncould certainly do this (with a minor performance impact), but it\nseems like jumping through hoops and around Git's restrictions,\nwithout any real benefit.\n\nPoint being: I understand the reason behind the restriction, and\nI wouldn't mind if it were default, but maybe there could be\na controlled way to circumvent it for cases like the one described\nabove, where it is safe to assume that the user^W^W the tool \"knows\"\nwhat it is doing.\n\n-- \n .''`. martin f. krafft Related projects:\n: :' : proud Debian developer http://debiansystem.info\n`. `'` http://people.debian.org/~madduck http://vcs-pkg.org\n `- Debian - when you have better things to do than fixing systems\n \nin the beginning was the word,\nand the word was content-type: text/plain\n"},{"id":"104447","messageId":"7vocx7i6xh.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"20090212214106.GC26573@piper.oerlikon.madduck.net","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-12T23:14:50Z","receivedAt":"2009-02-12T23:14:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"martin f krafft writes:\n\n> TopGit would need to make\n> a proper branch, merge the bases into it, merge that branch into the\n> topic branch, and the probably delete the branch pointer, as it's no\n> longer needed and would only pollute the refs/heads/* namespace.\n\nSo it happens purely inside TopGit and the end user never sees a state\nthat HEAD points outside refs/heads/, right?\n\nWhy can't the base flipping operation you descibed be done on detached\nHEAD? Perhaps with a shell variable or two that hold commit object names\nyou need to keep track of while it is doing is work?\n\n> Point being: I understand the reason behind the restriction, and\n> I wouldn't mind if it were default, but maybe there could be\n> a controlled way to circumvent it for cases like the one described\n> above, where it is safe to assume that the user^W^W the tool \"knows\"\n> what it is doing.\n\nSure, the tool would know what it is doing, I wouldn't doubt that.\n\nBut the end users don't. If TopGit dies (or killed) during the base\nflipping operation, doesn't the end user left in a funny state (granted, a\ndetached HEAD is also a funny state, but it is already a known funny state\nthey are familiar with. HEAD that is a symref but points outside\nrefs/heads/ is a lot funnier).\n\nYou did not actually answer a larger question. What other undocumented\nfeatures/restrictions does the code depend on, that tightening them to\nhelp normal git users inadvertently may cause breakages similar to this\none in TopGit?\n"},{"id":"104479","messageId":"20090213062818.GB16434@piper.oerlikon.madduck.net","threadId":"17740","inReplyTo":"7vocx7i6xh.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"martin f krafft","fromEmail":"madduck@debian.org","sentAt":"2009-02-13T06:28:18Z","receivedAt":"2009-02-13T06:28:18Z","isPatch":false,"sender":{"key":"madduck@debian.org","avatar":null},"body":"also sprach Junio C Hamano [2009.02.13.0014 +0100]:\n> > TopGit would need to make a proper branch, merge the bases into\n> > it, merge that branch into the topic branch, and the probably\n> > delete the branch pointer, as it's no longer needed and would\n> > only pollute the refs/heads/* namespace.\n> \n> So it happens purely inside TopGit and the end user never sees\n> a state that HEAD points outside refs/heads/, right?\n\nYes.\n\n> Why can't the base flipping operation you descibed be done on\n> detached HEAD? Perhaps with a shell variable or two that hold\n> commit object names you need to keep track of while it is doing is\n> work?\n\nI am not sure I understand. Isn't that what's currently happening?\n\nHave a look at line 110 of tg-update.sh:\n\n http://git.debian.org/?p=collab-maint/topgit.git;a=blob;f=tg-update.sh;hb=HEAD#l110\n\n> But the end users don't. If TopGit dies (or killed) during the base\n> flipping operation, doesn't the end user left in a funny state (granted, a\n> detached HEAD is also a funny state, but it is already a known funny state\n> they are familiar with. HEAD that is a symref but points outside\n> refs/heads/ is a lot funnier).\n\nIf topgit is killed, yes, then the repo could be left in a funny\nstate. I suppose this could be addressed by putting proper traps in\nplace.\n\nIf the merge fails, however, then the user is advised what to do;\nsee lines 114ff.\n\n> You did not actually answer a larger question.\n\nIt wasn't asked to me before... ;)\n\n> What other undocumented features/restrictions does the code depend\n> on, that tightening them to help normal git users inadvertently\n> may cause breakages similar to this one in TopGit?\n\nI think Petr would need to help out answering this.\n\nI agree that it would be good to address each such occurrence in\nturn and replace it with a method that only makes use of the public\nAPI. Up until now, however,\n\n git checkout -q \"refs/top-bases/$name\"\n\nwas not really something undocmented or restricted. I find it rather\ndifficult to separate\npublic-as-in-every-user-can-and-should-use-this features from\nrestricted-better-be-left-alone-unless-you-really-know-what-you-are-doing\nfeatures with Git. This has gotten *a lot* better, but the fact that\nI can still call e.g. git update-ref (as opposed to e.g. git\n_update-ref) and potentially turn my repository upside down\nexemplifies this.\n\nMaybe Petr remembers all the instances when he sneakily used tricks\nto make things work, and then we can look at each of them in turn.\n\nMaybe some of you could go through the code (which isn't /that/\nmuch), looking for instances of not-so-public API abuse and help us\nidentify them too.\n\nCheers,\n\n-- \n .''`. martin f. krafft Related projects:\n: :' : proud Debian developer http://debiansystem.info\n`. `'` http://people.debian.org/~madduck http://vcs-pkg.org\n `- Debian - when you have better things to do than fixing systems\n \n\"without music, life would be a mistake.\"\n - friedrich nietzsche\n"},{"id":"104486","messageId":"7vmycqeqqh.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"20090213062818.GB16434@piper.oerlikon.madduck.net","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T07:32:54Z","receivedAt":"2009-02-13T07:32:54Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"martin f krafft writes:\n\n> also sprach Junio C Hamano [2009.02.13.0014 +0100]:\n>> > TopGit would need to make a proper branch, merge the bases into\n>> > it, merge that branch into the topic branch, and the probably\n>> > delete the branch pointer, as it's no longer needed and would\n>> > only pollute the refs/heads/* namespace.\n>> \n>> So it happens purely inside TopGit and the end user never sees\n>> a state that HEAD points outside refs/heads/, right?\n>\n> Yes.\n\nNow I am confused by your answers. This \"Yes\" means that setting HEAD\noutside refs/heads/ happens purely as an intermediate state to avoid\nsetting HEAD to some branch ref. After the operation finishes correctly,\nHEAD will never be left outside refs/heads/. But this contradicts\ndirectly with what you say next.\n\n>> Why can't the base flipping operation you descibed be done on\n>> detached HEAD? Perhaps with a shell variable or two that hold\n>> commit object names you need to keep track of while it is doing is\n>> work?\n>\n> I am not sure I understand. Isn't that what's currently happening?\n\nIf you *are* setting HEAD to some ref that is outside refs/heads (or even\ninside refs/heads for that matter), at that point the HEAD is *not*\ndetached, so no, it obviously is *not* what is happening.\n\nI am asking why you need to use a ref to do that, *if* it is a tentative\nstate while the program is running. You are probably calling a git\nplumbing or Porcelain command that updates HEAD, and the reason why you\npoint HEAD outside refs/heads/ is beause you would want the command you\ncall to update one of the refs/top-bases/ ref through HEAD. I am asking\nwhy you are not running these commands on a normal detached HEAD, and then\nuse update-ref (not symbolic-ref) plumbing to update the refs/top-bases/\nref you would want to update when it is done.\n\n>> You did not actually answer a larger question.\n>\n> It wasn't asked to me before... ;)\n\nGo back to the original message and read it again.\n\n> Up until now, however,\n>\n> git checkout -q \"refs/top-bases/$name\"\n>\n> was not really something undocmented or restricted.\n\nGiving checkout anything that is not \"a branch name\" meant detaching HEAD\never since detached HEAD was introduced, and that is a documented feature.\ngit checkout \"refs/heads/master\" would behave the same way --- it won't\ncheck out the 'master' branch.\n\n> I can still call e.g. git update-ref (as opposed to e.g. git\n> _update-ref) and potentially turn my repository upside down\n> exemplifies this.\n\nThe distinction between Porcelain and plumbing is unfortunately not very\nclear at places. The change we reverted was probably a bad one. The\nstricter check was not done at the Porcelain level but was done at the\nplumbing level.\n"},{"id":"104502","messageId":"7v63jebtdb.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"7vmycqeqqh.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-13T09:04:16Z","receivedAt":"2009-02-13T09:04:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> If you *are* setting HEAD to some ref that is outside refs/heads (or even\n> inside refs/heads for that matter), at that point the HEAD is *not*\n> detached, so no, it obviously is *not* what is happening.\n>\n> I am asking why you need to use a ref to do that, *if* it is a tentative\n> state while the program is running. You are probably calling a git\n> plumbing or Porcelain command that updates HEAD, and the reason why you\n> point HEAD outside refs/heads/ is beause you would want the command you\n> call to update one of the refs/top-bases/ ref through HEAD. I am asking\n> why you are not running these commands on a normal detached HEAD, and then\n> use update-ref (not symbolic-ref) plumbing to update the refs/top-bases/\n> ref you would want to update when it is done.\n\nOk, I did read the script (yuck). You do break out of TopGit process when\na merge conflict prevents the update operation to complete and do give\ncontrol back to the end user, so you can leave HEAD in a state that points\nat a non-branch, and you do use the fact that the HEAD is pointing at\nsomething funny as a sign that you are in the middle of conflicted merge\nresolution.\n\nIt is just like how vanilla git uses MERGE_HEAD as the marker to signal\nthat it is in a funny state.\n\nWhile I think it is a cute idea to use which funny hierarchy HEAD points\nat to indicate what funny/intermediate state your interrupted operation is\nin, and it may seem to be cleaner than using a marker file like MERGE_HEAD\nat first sight, I do not think it is a wise thing to do in the long run.\n\nYou can only express two pieces of information (the overall \"category of\nstate\" by which funny ref/ hierarchy HEAD points at, and one object name\nby storing it in the ref pointed at by HEAD), and if you need more (such\nas MERGE_MSG that stores pre-packaged log message pieces is used during a\nmerge, in addition to MERGE_HEAD), you would need to use more than just\nthe \"cute HEAD\" trick to store them *anyway*. Which means that it is a\nbad tradeoff to use \"cute HEAD\" --- it closes the possibility to detect\nuser error to point HEAD at an incorrect place and I do not see the\nbenefit of \"cute HEAD\" outweigh the downside.\n"},{"id":"104540","messageId":"20090213182609.GB31860@coredump.intra.peff.net","threadId":"17740","inReplyTo":"7veiy3l689.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-13T18:26:09Z","receivedAt":"2009-02-13T18:26:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Feb 12, 2009 at 01:01:42PM -0800, Junio C Hamano wrote:\n\n> > Junio, I think we should probably revert b229d18 (and loosen\n> > symbolic-ref's check to just \"refs/\"). Even if you want to argue that\n> > topgit should be changed to handle this differently, we are still\n> > breaking existing topgit installations, and who knows what other scripts\n> > which might have relied on doing something like this.\n> \n> I'm Ok with the revert (and I agree it is absolutely the right thing to do\n> at least for the short term).\n\nIt looks like you have already pushed out the revert. But I think we\nneed this on top to make topgit work correctly.\n\n-- >8 --\nSubject: [PATCH] symbolic-ref: allow refs/ in HEAD\n\nCommit afe5d3d5 introduced a safety valve to symbolic-ref to\ndisallow installing an invalid HEAD. It was accompanied by\nb229d18a, which changed validate_headref to require that\nHEAD contain a pointer to refs/heads/ instead of just refs/.\nTherefore, the safety valve also checked for refs/heads/.\n\nAs it turns out, topgit is using refs/top-bases/ in HEAD,\nleading us to re-loosen (at least temporarily) the\nvalidate_headref check made in b229d18a. This patch does the\ncorresponding loosening for the symbolic-ref safety valve,\nso that the two are in agreement once more.\n\nSigned-off-by: Jeff King \n---\n builtin-symbolic-ref.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c\nindex cafc4eb..6ae6bcc 100644\n--- a/builtin-symbolic-ref.c\n+++ b/builtin-symbolic-ref.c\n@@ -45,8 +45,8 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase 2:\n \t\tif (!strcmp(argv[0], \"HEAD\") &&\n-\t\t prefixcmp(argv[1], \"refs/heads/\"))\n-\t\t\tdie(\"Refusing to point HEAD outside of refs/heads/\");\n+\t\t prefixcmp(argv[1], \"refs/\"))\n+\t\t\tdie(\"Refusing to point HEAD outside of refs/\");\n \t\tcreate_symref(argv[0], argv[1], msg);\n \t\tbreak;\n \tdefault:\n-- \n1.6.2.rc0.241.g088a\n"},{"id":"104567","messageId":"7vy6w93hdb.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"20090213182609.GB31860@coredump.intra.peff.net","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T02:02:56Z","receivedAt":"2009-02-14T02:02:56Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Thu, Feb 12, 2009 at 01:01:42PM -0800, Junio C Hamano wrote:\n>\n>> > Junio, I think we should probably revert b229d18 (and loosen\n>> > symbolic-ref's check to just \"refs/\"). Even if you want to argue that\n>> > topgit should be changed to handle this differently, we are still\n>> > breaking existing topgit installations, and who knows what other scripts\n>> > which might have relied on doing something like this.\n>> \n>> I'm Ok with the revert (and I agree it is absolutely the right thing to do\n>> at least for the short term).\n>\n> It looks like you have already pushed out the revert. But I think we\n> need this on top to make topgit work correctly.\n\n>\n> -- >8 --\n> Subject: [PATCH] symbolic-ref: allow refs/ in HEAD\n>\n> Commit afe5d3d5 introduced a safety valve to symbolic-ref to\n> disallow installing an invalid HEAD. It was accompanied by\n> b229d18a, which changed validate_headref to require that\n> HEAD contain a pointer to refs/heads/ instead of just refs/.\n> Therefore, the safety valve also checked for refs/heads/.\n>\n> As it turns out, topgit is using refs/top-bases/ in HEAD,\n> leading us to re-loosen (at least temporarily) the\n> validate_headref check made in b229d18a. This patch does the\n> corresponding loosening for the symbolic-ref safety valve,\n> so that the two are in agreement once more.\n>\n> Signed-off-by: Jeff King \n\nActually we should simply revert afe5d3d5 altogether with the above\nmessage, as it introduced a test that expects the tightened behaviour.\n"},{"id":"104569","messageId":"20090214020848.GA9907@coredump.intra.peff.net","threadId":"17740","inReplyTo":"7vy6w93hdb.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T02:08:48Z","receivedAt":"2009-02-14T02:08:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 06:02:56PM -0800, Junio C Hamano wrote:\n\n> > As it turns out, topgit is using refs/top-bases/ in HEAD,\n> > leading us to re-loosen (at least temporarily) the\n> > validate_headref check made in b229d18a. This patch does the\n> > corresponding loosening for the symbolic-ref safety valve,\n> > so that the two are in agreement once more.\n> >\n> > Signed-off-by: Jeff King \n> \n> Actually we should simply revert afe5d3d5 altogether with the above\n> message, as it introduced a test that expects the tightened behaviour.\n\nIs there any reason to throw away the \"must be in refs/\" safety valve,\nthough? That was the actual patch I started with and solved my problem,\nand the \"tighten to refs/heads/\" bit came from discussion. That is, I\nthink having a safety valve in symbolic-ref that matches\nvalidate_headref is orthogonal to how tightly validate_headref matches.\n\nBut yes, I obviously failed to run the test suite on the follow-up patch\nI sent. The final test in t1401 would need to be reverted, as well.\n\n-Peff\n"},{"id":"104571","messageId":"7vocx53gqw.fsf@gitster.siamese.dyndns.org","threadId":"17740","inReplyTo":"20090214020848.GA9907@coredump.intra.peff.net","subject":"Re: [topgit] tg update error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-14T02:16:23Z","receivedAt":"2009-02-14T02:16:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Feb 13, 2009 at 06:02:56PM -0800, Junio C Hamano wrote:\n>\n>> > As it turns out, topgit is using refs/top-bases/ in HEAD,\n>> > leading us to re-loosen (at least temporarily) the\n>> > validate_headref check made in b229d18a. This patch does the\n>> > corresponding loosening for the symbolic-ref safety valve,\n>> > so that the two are in agreement once more.\n>> >\n>> > Signed-off-by: Jeff King \n>> \n>> Actually we should simply revert afe5d3d5 altogether with the above\n>> message, as it introduced a test that expects the tightened behaviour.\n>\n> Is there any reason to throw away the \"must be in refs/\" safety valve,\n> though? That was the actual patch I started with and solved my problem,\n> and the \"tighten to refs/heads/\" bit came from discussion. That is, I\n> think having a safety valve in symbolic-ref that matches\n> validate_headref is orthogonal to how tightly validate_headref matches.\n>\n> But yes, I obviously failed to run the test suite on the follow-up patch\n> I sent. The final test in t1401 would need to be reverted, as well.\n\nSure.\n"},{"id":"104574","messageId":"20090214022436.GC9907@coredump.intra.peff.net","threadId":"17740","inReplyTo":"7vocx53gqw.fsf@gitster.siamese.dyndns.org","subject":"Re: [topgit] tg update error","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-14T02:24:36Z","receivedAt":"2009-02-14T02:24:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 13, 2009 at 06:16:23PM -0800, Junio C Hamano wrote:\n\n> > Is there any reason to throw away the \"must be in refs/\" safety valve,\n> > though? That was the actual patch I started with and solved my problem,\n> > and the \"tighten to refs/heads/\" bit came from discussion. That is, I\n> > think having a safety valve in symbolic-ref that matches\n> > validate_headref is orthogonal to how tightly validate_headref matches.\n> >\n> > But yes, I obviously failed to run the test suite on the follow-up patch\n> > I sent. The final test in t1401 would need to be reverted, as well.\n> \n> Sure.\n\nOK, here is the updated patch (that actually passes the test suite).\n\n-- >8 --\nSubject: [PATCH] symbolic-ref: allow refs/ in HEAD\n\nCommit afe5d3d5 introduced a safety valve to symbolic-ref to\ndisallow installing an invalid HEAD. It was accompanied by\nb229d18a, which changed validate_headref to require that\nHEAD contain a pointer to refs/heads/ instead of just refs/.\n\nAs it turns out, topgit is using refs/top-bases/ in HEAD,\nleading us to re-loosen (at least temporarily) the\nvalidate_headref check made in b229d18a. This patch does the\ncorresponding loosening for the symbolic-ref safety check,\nso that the two are in agreement once more.\n\nSigned-off-by: Jeff King \n---\n builtin-symbolic-ref.c | 4 ++--\n t/t1401-symbolic-ref.sh | 5 -----\n 2 files changed, 2 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c\nindex cafc4eb..6ae6bcc 100644\n--- a/builtin-symbolic-ref.c\n+++ b/builtin-symbolic-ref.c\n@@ -45,8 +45,8 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)\n \t\tbreak;\n \tcase 2:\n \t\tif (!strcmp(argv[0], \"HEAD\") &&\n-\t\t prefixcmp(argv[1], \"refs/heads/\"))\n-\t\t\tdie(\"Refusing to point HEAD outside of refs/heads/\");\n+\t\t prefixcmp(argv[1], \"refs/\"))\n+\t\t\tdie(\"Refusing to point HEAD outside of refs/\");\n \t\tcreate_symref(argv[0], argv[1], msg);\n \t\tbreak;\n \tdefault:\ndiff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh\nindex 569f341..7fa5f5b 100755\n--- a/t/t1401-symbolic-ref.sh\n+++ b/t/t1401-symbolic-ref.sh\n@@ -27,11 +27,6 @@ test_expect_success 'symbolic-ref refuses non-ref for HEAD' '\n '\n reset_to_sane\n \n-test_expect_success 'symbolic-ref refuses non-branch for HEAD' '\n-\ttest_must_fail git symbolic-ref HEAD refs/foo\n-'\n-reset_to_sane\n-\n test_expect_success 'symbolic-ref refuses bare sha1' '\n \techo content >file && git add file && git commit -m one\n \ttest_must_fail git symbolic-ref HEAD `git rev-parse HEAD`\n-- \n1.6.2.rc0.241.g088a\n"}]}