You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by Michael Osipov <mi...@apache.org> on 2018/09/09 10:46:35 UTC

Inconsistencies/bugs in peg revision parsing and help description

Folks,

while working on SCM-859 I have found some oddities on Subversion 1.9.7 
and 1.10.2.

Here are some examples for me which shall not be marked as an error or 
the documentation does not mention @REV/@PEG at all:

 > PS D:\Entwicklung\Projekte\toll@repo@test> svn info
 > Path: .
 > Working Copy Root Path: D:\Entwicklung\Projekte\toll@repo@test
 > URL: file:///D:/Entwicklung/Projekte/toll@repo
 > Relative URL: ^/
 > Repository Root: file:///D:/Entwicklung/Projekte/toll@repo
 > Repository UUID: 93f9f50a-2bc4-b345-885e-ec2050f72365
 > Revision: 9
 > Node Kind: directory
 > Schedule: normal
 > Last Changed Author: mosipov
 > Last Changed Rev: 9
 > Last Changed Date: 2018-09-09 12:06:38 +0200 (So, 09 Sep 2018)
 >
 > PS D:\Entwicklung\Projekte\toll@repo@test> svn mkdir --parents 
file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4 -m "dir"
 > svn: E200009: 
'file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4': a peg 
revision is not allowed here
 >
 > PS D:\Entwicklung\Projekte\toll@repo@test> svn export 
file:///D:/Entwicklung/Projekte/toll@repo@ ../ex@23
 > svn: E200009: '../ex@23': a peg revision is not allowed here

This list is not exhaustive, but merely an example of what is wrong. I 
simply don't expect any PEG parsing here at all.

Any thoughts?

Michael

Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Branko Čibej <br...@apache.org>.
On 09.09.2018 14:20, Mark Phippard wrote:
> On Sep 9, 2018, at 8:15 AM, Michael Osipov <mi...@apache.org> wrote:
>> Am 2018-09-09 um 13:50 schrieb Mark Phippard:
>>>>>> Any thoughts?
>>>>> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:
>>>> Correct.
>>>>
>>>>> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."
>>>> Hi Mark,
>>>>
>>>> I am aware of that paragraph and this is what I did actually: https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed
>>>>
>>>> It is still not clear why mkdir or export are subject to PEG parsing where it makes no sense at all, imho.
>>> My guess is that there is just one parser used in the code base, but do not know.  I do tend to agree that it seems to not make sense.  It is something that may have been discussed before and maybe someone had a logic behind just being consistent everywhere?
>> The common parser was the first idea which came into my mind too.
> After sending my last reply, it occurred to me that if there were not a common parser and a consistent set of rules then it would make your "simple workaround" a lot more difficult to use.  You would have to be aware on a command by command basis what the rules were and when you need to add an @ and when you had to not do that.  I would not be surprised if that sort of discussion happened back when this feature was added ... which was many years ago now.

Indeed. The interpretation of paths and URLs in the command-line must be
consistent, regardless of the command context. That makes the code
simpler, but more importantly, it makes it easier to educate users. (Or
at least, ideally it would make it easier ...).

Adding exceptions here just because parsing peg revisions appears
redundant in some places is definitely not the way we want to go.


>> Do you think it is worthwhile to file some issues about the incorrect help output?
> I would recommend posting here on what the specific changes are that you think ought to be made.  I did not understand from your original post what you were suggesting was incorrect.  Having the command transcripts was nice but you did not really highlight where you thought there were problems.

And a hint: use the bindings if you want to be explicit about peg
revisions. The @ in paths is only special in the command-line client,
not in the underlying APIs.

