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

Re: [PATCH] Fix for issue 1797 - take three

From: makl <makl_at_tigris.org>
Date: 2004-04-07 07:28:04 CEST

[[[
Fix for issue 1797. non-recursive delete of directory tree fails

Patch from <makl@tigris.org>

* subversion/include/svn_wc.h
       Add new function svn_wc_adm_extend.

* subversion/libsvn_wc/lock.c
       (do_open): Allow locking of directories that are already locked.
       (svn_wc_adm_open2):
       (svn_wc__adm_pre_open):
          Add the new parameters to the call of do_open.
       (svn_wc_adm_extend): New function. This function is similar to
       svn_wc_adm_open2 but doesn't return a SVN_ERR_WC_LOCKED error if
       the directory has already a lock in associated.

* subversion/libsvn_client/commit.c
       (svn_client_commit): Always lock deleted directories recursive.

* subversion/libsvn_client/commit_util.c
       (svn_client__do_commit): Remove any deleted entries where the parent
       is deleted too.

* subversion/tests/clients/cmdline/commit_tests.py
       (commit_deleted_directory_tree_non_recursive_1): New
       regression test for case one.
       (commit_deleted_directory_tree_non_recursive_2_1): New
       regression test for case two with one deleted directory.
       (commit_deleted_directory_tree_non_recursive_2_1): New
       regression test for case two with three independent deleted
       directories.
]]]

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 9290)
+++ subversion/include/svn_wc.h (working copy)
@@ -107,6 +107,18 @@
                                apr_pool_t *pool);
 
 /**
+ * This function is similar to @c svn_wc_adm_open2 but doesn't return a
+ * @c SVN_ERR_WC_LOCKED error if the directory has already a lock in @a
+ * associated.
+ */
+svn_error_t *svn_wc_adm_extend (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *associated,
+ const char *path,
+ svn_boolean_t write_lock,
+ int depth,
+ apr_pool_t *pool);
+
+/**
  * @deprecated Provided for backward compatibility with the 1.0.0 API.
  *
  * Similar to svn_wc_adm_open2(). @a depth is set to -1 if @a tree_lock
Index: subversion/libsvn_wc/lock.c
===================================================================
--- subversion/libsvn_wc/lock.c (revision 9290)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -329,22 +329,29 @@
 static svn_error_t *
 do_open (svn_wc_adm_access_t **adm_access,
          svn_wc_adm_access_t *associated,
+ const svn_wc_adm_access_t *root_associated,
          const char *path,
          svn_boolean_t write_lock,
          int depth,
          svn_boolean_t under_construction,
+ svn_boolean_t allow_existing_locks,
          apr_pool_t *pool)
 {
- svn_wc_adm_access_t *lock;
+ svn_wc_adm_access_t *lock = NULL;
   int wc_format;
   svn_error_t *err;
+ svn_boolean_t is_new_lock = FALSE;
 
   if (associated)
     {
       adm_ensure_set (associated);
 
       lock = apr_hash_get (associated->set, path, APR_HASH_KEY_STRING);
- if (lock && lock != &missing)
+
+ if (! lock && root_associated && allow_existing_locks)
+ lock = apr_hash_get (root_associated->set, path, APR_HASH_KEY_STRING);
+
+ if (lock && lock != &missing && ! allow_existing_locks)
         /* Already locked. The reason we don't return the existing baton
            here is that the user is supposed to know whether a directory is
            locked: if it's not locked call svn_wc_adm_open, if it is locked
@@ -352,9 +359,12 @@
         return svn_error_createf (SVN_ERR_WC_LOCKED, NULL,
                                   "Working copy '%s' locked",
                                   path);
+
+ if (lock == &missing)
+ lock = NULL;
     }
 
- if (! under_construction)
+ if (! under_construction && ! lock)
     {
       /* By reading the format file we check both that PATH is a directory
          and that it is a working copy. */
@@ -377,19 +387,21 @@
                                      pool));
     }
 
+ if (! lock)
+ {
+ lock = adm_access_alloc (svn_wc__adm_access_unlocked, path, pool);
+ is_new_lock = TRUE;
+ }
+
   /* Need to create a new lock */
- if (write_lock)
+ if (write_lock && lock->type != svn_wc__adm_access_write_lock)
     {
- lock = adm_access_alloc (svn_wc__adm_access_write_lock, path, pool);
+ lock->type = svn_wc__adm_access_write_lock;
       SVN_ERR (create_lock (lock, 0, pool));
       lock->lock_exists = TRUE;
     }
- else
- {
- lock = adm_access_alloc (svn_wc__adm_access_unlocked, path, pool);
- }
 
