public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
* Possible bug: git-svn leaves broken tree in case of error
@ 2007-10-30  7:30 Anton Korobeynikov
  2007-10-31  7:55 ` Eric Wong
  0 siblings, 1 reply; 5+ messages in thread
From: Anton Korobeynikov @ 2007-10-30  7:30 UTC (permalink / raw)
  To: git

Hello, Everyone.

I noticed this bug several times. Consider the following conditions are
met:
- We're syncing from svn using git-svn :)
- We have authors file provided
- We have a changeset with author unlisted in the authors file.

git-svn dies due to the following code:
sub check_author {
        my ($author) = @_;
        if (!defined $author || length $author == 0) {
                $author = '(no author)';
        }
        if (defined $::_authors && ! defined $::users{$author}) {
                die "Author: $author not defined in $::_authors file\n";
        }
        $author;
}

Unfortunately it leaves repository in some middle state: git-svn itself
thinks, that it synced with everything, but git itself doesn't "see" any
changesets anymore. I found no way to repair tree after such situation,
so I had to repull from scratch.

I found myself, that this should be warning (and fix in this case is
trivial), not error (maybe some commandline switch to control behaviour,
etc). It can be even error, but breaking tree is definitely bug in this
case.

Any thoughts?

-- 
With best regards, Anton Korobeynikov.

Faculty of Mathematics & Mechanics, Saint Petersburg State University.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Possible bug: git-svn leaves broken tree in case of error
  2007-10-30  7:30 Possible bug: git-svn leaves broken tree in case of error Anton Korobeynikov
@ 2007-10-31  7:55 ` Eric Wong
  2007-10-31  8:42   ` Eric Wong
       [not found]   ` <20071031084257.GA2911.SS5073SS@mayonaise>
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Wong @ 2007-10-31  7:55 UTC (permalink / raw)
  To: Anton Korobeynikov; +Cc: git

Anton Korobeynikov <asl@math•spbu.ru> wrote:
> Hello, Everyone.
> 
> I noticed this bug several times. Consider the following conditions are
> met:
> - We're syncing from svn using git-svn :)
> - We have authors file provided
> - We have a changeset with author unlisted in the authors file.
> 
> git-svn dies due to the following code:
> sub check_author {
>         my ($author) = @_;
>         if (!defined $author || length $author == 0) {
>                 $author = '(no author)';
>         }
>         if (defined $::_authors && ! defined $::users{$author}) {
>                 die "Author: $author not defined in $::_authors file\n";
>         }
>         $author;
> }
> 
> Unfortunately it leaves repository in some middle state: git-svn itself
> thinks, that it synced with everything, but git itself doesn't "see" any
> changesets anymore. I found no way to repair tree after such situation,
> so I had to repull from scratch.
> 
> I found myself, that this should be warning (and fix in this case is
> trivial), not error (maybe some commandline switch to control behaviour,
> etc). It can be even error, but breaking tree is definitely bug in this
> case.

You should be able to change the numbers in *-maxRev back to
an old revision in .git/svn/.metadata.  Does that fix things for you
so you can resume synching again?

I'll have to investigate the die()-ing of check_authors since
that should cause git-svn to quit before the maxRev numbers
get incremented.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Possible bug: git-svn leaves broken tree in case of error
  2007-10-31  7:55 ` Eric Wong
@ 2007-10-31  8:42   ` Eric Wong
  2007-10-31 10:23     ` Karl Hasselström
       [not found]   ` <20071031084257.GA2911.SS5073SS@mayonaise>
  1 sibling, 1 reply; 5+ messages in thread
From: Eric Wong @ 2007-10-31  8:42 UTC (permalink / raw)
  To: Anton Korobeynikov; +Cc: git

Eric Wong <normalperson@yhbt•net> wrote:
> Anton Korobeynikov <asl@math•spbu.ru> wrote:
> > Hello, Everyone.
> > 
> > I noticed this bug several times. Consider the following conditions are
> > met:
> > - We're syncing from svn using git-svn :)
> > - We have authors file provided
> > - We have a changeset with author unlisted in the authors file.
> > 
> > git-svn dies due to the following code:
> > sub check_author {
> >         my ($author) = @_;
> >         if (!defined $author || length $author == 0) {
> >                 $author = '(no author)';
> >         }
> >         if (defined $::_authors && ! defined $::users{$author}) {
> >                 die "Author: $author not defined in $::_authors file\n";
> >         }
> >         $author;
> > }
> > 
> > Unfortunately it leaves repository in some middle state: git-svn itself
> > thinks, that it synced with everything, but git itself doesn't "see" any
> > changesets anymore. I found no way to repair tree after such situation,
> > so I had to repull from scratch.
> > 
> > I found myself, that this should be warning (and fix in this case is
> > trivial), not error (maybe some commandline switch to control behaviour,
> > etc). It can be even error, but breaking tree is definitely bug in this
> > case.
> 
> You should be able to change the numbers in *-maxRev back to
> an old revision in .git/svn/.metadata.  Does that fix things for you
> so you can resume synching again?
> 
> I'll have to investigate the die()-ing of check_authors since
> that should cause git-svn to quit before the maxRev numbers
> get incremented.

With the following test case, I'm not able to reproduce what you're
describing.

But yes, die-ing here and not being able to gracefully recover is a
nasty bug...

Warning instead of die-ing here is not a good option, because it can
lead to inconsistent author data inside populating history.  I believe
it's better to error out immediately so the user can fix their authors
file.

diff --git a/t/t9117-git-svn-authors-file.sh b/t/t9117-git-svn-authors-file.sh
new file mode 100755
index 0000000..4566307
--- /dev/null
+++ b/t/t9117-git-svn-authors-file.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+#
+
+test_description='git-svn authors file tests'
+
+. ./lib-git-svn.sh
+
+cat > svn-authors <<EOF
+aa = AAAAAAA AAAAAAA <aa@example•com>
+bb = BBBBBBB BBBBBBB <bb@example•com>
+EOF
+
+test_expect_success 'setup svnrepo' "
+	svn mkdir -m aa --username aa $svnrepo/aa &&
+	svn mkdir -m bb --username bb $svnrepo/bb &&
+	svn mkdir -m cc --username cc $svnrepo/cc &&
+	svn mkdir -m dd --username dd $svnrepo/dd
+	"
+
+test_expect_failure 'start import with incomplete authors file' "
+	git-svn clone --authors-file=svn-authors $svnrepo x
+	"
+
+test_expect_success 'imported 2 revisions successfully' "
+	cd x &&
+		test \`git rev-list refs/remotes/git-svn | wc -l\` -eq 2 &&
+		git rev-list -1 --pretty=raw refs/remotes/git-svn | \
+		  grep '^author BBBBBBB BBBBBBB <bb@example\.com> ' &&
+		git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
+		  grep '^author AAAAAAA AAAAAAA <aa@example\.com> ' &&
+		cd ..
+	"
+
+cat >> svn-authors <<EOF
+cc = CCCCCCC CCCCCCC <cc@example•com>
+dd = DDDDDDD DDDDDDD <dd@example•com>
+EOF
+
+test_expect_success 'continues to import once authors have been added' "
+	cd x &&
+		git-svn fetch --authors-file=../svn-authors &&
+		test \`git rev-list refs/remotes/git-svn | wc -l\` -eq 4 &&
+		git rev-list -1 --pretty=raw refs/remotes/git-svn | \
+		  grep '^author DDDDDDD DDDDDDD <dd@example\.com> ' &&
+		git rev-list -1 --pretty=raw refs/remotes/git-svn~1 | \
+		  grep '^author CCCCCCC CCCCCCC <cc@example\.com> ' &&
+		cd ..
+	"
+
+test_expect_success 'authors-file against globs' "
+	svn mkdir -m globs --username aa \
+	  $svnrepo/aa/trunk $svnrepo/aa/branches $svnrepo/aa/tags &&
+	git-svn clone --authors-file=svn-authors -s $svnrepo/aa aa-work &&
+	svn mkdir -m aa/branches/bb --username bb $svnrepo/aa/branches/bb &&
+	svn mkdir -m aa/branches/ee --username ee $svnrepo/aa/branches/ee &&
+	svn mkdir -m aa/branches/cc --username cc $svnrepo/aa/branches/cc
+	"
+
+test_expect_failure 'fetch fails on ee' "
+	cd aa-work &&
+		git-svn fetch --authors-file=../svn-authors
+	"
+
+tmp_config_get () {
+	GIT_CONFIG=.git/svn/.metadata git config --get "$1"
+}
+
+test_expect_success 'failure happened without negative side effects' "
+	test 6 -eq \"\`tmp_config_get svn-remote.svn.branches-maxRev\`\" &&
+	test 6 -eq \"\`tmp_config_get svn-remote.svn.tags-maxRev\`\"
+	"
+
+cat >> ../svn-authors <<EOF
+ee = EEEEEEE EEEEEEE <ee@example•com>
+EOF
+
+test_expect_success 'fetch continues after authors-file is fixed' "
+	git-svn fetch --authors-file=../svn-authors &&
+	test 8 -eq \"\`tmp_config_get svn-remote.svn.branches-maxRev\`\" &&
+	test 8 -eq \"\`tmp_config_get svn-remote.svn.tags-maxRev\`\"
+	"
+
+test_done

-- 
Eric Wong

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: Possible bug: git-svn leaves broken tree in case of error
  2007-10-31  8:42   ` Eric Wong
