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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 2, 2011, 18:20 UTC
Message-ID: <20111102182015.GA11401@elie.hsd1.il.comcast.net>
In-Reply-To: <1320251895-6348-2-git-send-email-bwalton@artsci.utoronto.ca>

Hi,

Ben Walton wrote:

> After a colleague reported this problem to the subversion list, it
> was determined that the problem is in git, not svn.[1]  The SVN code
> expects URL's and paths to be pre-escaped.

Thanks for your work on this! I'm not really sure how one can decide that the problem is not in svn --- some existing functions changed ABI in such a way as to break existing applications and require code changes and a recompile. It would be better for Subversion to silently fix up paths provided by bad callers, or at least to return a sensible error code.

So the problem is that nobody who cared was testing prereleases of subversion and reporting bugs early enough for it to get this fixed before the 1.7 release. But yes, that's water under the bridge and git-svn (and libsvn-perl, and pysvn, and ...) should just adjust to the new world order.

> [1] http://news.gmane.org/gmane.comp.version-control.subversion.devel

Do you mean http://thread.gmane.org/gmane.comp.version-control.subversion.devel/132250 ?

> Signed-off-by: Ben Walton <bwalton@artsci.utoronto.ca>
> ---
>  git-svn.perl |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Sounds sensible in principle, though I haven't checked the patch in detail. When I run "make test" with svn 1.7 with this patch applied, I get the following result, unfortunately:

| expecting success: 
|         git svn clone -s "$svnrepo" g &&
|         (
|                 cd g &&
|                 test x`git rev-parse --verify refs/remotes/trunk^0` = \
|                      x`git rev-parse --verify refs/heads/master^0`
|         )
|
| Initialized empty Git repository in /home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/g/.git/
| Using higher level of URL: file:///home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/svnrepo => file:///home/jrn/src/git2/t/trash%20directory.t9145-git-svn-master-branch/svnrepo
| svn-remote.svn: remote ref '///home/jrn/src/git2/t/trash directory.t9145-git-svn-master-branch/svnrepo/trunk:refs/remotes/trunk' must start with 'refs/'
|
| not ok - 2 git svn clone --stdlayout sets up trunk as master
| #
| #               git svn clone -s "$svnrepo" g &&
| #               (
| #                       cd g &&
| #                       test x`git rev-parse --verify refs/remotes/trunk^0` = \
| #                            x`git rev-parse --verify refs/heads/master^0`
| #               )
| #
|
| # failed 1 among 2 test(s)
| 1..2
| make[3]: *** [t9145-git-svn-master-branch.sh] Error 1

Does it work for you?  This is with a merge of git 1.7.8-rc0 and 1.7.7.2.