Michael G Schwern wrote:

>   git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
>
> That should give you the information you need...

I guess so.  May we have your sign-off on these changes?  (A simple
reply of "yes" is enough, no need to resend patches to do this.)

Here it is in patch form for reviewers.  If I understand correctly,
the idea is to replace accesses to $gs->{path} with calls to a
$gs->path function that canonicalizes (and likewise for s/path/url/).

There are probably other subtleties, but that seems to be the gist.
---
 git-svn.perl                          |   96 +++++++--------------
 perl/Git/SVN.pm                       |  148 +++++++++++++++++++++------------
 perl/Git/SVN/Fetcher.pm               |    2 +-
 perl/Git/SVN/Migration.pm             |    6 +-
 perl/Git/SVN/Ra.pm                    |   96 ++++++++++++---------
 perl/Git/SVN/Utils.pm                 |  123 ++++++++++++++++++++++++++-
 t/Git-SVN/Ra/fix_dir.t                |   24 ++++++
 t/lib-git-svn.sh                      |    2 +
 t/t9118-git-svn-funky-branch-names.sh |    6 +-
 t/t9119-git-svn-info.sh               |    7 +-
 10 files changed, 341 insertions(+), 169 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 7b54f43f..74aeb4df 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -12,7 +12,12 @@ $VERSION = '@@GIT_VERSION@@';
 
 use Git::SVN;
 
