While working on issue #3282, I discovered that the propset below did
not error:
$ svn rm some_versioned_file
$ svn propset pname pval some_versioned_file
Instead, it just set 'pname' to 'pval' on 'some_versioned_file'.
That seemed wrong! If a file is scheduled for delete, should we
really allow further data-adding operations on it, like propset?
(This is tangentially related to issue #3282, by the way, though I
don't think it is necessary to change this behavior to fix #3282.)
So I came up with a patch (see end of this mail). Unfortunately, my
patch causes three merge tests to fail:
FAIL: merge_tests.py 61: merge fails if subtree is deleted on src
FAIL: merge_tests.py 93: dont merge revs into a subtree that predate it
FAIL: merge_tests.py 98: subtree merge source might not exist
Let's look at the first test (61) as an example. It fails because it
gets a new error (the one I intended, though not one the test expects).
Here's the relevant excerpt from running 'merge_tests.py -v 61':
--- Merging r3 through r5 into \
'svn-test-work/working_copies/merge_tests-61/A_copy':
D svn-test-work/working_copies/merge_tests-61/A_copy/D/gamma
subversion/libsvn_wc/props.c:2497: (apr_err=155023)
svn: 'svn-test-work/working_copies/merge_tests-61/A_copy/D/gamma' \
is scheduled for deletion; cannot set property 'svn:mergeinfo' on it
Hmmm. There are a couple of different ways we could go from here:
1) I could tweak my patch to move the schedule-delete check up to
somewhere "higher" than libsvn_wc (maybe libsvn_client? I'm not
sure what entry point the mergeinfo code is using...)
2) We could ask ourselves whether setting new mergeinfo on a
schedule-delete target really makes sense. If it doesn't, we
could trap and clear the error, or maybe detect that the entry
is schedule-delete and not attempt to set the mergeinfo in the
first place.
3) Something I haven't thought of.
I lean toward (2), but I'm not knowledgeable enough to be confident
that's the right way to go. Thoughts?
My initial patch is below.
-Karl
[[[
Fix bug whereby a schedule-delete file could still receive propsets.
(Found while debugging issue #3282, but does not fix that issue.)
#############################################################
### ###
### NOTE: DO NOT COMMIT THIS PATCH, IT CAUSES TESTS TO ###
### FAIL (merge_tests.py 61, 93, 98). ###
### ###
#############################################################
* subversion/libsvn_wc/props.c
(svn_wc_prop_set2): Error out if the file is scheduled for deletion.
* subversion/tests/cmdline/prop_tests.py
(propset_on_schedule_delete_target): New test.
(test_list): Run it.
]]]
Index: subversion/libsvn_wc/props.c
===================================================================
--- subversion/libsvn_wc/props.c (revision 32961)
+++ subversion/libsvn_wc/props.c (working copy)
@@ -2493,6 +2493,12 @@
/* Get the entry and name for this path. */
SVN_ERR(svn_wc__entry_versioned(&entry, path, adm_access, FALSE, pool));
+ if (entry->schedule == svn_wc_schedule_delete)
+ return svn_error_createf
+ (SVN_ERR_WC_INVALID_SCHEDULE, NULL,
+ _("'%s' is scheduled for deletion; cannot set property '%s' on it"),
+ path, name);
+
/* Get the access baton for the entry's directory. */
if (entry->kind == svn_node_dir)
SVN_ERR(svn_wc_adm_retrieve(&adm_access, adm_access, path, pool));
Index: subversion/tests/cmdline/prop_tests.py
===================================================================
--- subversion/tests/cmdline/prop_tests.py (revision 32962)
+++ subversion/tests/cmdline/prop_tests.py (working copy)
@@ -1679,7 +1679,18 @@
svntest.actions.run_and_verify_svn(None, expected_out, [],
'proplist', '-v', foo_url)
+def propset_on_schedule_delete_target(sbox):
+ "propset on schedule-delete target should fail"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+ iota_path = os.path.join(wc_dir, 'iota')
+ svntest.main.run_svn(None, 'rm', iota_path)
+ expected_stderr = ".*iota' is scheduled for deletion; " \
+ + "cannot set property 'pname' on it.*"
+ svntest.actions.run_and_verify_svn(None, [], expected_stderr,
+ 'propset', 'pname', 'pval', iota_path)
+
########################################################################
# Run the tests
@@ -1717,6 +1728,7 @@
SkipUnless(XFail(invalid_propvalues, svntest.main.is_ra_type_dav),
svntest.main.server_enforces_date_syntax),
XFail(same_replacement_props),
+ propset_on_schedule_delete_target,
]
if __name__ == '__main__':
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-08 04:41:27 CEST