Re: [PATCH 6/7] Switch path canonicalization to use the SVN API.

On 2012.7.28 6:55 AM, Jonathan Nieder wrote:

Michael G. Schwern wrote:

quoted

--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm

@@ -86,6 +86,27 @@ sub _collapse_dotdot {
 
 
 sub canonicalize_path {
+	my $path = shift;
+
+	# The 1.7 way to do it
+	if ( defined &SVN::_Core::svn_dirent_canonicalize ) {
+		$path = _collapse_dotdot($path);
+		return SVN::_Core::svn_dirent_canonicalize($path);
+	}
+	# The 1.6 way to do it
+	elsif ( defined &SVN::_Core::svn_path_canonicalize ) {
+		$path = _collapse_dotdot($path);
+		return SVN::_Core::svn_path_canonicalize($path);
+	}
+	# No SVN API canonicalization is available, do it ourselves
+	else {

When would this "else" case trip?

When svn_path_canonicalize() does not exist in the SVN API, presumably because
their SVN is too old.

Would it be safe to make it
return an error message, or even to do something like the following?

I don't know what your SVN backwards compat requirements are, or when
svn_path_canonicalize() appears in the API, so I left it as is.  git-svn's
home rolled path canonicalization worked and its no work to leave it working.
 No reason to break it IMO.

	sub canonicalize_path {
		my $path = shift;
		$path = _collapse_dotdot($path);

		# Subversion 1.7 split svn_path_canonicalize() into
		# svn_dirent_canonicalize() and svn_uri_canonicalize().
		if (!defined &SVN::_Core::svn_dirent_canonicalize) {
			return SVN::_Core::svn_path_canonicalize($path);
		}

		return SVN::_Core::svn_dirent_canonicalize($path);
	}

As a side note...
"If they don't have Mars bar, get me a Twix.  Else get me a Mars bar."
"If they have a Mars bar, get me one.  Else get me a Twix."


-- 
Look at me talking when there's science to do.
When I look out there it makes me glad I'm not you.
I've experiments to be run.
There is research to be done
On the people who are still alive.
    -- Jonathan Coulton, "Still Alive"