You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@subversion.apache.org by Johan Corveleyn <jc...@gmail.com> on 2016/10/17 08:18:35 UTC

Re: svn:auto-props: property specified in a parent dir but not specified in a subdir

[ dropped users@, added Paul Burba and Julian Foad, who have worked on
the inherited-props / repos-dictated-config features in the past, and
might have an educated opinion on this :-) ]

On Mon, Oct 17, 2016 at 1:45 AM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
>
> [ moving to dev@; please reply to dev@ only ]
>
> Johan Corveleyn wrote on Sat, Oct 15, 2016 at 22:59:28 +0200:
> > C:\autoprops\wc\trunk\dir>svn pg svn:auto-props --show-inherited-props
> > C:\autoprops\wc - *.txt = svn:eol-style=native
> >
> > C:\autoprops\wc\trunk - *.txt = svn:mime-type=application/octet-stream
> >
> >
> > C:\autoprops\wc\trunk\dir>echo test>test.txt
> >
> > C:\autoprops\wc\trunk\dir>svn add test.txt
> > svn: E200009: Can't set 'svn:eol-style': file
> > 'C:\autoprops\wc\trunk\dir\test.txt' has binary mime type property
>
> Let's take the the special interaction of svn:eol-tyle and non-texty
> svn:mime-type out of the picture; I'll reply on the assumption that the
> parent dir had «*.txt = k1=v1» and the subdir had «*.txt = k2=v2».
>
> I'm not entirely sure what's the expected behaviour here; that is:
> whether the k1 property being present in the overridden *.txt entry but
> absent from the overriding *.txt entry should mean (a) that the k1
> setting from the parent (overridden) entry is to be applied, or (b) that
> k1 property is not to be auto-set.
>
> You referenced the 1.8 release notes¹, which reference the wiki design doc,
> which uses the term "conflicts" without defining it.  Interpretation (a)
> makes sense if patterns "conflict" when they define different values for
> any one property; interpretation (b) makes sense if patterns "conflict"
> when they define different unordered lists of (propname, propvalue) pairs.
>
> autoprops_tests.py does not enlighten the ambiguity.
>
> Cheers,
>
> Daniel
>
> ¹ https://subversion.apache.org/docs/release-notes/1.8.html#repos-dictated-config

In the "Auto-Props Hierarchy and Precedence" section of the wiki doc
[1] which talks about conflicts and overriding, there is an example of
overriding the svn:eol-style property with another value (for the same
*.c pattern). But indeed no example about overriding the entire
k1=v1;k2=v2;... set for a specific file pattern. So this type of
"overriding" (are different k=v's merged together, or does the entire
set replace the other?) seems to be unspecified.

In case of the OP's problem reported on users@ (see example above),
it's the "replace entire k=v-set behavior" that would be needed to
make it work.

FWIW, it seems also impossible to "unset" svn:eol-style by giving it
an empty value, so that doesn't help (the error is different though
... it seems svn:eol-style does not accept an empty value):

[[[
C:\autoprops\wc\trunk\dir>svn pg svn:auto-props --show-inherited-props
C:\autoprops\wc - *.txt = svn:eol-style=native

C:\autoprops\wc\trunk - *.txt =
svn:eol-style=;svn:mime-type=application/octet-stream


C:\autoprops\wc\trunk\dir>echo test>test.txt

C:\autoprops\wc\trunk\dir>svn add test.txt
svn: E135001: Unrecognized line ending style '' for
'C:\autoprops\wc\trunk\dir\test.txt'
]]]


Omitting the '=' also doesn't cut it:

[[[
C:\autoprops\wc\trunk\dir>svn pg svn:auto-props --show-inherited-props
C:\autoprops\wc - *.txt = svn:eol-style=native

C:\autoprops\wc\trunk - *.txt =
svn:eol-style;svn:mime-type=application/octet-stream

C:\autoprops\wc\trunk\dir>svn add test.txt
svn: E135001: Unrecognized line ending style '' for
'C:\autoprops\wc\trunk\dir\test.txt'
]]]

[1] http://wiki.apache.org/subversion/Inheritable-Ignores-AutoProps#Auto-Props_Hierarchy_and_Precedence

-- 
Johan

Re: svn:auto-props: property specified in a parent dir but not specified in a subdir

Posted by Stefan Hett <st...@egosoft.com>.

