Hi,

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);
> +     }
> +}
> +
> +
> +sub _canonicalize_url_ourselves {
>       my ($url) = @_;
>       $url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;

Leaves me a bit nervous.

What effect should we expect this change to have?  Is our emulation
of svn_uri_canonicalize already perfect and this change just a little
futureproofing in case svn_uri_canonicalize gets even better, or is
this a trap waiting to happen when new callers of canonicalize_url
start relying on, e.g., %-encoding of special characters?

If I am reading Subversion r873487 correctly, in ancient times,
svn_path_canonicalize() did the appropriate tweaking for URIs.  Today
its implementation is comforting:

        const char *
        svn_path_canonicalize(const char *path, apr_pool_t *pool)
        {
          if (svn_path_is_url(path))
            return svn_uri_canonicalize(path, pool);
          else
            return svn_dirent_canonicalize(path, pool);
        }

It might be easier to rely on that on pre-1.7 systems.

Thanks,
Jonathan
--
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