-- Brane


Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Michael Osipov <mi...@apache.org>.
Am 2018-09-09 um 23:41 schrieb Daniel Shahaf:
> Michael Osipov wrote on Sun, 09 Sep 2018 20:35 +0200:
>> I would expect the help output to contain information that target which
>> do not explicitly have [@PEGREV] documented are still subject to peg rev
>> parsing and can cause subtile error messages which cannot be explained
>> via help:
>>> export: Create an unversioned copy of a tree.
>>> usage: 1. export [-r REV] URL[@PEGREV] [PATH]
>>>         2. export [-r REV] PATH1[@PEGREV] [PATH2]
>>>
>>>    1. Exports a clean directory tree from the repository specified by
>>>       URL, at revision REV if it is given, otherwise at HEAD, into
>>>       PATH. If PATH is omitted, the last component of the URL is used
>>>       for the local directory name.
>>>
>>>    2. Exports a clean directory tree from the working copy specified by
>>>       PATH1, at revision REV if it is given, otherwise at WORKING, into
>>>       PATH2.  If PATH2 is omitted, the last component of the PATH1 is used
>>>       for the local directory name. If REV is not specified, all local
>>>       changes will be preserved.  Files not under version control will
>>>       not be copied.
>>>
>>>    If specified, PEGREV determines in which revision the target is first
>>>    looked up.
>>
>> PATH and PATH2 are not subject to @PEGREV.
> 
> I think you have a point.
> 
> In «svn export foo bar», 'foo' is a coordinate in the history — a (path,
> revision) tuple — and therefore supports peg revisions.  However, that's
> not true for 'bar', which is a coordinate in the path space only, and
> not even the in-repository path space.  For example, one could easily
> imagine an 'svn export' syntax in which 'bar' points into an FTP upload
> space, or into a git repository, etc.; specifying peg revisions makes no
> more sense for unversioned local filesystem paths than for these examples.
> 
> A similar argument holds for «svn mkdir baz».  In that case, 'baz' is a
> coordinate in the repository's path space, but it does not yet exist in
> the repository's revision space; there is no (path, revision) tuple for
> 'baz' and hence no natural value to set the peg revision too.
> 
> So, I agree, it would have made sense not to parse peg revisions in
> these cases.

Correct.

> That's not to say that we should change now, though.  There are
> backwards compatibility implications to be considered.
> 
> We can, at least, clarify the synopsis.  For example:
> .
>      - usage: 1. export [-r REV] URL[@PEGREV] [PATH]
>      -        2. export [-r REV] PATH1[@PEGREV] [PATH2]
>      + usage: 1. export [-r REV] URL[@PEGREV] [UNVERSIONED_PATH[@]]
>      +        2. export [-r REV] WC_PATH[@PEGREV] [UNVERSIONED_PATH[@]]
> .
> where I made two changes: I added a [@] trailer and renamed the
> placeholder arguments.

That looks like a good comprise, but it does not explain to the user the 
purpose of the optional @. Especially because the former optional ARG 
says @PEGREV. Do you want to add some reasonable description for that?

One needs to identify further commands having the same problem, e.g., 
'svn cp': target with PEGREV does not make any sense.

Michael


Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Michael Osipov wrote on Sun, 09 Sep 2018 20:35 +0200:
> I would expect the help output to contain information that target which 
> do not explicitly have [@PEGREV] documented are still subject to peg rev 
> parsing and can cause subtile error messages which cannot be explained 
> via help:
> > export: Create an unversioned copy of a tree.
> > usage: 1. export [-r REV] URL[@PEGREV] [PATH]
> >        2. export [-r REV] PATH1[@PEGREV] [PATH2]
> > 
> >   1. Exports a clean directory tree from the repository specified by
> >      URL, at revision REV if it is given, otherwise at HEAD, into
> >      PATH. If PATH is omitted, the last component of the URL is used
> >      for the local directory name.
> > 
> >   2. Exports a clean directory tree from the working copy specified by
> >      PATH1, at revision REV if it is given, otherwise at WORKING, into
> >      PATH2.  If PATH2 is omitted, the last component of the PATH1 is used
> >      for the local directory name. If REV is not specified, all local
> >      changes will be preserved.  Files not under version control will
> >      not be copied.
> > 
> >   If specified, PEGREV determines in which revision the target is first
> >   looked up.
> 
> PATH and PATH2 are not subject to @PEGREV.

