[PATCH/RFC] git-svn: strip off leading slashes on --trunk argument

[PATCH/RFC] git-svn: strip off leading slashes on --trunk argument

[Jonathan Nieder]

The following command

 git svn clone \
	-r9500:10006 \
	svn://svn.debian.org/svn/pkg-games/packages \
	--trunk=/trunk/freedoom \
	--branches=/branches/freedoom \
	--tags=/tags/freedoom \
	freedoom.git.2009091

produces strange results:

With v1.6.3.3 (and perhaps earlier versions), this would fetch up to
and including r9978 (the last revision of the no_iwad_alternatives
branch before it was deleted), check it out, and prematurely declare
success, leaving out some commits to the trunk (r9984, r9985, r10006)
from after the branch was merged.

With v1.6.5-rc0~74 (svn: allow branches outside of refs/remotes,
2009-08-11) and later, this fetches up to and including r9978 and then
attempts a post-fetch checkout and fails.

 r9978 = 25f0920175c395f0f22f54ae7a2318147f745274
 (refs/remotes/no_iwad_alternatives)
 fatal: refs/remotes/trunk: not a valid SHA1
 update-ref refs/heads/master refs/remotes/trunk: command returned error: 128

Checking .git/config reveals

 fetch = packages//trunk/freedoom:refs/remotes/trunk

And with both 1.6.3.3 and 1.7.1, using --trunk=trunk/freedom without
the leading slash (/) works fine.

Moral: git-svn needs to scrub an initial / from $_trunk and related
arguments it receives.  Make it so.

Reported-by: Jon Dowland <jmtd@debian.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi Eric,

It’s not clear to me what’s going on here, but this seems to fix it.  Insights?

Jonathan

From

  http://bugs.debian.org/546733
  http://bugs.debian.org/572847

 git-svn.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 265852f..a352000 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -959,6 +959,7 @@ sub cmd_multi_init {
 	}
 	do_git_init_db();
 	if (defined $_trunk) {
+		$_trunk =~ s#^/+##;
 		my $trunk_ref = 'refs/remotes/' . $_prefix . 'trunk';
 		# try both old-style and new-style lookups:
 		my $gs_trunk = eval { Git::SVN->new($trunk_ref) };
-- 
1.7.0.dirty

Re: [PATCH/RFC] git-svn: strip off leading slashes on --trunk argument

[Eric Wong]

Jonathan Nieder <jrnieder@gmail.com> wrote:
> The following command
> 
>  git svn clone \
> 	-r9500:10006 \
> 	svn://svn.debian.org/svn/pkg-games/packages \
> 	--trunk=/trunk/freedoom \
> 	--branches=/branches/freedoom \
> 	--tags=/tags/freedoom \
> 	freedoom.git.2009091
> 
> produces strange results:
> 
> With v1.6.3.3 (and perhaps earlier versions), this would fetch up to
> and including r9978 (the last revision of the no_iwad_alternatives
> branch before it was deleted), check it out, and prematurely declare
> success, leaving out some commits to the trunk (r9984, r9985, r10006)
> from after the branch was merged.
> 
> With v1.6.5-rc0~74 (svn: allow branches outside of refs/remotes,
> 2009-08-11) and later, this fetches up to and including r9978 and then
> attempts a post-fetch checkout and fails.
> 
>  r9978 = 25f0920175c395f0f22f54ae7a2318147f745274
>  (refs/remotes/no_iwad_alternatives)
>  fatal: refs/remotes/trunk: not a valid SHA1
>  update-ref refs/heads/master refs/remotes/trunk: command returned error: 128
> 
> Checking .git/config reveals
> 
>  fetch = packages//trunk/freedoom:refs/remotes/trunk
> 
> And with both 1.6.3.3 and 1.7.1, using --trunk=trunk/freedom without
> the leading slash (/) works fine.
> 
> Moral: git-svn needs to scrub an initial / from $_trunk and related
> arguments it receives.  Make it so.
> 
> Reported-by: Jon Dowland <jmtd@debian.org>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

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

...and pushed out to git://git.bogomips.org/git-svn along with
one follow-up patch:

Eric Wong (1):
      git svn: avoid unnecessary '/' in paths for SVN

Jonathan Nieder (1):
      git-svn: strip off leading slashes on --trunk argument

> ---
> Hi Eric,
> 
> It’s not clear to me what’s going on here, but this seems to fix it.
> Insights?

The svn:// server is more picky about the "//" than http:// or file://.

So we were skipping fetches from trunk completely, and never created
a ref for it.  Since git-svn attempts to checkout a "master" ref based
on trunk, it fails since trunk is a non-existent ref.

From b1a954a37cea7d5a0a123758f6c2ad9005d4481e Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Mon, 14 Jun 2010 04:31:10 +0000
Subject: [PATCH] git svn: avoid unnecessary '/' in paths for SVN

svn:// servers are more picky regarding redundant slashes
than file:// and http(s)://-backed respositories.  Since
the last commit, we avoid putting unnecessary slashes in
$GIT_CONFIG, but this doesn't help users who are already
set up that way.

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

diff --git a/git-svn.perl b/git-svn.perl
index 80ab450..19d6848 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -2055,6 +2055,9 @@ sub new {
 		         "\":$ref_id\$\" in config\n";
 		($self->{path}, undef) = split(/\s*:\s*/, $fetch);
 	}
+	$self->{path} =~ s{/+}{/}g;
+	$self->{path} =~ s{\A/}{};
+	$self->{path} =~ s{/\z}{};
 	$self->{url} = command_oneline('config', '--get',
 	                               "svn-remote.$repo_id.url") or
                   die "Failed to read \"svn-remote.$repo_id.url\" in config\n";
-- 
Eric Wong