On 10.11.2018 01:31, Daniel Shahaf wrote:
> Branko Čibej wrote on Fri, 09 Nov 2018 12:56 +0100:
>> On 09.11.2018 12:54, brane_at_apache.org wrote:
>>> Author: brane
>>> Date: Fri Nov 9 11:54:21 2018
>>> New Revision: 1846237
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1846237&view=rev
>>> Log:
>>> More tests for peg revision parsing.
>> [...]
>>> +@Wimp("Removes E instead of reporting ENOENT or E125001 for E/@")
>>> +def remove_subdir_7a_no_escape_peg(sbox):
>>> + "remove missing 'E/@' without pegrev escape"
>>> + do_remove(sbox, 'E/@', 'E/@') #, "svn: E125001: 'E/@'")
>> Yes indeed ... our target parsing is so incredibly reliable that
>> attempting to delete a non-existent 'E/@' will happily delete 'E' with
>> all its contents. I think this may be too much of a good thing.
> The documented escaping rule for CLI consumers is to append an "@" after
> every filename. By this rule, a CLI consumer that _wants_ to remove E/
> would run "svn rm E/@". That command should parse as { target = 'E/',
> peg = '' } regardless of whether the directory "E" contains a child
> named "@".
>
> I don't understand what change you're proposing.
Consider this:
$ svn rm E@
D E
$ svn rm @
svn: E125001: '@' is just a peg revision. Maybe try '@@' instead?
$ svn rm E/@
D E
would it not be more consistent that trying to remove 'E/@' would raise
the same error as trying to remove '@'? Because otherwise the behaviour
of the client depends on whether you're removing things from the current
directory or not. This becomes especially error-prone in the case when
E/@ exists:
$ svn rm E/@
D E
D E/@
I'm pretty sure we should not remove a parent directory if the child
might have been the intended target.
These usages were not ambiguous and possibly error-prone until we
invented the peg-revision syntax. At the time we failed to strictly
define the semantics , so maybe it's time we did that now. For example,
given the path
foo_at_bar/baz
the parser will treat 'bar/baz' as the peg-revision specifier, even
though it's obvious that's not what the user had in mind. If we changed
the parser to only look for peg revisions on the basename of the path,
then at least the only restriction we'll have is that directory
separators can't be part of the peg revision specifier — which should be
an acceptable restriction, since currently we only support known
keywords, revision numbers or dates, none of which contain forward- or
backslashes.
Obviously that would only be a parser change, peg revisions would still
apply to whole paths for command semantics.
I'm aware that doing this would change the meaning of some forms of
commands, but for now I'm fairly sure we'd only produce new errors, not
silently behave differently. From the current set of tests, it seems to
follow that this inconsistency only arises with targets whose basename
starts with '@' or '.@' (and probably '..@'), the latter because '.'
(and '..') are removed by canonicalization.
-- Brane
Received on 2018-11-10 15:12:58 CET