[PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Gerrit Pape]

With subversion 1.5.0 (C and perl libraries) the git-svn selftest
t9101-git-svn-props.sh fails at test 25 and 26.  The following commands
cause assertions in the svn library

 $ cd deeply
 $ git-svn propget svn:ignore .
 perl: /build/buildd/subversion-1.5.0dfsg1/subversion/libsvn_ra/ra_loader.c:674: svn_ra_get_dir: Assertion `*path != '/'' failed.
 Aborted

 $ git-svn propget svn:ignore ..
 perl: /build/buildd/subversion-1.5.0dfsg1/subversion/libsvn_subr/path.c:120: svn_path_join: Assertion `is_canonical(component, clen)' failed.

With this commit, git-svn makes sure the path doesn't start with a
slash, and is not a dot, working around these assertions.

The breakage was reported by Lucas Nussbaum through
 http://bugs.debian.org/489108

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 git-svn.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

I ran into this on Debian/unstable.  With svn 1.5.0 the selftest fails
without the patch, with svn 1.4.6 it succeeds with and without the
patch.  I'm not familar with the svn interfaces, not sure whether this
is a regression in subversion, or a bug in git-svn.


diff --git a/git-svn.perl b/git-svn.perl
index f789a6e..a366c89 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -643,6 +643,8 @@ sub canonicalize_path {
 	$path =~ s#/[^/]+/\.\.##g;
 	$path =~ s#/$##g;
 	$path =~ s#^\./## if $dot_slash_added;
+	$path =~ s#^/##;
+	$path =~ s#^\.$##;
 	return $path;
 }
 
-- 
1.5.6

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Eric Wong]

Gerrit Pape <pape@smarden.org> wrote:
> With subversion 1.5.0 (C and perl libraries) the git-svn selftest
> t9101-git-svn-props.sh fails at test 25 and 26.  The following commands
> cause assertions in the svn library
> 
>  $ cd deeply
>  $ git-svn propget svn:ignore .
>  perl: /build/buildd/subversion-1.5.0dfsg1/subversion/libsvn_ra/ra_loader.c:674: svn_ra_get_dir: Assertion `*path != '/'' failed.
>  Aborted
> 
>  $ git-svn propget svn:ignore ..
>  perl: /build/buildd/subversion-1.5.0dfsg1/subversion/libsvn_subr/path.c:120: svn_path_join: Assertion `is_canonical(component, clen)' failed.
> 
> With this commit, git-svn makes sure the path doesn't start with a
> slash, and is not a dot, working around these assertions.
> 
> The breakage was reported by Lucas Nussbaum through
>  http://bugs.debian.org/489108
> 
> Signed-off-by: Gerrit Pape <pape@smarden.org>

Thanks Gerrit,

This works with svn 1.4.3 here

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

> ---
>  git-svn.perl |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> I ran into this on Debian/unstable.  With svn 1.5.0 the selftest fails
> without the patch, with svn 1.4.6 it succeeds with and without the
> patch.  I'm not familar with the svn interfaces, not sure whether this
> is a regression in subversion, or a bug in git-svn.
> 
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index f789a6e..a366c89 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -643,6 +643,8 @@ sub canonicalize_path {
>  	$path =~ s#/[^/]+/\.\.##g;
>  	$path =~ s#/$##g;
>  	$path =~ s#^\./## if $dot_slash_added;
> +	$path =~ s#^/##;
> +	$path =~ s#^\.$##;
>  	return $path;
>  }
>  
> -- 
> 1.5.6

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Gerrit Pape]

On Sun, Jul 06, 2008 at 07:28:50PM +0000, Gerrit Pape wrote:
> With subversion 1.5.0 (C and perl libraries) the git-svn selftest
> t9101-git-svn-props.sh fails at test 25 and 26.  The following commands

> The breakage was reported by Lucas Nussbaum through
>  http://bugs.debian.org/489108

> ---
>  git-svn.perl |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> I ran into this on Debian/unstable.  With svn 1.5.0 the selftest fails
> without the patch, with svn 1.4.6 it succeeds with and without the
> patch.  I'm not familar with the svn interfaces, not sure whether this
> is a regression in subversion, or a bug in git-svn.

Hi, while this commit fixed the selftests, it unfortunately is a
regression

 http://thread.gmane.org/gmane.comp.version-control.git/87822/
 http://bugs.debian.org/490400

