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

Jonathan Nieder· Jun 13, 2010, 11:27 UTC · lore

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(-)

Show changes to git-svn.perl +1 −0

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

Eric Wong· Jun 14, 2010, 04:57 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 39 quoted lines

> 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

Show 5 quoted lines

> ---
> 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(-)

Show changes to git-svn.perl +3 −0

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

RFC patchgit-svn: strip off leading slashes on --trunk argument