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