I'm still not sure whether this is a git-svn problem, or actually a
problem in subversion 1.5.0

 $ perl -MSVN::Client -e \
 'sub print_names { print "<$_[0]>\n"; } $ctx=new SVN::Client;
 $ctx->status(".", "BASE", \&print_names, 1, 1, 0, 1);' | head -5
 perl:
 /build/buildd/subversion-1.5.0dfsg1/subversion/libsvn_subr/path.c:119: svn_path_join: Assertion `is_canonical(base, blen)' failed.
 $ 
 [0]

Regards, Gerrit.

[0] http://bugs.debian.org/359679

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Dmitry Potapov]

On Thu, Jul 17, 2008 at 6:08 PM, Gerrit Pape <pape@smarden.org> wrote:
>
> Hi, while this commit fixed the selftests, it unfortunately is a
> regression
>
>  http://thread.gmane.org/gmane.comp.version-control.git/87822/
>  http://bugs.debian.org/490400

This particular breakage is easy to fix:

diff --git a/git-svn.perl b/git-svn.perl
index 3750e47..a5a5b1b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1226,7 +1226,7 @@ sub linearize_history {

 sub find_file_type_and_diff_status {
        my ($path) = @_;
-       return ('dir', '') if $path eq '.';
+       return ('dir', '') if $path eq '';

        my $diff_output =
            command_oneline(qw(diff --cached --name-status --), $path) || "";

but it could be some other places in git-svn that may need correction too.

>
> I'm still not sure whether this is a git-svn problem, or actually a
> problem in subversion 1.5.0

Accordingly to SVN developers you do not use SVN API correctly,
therefore you got assert. So, the problem in git-svn.
http://svn.haxx.se/dev/archive-2008-01/0425.shtml

Dmitry

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Junio C Hamano]

"Dmitry Potapov" <dpotapov@gmail.com> writes:

> On Thu, Jul 17, 2008 at 6:08 PM, Gerrit Pape <pape@smarden.org> wrote:
>>
>> Hi, while this commit fixed the selftests, it unfortunately is a
>> regression
>>
>>  http://thread.gmane.org/gmane.comp.version-control.git/87822/
>>  http://bugs.debian.org/490400
>
> This particular breakage is easy to fix:
>
> diff --git a/git-svn.perl b/git-svn.perl
> index 3750e47..a5a5b1b 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -1226,7 +1226,7 @@ sub linearize_history {
>
>  sub find_file_type_and_diff_status {
>         my ($path) = @_;
> -       return ('dir', '') if $path eq '.';
> +       return ('dir', '') if $path eq '';
>
>         my $diff_output =
>             command_oneline(qw(diff --cached --name-status --), $path) || "";
>
> but it could be some other places in git-svn that may need correction too.
>
>>
>> I'm still not sure whether this is a git-svn problem, or actually a
>> problem in subversion 1.5.0
>
> Accordingly to SVN developers you do not use SVN API correctly,
> therefore you got assert. So, the problem in git-svn.
> http://svn.haxx.se/dev/archive-2008-01/0425.shtml

So what's the conclusion of this issue?

I'll just revert 2fe403e (git-svn.perl: workaround assertions in svn
library 1.5.0, 2008-07-06) for 1.6.0-rc0 unless I hear better
suggestions.

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Dmitry Potapov]

On Sat, Jul 19, 2008 at 06:27:36PM -0700, Junio C Hamano wrote:
> 
> So what's the conclusion of this issue?
> 
> I'll just revert 2fe403e (git-svn.perl: workaround assertions in svn
> library 1.5.0, 2008-07-06) for 1.6.0-rc0 unless I hear better
> suggestions.

I have tested the change that I proposed, and it seems to solve the
problem and, as far as I can tell, no other correction is necessary.
Yet, I don't really understand git-svn well, so I could be wrong.

Reverting 2fe403e will only help users of svn library 1.4, while all
new linux distributives, which will include Git 1.6.0, are going to
install svn library 1.5.0, and if you use svn library 1.5.0, reverting
2fe403e does not fix anything but only add one more bug. Thus, unless
we are going to require to install git-svn only with svn library 1.4,
reverting this change does not seem to be very helpful for most users.

So, I hope my patch is better solution...

Dmitry

-- 8< --
From: Dmitry Potapov <dpotapov@gmail.com>
Date: Thu, 17 Jul 2008 22:32:02 +0400
Subject: [PATCH] git-svn: fix git svn info to work without arguments

commit 2fe403e7452bd6e1e8232445cf5434ce8f1af973 broke "git-svn info ."
due to replacing '.' with '' in canonicalize_path for the top directory,
while find_file_type_and_diff_status was not corrected.

Bug reports:

http://thread.gmane.org/gmane.comp.version-control.git/87822/
http://bugs.debian.org/490400

Signed-off-by: Dmitry Potapov <dpotapov@gmail.com>
---
 git-svn.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index a366c89..b00a91a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1226,7 +1226,7 @@ sub linearize_history {
 
 sub find_file_type_and_diff_status {
 	my ($path) = @_;
-	return ('dir', '') if $path eq '.';
+	return ('dir', '') if $path eq '';
 
 	my $diff_output =
 	    command_oneline(qw(diff --cached --name-status --), $path) || "";
-- 
1.5.6.3

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Eric Wong]

Dmitry Potapov <dpotapov@gmail.com> wrote:
> On Sat, Jul 19, 2008 at 06:27:36PM -0700, Junio C Hamano wrote:
> > 
> > So what's the conclusion of this issue?
> > 
> > I'll just revert 2fe403e (git-svn.perl: workaround assertions in svn
> > library 1.5.0, 2008-07-06) for 1.6.0-rc0 unless I hear better
> > suggestions.
> 
> I have tested the change that I proposed, and it seems to solve the
> problem and, as far as I can tell, no other correction is necessary.
> Yet, I don't really understand git-svn well, so I could be wrong.
> 
> Reverting 2fe403e will only help users of svn library 1.4, while all
> new linux distributives, which will include Git 1.6.0, are going to
> install svn library 1.5.0, and if you use svn library 1.5.0, reverting
> 2fe403e does not fix anything but only add one more bug. Thus, unless
> we are going to require to install git-svn only with svn library 1.4,
> reverting this change does not seem to be very helpful for most users.
> 
> So, I hope my patch is better solution...
> 
> Dmitry

Thanks Dmitry,

Your patch works for me on 1.4.3, so if it works with
1.5.0, consider it: Acked-by: Eric Wong <normalperson@yhbt.net>

-- 
Eric Wong

Re: [PATCH/rfc] git-svn.perl: workaround assertions in svn library 1.5.0

[Junio C Hamano]

Thanks, both.