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

Should propset on a schedule-delete target work?

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Sun, 07 Sep 2008 22:40:45 -0400

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

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