Re: [PATCH] Escape file:// URL's to meet subversion SVN::Ra requirements

[Jonathan Nieder <jrnieder@gmail.com>]

Hi again,

Ben Walton wrote:

> Previously only http/https URL's were uri escaped.  When building
> against subversion 1.7, this was causing a segfault in perl after
> an assertion failure in the SVN::Ra bindings during in t9134.

(Not a segfault, just a core dump.)  Thanks.

[....]
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5366,6 +5366,9 @@ sub escape_url {
>  	if ($url =~ m#^(https?)://([^/]+)(.*)$#) {
>  		my ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));
>  		$url = "$scheme://$domain$uri";
> +	} elsif ($url =~ m#^(file)://(.*)$#) {
> +		my ($scheme, $uri) = ($1, escape_uri_only($2));
> +		$url = "$scheme://$uri";

This has two obvious effects, one good and one bad.

The good effect is that it converts spaces to %20.  Both old and new
versions of Subversion seem to be happy to treat %20 as a space, and
new versions of Subversion are not happy to treat a space as a space,
so this conversion can only be a good thing.

The bad effect is that it converts percent signs to %25.  So commands
like "git svn clone file:///path/to/test%20repository" that previously
worked might not work any more, if v1.6.5-rc0~61 (svn: assume URLs
from the command-line are URI-encoded, 2009-08-16) did not do its job
completely.

In other words, it seems to me like you are on the right track. ;-)

Another possible approach: to imitate the svn command line tools, we
could use SVN::Client::url_from_path in some appropriate place. 

Next steps:

 - track down the trouble on svn 1.6.x that Eric mentioned
 - fix any remaining tests that still don't pass

Thanks for getting this started.  Will sleep and play with it a little
more.

Jonathan