On Wed, 2005-03-23 at 05:27, Julian Foad wrote:
> Madan U Sreenivasan 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?
> 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?
>
> The current behaviour is useful in usage like "svn propdel myprop --recursive
> *" where not all files have the property. Do others think it would be
> acceptable to change the behaviour of "svn propdel" so that it fails
> unconditionally if the property does not exist? (I vote -1.)
>
> "svn propdel" doesn't accept "--force". What about adding the "--force" option
> and requiring that to get the old behaviour? (I vote +0.)
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.
>
> >
> > * subversion/tests/clients/cmdline/prop_tests.py
> > (remove_nonexistent_prop): Added function
> > to test removal of non-existent property.
>
> Also:
> (test_list): Add the new test.
Okay. I didnt know *this* had to be done. Thank you.
>
> > ]]]
> >
> >
> > ------------------------------------------------------------------------
> >
> > Index: subversion/libsvn_wc/props.c
> > ===================================================================
> > --- subversion/libsvn_wc/props.c (revision 13553)
> > +++ subversion/libsvn_wc/props.c (working copy)
> > @@ -1188,6 +1188,13 @@
> > property into it. */
> > if (new_value)
> > value = svn_string_create_from_buf (new_value, pool);
> > + /* value == NULL means the operation is propdel
> > + * if so, check if such a property already exists.
> > + * If not, return an error
> > + */
> > + if (!value && (apr_hash_get (prophash, name, APR_HASH_KEY_STRING)) == NULL)
> > + return svn_error_createf (SVN_ERR_BAD_PROP_KIND, NULL,
>
> SVN_ERR_PROPERTY_NOT_FOUND would be more appropriate.
>
> > + _("Property '%s' does not exist"), name);
> > apr_hash_set (prophash, name, APR_HASH_KEY_STRING, value);
> >
> > /* Open the propfile for writing. */
> > Index: subversion/tests/clients/cmdline/prop_tests.py
> > ===================================================================
> > --- subversion/tests/clients/cmdline/prop_tests.py (revision 13553)
> > +++ subversion/tests/clients/cmdline/prop_tests.py (working copy)
> > @@ -1123,6 +1123,25 @@
> > verify_output([ prop1 + ' : ' + propval1, prop2 + ' : ' + propval2,
> > 'Properties on ' ], output, errput)
> >
> > +#----------------------------------------------------------------------
> > +# remove non existent property
>
> That line is not needed because...
>
> > +def remove_nonexistent_prop(sbox):
> > + "remove a property that does not exist"
>
> ... the doc string is here.
>
> > +
> > + # Bootstrap
> > + sbox.build()
> > + wc_dir = sbox.wc_dir
> > +
> > + # delete a non-existent property from a file
> > + iota_path = os.path.join(wc_dir, 'iota')
> > + exp_stderr = "svn: Property 'thispropdoesntexist' does not exist"
>
> Checking for this exact error message seems unnecessarily strict, but it's safe.
>
> > + output, errput = svntest.main.run_svn(1, 'propdel',
> > + 'thispropdoesntexist',
> > + iota_path)
> > + if(errput[1].find(exp_stderr) == -1):
>
> When no error is given, as before this fix, then the test fails with
> "'errput[1]': list index out of range" which is a bit ugly but probably OK.
>
> > + print 'Error: expected stderr: ', exp_stderr
> > + print ' actual stderr : ', errput[1]
> > + raise svntest.Failure
> >
> > ########################################################################
> > # Run the tests
> > @@ -1148,6 +1167,7 @@
> > binary_props,
> > recursive_base_wc_ops,
> > url_props_ops,
> > + remove_nonexistent_prop,
> > ]
> >
> > if __name__ == '__main__':
>
>
> - 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 05:57:55 2005