Re: SVN patch issue with property changes

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 ..."

-- 
Johan