- if (! under_construction)
+ if (! under_construction && is_new_lock)
     {
       lock->wc_format = wc_format;
       if (write_lock)
@@ -433,8 +445,9 @@
           entry_path = svn_path_join (lock->path, entry->name, subpool);
 
           /* Don't use the subpool pool here, the lock needs to persist */
- err = do_open (&entry_access, lock, entry_path, write_lock, depth,
- FALSE, lock->pool);
+ err = do_open (&entry_access, lock, root_associated, entry_path,
+ write_lock, depth,
+ FALSE, allow_existing_locks, lock->pool);
           if (err)
             {
               if (err->apr_err != SVN_ERR_WC_NOT_DIRECTORY)
@@ -494,8 +507,9 @@
      when the cleanup handler runs. If the apr_xlate_t cleanup handler
      were to run *before* the access baton cleanup handler, then the access
      baton's handler won't work. */
- apr_pool_cleanup_register (lock->pool, lock, pool_cleanup,
- pool_cleanup_child);
+ if (is_new_lock)
+ apr_pool_cleanup_register (lock->pool, lock, pool_cleanup,
+ pool_cleanup_child);
   *adm_access = lock;
   return SVN_NO_ERROR;
 }
@@ -521,16 +535,27 @@
                   int depth,
                   apr_pool_t *pool)
 {
- return do_open (adm_access, associated, path, write_lock, depth, FALSE,
- pool);
+ return do_open (adm_access, associated, NULL, path, write_lock, depth, FALSE,
+ FALSE, pool);
 }
 
+svn_error_t *svn_wc_adm_extend (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *associated,
+ const char *path,
+ svn_boolean_t write_lock,
+ int depth,
+ apr_pool_t *pool)
+{
+ return do_open (adm_access, associated, associated, path, write_lock, depth,
+ FALSE, TRUE, pool);
+}
+
 svn_error_t *
 svn_wc__adm_pre_open (svn_wc_adm_access_t **adm_access,
                       const char *path,
                       apr_pool_t *pool)
 {
- return do_open (adm_access, NULL, path, TRUE, 0, TRUE, pool);
+ return do_open (adm_access, NULL, NULL, path, TRUE, 0, TRUE, FALSE, pool);
 }
 
 
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 9290)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -1143,14 +1143,40 @@
 
   /* Build a hash from our COMMIT_ITEMS array, keyed on the
      URI-decoded relative paths (which come from the item URLs). And
- keep an array of those decoded paths, too. */
+ keep an array of those decoded paths, too.
+ In addition remove any deleted entries where the parent is also
+ deleted (deletes are always recursive). */
+
+ const char *delete_base = NULL;
   for (i = 0; i < commit_items->nelts; i++)
     {
+ svn_pool_clear (subpool);
+
       svn_client_commit_item_t *item =
         APR_ARRAY_IDX (commit_items, i, svn_client_commit_item_t *);
       const char *path = svn_path_uri_decode (item->url, pool);
- apr_hash_set (items_hash, path, APR_HASH_KEY_STRING, item);
- APR_ARRAY_PUSH (paths, const char *) = path;
+ svn_boolean_t do_commit = TRUE;
+
+ if (item->state_flags == SVN_CLIENT_COMMIT_ITEM_DELETE)
+ {
+ if (delete_base)
+ {
+ if (svn_path_is_child(delete_base, path, subpool))
+ do_commit = FALSE;
+ else
+ delete_base = path;
+ }
+ else
+ delete_base = path;
+ }
+ else
+ delete_base = NULL;
+
+ if (do_commit)
+ {
+ apr_hash_set (items_hash, path, APR_HASH_KEY_STRING, item);
+ APR_ARRAY_PUSH (paths, const char *) = path;
+ }
     }
 
   /* Setup the callback baton. */
Index: subversion/libsvn_client/commit.c
===================================================================
--- subversion/libsvn_client/commit.c (revision 9290)
+++ subversion/libsvn_client/commit.c (working copy)
@@ -1196,15 +1196,27 @@
       /* First lock all the dirs to be locked non-recursively */
       if (dirs_to_lock)
         {
+ const svn_wc_entry_t *entry;
+
           for (i = 0; i < dirs_to_lock->nelts ; ++i)
             {
               target = APR_ARRAY_IDX (dirs_to_lock, i, const char *);
 
- SVN_ERR (svn_wc_adm_open2 (&adm_access, base_dir_access,
- target,
- TRUE, /* Write lock */
- 0, /* Depth */
- pool));
+ SVN_ERR (svn_wc_adm_extend (&adm_access, base_dir_access,
+ target,
+ TRUE, /* Write lock */
+ 0, /* Depth */
+ pool));
+
+ /* Check if the directory is deleted. If yes extend the lock
+ to a recursive one. */
+ SVN_ERR (svn_wc_entry (&entry, target, adm_access, TRUE, pool));
+ if (entry && (entry->schedule == svn_wc_schedule_delete))
+ SVN_ERR (svn_wc_adm_extend (&adm_access, base_dir_access,
+ target,
+ TRUE, /* Write lock */
+ -1, /* Depth */
+ pool));
             }
         }
 
Index: subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- subversion/tests/clients/cmdline/commit_tests.py (revision 9290)
+++ subversion/tests/clients/cmdline/commit_tests.py (working copy)
@@ -1859,6 +1859,113 @@
   
 
 
+#----------------------------------------------------------------------
+# Regression test for issue 1797 case 2 part 1
+# Committing a single deleted directory
+
+def commit_deleted_directory_tree_non_recursive_2_1(sbox):
+ "non-recursively commit a deleted tree case 2-1"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ dir_path = os.path.join(sbox.wc_dir, 'A')
+
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', dir_path)
+ # Created expected output tree for 'svn ci'
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A' : Item(verb='Deleting'),
+ })
+
+ # Create expected status tree
+ expected_status = svntest.wc.State(wc_dir, {
+ '' : Item(status=' ', wc_rev=1, repos_rev=2),
+ 'iota' : Item(status=' ', wc_rev=1, repos_rev=2) ,
+ })
+
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, '-N',
+ dir_path)
+
+
+
+#----------------------------------------------------------------------
+# Regression test for issue 1797 case 2 part 2
+# Committing three independent deleted directories
+
+def commit_deleted_directory_tree_non_recursive_2_2(sbox):
+ "non-recursively commit a deleted tree case 2-2"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ dir_path1 = os.path.join(sbox.wc_dir, 'A/B')
+ dir_path2 = os.path.join(sbox.wc_dir, 'A/C')
+ dir_path3 = os.path.join(sbox.wc_dir, 'A/D')
+
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', dir_path1, dir_path2, dir_path3)
+ # Created expected output tree for 'svn ci'
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/B' : Item(verb='Deleting'),
+ 'A/C' : Item(verb='Deleting'),
+ 'A/D' : Item(verb='Deleting'),
+ })
+
+ # Create expected status tree
+ expected_status = svntest.wc.State(wc_dir, {
+ '' : Item(status=' ', wc_rev=1, repos_rev=2),
+ 'iota' : Item(status=' ', wc_rev=1, repos_rev=2) ,
+ 'A' : Item(status=' ', wc_rev=1, repos_rev=2) ,
+ 'A/mu' : Item(status=' ', wc_rev=1, repos_rev=2) ,
+ })
+
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, '-N',
+ dir_path1, dir_path2, dir_path3)
+
+
+
+#----------------------------------------------------------------------
+# Regression test for issue 1797 case 1
+# Committing multiple directories in the same directory tree
+
+def commit_deleted_directory_tree_non_recursive_1(sbox):
+ "non-recursively commit a deleted tree case 1"
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ dir_path1 = os.path.join(sbox.wc_dir, 'A')
+ dir_path2 = os.path.join(sbox.wc_dir, 'A/B')
+ dir_path3 = os.path.join(sbox.wc_dir, 'A/C')
+ dir_path4 = os.path.join(sbox.wc_dir, 'A/D')
+ dir_path5 = os.path.join(sbox.wc_dir, 'A/B/E')
+ dir_path6 = os.path.join(sbox.wc_dir, 'A/B/F')
+ dir_path7 = os.path.join(sbox.wc_dir, 'A/D/G')
+ dir_path8 = os.path.join(sbox.wc_dir, 'A/D/H')
+
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'rm', dir_path1)
+ # Created expected output tree for 'svn ci'
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A' : Item(verb='Deleting'),
+ })
+
+ # Create expected status tree
+ expected_status = svntest.wc.State(wc_dir, {
+ '' : Item(status=' ', wc_rev=1, repos_rev=2),
+ 'iota' : Item(status=' ', wc_rev=1, repos_rev=2) ,
+ })
+
+ svntest.actions.run_and_verify_commit(wc_dir, expected_output,
+ expected_status, None,
+ None, None, None, None, '-N',
+ dir_path1, dir_path2, dir_path3,
+ dir_path4, dir_path5, dir_path6,
+ dir_path7, dir_path8)
+
+
 ########################################################################
 # Run the tests
 
@@ -1896,6 +2003,9 @@
               commit_out_of_date_deletions,
               commit_with_bad_log_message,
               from_wc_top_with_bad_editor,
+ commit_deleted_directory_tree_non_recursive_1,
+ commit_deleted_directory_tree_non_recursive_2_1,
+ commit_deleted_directory_tree_non_recursive_2_2,
              ]
 
 if __name__ == '__main__':

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Apr 7 07:25:59 2004

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.