@ 2007-10-31 10:23     ` Karl Hasselström
  0 siblings, 0 replies; 5+ messages in thread
From: Karl Hasselström @ 2007-10-31 10:23 UTC (permalink / raw)
  To: Eric Wong; +Cc: Anton Korobeynikov, git

On 2007-10-31 01:42:57 -0700, Eric Wong wrote:

> Warning instead of die-ing here is not a good option, because it can
> lead to inconsistent author data inside populating history. I
> believe it's better to error out immediately so the user can fix
> their authors file.

Yes, that seems to be the only sensible opition.

-- 
Karl Hasselström, kha@treskal•com
      www.treskal.com/kalle

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Possible bug: git-svn leaves broken tree in case of error
       [not found]   ` <20071031084257.GA2911.SS5073SS@mayonaise>
@ 2007-10-31 14:04     ` Anton Korobeynikov
  0 siblings, 0 replies; 5+ messages in thread
From: Anton Korobeynikov @ 2007-10-31 14:04 UTC (permalink / raw)
  To: Eric Wong; +Cc: git

Hello, Eric

> With the following test case, I'm not able to reproduce what you're
> describing.
Looks like something nasty here. I also failed to reproduce with such
test, however I definitely was sure, that I modelled it properly.

The typical scenario here is that I'm syncing with external repository
by hands. I tried to replay this with "bad" authors file, but it doesn't
allow me to past through "bad" changeset. And yes, adding new entry to
authors files fixes the problem.

I saw corruption, when git-svn in the next run continues fetch
changesets. As I saw this problem several times with my current setup, I
added some extra backups, so I hope next time I'll catch tree before and
after breakage. Sorry for bothering.

Btw, there is way, how repository can be broken (however, by user only):
after such error 'git fsck' reports dangling trees, and running 'git gc
--prune' will break any future sync.

Anyway, thanks alot.

-- 
With best regards, Anton Korobeynikov.

Faculty of Mathematics & Mechanics, Saint Petersburg State University.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2007-10-31 14:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-30  7:30 Possible bug: git-svn leaves broken tree in case of error Anton Korobeynikov
2007-10-31  7:55 ` Eric Wong
2007-10-31  8:42   ` Eric Wong
2007-10-31 10:23     ` Karl Hasselström
     [not found]   ` <20071031084257.GA2911.SS5073SS@mayonaise>
2007-10-31 14:04     ` Anton Korobeynikov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox