git-svn fetch doesn't like spaces in branch names

mini:TextMateBundles mjc$ git-svn fetch
Found possible branch point: http://macromates.com/svn/Bundles/trunk/Tools/Dialog 
  PlugIn => http://macromates.com/svn/Bundles/branches/Dialog PlugIn  
Completion Menu, 8089
Initializing parent: Dialog PlugIn Completion Menu@8089
Bad URL passed to RA layer: Malformed URL for repository at /opt/local/ 
bin/git-svn line 1607

looks like that might need to be %20 ?

-mjc

mini:TextMateBundles mjc$ git-svn fetch
Found possible branch point: http://macromates.com/svn/Bundles/trunk/Tools/Dialog 
 PlugIn => http://macromates.com/svn/Bundles/branches/Dialog PlugIn  
Completion Menu, 8089
Initializing parent: Dialog PlugIn Completion Menu@8089
Bad URL passed to RA layer: Malformed URL for repository at /opt/ 
local/bin/git-svn line 1607

looks like that might need to be %20 ?

Hacky, but it works.

Signed-off-by: Michael J. Cohen <redacted>

diff --git a/git-svn.perl b/git-svn.perl
index dd93e32..5dc3b9c 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -1976,6 +1976,7 @@ sub find_parent_branch {

	my $r = $i->{copyfrom_rev};
	my $repos_root = $self->ra->{repos_root};
	my $url = $self->ra->{url};
+	$branch_from =~ s@([\s])@sprintf("%%%02X", ord($1))@seg;
	my $new_url = $repos_root . $branch_from;
	print STDERR  "Found possible branch point: ",
	              "$new_url => ", $self->full_url, ", $r\n";
-- 
1.5.3.5.1590.gfadfad-dirty

Michael J. Cohen, Thu, Nov 08, 2007 01:53:07 +0100:

You don't need "[" and "]".

On Nov 8, 2007, at 8:29 AM, Alex Riesen wrote:

Michael J. Cohen, Thu, Nov 08, 2007 01:53:07 +0100:

quoted

quoted

mini:TextMateBundles mjc$ git-svn fetch
Found possible branch point:
http://macromates.com/svn/Bundles/trunk/Tools/Dialog PlugIn =>
http://macromates.com/svn/Bundles/branches/Dialog PlugIn  
Completion Menu,
8089
Initializing parent: Dialog PlugIn Completion Menu@8089
Bad URL passed to RA layer: Malformed URL for repository at
/opt/local/bin/git-svn line 1607

looks like that might need to be %20 ?

Hacky, but it works.

Signed-off-by: Michael J. Cohen <redacted>

diff --git a/git-svn.perl b/git-svn.perl
index dd93e32..5dc3b9c 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -1976,6 +1976,7 @@ sub find_parent_branch {

	my $r = $i->{copyfrom_rev};
	my $repos_root = $self->ra->{repos_root};
	my $url = $self->ra->{url};
+	$branch_from =~ s@([\s])@sprintf("%%%02X", ord($1))@seg;

You don't need "[" and "]".

You don't even need the "(" and ")"

$branch_from =~ s@\s@sprintf("%%%02X", ord($&))@seg;

But I think it'd be better to fix this properly.  I guess some people  
use branch names with accentuated characters such as é è ü whatever.   
What about this instead (untested):

$branch_from =~ s@[^\w\d_]@sprintf("%%%02X", ord($&))@seg;

Otherwise there are various existing Perl modules such as http:// 
search.cpan.org/dist/URI/URI/Escape.pm but this seems overkill / not  
portable (unless we distribute these files along with Git).

Cheers,

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory

SVN requires that paths be URI-escaped for HTTP(S) repositories.
file:// and svn:// repositories do not need these rules.

Additionally, accessing individual paths inside repositories
(check_path() and get_log() do NOT require escapes to function
and in fact it breaks things).

Noticed-by: Michael J. Cohen [off-list ref]
Signed-off-by: Eric Wong <redacted>
---
 git-svn.perl                          |   24 ++++++++++++++++++-
 t/t9118-git-svn-funky-branch-names.sh |   40 +++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 2 deletions(-)
 create mode 100755 t/t9118-git-svn-funky-branch-names.sh

  "Michael J. Cohen" [off-list ref] wrote:
  > mini:TextMateBundles mjc$ git-svn fetch
  > Found possible branch point: 
  > http://macromates.com/svn/Bundles/trunk/Tools/Dialog PlugIn => 
  >  http://macromates.com/svn/Bundles/branches/Dialog PlugIn  Completion Menu, 
  > 8089
  > Initializing parent: Dialog PlugIn Completion Menu@8089
  > Bad URL passed to RA layer: Malformed URL for repository at /opt/local/ 
  > bin/git-svn line 1607
  > 
  > looks like that might need to be %20 ?
  > 
  > -mjc

diff --git a/git-svn.perl b/git-svn.perl
index 7589276..9ffda3e 100755
--- a/git-svn.perl
+++ b/git-svn.perl

@@ -3223,6 +3223,25 @@ sub _auth_providers () {
 	]
 }
 
+sub escape_uri_only {
+	my ($uri) = @_;
+	my @tmp;
+	foreach (split m{/}, $uri) {
+		s/([^\w.-])/sprintf("%%%02X",ord($1))/eg;
+		push @tmp, $_;
+	}
+	join('/', @tmp);
+}
+
+sub escape_url {
+	my ($url) = @_;
+	if ($url =~ m#^(https?)://([^/]+)(.*)$#) {
+		my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
+		$url = "$scheme://$domain$uri";
+	}
+	$url;
+}
+
 sub new {
 	my ($class, $url) = @_;
 	$url =~ s!/+$!!;

@@ -3255,10 +3274,11 @@ sub new {
 			$Git::SVN::Prompt::_no_auth_cache = 1;
 		}
 	} # no warnings 'once'
-	my $self = SVN::Ra->new(url => $url, auth => $baton,
+	my $self = SVN::Ra->new(url => escape_url($url), auth => $baton,
 	                      config => $config,
 			      pool => SVN::Pool->new,
 	                      auth_provider_callbacks => $callbacks);
+	$self->{url} = $url;
 	$self->{svn_path} = $url;
 	$self->{repos_root} = $self->get_repos_root;
 	$self->{svn_path} =~ s#^\Q$self->{repos_root}\E(/|$)##;

@@ -3384,7 +3404,7 @@ sub gs_do_switch {
 
 	my $full_url = $self->{url};
 	my $old_url = $full_url;
-	$full_url .= "/$path" if length $path;
+	$full_url .= '/' . escape_uri_only($path) if length $path;
 	my ($ra, $reparented);
 	if ($old_url ne $full_url) {
 		if ($old_url !~ m#^svn(\+ssh)?://#) {

diff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh
new file mode 100755
index 0000000..640bb06
--- /dev/null
+++ b/t/t9118-git-svn-funky-branch-names.sh

@@ -0,0 +1,40 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Eric Wong
+#
+
+test_description='git-svn funky branch names'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup svnrepo' "
+	mkdir project project/trunk project/branches project/tags &&
+	echo foo > project/trunk/foo &&
+	svn import -m '$test_description' project \"$svnrepo/pr ject\" &&
+	rm -rf project &&
+	svn cp -m 'fun' \"$svnrepo/pr ject/trunk\" \
+	                \"$svnrepo/pr ject/branches/fun plugin\" &&
+	svn cp -m 'more fun!' \"$svnrepo/pr ject/branches/fun plugin\" \
+	                      \"$svnrepo/pr ject/branches/more fun plugin!\" &&
+	start_httpd
+	"
+
+test_expect_success 'test clone with funky branch names' "
+	git svn clone -s \"$svnrepo/pr ject\" project &&
+	cd project &&
+		git rev-parse 'refs/remotes/fun%20plugin' &&
+		git rev-parse 'refs/remotes/more%20fun%20plugin!' &&
+	cd ..
+	"
+
+test_expect_success 'test dcommit to funky branch' "
+	cd project &&
+	git reset --hard 'refs/remotes/more%20fun%20plugin!' &&
+	echo hello >> foo &&
+	git commit -m 'hello' -- foo &&
+	git svn dcommit &&
+	cd ..
+	"
+
+stop_httpd
+
+test_done

-- 
Eric Wong