[PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[James Y Knight]

---
 git-svn.perl                    |    2 +-
 t/t9112-git-svn-md5less-file.sh |   45 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletions(-)
 create mode 100755 t/t9112-git-svn-md5less-file.sh

diff --git a/git-svn.perl b/git-svn.perl
index 3c4f490..b2773dc 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2471,7 +2471,7 @@ sub close_file {
 		$md5->addfile($fh);
 		my $got = $md5->hexdigest;
 		die "Checksum mismatch: $path\n",
-		    "expected: $exp\n    got: $got\n" if ($got ne $exp);
+		    "expected: $exp\n    got: $got\n" if (defined $exp && $got ne $exp);
 		sysseek($fh, 0, 0) or croak $!;
 		if ($fb->{mode_b} == 120000) {
 			sysread($fh, my $buf, 5) == 5 or croak $!;
diff --git a/t/t9112-git-svn-md5less-file.sh b/t/t9112-git-svn-md5less-file.sh
new file mode 100755
index 0000000..08313bb
--- /dev/null
+++ b/t/t9112-git-svn-md5less-file.sh
@@ -0,0 +1,45 @@
+test_description='test that git handles an svn repository with missing md5sums'
+
+. ./lib-git-svn.sh
+
+# Loading a node from a svn dumpfile without a Text-Content-Length
+# field causes svn to neglect to store or report an md5sum.  (it will
+# calculate one if you had put Text-Content-Length: 0).  This showed
+# up in a repository creted with cvs2svn.
+
+cat > dumpfile.svn <<EOF
+SVN-fs-dump-format-version: 1
+
+Revision-number: 1
+Prop-content-length: 98
+Content-length: 98
+
+K 7
+svn:log
+V 0
+
+K 10
+svn:author
+V 4
+test
+K 8
+svn:date
+V 27
+2007-05-06T12:37:01.153339Z
+PROPS-END
+
+Node-path: md5less-file
+Node-kind: file
+Node-action: add
+Prop-content-length: 10
+Content-length: 10
+
+PROPS-END
+
+EOF
+
+test_expect_success 'load svn dumpfile' "svnadmin load $rawsvnrepo < dumpfile.svn"
+
+test_expect_success 'initialize git-svn' "git-svn init $svnrepo"
+test_expect_success 'fetch revisions from svn' 'git-svn fetch'
+test_done
-- 
1.5.2.rc2.1.g7f0b

Re: [PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[Eric Wong]

James Y Knight <foom@fuhm.net> wrote:

Thanks.

Acked-by: Eric Wong <normalperson@yhbt.net>

> ---
>  git-svn.perl                    |    2 +-
>  t/t9112-git-svn-md5less-file.sh |   45 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 1 deletions(-)
>  create mode 100755 t/t9112-git-svn-md5less-file.sh
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 3c4f490..b2773dc 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -2471,7 +2471,7 @@ sub close_file {
>  		$md5->addfile($fh);
>  		my $got = $md5->hexdigest;
>  		die "Checksum mismatch: $path\n",
> -		    "expected: $exp\n    got: $got\n" if ($got ne $exp);
> +		    "expected: $exp\n    got: $got\n" if (defined $exp && $got ne $exp);
>  		sysseek($fh, 0, 0) or croak $!;
>  		if ($fb->{mode_b} == 120000) {
>  			sysread($fh, my $buf, 5) == 5 or croak $!;
> diff --git a/t/t9112-git-svn-md5less-file.sh b/t/t9112-git-svn-md5less-file.sh
> new file mode 100755
> index 0000000..08313bb
> --- /dev/null
> +++ b/t/t9112-git-svn-md5less-file.sh
> @@ -0,0 +1,45 @@
> +test_description='test that git handles an svn repository with missing md5sums'
> +
> +. ./lib-git-svn.sh
> +
> +# Loading a node from a svn dumpfile without a Text-Content-Length
> +# field causes svn to neglect to store or report an md5sum.  (it will
> +# calculate one if you had put Text-Content-Length: 0).  This showed
> +# up in a repository creted with cvs2svn.
> +
> +cat > dumpfile.svn <<EOF
> +SVN-fs-dump-format-version: 1
> +
> +Revision-number: 1
> +Prop-content-length: 98
> +Content-length: 98
> +
> +K 7
> +svn:log
> +V 0
> +
> +K 10
> +svn:author
> +V 4
> +test
> +K 8
> +svn:date
> +V 27
> +2007-05-06T12:37:01.153339Z
> +PROPS-END
> +
> +Node-path: md5less-file
> +Node-kind: file
> +Node-action: add
> +Prop-content-length: 10
> +Content-length: 10
> +
> +PROPS-END
> +
> +EOF
> +
> +test_expect_success 'load svn dumpfile' "svnadmin load $rawsvnrepo < dumpfile.svn"
> +
> +test_expect_success 'initialize git-svn' "git-svn init $svnrepo"
> +test_expect_success 'fetch revisions from svn' 'git-svn fetch'
> +test_done
> -- 
> 1.5.2.rc2.1.g7f0b
> 

-- 
Eric Wong

Re: [PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[A Large Angry SCM]

James Y Knight wrote:
> ---
>  git-svn.perl                    |    2 +-
>  t/t9112-git-svn-md5less-file.sh |   45 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 1 deletions(-)
>  create mode 100755 t/t9112-git-svn-md5less-file.sh
[...]

The new test fails here (Suse 9.3 fully patched) w/ the following:

*** t9112-git-svn-md5less-file.sh ***
*   ok 1: load svn dumpfile
*   ok 2: initialize git-svn
* FAIL 3: fetch revisions from svn
         git-svn fetch
* failed 1 among 3 test(s)
make[1]: *** [t9112-git-svn-md5less-file.sh] Error 1

Re: [PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[Eric Wong]

A Large Angry SCM <gitzilla@gmail.com> wrote:
> James Y Knight wrote:
> >---
> > git-svn.perl                    |    2 +-
> > t/t9112-git-svn-md5less-file.sh |   45 
> > +++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 46 insertions(+), 1 deletions(-)
> > create mode 100755 t/t9112-git-svn-md5less-file.sh
> [...]
> 
> The new test fails here (Suse 9.3 fully patched) w/ the following:
> 
> *** t9112-git-svn-md5less-file.sh ***
> *   ok 1: load svn dumpfile
> *   ok 2: initialize git-svn
> * FAIL 3: fetch revisions from svn
>         git-svn fetch
> * failed 1 among 3 test(s)
> make[1]: *** [t9112-git-svn-md5less-file.sh] Error 1

I can't reproduce it here (on Debian Etch, SVN 1.4.2).  Can you run with
the test with the -v switch?  Thanks.

-- 
Eric Wong

Re: [PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[A Large Angry SCM]

Eric Wong wrote:
 > A Large Angry SCM <gitzilla@gmail.com> wrote:
 >> James Y Knight wrote:
 >>> ---
 >>> git-svn.perl                    |    2 +-
 >>> t/t9112-git-svn-md5less-file.sh |   45
 >>> +++++++++++++++++++++++++++++++++++++++
 >>> 2 files changed, 46 insertions(+), 1 deletions(-)
 >>> create mode 100755 t/t9112-git-svn-md5less-file.sh
 >> [...]
 >>
 >> The new test fails here (Suse 9.3 fully patched) w/ the following:
 >>
 >> *** t9112-git-svn-md5less-file.sh ***
 >> *   ok 1: load svn dumpfile
 >> *   ok 2: initialize git-svn
 >> * FAIL 3: fetch revisions from svn
 >>         git-svn fetch
 >> * failed 1 among 3 test(s)
 >> make[1]: *** [t9112-git-svn-md5less-file.sh] Error 1
 >
 > I can't reproduce it here (on Debian Etch, SVN 1.4.2).  Can you run with
 > the test with the -v switch?  Thanks.
 >


~/GIT/git/t> sh ./t9112-git-svn-md5less-file.sh -v
* expecting success: svnadmin load /home/test/GIT/git/t/trash/svnrepo < 
dumpfile.svn
<<< Started new transaction, based on original revision 1
      * adding path : md5less-file ... done.

------- Committed revision 1 >>>

*   ok 1: load svn dumpfile

* expecting success: git-svn init file:///home/test/GIT/git/t/trash/svnrepo
*   ok 2: initialize git-svn

* expecting success: git-svn fetch
./test-lib.sh: line 141:  8163 Segmentation fault      git-svn fetch
* FAIL 3: fetch revisions from svn
         git-svn fetch

* failed 1 among 3 test(s)
~/GIT/git/t>


And here is the failing part of the test using sh -x:

+ test_expect_success 'fetch revisions from svn' 'git-svn fetch'
+ test 2 = 2
+ test_skip 'fetch revisions from svn' 'git-svn fetch'
++ expr ././t9112-git-svn-md5less-file.sh : '.*/\(t[0-9]*\)-[^/]*$'
+ this_test=t9112
++ expr 2 + 1
+ this_test=t9112.3
+ to_skip=
+ case "$to_skip" in
+ false
+ say 'expecting success: git-svn fetch'
+ echo '* expecting success: git-svn fetch'
* expecting success: git-svn fetch
+ test_run_ 'git-svn fetch'
+ eval 'git-svn fetch'
++ git-svn fetch
./test-lib.sh: line 141:  8276 Segmentation fault      git-svn fetch
+ eval_ret=139
+ return 0
+ '[' 0 = 0 -a 139 = 0 ']'
+ test_failure_ 'fetch revisions from svn' 'git-svn fetch'
++ expr 2 + 1
+ test_count=3
++ expr 0 + 1
+ test_failure=1
+ say 'FAIL 3: fetch revisions from svn'
+ echo '* FAIL 3: fetch revisions from svn'
* FAIL 3: fetch revisions from svn
+ shift
+ echo 'git-svn fetch'
+ sed -e 's/^/  /'
         git-svn fetch
+ test '' = ''
+ echo ''

This began after the 18bece4..99b5a79 update to master. Prior to that 
the svn tests were passing.

Re: [PATCH] Fix git-svn to handle svn not reporting the md5sum of a file, and test.

[Eric Wong]

A Large Angry SCM <gitzilla@gmail.com> wrote:
> Eric Wong wrote:
> > A Large Angry SCM <gitzilla@gmail.com> wrote:
> >> James Y Knight wrote:
> >>> ---
> >>> git-svn.perl                    |    2 +-
> >>> t/t9112-git-svn-md5less-file.sh |   45
> >>> +++++++++++++++++++++++++++++++++++++++
> >>> 2 files changed, 46 insertions(+), 1 deletions(-)
> >>> create mode 100755 t/t9112-git-svn-md5less-file.sh
> >> [...]
> >>
> >> The new test fails here (Suse 9.3 fully patched) w/ the following:
> >>
> >> *** t9112-git-svn-md5less-file.sh ***
> >> *   ok 1: load svn dumpfile
> >> *   ok 2: initialize git-svn
> >> * FAIL 3: fetch revisions from svn
> >>         git-svn fetch
> >> * failed 1 among 3 test(s)
> >> make[1]: *** [t9112-git-svn-md5less-file.sh] Error 1
> >
> > I can't reproduce it here (on Debian Etch, SVN 1.4.2).  Can you run with
> > the test with the -v switch?  Thanks.
> >
> 
> 
> ~/GIT/git/t> sh ./t9112-git-svn-md5less-file.sh -v
> * expecting success: svnadmin load /home/test/GIT/git/t/trash/svnrepo < 
> dumpfile.svn
> <<< Started new transaction, based on original revision 1
>      * adding path : md5less-file ... done.
> 
> ------- Committed revision 1 >>>
> 
> *   ok 1: load svn dumpfile
> 
> * expecting success: git-svn init file:///home/test/GIT/git/t/trash/svnrepo
> *   ok 2: initialize git-svn
> 
> * expecting success: git-svn fetch
> ./test-lib.sh: line 141:  8163 Segmentation fault      git-svn fetch
> * FAIL 3: fetch revisions from svn
>         git-svn fetch
> 
> * failed 1 among 3 test(s)
> ~/GIT/git/t>
> 
> 
> And here is the failing part of the test using sh -x:
> 
> + test_expect_success 'fetch revisions from svn' 'git-svn fetch'
> + test 2 = 2
> + test_skip 'fetch revisions from svn' 'git-svn fetch'
> ++ expr ././t9112-git-svn-md5less-file.sh : '.*/\(t[0-9]*\)-[^/]*$'
> + this_test=t9112
> ++ expr 2 + 1
> + this_test=t9112.3
> + to_skip=
> + case "$to_skip" in
> + false
> + say 'expecting success: git-svn fetch'
> + echo '* expecting success: git-svn fetch'
> * expecting success: git-svn fetch
> + test_run_ 'git-svn fetch'
> + eval 'git-svn fetch'
> ++ git-svn fetch
> ./test-lib.sh: line 141:  8276 Segmentation fault      git-svn fetch
> + eval_ret=139
> + return 0
> + '[' 0 = 0 -a 139 = 0 ']'
> + test_failure_ 'fetch revisions from svn' 'git-svn fetch'
> ++ expr 2 + 1
> + test_count=3
> ++ expr 0 + 1
> + test_failure=1
> + say 'FAIL 3: fetch revisions from svn'
> + echo '* FAIL 3: fetch revisions from svn'
> * FAIL 3: fetch revisions from svn
> + shift
> + echo 'git-svn fetch'
> + sed -e 's/^/  /'
>         git-svn fetch
> + test '' = ''
> + echo ''
> 
> This began after the 18bece4..99b5a79 update to master. Prior to that 
> the svn tests were passing.

Thanks.

I'm definitely not able to reproduce this here, and I'm sure Junio
wouldn't have pushed out if he could, either...  Which versions of SVN
and Perl (MD5) do you have?

A backtrace with debugging symbols could be helpful if the below
stab in the dark doesn't work out:

Maybe there's an off chance that the MD5 implementation you're using
can't handle zero-sized files?

Junio: can you apply the following patch regardless of whether or not it
fixes this issue?  It just makes more sense, thanks.

>From 3229470be27589a0428994475b0a597cc549cf78 Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 27 May 2007 15:59:01 -0700
Subject: [PATCH] git-svn: avoid md5 calculation entirely if SVN doesn't provide one

There's no point in calculating an MD5 if we're not going to use
it.  We'll also avoid the possibility of there being a bug in the
Perl MD5 library not being able to handle zero-sized files.

This is a followup to 20b3d206acbbb042c7ad5f42d36ff8d036a538c5,
which allows us to track repositories that do not provide MD5
checksums.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index eeaeb2d..58f7dd0 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2472,12 +2472,16 @@ sub close_file {
 	my $hash;
 	my $path = $self->git_path($fb->{path});
 	if (my $fh = $fb->{fh}) {
-		seek($fh, 0, 0) or croak $!;
-		my $md5 = Digest::MD5->new;
-		$md5->addfile($fh);
-		my $got = $md5->hexdigest;
-		die "Checksum mismatch: $path\n",
-		    "expected: $exp\n    got: $got\n" if (defined $exp && $got ne $exp);
+		if (defined $exp) {
+			seek($fh, 0, 0) or croak $!;
+			my $md5 = Digest::MD5->new;
+			$md5->addfile($fh);
+			my $got = $md5->hexdigest;
+			if ($got ne $exp) {
+				die "Checksum mismatch: $path\n",
+				    "expected: $exp\n    got: $got\n";
+			}
+		}
 		sysseek($fh, 0, 0) or croak $!;
 		if ($fb->{mode_b} == 120000) {
 			sysread($fh, my $buf, 5) == 5 or croak $!;
-- 
Eric Wong