On 29.06.2018 10:45, Johan Corveleyn wrote:
> On Fri, Jun 29, 2018 at 10:34 AM, Branko Čibej <brane_at_apache.org> wrote:
>> On 28.06.2018 22:08, Julian Foad wrote:
>>> Philip Martin wrote:
>>>> Julian Foad <julianfoad_at_apache.org> writes:
>>>>> Julian Foad wrote:
>>>>>> The bug seems to be that 'svn patch' fails to apply any patch of
>>>>>> this form, that tries to change a property value from empty to
>>>>>> non-empty.
>>>>> I committed a test for this in http://svn.apache.org/r1834628
>>>> I'm confused, you are treating as '' special? Suppose an existing
>>>> property P has value 'foo'. Should a patch that adds P with value 'bar'
>>> The issue is about a patch that *changes* the current value to another value, not a patch that *adds* a property.
>>>
>>> I am pointing out that a patch that changes the empty value '' to 'bar' should be (and isn't being) applied successfully to a property that already has the empty value ''.
>>>
>>> The patch format for modifying an empty value is mostly the same as the patch format for adding a new property:
>>>
>>> [[[
>>> Property changes on: f
>>> ___________________________________________________________________
>>> Added: empty
>>> ## -0,0 +1 ##
>>> +foo
>>> \ No newline at end of property
>>> ]]]
>>> vs.
>>> [[[
>>> Property changes on: f
>>> ___________________________________________________________________
>>> Modified: empty
>>> ## -0,0 +1 ##
>>> +foo
>>> \ No newline at end of property
>>> ]]]
>>>
>>> It's similar to the ambiguous patch representation of create a file or add text to an empty file. In that case, 'svn patch' first creates the file if it doesn't exist, and in both scenarios adds the patch text.
>>>
>>> For a property patch, a property-patch header line specifies whether it's a modify or an add, so there is no ambiguity. If the patch asks to 'modify' from empty to non-empty, when the target property already exists and has an empty value, this should certainly succeed.
>> However, when libsvn_client creates the svn:executable property, it
>> *always* sets its value to *. Applying a patch from empty to something
>> else will then very likely result in a conflict; because the
>> svn:executable property value cannot be empty (unless someone used a
>> broken client). This has been true since at least version 0.14., some 15
>> years ago.
> I think the cvs2svn conversion tool makes it possible to have an empty
> svn:executable property. I don't remember if it does this by default,
> or you need to do something special. I just recall we had this with an
> early attempt at converting years ago (we ended up throwing that
> version away, and replaced it with *).
>
> I remember thinking at the time: "Why do we need to set it to *? The
> docs only say the property needs to be set, but it doesn't require it
> to be *, it can be _any value_ ... it's not because the default svn
> client does so that it's required ..."
It's not "required" until you begin playing around with 'svn patch',
which did not exist at the time. :) libsvn_client will surely do the
right thing as soon as it sees the property, setting the executable bit
in the WC. But 'svn patch', if it's a bit naïve, may get things wrong.
Which would be a bug in 'svn patch' because it should treat special
properties just as specially as the rest of libsvn_client does. For
example, we have properties where we verify that the contents are utf-8
text with \n line endings; it shouldn't be possible for 'svn patch' to
break those constraints. svn:executable just happens to have a different
constraint.
-- Brane
Received on 2018-06-29 10:58:58 CEST