I think you have a point.

In «svn export foo bar», 'foo' is a coordinate in the history — a (path,
revision) tuple — and therefore supports peg revisions.  However, that's
not true for 'bar', which is a coordinate in the path space only, and
not even the in-repository path space.  For example, one could easily
imagine an 'svn export' syntax in which 'bar' points into an FTP upload
space, or into a git repository, etc.; specifying peg revisions makes no
more sense for unversioned local filesystem paths than for these examples.

A similar argument holds for «svn mkdir baz».  In that case, 'baz' is a
coordinate in the repository's path space, but it does not yet exist in
the repository's revision space; there is no (path, revision) tuple for
'baz' and hence no natural value to set the peg revision too.

So, I agree, it would have made sense not to parse peg revisions in
these cases.

That's not to say that we should change now, though.  There are
backwards compatibility implications to be considered.

We can, at least, clarify the synopsis.  For example:
.
    - usage: 1. export [-r REV] URL[@PEGREV] [PATH]
    -        2. export [-r REV] PATH1[@PEGREV] [PATH2]
    + usage: 1. export [-r REV] URL[@PEGREV] [UNVERSIONED_PATH[@]]
    +        2. export [-r REV] WC_PATH[@PEGREV] [UNVERSIONED_PATH[@]]
.
where I made two changes: I added a [@] trailer and renamed the
placeholder arguments.

Cheers,

Daniel

Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Michael Osipov <mi...@apache.org>.
Am 2018-09-09 um 14:20 schrieb Mark Phippard:
> On Sep 9, 2018, at 8:15 AM, Michael Osipov <mi...@apache.org> wrote:
>>
>> Am 2018-09-09 um 13:50 schrieb Mark Phippard:
>>>>>> Any thoughts?
>>>>> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:
>>>>
>>>> Correct.
>>>>
>>>>> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."
>>>>
>>>> Hi Mark,
>>>>
>>>> I am aware of that paragraph and this is what I did actually: https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed
>>>>
>>>> It is still not clear why mkdir or export are subject to PEG parsing where it makes no sense at all, imho.
>>> My guess is that there is just one parser used in the code base, but do not know.  I do tend to agree that it seems to not make sense.  It is something that may have been discussed before and maybe someone had a logic behind just being consistent everywhere?
>>
>> The common parser was the first idea which came into my mind too.
> 
> After sending my last reply, it occurred to me that if there were not a common parser and a consistent set of rules then it would make your "simple workaround" a lot more difficult to use.  You would have to be aware on a command by command basis what the rules were and when you need to add an @ and when you had to not do that.  I would not be surprised if that sort of discussion happened back when this feature was added ... which was many years ago now.
> 
>> Do you think it is worthwhile to file some issues about the incorrect help output?
> 
> I would recommend posting here on what the specific changes are that you think ought to be made.  I did not understand from your original post what you were suggesting was incorrect.  Having the command transcripts was nice but you did not really highlight where you thought there were problems.

I would expect the help output to contain information that target which 
do not explicitly have [@PEGREV] documented are still subject to peg rev 
parsing and can cause subtile error messages which cannot be explained 
via help:
> export: Create an unversioned copy of a tree.
> usage: 1. export [-r REV] URL[@PEGREV] [PATH]
>        2. export [-r REV] PATH1[@PEGREV] [PATH2]
> 
>   1. Exports a clean directory tree from the repository specified by
>      URL, at revision REV if it is given, otherwise at HEAD, into
>      PATH. If PATH is omitted, the last component of the URL is used
>      for the local directory name.
> 
>   2. Exports a clean directory tree from the working copy specified by
>      PATH1, at revision REV if it is given, otherwise at WORKING, into
>      PATH2.  If PATH2 is omitted, the last component of the PATH1 is used
>      for the local directory name. If REV is not specified, all local
>      changes will be preserved.  Files not under version control will
>      not be copied.
> 
>   If specified, PEGREV determines in which revision the target is first
>   looked up.

