[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Issue #2220: propdel returns success on deleting a non-existent prop

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2005-03-23 13:50:29 CET

Madan U Sreenivasan wrote:
> On Wed, 2005-03-23 at 05:27, Julian Foad wrote:
>>I would accept a patch that makes
>>svn_wc_prop_set2 fail only if "force" is set,
>
> You mean if the "force" is *not* set?

Yes. Sorry.

>> if it also makes the "propdel" command pass force=true so that the command-line interface behaviour is
>>unchanged. That would not by itself solve issue #2220, but it would be a step
>>closer
>
> Wouldn't this make the above change useless? Or am I missing something?

It would make no change to the behaviour of the "svn" command-line client, but
it would make it easier for "svn" and other clients to change to that behaviour
later. That is what I meant by "a step closer": it would provide the
infrastructure so that issue #2220 could be fixed with a smaller change in a
separate patch.

> No...I dont agree. force or no force... svn cant tell the user that a
> property has been deleted, when in fact, the property didnt exist
> beforehand. Yes, in case of recursive propdel, I agree with your
> point... My opinion is... to handle this, we should give the "property
> not present" message to stdout instead of generating an error.

That's a fair opinion.

There are two sides to this issue: the UI (User Interface) and the API. I am
going to address the API first.

The "force" option (in general) is a UI hack for a human user to get around
some particular temporary problem. It temporarily disables a whole bunch of
checks and restrictions. It's a bit like the option that some programs provide
to "Answer Yes to any question the program might ask", which is absolutely
horrible. It's a bit like turning off your firewall because it's blocking
something that you want to receive. It can quickly get the user out of a
difficult situation, but the user just has to hope that it won't allow
something bad to happen because of the other checks being disabled.

The "force" option thus has no place in an API unless the API is intended to
directly implement one particular UI, which ours are not. If an API wants to
provide a way to disable the checks for validity of "svn:*" properties, then
the option should be called "disable_svn_property_checks", etc. To allow
deletion of a nonexistent property, add another option,
"allow_delete_nonexistent_property", etc.

So, I think the "svn_wc_prop_set2" API needs to be fixed, at least in the name
of its "force" option. Also (many?) other APIs. Also, these property APIs
ought to use a notification callback, as noted in the issue. Also perhaps
ought to take a list of paths, for efficiency.

As for the UI, generating a warning or a different error is a possibility, but
I don't know if we should be changing the UI at this stage. Maybe we should
just accept that it does what it does.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 23 13:52:43 2005

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.