Re: SVN-4530 describes a really nasty bug in our command-line parsing

2018-10-27 Thread Branko Čibej
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: E29: '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

2018-10-27 Thread Daniel Shahaf
Branko Čibej wrote on Sat, 27 Oct 2018 23:19 +0200:
> $ touch wc/foo/@bar
> $ svn add wc/foo/@bar
> svn: E29: '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


Re: SVN-4530 describes a really nasty bug in our command-line parsing

2018-10-27 Thread James McCoy
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: E29: '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


SVN-4530 describes a really nasty bug in our command-line parsing

2018-10-27 Thread Branko Čibej
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: E29: '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: The future of the Subversion book

2018-10-27 Thread Branko Čibej
On 18.10.2018 20:40, Branko Čibej wrote:
> On 06.09.2018 15:25, Branko Čibej wrote:
>> On 06.09.2018 15:10, C. Michael Pilato wrote:
>>> On 09/05/2018 04:49 PM, Mark Phippard wrote:
 Assuming the PMC wanted it, is it possible for the book to be
 contributed to this project and hosted in the Apache SVN repository?
 Many people seem to post questions and issues in these mailing lists
 as if it is part of the project anyway so maybe we ought to just
 make this the reality.  I guess what I am saying is, before we gauge
 opinion on whether we want to bring this into the project, my
 question is whether there are any blockers that prevent this on the
 book side from being an option?  Such as copyright or licensing
 issues that make it not possible.  It feels like this has been
 discussed in the past and there were reasons it was kept separate
 from the project even after the publishing of the book by O'Reilly
 was in the past, but I no longer recall them.
>>> Honestly, I think the book belongs with the PMC. It is easy to imagine a
>>> day when a developer is expected to provide at least rudimentary
>>> documentation updates in the same commit that carries his or her new
>>> feature or behavioral change.
>>>
>>> The book carries a cc-by-2.0 license, with Ben, Fitz and myself named as
>>> the copyright holders. I suspect that in order to be absorbed by the
>>> PMC, that licensing would have to change to an Apache License. Does that
>>> mean that the three primary authors would need to officially re-license
>>> it somehow? Or maybe it's a software grant to the ASF (rather like
>>> Subversion itself was)?
>> We'd have to ask legal@ but I'd be surprised if we'd be required to
>> re-license the book; it's not code, and the Apache license isn't really
>> suitable. Also we wouldn't really be making releases of it, just updates
>> on the web.
>
> I didn't quite forget about this: LEGAL-421

The answer to this question appears to be that we should relicense the
book before importing it into our repository. I suppose that means the
original authors, and any other major contributors, should create a
Software Grant.

-- Brane


Issue cleanup

2018-10-27 Thread Branko Čibej
FYI, I'm moving all our issues that are resolved and haven't been
changed since 1st January to the "closed" state. There were almost 4k of
them.

-- Brane