On 30.10.2018 05:51, Daniel Shahaf wrote:
> Branko Čibej wrote on Tue, 30 Oct 2018 01:04 +0100:
>> On 29.10.2018 19:08, Daniel Shahaf wrote:
>>> Good morning Brane,
>>>
>>> brane_at_apache.org wrote on Sun, 28 Oct 2018 10:40 +0000:
>>>> Add XFAIL tests for issue #4530.
>>>>
>>>> +++ subversion/trunk/subversion/tests/cmdline/basic_tests.py Sun Oct 28 10:40:04 2018
>>>> @@ -3050,6 +3050,33 @@ def peg_rev_on_non_existent_wc_path(sbox
>>>> +def do_move_with_at_signs(sbox, src, dst, dst_cmdline):
>>>> + sbox.build()
>>>> +
>>>> + expected_status = svntest.actions.get_virginal_state(sbox.wc_dir, 1)
>>>> + expected_status.tweak(src, status='D ', moved_to=dst)
>>>> + expected_status.add({dst: Item(status='A ', copied='+',
>>>> + moved_from=src, wc_rev='-')})
>>>> +
>>>> + sbox.simple_move(src, dst_cmdline)
>>>> + svntest.actions.run_and_verify_status(sbox.wc_dir, expected_status)
>>>> +
>>>> +@Issue(4530)
>>>> +@XFail()
>>>> +def move_to_target_with_leading_at_sign(sbox):
>>>> + "rename to dir/@file"
>>>> +
>>>> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon')
>>>> +
>>>> +
>>>> +@Issue(4530)
>>>> +@XFail()
>>>> +def move_to_target_with_leading_and_trailing_at_sign(sbox):
>>>> + "rename to dir/@file@"
>>>> +
>>>> + do_move_with_at_signs(sbox, 'iota', 'A/@upsilon', 'A/@upsilon@')
>>> I'm not actually sure these are supposed to work as these tests expect:
>>> supporting A/@upsilon as the first argument like this would prevent us
>>> from adding "foo_at_upsilon" as a command-line syntax for '"foo" at peg
>>> revision svn_opt_revision_working' in a future minor release. I think
>>> this line of thought applies to the second argument as well.
>> You must have misread what the test does. A/@upsilon (and A/@upsilon@)
>> are the _second_ arguments to 'svn rename'.
> Indeed I have. Apologies.
>
>> The command tested is one of:
>>
>> * 'svn rename iota A/@upsilon' (whch creates A_at_upsilon, which is wrong)
> Agreed.
>
>> * 'svn rename iota A/@upsilont@' (which creates A/@upsilon@ which is
>> also wrong)
>>
> Agreed.
>
>> So there's no way to rename iota to A/@upsilon. We effectively forbd
>> renames to fiels with leading @-signs, unless one jumps through a lot of
>> hoops. Consider this case, trying to do the rename in the same directory:
>> $ svn mv @bar@ @qux
>> svn: E125001: '@qux' is just a peg revision. Maybe try '@qux@' instead?
>> $ svn mv @bar@ @qux@
>> A @qux@
>> D @bar
>>
>>
>> Notice how the code complains about bare '@qux' but doesn't about
>> 'foo/@qux'. There are so many bugs in those few lines of code that I've
>> not even counted them all yet.
> "@qux" should be equivalent to ".@qux", I assume? Since "." is canonicalized
> to "".
Actually, as the code currently stands, it will error out for "@qux"
("'@qux' is just a peg revision ...") but not for "._at_qux" because
canonicalization happens later.
Actually it does error out for "._at_qux":
$ svn mkdir ._at_brl
svn: E200009: '@brl': a peg revision is not allowed here
but this happens later, after canonicalisation, yet still before it
tries to create the current directory. This seems to go through the same
code path as 'svn add'.
>>> I can see a number of options:
>>>
>>> - All PATH arguments (and all TARGET formal arguments when the actual
>>> argument isn't a URI) are parsed for peg revisions, no exceptions.
>> They seem to be, but edge cases are handled incorrectly.
> +1
>
> Thanks for the patient answer.
Oh come, you know it wasn't really all that patient, this is me after
all. :)
Truthfully I'm just a bit angry at myself for not adding a bunch of
tests (or insisting they be added) when the peg-revision syntax was
first implemented.
-- Brane
Received on 2018-10-30 09:59:24 CET