PATH and PATH2 are not subject to @PEGREV.

Michael



Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Mark Phippard <ma...@gmail.com>.
On Sep 9, 2018, at 8:15 AM, Michael Osipov <mi...@apache.org> wrote:
> 
> Am 2018-09-09 um 13:50 schrieb Mark Phippard:
>>>>> Any thoughts?
>>>> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:
>>> 
>>> Correct.
>>> 
>>>> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."
>>> 
>>> Hi Mark,
>>> 
>>> I am aware of that paragraph and this is what I did actually: https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed
>>> 
>>> It is still not clear why mkdir or export are subject to PEG parsing where it makes no sense at all, imho.
>> My guess is that there is just one parser used in the code base, but do not know.  I do tend to agree that it seems to not make sense.  It is something that may have been discussed before and maybe someone had a logic behind just being consistent everywhere?
> 
> The common parser was the first idea which came into my mind too.

After sending my last reply, it occurred to me that if there were not a common parser and a consistent set of rules then it would make your "simple workaround" a lot more difficult to use.  You would have to be aware on a command by command basis what the rules were and when you need to add an @ and when you had to not do that.  I would not be surprised if that sort of discussion happened back when this feature was added ... which was many years ago now.

> Do you think it is worthwhile to file some issues about the incorrect help output?

I would recommend posting here on what the specific changes are that you think ought to be made.  I did not understand from your original post what you were suggesting was incorrect.  Having the command transcripts was nice but you did not really highlight where you thought there were problems.

Mark

Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Michael Osipov <mi...@apache.org>.
Am 2018-09-09 um 13:50 schrieb Mark Phippard:
> 
>>>> Any thoughts?
>>> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:
>>
>> Correct.
>>
>>> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."
>>
>> Hi Mark,
>>
>> I am aware of that paragraph and this is what I did actually: https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed
>>
>> It is still not clear why mkdir or export are subject to PEG parsing where it makes no sense at all, imho.
> 
> My guess is that there is just one parser used in the code base, but do not know.  I do tend to agree that it seems to not make sense.  It is something that may have been discussed before and maybe someone had a logic behind just being consistent everywhere?

The common parser was the first idea which came into my mind too.

Do you think it is worthwhile to file some issues about the incorrect 
help output?

>> As far as I understand the paragraph, it is idiotproof to append the @ to all possible spots and have the issue fixed with thhat?
> 
> I believe so, yes.

Thanks, I'll keep the code as-is for now.

Michael


Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Mark Phippard <ma...@gmail.com>.
>>> Any thoughts?
>> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:
> 
> Correct.
> 
>> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."
> 
> Hi Mark,
> 
> I am aware of that paragraph and this is what I did actually: https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed
> 
> It is still not clear why mkdir or export are subject to PEG parsing where it makes no sense at all, imho.

My guess is that there is just one parser used in the code base, but do not know.  I do tend to agree that it seems to not make sense.  It is something that may have been discussed before and maybe someone had a logic behind just being consistent everywhere?

> As far as I understand the paragraph, it is idiotproof to append the @ to all possible spots and have the issue fixed with thhat?

I believe so, yes.

Mark

Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Michael Osipov <mi...@apache.org>.
Am 2018-09-09 um 13:33 schrieb Mark Phippard:
> 
>> On Sep 9, 2018, at 6:46 AM, Michael Osipov <mi...@apache.org> wrote:
>>
>> Folks,
>>
>> while working on SCM-859 I have found some oddities on Subversion 1.9.7 and 1.10.2.
>>
>> Here are some examples for me which shall not be marked as an error or the documentation does not mention @REV/@PEG at all:
>>
>>> PS D:\Entwicklung\Projekte\toll@repo@test> svn info
>>> Path: .
>>> Working Copy Root Path: D:\Entwicklung\Projekte\toll@repo@test
>>> URL: file:///D:/Entwicklung/Projekte/toll@repo
>>> Relative URL: ^/
>>> Repository Root: file:///D:/Entwicklung/Projekte/toll@repo
>>> Repository UUID: 93f9f50a-2bc4-b345-885e-ec2050f72365
>>> Revision: 9
>>> Node Kind: directory
>>> Schedule: normal
>>> Last Changed Author: mosipov
>>> Last Changed Rev: 9
>>> Last Changed Date: 2018-09-09 12:06:38 +0200 (So, 09 Sep 2018)
>>>
>>> PS D:\Entwicklung\Projekte\toll@repo@test> svn mkdir --parents file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4 -m "dir"
>>> svn: E200009: 'file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4': a peg revision is not allowed here
>>>
>>> PS D:\Entwicklung\Projekte\toll@repo@test> svn export file:///D:/Entwicklung/Projekte/toll@repo@ ../ex@23
>>> svn: E200009: '../ex@23': a peg revision is not allowed here
>>
>> This list is not exhaustive, but merely an example of what is wrong. I simply don't expect any PEG parsing here at all.
>>
>> Any thoughts?
> 
> If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:

Correct.

> "The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."

Hi Mark,

I am aware of that paragraph and this is what I did actually: 
https://github.com/apache/maven-scm/commit/c1f4f0fe1e0fafb876e098d8ecc17745664396ed

It is still not clear why mkdir or export are subject to PEG parsing 
where it makes no sense at all, imho.

As far as I understand the paragraph, it is idiotproof to append the @ 
to all possible spots and have the issue fixed with thhat?

Michael


Re: Inconsistencies/bugs in peg revision parsing and help description

Posted by Mark Phippard <ma...@gmail.com>.
> On Sep 9, 2018, at 6:46 AM, Michael Osipov <mi...@apache.org> wrote:
> 
> Folks,
> 
> while working on SCM-859 I have found some oddities on Subversion 1.9.7 and 1.10.2.
> 
> Here are some examples for me which shall not be marked as an error or the documentation does not mention @REV/@PEG at all:
> 
> > PS D:\Entwicklung\Projekte\toll@repo@test> svn info
> > Path: .
> > Working Copy Root Path: D:\Entwicklung\Projekte\toll@repo@test
> > URL: file:///D:/Entwicklung/Projekte/toll@repo
> > Relative URL: ^/
> > Repository Root: file:///D:/Entwicklung/Projekte/toll@repo
> > Repository UUID: 93f9f50a-2bc4-b345-885e-ec2050f72365
> > Revision: 9
> > Node Kind: directory
> > Schedule: normal
> > Last Changed Author: mosipov
> > Last Changed Rev: 9
> > Last Changed Date: 2018-09-09 12:06:38 +0200 (So, 09 Sep 2018)
> >
> > PS D:\Entwicklung\Projekte\toll@repo@test> svn mkdir --parents file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4 -m "dir"
> > svn: E200009: 'file:///D:/Entwicklung/Projekte/toll@repo/non/existent@4': a peg revision is not allowed here
> >
> > PS D:\Entwicklung\Projekte\toll@repo@test> svn export file:///D:/Entwicklung/Projekte/toll@repo@ ../ex@23
> > svn: E200009: '../ex@23': a peg revision is not allowed here
> 
> This list is not exhaustive, but merely an example of what is wrong. I simply don't expect any PEG parsing here at all.
> 
> Any thoughts?

If I understand your examples, you are showing what happens when the filename contains an @, right?  If so, this is addressed in the book in this paragraph:

"The perceptive reader is probably wondering at this point whether the peg revision syntax causes problems for working copy paths or URLs that actually have at signs in them. After all, how does svn know whether news@11 is the name of a directory in my tree or just a syntax for “revision 11 of news”? Thankfully, while svn will always assume the latter, there is a trivial workaround. You need only append an at sign to the end of the path, such as news@11@. svn cares only about the last at sign in the argument, and it is not considered illegal to omit a literal peg revision specifier after that at sign. This workaround even applies to paths that end in an at sign—you would use filename@@ to talk about a file namedfilename@."

Mark