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

From: Eric Wong <normalperson@yhbt.net>
Date: Jun 14, 2010, 04:57 UTC
Message-ID: <20100614045735.GA22999@dcvr.yhbt.net>
In-Reply-To: <20100613112743.GA21855@burratino>

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

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