All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Eric Wong <normalperson@yhbt.net>
Cc: Michael G Schwern <schwern@pobox.com>,
	git@vger.kernel.org, gitster@pobox.com, robbat2@gentoo.org,
	bwalton@artsci.utoronto.ca
Subject: [PATCH] git-svn: keep leading slash when canonicalizing paths (fallback case)
Date: Fri, 5 Oct 2012 00:04:31 -0700	[thread overview]
Message-ID: <20121005070430.GA23572@elie.Belkin> (raw)
In-Reply-To: <5014387C.50903@pobox.com>

Subversion's svn_dirent_canonicalize() and svn_path_canonicalize()
APIs keep a leading slash in the return value if one was present on
the argument, which can be useful since it allows relative and
absolute paths to be distinguished.

When git-svn's canonicalize_path() learned to use these functions if
available, its semantics changed in the corresponding way.  Some new
callers rely on the leading slash --- for example, if the slash is
stripped out then _canonicalize_url_ourselves() will transform
"proto://host/path/to/resource" to "proto://hostpath/to/resource".

Unfortunately the fallback _canonicalize_path_ourselves(), used when
the appropriate SVN APIs are not usable, still follows the old
semantics, so if that code path is exercised then it breaks.  Fix it
to follow the new convention.

Noticed by forcing the fallback on and running tests.  Without this
patch, t9101.4 fails:

 Bad URL passed to RA layer: Unable to open an ra_local session to \
 URL: Local URL 'file://homejrnsrcgit-scratch/t/trash%20directory.\
 t9101-git-svn-props/svnrepo' contains unsupported hostname at \
 /home/jrn/src/git-scratch/perl/blib/lib/Git/SVN.pm line 148

With it, the git-svn tests pass again.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi Eric,

Michael G Schwern wrote:
> On 2012.7.28 6:55 AM, Jonathan Nieder wrote:

>> When would this "else" case trip?
>
> When svn_path_canonicalize() does not exist in the SVN API, presumably because
> their SVN is too old.

I accidentally tested this "else" branch by making the other cases
false.  t9101.4 failed as described above, or in other words,
canonicalize_url_ourselves() stripped out a few too many slashes.  For
reference:

| sub _canonicalize_url_ourselves {
|         my ($url) = @_;
|         if ($url =~ m#^([^:]+)://([^/]*)(.*)$#) {
|                 my ($scheme, $domain, $uri) = ($1, $2, _canonicalize_url_path(canonicalize_path($3)));
|                 $url = "$scheme://$domain$uri";
|         }
|         $url;
| }

When $url is http://host/path/to/resource,

	$1 = "http", $2 = "host", $3 = "/path/to/resource"
	canonicalize_path($3) = "path/to/resource" <--- (??)
	_canonicalize_url_path(ditto) = "path/to/resource"
	$url = "http://hostpath/to/resource"

How about this patch?

 perl/Git/SVN/Utils.pm |    1 -
 1 file changed, 1 deletion(-)

diff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm
index 4bb4dde8..8b8cf375 100644
--- a/perl/Git/SVN/Utils.pm
+++ b/perl/Git/SVN/Utils.pm
@@ -122,7 +122,6 @@ sub _canonicalize_path_ourselves {
 	$path = _collapse_dotdot($path);
 	$path =~ s#/$##g;
 	$path =~ s#^\./## if $dot_slash_added;
-	$path =~ s#^/##;
 	$path =~ s#^\.$##;
 	return $path;
 }
-- 
1.7.10.4

  parent reply	other threads:[~2012-10-05  7:04 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-28  9:38 Canonicalize the git-svn path & url accessors Michael G. Schwern
2012-07-28  9:38 ` [PATCH 1/7] Move the canonicalization functions to Git::SVN::Utils Michael G. Schwern
2012-07-28  9:38 ` [PATCH 2/7] Change canonicalize_url() to use the SVN 1.7 API when available Michael G. Schwern
2012-07-28 13:50   ` Jonathan Nieder
2012-07-28 19:01     ` Michael G Schwern
2012-07-28 19:30       ` Jonathan Nieder
2012-07-28 19:51         ` Michael G Schwern
2012-07-28 19:57           ` Jonathan Nieder
2012-07-28 20:02             ` Jonathan Nieder
2012-07-28 20:24               ` Michael G Schwern
2012-10-14 11:42       ` [PATCH/RFC 0/2] " Jonathan Nieder
2012-10-14 11:45         ` [PATCH 1/2] git svn: do not overescape URLs (fallback case) Jonathan Nieder
2012-10-14 11:48         ` [PATCH 2/2] git svn: canonicalize_url(): use svn_path_canonicalize when available Jonathan Nieder
2012-10-23 22:58           ` Eric Wong
2012-07-28  9:38 ` [PATCH 3/7] Extract, test and enhance the logic to collapse ../foo paths Michael G. Schwern
2012-07-30 19:51   ` Eric Wong
2012-07-30 20:46     ` Michael G Schwern
2012-09-26 19:45       ` Jonathan Nieder
2012-09-26 20:58         ` Eric Wong
2012-09-26 21:38           ` Jonathan Nieder
2012-09-26 21:54             ` Eric Wong
2012-09-26 22:43               ` Jonathan Nieder
2012-09-27  0:15                 ` Eric Wong
2012-09-27  2:11                   ` Jonathan Nieder
2012-07-28  9:38 ` [PATCH 4/7] Add join_paths() to safely concatenate paths Michael G. Schwern
2012-09-26 20:51   ` Jonathan Nieder
2012-07-28  9:38 ` [PATCH 5/7] Remove irrelevant comment Michael G. Schwern
2012-07-28  9:38 ` [PATCH 6/7] Switch path canonicalization to use the SVN API Michael G. Schwern
2012-07-28 13:55   ` Jonathan Nieder
2012-07-28 19:07     ` Michael G Schwern
2012-07-30 20:04       ` Eric Wong
2012-08-02 21:51         ` Eric Wong
2012-08-02 23:18           ` Michael G Schwern
2012-10-05  7:04       ` Jonathan Nieder [this message]
2012-10-05 23:12         ` [PATCH] git-svn: keep leading slash when canonicalizing paths (fallback case) Eric Wong
2012-10-06  5:36           ` Junio C Hamano
2012-07-28  9:38 ` [PATCH 7/7] Make Git::SVN and Git::SVN::Ra canonicalize paths and urls Michael G. Schwern
2012-07-28 14:11   ` Jonathan Nieder
2012-07-28 19:15     ` Michael G Schwern

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121005070430.GA23572@elie.Belkin \
    --to=jrnieder@gmail.com \
    --cc=bwalton@artsci.utoronto.ca \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=normalperson@yhbt.net \
    --cc=robbat2@gentoo.org \
    --cc=schwern@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.