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: >>> futat...@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@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 <futat...@yf.bsclub.org>
