Odd number of elements in anonymous hash

Odd number of elements in anonymous hash

[Dennis Schridde]

Hello!

I am getting "Odd number of elements in anonymous hash at /usr/bin/git-svn 
line 1760." (normal output, no warning/error) during git-svn-clone.
I am using git version 1.5.4.rc2.

Line 1760 is this (with context, marked with '!!'):
    # 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"),
        }
    };

The commandline was "git svn 
clone --authors-file=/var/git/org.gna.warzone.git/authors --use-svnsync-props --stdlayout 
file:///var/svn/warzone2100/ org.gna.warzone.git/"

I assume this is some kind of bug?

--Dennis

Re: Odd number of elements in anonymous hash

[Junio C Hamano]

Dennis Schridde <devurandom@gmx.net> writes:

> Hello!
>
> I am getting "Odd number of elements in anonymous hash at /usr/bin/git-svn 
> line 1760." (normal output, no warning/error) during git-svn-clone.
> I am using git version 1.5.4.rc2.
>
> Line 1760 is this (with context, marked with '!!'):
>     # 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"),
>         }
>     };
>
> The commandline was "git svn 
> clone --authors-file=/var/git/org.gna.warzone.git/authors --use-svnsync-props --stdlayout 
> file:///var/svn/warzone2100/ org.gna.warzone.git/"
>
> I assume this is some kind of bug?

More than one svn-remote.$your_repo.svnsync-{url,uuid}?

(Eric CC'ed).

Re: Odd number of elements in anonymous hash

[Dennis Schridde]

[-- Attachment #1: Type: text/plain, Size: 1165 bytes --]

Am Dienstag, 8. Januar 2008 18:21:55 schrieb Junio C Hamano:
> Dennis Schridde <devurandom@gmx.net> writes:
> > Hello!
> >
> > I am getting "Odd number of elements in anonymous hash at
> > /usr/bin/git-svn line 1760." (normal output, no warning/error) during
> > git-svn-clone. I am using git version 1.5.4.rc2.
> >
> > Line 1760 is this (with context, marked with '!!'):
> >     # 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"),
> >         }
> >     };
> >
> > The commandline was "git svn
> > clone --authors-file=/var/git/org.gna.warzone.git/authors
> > --use-svnsync-props --stdlayout file:///var/svn/warzone2100/
> > org.gna.warzone.git/"
> >
> > I assume this is some kind of bug?
>
> More than one svn-remote.$your_repo.svnsync-{url,uuid}?
PS: It doesn't happen for every revision and the folders (and thus config) 
were empty before. The config doesn't list any "svnsync-*" during the import 
process.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Odd number of elements in anonymous hash

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Dennis Schridde <devurandom@gmx.net> writes:
> 
> > Hello!
> >
> > I am getting "Odd number of elements in anonymous hash at /usr/bin/git-svn 
> > line 1760." (normal output, no warning/error) during git-svn-clone.
> > I am using git version 1.5.4.rc2.
> >
> > Line 1760 is this (with context, marked with '!!'):
> >     # 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"),
> >         }
> >     };
> >
> > The commandline was "git svn 
> > clone --authors-file=/var/git/org.gna.warzone.git/authors --use-svnsync-props --stdlayout 
> > file:///var/svn/warzone2100/ org.gna.warzone.git/"
> >
> > I assume this is some kind of bug?
> 
> More than one svn-remote.$your_repo.svnsync-{url,uuid}?
> 
> (Eric CC'ed).

I don't use svnsync myself.  I think Sam does, or would have more
insight into this than myself.  I'll hopefully have time to take a
harder look at it later tonight.

-- 
Eric Wong

Re: Odd number of elements in anonymous hash

[Eric Wong]

Dennis Schridde <devurandom@gmx.net> wrote:
> Am Dienstag, 8. Januar 2008 18:21:55 schrieb Junio C Hamano:
> > Dennis Schridde <devurandom@gmx.net> writes:
> > > Hello!
> > >
> > > I am getting "Odd number of elements in anonymous hash at
> > > /usr/bin/git-svn line 1760." (normal output, no warning/error) during
> > > git-svn-clone. I am using git version 1.5.4.rc2.
> > >
> > > Line 1760 is this (with context, marked with '!!'):
> > >     # 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"),
> > >         }
> > >     };
> > >
> > > The commandline was "git svn
> > > clone --authors-file=/var/git/org.gna.warzone.git/authors
> > > --use-svnsync-props --stdlayout file:///var/svn/warzone2100/
> > > org.gna.warzone.git/"
> > >
> > > I assume this is some kind of bug?
> >
> > More than one svn-remote.$your_repo.svnsync-{url,uuid}?
> PS: It doesn't happen for every revision and the folders (and thus config) 
> were empty before. The config doesn't list any "svnsync-*" during the import 
> process.

