Jonathan Nieder [off-list ref] writes:
Michael G Schwern wrote:
quoted
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of "yes" is enough, no need to resend patches to do this.)
Here it is in patch form for reviewers. If I understand correctly,
the idea is to replace accesses to $gs->{path} with calls to a
$gs->path function that canonicalizes (and likewise for s/path/url/).
There are probably other subtleties, but that seems to be the gist.
The impression I am getting is that the updated code wants to handle
URL and paths without any funny encoding, but it is unclear from my
cursory read (e.g. what goes on with escape_url?).
if ($old_url =~ m#^svn(\+ssh)?://# ||
($full_url =~ m#^https?://# &&
- escape_url($full_url) ne $full_url)) {
+ $full_url ne $full_url)) {
How can the latter part of this conditional be true?
On 2012.7.24 4:45 PM, Junio C Hamano wrote:
quoted
quoted
git log -p schwern/git-svn/extract-classes..schwern/git-svn/fix-canonical
That should give you the information you need...
I guess so. May we have your sign-off on these changes? (A simple
reply of "yes" is enough, no need to resend patches to do this.)
Here it is in patch form for reviewers. If I understand correctly,
the idea is to replace accesses to $gs->{path} with calls to a
$gs->path function that canonicalizes (and likewise for s/path/url/).
There are probably other subtleties, but that seems to be the gist.
The impression I am getting is that the updated code wants to handle
URL and paths without any funny encoding, but it is unclear from my
cursory read (e.g. what goes on with escape_url?).
No, now it's just canonicalizing as early as possible. Preferably within the
object accessor rather than at the point of use. So in the code below,
$full_url is already escaped/canonicalized.
In general this blob patch isn't going to make a lot of overall sense. I'm
working with Jonathan to get it submitted in manageable pieces.
quoted
if ($old_url =~ m#^svn(\+ssh)?://# ||
($full_url =~ m#^https?://# &&
- escape_url($full_url) ne $full_url)) {
+ $full_url ne $full_url)) {
How can the latter part of this conditional be true?
Good point. More importantly, what was it trying to accomplish before and
does it need to be preserved?
"If the URL is svn OR its http and needs to be escaped... do something
special". I don't really understand what the special stuff in the following
block is. Anything that undef's the invocant (ie. $self) is probably broken.
a51cdb0c0420ee3bef26bbd1a9aa75e1d464e5b7 and
2a679c7a3148978a3f58f1c12100383638e744c5 shed some light. 2a679 looks like it
specifically holds off on escaping $full_url. It would be very nice if that
was not necessary. It would be helpful if the bug mentioned in 2a679 could be
reproduced to see if it still applies or can be dealt with in another way.
--
185. My name is not a killing word.
-- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army
http://skippyslist.com/list/
Michael G Schwern wrote:
No, now it's just canonicalizing as early as possible. Preferably within the
object accessor rather than at the point of use. So in the code below,
$full_url is already escaped/canonicalized.
Let's start with this.
Is svn_path_canonicalize() idempotent? What does it do when it
encounters a percent-sign?
Thanks,
Jonathan
On 2012.7.24 9:53 PM, Jonathan Nieder wrote:
Michael G Schwern wrote:
quoted
No, now it's just canonicalizing as early as possible. Preferably within the
object accessor rather than at the point of use. So in the code below,
$full_url is already escaped/canonicalized.
Let's start with this.
Is svn_path_canonicalize() idempotent? What does it do when it
encounters a percent-sign?
Nothing, because paths are not URI escaped. :)
You probably meant svn_uri_canonicalize(). And no, it does not double escape,
so its safe to escape as early as possible.
use SVN::Core;
my $uri = "http://www.example.com/ foo";
print SVN::_Core::svn_uri_canonicalize(
SVN::_Core::svn_uri_canonicalize($uri)
);
That produces "http://www.example.com/%20foo".
The API docs don't say it specifically, but if it were otherwise it would be
impossible to use. You'd have to check first if anything were escaped before
canonicalizing. And a user couldn't pass in an escaped URL without risking it
being double escaped.
http://subversion.apache.org/docs/api/latest/svn__dirent__uri_8h.html#a8bae33a2fbf86857869f7b0e39a553e7
--
'All anyone gets in a mirror is themselves,' she said. 'But what you
gets in a good gumbo is everything.'
-- "Witches Abroad" by Terry Prachett