You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@apache.org> on 2018/10/27 21:19:10 UTC
SVN-4530 describes a really nasty bug in our command-line parsing
First, read this: https://issues.apache.org/jira/browse/SVN-4530
Then watch this (hold my beer):
$ svnadmin create repo
$ svn co file://$(pwd)/repo wc
Checked out revision 0.
$ svn mkdir wc/foo
A wc/foo
$ touch wc/foo/@bar
$ svn add wc/foo/@bar
svn: E200009: 'wc/foo@bar': a peg revision is not allowed here
Oh really? Since when are we allowed to ignore directory separators in
paths? And why the blazes would 'svn add' ever look for the peg revision
tag?
Digging into this now ... any pointers would be most welcome.
-- Brane
P.S.: I'm also trying to find that mail from years ago when I explained
why I hate this silly peg revision syntax.
Re: SVN-4530 describes a really nasty bug in our command-line parsing
Posted by James McCoy <ja...@jamessan.com>.
On Sat, Oct 27, 2018 at 11:19:10PM +0200, Branko Čibej wrote:
> First, read this: https://issues.apache.org/jira/browse/SVN-4530
>
> Then watch this (hold my beer):
>
> $ svnadmin create repo
> $ svn co file://$(pwd)/repo wc
> Checked out revision 0.
> $ svn mkdir wc/foo
> A wc/foo
> $ touch wc/foo/@bar
> $ svn add wc/foo/@bar
> svn: E200009: 'wc/foo@bar': a peg revision is not allowed here
>
> Oh really? Since when are we allowed to ignore directory separators in
> paths? And why the blazes would 'svn add' ever look for the peg revision
> tag?
Is it generic code handling the peg revision and then once that gets
processed, wc/foo/ is normalized to wc/foo? Seems like it since "svn
add wc/foo/@bar@" works.
Cheers,
--
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7 2D23 DFE6 91AE 331B A3DB
Re: SVN-4530 describes a really nasty bug in our command-line parsing
Posted by Branko Čibej <br...@apache.org>.
On 28.10.2018 01:57, Daniel Shahaf wrote:
> Branko Čibej wrote on Sat, 27 Oct 2018 23:19 +0200:
>> $ touch wc/foo/@bar
>> $ svn add wc/foo/@bar
>> svn: E200009: 'wc/foo@bar': a peg revision is not allowed here
>>
>> Oh really? Since when are we allowed to ignore directory separators in
>> paths?
> wc/foo/@bar is parsed as path="wc/foo/", peg="bar", and the trailing
> slash is stripped by canonicalization.
Yes ... and this is wrong. We do have functions that properly
concatenate bits of paths, so it looks like we're not using them in at
least one place where we should.
[later] Yup ... we explicitly canonicalize the path then join the parts
back with apr_pstrcat. Oh dear.
> We _are_ overzealous about removing slashes: 'svn diff README/' works,
> because of how early we canonicalize, but really shouldn't. However,
> that's orthogonal to peg revisions.
>
>> And why the blazes would 'svn add' ever look for the peg revision tag?
> I assume it's for consistency, so scripts that call svn can do
> .
> svn foo -- "${filename}@"
> .
> regardless of the values of foo and ${filename}.
Well that won't work for the second argument of 'svn rename', see below.
>> Digging into this now ... any pointers would be most welcome.
> The example you gave results in an error message, so changing it raises
> little compatibility concerns. However, when the peg revision is empty,
> that's not the case. Consider:
>
> % mkdir foo
> % touch foo/@
> % touch foo/bar@
> % touch foo/bar
> % svn add foo/@ # currently equivalent to 'svn add foo/'
> % svn add foo/bar@ # currently equivalent to 'svn add foo/bar'
>
> Making 'add' not parse peg revisions will change the meaning of
> these commands.
Indeed.
The original issue is about this case:
svn rename foo/@bar foo/@baz # fails
svn rename foo/@bar@ foo/@baz # creates foo@baz???
svn rename foo/@bar@ foo/@baz@ # creates foo/@baz@
Fixing this without breaking backward compatibility in edge cases may
not be as trivial as I'd thought.
-- Brane
Re: SVN-4530 describes a really nasty bug in our command-line parsing
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Sat, 27 Oct 2018 23:19 +0200:
> $ touch wc/foo/@bar
> $ svn add wc/foo/@bar
> svn: E200009: 'wc/foo@bar': a peg revision is not allowed here
>
> Oh really? Since when are we allowed to ignore directory separators in
> paths?
wc/foo/@bar is parsed as path="wc/foo/", peg="bar", and the trailing
slash is stripped by canonicalization.
We _are_ overzealous about removing slashes: 'svn diff README/' works,
because of how early we canonicalize, but really shouldn't. However,
that's orthogonal to peg revisions.
> And why the blazes would 'svn add' ever look for the peg revision tag?
I assume it's for consistency, so scripts that call svn can do
.
svn foo -- "${filename}@"
.
regardless of the values of foo and ${filename}.
> Digging into this now ... any pointers would be most welcome.
The example you gave results in an error message, so changing it raises
little compatibility concerns. However, when the peg revision is empty,
that's not the case. Consider:
% mkdir foo
% touch foo/@
% touch foo/bar@
% touch foo/bar
% svn add foo/@ # currently equivalent to 'svn add foo/'
% svn add foo/bar@ # currently equivalent to 'svn add foo/bar'
Making 'add' not parse peg revisions will change the meaning of
these commands.
Cheers,
Daniel