Can you look in .git/svn/.metadata for the svnsync-* values?  Thanks.

I downloaded your repository and couldn't reproduce it locally.

-- 
Eric Wong

Re: Odd number of elements in anonymous hash

[Dennis Schridde]

[-- Attachment #1: Type: text/plain, Size: 2223 bytes --]

Am Donnerstag, 10. Januar 2008 09:38:46 schrieb Eric Wong:
> Dennis Schridde <devurandom@gmx.net> wrote:
> > Am Dienstag, 8. Januar 2008 18:21:55 schrieb Junio C Hamano:
> > > Dennis Schridde <devurandom@gmx.net> writes:
> > > > Hello!
> > > >
> > > > I am getting "Odd number of elements in anonymous hash at
> > > > /usr/bin/git-svn line 1760." (normal output, no warning/error) during
> > > > git-svn-clone. I am using git version 1.5.4.rc2.
> > > >
> > > > Line 1760 is this (with context, marked with '!!'):
> > > >     # 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"),
> > > >         }
> > > >     };
> > > >
> > > > The commandline was "git svn
> > > > clone --authors-file=/var/git/org.gna.warzone.git/authors
> > > > --use-svnsync-props --stdlayout file:///var/svn/warzone2100/
> > > > org.gna.warzone.git/"
> > > >
> > > > I assume this is some kind of bug?
> > >
> > > More than one svn-remote.$your_repo.svnsync-{url,uuid}?
> >
> > PS: It doesn't happen for every revision and the folders (and thus
> > config) were empty before. The config doesn't list any "svnsync-*" during
> > the import process.
>
> Can you look in .git/svn/.metadata for the svnsync-* values?  Thanks.
>
> I downloaded your repository and couldn't reproduce it locally.
This is the file left after the segfault from the other mail:
---
[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
---

I will post the whole file asap. (I think recompiling git,svn,perl with 
CFLAGS=-g and not striping the binary magically solves the problem, at least 
it seemed to when I tried to create a backtrace.)

--Dennis

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Odd number of elements in anonymous hash

[Dennis Schridde]

[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]

Am Donnerstag, 10. Januar 2008 12:04:54 schrieb Dennis Schridde:
> Am Donnerstag, 10. Januar 2008 09:38:46 schrieb Eric Wong:
> > Dennis Schridde <devurandom@gmx.net> wrote:
> > > Am Dienstag, 8. Januar 2008 18:21:55 schrieb Junio C Hamano:
> > > > Dennis Schridde <devurandom@gmx.net> writes:
> > > > > I am getting "Odd number of elements in anonymous hash at
> > > > > /usr/bin/git-svn line 1760." (normal output, no warning/error)
> > > > > during git-svn-clone. I am using git version 1.5.4.rc2.
> >
> > Can you look in .git/svn/.metadata for the svnsync-* values?  Thanks.
> >
> > I downloaded your repository and couldn't reproduce it locally.
>
> This is the file left after the segfault from the other mail:
> ---
> [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.

--Dennis

svn, version 1.4.6 (r28521)
git version 1.5.4.rc2
This is perl, v5.8.8 built for i686-linux

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Odd number of elements in anonymous hash

[Junio C Hamano]

Dennis Schridde <devurandom@gmx.net> writes:

>> [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,
		}
	};

Re: Odd number of elements in anonymous hash

[Dennis Schridde]

[-- Attachment #1: Type: text/plain, Size: 3092 bytes --]

Am Donnerstag, 10. Januar 2008 21:08:52 schrieb Junio C Hamano:
> Dennis Schridde <devurandom@gmx.net> writes:
> >> [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)
---

--Dennis

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Odd number of elements in anonymous hash

[Junio C Hamano]

Dennis Schridde <devurandom@gmx.net> writes:

> Am Donnerstag, 10. Januar 2008 21:08:52 schrieb Junio C Hamano:
>> Dennis Schridde <devurandom@gmx.net> writes:
>> >> [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.

[PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Eric Wong]

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 <normalperson@yhbt.net>
---
 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

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:
> 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";

When I checked tmp_config(), I thought that it was returning the
first element (taking the output of "config --get" as a
LF-terminated list) when called in list context, so this would
give the correct sync-url after stripping the trailing LF for
Dennis's case.  The first assignment looks fine, but the second
one looks very funny.

Is it only me who finds that

	($var) = ($var =~ m{^(any regexp)$}) or die "message"

is extremely a roundabout and hard-to-read way to say:

	if ($var !~ m{^(the same regexp)$} {
        	die "message";
	}

which is much easier to read?

I said the first one looks fine, but actually I really _hate_
Perl subs that use wantarray and return one element in scalar
context, and return a list in list context.

This forces the programmer who calls the sub to read the code of
the sub and know that it uses this unusual calling convention.
It forbids them to rely on their common-sense acquired from
experience while coding in Perl, that most functions do not do
that, and a usual sub that is capable of returning a list in a
list context you can find out the number of elemenets, the first
element, and the first few elements by using:

	sub returns_list {
        	my @a = qw(a b c);
                return @a;
	}

	my $number_of_elems = returns_list();
	my ($first_element) = returns_list();
	my ($first, $second) = returns_list();

IOW, writing

	sub returns_funny {
        	my @a = qw(a b c);
                return wantarray ? @a : $a[0];
	}

may make you feel as if you have become a power-coder, and might
even give you a false sense that some parts of your code can
become simpler thanks to that, because you can now write:

	my $first_element = returns_list();

i.e. you did not have to type WHOLE TWO LETTERS ( and )!

But in the longer run, when you want/need other people to help
maintaining such a code, you create unnecessary burden on them.

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Eric Wong]

Junio C Hamano <gitster@pobox.com> wrote:
> Eric Wong <normalperson@yhbt.net> writes:
> > 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";
> 
> When I checked tmp_config(), I thought that it was returning the
> first element (taking the output of "config --get" as a
> LF-terminated list) when called in list context, so this would
> give the correct sync-url after stripping the trailing LF for
> Dennis's case.  The first assignment looks fine, but the second
> one looks very funny.
> 
> Is it only me who finds that
> 
> 	($var) = ($var =~ m{^(any regexp)$}) or die "message"
> 
> is extremely a roundabout and hard-to-read way to say:
> 
> 	if ($var !~ m{^(the same regexp)$} {
>         	die "message";
> 	}
> 
> which is much easier to read?

The statements are not equivalent, however.  I'd have to add

	$var = $1;

too, because I needed to extract what was inside the ( ) since the '$'
doesn't catch the trailing newline, either.  I also couldn't find any of
the /sm, /m, /s switches useful for making '$' not accept the trailing
newline, either.

I also consider the "or die " part of the line to be uninteresting,
and would rather not waste extra space on it.

> I said the first one looks fine, but actually I really _hate_
> Perl subs that use wantarray and return one element in scalar
> context, and return a list in list context.
> 
> This forces the programmer who calls the sub to read the code of
> the sub and know that it uses this unusual calling convention.
> It forbids them to rely on their common-sense acquired from
> experience while coding in Perl, that most functions do not do
> that, and a usual sub that is capable of returning a list in a
> list context you can find out the number of elemenets, the first
> element, and the first few elements by using:
> 
> 	sub returns_list {
>         	my @a = qw(a b c);
>                 return @a;
> 	}
> 
> 	my $number_of_elems = returns_list();
> 	my ($first_element) = returns_list();
> 	my ($first, $second) = returns_list();
> 
> IOW, writing
> 
> 	sub returns_funny {
>         	my @a = qw(a b c);
>                 return wantarray ? @a : $a[0];
> 	}
> 
> may make you feel as if you have become a power-coder, and might
> even give you a false sense that some parts of your code can
> become simpler thanks to that, because you can now write:
> 
> 	my $first_element = returns_list();
> 
> i.e. you did not have to type WHOLE TWO LETTERS ( and )!
> 
> But in the longer run, when you want/need other people to help
> maintaining such a code, you create unnecessary burden on them.

Good points, I've been mindlessly taking "interesting" things from other
Perl code I've seen over the years and using it in my own without
thinking about it too hard :x

I'll avoid them in the future.  Unfortunately, Git.pm also suffers from
this as well.

-- 
Eric Wong

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Björn Steinbrink]

On 2008.01.12 01:12:42 -0800, Eric Wong wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > Is it only me who finds that
> > 
> > 	($var) = ($var =~ m{^(any regexp)$}) or die "message"
> > 
> > is extremely a roundabout and hard-to-read way to say:
> > 
> > 	if ($var !~ m{^(the same regexp)$} {
> >         	die "message";
> > 	}
> > 
> > which is much easier to read?
> 
> The statements are not equivalent, however.  I'd have to add
> 
> 	$var = $1;
> 
> too, because I needed to extract what was inside the ( ) since the '$'
> doesn't catch the trailing newline, either.  I also couldn't find any of
> the /sm, /m, /s switches useful for making '$' not accept the trailing
> newline, either.

The \z assertion will do, eg m/^foo\z/ will only match exactly "foo",
not "foo\n". As it is a zero-width assertion, you can also write
m/^foo\z$/ if you prefer that.

Björn

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Dennis Schridde]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

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

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Junio C Hamano]

Eric Wong <normalperson@yhbt.net> writes:

> The statements are not equivalent, however.  I'd have to add
>
> 	$var = $1;
>
> too, because I needed to extract what was inside the ( ) since the '$'
> doesn't catch the trailing newline, either.

Ahh, _stupid me_.

Yes, you said '$', not '\Z', but somehow I mistook m|^(.*)$| as
a no-op "whole thing".  Sorry.

> Good points, I've been mindlessly taking "interesting" things from other
> Perl code I've seen over the years and using it in my own without
> thinking about it too hard :x
>
> I'll avoid them in the future.  Unfortunately, Git.pm also suffers from
> this as well.

Git.pm is even worse.  It uses the line-noise prototype which is
a very good and cute hack to allow people to (1) emulate Perl's
built-in and (2) come up with syntax sugars, but has a similar
issue that defeats old-school intuition as wantarray-return
subroutines does.

The caller needs to be careful about receiving return values
with wantarray-return subroutines.  The caller needs to be
careful about how to send in the parameters with line-noise
prototyped subs.

In any case, this kind of clean-up is not within the scope of
changes during rc cycle.  I'll take your bugfix as is.

Thanks.

Re: [PATCH] git-svn: handle leading/trailing whitespace from svnsync revprops

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Git.pm is even worse.  It uses the line-noise prototype...

Sorry, I misremembered.  Git.pm is not a prototype offender.

Re: Odd number of elements in anonymous hash

[Sam Vilain]

Dennis Schridde wrote:
>>> Server: git config:
>>> ------
 [...]
>>> [svn-remote "svn"]
>>>         useSvnsyncProps = 1
>>>         url = file:///var/svn/warzone2100
>>>         fetch = trunk:refs/remotes/trunk
>>>         branches = branches/*:refs/remotes/*
>>>         tags = tags/*:refs/remotes/tags/*
>>> ------
 [...]
>>> Local config:
>>> ------
 [...]
>>> [remote "origin"]
>>>         url = git://git.kynes.de/org.gna.warzone2100.git
>>>         fetch = +refs/heads/*:refs/remotes/origin/*
>> You need to make that:
>>
>>   fetch = refs/remotes/*:refs/remotes/*
>>
>> And re-run 'git fetch' to use git-svn fetch on the cloned repository.
>> git-svn is expecting the refs/ space to be populated in the same way
>> that it is on the repository doing the initial fetch.
> Thanks a lot!
> That worked!
> 
> Possibly you should update the manpage, so no others run into this.
> It currently just says:
>        # Tell git-svn which branch contains the Subversion commits
>                git update-ref refs/remotes/git-svn origin/master
> Which is apparently not sufficient (or outdated?).

Indeed, that would be helpful.

Subject: [PATCH] git-svn(1): update instructions for resuming a git-svn clone

git-svn expects its references under refs/remotes/*; but these will
not be copied or set by "git clone"; put in this man page the manual
fiddling that is required with current git-svn to get this to work.

Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
 Documentation/git-svn.txt |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index e1a1d46..b1d527f 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -456,10 +456,13 @@ have each person clone that repository with 'git clone':
 ------------------------------------------------------------------------
 # Do the initial import on a server
 	ssh server "cd /pub && git-svn clone http://svn.foo.org/project
-# Clone locally
-	git clone server:/pub/project
-# Tell git-svn which branch contains the Subversion commits
-	git update-ref refs/remotes/git-svn origin/master
+# Clone locally - make sure the refs/remotes/ space matches the server
+	mkdir project
+	cd project
+	git-init
+	git remote add origin server:/pub/project
+	git config --add remote.origin.fetch=+refs/remotes/*:refs/remotes/*
+	git fetch
 # Initialize git-svn locally (be sure to use the same URL and -T/-b/-t options as were used on server)
 	git-svn init http://svn.foo.org/project
 # Pull the latest changes from Subversion
-- 
1.5.3.5