{"thread":{"id":"27845","subject":"SP in committer line in fast-import stream","startedAt":"2011-07-18T14:26:45Z","lastAt":"2011-07-18T19:10:13Z","messageCount":5,"participants":["SASAKI Suguru","Dmitry Ivankov"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"171577","messageId":"CAE3X6mwJquoHj06FVGTsg0qtzyTwbd6gNqy7J4yWiVF-+p-23Q@mail.gmail.com","threadId":"27845","inReplyTo":null,"subject":"SP in committer line in fast-import stream","fromName":"SASAKI Suguru","fromEmail":"sss.sonik@gmail.com","sentAt":"2011-07-18T14:26:45Z","receivedAt":"2011-07-18T14:26:45Z","isPatch":false,"sender":{"key":"sss.sonik@gmail.com","avatar":"https://gravatar.com/avatar/07e68228461cd7a89e33c31b00998b53d5be29ec0fddc7bb72d4270177c86f69?d=mp&s=160"},"body":"Hi,\n\n\nI'm working with data from `bzr fast-export` and `git fast-import`.\n(bzr is 2.4b5, git is 1.7.5.4, on Debian GNU/Linux (sid))\n\nExport and import themselves are OK,\nbut `git fsck --strict` exits with error, saying:\n\n  error in commit 2e7a16fbe57b555c1c5954470ef66f3a2a089288: invalid\nauthor/committer line - missing space before email\n\nand pushing to remote like GitHub fails.\n\n\nI found minimal OK-data unlike `bzr fast-export` outputs and NG-data\nlike `bzr fast-export`.\n(Attached: test_NG.data.txt and test_OK.data.txt)\n\nOnly one difference between these is a space in committer line.\n  * OK: 'committer' SP SP LT GT ...\n  * NG: 'committer' SP    LT GT ...\n\n`man git-fast-import` says:\n\n  commit\n    Create or update a branch with a new commit, recording one logical\nchange to the project.\n\n      'commit' SP <ref> LF\n      mark?\n      ('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n      'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n      data\n      ('from' SP <committish> LF)?\n      ('merge' SP <committish> LF)?\n      (filemodify | filedelete | filecopy | filerename | filedeleteall\n| notemodify)*\n      LF?\n\nI think, from this notations, both data is OK.\nWhat's the problem?\n\nRegards,\n\n-- \nSASAKI Suguru\n  mailto:sss.sonik@gmail.com\n\n\ncommit refs/heads/master\nmark :1\ncommitter  <> 1162316103 +0000\ndata 4\ntest\nM 644 inline README\ndata 11\nThis is OK.\n\n\n\ncommit refs/heads/master\nmark :1\ncommitter <> 1162316103 +0000\ndata 4\ntest\nM 644 inline README\ndata 11\nThis is NG.\n\n"},{"id":"171580","messageId":"loom.20110718T172927-173@post.gmane.org","threadId":"27845","inReplyTo":"CAE3X6mwJquoHj06FVGTsg0qtzyTwbd6gNqy7J4yWiVF-+p-23Q@mail.gmail.com","subject":"Re: SP in committer line in fast-import stream","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-18T15:38:19Z","receivedAt":"2011-07-18T15:38:19Z","isPatch":false,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"Hi,\n\nSASAKI Suguru <sss.sonik <at> gmail.com> writes:\n\n> \n> Hi,\n> \n> I'm working with data from `bzr fast-export` and `git fast-import`.\n> (bzr is 2.4b5, git is 1.7.5.4, on Debian GNU/Linux (sid))\n> \n> Export and import themselves are OK,\n> but `git fsck --strict` exits with error, saying:\n> \n>   error in commit 2e7a16fbe57b555c1c5954470ef66f3a2a089288: invalid\n> author/committer line - missing space before email\n> \n> and pushing to remote like GitHub fails.\n> \n> I found minimal OK-data unlike `bzr fast-export` outputs and NG-data\n> like `bzr fast-export`.\n> (Attached: test_NG.data.txt and test_OK.data.txt)\n> \n> Only one difference between these is a space in committer line.\n>   * OK: 'committer' SP SP LT GT ...\n>   * NG: 'committer' SP    LT GT ...\n> \n> `man git-fast-import` says:\n> \n>   commit\n>     Create or update a branch with a new commit, recording one logical\n> change to the project.\n> \n>       'commit' SP <ref> LF\n>       mark?\n>       ('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n>       'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n>       data\n>       ('from' SP <committish> LF)?\n>       ('merge' SP <committish> LF)?\n>       (filemodify | filedelete | filecopy | filerename | filedeleteall\n> | notemodify)*\n>       LF?\n> \n> I think, from this notations, both data is OK.\n> What's the problem?\nThe problem is with git-fast-import that it doesn't verify the format strictly \nhere.\nFor example following (no LT) will pass:\n<name> SP <email> GT \nThe second problem is that it generates \"bad\" committer, in fact name-email is \nused as-is, so at least it should convert absent name to a empty name. Or maybe \njust fix the format to make string obligatory.\nThere even is a third minor problem, fsck will report confusing \"missing space\" \nfor the no-LT example.\n\nThird one is a clear.\nYour one is the second one, while internally it pulls the first one too.\n\nThe shortest fix is to read documentation as\n'committer' SP <name> SP LT <email> GT SP <when> LF\n\n\n> \n> Regards,\n> \n"},{"id":"171584","messageId":"CAE3X6mxbMBwd5O+md0J3M6DUu38Q1uzDHNhAU7iGbqYVm2TyRw@mail.gmail.com","threadId":"27845","inReplyTo":"loom.20110718T172927-173@post.gmane.org","subject":"Re: SP in committer line in fast-import stream","fromName":"SASAKI Suguru","fromEmail":"sss.sonik@gmail.com","sentAt":"2011-07-18T16:18:35Z","receivedAt":"2011-07-18T16:18:35Z","isPatch":false,"sender":{"key":"sss.sonik@gmail.com","avatar":"https://gravatar.com/avatar/07e68228461cd7a89e33c31b00998b53d5be29ec0fddc7bb72d4270177c86f69?d=mp&s=160"},"body":"Hi,\n\n\n2011-07-19 Dmitry Ivankov <divanorama@gmail.com>:\n> The problem is with git-fast-import that it doesn't verify the format strictly\n> here.\n> For example following (no LT) will pass:\n> <name> SP <email> GT\n> The second problem is that it generates \"bad\" committer, in fact name-email is\n> used as-is, so at least it should convert absent name to a empty name. Or maybe\n> just fix the format to make string obligatory.\n> There even is a third minor problem, fsck will report confusing \"missing space\"\n> for the no-LT example.\n>\n> Third one is a clear.\n> Your one is the second one, while internally it pulls the first one too.\n>\n> The shortest fix is to read documentation as\n> 'committer' SP <name> SP LT <email> GT SP <when> LF\n\nThanks.  I understand what happens.\nFor now, I'll write some wrapper around git-fast-import as a workaound for this.\n\nBut, if git-fast-import successfully import and git-fsck will confuse,\naren't some fixes necessary?\nIt might be too done if git-fast-import will check as if git-fsck does,\nbut I think some simple checks will help us.\n\nAny comments?\n\n\nRegards,\n\n-- \nSASAKI Suguru\n  mailto:sss.sonik@gmail.com\n"},{"id":"171589","messageId":"loom.20110718T184404-335@post.gmane.org","threadId":"27845","inReplyTo":"CAE3X6mxbMBwd5O+md0J3M6DUu38Q1uzDHNhAU7iGbqYVm2TyRw@mail.gmail.com","subject":"Re: SP in committer line in fast-import stream","fromName":"Dmitry Ivankov","fromEmail":"divanorama@gmail.com","sentAt":"2011-07-18T16:57:52Z","receivedAt":"2011-07-18T16:57:52Z","isPatch":false,"sender":{"key":"divanorama@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158999?v=4"},"body":"SASAKI Suguru <sss.sonik <at> gmail.com> writes:\n\n> >\n> > The shortest fix is to read documentation as\n> > 'committer' SP <name> SP LT <email> GT SP <when> LF\n> \n> Thanks.  I understand what happens.\n> For now, I'll write some wrapper around git-fast-import as a workaound for \nthis.\n> \n> But, if git-fast-import successfully import and git-fsck will confuse,\n> aren't some fixes necessary?\n> It might be too done if git-fast-import will check as if git-fsck does,\n> but I think some simple checks will help us.\n> \n> Any comments?\nOne patch is at the bottom, it makes fast-import behave well on proper input \nstreams like yours.\nMaking fast-import stricter is worthy but will be a larger patch and effort. \nI'll try not to forget about and at least to write some failing tests.\n\n> \n> Regards,\n> \n\nName cannot contain LT or GT and ident comes after SP in fast-import. So \npretend there was a <empty name> SP if there is no name at all.\n\nParsing isn't strict still.\ndiff --git a/fast-import.c b/fast-import.c\nindex 78d9786..91a90e2 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1971,6 +1971,9 @@ static char *parse_ident(const char *buf)\n        size_t name_len;\n        char *ident;\n \n+       /* ensure there is a space delimiter even if there is no name */\n+       if (*buf == '<')\n+               --buf;\n        gt = strrchr(buf, '>');\n        if (!gt)\n                die(\"Missing > in ident string: %s\", buf);\n"},{"id":"171600","messageId":"CAE3X6mx+X=ptcXTXmm2GaKKU1nag4P+cp_e6NRAcN5bgaob7Cg@mail.gmail.com","threadId":"27845","inReplyTo":"loom.20110718T184404-335@post.gmane.org","subject":"Re: SP in committer line in fast-import stream","fromName":"SASAKI Suguru","fromEmail":"sss.sonik@gmail.com","sentAt":"2011-07-18T19:10:13Z","receivedAt":"2011-07-18T19:10:13Z","isPatch":false,"sender":{"key":"sss.sonik@gmail.com","avatar":"https://gravatar.com/avatar/07e68228461cd7a89e33c31b00998b53d5be29ec0fddc7bb72d4270177c86f69?d=mp&s=160"},"body":"Hi,\n\n\n(2011-07-19 01:57), Dmitry Ivankov wrote:\n> One patch is at the bottom,\n > it makes fast-import behave well on proper input streams like yours.\n\nThanks.\nfast-import with you patch has worked well on these input streams.\n\n> Making fast-import stricter is worthy but will be a larger patch and effort.\n\nExactly.\n\n> I'll try not to forget about and at least to write some failing tests.\n\nAddinng tests to t/t9300-fast-import.sh ?\nIf tests on input streams like these streams of mine will do,\nI'll write some failing tests.  Will they?\n\n\nRegards,\n\n-- \nSASAKI Suguru\n   mailto:sss.sonik@gmail.com\n"}]}