Re: Odd number of elements in anonymous hash

Dennis Schridde [off-list ref] writes:

Am Donnerstag, 10. Januar 2008 21:08:52 schrieb Junio C Hamano:

quoted

Dennis Schridde [off-list ref] writes:

quoted

quoted

[svn-remote "svn"]
    reposRoot = file:///var/svn/warzone2100
    uuid = 4a71c877-e1ca-e34f-864e-861f7616d084
    branches-maxRev = 14
    tags-maxRev = 14
    svnsync-uuid = 4a71c877-e1ca-e34f-864e-861f7616d084\n
    svnsync-url = http://svn.gna.org/svn/warzone
[svn-remote "tags/1.10a.12"]
    reposRoot = file:///var/svn/warzone2100
    uuid = 4a71c877-e1ca-e34f-864e-861f7616d084
---

The rest of the file is rather boring. The "svn" remote is not changed
(besides having higher revisions) and the other remotes look exactly like
the "tags/1.10a.12" one.

Somehow I think that the \n at the end of the svnsync-uuid shouldn't be
there... It could be that this is the same linebreak which prevents
people from relocating (svn switch --relocate) from
svn://svn.gna.org/svn/warzone to http://svn.gna.org/svn/warzone, so that
would be a Gna bug.
However git-svn shouldn't throw any warnings (or even (make perl) crash?)
on such occasions, either...

I now got it to run through without a segfault, by compiling an unstriped
perl binary with debug symbols (Gentoo: FEATURES=nostrip CFLAGS="...
-g"). Maybe this is a bug in GCC or something...

The "Odd number of elements in anonymous hash" still stays, though.

The code in question is:

	my $svnsync;
	# see if we have it in our config, first:
	eval {
		my $section = "svn-remote.$self->{repo_id}";
		$svnsync = {
		  url => tmp_config('--get', "$section.svnsync-url"),
		  uuid => tmp_config('--get', "$section.svnsync-uuid"),
		}
	};

I think the "Odd number" is an indication that one of the
tmp_config() calls is returning an even number of elements (so
the hash whose ref will be stored in $svnsync ends up having an
odd number of elements), and that is why I initially asked you
about "more than one" svnsync-url.  0 is also an even number,
and it could be that it is not finding any.

How about doing something ugly like this _just for diagnosis_?

	my $svnsync;
	# see if we have it in our config, first:
	eval {
		my $section = "svn-remote.$self->{repo_id}";
		my @u = tmp_config('--get', "$section.svnsync-url");
		my @v = tmp_config('--get', "$section.svnsync-uuid");
		if (@u != 1 || @v != 1) {
                	print STDERR "Oops: <$section> $#u <@u> $#v <@v>\n";
		}
		$svnsync = {
		  url => @u,
		  uuid => @v,
		}
	};

I've created /usr/bin/git-svndbg and changed that part, like you proposed.
I now get this output. (As it continues to run, there are probably more 
occassions of the Oops.)
---
Oops: <svn-remote.svn> 0 <http://svn.gna.org/svn/warzone> 1 
<4a71c877-e1ca-e34f-864e-861f7616d084 >
Odd number of elements in anonymous hash at /usr/bin/git-svndbg line 1768.
r13 = ee6d5a48dd5cf1a96ed5217d638f372d2c173d89 (tags/1.10a)
---

Exactly.  The trailing newline is taken as a record separator by
tmp_config subroutine.

Repositories generated by svnsync cannot be relied on to have
properly set revprops without newlines in UUIDs and URLs.  There
may be broken versions of svnsync out there that append extra
newlines to UUIDs, or the revprops could've been changed by
repository administrators at any time, too.

At least one repository we've come across has an embedded
newline erroneously set in the svnsync-uuid prop.  This is bad
because the trailing newline is taken as another record by the
Git.pm library, and the wantarray detection causes tmp_config()
to return an array with an empty-but-existing second element.

We will now strip leading and trailing whitespace both before
setting and after reading the uuid and url for svnsync values.
We will also force tmp_config to return a single scalar when
reading existing values.

SVN UUIDs should never have whitespace in them, and SVN
repository URLs should be URI-escaped, so neither of those
values we ever see in git-svn should actually have whitespace
in them.

Thanks to Dennis Schridde for the bug report and Junio for
helping diagnose this.

Signed-off-by: Eric Wong <redacted>
---
 git-svn.perl |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 3308fe1..f40ad2c 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -1758,10 +1758,16 @@ sub svnsync {
 	# see if we have it in our config, first:
 	eval {
 		my $section = "svn-remote.$self->{repo_id}";
-		$svnsync = {
-		  url => tmp_config('--get', "$section.svnsync-url"),
-		  uuid => tmp_config('--get', "$section.svnsync-uuid"),
-		}
+
+	        my $url = tmp_config('--get', "$section.svnsync-url");
+		($url) = ($url =~ m{^([a-z\+]+://\S+)$}) or
+	           die "doesn't look right - svn:sync-from-url is '$url'\n";
+
+	        my $uuid = tmp_config('--get', "$section.svnsync-uuid");
+		($uuid) = ($uuid =~ m{^([0-9a-f\-]{30,})$}) or
+	           die "doesn't look right - svn:sync-from-uuid is '$uuid'\n";
+
+		$svnsync = { url => $url, uuid => $uuid }
 	};
 	if ($svnsync && $svnsync->{url} && $svnsync->{uuid}) {
 		return $self->{svnsync} = $svnsync;

@@ -1772,11 +1778,11 @@ sub svnsync {
 	my $rp = $self->ra->rev_proplist(0);
 
 	my $url = $rp->{'svn:sync-from-url'} or die $err . "url\n";
-	$url =~ m{^[a-z\+]+://} or
+	($url) = ($url =~ m{^([a-z\+]+://\S+)$}) or
 	           die "doesn't look right - svn:sync-from-url is '$url'\n";
 
 	my $uuid = $rp->{'svn:sync-from-uuid'} or die $err . "uuid\n";
-	$uuid =~ m{^[0-9a-f\-]{30,}$} or
+	($uuid) = ($uuid =~ m{^([0-9a-f\-]{30,})$}) or
 	           die "doesn't look right - svn:sync-from-uuid is '$uuid'\n";
 
 	my $section = "svn-remote.$self->{repo_id}";

-- 
Eric Wong

Am Samstag, 12. Januar 2008 08:13:55 schrieben Sie:

Repositories generated by svnsync cannot be relied on to have
properly set revprops without newlines in UUIDs and URLs.  There
may be broken versions of svnsync out there that append extra
newlines to UUIDs, or the revprops could've been changed by
repository administrators at any time, too.

At least one repository we've come across has an embedded
newline erroneously set in the svnsync-uuid prop.  This is bad
because the trailing newline is taken as another record by the
Git.pm library, and the wantarray detection causes tmp_config()
to return an array with an empty-but-existing second element.

We will now strip leading and trailing whitespace both before
setting and after reading the uuid and url for svnsync values.
We will also force tmp_config to return a single scalar when
reading existing values.

SVN UUIDs should never have whitespace in them, and SVN
repository URLs should be URI-escaped, so neither of those
values we ever see in git-svn should actually have whitespace
in them.

Thanks to Dennis Schridde for the bug report and Junio for
helping diagnose this.

Thanks! This patch seems to work, at least the "Odd number..." message is 
gone. The segfault with a non-debug perl stays, though. But I guess that is a 
different problem.

--Dennis