On 29.06.2018 10:45, Johan Corveleyn wrote: > On Fri, Jun 29, 2018 at 10:34 AM, Branko Čibej <br...@apache.org> wrote: >> On 28.06.2018 22:08, Julian Foad wrote: >>> Philip Martin wrote: >>>> Julian Foad <julianf...@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
