You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2012/11/19 03:50:46 UTC
[RFC] svn propset should require 'force' to set unknown svn: propnames
'svn propset' lets us create any property name 'svn:foo', with good reason: we want old clients to be able to set and edit properties that only the newer clients know about. However, there is a pitfall for unwary users: it's east to mis-spell a prop name and not notice. For example:
svn:ignores
svn:global-ignore
are both mis-spellings.
It would seem reasonable to require '--force' to set an unknown 'svn:' property name. Note that we already require '--force' to set a known svn:something property to an invalid value.
Proposal:
For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
svn pset svn:foo
svn pedit svn:foo
and all other subcommands where properties are handled would continue to work as normal, no '--force' needed, including:
svn pdel svn:foo
svn pget svn:foo
svn plist
svn patch, merge, diff, checkout, update, switch, ...
Rationale notes:
'propset' would bail out even if the property already exists. One reason for this is so that multi-target or 'recursive' mode doesn't have to do anything special if the property exists on only some of the targets.
'pset' is the only one that really needs 'force' to meet the needs of this proposal. If a property already exists then it's probably best to assume that it exists on purpose, so we could argue that there is little harm in allowing 'pedit' and 'pdel'. Currently, 'pset' and 'pedit' require '--force' to set an invalid *value* for a known property (such as 'svn pset svn:eol-style x'); pdel doesn't take a '--force' option and intentionally allows properties with invalid values to be deleted. An advantage in discouraging 'svn pedit svn:foo' is that it can't validate the value if it doesn't know anything about 'svn:foo'. For these reasons and for consistency with the value-checking, the proposal has both 'pset' and 'pedit' validate the name, but not 'pdel'.
Thoughts, objections?
- Julian
--
Subversion Live 2012 - 2 Days: training, networking, demos & more! http://bit.ly/NgUaRi
Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 24.11.2012 15:20, Daniel Shahaf wrote:
> Branko Čibej wrote on Sat, Nov 24, 2012 at 13:38:39 +0100:
>> On 24.11.2012 13:30, Daniel Shahaf wrote:
>>> Branko Čibej wrote on Sat, Nov 24, 2012 at 12:33:01 +0100:
>>>> I'm also considering requiring --force if one tries to use a revision
>>>> property name as a node property, and vice versa.
>>>>
>>> +0
>>>
>>>> $ svn ps svn:barfoo x .
>>>> svn: E195011: 'svn:barfoo' is not a valid svn: property name; did you mean 'svn:group'?
>>>> (To set the 'svn:barfoo' property, re-run with '--force'.)
>>>>
>>>> In this case the suggestion is clearly bogus.
>>> It seems the code should filter svn:user and svn:group from
>>> SVN_PROP_ALL_NODE_PROPS?
>> That wouldn't make suggestion any less silly. :)
>>
> That would make the suggestion of 'svn:group' not happen at all.
>
>> In fact, with the change I made just now (to not consider the prefix
>> when sorting the properties by similarity), trying to set "svn:barfoo"
>> results in the suggestion to use "svn:mergeinfo" instead, which is just
>> as wrong.
>>
> Maybe it shouldn't suggest anything? IIRC the code just does a qsort()
> and suggests the first option --- it doesn't check that the similarity
> score of that option exceeds some minimum threshold.
Which is exactly what I'm working on right now.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Sat, Nov 24, 2012 at 13:38:39 +0100:
> On 24.11.2012 13:30, Daniel Shahaf wrote:
> > Branko Čibej wrote on Sat, Nov 24, 2012 at 12:33:01 +0100:
> >> I'm also considering requiring --force if one tries to use a revision
> >> property name as a node property, and vice versa.
> >>
> > +0
> >
> >> $ svn ps svn:barfoo x .
> >> svn: E195011: 'svn:barfoo' is not a valid svn: property name; did you mean 'svn:group'?
> >> (To set the 'svn:barfoo' property, re-run with '--force'.)
> >>
> >> In this case the suggestion is clearly bogus.
> > It seems the code should filter svn:user and svn:group from
> > SVN_PROP_ALL_NODE_PROPS?
>
> That wouldn't make suggestion any less silly. :)
>
That would make the suggestion of 'svn:group' not happen at all.
> In fact, with the change I made just now (to not consider the prefix
> when sorting the properties by similarity), trying to set "svn:barfoo"
> results in the suggestion to use "svn:mergeinfo" instead, which is just
> as wrong.
>
Maybe it shouldn't suggest anything? IIRC the code just does a qsort()
and suggests the first option --- it doesn't check that the similarity
score of that option exceeds some minimum threshold.
> Regarding svn:group, svn:user and svn:unix-mode, these are defined in
> svn_props.h as reserved, user-visible properties and I see no reason to
> require --force to set them on /this/ level. This is purely a syntactic
> filter; it would be wrong IMO to infuse any kind of awareness of
> semantics into it.
>
I could go either way about whether it should ever suggest
'svn:mergeinfo'.
> -- Brane
>
> --
> Branko Čibej
> Director of Subversion | WANdisco | www.wandisco.com
>
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 24.11.2012 13:30, Daniel Shahaf wrote:
> Branko Čibej wrote on Sat, Nov 24, 2012 at 12:33:01 +0100:
>> I'm also considering requiring --force if one tries to use a revision
>> property name as a node property, and vice versa.
>>
> +0
>
>> $ svn ps svn:barfoo x .
>> svn: E195011: 'svn:barfoo' is not a valid svn: property name; did you mean 'svn:group'?
>> (To set the 'svn:barfoo' property, re-run with '--force'.)
>>
>> In this case the suggestion is clearly bogus.
> It seems the code should filter svn:user and svn:group from
> SVN_PROP_ALL_NODE_PROPS?
That wouldn't make suggestion any less silly. :)
In fact, with the change I made just now (to not consider the prefix
when sorting the properties by similarity), trying to set "svn:barfoo"
results in the suggestion to use "svn:mergeinfo" instead, which is just
as wrong.
Regarding svn:group, svn:user and svn:unix-mode, these are defined in
svn_props.h as reserved, user-visible properties and I see no reason to
require --force to set them on /this/ level. This is purely a syntactic
filter; it would be wrong IMO to infuse any kind of awareness of
semantics into it.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Sat, Nov 24, 2012 at 12:33:01 +0100:
> I'm also considering requiring --force if one tries to use a revision
> property name as a node property, and vice versa.
>
+0
> $ svn ps svn:barfoo x .
> svn: E195011: 'svn:barfoo' is not a valid svn: property name; did you mean 'svn:group'?
> (To set the 'svn:barfoo' property, re-run with '--force'.)
>
> In this case the suggestion is clearly bogus.
It seems the code should filter svn:user and svn:group from
SVN_PROP_ALL_NODE_PROPS?
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Peter Samuelson <pe...@p12n.org>.
[Peter Samuelson]
> If the "misspelled" property exists, probably the user already noticed
> the typo, that's why they're deleting it! I see no need for --force
> here.
>
> If the "misspelled" property does not exist, I think the warning that
> the property does not exist is just as good as a warning that they
> misspelled it or forgot to use --force.
There is an additional case. If you _expect_ some warnings that a
property does not exist (which you should expect a lot of, if you use
'svn propdel -R'), and indeed the property does not exist, perhaps it
would be helpful to alert the user of a possible misspelling.
But that's still not a --force flag, just an additional warning line
after "Attempting to delete nonexistent property 'svm:eol-style' on...."
But in the -R case, those "nonexistent property" lines will overwhelm
the output, so if we wanted to do the additional spell-check warning,
we'd have to make sure it appears last, and probably only if the
propdel was not able to delete anything. (I.e., not only if there was
at least one "nonexistent property" warning.)
Peter
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
Ben Reser wrote:
> On Mon, Nov 26, 2012 at 3:38 PM, Peter Samuelson <pe...@p12n.org> wrote:
>> I don't get why we would want 'propdel --force' at all. Either the
>> "misspelled" property exists, or it doesn't.
>>
>> If the "misspelled" property exists, probably the user already
>> noticed the typo, that's why they're deleting it! I see no need for
>> --force here.
>>
>> If the "misspelled" property does not exist, I think the warning that
>> the property does not exist is just as good as a warning that they
>> misspelled it or forgot to use --force.
>
> +1
Hmm, that does sound sane when you put it like that. I dunno, I can't think straight. If it makes sense to two or three of you, sure, go for it.
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Ben Reser <be...@reser.org>.
On Mon, Nov 26, 2012 at 3:38 PM, Peter Samuelson <pe...@p12n.org> wrote:
> I don't get why we would want 'propdel --force' at all. Either the
> "misspelled" property exists, or it doesn't.
>
> If the "misspelled" property exists, probably the user already noticed
> the typo, that's why they're deleting it! I see no need for --force
> here.
>
> If the "misspelled" property does not exist, I think the warning that
> the property does not exist is just as good as a warning that they
> misspelled it or forgot to use --force.
+1
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/26/2012 06:38 PM, Peter Samuelson wrote:
>
> [Julian Foad]
>> In general, setting *or* deleting an unknown prop name causes nothing
>> significant to happen on *this* client, and may cause something
>> (expected or unexpected) to happen on another client.
>
> I don't get why we would want 'propdel --force' at all. Either the
> "misspelled" property exists, or it doesn't.
>
> If the "misspelled" property exists, probably the user already noticed
> the typo, that's why they're deleting it! I see no need for --force
> here.
>
> If the "misspelled" property does not exist, I think the warning that
> the property does not exist is just as good as a warning that they
> misspelled it or forgot to use --force.
+1. (And thanks, Peter, for effectively composing my response for me.)
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Peter Samuelson <pe...@p12n.org>.
[Julian Foad]
> In general, setting *or* deleting an unknown prop name causes nothing
> significant to happen on *this* client, and may cause something
> (expected or unexpected) to happen on another client.
I don't get why we would want 'propdel --force' at all. Either the
"misspelled" property exists, or it doesn't.
If the "misspelled" property exists, probably the user already noticed
the typo, that's why they're deleting it! I see no need for --force
here.
If the "misspelled" property does not exist, I think the warning that
the property does not exist is just as good as a warning that they
misspelled it or forgot to use --force.
Peter
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 26.11.2012 21:33, Julian Foad wrote:
> My conclusion is flagging 'propdel' is less important than flagging
> 'propset' but still useful, but not enough to make a fuss about.
> Oops, too late, I'va already made a fuss about it.
Heh. Actually, adding the check to propdel takes all of two lines of
code, so we may as well do it and see if anyone complains too loudly.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> On 25.11.2012 23:30, Julian Foad wrote:
>> To answer the question I think you meant: requiring force for
>> setting svn:mergeinfo is a separate issue and shouldn't necessarily
>> work the same way or produce the same error message as the unknown
>> propname check. Personally, I have not been feeling that we need
>> to do it, though I am willing to be persuaded otherwise. If we do,
>> we would probably want to include 'propdel' as well as propset and
>> propedit.
>
> Well, svn:mergeinfo is special in the sense that changing it can
> cause wrong results later on. None of the others have as far-reaching
> consequences.
>
> So what I'm really asking is if we can risk ever suggesting
> "svn:mergeinfo" as the correct spelling of a mistyped property name.
Certainly we can and should suggest 'svn:mergeinfo' if the user typed 'svn:mergefino' or 'svn:ergeinfo' or 'svn:merginfo' or 'svn:mergeinfoo' or 'snv:mergeinfo'. Zero risk.
> Which I agree is orthogonal to the question of whether we should
> always require --force for anything pertaining to svn:mergeinfo,
> but is relevant to my original question in the sense that we may
> not want it to be a "user-visible and -settable versioned node
> property".
>> That last point makes me wonder: should this 'unknown prop name'
>> check also apply to 'propdel'? My proposal argued not, but now I'm
>> reconsidering. My arguments were (1) We intentionally don't check
>> or care if deleting a prop with a bad value, and (2) we don't have
>> a '--force' option on 'propdel' already. But (1) is not really
>> analogous to deleting a prop with an unrecognized name, and (2) is
>> weak. Now I'm leaning towards making 'propdel' consistent with
>> 'propset', because removing a property such as mime-type or
>> eol-style is semantically quite similar to adding or changing the
>> property. I imagine more annoyance results from mis-spelling a
>> propdel propname than would result from having to specify '--force'
>> to delete some svn: propname that the client doesn't know about.
>
> Possibly. Setting an unknown prop name causes nothing to happen
> silently; whereas deleting it may cause something unexpected to happen,
> also silently.
I don't understand why you say that.
In general, setting *or* deleting an unknown prop name causes nothing significant to happen on *this* client, and may cause something (expected or unexpected) to happen on another client. More specifically, we can talk about two classes of property names that are unknown to this client: those unknown to any client and those known to some other clients (such as a future 'svn' client or a non-svn client). The effect, if we don't catch it, can be summarized as:
Example This client Other client
------------------- ----------- ------------
pset svn:mergefino [1] { Fails to have the expected effect }
pdel svn:mergefino [1] { Error 'nonexistent...' [3], and
fails to have the expected effect }
pset svn:mergeinfo2 [2] No-op SOME EFFECT
pdel svn:mergeinfo2 [2] No-op SOME EFFECT
(and error?[3])
[1] Typo -- prop name unknown to any client.
[2] Prop name known to some other clients (let's pretend).
[3] Depends whether the prop already exists.
If the prop name is unknown to any client, it's almost certainly a typo and should be diagnosed as such, no matter whether setting or deleting. The prop almost certainly isn't already set; if it is then the user has probably set it by accident (or discovered that somebody else did) and is now running 'svn:propdel' to delete it. If we don't diagnose it, the inconvenience for the user is greater for 'propset' (where svn 1.7 proceeds without complaint) and less for 'propdel' (where svn 1.7 already complains 'Attempting to delete nonexistent property ...').
If the prop name is known to other clients, setting and deleting are equally significant as far as this client can tell: either of them can cause some effect on another client that is unexpected (or quite possibly expected and intended) by this user.
It's probably fair to assume that the 'typo -- unknown to any client' scenario is common and the 'known to other clients' scenario uncommon in practice.
My conclusion is flagging 'propdel' is less important than flagging 'propset' but still useful, but not enough to make a fuss about. Oops, too late, I'va already made a fuss about it.
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 25.11.2012 23:30, Julian Foad wrote:
> Branko Čibej wrote:
>
>> The latest change takes account of property name similarity. So for example,
>>
>> svn propset svn:foobar .
>>
>> will emit an error but will not suggest an alternative spelling, whereas
>>
>> svn propset svn:ignores .
>>
>> will suggest two, svn:ignore and svn:global-ignores.
>>
>> The only open question now, IMO, is whether I should remove
>> svn:mergeinfo from SVN_PROP_NODE_ALL_PROPS defined in
>> svn_prop.h. I'm leaning towards "yes" but would like to hear
>> opinions from the merge(info) experts.
> EDOMAIN: question containing symbol 'SVN_PROP_NODE_ALL_PROPS' is not formulated in the problem domain.
>
> /**
> * This is a list of all user-vixible and -settable versioned node properties.
> *
> * @since New in 1.8
> */
> #define SVN_PROP_NODE_ALL_PROPS SVN_PROP_MIME_TYPE, ...
>
> To answer the question I think you meant: requiring force for setting svn:mergeinfo is a separate issue and shouldn't necessarily work the same way or produce the same error message as the unknown propname check. Personally, I have not been feeling that we need to do it, though I am willing to be persuaded otherwise. If we do, we would probably want to include 'propdel' as well as propset and propedit.
Well, svn:mergeinfo is special in the sense that changing it can cause
wrong results later on. None of the others have as far-reaching
consequences.
So what I'm really asking is if we can risk ever suggesting
"svn:mergeinfo" as the correct spelling of a mistyped property name.
Which I agree is orthogonal to the question of whether we should always
require --force for anything pertaining to svn:mergeinfo, but is
relevant to my original question in the sense that we may not want it to
be a "user-visible and -settable versioned node property".
§
> That last point makes me wonder: should this 'unknown prop name' check also apply to 'propdel'? My proposal argued not, but now I'm reconsidering. My arguments were (1) We intentionally don't check or care if deleting a prop with a bad value, and (2) we don't have a '--force' option on 'propdel' already. But (1) is not really analogous to deleting a prop with an unrecognized name, and (2) is weak. Now I'm leaning towards making 'propdel' consistent with 'propset', because removing a property such as mime-type or eol-style is semantically quite similar to adding or changing the property. I imagine more annoyance results from mis-spelling a propdel propname than would result from having to specify '--force' to delete some svn: propname that the client doesn't know about.
Possibly. Setting an unknown prop name causes nothing to happen
silently; whereas deleting it may cause something unexpected to happen,
also silently.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> The latest change takes account of property name similarity. So for example,
>
> svn propset svn:foobar .
>
> will emit an error but will not suggest an alternative spelling, whereas
>
> svn propset svn:ignores .
>
> will suggest two, svn:ignore and svn:global-ignores.
>
> The only open question now, IMO, is whether I should remove
> svn:mergeinfo from SVN_PROP_NODE_ALL_PROPS defined in
> svn_prop.h. I'm leaning towards "yes" but would like to hear
> opinions from the merge(info) experts.
EDOMAIN: question containing symbol 'SVN_PROP_NODE_ALL_PROPS' is not formulated in the problem domain.
/**
* This is a list of all user-vixible and -settable versioned node properties.
*
* @since New in 1.8
*/
#define SVN_PROP_NODE_ALL_PROPS SVN_PROP_MIME_TYPE, ...
To answer the question I think you meant: requiring force for setting svn:mergeinfo is a separate issue and shouldn't necessarily work the same way or produce the same error message as the unknown propname check. Personally, I have not been feeling that we need to do it, though I am willing to be persuaded otherwise. If we do, we would probably want to include 'propdel' as well as propset and propedit.
That last point makes me wonder: should this 'unknown prop name' check also apply to 'propdel'? My proposal argued not, but now I'm reconsidering. My arguments were (1) We intentionally don't check or care if deleting a prop with a bad value, and (2) we don't have a '--force' option on 'propdel' already. But (1) is not really analogous to deleting a prop with an unrecognized name, and (2) is weak. Now I'm leaning towards making 'propdel' consistent with 'propset', because removing a property such as mime-type or eol-style is semantically quite similar to adding or changing the property. I imagine more annoyance results from mis-spelling a propdel propname than would result from having to specify '--force' to delete some svn: propname that the client doesn't know about.
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
The latest change takes account of property name similarity. So for example,
svn propset svn:foobar .
will emit an error but will not suggest an alternative spelling, whereas
svn propset svn:ignores .
will suggest two, svn:ignore and svn:global-ignores.
The only open question now, IMO, is whether I should remove
svn:mergeinfo from
SVN_PROP_NODE_ALL_PROPS defined in svn_prop.h. I'm leaning towards "yes"
but would like to hear opinions from the merge(info) experts.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 23.11.2012 18:09, Branko Čibej wrote:
> I agree that looking at the prefix is dicey, ...
... so I've tightened up the prefix matching so that --force is required
only when:
* the prefix is exactly 3 letters long (propname[3] == ':');
* and the first three letters differ in only one change from "svn"
o this means, one replacement (e.g., "gvn" or "svm" or Svn", etc.)
o or one transposition ("snv" or "nsv")
* and if the rest of the property name following the colon exactly
matches one of the known, user-visible property names.
If the prefix is exactly "svn:", then we'll require --force whenever the
property name is not known.
These rule will trip on
svm:executable
svn:exemutable
but not on
svm:exemutable
I actually think the last case is a bit unfortunate, and would consider
allowing a one-letter difference in the property name and the prefix; so
that the last example would be flagged, but this would not:
svm:ixemutable
I'm also considering requiring --force if one tries to use a revision
property name as a node property, and vice versa.
And I think it would be a good idea /not/ to suggest a different
spelling if the property name is too different from a know one.
Currently we'll get this:
$ svn ps svn:barfoo x .
svn: E195011: 'svn:barfoo' is not a valid svn: property name; did you mean 'svn:group'?
(To set the 'svn:barfoo' property, re-run with '--force'.)
In this case the suggestion is clearly bogus.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Nov 23, 2012 at 11:22:44AM -0800, Ben Reser wrote:
> On Fri, Nov 23, 2012 at 9:09 AM, Branko Čibej <br...@wandisco.com> wrote:
> > No. The check specifically looks for a three-letter prefix ending in a
> > colon, and allows only one wrong character or transposition. It will
> > flag svm: but not tsvn:.
>
> svk would pop up for anyone still using (no idea if anyone does) SVK.
FWIW, SVK is still in use:
http://marc.info/?l=openbsd-ports&m=134697969823088&w=2
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Ben Reser <be...@reser.org>.
On Fri, Nov 23, 2012 at 9:09 AM, Branko Čibej <br...@wandisco.com> wrote:
> No. The check specifically looks for a three-letter prefix ending in a
> colon, and allows only one wrong character or transposition. It will
> flag svm: but not tsvn:.
svk would pop up for anyone still using (no idea if anyone does) SVK.
> I agree that looking at the prefix is dicey, which is why I defined the
> constraints as explained above. I'm also looking at options for not
> flagging non-svn: properties that are different enough from the known
> ones; for example, these would be flagged:
>
> svm:needs-lock
> svn:global-ignore
> gvn:ignore
>
> but not
>
> gvn:defrobify
> gvn:local-ignores
>
>
> I know how to do this, but I'm not clear yet about how or where to draw
> the line.
Difficult decision here. On one hand we have something that's
probably going to help make a lot of peoples lives easier but it might
make a small minority's life harder.
I'm inclined to agree with Mark here that we shouldn't match on prefixes.
However, on the other hand, aren't most of these property names likely
only to being set by programs using our library and not the command
line client. As such they wouldn't be prone to this code since
Brane's changes are only in the command line client. In the svk
example I gave above, svk:merge is not really intended to be modified
by hand.
gvn might be the only exception to that which we've mentioned, I'm not
super familiar with it. Looking at the page it says it uses the svn
client for most operations and the python bindings for others.
It's also worth nothing that I think both gvn and svk are mostly
unused at this point. Are there other common examples out there that
would trip this?
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 23.11.2012 17:17, Mark Phippard wrote:
> Personally, I think checking for mis-spellings in "svn:" goes too far.
> For example, do these checks interfere with setting any of the tsvn:
> properties?
No. The check specifically looks for a three-letter prefix ending in a
colon, and allows only one wrong character or transposition. It will
flag svm: but not tsvn:.
> I would be very much against that as they are widely
> used. Your example shows blocking gvn:ignore. AFAIK, the gvn project
> is not being used since Google decided not to use SVN, but what if
> they had? Would we really want to require --force for properties
> named gvn:*?
Yes.
> http://code.google.com/p/gvn/
>
> It is difficult to say because I can see the value in what you are
> doing here. It just seems like it is walking us down a path that we
> typically try to stay away from.
The intent of the proposed change is to warn people about misspelling
svn: properties. Surely the prefix can be misspelt as well? Why should
svm:needs-lock be silently accepted, but not svn:global-ignore?
I agree that looking at the prefix is dicey, which is why I defined the
constraints as explained above. I'm also looking at options for not
flagging non-svn: properties that are different enough from the known
ones; for example, these would be flagged:
svm:needs-lock
svn:global-ignore
gvn:ignore
but not
gvn:defrobify
gvn:local-ignores
I know how to do this, but I'm not clear yet about how or where to draw
the line.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Mark Phippard <ma...@gmail.com>.
On Fri, Nov 23, 2012 at 5:43 AM, Branko Čibej <br...@wandisco.com> wrote:
> On 19.11.2012 03:50, Julian Foad wrote:
>> Proposal:
>>
>> For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
>>
>> svn pset svn:foo
>> svn pedit svn:foo
>>
>> and all other subcommands where properties are handled would continue to work as normal, no '--force' needed
>
> Here's what the prototype currently does. Any suggestions for better
> wording are most welcome.
>
> $ svn ps svm:ignore x .
> svn: E195011: 'svm:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
> (Use --force if you're sure about 'svm:ignore'.)
>
> $ svn ps gvm:ignore x .
> property 'gvm:ignore' set on '.'
>
> $ svn ps gvn:ignore x .
> svn: E195011: 'gvn:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
> (Use --force if you're sure about 'gvn:ignore'.)
>
> $ svn ps svn:ignores x .
> svn: E195011: 'svn:ignores' is not a valid svn: property name; did you mean 'svn:ignore'?
> (Use --force if you're sure about 'svn:ignores'.)
>
> $ svn ps svn:local-ignore x .
> svn: E195011: 'svn:local-ignore' is not a valid svn: property name; did you mean 'svn:global-ignores'?
> (Use --force if you're sure about 'svn:local-ignore'.)
Personally, I think checking for mis-spellings in "svn:" goes too far.
For example, do these checks interfere with setting any of the tsvn:
properties? I would be very much against that as they are widely
used. Your example shows blocking gvn:ignore. AFAIK, the gvn project
is not being used since Google decided not to use SVN, but what if
they had? Would we really want to require --force for properties
named gvn:*?
http://code.google.com/p/gvn/
It is difficult to say because I can see the value in what you are
doing here. It just seems like it is walking us down a path that we
typically try to stay away from.
--
Thanks
Mark Phippard
http://markphip.blogspot.com/
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 23.11.2012 16:31, Daniel Shahaf wrote:
> Branko Čibej wrote on Fri, Nov 23, 2012 at 15:59:16 +0100:
>> On 23.11.2012 15:35, Julian Foad wrote:
>>> In file included from subversion/libsvn_delta/compat.c:36:0:
>>> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
>>> --
>>> In file included from subversion/libsvn_delta/svndiff.c:31:0:
>>> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
>>> --
>>> In file included from subversion/libsvn_subr/auth.c:34:0:
>>> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
>>> --
>>> In file included from subversion/libsvn_subr/cache-inprocess.c:30:0:
>>> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
>>> --
>>> In file included from subversion/libsvn_subr/cache-membuffer.c:31:0:
>>> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
>>> --
>>> [...]
>>>
>>> (The relevant line number looks like 235 in my editor not 236.)
>> Julian, we've had this discussion before. I'm not going to change the
>> accepted way of checking autoconf macros just because you insist on
>> turning on warnings about perfectly valid and 15-years standard
>> behaviour of the C preprocessor. That by the way is not even turned on
>> in maintainer-mode.
> The warning is useful to catch spelling errors in macros (example: '#if
> APR_HAS_IPv6').
>
> +1 to changing the code to always define SVN_QSORT_R_NORMAL_ARG_ORDER,
> as either 0 or 1. I see no downside to that, do you?
Are we going to change every instance of using #if in the code to check
autoconf macros? If yes, please update hacking.html first.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Daniel Shahaf wrote on Fri, Nov 23, 2012 at 17:31:05 +0200:
> Branko Čibej wrote on Fri, Nov 23, 2012 at 15:59:16 +0100:
> > On 23.11.2012 15:35, Julian Foad wrote:
> > > In file included from subversion/libsvn_delta/compat.c:36:0:
> > > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > > --
> > > In file included from subversion/libsvn_delta/svndiff.c:31:0:
> > > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > > --
> > > In file included from subversion/libsvn_subr/auth.c:34:0:
> > > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > > --
> > > In file included from subversion/libsvn_subr/cache-inprocess.c:30:0:
> > > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > > --
> > > In file included from subversion/libsvn_subr/cache-membuffer.c:31:0:
> > > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > > --
> > > [...]
> > >
> > > (The relevant line number looks like 235 in my editor not 236.)
> >
> > Julian, we've had this discussion before. I'm not going to change the
> > accepted way of checking autoconf macros just because you insist on
> > turning on warnings about perfectly valid and 15-years standard
> > behaviour of the C preprocessor. That by the way is not even turned on
> > in maintainer-mode.
>
> The warning is useful to catch spelling errors in macros (example: '#if
> APR_HAS_IPv6').
>
> +1 to changing the code to always define SVN_QSORT_R_NORMAL_ARG_ORDER,
> as either 0 or 1. I see no downside to that, do you?
Put another way: saying that '#if UNDEFINED_MACRO' is well-defined just
says the code is correct. Hence, changing the code to avoid that
construct is about robustness, or clarity, rahter than correctness.
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Fri, Nov 23, 2012 at 15:59:16 +0100:
> On 23.11.2012 15:35, Julian Foad wrote:
> > In file included from subversion/libsvn_delta/compat.c:36:0:
> > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > --
> > In file included from subversion/libsvn_delta/svndiff.c:31:0:
> > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > --
> > In file included from subversion/libsvn_subr/auth.c:34:0:
> > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > --
> > In file included from subversion/libsvn_subr/cache-inprocess.c:30:0:
> > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > --
> > In file included from subversion/libsvn_subr/cache-membuffer.c:31:0:
> > ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> > --
> > [...]
> >
> > (The relevant line number looks like 235 in my editor not 236.)
>
> Julian, we've had this discussion before. I'm not going to change the
> accepted way of checking autoconf macros just because you insist on
> turning on warnings about perfectly valid and 15-years standard
> behaviour of the C preprocessor. That by the way is not even turned on
> in maintainer-mode.
The warning is useful to catch spelling errors in macros (example: '#if
APR_HAS_IPv6').
+1 to changing the code to always define SVN_QSORT_R_NORMAL_ARG_ORDER,
as either 0 or 1. I see no downside to that, do you?
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
Branko Čibej wrote:
> On 23.11.2012 15:35, Julian Foad wrote:
>> In file included from subversion/libsvn_delta/compat.c:36:0:
>> ./subversion/svn_private_config.h:236:7:
> "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
[...]
>
> Julian, we've had this discussion before. I'm not going to change the
> accepted way of checking autoconf macros just because you insist on
> turning on warnings about perfectly valid and 15-years standard
> behaviour of the C preprocessor. That by the way is not even turned on
> in maintainer-mode.
You clearly feel very strongly about that. Fine. I'm removing "-Wundef" from my local compiler flags now.
I don't "insist" on using that warning; rather I found it useful: in conjunction with a consistent *convention* of testing with "#ifdef" it has occasionally helped me catch bugs in the past. Yet it is no silver bullet: the opposite kind of bugs can happen too.
I'm fully aware the usage is "perfectly valid" C, but that and being however many years old doesn't by itself mean it's a good idea, of course.
On the other hand, I do believe consistency is important. Presently Subversion code is self-consistent in using "#ifdef", apart from this one instance, while inconsistent with some (many?) other autoconf users, including APR, which use "#if". If we still value consistency in this project, we should consider switching all our usage to "#if". I'm not volunteering to do that. And I only said "consider" -- I'm not insisting and I don't wish to prolong this discussion.
Ugh, I hate conflict. Can we talk about some exciting technical design issue next please :-)
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 23.11.2012 15:35, Julian Foad wrote:
> In file included from subversion/libsvn_delta/compat.c:36:0:
> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> --
> In file included from subversion/libsvn_delta/svndiff.c:31:0:
> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> --
> In file included from subversion/libsvn_subr/auth.c:34:0:
> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> --
> In file included from subversion/libsvn_subr/cache-inprocess.c:30:0:
> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> --
> In file included from subversion/libsvn_subr/cache-membuffer.c:31:0:
> ./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
> --
> [...]
>
> (The relevant line number looks like 235 in my editor not 236.)
Julian, we've had this discussion before. I'm not going to change the
accepted way of checking autoconf macros just because you insist on
turning on warnings about perfectly valid and 15-years standard
behaviour of the C preprocessor. That by the way is not even turned on
in maintainer-mode.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
In file included from subversion/libsvn_delta/compat.c:36:0:
./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
--
In file included from subversion/libsvn_delta/svndiff.c:31:0:
./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
--
In file included from subversion/libsvn_subr/auth.c:34:0:
./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
--
In file included from subversion/libsvn_subr/cache-inprocess.c:30:0:
./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
--
In file included from subversion/libsvn_subr/cache-membuffer.c:31:0:
./subversion/svn_private_config.h:236:7: "SVN_QSORT_R_NORMAL_ARG_ORDER" is not defined
--
[...]
(The relevant line number looks like 235 in my editor not 236.)
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 23.11.2012 12:58, Daniel Shahaf wrote:
> Branko Čibej wrote on Fri, Nov 23, 2012 at 11:43:29 +0100:
>> $ svn ps svm:ignore x .
>> svn: E195011: 'svm:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
>> (Use --force if you're sure about 'svm:ignore'.)
>
> "To set the 'svm:ignore' property, re-run with '--force'."
>
> Two differences: (a) drop the "if you're sure" wording in favour of "to
> do X", (b) single quotes around the --option.
Thanks, I'll amend the message.
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Branko Čibej wrote on Fri, Nov 23, 2012 at 11:43:29 +0100:
> On 19.11.2012 03:50, Julian Foad wrote:
> > Proposal:
> >
> > For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
> >
> > svn pset svn:foo
> > svn pedit svn:foo
> >
> > and all other subcommands where properties are handled would continue to work as normal, no '--force' needed
>
> Here's what the prototype currently does. Any suggestions for better
> wording are most welcome.
Nice work :-)
> $ svn ps svm:ignore x .
> svn: E195011: 'svm:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
> (Use --force if you're sure about 'svm:ignore'.)
"To set the 'svm:ignore' property, re-run with '--force'."
Two differences: (a) drop the "if you're sure" wording in favour of "to
do X", (b) single quotes around the --option.
---
For reference:
main.c- _("Lock comment file is a versioned file; "
main.c: "use '--force-log' to override"));
--
main.c- _("The log message is a pathname "
main.c: "(was -F intended?); use '--force-log' to override"));
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Branko Čibej <br...@wandisco.com>.
On 19.11.2012 03:50, Julian Foad wrote:
> Proposal:
>
> For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
>
> svn pset svn:foo
> svn pedit svn:foo
>
> and all other subcommands where properties are handled would continue to work as normal, no '--force' needed
Here's what the prototype currently does. Any suggestions for better
wording are most welcome.
$ svn ps svm:ignore x .
svn: E195011: 'svm:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
(Use --force if you're sure about 'svm:ignore'.)
$ svn ps gvm:ignore x .
property 'gvm:ignore' set on '.'
$ svn ps gvn:ignore x .
svn: E195011: 'gvn:ignore' is not a valid svn: property name; did you mean 'svn:ignore'?
(Use --force if you're sure about 'gvn:ignore'.)
$ svn ps svn:ignores x .
svn: E195011: 'svn:ignores' is not a valid svn: property name; did you mean 'svn:ignore'?
(Use --force if you're sure about 'svn:ignores'.)
$ svn ps svn:local-ignore x .
svn: E195011: 'svn:local-ignore' is not a valid svn: property name; did you mean 'svn:global-ignores'?
(Use --force if you're sure about 'svn:local-ignore'.)
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Greg Stein <gs...@gmail.com>.
Brane has done some excellent work here. But it seems nobody has asked
the question: why --force instead of (say) --allow-propname ?
Haven't we already decided that --force is horribly overloaded, and
horribly undefined?
Thx,
-g
On Sun, Nov 18, 2012 at 9:50 PM, Julian Foad <ju...@btopenworld.com> wrote:
> 'svn propset' lets us create any property name 'svn:foo', with good reason: we want old clients to be able to set and edit properties that only the newer clients know about. However, there is a pitfall for unwary users: it's east to mis-spell a prop name and not notice. For example:
>
> svn:ignores
> svn:global-ignore
>
> are both mis-spellings.
>
> It would seem reasonable to require '--force' to set an unknown 'svn:' property name. Note that we already require '--force' to set a known svn:something property to an invalid value.
>
> Proposal:
>
> For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
>
> svn pset svn:foo
> svn pedit svn:foo
>
> and all other subcommands where properties are handled would continue to work as normal, no '--force' needed, including:
>
> svn pdel svn:foo
> svn pget svn:foo
> svn plist
> svn patch, merge, diff, checkout, update, switch, ...
>
> Rationale notes:
>
> 'propset' would bail out even if the property already exists. One reason for this is so that multi-target or 'recursive' mode doesn't have to do anything special if the property exists on only some of the targets.
>
> 'pset' is the only one that really needs 'force' to meet the needs of this proposal. If a property already exists then it's probably best to assume that it exists on purpose, so we could argue that there is little harm in allowing 'pedit' and 'pdel'. Currently, 'pset' and 'pedit' require '--force' to set an invalid *value* for a known property (such as 'svn pset svn:eol-style x'); pdel doesn't take a '--force' option and intentionally allows properties with invalid values to be deleted. An advantage in discouraging 'svn pedit svn:foo' is that it can't validate the value if it doesn't know anything about 'svn:foo'. For these reasons and for consistency with the value-checking, the proposal has both 'pset' and 'pedit' validate the name, but not 'pdel'.
>
>
> Thoughts, objections?
>
> - Julian
>
>
>
> --
> Subversion Live 2012 - 2 Days: training, networking, demos & more! http://bit.ly/NgUaRi
> Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
>
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/19/2012 09:13 AM, Daniel Shahaf wrote:
> Another thing we could do is warn when unrecognised options are found in
> the config file (~/.subversion/*) or in the --config-option command-line
> option.
>
> This would be an independent improvement, i.e., it neither blocks nor
> is blocked by the propset improvements in #4261.
As regards the config files, -0 (tending towards -1). The runtime
configuration area is not release-version-specific. Folks like myself who
keep their ~/.subversion under version control and use it across many
different clients (which may be running different versions of Subversion)
could be constantly irritated by such warnings. And of course, we devs
would be likely to run into such things often, too. Besides, the config
files are generated as templates -- you'd have to work extra hard to botch
the spelling of an option name there. :-)
As for warning on bogus --config-option input ... I'm closer to +0 or +1 there.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Mon, Nov 19, 2012 at 14:08:34 +0000:
> C. Michael Pilato wrote:
>
> > On 11/18/2012 10:56 PM, Daniel Shahaf wrote:
> >> Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +0000:
> >>> Thoughts, objections?
> >>
> >> Another related trap is setting a revprop as a nodeprop, or vice-versa:
> >> svn commit --with-revprop=svn:eol-style=native -mm foo.c
> >> svn propset svn:log x foo.c
> >>
> >> You might want to require --force for these too.
> >
> > +1 to both suggestions. I can never remember if it's
> > "svn:ignore[s]" or
> > "svn:keyword[s]". Don't mind getting smacked for choosing
> > wrongly, but it's
> > always irritating for the propset to succeed yet the behavior not change.
>
> I agree with Daniel's suggestion too.
>
> I have filed enhancement issue #4261 "Setting unknown svn: propnames should require 'force'", which covers both suggestions together.
>
Another thing we could do is warn when unrecognised options are found in
the config file (~/.subversion/*) or in the --config-option command-line
option.
This would be an independent improvement, i.e., it neither blocks nor
is blocked by the propset improvements in #4261.
> Anybody want to work on it? Please do. I can get to it some time, but I don't know when, if nobody else picks it up.
>
> - Julian
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> On 11/18/2012 10:56 PM, Daniel Shahaf wrote:
>> Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +0000:
>>> Thoughts, objections?
>>
>> Another related trap is setting a revprop as a nodeprop, or vice-versa:
>> svn commit --with-revprop=svn:eol-style=native -mm foo.c
>> svn propset svn:log x foo.c
>>
>> You might want to require --force for these too.
>
> +1 to both suggestions. I can never remember if it's
> "svn:ignore[s]" or
> "svn:keyword[s]". Don't mind getting smacked for choosing
> wrongly, but it's
> always irritating for the propset to succeed yet the behavior not change.
I agree with Daniel's suggestion too.
I have filed enhancement issue #4261 "Setting unknown svn: propnames should require 'force'", which covers both suggestions together.
Anybody want to work on it? Please do. I can get to it some time, but I don't know when, if nobody else picks it up.
- Julian
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 11/18/2012 10:56 PM, Daniel Shahaf wrote:
> Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +0000:
>> Thoughts, objections?
>>
>
> Another related trap is setting a revprop as a nodeprop, or vice-versa:
> svn commit --with-revprop=svn:eol-style=native -mm foo.c
> svn propset svn:log x foo.c
>
> You might want to require --force for these too.
+1 to both suggestions. I can never remember if it's "svn:ignore[s]" or
"svn:keyword[s]". Don't mind getting smacked for choosing wrongly, but it's
always irritating for the propset to succeed yet the behavior not change.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Enterprise Cloud Development
Re: [RFC] svn propset should require 'force' to set unknown svn:
propnames
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Julian Foad wrote on Mon, Nov 19, 2012 at 02:50:46 +0000:
> Thoughts, objections?
>
Another related trap is setting a revprop as a nodeprop, or vice-versa:
svn commit --with-revprop=svn:eol-style=native -mm foo.c
svn propset svn:log x foo.c
You might want to require --force for these too.
Re: [RFC] svn propset should require 'force' to set unknown svn: propnames
Posted by Paul Burba <pt...@gmail.com>.
On Sun, Nov 18, 2012 at 9:50 PM, Julian Foad <ju...@btopenworld.com> wrote:
> 'svn propset' lets us create any property name 'svn:foo', with good reason: we want old clients to be able to set and edit properties that only the newer clients know about. However, there is a pitfall for unwary users: it's east to mis-spell a prop name and not notice. For example:
>
> svn:ignores
> svn:global-ignore
>
> are both mis-spellings.
>
> It would seem reasonable to require '--force' to set an unknown 'svn:' property name. Note that we already require '--force' to set a known svn:something property to an invalid value.
>
> Proposal:
>
> For any unrecognized property name in the 'svn:' name space, these would bail out with an error unless '--force' is given:
>
> svn pset svn:foo
> svn pedit svn:foo
+1 Seems totally reasonable. While working on svn:global-ignores, I
had a moment of "why isn't svn:gobal-ignore working?" during some ad
hoc testing.
> and all other subcommands where properties are handled would continue to work as normal, no '--force' needed, including:
>
> svn pdel svn:foo
> svn pget svn:foo
> svn plist
> svn patch, merge, diff, checkout, update, switch, ...
>
> Rationale notes:
>
> 'propset' would bail out even if the property already exists. One reason for this is so that multi-target or 'recursive' mode doesn't have to do anything special if the property exists on only some of the targets.
>
> 'pset' is the only one that really needs 'force' to meet the needs of this proposal. If a property already exists then it's probably best to assume that it exists on purpose, so we could argue that there is little harm in allowing 'pedit' and 'pdel'. Currently, 'pset' and 'pedit' require '--force' to set an invalid *value* for a known property (such as 'svn pset svn:eol-style x'); pdel doesn't take a '--force' option and intentionally allows properties with invalid values to be deleted. An advantage in discouraging 'svn pedit svn:foo' is that it can't validate the value if it doesn't know anything about 'svn:foo'. For these reasons and for consistency with the value-checking, the proposal has both 'pset' and 'pedit' validate the name, but not 'pdel'.
>
>
> Thoughts, objections?
>
> - Julian
>
>
>
> --
> Subversion Live 2012 - 2 Days: training, networking, demos & more! http://bit.ly/NgUaRi
> Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
>
--
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba