[[[
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 9311)
+++ 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 unless you change the type of the lock.
+ */
+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 9311)
+++ subversion/libsvn_wc/lock.c (working copy)
@@ -325,39 +325,64 @@
return SVN_NO_ERROR;
}
-/* This is essentially the guts of svn_wc_adm_open2, with the additional
- * parameter UNDER_CONSTRUCTION that gets set TRUE only when locking the
- * admin directory during initial creation.
+/* This is essentially the guts of svn_wc_adm_open2 and svn_wc_adm_extend, with
+ * the additional parameters UNDER_CONSTRUCTION that gets set TRUE only when
+ * locking the admin directory during initial creation and ALLOW_EXISTING_LOCKS
+ * that gets set TRUE to avoid errors if a directory has already a lock in
+ * associated.
+ *
+ * ROOT_ASSOCIATED is used to pass an access baton to all subdirectories. For
+ * svn_wc_adm_extend this is the access baton that the application specifies
+ * with the call of svn_wc_adm_extend.
*/
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)
- /* 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
- call svn_wc_adm_retrieve. */
+
+ 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 and no existing locks allowed. */
return svn_error_createf (SVN_ERR_WC_LOCKED, NULL,
_("Working copy '%s' locked"),
path);
+
+ if (lock == &missing)
+ lock = NULL;
+
+ /* Look whether the existing and requested lock are compatible */
+ if (lock)
+ {
+ if ( ( write_lock && (lock->type != svn_wc__adm_access_write_lock))
+ ||(! write_lock && (lock->type != svn_wc__adm_access_unlocked)))
+ {
+ return svn_error_createf (SVN_ERR_WC_LOCKED, NULL,
+ "Can't change the type of the lock for '%s'",
+ path);
+ }
+ }
}
- 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. */
@@ -381,18 +406,20 @@
}
/* Need to create a new lock */
- if (write_lock)
+ if (! lock)
{
- lock = adm_access_alloc (svn_wc__adm_access_write_lock, path, pool);
- SVN_ERR (create_lock (lock, 0, pool));
- lock->lock_exists = TRUE;
+ if (write_lock)
+ {
+ lock = adm_access_alloc (svn_wc__adm_access_write_lock, path, pool);
+ SVN_ERR (create_lock (lock, 0, pool));
+ lock->lock_exists = TRUE;
+ }
+ else
+ lock = adm_access_alloc (svn_wc__adm_access_unlocked, path, pool);
+ is_new_lock = 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)
@@ -436,8 +463,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)
@@ -497,8 +525,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;
}
@@ -524,16 +553,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 9311)
+++ 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 9311)
+++ 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 9311)
+++ 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 Sat Apr 10 10:05:28 2004