I'm sorry for replying late.
On 2020/09/26 19:12, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Fri, 25 Sep 2020 19:09 +0900:
First of all, I'd like to correct my misunderstanding.
>> On 2020/09/25 17:05, Yasuhito FUTATSUKI wrote:
>>> On 2020/09/25 16:06, Daniel Shahaf wrote:
>>
>>>> First, if the client applies any transformation at all to property
>>>> values, that'd be a bug. Property values are opaque octet sequences,
>>>> just like file contents, so they must be emitted verbatim. It so
>>>> happens that values of svn:* properties are UTF-8 with LF line endings,
>>>> so that's what Python should see, regardless of the local encoding and
>>>> EOL style.
>>>
>>> I judged that EOL conversion of property values on 'svn pg' isn't bug
>>> because this comment is found in test for it. If it is a bug,
>>> prop_tests.py also does not test correctly.
>>
>> This behavior was introduced in r1003238, as a "fix" of the issue #3721.
>
> Good find — but if that's the case, why was the comment describing this
> behaviour was added to the test much earlier, in r845369 (= r5295)?
I'm very sorry, this was my misunderstanding. r1003238 was fix of
EOL style on a property name printing, not for a value.
> Yasuhito FUTATSUKI wrote on Fri, 25 Sep 2020 17:05 +0900:
>> On 2020/09/25 16:06, Daniel Shahaf wrote:
>>> futatuki_at_apache.org wrote on Thu, 24 Sep 2020 17:06 -0000:
>>>> Author: futatuki
>>>> Date: Thu Sep 24 17:06:39 2020
>>>> New Revision: 1881985
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1881985&view=rev
>>>> Log:
>>>> Follow up to r1880192: Fix an EOL issue in test on Windows.
>>>>
>>>> * subversion/tests/cmdline/merge-tests.py
>>>> (merge_deleted_folder_with_mergeinfo_2): Use os.linesep instead of '\n'
>>>> in expected values of svn:mergeinfo.
>>>>
>>>> Modified:
>>>> subversion/trunk/subversion/tests/cmdline/merge_tests.py
>>>>
>>>> Modified: subversion/trunk/subversion/tests/cmdline/merge_tests.py
>>>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/merge_tests.py?rev=1881985&r1=1881984&r2=1881985&view=diff
>>>> ==============================================================================
>>>> --- subversion/trunk/subversion/tests/cmdline/merge_tests.py (original)
>>>> +++ subversion/trunk/subversion/tests/cmdline/merge_tests.py Thu Sep 24 17:06:39 2020
>>>> @@ -18639,11 +18639,19 @@ def merge_deleted_folder_with_mergeinfo_
>>>> )
>>>>
>>>> # verify that mergeinfo is set/changed on A/D, A/D/G, A/D/G2.
>>>> +
>>>> + # NOTE: When writing out multi-line prop values in svn:* props, the
>>>> + # client converts to local encoding and local eol style.
>>>> + # Therefore, the expected output must contain the right kind of eoln
>>>> + # strings. That's why we use os.linesep in the tests below, not just
>>>> + # plain '\n'. The _last_ \n is also from the client, but it's not
>>>> + # part of the prop value and it doesn't get converted in the pipe.
>>
>> Before answer the questions, the comment above was brougt from
^^^^^^ brought
>> prop_value_conversion() in prop_tests.py.
>>
>
> *nod*
>
>>> I'm confused.
>>>
>>> First, if the client applies any transformation at all to property
>>> values, that'd be a bug. Property values are opaque octet sequences,
>>> just like file contents, so they must be emitted verbatim. It so
>>> happens that values of svn:* properties are UTF-8 with LF line endings,
>>> so that's what Python should see, regardless of the local encoding and
>>> EOL style.
>>
>> I judged that EOL conversion of property values on 'svn pg' isn't bug
>> because this comment is found in test for it.
>
> If you mean prop_value_conversion(), that test isn't about EOL style
> and encodings at all. It's about normalization of the values of some
> svn:* properties, such as this:
> .
> % svn ps -q svn:executable yes iota
> % svn pg svn:executable iota
> *
> %
>
> The test for binary property values is prop_tests.py:binary_props(). It
> doesn't cover newlines.
>
>> If it is a bug, prop_tests.py also does not test correctly.
>
> I've looked further and I think it's working as designed, but the
> design is rather unintuitive.
>
> In the data model, property values are binary data. That's why, in
> the API, property hashes' value type is svn_string_t. However, the
> cmdline client doesn't print all properties as binary data; it prints
> svn:* properties as text, transcoded and EOL-converted, even as it
> prints other properties in binary:
> .
> https://svn.apache.org/viewvc/subversion/tags/1.0.0/subversion/clients/cmdline/props.c?view=markup#l50
>
> The code still exists in trunk_at_HEAD, in propget-cmd.c.
>
> These semantics mean prop_tests.py and merge_tests.py are both correct
> to expect os.linesep in the output.
>
I confirmed it the code on trunk. Thank you. In propget-cmd.c,
svn_cl__propget() calls print_single_prop() through print_properties()
for non-xml output, and print_single_prop() calls
svn_cmdline__print_prop_hash() for single value for "svn pg -v" or
calls svn_subst_detranslate_string() for "svn:*" props before output
it to the out stream.
I'll fix the comment in merge_tests.py. As the issue on the comment
in prop_tests.py is not related to the change on r1881985 directly,
I won't fix it simultanously.
I think the rest of your mail needs farther discussion, so I'll
try to reply later, with changing Subject: header.
Cheers,
--
Yasuhito FUTATSUKI <futatuki_at_yf.bsclub.org>
Received on 2020-09-27 22:48:23 CEST