{"thread":{"id":"28142","subject":"Malformed branch name in fast-export when specifying non-HEAD/branch revision","startedAt":"2011-08-17T16:21:34Z","lastAt":"2011-08-22T21:32:14Z","messageCount":11,"participants":["Owen Stephens","Elijah Newren","Junio C Hamano","Jeff King","Sverre Rabbelier"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"173679","messageId":"CAORuUR154Dhg5vDojga-01bDxxf+=R2X-oJK-0417CgmqxCwvA@mail.gmail.com","threadId":"28142","inReplyTo":"CAORuUR1viqG27+dYOFS_5SLxFOE2wHJqAQ3i3RByg_fbWACh-Q@mail.gmail.com","subject":"Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Owen Stephens","fromEmail":"git@owenstephens.co.uk","sentAt":"2011-08-17T16:21:34Z","receivedAt":"2011-08-17T16:21:34Z","isPatch":false,"sender":{"key":"git@owenstephens.co.uk","avatar":null},"body":"Hi,\n\nfast-export gives an invalid branch name for commits, if the revision specified\nis not HEAD or a branch name.\n\n$ git --version\ngit version 1.7.6\n\n$ # Create\n$ git init export_test && cd export_test\n$ touch a && git add a && git commit -m 'Add a'\n$ touch b && git add b && git commit -m 'Add b'\n$ git branch branch1\n\n$ # ok\n$ git fast-export HEAD\n\n$ # also ok\n$ git fast-export branch1\n\n$ # uses HEAD~1 instead of refs/heads/master\n$ git fast-export HEAD~1\n\nblob\nmark :1\ndata 0\n\nreset HEAD~1\ncommit HEAD~1\nmark :2\nauthor Owen Stephens 1313597436 +0100\ncommitter Owen Stephens 1313597436 +0100\ndata 6\nAdd a\nM 100644 :1 a\n\n$ # Uses the commit hash rather than refs/heads/master\n$ git fast-export $(git rev-parse HEAD~1)\n\nblob\nmark :1\ndata 0\n\nreset 5a8a8abb1ab44890501c64f2d51f671ab2108d60\ncommit 5a8a8abb1ab44890501c64f2d51f671ab2108d60\nmark :2\nauthor Owen Stephens 1313597436 +0100\ncommitter Owen Stephens 1313597436 +0100\ndata 6\nAdd a\nM 100644 :1 a\n\n$ # fast-import cannot handle this stream:\n$ git init subgit\n$ git fast-export HEAD~1 | (cd subgit/ && git fast-import)\nfatal: Branch name doesn't conform to GIT standards: HEAD~1\nfast-import: dumping crash report to .git/fast_import_crash_26863\n\nRegards,\nOwen.\n"},{"id":"173689","messageId":"CABPp-BFRZMZjhWuUUeD7Oa1HbWQMnZot7dRm3zKOpCoj_QwZeg@mail.gmail.com","threadId":"28142","inReplyTo":"CAORuUR154Dhg5vDojga-01bDxxf+=R2X-oJK-0417CgmqxCwvA@mail.gmail.com","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2011-08-17T19:36:32Z","receivedAt":"2011-08-17T19:36:32Z","isPatch":false,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Hi,\n\nOn Wed, Aug 17, 2011 at 10:21 AM, Owen Stephens wrote:\n> fast-export gives an invalid branch name for commits, if the revision specified\n> is not HEAD or a branch name.\n>\n> $ git --version\n> git version 1.7.6\n>\n> $ # Create\n> $ git init export_test && cd export_test\n> $ touch a && git add a && git commit -m 'Add a'\n> $ touch b && git add b && git commit -m 'Add b'\n> $ git branch branch1\n>\n> $ # ok\n> $ git fast-export HEAD\n>\n> $ # also ok\n> $ git fast-export branch1\n>\n> $ # uses HEAD~1 instead of refs/heads/master\n> $ git fast-export HEAD~1\n>\n> blob\n> mark :1\n> data 0\n>\n> reset HEAD~1\n> commit HEAD~1\n\nThanks for the report. It turns out this bug has been reported and is\nin the testsuite as t9350.19 -- currently marked as expected to fail.\nI looked at the problem a couple years ago for a little bit but never\nfinished that particular patch and never got back around to it.\n"},{"id":"173701","messageId":"7vliurd62x.fsf@alter.siamese.dyndns.org","threadId":"28142","inReplyTo":"CABPp-BFRZMZjhWuUUeD7Oa1HbWQMnZot7dRm3zKOpCoj_QwZeg@mail.gmail.com","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-17T22:30:14Z","receivedAt":"2011-08-17T22:30:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren writes:\n\n>> $ # uses HEAD~1 instead of refs/heads/master\n>> $ git fast-export HEAD~1\n>>\n>> blob\n>> mark :1\n>> data 0\n>>\n>> reset HEAD~1\n>> commit HEAD~1\n>\n> Thanks for the report. It turns out this bug has been reported and is\n> in the testsuite as t9350.19 -- currently marked as expected to fail.\n> I looked at the problem a couple years ago for a little bit but never\n> finished that particular patch and never got back around to it.\n\nWhat _should_ be the right behaviour to begin with, I have to wonder.\n\nEven though it is very clear that the set of objects that are exported are\ndefined by the \"rev-list arguments\" given to the command, I do not think\nfast-export's semantics is not clearly defined as to what \"refs\" are to be\nupdated.\n\nThe easiest fix for this issue would be to forbid \"git fast-export HEAD~1\"\n(or any range whose positive endpoints are _not_ refs), and I think that\nwould be in line with the original motivation of the command to export the\nwhole repository in a format fast-import would understand. The original\nf2dc849 (Add 'git fast-export', the sister of 'git fast-import',\n2007-12-02) says \"This program dumps (parts of) a git repository...\",\nimplying that partial export is within the scope of the command, but I do\nnot think it was designed carefully enough to deal with ranges more\ncomplex than just \"a set of branches\".\n\nI however have a feeling that people would want to say:\n\n - I want to export up to that commit, and have that commit on this branch\n on the importing side; or even better\n\n - I want to export up to that commit, but what refs points at the commits\n contained in the output stream will be decided when the output is\n imported.\n\nI do not think the latter meshes well with how \"fast-import\" works,\nthough. But fast-export should be fixable to allow the former without\nbreaking the semantics of fast-import.\n\nYou can think of \"fast-export\" an off-line \"push\" command [*1*]; instead\nof giving a random commit object, e.g. \"git fast-export HEAD~1\", that can\nnot be used as a ref, you can use the refspec notation to tell where the\nresult should go, e.g. \"git fast-export HEAD~1:refs/heads/a-bit-older\",\nfrom the command line of fast-export.\n\nI suspect that also may clarify what Sverre was trying to do in his recent\nseries. The root cause of both this and the issue Sverre wanted to fix is\nthe design mistake of fast-export that tries to reuse the notation of\nobject range specification for a different purpose of telling which \"ref\"\nto update, I think.\n\n[Footnote]\n\n*1* In a similar sense, unpacking \"git bundle\" output is an off-line\n\"fetch\"; the bundle creator gave anchor points for tip objects, and allows\nthe unpacker to map them into its own namespace.\n"},{"id":"173703","messageId":"20110817231922.GA28966@sigill.intra.peff.net","threadId":"28142","inReplyTo":"7vliurd62x.fsf@alter.siamese.dyndns.org","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-17T23:19:22Z","receivedAt":"2011-08-17T23:19:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 17, 2011 at 03:30:14PM -0700, Junio C Hamano wrote:\n\n> You can think of \"fast-export\" an off-line \"push\" command [*1*]; instead\n> of giving a random commit object, e.g. \"git fast-export HEAD~1\", that can\n> not be used as a ref, you can use the refspec notation to tell where the\n> result should go, e.g. \"git fast-export HEAD~1:refs/heads/a-bit-older\",\n> from the command line of fast-export.\n> \n> I suspect that also may clarify what Sverre was trying to do in his recent\n> series. The root cause of both this and the issue Sverre wanted to fix is\n> the design mistake of fast-export that tries to reuse the notation of\n> object range specification for a different purpose of telling which \"ref\"\n> to update, I think.\n\nYes, this was the conclusion I came to when I looked at this a month or\nso ago. You really need to give fast-export a mapping of objects to\nrefnames, and it should output ref names _only_ for the mapping. That\nwould handle this \"not a ref\" case, but would also let you push\n\"refs/heads/foo\" when it is equivalent to \"refs/heads/master\", without\nfast-export mentioning \"refs/heads/master\" at all.\n\n-Peff\n"},{"id":"173975","messageId":"CAGdFq_iiTt8F+kPXwZZT3fAKwZLCpPr7BOYtistxvv6s52Q5nQ@mail.gmail.com","threadId":"28142","inReplyTo":"20110817231922.GA28966@sigill.intra.peff.net","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-08-21T22:29:38Z","receivedAt":"2011-08-21T22:29:38Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Wed, Aug 17, 2011 at 16:19, Jeff King wrote:\n> On Wed, Aug 17, 2011 at 03:30:14PM -0700, Junio C Hamano wrote:\n>\n>> You can think of \"fast-export\" an off-line \"push\" command [*1*]; instead\n>> of giving a random commit object, e.g. \"git fast-export HEAD~1\", that can\n>> not be used as a ref, you can use the refspec notation to tell where the\n>> result should go, e.g. \"git fast-export HEAD~1:refs/heads/a-bit-older\",\n>> from the command line of fast-export.\n>>\n>> I suspect that also may clarify what Sverre was trying to do in his recent\n>> series. The root cause of both this and the issue Sverre wanted to fix is\n>> the design mistake of fast-export that tries to reuse the notation of\n>> object range specification for a different purpose of telling which \"ref\"\n>> to update, I think.\n>\n> Yes, this was the conclusion I came to when I looked at this a month or\n> so ago. You really need to give fast-export a mapping of objects to\n> refnames, and it should output ref names _only_ for the mapping. That\n> would handle this \"not a ref\" case, but would also let you push\n> \"refs/heads/foo\" when it is equivalent to \"refs/heads/master\", without\n> fast-export mentioning \"refs/heads/master\" at all.\n\nDoes this bring any new insights into how the problem I was pointing\nout (trying to push next if master points at the same commit does\nnothing) could/should be solved?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"174024","messageId":"20110822161932.GA1945@sigill.intra.peff.net","threadId":"28142","inReplyTo":"CAGdFq_iiTt8F+kPXwZZT3fAKwZLCpPr7BOYtistxvv6s52Q5nQ@mail.gmail.com","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-22T16:19:32Z","receivedAt":"2011-08-22T16:19:32Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 21, 2011 at 03:29:38PM -0700, Sverre Rabbelier wrote:\n\n> > Yes, this was the conclusion I came to when I looked at this a month or\n> > so ago. You really need to give fast-export a mapping of objects to\n> > refnames, and it should output ref names _only_ for the mapping. That\n> > would handle this \"not a ref\" case, but would also let you push\n> > \"refs/heads/foo\" when it is equivalent to \"refs/heads/master\", without\n> > fast-export mentioning \"refs/heads/master\" at all.\n> \n> Does this bring any new insights into how the problem I was pointing\n> out (trying to push next if master points at the same commit does\n> nothing) could/should be solved?\n\nHmm. Maybe I am misremembering the problem, but I thought that worked\nalready. If you say:\n\n git fast-export refs/heads/foo\n\nyou should get only reset/commit lines in the output for refs/heads/foo,\nno?\n\nNow I can't seem to replicate the case where refs/heads/master is\nmentioned, but you didn't want it to be. I may have to go back and\nre-read the thread from a month or two ago when we discussed these\nissues.\n\n-Peff\n"},{"id":"174025","messageId":"CAGdFq_gx+NutDCU7egr_fYUvVG=g1t76LiV1SV-W=0KWBTjHyQ@mail.gmail.com","threadId":"28142","inReplyTo":"20110822161932.GA1945@sigill.intra.peff.net","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-08-22T16:54:47Z","receivedAt":"2011-08-22T16:54:47Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 22, 2011 at 09:19, Jeff King wrote:\n> Hmm. Maybe I am misremembering the problem, but I thought that worked\n> already. If you say:\n>\n>  git fast-export refs/heads/foo\n>\n> you should get only reset/commit lines in the output for refs/heads/foo,\n> no?\n>\n> Now I can't seem to replicate the case where refs/heads/master is\n> mentioned, but you didn't want it to be. I may have to go back and\n> re-read the thread from a month or two ago when we discussed these\n> issues.\n\nDo you agree that this is expected behavior?\n\n$ git init test\nInitialized empty Git repository in /home/sverre/code/test/.git/\n$ cd test/\n$ echo content >> foo\nsverre@laptop-sverre:~/code/test\n$ git add foo\n$ git commit -m first\n[master (root-commit) 821176f] first\n 1 files changed, 1 insertions(+), 0 deletions(-)\n create mode 100644 foo\n$ echo content >> foo\n$ git commit -am second\n[master 1934282] second\n 1 files changed, 1 insertions(+), 0 deletions(-)\n$ git branch other\n$ git fast-export ^master other\nreset refs/heads/other\nfrom 1934282469e3a83a5ef827fd31e074cfb4f3eadf\n\nBecause in current git.git, this doesn't work (the above is generated\nusing a git that has the patch series Dscho and I sent out). Current\ngit will instead do the following:\n\n$ git fast-export ^master other\nreset refs/heads/other\nfrom :0\n\nThe 'from :0' here is obviously a bug (which is fixed by our patch series).\n\nYou might wonder, 'why would anyone do that', well, for example, they\nmight be using marks:\n\n$ git fast-export --export-marks=marksfile master > /dev/null\n$ git fast-export --import-marks=marksfile other\nreset refs/heads/other\nfrom :4\n\nAgain, the above is generated with my patched git, current git.git\nsimply outputs nothing.\n\n$ git fast-export --import-marks=marksfile other\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"174028","messageId":"20110822175705.GB1945@sigill.intra.peff.net","threadId":"28142","inReplyTo":"CAGdFq_gx+NutDCU7egr_fYUvVG=g1t76LiV1SV-W=0KWBTjHyQ@mail.gmail.com","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-22T17:57:05Z","receivedAt":"2011-08-22T17:57:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 22, 2011 at 09:54:47AM -0700, Sverre Rabbelier wrote:\n\n> Do you agree that this is expected behavior?\n> \n> $ git init test\n> Initialized empty Git repository in /home/sverre/code/test/.git/\n> $ cd test/\n> $ echo content >> foo\n> sverre@laptop-sverre:~/code/test\n> $ git add foo\n> $ git commit -m first\n> [master (root-commit) 821176f] first\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> create mode 100644 foo\n> $ echo content >> foo\n> $ git commit -am second\n> [master 1934282] second\n> 1 files changed, 1 insertions(+), 0 deletions(-)\n> $ git branch other\n> $ git fast-export ^master other\n> reset refs/heads/other\n> from 1934282469e3a83a5ef827fd31e074cfb4f3eadf\n\nYeah, that seems reasonable to me.\n\n> Because in current git.git, this doesn't work (the above is generated\n> using a git that has the patch series Dscho and I sent out). Current\n> git will instead do the following:\n> \n> $ git fast-export ^master other\n> reset refs/heads/other\n> from :0\n> \n> The 'from :0' here is obviously a bug (which is fixed by our patch series).\n\nYep, the current behavior is definitely wrong. But I thought your\nquestion was about accidentally mentioning refs/heads/master, which this\ndoesn't do (nor should it).\n\nI just read through the remote-helper threads from early June, and the\nonly mention of triggering that is when you actually have a rename\n(i.e., your \"refs/heads/foo\" becomes remote's \"refs/heads/bar\", but we\nmention \"refs/heads/foo\" in the export stream). I was thinking there was\nanother case, but I couldn't find mention of it.\n\n> You might wonder, 'why would anyone do that', well, for example, they\n> might be using marks:\n> \n> $ git fast-export --export-marks=marksfile master > /dev/null\n> $ git fast-export --import-marks=marksfile other\n> reset refs/heads/other\n> from :4\n> \n> Again, the above is generated with my patched git, current git.git\n> simply outputs nothing.\n> \n> $ git fast-export --import-marks=marksfile other\n\nYeah, the behavior of your patch looks fine to me. I thought the point\nin contention was that having export understand refspecs would fix a lot\nof _other_ cases, too.\n\n-Peff\n"},{"id":"174029","messageId":"CAGdFq_hU3EnZnBh_eb7uax_Cb=0eA1K3GxMys+S60SEm94UE1w@mail.gmail.com","threadId":"28142","inReplyTo":"20110822175705.GB1945@sigill.intra.peff.net","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-08-22T18:05:08Z","receivedAt":"2011-08-22T18:05:08Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 22, 2011 at 10:57, Jeff King wrote:\n> I just read through the remote-helper threads from early June, and the\n> only mention of triggering that is when you actually have a rename\n> (i.e., your \"refs/heads/foo\" becomes remote's \"refs/heads/bar\", but we\n> mention \"refs/heads/foo\" in the export stream). I was thinking there was\n> another case, but I couldn't find mention of it.\n\nThe patches Dscho and I sent are in response to the RFC patches (that\nare currently \"stalled\" in whats-cooking) I added on top of the series\nthat rerolled yours.\n\n> Yeah, the behavior of your patch looks fine to me. I thought the point\n> in contention was that having export understand refspecs would fix a lot\n> of _other_ cases, too.\n\nRight. Sadly it doesn't look like I'll have time to try and fix 'git\nbundle' anytime soon, so this'll remain broken.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"174046","messageId":"7vpqjxuogr.fsf@alter.siamese.dyndns.org","threadId":"28142","inReplyTo":"20110822175705.GB1945@sigill.intra.peff.net","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-08-22T21:27:00Z","receivedAt":"2011-08-22T21:27:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Yeah, the behavior of your patch looks fine to me. I thought the point\n> in contention was that having export understand refspecs would fix a lot\n> of _other_ cases, too.\n\nPerhaps if we added refspecs we could cover other cases, too, but I do not\nthink we need to require the patch to cover additional cases. The more\nproblematic was that the particular implementation in the patch smelled\nbad.\n"},{"id":"174047","messageId":"20110822213214.GA14404@sigill.intra.peff.net","threadId":"28142","inReplyTo":"7vpqjxuogr.fsf@alter.siamese.dyndns.org","subject":"Re: Malformed branch name in fast-export when specifying non-HEAD/branch revision","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-08-22T21:32:14Z","receivedAt":"2011-08-22T21:32:14Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 22, 2011 at 02:27:00PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Yeah, the behavior of your patch looks fine to me. I thought the point\n> > in contention was that having export understand refspecs would fix a lot\n> > of _other_ cases, too.\n> \n> Perhaps if we added refspecs we could cover other cases, too, but I do not\n> think we need to require the patch to cover additional cases. The more\n> problematic was that the particular implementation in the patch smelled\n> bad.\n\nOK. I didn't look at that at all. I just wanted to express my support\nfor \"refspecs would solve other problems, too\". :)\n\n-Peff\n"}]}