From: Jonathan Nieder Date: Sun, 14 Oct 2012 11:42:34 GMT Subject: [PATCH/RFC 0/2] Re: [PATCH 2/7] Change canonicalize_url() to use the SVN 1.7 API when available. Message-ID: <20121014114234.GA18127@elie.Belkin> In-Reply-To: <50143700.80900@pobox.com> Hi Eric, Michael G Schwern wrote: > On 2012.7.28 6:50 AM, Jonathan Nieder wrote: >> Michael G Schwern wrote: >>> --- a/perl/Git/SVN/Utils.pm >>> +++ b/perl/Git/SVN/Utils.pm >> [...] >>> @@ -100,6 +102,20 @@ API as a URL. >>> =cut >>> >>> sub canonicalize_url { >>> + my $url = shift; >>> + >>> + # The 1.7 way to do it >>> + if ( defined &SVN::_Core::svn_uri_canonicalize ) { >>> + return SVN::_Core::svn_uri_canonicalize($url); >>> + } >>> + # There wasn't a 1.6 way to do it, so we do it ourself. >>> + else { >>> + return _canonicalize_url_ourselves($url); [...] >> Leaves me a bit nervous. > > As it should, SVN dumped a mess on us. Here's a pair of patches that address some of the bugs I was alluding to. Patch 1 makes _canonicalize_url_ourselves() match Subversion's own canonicalization behavior more closely, though even with that patch it still does not meet Subversion's requirements perfectly (e.g., "%ab" is not canonicalized to "%AB"). Patch 2 makes that not matter by using svn_path_canonicalize() when possible, which is the standard way to do this kind of thing. Sorry for the lack of clarity before. Jonathan Nieder (2): git svn: do not overescape URLs (fallback case) Git::SVN::Utils::canonicalize_url: use svn_path_canonicalize when available perl/Git/SVN/Utils.pm | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)