On 10/18/2016 12:06 PM, Branko \u010cibej wrote:
> On 18.10.2016 09:32, Stefan wrote:
>> On 10/17/2016 15:37, Branko \u010cibej wrote:
>>> On 17.10.2016 14:12, Daniel Shahaf wrote:
>>>> Johan Corveleyn wrote on Mon, Oct 17, 2016 at 10:18:35 +0200:
>>>>> Omitting the '=' also doesn't cut it:
>>>>>
>>>>> [[[
>>>>> C:\autoprops\wc\trunk\dir>svn pg svn:auto-props --show-inherited-props
>>>>> C:\autoprops\wc - *.txt = svn:eol-style=native
>>>>>
>>>>> C:\autoprops\wc\trunk - *.txt =
>>>>> svn:eol-style;svn:mime-type=application/octet-stream
>>>>>
>>>>> C:\autoprops\wc\trunk\dir>svn add test.txt
>>>>> svn: E135001: Unrecognized line ending style '' for
>>>>> 'C:\autoprops\wc\trunk\dir\test.txt'
>>>>> ]]]
>>>> Quoting ~/.subversion/config:
>>>>
>>>>      [auto-props]
>>>>      ### The format of the entries is:
>>>>      ###   file-name-pattern = propname[=value][;propname[=value]...]
>>>>
>>>> So the �=value� part is allowed to be omitted entirely, but what are the
>>>> semantics of that?  They don't seem to be documented.
>>>>
>>>> It seems to me that we can *define* that if the = sign is absent, then
>>>> the value of the property will be not "" but NULL.
>>> Unfortunately the implementation
>>> (libsvn_client/add.c:all_auto_props_collector()) already explicitly
>>> handles 'propname' as if it were 'propname=', i.e., sets the value to
>>> "". Since the "documentation" in the generated config file already
>>> implies that the '=' can be absend, I'd say this behaviour is pretty
>>> much set in stone.
>>>
>>>
>>> -- Brane
>>  From a user's point of view I'm also not sure whether a syntax of *.foo
>> = k1 would be intuitive to read as if that's unsetting the property...
>> Personally I wouldn't get that behavior tbh, without reading the
>> documentation.
> That's a good point. Also from a user's perspective, I'd find it
> extremely confusing that, having seen an svn:auto-props value containing:
>
>      *.txt = svn:eol-style=native
>
> and then 'svn add'ing foo.txt, to find that the newly added file has,
> e.g., svn:keywords set because some auto-props value higher up in the
> tree happens to specify it for the '*.txt' pattern.
>
> If we change the behaviour so that a rule for a (case-insensitive)
> pattern completely overrides a hierarchically previoius rule, we can
> also introduce the concept of disabling auto-props rules; e.g., just
> having a
>
>      *.txt =
>
> could mean "do nothing with files matching '*.txt' from this point
> onwards." I think that would be a quite valuable addition to the
> auto-props feature set.
>
> -- Brane
Maybe it's possible to extend the grammar to provide a way to include 
inherited settings without breaking backwards compatibility?
Aka something like:
:inherit:;*.txt = svn:eol-style=native

-- 
Regards,
Stefan Hett

Re: svn:auto-props: property specified in a parent dir but not specified in a subdir

Posted by Stefan Hett <st...@egosoft.com>.

On 10/18/2016 12:29 PM, Daniel Shahaf wrote:
> Branko \u010cibej wrote on Tue, Oct 18, 2016 at 12:06:32 +0200:
>> On 18.10.2016 09:32, Stefan wrote:
>>> On 10/17/2016 15:37, Branko \u010cibej wrote:
>>>> On 17.10.2016 14:12, Daniel Shahaf wrote:
>>>>> Johan Corveleyn wrote on Mon, Oct 17, 2016 at 10:18:35 +0200:
>>>>>> Omitting the '=' also doesn't cut it:
>>>>>>
>>>>>> [[[
>>>>>> C:\autoprops\wc\trunk\dir>svn pg svn:auto-props --show-inherited-props
>>>>>> C:\autoprops\wc - *.txt = svn:eol-style=native
>>>>>>
>>>>>> C:\autoprops\wc\trunk - *.txt =
>>>>>> svn:eol-style;svn:mime-type=application/octet-stream
>>>>>>
>>>>>> C:\autoprops\wc\trunk\dir>svn add test.txt
>>>>>> svn: E135001: Unrecognized line ending style '' for
>>>>>> 'C:\autoprops\wc\trunk\dir\test.txt'
>>>>>> ]]]
>>>>> Quoting ~/.subversion/config:
>>>>>
>>>>>      [auto-props]
>>>>>      ### The format of the entries is:
>>>>>      ###   file-name-pattern = propname[=value][;propname[=value]...]
>>>>>
>>>>> So the �=value� part is allowed to be omitted entirely, but what are the
>>>>> semantics of that?  They don't seem to be documented.
>>>>>
>>>>> It seems to me that we can *define* that if the = sign is absent, then
>>>>> the value of the property will be not "" but NULL.
>>>> Unfortunately the implementation
>>>> (libsvn_client/add.c:all_auto_props_collector()) already explicitly
>>>> handles 'propname' as if it were 'propname=', i.e., sets the value to
>>>> "". Since the "documentation" in the generated config file already
>>>> implies that the '=' can be absend, I'd say this behaviour is pretty
>>>> much set in stone.
>>>>
>>>>
>>>> -- Brane
>>>  From a user's point of view I'm also not sure whether a syntax of *.foo
>>> = k1 would be intuitive to read as if that's unsetting the property...
>>> Personally I wouldn't get that behavior tbh, without reading the
>>> documentation.
>> That's a good point. Also from a user's perspective, I'd find it
>> extremely confusing that, having seen an svn:auto-props value containing:
>>
>>      *.txt = svn:eol-style=native
>>
>> and then 'svn add'ing foo.txt, to find that the newly added file has,
>> e.g., svn:keywords set because some auto-props value higher up in the
>> tree happens to specify it for the '*.txt' pattern.
>>
> I can easily imagine other scenarios in which the overlaying behaviour
> is not confusing, but expected; for example:
>
> % svn propset svn:auto-props '*.txt = svn:mime=type=text/plain;;charset=UTF-8' Documentation # sic
> % svn propset svn:auto-props '*.txt = svn:eol-style=LF'                        Documentation/unix
> % svn propset svn:auto-props '*.txt = svn:eol-style=CRLF'                      Documentation/windows
>
> The output of 'svn help ps' may propagate this expectation, too:
> .
>      svn:auto-props     -
>        \u22ee
>        Applies recursively to all files added or imported under the directory
>        it is set on.
> .
> (The term "recursively" is not qualified or restricted.)
>
>> If we change the behaviour so that a rule for a (case-insensitive)
>> pattern completely overrides a hierarchically previoius rule, we can
>> also introduce the concept of disabling auto-props rules; e.g., just
>> having a
>>
>>      *.txt =
>>
>> could mean "do nothing with files matching '*.txt' from this point
>> onwards." I think that would be a quite valuable addition to the
>> auto-props feature set.
> This is a very compelling argument :-)  However, I note that it, too, is
> backwards incompatible.
>
> I was thinking of inventing new syntaxes, �*.txt := PROPHASH� for
> override (as you suggest) and �*.txt += PROPHASH� for overlay (as now).
> Released libsvn's interpret these syntaxes as the shell pattern "files
> whose name ends with the 6-character substring '.txt +'".
>
> Cheers,
>
> Daniel
>
FWIW: I favor your approach/syntax to define overrides/additions to 
properties over the syntax I brought up in my other mail.

