{"thread":{"id":"10523","subject":"Possible bug: git-svn leaves broken tree in case of error","startedAt":"2007-10-30T07:30:26Z","lastAt":"2007-10-31T14:04:39Z","messageCount":5,"participants":["Anton Korobeynikov","Eric Wong","Karl Hasselström"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"57537","messageId":"1193729426.30755.32.camel@asl.dorms.spbu.ru","threadId":"10523","inReplyTo":null,"subject":"Possible bug: git-svn leaves broken tree in case of error","fromName":"Anton Korobeynikov","fromEmail":"asl@math.spbu.ru","sentAt":"2007-10-30T07:30:26Z","receivedAt":"2007-10-30T07:30:26Z","isPatch":false,"sender":{"key":"asl@math.spbu.ru","avatar":null},"body":"Hello, Everyone.\n\nI noticed this bug several times. Consider the following conditions are\nmet:\n- We're syncing from svn using git-svn :)\n- We have authors file provided\n- We have a changeset with author unlisted in the authors file.\n\ngit-svn dies due to the following code:\nsub check_author {\n        my ($author) = @_;\n        if (!defined $author || length $author == 0) {\n                $author = '(no author)';\n        }\n        if (defined $::_authors && ! defined $::users{$author}) {\n                die \"Author: $author not defined in $::_authors file\\n\";\n        }\n        $author;\n}\n\nUnfortunately it leaves repository in some middle state: git-svn itself\nthinks, that it synced with everything, but git itself doesn't \"see\" any\nchangesets anymore. I found no way to repair tree after such situation,\nso I had to repull from scratch.\n\nI found myself, that this should be warning (and fix in this case is\ntrivial), not error (maybe some commandline switch to control behaviour,\netc). It can be even error, but breaking tree is definitely bug in this\ncase.\n\nAny thoughts?\n\n-- \nWith best regards, Anton Korobeynikov.\n\nFaculty of Mathematics & Mechanics, Saint Petersburg State University.\n"},{"id":"57664","messageId":"20071031075524.GB7798@muzzle","threadId":"10523","inReplyTo":"1193729426.30755.32.camel@asl.dorms.spbu.ru","subject":"Re: Possible bug: git-svn leaves broken tree in case of error","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-31T07:55:24Z","receivedAt":"2007-10-31T07:55:24Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Anton Korobeynikov <asl@math.spbu.ru> wrote:\n> Hello, Everyone.\n> \n> I noticed this bug several times. Consider the following conditions are\n> met:\n> - We're syncing from svn using git-svn :)\n> - We have authors file provided\n> - We have a changeset with author unlisted in the authors file.\n> \n> git-svn dies due to the following code:\n> sub check_author {\n>         my ($author) = @_;\n>         if (!defined $author || length $author == 0) {\n>                 $author = '(no author)';\n>         }\n>         if (defined $::_authors && ! defined $::users{$author}) {\n>                 die \"Author: $author not defined in $::_authors file\\n\";\n>         }\n>         $author;\n> }\n> \n> Unfortunately it leaves repository in some middle state: git-svn itself\n> thinks, that it synced with everything, but git itself doesn't \"see\" any\n> changesets anymore. I found no way to repair tree after such situation,\n> so I had to repull from scratch.\n> \n> I found myself, that this should be warning (and fix in this case is\n> trivial), not error (maybe some commandline switch to control behaviour,\n> etc). It can be even error, but breaking tree is definitely bug in this\n> case.\n\nYou should be able to change the numbers in *-maxRev back to\nan old revision in .git/svn/.metadata.  Does that fix things for you\nso you can resume synching again?\n\nI'll have to investigate the die()-ing of check_authors since\nthat should cause git-svn to quit before the maxRev numbers\nget incremented.\n\n-- \nEric Wong\n"},{"id":"57670","messageId":"20071031084257.GA2911@mayonaise","threadId":"10523","inReplyTo":"20071031075524.GB7798@muzzle","subject":"Re: Possible bug: git-svn leaves broken tree in case of error","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-10-31T08:42:57Z","receivedAt":"2007-10-31T08:42:57Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Eric Wong <normalperson@yhbt.net> wrote:\n> Anton Korobeynikov <asl@math.spbu.ru> wrote:\n> > Hello, Everyone.\n> > \n> > I noticed this bug several times. Consider the following conditions are\n> > met:\n> > - We're syncing from svn using git-svn :)\n> > - We have authors file provided\n> > - We have a changeset with author unlisted in the authors file.\n> > \n> > git-svn dies due to the following code:\n> > sub check_author {\n> >         my ($author) = @_;\n> >         if (!defined $author || length $author == 0) {\n> >                 $author = '(no author)';\n> >         }\n> >         if (defined $::_authors && ! defined $::users{$author}) {\n> >                 die \"Author: $author not defined in $::_authors file\\n\";\n> >         }\n> >         $author;\n> > }\n> > \n> > Unfortunately it leaves repository in some middle state: git-svn itself\n> > thinks, that it synced with everything, but git itself doesn't \"see\" any\n> > changesets anymore. I found no way to repair tree after such situation,\n> > so I had to repull from scratch.\n> > \n> > I found myself, that this should be warning (and fix in this case is\n> > trivial), not error (maybe some commandline switch to control behaviour,\n> > etc). It can be even error, but breaking tree is definitely bug in this\n> > case.\n> \n> You should be able to change the numbers in *-maxRev back to\n> an old revision in .git/svn/.metadata.  Does that fix things for you\n> so you can resume synching again?\n> \n> I'll have to investigate the die()-ing of check_authors since\n> that should cause git-svn to quit before the maxRev numbers\n> get incremented.\n\nWith the following test case, I'm not able to reproduce what you're\ndescribing.\n\nBut yes, die-ing here and not being able to gracefully recover is a\nnasty bug...\n\nWarning instead of die-ing here is not a good option, because it can\nlead to inconsistent author data inside populating history.  I believe\nit's better to error out immediately so the user can fix their authors\nfile.\n\ndiff --git a/t/t9117-git-svn-authors-file.sh b/t/t9117-git-svn-authors-file.sh\nnew file mode 100755\nindex 0000000..4566307\n--- /dev/null\n+++ b/t/t9117-git-svn-authors-file.sh\n@@ -0,0 +1,85 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Eric Wong\n+#\n+\n+test_description='git-svn authors file tests'\n+\n+. ./lib-git-svn.sh\n+\n+cat > svn-authors <<EOF\n+aa = AAAAAAA AAAAAAA <aa@example.com>\n+bb = BBBBBBB BBBBBBB <bb@example.com>\n+EOF\n+\n+test_expect_success 'setup svnrepo' \"\n+\tsvn mkdir -m aa --username aa $svnrepo/aa &&\n+\tsvn mkdir -m bb --username bb $svnrepo/bb &&\n+\tsvn mkdir -m cc --username cc $svnrepo/cc &&\n+\tsvn mkdir -m dd --username dd $svnrepo/dd\n+\t\"\n+\n+test_expect_failure 'start import with incomplete authors file' \"\n+\tgit-svn clone --authors-file=svn-authors $svnrepo x\n+\t\"\n+\n+test_expect_success 'imported 2 revisions successfully' \"\n+\tcd x &&\n+\t\ttest \\`git rev-list refs/remotes/git-svn | wc -l\\` -eq 2 &&\n+\t\tgit rev-list -1 --pretty=raw refs/remotes/git-svn | \\\n+\t\t  grep '^author BBBBBBB BBBBBBB <bb@example\\.com> ' &&\n+\t\tgit rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \\\n+\t\t  grep '^author AAAAAAA AAAAAAA <aa@example\\.com> ' &&\n+\t\tcd ..\n+\t\"\n+\n+cat >> svn-authors <<EOF\n+cc = CCCCCCC CCCCCCC <cc@example.com>\n+dd = DDDDDDD DDDDDDD <dd@example.com>\n+EOF\n+\n+test_expect_success 'continues to import once authors have been added' \"\n+\tcd x &&\n+\t\tgit-svn fetch --authors-file=../svn-authors &&\n+\t\ttest \\`git rev-list refs/remotes/git-svn | wc -l\\` -eq 4 &&\n+\t\tgit rev-list -1 --pretty=raw refs/remotes/git-svn | \\\n+\t\t  grep '^author DDDDDDD DDDDDDD <dd@example\\.com> ' &&\n+\t\tgit rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \\\n+\t\t  grep '^author CCCCCCC CCCCCCC <cc@example\\.com> ' &&\n+\t\tcd ..\n+\t\"\n+\n+test_expect_success 'authors-file against globs' \"\n+\tsvn mkdir -m globs --username aa \\\n+\t  $svnrepo/aa/trunk $svnrepo/aa/branches $svnrepo/aa/tags &&\n+\tgit-svn clone --authors-file=svn-authors -s $svnrepo/aa aa-work &&\n+\tsvn mkdir -m aa/branches/bb --username bb $svnrepo/aa/branches/bb &&\n+\tsvn mkdir -m aa/branches/ee --username ee $svnrepo/aa/branches/ee &&\n+\tsvn mkdir -m aa/branches/cc --username cc $svnrepo/aa/branches/cc\n+\t\"\n+\n+test_expect_failure 'fetch fails on ee' \"\n+\tcd aa-work &&\n+\t\tgit-svn fetch --authors-file=../svn-authors\n+\t\"\n+\n+tmp_config_get () {\n+\tGIT_CONFIG=.git/svn/.metadata git config --get \"$1\"\n+}\n+\n+test_expect_success 'failure happened without negative side effects' \"\n+\ttest 6 -eq \\\"\\`tmp_config_get svn-remote.svn.branches-maxRev\\`\\\" &&\n+\ttest 6 -eq \\\"\\`tmp_config_get svn-remote.svn.tags-maxRev\\`\\\"\n+\t\"\n+\n+cat >> ../svn-authors <<EOF\n+ee = EEEEEEE EEEEEEE <ee@example.com>\n+EOF\n+\n+test_expect_success 'fetch continues after authors-file is fixed' \"\n+\tgit-svn fetch --authors-file=../svn-authors &&\n+\ttest 8 -eq \\\"\\`tmp_config_get svn-remote.svn.branches-maxRev\\`\\\" &&\n+\ttest 8 -eq \\\"\\`tmp_config_get svn-remote.svn.tags-maxRev\\`\\\"\n+\t\"\n+\n+test_done\n\n-- \nEric Wong\n"},{"id":"57679","messageId":"20071031102339.GA26565@diana.vm.bytemark.co.uk","threadId":"10523","inReplyTo":"20071031084257.GA2911@mayonaise","subject":"Re: Possible bug: git-svn leaves broken tree in case of error","fromName":"Karl Hasselström","fromEmail":"kha@treskal.com","sentAt":"2007-10-31T10:23:39Z","receivedAt":"2007-10-31T10:23:39Z","isPatch":false,"sender":{"key":"kha@treskal.com","avatar":"https://gravatar.com/avatar/f0120c734b5279b345075a28521e1ac66acb20c9913ffe9bf6ae97e53f7f3f13?d=mp&s=160"},"body":"On 2007-10-31 01:42:57 -0700, Eric Wong wrote:\n\n> Warning instead of die-ing here is not a good option, because it can\n> lead to inconsistent author data inside populating history. I\n> believe it's better to error out immediately so the user can fix\n> their authors file.\n\nYes, that seems to be the only sensible opition.\n\n-- \nKarl Hasselström, kha@treskal.com\n      www.treskal.com/kalle\n"},{"id":"57698","messageId":"1193839479.26720.16.camel@asl.dorms.spbu.ru","threadId":"10523","inReplyTo":"20071031084257.GA2911.SS5073SS@mayonaise","subject":"Re: Possible bug: git-svn leaves broken tree in case of error","fromName":"Anton Korobeynikov","fromEmail":"asl@math.spbu.ru","sentAt":"2007-10-31T14:04:39Z","receivedAt":"2007-10-31T14:04:39Z","isPatch":false,"sender":{"key":"asl@math.spbu.ru","avatar":null},"body":"Hello, Eric\n\n> With the following test case, I'm not able to reproduce what you're\n> describing.\nLooks like something nasty here. I also failed to reproduce with such\ntest, however I definitely was sure, that I modelled it properly.\n\nThe typical scenario here is that I'm syncing with external repository\nby hands. I tried to replay this with \"bad\" authors file, but it doesn't\nallow me to past through \"bad\" changeset. And yes, adding new entry to\nauthors files fixes the problem.\n\nI saw corruption, when git-svn in the next run continues fetch\nchangesets. As I saw this problem several times with my current setup, I\nadded some extra backups, so I hope next time I'll catch tree before and\nafter breakage. Sorry for bothering.\n\nBtw, there is way, how repository can be broken (however, by user only):\nafter such error 'git fsck' reports dangling trees, and running 'git gc\n--prune' will break any future sync.\n\nAnyway, thanks alot.\n\n-- \nWith best regards, Anton Korobeynikov.\n\nFaculty of Mathematics & Mechanics, Saint Petersburg State University.\n"}]}