On 11.11.2018 10:54, Daniel Shahaf wrote:
> Branko Čibej wrote on Sun, Nov 11, 2018 at 07:47:18 +0100:
>> On 11.11.2018 07:29, Daniel Shahaf wrote:
>>> Branko Čibej wrote on Sat, Nov 10, 2018 at 15:12:48 +0100:
>>>> 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, br...@apache.org wrote:
>>> The correct syntaxes for removing "./@" and "E/@", of course, are
>>> «svn rm E/@@» and «svn rm @@» respectively.
>>>
>>> I think that «svn rm E/@» should _not_ remove "E/@", for two reasons:
>> I didn't say it should. I said it should error out just like 'svn rm @'
>> does.
>>
> It wasn't clear to me what you were proposing.
>
> It still isn't, actually. Have I overlooked a commit or email where you
> spelled out what the new algorithm would be? None of the emails in this
> thread spells out an algorithm.
That's because I don't know it yet ... since I can't describe precisely
what is wrong with the way we currently do things, but I do have the
feeling that we're doing something wrong. I was hoping this discussion
would clarify things.
>>> Whatever we do, we must have a documented syntax that means "recursively
>>> delete E/ and everything thereunder" and works regardless of what
>>> filenames E/ contains.
>> Yes, it's called 'svn rm E'.
> That won't work in the general case:
>
> % /bin/mkdir foo@bar foo@bar@ foo@bar/@
> % svn add .
> % svn ci
> % <how do I 'svn rm' foo@bar now?>
>
> Secondly, the incumbent escaping syntax is independent of the subcommand,
> filename, and tree contents: to run «svn verb» on the file called
> ${foo} on disk, one runs «svn verb -- "${foo}@"», no exceptions.
> I think this property should be preserved by the new escaping rules.
> (Not necessarily this specific syntax, but the
> independence/no-exceptions aspect.)
Agreed.
Given your example above, this is what works now:
* to remove foo@bar: 'svn rm foo@bar@' or 'svn rm foo@bar/@'
* to remove foo@bar@: 'svn rm foo@bar@@' or 'svn rm foo@bar@/@'
* to remove foo@bar/@: 'svn rm foo@bar/@@' or 'svn rm foo@bar/@/@'
I /think/ I'm objecting to the fact that those extra directory
separators in the second variants have no effect ... but I'm not yet
sure what to do about it.
>>> 2. In order to preserve the property that «svn ${subcommand} -- ${target}@»
>>> is parsed in exactly the same way regardless of the values of
>>> ${subcommand} and ${target} and regardless of the tree contents.
>> I do think that this rule was not sufficiently thought out.
>>
>>> IIRC «svn up @» used to be the same as «svn up ./», so I think we
>>> shouldn't make it mean "update the file './@'" because that would be
>>> a silent incompatibility for some users.
>> Again, I didn't say it should. I said it should be an error because
>> './@' is ambiguous.
> As a point of fact, it is not ambiguous. "./@" means {path_or_url="./",
> peg=""}.
>
> Terminology aside, the problem you have identified here is that if
> a user has a file called "@" and forgets to escape it, he doesn't get
> a syntax error. That we can address.
>
>>> I don't remember what the last
>>> minor line with the non-error meaning was, though.
>>>
>>>> Because otherwise the behaviour of the client depends on whether you're
>>>> removing things from the current directory or not.
>>> To be more precise, the question is whether one spells the command
>>> «svn rm @» or «svn rm ./@».
>> or 'svn rm .@' or 'svn rm './.@'. I'd argue that premature
>> canonicalization is a bad thing.
>>
>>
> What change, if any, do you propose?
As I said, I'm still trying to work this out. For example, one of the
things that's been driving me up the wall is that when the user writes
'foo/.@bar', the error message says a peg revision isn't allowed at
'foo@bar', regardless of whether 'foo/.@bar' exists. Yes, the syntax is
wrong, the user should have typed 'foo/.@bar@' instead, but surely we
can be smart enough to notice that instead of emitting an error about
something the user almost certainly didn't have in mind?
>>>> 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.
>>>>
>>> I see that there's an argument that we should require some option here,
>>> à la --force-log, but frankly, I don't think files literally called "@"
>>> or "@HEAD" or "@{2018-01-01}" and so on are common enough to worry
>>> about.
>> Perhaps not, but then there's SVN-4530, so I'd say they're common enough.
> Fair enough.
>
>>>> 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.
>>> I don't follow what you mean by "this"; do you refer to forbidding
>>> slashes in pegrevs or to something else? (The former wouldn't seem to
>>> address all the concerns you described.)
>>>
>>> Cheers,
>>>
>>> Daniel
>>>
>>> P.S. Other languages also have examples of usage errors that aren't
>>> syntax errors. For example, in C the tokens «+», «-», and «*» can be
>>> either unary or binary operators, so «double hypotenuse(double a, double b)
>>> { return sqrt(*a + b*b); }» is a syntax error but «return sqrt( + b*b)»
>>> is not.
>> I really don't see how this comparison is valid. C operator syntax and
>> precedence is precisely defined. Our usage of @ in paths may be
>> well-defined too, but its interaction with aggressive canonicalization
>> creates unintentional side effects in the implementation.
> The relevance of the example is that it's another case in which a usage
> error (programmer forgot to paste «a*a» after the opening parenthesis)
> is not a syntax error, in relation to your observation that using «svn
> rm E/@» to remove "E/@" isn't a syntax error.
Ack.
-- Brane