-- 
Regards,
Stefan Hett

Re: svn:auto-props: property specified in a parent dir but not specified in a subdir

Posted by Martin Furter <mf...@apache.org>.


On 2016-10-18 17:01 (-0400), Johan Corveleyn <jc...@gmail.com> wrote: 
> On Tue, Oct 18, 2016 at 1:35 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > Branko \u010cibej wrote on Tue, Oct 18, 2016 at 13:08:37 +0200:
> >> It is. I don't think there's any way we can /not/ make it backwards
> >> incompatible, given the current implementation. Which is why I think we
> >> should just admit that current behaviour is a bug.
> >>
> >> Either that, or introduce a new property that behaves sensibly.
> >
> > Some IRC discussion lead us to svn_prop_name_is_valid(), which requires
> > that the first character of a property name be alphabetic or colon or
> > underscore.
> >
> > Brane therefore proposes «*.txt == propname=propval» for the "override"
> > syntax, and keep «*.txt = propname=propval» for the "overlay" syntax.
> >
> > This change would be backwards compatible: this syntax does not have
> > a meaning currently.  1.9 clients just ignore it.
> >
> > We can bikeshed about finding a syntax that looks less like
> > a C comparison, of course.  We can't use =: because property names can
> > start with a colon, we can't use := because filename patterns can end
> > with a colon, but any other option is open.
> 
> Great idea. And I like that color of bikeshed :-).
> 
> Another option would be to introduce a "subtract" syntax, with =- or
> =!, but I don't know if that would be easier / better for the user (in
> that case, he'd have to specify the *.txt pattern two times, to
> subtract svn:eol-style with =!, and add svn:mime-type with =).

All these variants seem to suffer the same problem, old clients will set the properties after the first semicolon:

*.txt == prop1 ; prop2

There is a way to prevent that, filenames do not contain slashes:

*.txt /= prop1 ; prop2

This one passed my tests with an "old" client.

- Martin

Re: svn:auto-props: property specified in a parent dir but not specified in a subdir

Posted by Johan Corveleyn <jc...@gmail.com>.

On Tue, Oct 18, 2016 at 1:35 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> Branko Čibej wrote on Tue, Oct 18, 2016 at 13:08:37 +0200:
>> It is. I don't think there's any way we can /not/ make it backwards
>> incompatible, given the current implementation. Which is why I think we
>> should just admit that current behaviour is a bug.
>>
>> Either that, or introduce a new property that behaves sensibly.
>
> Some IRC discussion lead us to svn_prop_name_is_valid(), which requires
> that the first character of a property name be alphabetic or colon or
> underscore.
>
> Brane therefore proposes «*.txt == propname=propval» for the "override"
> syntax, and keep «*.txt = propname=propval» for the "overlay" syntax.
>
> This change would be backwards compatible: this syntax does not have
> a meaning currently.  1.9 clients just ignore it.
>
> We can bikeshed about finding a syntax that looks less like
> a C comparison, of course.  We can't use =: because property names can
> start with a colon, we can't use := because filename patterns can end
> with a colon, but any other option is open.

Great idea. And I like that color of bikeshed :-).

Another option would be to introduce a "subtract" syntax, with =- or
=!, but I don't know if that would be easier / better for the user (in
that case, he'd have to specify the *.txt pattern two times, to
subtract svn:eol-style with =!, and add svn:mime-type with =).

-- 
Johan