From 6a8d999ed4a0030ab7902382ed3b122ce448e581 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Fri, 27 Jul 2012 13:00:51 -0700
Subject: use Git::SVN->path accessor globally

No functional change.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 5711c5719f..039623e5c5 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1195,7 +1195,7 @@ sub cmd_show_ignore {
 	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
 	$gs ||= Git::SVN->new;
 	my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum);
-	$gs->prop_walk($gs->{path}, $r, sub {
+	$gs->prop_walk($gs->path, $r, sub {
 		my ($gs, $path, $props) = @_;
 		print STDOUT "\n# $path\n";
 		my $s = $props->{'svn:ignore'} or return;
@@ -1211,7 +1211,7 @@ sub cmd_show_externals {
 	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
 	$gs ||= Git::SVN->new;
 	my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum);
-	$gs->prop_walk($gs->{path}, $r, sub {
+	$gs->prop_walk($gs->path, $r, sub {
 		my ($gs, $path, $props) = @_;
 		print STDOUT "\n# $path\n";
 		my $s = $props->{'svn:externals'} or return;
@@ -1226,7 +1226,7 @@ sub cmd_create_ignore {
 	my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
 	$gs ||= Git::SVN->new;
 	my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum);
-	$gs->prop_walk($gs->{path}, $r, sub {
+	$gs->prop_walk($gs->path, $r, sub {
 		my ($gs, $path, $props) = @_;
 		# $path is of the form /path/to/dir/
 		$path = '.' . $path;
@@ -1396,7 +1396,7 @@ sub cmd_commit_diff {
 			      "the command-line\n", $usage);
 		}
 		$url = $gs->{url};
-		$svn_path = $gs->{path};
+		$svn_path = $gs->path;
 	}
 	unless (defined $_revision) {
 		fatal("-r|--revision is a required argument\n", $usage);
@@ -1634,6 +1634,8 @@ sub post_fetch_checkout {
 sub complete_svn_url {
 	my ($url, $path) = @_;
 	$path =~ s#/+$##;
+
+	# If the path is not a URL...
 	if ($path !~ m#^[a-z\+]+://#) {
 		if (!defined $url || $url !~ m#^[a-z\+]+://#) {
 			fatal("E: '$path' is not a complete URL ",
@@ -1670,7 +1672,7 @@ sub complete_url_ls_init {
 		    "wanted to set to: $gs->{url}\n";
 	}
 	command_oneline('config', $k, $gs->{url}) unless $orig_url;
-	my $remote_path = "$gs->{path}/$repo_path";
+	my $remote_path = $gs->path . "/$repo_path";
 	$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
 	$remote_path =~ s#/+#/#g;
 	$remote_path =~ s#^/##g;
-- 
cgit v1.2.3


From b1ea6c3829109aff412095b889432bf496f46a90 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Fri, 27 Jul 2012 13:00:52 -0700
Subject: use Git::SVN{,::RA}->url accessor globally

Note: The structure returned from Git::SVN->read_all_remotes() does not
appear to contain objects, so I'm leaving them alone.

That's everything converted over to the url and path accessors.

No functional change.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl              | 11 ++++++-----
 perl/Git/SVN.pm           | 11 ++++++-----
 perl/Git/SVN/Migration.pm |  6 +++---
 3 files changed, 15 insertions(+), 13 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 039623e5c5..de1ddd1051 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1395,7 +1395,7 @@ sub cmd_commit_diff {
 			fatal("Needed URL or usable git-svn --id in ",
 			      "the command-line\n", $usage);
 		}
-		$url = $gs->{url};
+		$url = $gs->url;
 		$svn_path = $gs->path;
 	}
 	unless (defined $_revision) {
@@ -1663,15 +1663,16 @@ sub complete_url_ls_init {
 			      "and a separate URL is not specified");
 		}
 	}
-	my $url = $ra->{url};
+	my $url = $ra->url;
 	my $gs = Git::SVN->init($url, undef, undef, undef, 1);
 	my $k = "svn-remote.$gs->{repo_id}.url";
 	my $orig_url = eval { command_oneline(qw/config --get/, $k) };
-	if ($orig_url && ($orig_url ne $gs->{url})) {
+	if ($orig_url && ($orig_url ne $gs->url)) {
 		die "$k already set: $orig_url\n",
-		    "wanted to set to: $gs->{url}\n";
+		    "wanted to set to: $gs->url\n";
 	}
-	command_oneline('config', $k, $gs->{url}) unless $orig_url;
+	command_oneline('config', $k, $gs->url) unless $orig_url;
+
 	my $remote_path = $gs->path . "/$repo_path";
 	$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
 	$remote_path =~ s#/+#/#g;
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index a16acbbbff..fc1ac07136 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -560,7 +560,7 @@ sub _set_svm_vars {
 		# username is of no interest
 		$src =~ s{(^[a-z\+]*://)[^/@]*@}{$1};
 
-		my $replace = $ra->{url};
+		my $replace = $ra->url;
 		$replace .= "/$path" if length $path;
 
 		my $section = "svn-remote.$self->{repo_id}";
@@ -599,16 +599,17 @@ sub _set_svm_vars {
 	$path = $ra->{svn_path};
 	$ra = Git::SVN::Ra->new($ra->{repos_root});
 	while (length $path) {
-		unless ($tried{"$ra->{url}/$path"}) {
+		my $try = $ra->url ."/$path";
+		unless ($tried{$try}) {
 			$ok = $self->read_svm_props($ra, $path, $r);
 			last if $ok;
-			$tried{"$ra->{url}/$path"} = 1;
+			$tried{$try} = 1;
 		}
 		$path =~ s#/?[^/]+$##;
 	}
 	die "Path: '$path' should be ''\n" if $path ne '';
 	$ok ||= $self->read_svm_props($ra, $path, $r);
-	$tried{"$ra->{url}/$path"} = 1;
+	$tried{$ra->url ."/$path"} = 1;
 	if (!$ok) {
 		die @err, (map { "  $_\n" } keys %tried), "\n";
 	}
@@ -1108,7 +1109,7 @@ sub find_parent_branch {
 	}
 	my $r = $i->{copyfrom_rev};
 	my $repos_root = $self->ra->{repos_root};
-	my $url = $self->ra->{url};
+	my $url = $self->ra->url;
 	my $new_url = $url . $branch_from;
 	print STDERR  "Found possible branch point: ",
 	              "$new_url => ", $self->full_url, ", $r\n"
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
index 75d74298ea..30daf35465 100644
--- a/perl/Git/SVN/Migration.pm
+++ b/perl/Git/SVN/Migration.pm
@@ -177,14 +177,14 @@ sub minimize_connections {
 		my $ra = Git::SVN::Ra->new($url);
 
 		# skip existing cases where we already connect to the root
-		if (($ra->{url} eq $ra->{repos_root}) ||
+		if (($ra->url eq $ra->{repos_root}) ||
 		    ($ra->{repos_root} eq $repo_id)) {
-			$root_repos->{$ra->{url}} = $repo_id;
+			$root_repos->{$ra->url} = $repo_id;
 			next;
 		}
 
 		my $root_ra = Git::SVN::Ra->new($ra->{repos_root});
-		my $root_path = $ra->{url};
+		my $root_path = $ra->url;
 		$root_path =~ s#^\Q$ra->{repos_root}\E(/|$)##;
 		foreach my $path (keys %$fetch) {
 			my $ref_id = $fetch->{$path};
-- 
cgit v1.2.3


From 91e6e0c56cc83e5f53d93e90726380a2d392f5f1 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:38:26 -0700
Subject: git-svn: move canonicalization to Git::SVN::Utils

So they can be used by others.

I'd like to test them, but they're going to become SVN API wrappers shortly
and those aren't predictable.

No functional change.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl          | 33 +++++++-------------------------
 perl/Git/SVN/Utils.pm | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 27 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index de1ddd1051..a857484c58 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -29,7 +29,13 @@ use Git::SVN::Prompt;
 use Git::SVN::Log;
 use Git::SVN::Migration;
 
-use Git::SVN::Utils qw(fatal can_compress);
+use Git::SVN::Utils qw(
+	fatal
+	can_compress
+	canonicalize_path
+	canonicalize_url
+);
+
 use Git qw(
 	git_cmd_try
 	command
@@ -1256,31 +1262,6 @@ sub cmd_mkdirs {
 	$gs->mkemptydirs($_revision);
 }
 
-sub canonicalize_path {
-	my ($path) = @_;
-	my $dot_slash_added = 0;
-	if (substr($path, 0, 1) ne "/") {
-		$path = "./" . $path;
-		$dot_slash_added = 1;
-	}
-	# File::Spec->canonpath doesn't collapse x/../y into y (for a
-	# good reason), so let's do this manually.
-	$path =~ s#/+#/#g;
-	$path =~ s#/\.(?:/|$)#/#g;
-	$path =~ s#/[^/]+/\.\.##g;
-	$path =~ s#/$##g;
-	$path =~ s#^\./## if $dot_slash_added;
-	$path =~ s#^/##;
-	$path =~ s#^\.$##;
-	return $path;
-}
-
-sub canonicalize_url {
-	my ($url) = @_;
-	$url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;
-	return $url;
-}
-
 # get_svnprops(PATH)
 # ------------------
 # Helper for cmd_propget and cmd_proplist below.
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 496006bc7b..ad5351e9ba 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -5,7 +5,12 @@ use warnings;
 
 use base qw(Exporter);
 
-our @EXPORT_OK = qw(fatal can_compress);
+our @EXPORT_OK = qw(
+	fatal
+	can_compress
+	canonicalize_path
+	canonicalize_url
+);
 
 
 =head1 NAME
@@ -56,4 +61,49 @@ sub can_compress {
 }
 
 
+=head3 canonicalize_path
+
+    my $canoncalized_path = canonicalize_path($path);
+
+Converts $path into a canonical form which is safe to pass to the SVN
+API as a file path.
+
+=cut
+
+sub canonicalize_path {
+	my ($path) = @_;
+	my $dot_slash_added = 0;
+	if (substr($path, 0, 1) ne "/") {
+		$path = "./" . $path;
+		$dot_slash_added = 1;
+	}
+	# File::Spec->canonpath doesn't collapse x/../y into y (for a
+	# good reason), so let's do this manually.
+	$path =~ s#/+#/#g;
+	$path =~ s#/\.(?:/|$)#/#g;
+	$path =~ s#/[^/]+/\.\.##g;
+	$path =~ s#/$##g;
+	$path =~ s#^\./## if $dot_slash_added;
+	$path =~ s#^/##;
+	$path =~ s#^\.$##;
+	return $path;
+}
+
+
+=head3 canonicalize_url
+
+    my $canonicalized_url = canonicalize_url($url);
+
+Converts $url into a canonical form which is safe to pass to the SVN
+API as a URL.
+
+=cut
+
+sub canonicalize_url {
+	my ($url) = @_;
+	$url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;
+	return $url;
+}
+
+
 1;
-- 
cgit v1.2.3


From ca475a61f8c07d475c505bf64d219f7e9d61e728 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:38:29 -0700
Subject: git-svn: add join_paths() to safely concatenate paths

Otherwise you might wind up with things like...

    my $path1 = undef;
    my $path2 = 'foo';
    my $path = $path1 . '/' . $path2;

creating '/foo'.  Or this...

    my $path1 = 'foo/';
    my $path2 = 'bar';
    my $path = $path1 . '/' . $path2;

creating 'foo//bar'.

Could have used File::Spec, but I'm shying away from it due to SVN
1.7's pickiness about paths.  Felt it would be better to have our own
we can control completely.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl                 |  3 ++-
 perl/Git/SVN.pm              | 10 ++++++----
 perl/Git/SVN/Utils.pm        | 32 ++++++++++++++++++++++++++++++++
 t/Git-SVN/Utils/join_paths.t | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 72 insertions(+), 5 deletions(-)
 create mode 100644 t/Git-SVN/Utils/join_paths.t

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index a857484c58..6e3e240473 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -34,6 +34,7 @@ use Git::SVN::Utils qw(
 	can_compress
 	canonicalize_path
 	canonicalize_url
+	join_paths
 );
 
 use Git qw(
@@ -1275,7 +1276,7 @@ sub get_svnprops {
 	$path = $cmd_dir_prefix . $path;
 	fatal("No such file or directory: $path") unless -e $path;
 	my $is_dir = -d $path ? 1 : 0;
-	$path = $gs->{path} . '/' . $path;
+	$path = join_paths($gs->{path}, $path);
 
 	# canonicalize the path (otherwise libsvn will abort or fail to
 	# find the file)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index fc1ac07136..ff747821fb 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -23,7 +23,11 @@ use Git qw(
     command_output_pipe
     command_close_pipe
 );
-use Git::SVN::Utils qw(fatal can_compress);
+use Git::SVN::Utils qw(
+	fatal
+	can_compress
+	join_paths
+);
 
 my $can_use_yaml;
 BEGIN {
@@ -316,9 +320,7 @@ sub init_remote_config {
 			}
 			my $old_path = $self->path;
 			$url =~ s!^\Q$min_url\E(/|$)!!;
-			if (length $old_path) {
-				$url .= "/$old_path";
-			}
+			$url = join_paths($url, $old_path);
 			$self->path($url);
 			$url = $min_url;
 		}
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 4925410dd1..4005da9d7d 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -12,6 +12,7 @@ our @EXPORT_OK = qw(
 	can_compress
 	canonicalize_path
 	canonicalize_url
+	join_paths
 );
 
 
@@ -134,4 +135,35 @@ sub _canonicalize_url_ourselves {
 }
 
 
+=head3 join_paths
+
+    my $new_path = join_paths(@paths);
+
+Appends @paths together into a single path.  Any empty paths are ignored.
+
+=cut
+
+sub join_paths {
+	my @paths = @_;
+
+	@paths = grep { defined $_ && length $_ } @paths;
+
+	return '' unless @paths;
+	return $paths[0] if @paths == 1;
+
+	my $new_path = shift @paths;
+	$new_path =~ s{/+$}{};
+
+	my $last_path = pop @paths;
+	$last_path =~ s{^/+}{};
+
+	for my $path (@paths) {
+		$path =~ s{^/+}{};
+		$path =~ s{/+$}{};
+		$new_path .= "/$path";
+	}
+
+	return $new_path .= "/$last_path";
+}
+
 1;
diff --git a/t/Git-SVN/Utils/join_paths.t b/t/Git-SVN/Utils/join_paths.t
new file mode 100644
index 0000000000..d4488e7162
--- /dev/null
+++ b/t/Git-SVN/Utils/join_paths.t
@@ -0,0 +1,32 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use Test::More 'no_plan';
+
+use Git::SVN::Utils qw(
+	join_paths
+);
+
+# A reference cannot be a hash key, so we use an array.
+my @tests = (
+	[]					=> '',
+	["/x.com", "bar"]			=> '/x.com/bar',
+	["x.com", ""]				=> 'x.com',
+	["/x.com/foo/", undef, "bar"]		=> '/x.com/foo/bar',
+	["x.com/foo/", "/bar/baz/"]		=> 'x.com/foo/bar/baz/',
+	["foo", "bar"]				=> 'foo/bar',
+	["/foo/bar", "baz", "/biff"]		=> '/foo/bar/baz/biff',
+	["", undef, "."]			=> '.',
+	[]					=> '',
+
+);
+
+while(@tests) {
+	my($have, $want) = splice @tests, 0, 2;
+
+	my $args = join ", ", map { qq['$_'] } map { defined($_) ? $_ : 'undef' } @$have;
+	my $name = "join_paths($args) eq '$want'";
+	is join_paths(@$have), $want, $name;
+}
-- 
cgit v1.2.3


From 9c27a57b2da502b7dd3736013b7a185fb6e5064e Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:47:48 -0700
Subject: git-svn: replace URL escapes with canonicalization

The old hand-rolled URL escape functions were inferior to
canonicalization functions.

Continuing to move towards getting everything canonicalizing the same way.

* Git::SVN->init_remote_config and Git::SVN::Ra->minimize_url both
  have to canonicalize the same way else init_remote_config
  will incorrectly think they're different URLs causing
  t9107-git-svn-migrate.sh to fail.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl       | 24 +++---------------------
 perl/Git/SVN.pm    |  2 +-
 perl/Git/SVN/Ra.pm | 27 +++++----------------------
 3 files changed, 9 insertions(+), 44 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 6e3e240473..6e975457f9 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1412,24 +1412,6 @@ sub cmd_commit_diff {
 	}
 }
 
-sub escape_uri_only {
-	my ($uri) = @_;
-	my @tmp;
-	foreach (split m{/}, $uri) {
-		s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf("%%%02X",ord($1))/eg;
-		push @tmp, $_;
-	}
-	join('/', @tmp);
-}
-
-sub escape_url {
-	my ($url) = @_;
-	if ($url =~ m#^([^:]+)://([^/]*)(.*)$#) {
-		my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
-		$url = "$scheme://$domain$uri";
-	}
-	$url;
-}
 
 sub cmd_info {
 	my $path = canonicalize_path(defined($_[0]) ? $_[0] : ".");
@@ -1457,18 +1439,18 @@ sub cmd_info {
 	my $full_url = $url . ($fullpath eq "" ? "" : "/$fullpath");
 
 	if ($_url) {
-		print escape_url($full_url), "\n";
+		print canonicalize_url($full_url), "\n";
 		return;
 	}
 
 	my $result = "Path: $path\n";
 	$result .= "Name: " . basename($path) . "\n" if $file_type ne "dir";
-	$result .= "URL: " . escape_url($full_url) . "\n";
+	$result .= "URL: " . canonicalize_url($full_url) . "\n";
 
 	eval {
 		my $repos_root = $gs->repos_root;
 		Git::SVN::remove_username($repos_root);
-		$result .= "Repository Root: " . escape_url($repos_root) . "\n";
+		$result .= "Repository Root: " . canonicalize_url($repos_root) . "\n";
 	};
 	if ($@) {
 		$result .= "Repository Root: (offline)\n";
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index a2e7144b4c..6a2a52e808 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -296,7 +296,7 @@ sub find_existing_remote {
 
 sub init_remote_config {
 	my ($self, $url, $no_write) = @_;
-	$url =~ s!/+$!!; # strip trailing slash
+	$url = canonicalize_url($url);
 	my $r = read_all_remotes();
 	my $existing = find_existing_remote($url, $r);
 	if ($existing) {
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 77bceb913a..268b368de3 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -66,24 +66,6 @@ sub _auth_providers () {
 	\@rv;
 }
 
-sub escape_uri_only {
-	my ($uri) = @_;
-	my @tmp;
-	foreach (split m{/}, $uri) {
-		s/([^~\w.%+-]|%(?![a-fA-F0-9]{2}))/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) = @_;
@@ -119,7 +101,7 @@ sub new {
 			$Git::SVN::Prompt::_no_auth_cache = 1;
 		}
 	} # no warnings 'once'
-	my $self = SVN::Ra->new(url => escape_url($url), auth => $baton,
+	my $self = SVN::Ra->new(url => canonicalize_url($url), auth => $baton,
 	                      config => $config,
 			      pool => SVN::Pool->new,
 	                      auth_provider_callbacks => $callbacks);
@@ -312,7 +294,7 @@ sub gs_do_switch {
 
 	if ($old_url =~ m#^svn(\+ssh)?://# ||
 	    ($full_url =~ m#^https?://# &&
-	     escape_url($full_url) ne $full_url)) {
+	     canonicalize_url($full_url) ne $full_url)) {
 		$_[0] = undef;
 		$self = undef;
 		$RA = undef;
@@ -325,7 +307,7 @@ sub gs_do_switch {
 	}
 
 	$ra ||= $self;
-	$url_b = escape_url($url_b);
+	$url_b = canonicalize_url($url_b);
 	my $reporter = $ra->do_switch($rev_b, '', 1, $url_b, $editor, $pool);
 	my @lock = (::compare_svn_version('1.2.0') >= 0) ? (undef) : ();
 	$reporter->set_path('', $rev_a, 0, @lock, $pool);
@@ -580,7 +562,8 @@ sub minimize_url {
 			$ra->get_log("", $latest, 0, 1, 0, 1, sub {});
 		};
 	} while ($@ && ($c = shift @components));
-	$url;
+
+	return canonicalize_url($url);
 }
 
 sub can_do_switch {
-- 
cgit v1.2.3


From 8266fc8be19ef1405d4ef175bb0e75ebc2730f5d Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:47:49 -0700
Subject: git-svn: canonicalize earlier

Just a few things I noticed.  Its good to canonicalize as early as
possible.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl       | 6 +++---
 perl/Git/SVN/Ra.pm | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 6e975457f9..6b9076599b 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1436,16 +1436,16 @@ sub cmd_info {
 	# canonicalize_path() will return "" to make libsvn 1.5.x happy,
 	$path = "." if $path eq "";
 
-	my $full_url = $url . ($fullpath eq "" ? "" : "/$fullpath");
+	my $full_url = canonicalize_url( $url . ($fullpath eq "" ? "" : "/$fullpath") );
 
 	if ($_url) {
-		print canonicalize_url($full_url), "\n";
+		print "$full_url\n";
 		return;
 	}
 
 	my $result = "Path: $path\n";
 	$result .= "Name: " . basename($path) . "\n" if $file_type ne "dir";
-	$result .= "URL: " . canonicalize_url($full_url) . "\n";
+	$result .= "URL: $full_url\n";
 
 	eval {
 		my $repos_root = $gs->repos_root;
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 268b368de3..c88b2b91da 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -69,7 +69,7 @@ sub _auth_providers () {
 
 sub new {
 	my ($class, $url) = @_;
-	$url =~ s!/+$!!;
+	$url = canonicalize_url($url);
 	return $RA if ($RA && $RA->url eq $url);
 
 	::_req_svn();
@@ -101,7 +101,7 @@ sub new {
 			$Git::SVN::Prompt::_no_auth_cache = 1;
 		}
 	} # no warnings 'once'
-	my $self = SVN::Ra->new(url => canonicalize_url($url), auth => $baton,
+	my $self = SVN::Ra->new(url => $url, auth => $baton,
 	                      config => $config,
 			      pool => SVN::Pool->new,
 	                      auth_provider_callbacks => $callbacks);
-- 
cgit v1.2.3


From d2fd119c4fcaea266a894354b506959376140c37 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:47:50 -0700
Subject: git-svn: introduce add_path_to_url function

Remove the ad-hoc versions.

This is mostly to normalize the process and ensure the URLs produced
don't have double slashes or anything.

Also provides a place to fix the corner case where a file path
contains a percent sign.

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl                      |  3 ++-
 perl/Git/SVN.pm                   | 33 +++++++++++++++------------------
 perl/Git/SVN/Ra.pm                |  8 ++++----
 perl/Git/SVN/Utils.pm             | 27 +++++++++++++++++++++++++++
 t/Git-SVN/Utils/add_path_to_url.t | 27 +++++++++++++++++++++++++++
 5 files changed, 75 insertions(+), 23 deletions(-)
 create mode 100644 t/Git-SVN/Utils/add_path_to_url.t

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 6b9076599b..3d120d5bc4 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -35,6 +35,7 @@ use Git::SVN::Utils qw(
 	canonicalize_path
 	canonicalize_url
 	join_paths
+	add_path_to_url
 );
 
 use Git qw(
@@ -1436,7 +1437,7 @@ sub cmd_info {
 	# canonicalize_path() will return "" to make libsvn 1.5.x happy,
 	$path = "." if $path eq "";
 
-	my $full_url = canonicalize_url( $url . ($fullpath eq "" ? "" : "/$fullpath") );
+	my $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );
 
 	if ($_url) {
 		print "$full_url\n";
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 6a2a52e808..ec5e826468 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -29,6 +29,7 @@ use Git::SVN::Utils qw(
 	join_paths
 	canonicalize_path
 	canonicalize_url
+	add_path_to_url
 );
 
 my $can_use_yaml;
@@ -564,8 +565,7 @@ sub _set_svm_vars {
 		# username is of no interest
 		$src =~ s{(^[a-z\+]*://)[^/@]*@}{$1};
 
-		my $replace = $ra->url;
-		$replace .= "/$path" if length $path;
+		my $replace = add_path_to_url($ra->url, $path);
 
 		my $section = "svn-remote.$self->{repo_id}";
 		tmp_config("$section.svm-source", $src);
@@ -582,7 +582,7 @@ sub _set_svm_vars {
 	my $path = $self->path;
 	my %tried;
 	while (length $path) {
-		my $try = $self->url . "/$path";
+		my $try = add_path_to_url($self->url, $path);
 		unless ($tried{$try}) {
 			return $ra if $self->read_svm_props($ra, $path, $r);
 			$tried{$try} = 1;
@@ -591,7 +591,7 @@ sub _set_svm_vars {
 	}
 	die "Path: '$path' should be ''\n" if $path ne '';
 	return $ra if $self->read_svm_props($ra, $path, $r);
-	$tried{$self->url."/$path"} = 1;
+	$tried{ add_path_to_url($self->url, $path) } = 1;
 
 	if ($ra->{repos_root} eq $self->url) {
 		die @err, (map { "  $_\n" } keys %tried), "\n";
@@ -603,7 +603,7 @@ sub _set_svm_vars {
 	$path = $ra->{svn_path};
 	$ra = Git::SVN::Ra->new($ra->{repos_root});
 	while (length $path) {
-		my $try = $ra->url ."/$path";
+		my $try = add_path_to_url($ra->url, $path);
 		unless ($tried{$try}) {
 			$ok = $self->read_svm_props($ra, $path, $r);
 			last if $ok;
@@ -613,7 +613,7 @@ sub _set_svm_vars {
 	}
 	die "Path: '$path' should be ''\n" if $path ne '';
 	$ok ||= $self->read_svm_props($ra, $path, $r);
-	$tried{$ra->url ."/$path"} = 1;
+	$tried{ add_path_to_url($ra->url, $path) } = 1;
 	if (!$ok) {
 		die @err, (map { "  $_\n" } keys %tried), "\n";
 	}
@@ -933,20 +933,19 @@ sub rewrite_uuid {
 
 sub metadata_url {
 	my ($self) = @_;
-	($self->rewrite_root || $self->url) .
-	   (length $self->path ? '/' . $self->path : '');
+	my $url = $self->rewrite_root || $self->url;
+	return add_path_to_url( $url, $self->path );
 }
 
 sub full_url {
 	my ($self) = @_;
-	$self->url . (length $self->path ? '/' . $self->path : '');
+	return add_path_to_url( $self->url, $self->path );
 }
 
 sub full_pushurl {
 	my ($self) = @_;
 	if ($self->{pushurl}) {
-		return $self->{pushurl} . (length $self->path ? '/' .
-		       $self->path : '');
+		return add_path_to_url( $self->{pushurl}, $self->path );
 	} else {
 		return $self->full_url;
 	}
@@ -1114,7 +1113,7 @@ sub find_parent_branch {
 	my $r = $i->{copyfrom_rev};
 	my $repos_root = $self->ra->{repos_root};
 	my $url = $self->ra->url;
-	my $new_url = $url . $branch_from;
+	my $new_url = add_path_to_url( $url, $branch_from );
 	print STDERR  "Found possible branch point: ",
 	              "$new_url => ", $self->full_url, ", $r\n"
 	              unless $::_q > 1;
@@ -1443,12 +1442,11 @@ sub find_extra_svk_parents {
 	for my $ticket ( @tickets ) {
 		my ($uuid, $path, $rev) = split /:/, $ticket;
 		if ( $uuid eq $self->ra_uuid ) {
-			my $url = $self->url;
-			my $repos_root = $url;
+			my $repos_root = $self->url;
 			my $branch_from = $path;
 			$branch_from =~ s{^/}{};
-			my $gs = $self->other_gs($repos_root."/".$branch_from,
-			                         $url,
+			my $gs = $self->other_gs(add_path_to_url( $repos_root, $branch_from ),
+			                         $repos_root,
 			                         $branch_from,
 			                         $rev,
 			                         $self->{ref_id});
@@ -1871,8 +1869,7 @@ sub make_log_entry {
 		$email ||= "$author\@$uuid";
 		$commit_email ||= "$author\@$uuid";
 	} elsif ($self->use_svnsync_props) {
-		my $full_url = $self->svnsync->{url};
-		$full_url .= "/".$self->path if length $self->path;
+		my $full_url = add_path_to_url( $self->svnsync->{url}, $self->path );
 		remove_username($full_url);
 		my $uuid = $self->svnsync->{uuid};
 		$log_entry{metadata} = "$full_url\@$rev $uuid";
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index c88b2b91da..ec3c570b21 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -5,6 +5,7 @@ use warnings;
 use SVN::Client;
 use Git::SVN::Utils qw(
 	canonicalize_url
+	add_path_to_url
 );
 
 use SVN::Ra;
@@ -287,9 +288,8 @@ sub gs_do_switch {
 	my $path = $gs->path;
 	my $pool = SVN::Pool->new;
 
-	my $full_url = $self->url;
-	my $old_url = $full_url;
-	$full_url .= '/' . $path if length $path;
+	my $old_url = $self->url;
+	my $full_url = add_path_to_url( $self->url, $path );
 	my ($ra, $reparented);
 
 	if ($old_url =~ m#^svn(\+ssh)?://# ||
@@ -555,7 +555,7 @@ sub minimize_url {
 	my @components = split(m!/!, $self->{svn_path});
 	my $c = '';
 	do {
-		$url .= "/$c" if length $c;
+		$url = add_path_to_url($url, $c);
 		eval {
 			my $ra = (ref $self)->new($url);
 			my $latest = $ra->get_latest_revnum;
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index ab7add5e8b..4bb4dde89a 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -13,6 +13,7 @@ our @EXPORT_OK = qw(
 	canonicalize_path
 	canonicalize_url
 	join_paths
+	add_path_to_url
 );
 
 
@@ -203,4 +204,30 @@ sub join_paths {
 	return $new_path .= "/$last_path";
 }
 
+
+=head3 add_path_to_url
+
+    my $new_url = add_path_to_url($url, $path);
+
+Appends $path onto the $url.  If $path is empty, $url is returned unchanged.
+
+=cut
+
+sub add_path_to_url {
+	my($url, $path) = @_;
+
+	return $url if !defined $path or !length $path;
+
+	# Strip trailing and leading slashes so we don't
+	# wind up with http://x.com///path
+	$url  =~ s{/+$}{};
+	$path =~ s{^/+}{};
+
+	# If a path has a % in it, URI escape it so it's not
+	# mistaken for a URI escape later.
+	$path =~ s{%}{%25}g;
+
+	return join '/', $url, $path;
+}
+
 1;
diff --git a/t/Git-SVN/Utils/add_path_to_url.t b/t/Git-SVN/Utils/add_path_to_url.t
new file mode 100644
index 0000000000..bfbd87845f
--- /dev/null
+++ b/t/Git-SVN/Utils/add_path_to_url.t
@@ -0,0 +1,27 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use Test::More 'no_plan';
+
+use Git::SVN::Utils qw(
+	add_path_to_url
+);
+
+# A reference cannot be a hash key, so we use an array.
+my @tests = (
+	["http://x.com", "bar"]			=> 'http://x.com/bar',
+	["http://x.com", ""]			=> 'http://x.com',
+	["http://x.com/foo/", undef]		=> 'http://x.com/foo/',
+	["http://x.com/foo/", "/bar/baz/"]	=> 'http://x.com/foo/bar/baz/',
+	["http://x.com", 'per%cent']		=> 'http://x.com/per%25cent',
+);
+
+while(@tests) {
+	my($have, $want) = splice @tests, 0, 2;
+
+	my $args = join ", ", map { qq['$_'] } map { defined($_) ? $_ : 'undef' } @$have;
+	my $name = "add_path_to_url($args) eq $want";
+	is add_path_to_url(@$have), $want, $name;
+}
-- 
cgit v1.2.3


From 5eaa1fd086e826b1ac8d9346a740527edbdb3c34 Mon Sep 17 00:00:00 2001
From: "Michael G. Schwern" <schwern@pobox.com>
Date: Sat, 28 Jul 2012 02:47:52 -0700
Subject: git-svn: remove ad-hoc canonicalizations

[ew: commit title]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-svn.perl    | 8 ++++----
 perl/Git/SVN.pm | 1 -
 2 files changed, 4 insertions(+), 5 deletions(-)

(limited to 'git-svn.perl')

diff --git a/git-svn.perl b/git-svn.perl
index 3d120d5bc4..56d1ba712a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -36,6 +36,7 @@ use Git::SVN::Utils qw(
 	canonicalize_url
 	join_paths
 	add_path_to_url
+	join_paths
 );
 
 use Git qw(
@@ -1598,7 +1599,7 @@ sub post_fetch_checkout {
 
 sub complete_svn_url {
 	my ($url, $path) = @_;
-	$path =~ s#/+$##;
+	$path = canonicalize_path($path);
 
 	# If the path is not a URL...
 	if ($path !~ m#^[a-z\+]+://#) {
@@ -1617,7 +1618,7 @@ sub complete_url_ls_init {
 		print STDERR "W: $switch not specified\n";
 		return;
 	}
-	$repo_path =~ s#/+$##;
+	$repo_path = canonicalize_path($repo_path);
 	if ($repo_path =~ m#^[a-z\+]+://#) {
 		$ra = Git::SVN::Ra->new($repo_path);
 		$repo_path = '';
@@ -1638,9 +1639,8 @@ sub complete_url_ls_init {
 	}
 	command_oneline('config', $k, $gs->url) unless $orig_url;
 
-	my $remote_path = $gs->path . "/$repo_path";
+	my $remote_path = join_paths( $gs->path, $repo_path );
 	$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
-	$remote_path =~ s#/+#/#g;
 	$remote_path =~ s#^/##g;
 	$remote_path .= "/*" if $remote_path !~ /\*/;
 	my ($n) = ($switch =~ /^--(\w+)/);
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index d0087b2897..08891452a1 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -460,7 +460,6 @@ sub new {
 	}
 	{
 		my $path = $self->path;
-		$path =~ s{/+}{/}g;
 		$path =~ s{\A/}{};
 		$path =~ s{/\z}{};
 		$self->path($path);
-- 
cgit v1.2.3