-use Git::SVN::Utils qw(fatal can_compress);
+use Git::SVN::Utils qw(
+    fatal
+    can_compress
+    canonicalize_path
+    canonicalize_url
+);
 
 # From which subdir have we been invoked?
 my $cmd_dir_prefix = eval {
@@ -100,8 +105,6 @@ BEGIN {
        Memoize::memoize 'Git::config_bool';
 }
 
-my ($SVN);
-
 $sha1 = qr/[a-f\d]{40}/;
 $sha1_short = qr/[a-f\d]{4,40}/;
 my ($_stdin, $_help, $_edit,
@@ -518,7 +521,6 @@ sub cmd_init {
        }
        my $url = shift or die "SVN repository location required ",
                               "as a command-line argument\n";
-       $url = canonicalize_url($url);
        init_subdir(@_);
        do_git_init_db();
 
@@ -1199,7 +1201,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;
@@ -1215,7 +1217,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;
@@ -1230,7 +1232,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;
@@ -1260,36 +1262,12 @@ 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.
 sub get_svnprops {
        my $path = shift;
+
        my ($url, $rev, $uuid, $gs) = working_head_info('HEAD');
        $gs ||= Git::SVN->new;
 
@@ -1298,7 +1276,8 @@ 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 = $gs->path . '/' . $path if $gs->path;
 
        # canonicalize the path (otherwise libsvn will abort or fail to
        # find the file)
@@ -1399,8 +1378,8 @@ sub cmd_commit_diff {
                        fatal("Needed URL or usable git-svn --id in ",
                              "the command-line\n", $usage);
                }
-               $url = $gs->{url};
-               $svn_path = $gs->{path};
+               $url = $gs->url;
+               $svn_path = $gs->path;
        }
        unless (defined $_revision) {
                fatal("-r|--revision is a required argument\n", $usage);
@@ -1434,25 +1413,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] : ".");
        my $fullpath = canonicalize_path($cmd_dir_prefix . $path);
@@ -1476,21 +1436,21 @@ 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 escape_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: " . escape_url($full_url) . "\n";
+       $result .= "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";
@@ -1638,7 +1598,10 @@ sub post_fetch_checkout {
 
 sub complete_svn_url {
        my ($url, $path) = @_;
-       $path =~ s#/+$##;
+
+       $path = canonicalize_path($path);
+
+       # 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 ",
@@ -1655,7 +1618,8 @@ 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 = '';
@@ -1666,18 +1630,18 @@ 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;
-       my $remote_path = "$gs->{path}/$repo_path";
+       command_oneline('config', $k, $gs->url) unless $orig_url;
+
+       my $remote_path = canonicalize_path($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 247ee1d0..ebfa8da5 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -23,7 +23,12 @@ BEGIN {
 }
 
 use Git qw(command command_oneline command_noisy command_output_pipe 
command_close_pipe);
-use Git::SVN::Utils qw(fatal can_compress);
+use Git::SVN::Utils qw(
+    fatal
+    can_compress
+    canonicalize_url
+    canonicalize_path
+);
 
 our $_follow_parent  = 1;
 our $_minimize_url   = 'unset';
@@ -310,12 +315,12 @@ sub init_remote_config {
                                print STDERR "Using higher level of URL: ",
                                             "$url => $min_url\n";
                        }
-                       my $old_path = $self->{path};
-                       $self->{path} = $url;
-                       $self->{path} =~ s!^\Q$min_url\E(/|$)!!;
+                       my $old_path = $self->path;
+                       $url =~ s!^\Q$min_url\E(/|$)!!;
                        if (length $old_path) {
-                               $self->{path} .= "/$old_path";
+                               $url .= "/$old_path";
                        }
+                       $self->path($url);
                        $url = $min_url;
                }
        }
@@ -339,13 +344,15 @@ sub init_remote_config {
        unless ($no_write) {
                command_noisy('config',
                              "svn-remote.$self->{repo_id}.url", $url);
-               $self->{path} =~ s{^/}{};
-               $self->{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
+               my $path = $self->path;
+               $path =~ s{^/}{};
+               $path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;
+               $self->path($path);
                command_noisy('config', '--add',
                              "svn-remote.$self->{repo_id}.fetch",
-                             "$self->{path}:".$self->refname);
+                             $self->path.":".$self->refname);
        }
-       $self->{url} = $url;
+       $self->url($url);
 }
 
 sub find_by_url { # repos_root and, path are optional
@@ -431,20 +438,19 @@ sub new {
                }
        }
        my $self = _new($class, $repo_id, $ref_id, $path);
-       if (!defined $self->{path} || !length $self->{path}) {
+       if (!defined $self->path || !length $self->path) {
                my $fetch = command_oneline('config', '--get',
                                            "svn-remote.$repo_id.fetch",
                                            ":$ref_id\$") or
                     die "Failed to read \"svn-remote.$repo_id.fetch\" ",
                         "\":$ref_id\$\" in config\n";
-               ($self->{path}, undef) = split(/\s*:\s*/, $fetch);
+               my($path) = split(/\s*:\s*/, $fetch);
+               $self->path($path);
        }
-       $self->{path} =~ s{/+}{/}g;
-       $self->{path} =~ s{\A/}{};
-       $self->{path} =~ s{/\z}{};
-       $self->{url} = command_oneline('config', '--get',
-                                      "svn-remote.$repo_id.url") or
+       my $url = command_oneline('config', '--get',
+                                 "svn-remote.$repo_id.url") or
                   die "Failed to read \"svn-remote.$repo_id.url\" in config\n";
+       $self->url($url);
        $self->{pushurl} = eval { command_oneline('config', '--get',
                                  "svn-remote.$repo_id.pushurl") };
        $self->rebuild;
@@ -548,7 +554,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}";
@@ -563,20 +569,21 @@ sub _set_svm_vars {
        }
 
        my $r = $ra->get_latest_revnum;
-       my $path = $self->{path};
+       my $path = $self->path;
        my %tried;
        while (length $path) {
-               unless ($tried{"$self->{url}/$path"}) {
+               my $try = $self->url . "/$path";
+               unless ($tried{$try}) {
                        return $ra if $self->read_svm_props($ra, $path, $r);
-                       $tried{"$self->{url}/$path"} = 1;
+                       $tried{$try} = 1;
                }
                $path =~ s#/?[^/]+$##;
        }
        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{$self->url."/$path"} = 1;
 
-       if ($ra->{repos_root} eq $self->{url}) {
+       if ($ra->{repos_root} eq $self->url) {
                die @err, (map { "  $_\n" } keys %tried), "\n";
        }
 
@@ -586,20 +593,21 @@ 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";
        }
-       Git::SVN::Ra->new($self->{url});
+       Git::SVN::Ra->new($self->url);
 }
 
 sub svnsync {
@@ -666,7 +674,7 @@ sub ra_uuid {
                if (!$@ && $uuid && $uuid =~ /^([a-f\d\-]{30,})$/i) {
                        $self->{ra_uuid} = $uuid;
                } else {
-                       die "ra_uuid called without URL\n" unless $self->{url};
+                       die "ra_uuid called without URL\n" unless $self->url;
                        $self->{ra_uuid} = $self->ra->get_uuid;
                        tmp_config('--add', $key, $self->{ra_uuid});
                }
@@ -690,7 +698,7 @@ sub repos_root {
 
 sub ra {
        my ($self) = shift;
-       my $ra = Git::SVN::Ra->new($self->{url});
+       my $ra = Git::SVN::Ra->new($self->url);
        $self->_set_repos_root($ra->{repos_root});
        if ($self->use_svm_props && !$self->{svm}) {
                if ($self->no_metadata) {
@@ -724,7 +732,7 @@ sub prop_walk {
        $path =~ s#^/*#/#g;
        my $p = $path;
        # Strip the irrelevant part of the path.
-       $p =~ s#^/+\Q$self->{path}\E(/|$)#/#;
+       $p =~ s#^/+\Q@{[$self->path]}\E(/|$)#/#;
        # Ensure the path is terminated by a `/'.
        $p =~ s#/*$#/#;
 
@@ -745,7 +753,7 @@ sub prop_walk {
 
        foreach (sort keys %$dirent) {
                next if $dirent->{$_}->{kind} != $SVN::Node::dir;
-               $self->prop_walk($self->{path} . $p . $_, $rev, $sub);
+               $self->prop_walk($self->path . $p . $_, $rev, $sub);
        }
 }
 
@@ -915,20 +923,20 @@ sub rewrite_uuid {
 
 sub metadata_url {
        my ($self) = @_;
-       ($self->rewrite_root || $self->{url}) .
-          (length $self->{path} ? '/' . $self->{path} : '');
+       ($self->rewrite_root || $self->url) .
+          (length $self->path ? '/' . $self->path : '');
 }
 
 sub full_url {
        my ($self) = @_;
-       $self->{url} . (length $self->{path} ? '/' . $self->{path} : '');
+       return canonicalize_url( $self->url . (length $self->path ? '/' . 
$self->path : '') );
 }
 
 sub full_pushurl {
        my ($self) = @_;
        if ($self->{pushurl}) {
-               return $self->{pushurl} . (length $self->{path} ? '/' .
-                      $self->{path} : '');
+               return $self->{pushurl} . (length $self->path ? '/' .
+                      $self->path : '');
        } else {
                return $self->full_url;
        }
@@ -1044,20 +1052,20 @@ sub do_git_commit {
 
 sub match_paths {
        my ($self, $paths, $r) = @_;
-       return 1 if $self->{path} eq '';
-       if (my $path = $paths->{"/$self->{path}"}) {
+       return 1 if $self->path eq '';
+       if (my $path = $paths->{"/".$self->path}) {
                return ($path->{action} eq 'D') ? 0 : 1;
        }
-       $self->{path_regex} ||= qr/^\/\Q$self->{path}\E\//;
+       $self->{path_regex} ||= qr{^/\Q@{[$self->path]}\E/};
        if (grep /$self->{path_regex}/, keys %$paths) {
                return 1;
        }
        my $c = '';
-       foreach (split m#/#, $self->{path}) {
+       foreach (split m#/#, $self->path) {
                $c .= "/$_";
                next unless ($paths->{$c} &&
                             ($paths->{$c}->{action} =~ /^[AR]$/));
-               if ($self->ra->check_path($self->{path}, $r) ==
+               if ($self->ra->check_path($self->path, $r) ==
                    $SVN::Node::dir) {
                        return 1;
                }
@@ -1071,14 +1079,14 @@ sub find_parent_branch {
        unless (defined $paths) {
                my $err_handler = $SVN::Error::handler;
                $SVN::Error::handler = \&Git::SVN::Ra::skip_unknown_revs;
-               $self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1,
+               $self->ra->get_log([$self->path], $rev, $rev, 0, 1, 1,
                                   sub { $paths = $_[0] });
                $SVN::Error::handler = $err_handler;
        }
        return undef unless defined $paths;
 
        # look for a parent from another branch:
-       my @b_path_components = split m#/#, $self->{path};
+       my @b_path_components = split m#/#, $self->path;
        my @a_path_components;
        my $i;
        while (@b_path_components) {
@@ -1089,14 +1097,12 @@ sub find_parent_branch {
        return undef unless defined $i && defined $i->{copyfrom_path};
        my $branch_from = $i->{copyfrom_path};
        if (@a_path_components) {
-               print STDERR "branch_from: $branch_from => ";
                $branch_from .= '/'.join('/', @a_path_components);
-               print STDERR $branch_from, "\n";
        }
        my $r = $i->{copyfrom_rev};
        my $repos_root = $self->ra->{repos_root};
-       my $url = $self->ra->{url};
-       my $new_url = $url . $branch_from;
+       my $url = $self->ra->url;
+       my $new_url = canonicalize_url($url . $branch_from);
        print STDERR  "Found possible branch point: ",
                      "$new_url => ", $self->full_url, ", $r\n"
                      unless $::_q > 1;
@@ -1110,7 +1116,7 @@ sub find_parent_branch {
                        ($base, $head) = parse_revision_argument(0, $r);
                } else {
                        if ($r0 < $r) {
-                               $gs->ra->get_log([$gs->{path}], $r0 + 1, $r, 1,
+                               $gs->ra->get_log([$gs->path], $r0 + 1, $r, 1,
                                        0, 1, sub { $base = $_[1] - 1 });
                        }
                }
@@ -1132,7 +1138,7 @@ sub find_parent_branch {
                        # at the moment), so we can't rely on it
                        $self->{last_rev} = $r0;
                        $self->{last_commit} = $parent;
-                       $ed = Git::SVN::Fetcher->new($self, $gs->{path});
+                       $ed = Git::SVN::Fetcher->new($self, $gs->path);
                        $gs->ra->gs_do_switch($r0, $rev, $gs,
                                              $self->full_url, $ed)
                          or die "SVN connection failed somewhere...\n";
@@ -1231,7 +1237,7 @@ sub mkemptydirs {
                close $fh;
        }
 
-       my $strip = qr/\A\Q$self->{path}\E(?:\/|$)/;
+       my $strip = qr/\A\Q@{[$self->path]}\E(?:\/|$)/;
        foreach my $d (sort keys %empty_dirs) {
                $d = uri_decode($d);
                $d =~ s/$strip//;
@@ -1425,7 +1431,7 @@ 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 $url = $self->url;
                        my $repos_root = $url;
                        my $branch_from = $path;
                        $branch_from =~ s{^/}{};
@@ -1671,7 +1677,7 @@ sub find_extra_svn_parents {
        # are now marked as merge, we can add the tip as a parent.
        my @merges = split "\n", $mergeinfo;
        my @merge_tips;
-       my $url = $self->{url};
+       my $url = $self->url;
        my $uuid = $self->ra_uuid;
        my %ranges;
        for my $merge ( @merges ) {
@@ -1854,7 +1860,7 @@ sub make_log_entry {
                $commit_email ||= "$author\@$uuid";
        } elsif ($self->use_svnsync_props) {
                my $full_url = $self->svnsync->{url};
-               $full_url .= "/$self->{path}" if length $self->{path};
+               $full_url .= "/".$self->path if length $self->path;
                remove_username($full_url);
                my $uuid = $self->svnsync->{uuid};
                $log_entry{metadata} = "$full_url\@$rev $uuid";
@@ -1901,7 +1907,7 @@ sub set_tree {
                        tree_b => $tree,
                        editor_cb => sub {
                               $self->set_tree_cb($log_entry, $tree, @_) },
-                       svn_path => $self->{path} );
+                       svn_path => $self->path );
        if (!Git::SVN::Editor->new(\%ed_opts)->apply_diff) {
                print "No changes\nr$self->{last_rev} = $tree\n";
        }
@@ -2272,12 +2278,44 @@ sub _new {
 
        $_[3] = $path = '' unless (defined $path);
        mkpath([$dir]);
-       bless {
+       my $obj = bless {
                ref_id => $ref_id, dir => $dir, index => "$dir/index",
-               path => $path, config => "$ENV{GIT_DIR}/svn/config",
+               config => "$ENV{GIT_DIR}/svn/config",
                map_root => "$dir/.rev_map", repo_id => $repo_id }, $class;
+
+       # Ensure it gets canonicalized
+       $obj->path($path);
+
+       return $obj;
+}
+
+
+sub path {
+    my $self = shift;
+
+    if( @_ ) {
+        my $path = shift;
+        $self->{path} = canonicalize_path($path);
+        return;
+    }
+
+    return $self->{path};
 }
 
+
+sub url {
+    my $self = shift;
+
+    if( @_ ) {
+        my $url = shift;
+        $self->{url} = canonicalize_url($url);
+        return;
+    }
+
+    return $self->{url};
+}
+
+
 # for read-only access of old .rev_db formats
 sub unlink_rev_db_symlink {
        my ($self) = @_;
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index 69486efe..b606fe5a 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -84,7 +84,7 @@ sub _mark_empty_symlinks {
        chomp(my $empty_blob = `git hash-object -t blob --stdin < /dev/null`);
        my ($ls, $ctx) = command_output_pipe(qw/ls-tree -r -z/, $cmt);
        local $/ = "\0";
-       my $pfx = defined($switch_path) ? $switch_path : $git_svn->{path};
+       my $pfx = defined($switch_path) ? $switch_path : $git_svn->path;
        $pfx .= '/' if length($pfx);
        while (<$ls>) {
                chomp;
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
index b17fe003..5a9d3c71 100644
--- a/perl/Git/SVN/Migration.pm
+++ b/perl/Git/SVN/Migration.pm
@@ -180,14 +180,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};
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 23ff43e8..7bd6db6f 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -8,6 +8,11 @@ BEGIN {
        @ISA = qw(SVN::Ra);
 }
 
+use Git::SVN::Utils qw(
+    canonicalize_url
+    canonicalize_path
+);
+
 my ($ra_invalid, $can_do_switch, %ignored_err, $RA);
 
 BEGIN {
@@ -62,29 +67,10 @@ 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) = @_;
        $url =~ s!/+$!!;
-       return $RA if ($RA && $RA->{url} eq $url);
+       return $RA if ($RA && $RA->url eq $url);
 
        ::_req_svn();
 
@@ -115,19 +101,37 @@ 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);
-       $self->{url} = $url;
+       $RA = bless $self, $class;
+
+       # Make sure its canonicalized
+       $self->url($url);
        $self->{svn_path} = $url;
        $self->{repos_root} = $self->get_repos_root;
        $self->{svn_path} =~ s#^\Q$self->{repos_root}\E(/|$)##;
        $self->{cache} = { check_path => { r => 0, data => {} },
                           get_dir => { r => 0, data => {} } };
-       $RA = bless $self, $class;
+
+       return $RA;
 }
 
+
+sub url {
+    my $self = shift;
+
+    if( @_ ) {
+        my $url = shift;
+        $self->{url} = canonicalize_url($url);
+        return;
+    }
+
+    return $self->{url};
+}
+
+
 sub check_path {
        my ($self, $path, $r) = @_;
        my $cache = $self->{cache}->{check_path};
@@ -146,6 +150,9 @@ sub check_path {
 
 sub get_dir {
        my ($self, $dir, $r) = @_;
+
+        $dir = $self->fix_dir($dir);
+
        my $cache = $self->{cache}->{get_dir};
        if ($r == $cache->{r}) {
                if (my $x = $cache->{data}->{$dir}) {
@@ -164,6 +171,20 @@ sub get_dir {
        wantarray ? (\%dirents, $r, $props) : \%dirents;
 }
 
+sub fix_dir {
+    my $self = shift;
+    my $dir = shift;
+
+    # SVN::Ra->get_dir will fail if given foo/.. and the
+    # canonicalize functions will not fix that.
+    $dir =~ s{[^/]+/\.\./?}{}g;
+
+    # It also doesn't like a single dot.
+    $dir =~ s{^\.$}{};
+
+    return $dir;
+}
+
 sub DESTROY {
        # do not call the real DESTROY since we store ourselves in $RA
 }
@@ -195,6 +216,7 @@ sub get_log {
                                qw/copyfrom_path copyfrom_rev action/;
                        if ($s{'copyfrom_path'}) {
                                $s{'copyfrom_path'} =~ s/$prefix_regex//;
+                               $s{'copyfrom_path'} = 
canonicalize_path($s{'copyfrom_path'});
                        }
                        $_[0]{$p} = \%s;
                }
@@ -246,7 +268,7 @@ sub get_commit_editor {
 sub gs_do_update {
        my ($self, $rev_a, $rev_b, $gs, $editor) = @_;
        my $new = ($rev_a == $rev_b);
-       my $path = $gs->{path};
+       my $path = $gs->path;
 
        if ($new && -e $gs->{index}) {
                unlink $gs->{index} or die
@@ -282,17 +304,17 @@ sub gs_do_update {
 # svn_ra_reparent didn't work before 1.4)
 sub gs_do_switch {
        my ($self, $rev_a, $rev_b, $gs, $url_b, $editor) = @_;
-       my $path = $gs->{path};
+       my $path = $gs->path;
        my $pool = SVN::Pool->new;
 
-       my $full_url = $self->{url};
+       my $full_url = $self->url;
        my $old_url = $full_url;
-       $full_url .= '/' . $path if length $path;
+       $full_url = canonicalize_url( $full_url . '/' . $path ) if length $path;
        my ($ra, $reparented);
 
        if ($old_url =~ m#^svn(\+ssh)?://# ||
            ($full_url =~ m#^https?://# &&
-            escape_url($full_url) ne $full_url)) {
+            $full_url ne $full_url)) {
                $_[0] = undef;
                $self = undef;
                $RA = undef;
@@ -300,12 +322,12 @@ sub gs_do_switch {
                $ra_invalid = 1;
        } elsif ($old_url ne $full_url) {
                SVN::_Ra::svn_ra_reparent($self->{session}, $full_url, $pool);
-               $self->{url} = $full_url;
+               $self->url($full_url);
                $reparented = 1;
        }
 
        $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);
@@ -313,7 +335,7 @@ sub gs_do_switch {
 
        if ($reparented) {
                SVN::_Ra::svn_ra_reparent($self->{session}, $old_url, $pool);
-               $self->{url} = $old_url;
+               $self->url($old_url);
        }
 
        $pool->clear;
@@ -326,7 +348,7 @@ sub longest_common_path {
        my $common_max = scalar @$gsv;
 
        foreach my $gs (@$gsv) {
-               my @tmp = split m#/#, $gs->{path};
+               my @tmp = split m#/#, $gs->path;
                my $p = '';
                foreach (@tmp) {
                        $p .= length($p) ? "/$_" : $_;
@@ -362,7 +384,7 @@ sub gs_fetch_loop_common {
        my $inc = $_log_window_size;
        my ($min, $max) = ($base, $head < $base + $inc ? $head : $base + $inc);
        my $longest_path = longest_common_path($gsv, $globs);
-       my $ra_url = $self->{url};
+       my $ra_url = $self->url;
        my $find_trailing_edge;
        while (1) {
                my %revs;
@@ -508,7 +530,7 @@ sub match_globs {
                                 ($self->check_path($p, $r) !=
                                  $SVN::Node::dir));
                        next unless $p =~ /$g->{path}->{regex}/;
-                       $exists->{$p} = Git::SVN->init($self->{url}, $p, undef,
+                       $exists->{$p} = Git::SVN->init($self->url, $p, undef,
                                         $g->{ref}->full_path($de), 1);
                }
        }
@@ -532,7 +554,7 @@ sub match_globs {
                        next if ($self->check_path($pathname, $r) !=
                                 $SVN::Node::dir);
                        $exists->{$pathname} = Git::SVN->init(
-                                             $self->{url}, $pathname, undef,
+                                             $self->url, $pathname, undef,
                                              $g->{ref}->full_path($p), 1);
                }
                my $c = '';
@@ -548,7 +570,7 @@ sub match_globs {
 
 sub minimize_url {
        my ($self) = @_;
-       return $self->{url} if ($self->{url} eq $self->{repos_root});
+       return $self->url if ($self->url eq $self->{repos_root});
        my $url = $self->{repos_root};
        my @components = split(m!/!, $self->{svn_path});
        my $c = '';
@@ -568,7 +590,7 @@ sub can_do_switch {
        unless (defined $can_do_switch) {
                my $pool = SVN::Pool->new;
                my $rep = eval {
-                       $self->do_switch(1, '', 0, $self->{url},
+                       $self->do_switch(1, '', 0, $self->url,
                                         SVN::Delta::Editor->new, $pool);
                };
                if ($@) {
diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index d8f63e68..e7087435 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -5,10 +5,54 @@ 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
+
+Git::SVN::Utils - utility functions used across Git::SVN
+
+=head1 SYNOPSIS
+
+    use Git::SVN::Utils qw(functions to import);
+
+=head1 DESCRIPTION
+
+This module contains functions which are useful across many different
+parts of Git::SVN.  Mostly it's a place to put utility functions
+rather than duplicate the code or have classes grabbing at other
+classes.
+
+=head1 FUNCTIONS
+
+All functions can be imported only on request.
+
+=head3 fatal
+
+    fatal(@message);
+
+Display a message and exit with a fatal error code.
+
+=cut
+
+# Note: not certain why this is in use instead of die.  Probably because
+# the exit code of die is 255?  Doesn't appear to be used consistently.
 sub fatal (@) { print STDERR "@_\n"; exit 1 }
 
+
+=head3 can_compress
+
+    my $can_compress = can_compress;
+
+Returns true if Compress::Zlib is available, false otherwise.
+
+=cut
+
 my $can_compress;
 sub can_compress {
     return $can_compress if defined $can_compress;
@@ -16,4 +60,81 @@ sub can_compress {
     return $can_compress = eval { require Compress::Zlib; } ? 1 : 0;
 }
 
+
+=head3 canonicalize_path
+
+    my $canonical_path = canonicalize_path($path);
+
+Cleans up the $path so its acceptable to the SVN API's strict idea of
+what a path should look like.
+
+=cut
+
+# SVN 1.7 changed its canonicalization API, so decide on the best way to 
handle the problem.
+sub canonicalize_path {
+    my $path = shift;
+
+    # The 1.7 way to do it
+    if( defined &SVN::_Core::svn_dirent_canonicalize ) {
+        return SVN::_Core::svn_dirent_canonicalize($path);
+    }
+    # The 1.6 way to do it
+    elsif( defined &SVN::_Core::svn_path_canonicalize ) {
+        return SVN::_Core::svn_path_canonicalize($path);
+    }
+    # No SVN API canonicalization is available, do it ourselves
+    else {
+        return _canonicalize_path_ourselves($path);
+    }
+}
+
+
+# A pure Perl implementation of path canonicalization in case the SVN
+# API ones are unavailable.
+sub _canonicalize_path_ourselves {
+       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 $canonical_url = canonicalize_url($url);
+
+Cleans up the $url so its acceptable to the SVN API's strict idea of
+what a URL should look like.
+
+=cut
+
+# SVN 1.7 changed its canonicalization API, so decide on the best way to 
handle the problem.
+sub canonicalize_url {
+    my $url = shift;
+
+    # The 1.7 way to do it
+    if( defined &SVN::_Core::svn_uri_canonicalize ) {
+        $url = SVN::_Core::svn_uri_canonicalize($url);
+    }
+    # There wasn't a 1.6 way to do it, so we do it ourself.
+    else {
+        $url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;
+    }
+
+    return $url;
+}
+
+
 1;
diff --git a/t/Git-SVN/Ra/fix_dir.t b/t/Git-SVN/Ra/fix_dir.t
new file mode 100644
index 00000000..ec0c858a
--- /dev/null
+++ b/t/Git-SVN/Ra/fix_dir.t
@@ -0,0 +1,24 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+use Test::More 'no_plan';
+
+my $CLASS = 'Git::SVN::Ra';
+require_ok $CLASS;
+
+my %tests = (
+    "foo/.."            => "",
+    "/foo/.."           => "/",
+    "."                 => "",
+    "foo/../"           => "",
+    "foo/bar/baz"       => "foo/bar/baz",
+    "foo/bar/../baz"    => "foo/baz",
+);
+
+for my $have (keys %tests) {
+    my $want = $tests{$have};
+
+    is $CLASS->fix_dir($have), $want, "fix_dir $have => $want";
+}
diff --git a/t/lib-git-svn.sh b/t/lib-git-svn.sh
index 199f22c2..c6911cdf 100644
--- a/t/lib-git-svn.sh
+++ b/t/lib-git-svn.sh
@@ -68,6 +68,8 @@ svn_cmd () {
        svn "$orig_svncmd" --config-dir "$svnconf" "$@"
 }
 
+SVN_VERSION=`svn_cmd --version | sed -n -e 's/^svn, version 
\(1\.[0-9]*\.[0-9]*\).*$/\1/p'`
+
 prepare_httpd () {
        for d in \
                "$SVN_HTTPD_PATH" \
diff --git a/t/t9118-git-svn-funky-branch-names.sh 
b/t/t9118-git-svn-funky-branch-names.sh
index 63fc982c..66787acc 100755
--- a/t/t9118-git-svn-funky-branch-names.sh
+++ b/t/t9118-git-svn-funky-branch-names.sh
@@ -32,6 +32,10 @@ test_expect_success 'setup svnrepo' '
        start_httpd
        '
 
+# SVN 1.7 will truncate "not-a%40{0]" to just "not-a".
+# Look at what SVN wound up naming the branch and use that.
+non_reflog=`svn_cmd ls "$svnrepo/pr ject/branches" | grep not-a | sed 's/\///'`
+
 test_expect_success 'test clone with funky branch names' '
        git svn clone -s "$svnrepo/pr ject" project &&
        (
@@ -42,7 +46,7 @@ test_expect_success 'test clone with funky branch names' '
                git rev-parse "refs/remotes/%2Eleading_dot" &&
                git rev-parse "refs/remotes/trailing_dot%2E" &&
                git rev-parse "refs/remotes/trailing_dotlock%2Elock" &&
-               git rev-parse "refs/remotes/not-a%40{0}reflog"
+               git rev-parse "refs/remotes/$non_reflog"
        )
        '
 
diff --git a/t/t9119-git-svn-info.sh b/t/t9119-git-svn-info.sh
index ff19695e..9b6075c6 100755
--- a/t/t9119-git-svn-info.sh
+++ b/t/t9119-git-svn-info.sh
@@ -6,14 +6,11 @@ test_description='git svn info'
 
 . ./lib-git-svn.sh
 
-# Tested with: svn, version 1.4.4 (r25188)
-# Tested with: svn, version 1.6.[12345689]
-v=`svn_cmd --version | sed -n -e 's/^svn, version 
\(1\.[0-9]*\.[0-9]*\).*$/\1/p'`
-case $v in
+case $SVN_VERSION in
 1.[456].*)
        ;;
 *)
-       skip_all="skipping svn-info test (SVN version: $v not supported)"
+       skip_all="skipping svn-info test (SVN version: $SVN_VERSION not 
supported)"
        test_done
        ;;
 esac
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to