Continuing issue #749. Make svn_wc_adm_access_t optionally
hierarchical. Lock entire tree before starting a commit. Revert
r2504 as it's no longer needed.
This got more complicated than I would have liked. I am still
concerned about which directories get locked, and in which order they
get locked.
* subversion/include/svn_error_codes.h (SVN_ERR_WC_INVALID_LOCK): New
error code.
* subversion/include/svn_wc.h
(svn_wc_adm_open): Add parent_access and tree_lock parameters.
(svn_wc_adm_retrieve): New function.
(svn_wc_adm_close): Document new behavior.
* subversion/libsvn_wc/adm_ops.c
(svn_wc_process_committed, svn_wc_remove_from_revision_control):
Replace svn_wc_adm_open with svn_wc_adm_retrieve and delete
svn_wc_adm_close.
(svn_wc_delete, svn_wc_revert): Pass new parameters to svn_wc_adm_open.
* subversion/libsvn_wc/log.c
(log_do_delete_entry): Replace svn_wc_adm_open with svn_wc_adm_retrieve.
(log_do_committed): Replace svn_wc_adm_open with svn_wc_adm_retrieve
and delete svn_wc_adm_close.
* subversion/libsvn_wc/props.c (svn_wc_merge_prop_diffs): Pass new
parameters to svn_wc_adm_open.
* subversion/libsvn_wc/adm_files.c (init_adm): Pass new parameters to
svn_wc_adm_open.
* subversion/libsvn_client/commit.c
(svn_client_commit): Revert r2504. Return an error if the hash lookup
for an access baton fails.
* subversion/libsvn_client/commit_util.c
(lock_dir): Lookup parent baton. Pass new parameters to svn_wc_adm_open.
(harvest_committables): New lock_subdirs parameter. Lock all
sub-directories when locking a directory.
(svn_client__harvest_committables, svn_client__get_copy_committables): Pass
new parameter to harvest_committables.
* subversion/libsvn_wc/update_editor.c
(delete_entry): Pass new parameters to svn_wc_adm_open to lock
entire tree.
(close_directory, svn_wc_install_file): Pass new parameters to
svn_wc_adm_open.
* subversion/libsvn_client/externals.c (relegate_external,
handle_external_item_change): Pass new parameters to svn_wc_adm_open.
* subversion/libsvn_wc/wc.h
(enum svn_wc__adm_access_type): Renamed from enum svn_wc_adm_access_type.
(svn_wc__adm_access_unlocked, svn_wc__adm_access_read_lock,
svn_wc__adm_access_write_lock): Renamed enum identifiers.
(svn_wc__adm_access_closed): New enum identifier.
(struct svn_wc_adm_access_t): Added parent and children members.
* subversion/libsvn_wc/lock.c
(svn_wc__adm_access_pool_cleanup, svn_wc__adm_access_pool_cleanup_child,
svn_wc__adm_access_alloc, svn_wc_adm_retrieve): New functions.
(svn_wc__adm_steal_write_lock): Use svn_wc__adm_access_alloc.
(svn_wc_adm_open): Add parent_access and tree_lock parameters and
processing. Use svn_wc__adm_access_alloc. For read locks check path
is a directory. Register pool cleanup handler.
(svn_wc__do_adm_close): Renamed from svn_wc_adm_close and made
static. New parameter to preserver lock file. Close children. Kill
pool cleanup handler. Reset state to svn_wc__adm_access_closed.
(svn_wc_adm_close): Now a simple wrapper that calls svn_wc__do_adm_close.
(svn_wc_adm_write_check): enum svn_wc__adm_access_type name change.
* subversion/tests/clients/cmdline/copy_tests.py (copy_modify_commit): New
test.
git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/issue-749-dev@842628 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/include/svn_error_codes.h b/subversion/include/svn_error_codes.h
index 88e7656..f7f2e91 100644
--- a/subversion/include/svn_error_codes.h
+++ b/subversion/include/svn_error_codes.h
@@ -200,6 +200,9 @@
SVN_ERRDEF (SVN_ERR_WC_NOT_LOCKED,
"Working copy not locked")
+ SVN_ERRDEF (SVN_ERR_WC_INVALID_LOCK,
+ "Invalid lock")
+
SVN_ERRDEF (SVN_ERR_WC_NOT_DIRECTORY,
"Path is not a working copy directory")
diff --git a/subversion/include/svn_wc.h b/subversion/include/svn_wc.h
index ca037db..b58df44 100644
--- a/subversion/include/svn_wc.h
+++ b/subversion/include/svn_wc.h
@@ -59,17 +59,57 @@
/* Return an access baton in ADM_ACCESS for the working copy administrative
area associated with the directory PATH. If WRITE_LOCK is set the baton
will include a write lock, otherwise the baton can only be used for read
- access. POOL will be used to allocate the baton and any subsequently
- cached items. */
+ access.
+
+ If PARENT_ACCESS is an open access baton PATH must refer to a
+ subdirectory of the PARENT_ACCESS directory, and then ADM_ACCESS will be
+ a child of PARENT_ACCESS. svn_wc_adm_close can be used to close
+ ADM_ACCESS. If ADM_ACCESS has not been closed when PARENT_ACCESS is
+ closed, it wil be closed automatically at that stage.
+
+ If PARENT_ACCESS is an open access baton, and PATH refers to a
+ subdirectory that has already been opened as a child of PARENT_ACCESS
+ then the error SVN_ERR_WC_LOCKED will be returned.
+
+ PARENT_ACCESS can be NULL, in which case ADM_ACCESS is not a child of
+ any other baton and must be explicitly closed.
+
+ If TREE_LOCK is TRUE then the working copy directory hierarchy under
+ PATH will be locked. At each level batons for the subdirectories will be
+ children of the parent directory's baton. This is an all-or-nothing
+ option, if it is not possible to lock the entire tree then an error will
+ be returned, there will be no locks and ADM_ACCESS will not be valid.
+
+ POOL will be used to allocate memory for the baton and any subsequently
+ cached items. If ADM_ACCESS has not been closed when the pool is
+ cleared, it will be closed automatically at the point but any physical
+ lock will remain in the working copy. */
svn_error_t *svn_wc_adm_open (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *parent_access,
const char *path,
svn_boolean_t write_lock,
+ svn_boolean_t tree_lock,
apr_pool_t *pool);
-/* Give up the access baton ADM_ACCESS, and its lock if any */
+/* Return an access baton in ADM_ACCESS associated with PATH. PATH must be
+ a subdirectory of the PARENT_ACCESS directory, and must have been
+ previously opened with PARENT_ACCESS as a parent.
+
+ POOL is used only for local processing it is not used for the batons.
+
+ ### I can't think of a good name for this function. */
+svn_error_t *svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *parent_access,
+ const char *path,
+ apr_pool_t *pool);
+
+/* Give up the access baton ADM_ACCESS, and its lock if any. It will also
+ close any child batons, for which this is a parent baton, that have not
+ yet been explicitly closed. Any physical locks will be removed from the
+ working copy. */
svn_error_t *svn_wc_adm_close (svn_wc_adm_access_t *adm_access);
-/* Ensure ADM_ACCESS has a write lock, and that it still exists. Returns
+/* Ensure ADM_ACCESS has a write lock, and that it is still valid. Returns
SVN_ERR_WC_NOT_LOCKED if this is not the case. */
svn_error_t *svn_wc_adm_write_check (svn_wc_adm_access_t *adm_access);
diff --git a/subversion/libsvn_client/commit.c b/subversion/libsvn_client/commit.c
index ca4a739..87a7bc8 100644
--- a/subversion/libsvn_client/commit.c
+++ b/subversion/libsvn_client/commit.c
@@ -919,15 +919,12 @@
svn_path_split_nts (item->path, &adm_access_path, NULL, pool);
adm_access = apr_hash_get (locked_dirs, adm_access_path,
APR_HASH_KEY_STRING);
-
- if ( ! adm_access )
- {
- SVN_ERR (svn_wc_adm_open (&adm_access, adm_access_path, TRUE,
- pool));
- apr_hash_set (locked_dirs, apr_pstrdup(pool, adm_access_path),
- APR_HASH_KEY_STRING, adm_access);
- }
-
+ if (! adm_access)
+ /* This should *never* happen */
+ return svn_error_createf(SVN_ERR_GENERAL, 0, NULL, pool,
+ "BUG: no lock for %s",
+ item->path);
+
if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
&& (item->kind == svn_node_dir)
&& (item->copyfrom_url))
diff --git a/subversion/libsvn_client/commit_util.c b/subversion/libsvn_client/commit_util.c
index 7208e51..05fbb73 100644
--- a/subversion/libsvn_client/commit_util.c
+++ b/subversion/libsvn_client/commit_util.c
@@ -60,8 +60,15 @@
if (! apr_hash_get (locked_dirs, dir, APR_HASH_KEY_STRING))
{
- svn_wc_adm_access_t *adm_access;
- SVN_ERR (svn_wc_adm_open (&adm_access, dir, TRUE, hash_pool));
+ /* Short lived, don't use hash pool */
+ const char *parent_dir = svn_path_remove_component_nts (dir, pool);
+ svn_wc_adm_access_t *adm_access, *parent_access;
+
+ /* Get parent, NULL is OK */
+ parent_access = apr_hash_get (locked_dirs, parent_dir,
+ APR_HASH_KEY_STRING);
+ SVN_ERR (svn_wc_adm_open (&adm_access, parent_access, dir, TRUE, FALSE,
+ hash_pool));
apr_hash_set (locked_dirs, apr_pstrdup (hash_pool, dir),
APR_HASH_KEY_STRING, adm_access);
}
@@ -139,6 +146,7 @@
svn_boolean_t adds_only,
svn_boolean_t copy_mode,
svn_boolean_t nonrecursive,
+ svn_boolean_t lock_subdirs,
apr_pool_t *pool)
{
apr_pool_t *subpool = svn_pool_create (pool); /* ### why? */
@@ -293,6 +301,8 @@
{
/* If the commit item is a directory, lock it, else lock its parent. */
if (entry->kind == svn_node_dir)
+ /* ### Need to lock parent here as well? We will need to modify the
+ ### parents entry file if this is not the root */
SVN_ERR (lock_dir (locked_dirs, path, pool));
else
SVN_ERR (lock_dir (locked_dirs, p_path, pool));
@@ -301,46 +311,50 @@
add_committable (committables, path, entry->kind, url,
cf_url ? entry->copyfrom_rev : entry->revision,
cf_url, state_flags);
+
+ /* We need to lock all directories to enable post-commit
+ processing of the working copy */
+ lock_subdirs = TRUE;
}
- /* For directories, recursively handle each of their entries (except
- when the directory is being deleted, unless the deletion is part
- of a replacement ... how confusing). */
- if ((entries)
- && ((! (state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE))
- || (state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)))
+ if (entries)
{
apr_hash_index_t *hi;
svn_wc_entry_t *this_entry;
apr_pool_t *loop_pool = svn_pool_create (subpool);
/* Loop over all other entries in this directory, skipping the
- "this dir" entry. */
+ "this dir" entry.
+
+ ### May need to do this twice, if we need to do sub-directories,
+ ### before files. Subdirectories may require us to lock the
+ ### parent, files only lock this dir, and doing them in the wrong
+ ### order may break the parent-child lock relationship. */
for (hi = apr_hash_first (subpool, entries);
hi;
hi = apr_hash_next (hi))
{
const void *key;
- apr_ssize_t klen;
void *val;
const char *name;
const char *used_url = NULL;
+ /* ### Why do we need to alloc these? */
const char *full_path = apr_pstrdup (loop_pool, path);
const char *this_url = apr_pstrdup (loop_pool, url);
const char *this_cf_url
= cf_url ? apr_pstrdup (loop_pool, cf_url) : NULL;
/* Get the next entry */
- apr_hash_this (hi, &key, &klen, &val);
- name = (const char *) key;
+ apr_hash_this (hi, &key, NULL, &val);
+ name = key;
/* Skip "this dir" */
if (! strcmp (name, SVN_WC_ENTRY_THIS_DIR))
continue;
/* Name is an entry name; value is an entry structure. */
- this_entry = (svn_wc_entry_t *) val;
+ this_entry = val;
/* Skip subdirectory entries when we're not recursing.
@@ -358,6 +372,16 @@
if (this_cf_url)
this_cf_url = svn_path_join (this_cf_url, name, loop_pool);
+ if (lock_subdirs && this_entry->kind == svn_node_dir)
+ SVN_ERR (lock_dir (locked_dirs, full_path, pool));
+
+ /* For directories, recursively handle each of their entries
+ (except when the directory is being deleted, unless the
+ deletion is part of a replacement ... how confusing). */
+ if ((state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
+ && ! (state_flags & SVN_CLIENT_COMMIT_ITEM_ADD))
+ continue;
+
/* We'll use the entry's URL if it has one and if we aren't
in copy_mode, else, we'll just extend the parent's URL
with the entry's basename. */
@@ -377,6 +401,7 @@
adds_only,
copy_mode,
FALSE,
+ lock_subdirs,
subpool));
svn_pool_clear (loop_pool);
@@ -493,7 +518,7 @@
/* Handle our TARGET. */
SVN_ERR (harvest_committables (*committables, *locked_dirs, target,
url, NULL, entry, NULL, FALSE, FALSE,
- nonrecursive, pool));
+ nonrecursive, FALSE, pool));
i++;
}
@@ -527,7 +552,7 @@
/* Handle our TARGET. */
SVN_ERR (harvest_committables (*committables, *locked_dirs, target,
new_url, entry->url, entry, NULL,
- FALSE, TRUE, FALSE, pool));
+ FALSE, TRUE, FALSE, FALSE, pool));
return SVN_NO_ERROR;
}
diff --git a/subversion/libsvn_client/externals.c b/subversion/libsvn_client/externals.c
index 6ddf4e1..29d30a1 100644
--- a/subversion/libsvn_client/externals.c
+++ b/subversion/libsvn_client/externals.c
@@ -232,7 +232,7 @@
svn_error_t *err;
svn_wc_adm_access_t *adm_access;
- SVN_ERR (svn_wc_adm_open (&adm_access, path, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, path, TRUE, FALSE, pool));
err = svn_wc_remove_from_revision_control (adm_access,
SVN_WC_ENTRY_THIS_DIR,
TRUE,
@@ -378,7 +378,8 @@
svn_error_t *err;
svn_wc_adm_access_t *adm_access;
- SVN_ERR (svn_wc_adm_open (&adm_access, path, TRUE, ib->pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, path, TRUE, FALSE,
+ ib->pool));
/* We don't use relegate_external() here, because we know that
nothing else in this externals description (at least) is
diff --git a/subversion/libsvn_wc/adm_files.c b/subversion/libsvn_wc/adm_files.c
index 0de4bae..8549b86 100644
--- a/subversion/libsvn_wc/adm_files.c
+++ b/subversion/libsvn_wc/adm_files.c
@@ -1107,7 +1107,7 @@
/* Lock it immediately. Theoretically, no compliant wc library
would ever consider this an adm area until a README file were
present... but locking it is still appropriately paranoid. */
- SVN_ERR (svn_wc_adm_open (&adm_access, path, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, path, TRUE, FALSE, pool));
/** Make subdirectories. ***/
diff --git a/subversion/libsvn_wc/adm_ops.c b/subversion/libsvn_wc/adm_ops.c
index ff79b88..64a3a70 100644
--- a/subversion/libsvn_wc/adm_ops.c
+++ b/subversion/libsvn_wc/adm_ops.c
@@ -354,8 +354,8 @@
this_path = svn_path_join (path, name, subpool);
if (current_entry->kind == svn_node_dir)
- SVN_ERR (svn_wc_adm_open (&child_access, this_path, TRUE,
- subpool));
+ SVN_ERR (svn_wc_adm_retrieve (&child_access, adm_access, this_path,
+ subpool));
else
child_access = adm_access;
@@ -366,9 +366,6 @@
(current_entry->kind == svn_node_dir) ? TRUE : FALSE,
new_revnum, rev_date, rev_author, subpool));
- if (current_entry->kind == svn_node_dir)
- SVN_ERR (svn_wc_adm_close (child_access));
-
svn_pool_clear (subpool);
}
@@ -630,7 +627,8 @@
### function, at which stage this open will fail and can be
### removed. */
svn_wc_adm_access_t *adm_access;
- SVN_ERR (svn_wc_adm_open (&adm_access, path, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, path, TRUE, FALSE,
+ pool));
SVN_ERR (svn_wc_remove_from_revision_control
(adm_access, SVN_WC_ENTRY_THIS_DIR, FALSE, pool));
@@ -1201,7 +1199,8 @@
if (entry->kind == svn_node_file)
{
was_deleted = entry->deleted;
- SVN_ERR (svn_wc_adm_open (&adm_access, parent, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent, TRUE, FALSE,
+ pool));
}
else if (entry->kind == svn_node_dir)
{
@@ -1211,7 +1210,8 @@
parents_entry = apr_hash_get (entries, basey, APR_HASH_KEY_STRING);
if (parents_entry)
was_deleted = parents_entry->deleted;
- SVN_ERR (svn_wc_adm_open (&adm_access, path, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, path, TRUE, TRUE,
+ pool));
}
/* Remove the item from revision control. */
@@ -1507,8 +1507,8 @@
const char *entrypath = svn_path_join (adm_access->path,
current_entry_name,
subpool);
- SVN_ERR (svn_wc_adm_open (&entry_access, entrypath, TRUE,
- subpool));
+ SVN_ERR (svn_wc_adm_retrieve (&entry_access, adm_access,
+ entrypath, pool));
err = svn_wc_remove_from_revision_control (entry_access,
SVN_WC_ENTRY_THIS_DIR,
destroy_wf, subpool);
@@ -1519,7 +1519,6 @@
}
else if (err)
return err;
- SVN_ERR (svn_wc_adm_close (entry_access));
}
}
diff --git a/subversion/libsvn_wc/lock.c b/subversion/libsvn_wc/lock.c
index 651ba99..668e389 100644
--- a/subversion/libsvn_wc/lock.c
+++ b/subversion/libsvn_wc/lock.c
@@ -23,9 +23,14 @@
#include "wc.h"
#include "adm_files.h"
+#include "svn_pools.h"
static svn_error_t *
+svn_wc__do_adm_close (svn_wc_adm_access_t *adm_access,
+ svn_boolean_t abort);
+
+static svn_error_t *
svn_wc__lock (svn_wc_adm_access_t *adm_access, int wait_for, apr_pool_t *pool)
{
svn_error_t *err;
@@ -59,17 +64,64 @@
return svn_wc__remove_adm_file (path, pool, SVN_WC__ADM_LOCK, NULL);
}
+static apr_status_t
+svn_wc__adm_access_pool_cleanup (void *p)
+{
+ svn_wc_adm_access_t *lock = p;
+ svn_error_t *err;
+
+ err = svn_wc__do_adm_close (lock, TRUE);
+
+ /* ### Is this the correct way to handle the error? */
+ if (err)
+ {
+ apr_status_t apr_err = err->apr_err;
+ svn_error_clear_all (err);
+ return apr_err;
+ }
+ else
+ return APR_SUCCESS;
+}
+
+static apr_status_t
+svn_wc__adm_access_pool_cleanup_child (void *p)
+{
+ svn_wc_adm_access_t *lock = p;
+ apr_pool_cleanup_kill (lock->pool, lock, svn_wc__adm_access_pool_cleanup);
+ return APR_SUCCESS;
+}
+
+static svn_wc_adm_access_t *
+svn_wc__adm_access_alloc (enum svn_wc__adm_access_type type,
+ svn_wc_adm_access_t *parent,
+ const char *path,
+ apr_pool_t *pool)
+{
+ svn_wc_adm_access_t *lock = apr_palloc (pool, sizeof (*lock));
+ lock->type = type;
+ lock->parent = parent;
+ lock->children = NULL;
+ lock->lock_exists = FALSE;
+ /* ### Some places lock with a path that is not canonical, we need
+ ### cannonical paths for reliable parent-child determination */
+ lock->path = svn_path_canonicalize_nts (apr_pstrdup (pool, path), pool);
+ lock->pool = pool;
+
+ apr_pool_cleanup_register (lock->pool, lock,
+ svn_wc__adm_access_pool_cleanup,
+ svn_wc__adm_access_pool_cleanup_child);
+ return lock;
+}
+
svn_error_t *
svn_wc__adm_steal_write_lock (svn_wc_adm_access_t **adm_access,
const char *path,
apr_pool_t *pool)
{
svn_error_t *err;
- svn_wc_adm_access_t *lock = apr_palloc (pool, sizeof (**adm_access));
- lock->type = svn_wc_adm_access_write_lock;
- lock->lock_exists = FALSE;
- lock->path = apr_pstrdup (pool, path);
- lock->pool = pool;
+ svn_wc_adm_access_t *lock
+ = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, NULL, path,
+ pool);
err = svn_wc__lock (lock, 0, pool);
if (err)
@@ -85,27 +137,126 @@
return SVN_NO_ERROR;
}
+static svn_error_t *
+svn_wc__adm_access_child (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *parent_access,
+ const char *path,
+ apr_pool_t *pool)
+{
+
+ /* PATH must be a child. We only need the result, not the allocated
+ name. */
+ const char *name = svn_path_is_child (parent_access->path, path, pool);
+ if (!name)
+ return svn_error_createf (SVN_ERR_WC_INVALID_LOCK, 0, NULL, pool,
+ "lock path is not a child (%s)",
+ path);
+
+ if (parent_access->children)
+ *adm_access = apr_hash_get (parent_access->children, path,
+ APR_HASH_KEY_STRING);
+ else
+ *adm_access = NULL;
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_wc_adm_open (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *parent_access,
const char *path,
svn_boolean_t write_lock,
+ svn_boolean_t tree_lock,
apr_pool_t *pool)
{
- svn_wc_adm_access_t *lock = apr_palloc (pool, sizeof (**adm_access));
- lock->lock_exists = FALSE;
- lock->path = apr_pstrdup (pool, path);
- lock->pool = pool;
+ svn_wc_adm_access_t *lock;
+ if (parent_access)
+ {
+ SVN_ERR (svn_wc__adm_access_child (&lock, parent_access, path,
+ pool));
+ if (lock)
+ /* Already locked */
+ return svn_error_createf (SVN_ERR_WC_LOCKED, 0, NULL, pool,
+ "directory already locked (%s)",
+ path);
+ }
+
+ /* Need to create a new lock */
if (write_lock)
{
- lock->type = svn_wc_adm_access_write_lock;
+ lock = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock,
+ parent_access, path, pool);
SVN_ERR (svn_wc__lock (lock, 0, pool));
lock->lock_exists = TRUE;
}
else
{
- /* ### Read-lock not yet implemented */
- lock->type = svn_wc_adm_access_unlocked;
+ /* ### Read-lock not yet implemented. Since no physical lock gets
+ ### created we must check PATH is not a file. */
+ enum svn_node_kind node_kind;
+ SVN_ERR (svn_io_check_path (path, &node_kind, pool));
+ if (node_kind != svn_node_dir)
+ return svn_error_createf (SVN_ERR_WC_INVALID_LOCK, 0, NULL, pool,
+ "lock path is not a directory (%s)",
+ path);
+
+ lock = svn_wc__adm_access_alloc (svn_wc__adm_access_unlocked,
+ parent_access, path, pool);
+ }
+
+ lock->parent = parent_access;
+ if (lock->parent)
+ {
+ if (! lock->parent->children)
+ lock->parent->children = apr_hash_make (lock->parent->pool);
+
+#if 0
+ /* ### Need to think about this. Creating an empty APR hash allocates
+ ### several hundred bytes. Since the hash key is the complete
+ ### path, children for different parents cannot clash. Can we save
+ ### memory by reusing the parent's hash? */
+ lock->children = lock->parent->children;
+#endif
+
+ apr_hash_set (lock->parent->children, lock->path, APR_HASH_KEY_STRING,
+ lock);
+ }
+
+ if (tree_lock)
+ {
+ /* ### Could use this code to initialise the cache, in which case the
+ ### subpool should be removed. */
+ apr_hash_t *entries;
+ apr_hash_index_t *hi;
+ apr_pool_t *subpool = svn_pool_create (pool);
+ SVN_ERR (svn_wc_entries_read (&entries, path, FALSE, subpool));
+ for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi))
+ {
+ void *val;
+ svn_wc_entry_t *entry;
+ svn_wc_adm_access_t *entry_access;
+ const char *entry_path;
+ svn_error_t *svn_err;
+
+ apr_hash_this (hi, NULL, NULL, &val);
+ entry = val;
+ if (entry->kind != svn_node_dir
+ || ! strcmp (entry->name, SVN_WC_ENTRY_THIS_DIR))
+ continue;
+ entry_path = svn_path_join (lock->path, entry->name, subpool);
+
+ /* Use the main lock's pool here, it needs to persist */
+ svn_err = svn_wc_adm_open (&entry_access, lock, entry_path,
+ write_lock, tree_lock, lock->pool);
+ if (svn_err)
+ {
+ /* Give up any locks we have acquired. */
+ svn_wc_adm_close (lock);
+ return svn_err;
+ }
+ }
+ svn_pool_destroy (subpool);
}
*adm_access = lock;
@@ -113,26 +264,73 @@
}
svn_error_t *
-svn_wc_adm_close (svn_wc_adm_access_t *adm_access)
+svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access,
+ svn_wc_adm_access_t *parent_access,
+ const char *path,
+ apr_pool_t *pool)
{
- if (adm_access->type == svn_wc_adm_access_write_lock)
+ SVN_ERR (svn_wc__adm_access_child (adm_access, parent_access, path, pool));
+ if (! *adm_access)
+ return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, 0, NULL, pool,
+ "directory not locked (%s)",
+ path);
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+svn_wc__do_adm_close (svn_wc_adm_access_t *adm_access,
+ svn_boolean_t preserve_lock)
+{
+ apr_hash_index_t *hi;
+
+ apr_pool_cleanup_kill (adm_access->pool, adm_access,
+ svn_wc__adm_access_pool_cleanup);
+
+ /* Close children */
+ if (adm_access->children)
{
- if (adm_access->lock_exists)
+ for (hi = apr_hash_first (adm_access->pool, adm_access->children);
+ hi;
+ hi = apr_hash_next (hi))
+ {
+ void *val;
+ svn_wc_adm_access_t *child_access;
+ apr_hash_this (hi, NULL, NULL, &val);
+ child_access = val;
+ SVN_ERR (svn_wc__do_adm_close (child_access, preserve_lock));
+ }
+ }
+
+ /* Physically unlock if required */
+ if (adm_access->type == svn_wc__adm_access_write_lock)
+ {
+ if (adm_access->lock_exists && ! preserve_lock)
{
SVN_ERR (svn_wc__unlock (adm_access->path, adm_access->pool));
adm_access->lock_exists = FALSE;
}
/* Reset to prevent further use of the write lock. */
- adm_access->type = svn_wc_adm_access_unlocked;
+ adm_access->type = svn_wc__adm_access_closed;
}
+ /* Detach from parent */
+ if (adm_access->parent)
+ apr_hash_set (adm_access->parent->children, adm_access->path,
+ APR_HASH_KEY_STRING, NULL);
+
return SVN_NO_ERROR;
}
svn_error_t *
+svn_wc_adm_close (svn_wc_adm_access_t *adm_access)
+{
+ return svn_wc__do_adm_close (adm_access, FALSE);
+}
+
+svn_error_t *
svn_wc_adm_write_check (svn_wc_adm_access_t *adm_access)
{
- if (adm_access->type == svn_wc_adm_access_write_lock)
+ if (adm_access->type == svn_wc__adm_access_write_lock)
{
if (adm_access->lock_exists)
{
diff --git a/subversion/libsvn_wc/log.c b/subversion/libsvn_wc/log.c
index eecb831..f5d1ece 100644
--- a/subversion/libsvn_wc/log.c
+++ b/subversion/libsvn_wc/log.c
@@ -601,7 +601,8 @@
if (entry->kind == svn_node_dir)
{
svn_wc_adm_access_t *adm_access;
- SVN_ERR (svn_wc_adm_open (&adm_access, full_path, TRUE, loggy->pool));
+ SVN_ERR (svn_wc_adm_retrieve (&adm_access, loggy->adm_access, full_path,
+ loggy->pool));
err = svn_wc_remove_from_revision_control (adm_access,
SVN_WC_ENTRY_THIS_DIR,
TRUE, loggy->pool);
@@ -795,15 +796,13 @@
{
pdir = svn_path_join (loggy->adm_access->path, key, pool);
base_name = SVN_WC_ENTRY_THIS_DIR;
- SVN_ERR (svn_wc_adm_open (&entry_access, pdir, TRUE, pool));
+ SVN_ERR (svn_wc_adm_retrieve (&entry_access, loggy->adm_access,
+ pdir, pool));
}
if (base_name)
SVN_ERR (svn_wc_remove_from_revision_control
(entry_access, base_name, FALSE, pool));
-
- if (cur_entry->kind == svn_node_dir)
- SVN_ERR (svn_wc_adm_close (entry_access));
}
}
diff --git a/subversion/libsvn_wc/props.c b/subversion/libsvn_wc/props.c
index a8b77a8..919c08d 100644
--- a/subversion/libsvn_wc/props.c
+++ b/subversion/libsvn_wc/props.c
@@ -393,7 +393,7 @@
return SVN_NO_ERROR; /* ### svn_node_none or svn_node_unknown */
}
- SVN_ERR (svn_wc_adm_open (&adm_access, parent, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent, TRUE, FALSE, pool));
SVN_ERR (svn_wc__open_adm_file (&log_fp, parent, SVN_WC__ADM_LOG,
(APR_WRITE | APR_CREATE), /* not excl */
pool));
diff --git a/subversion/libsvn_wc/update_editor.c b/subversion/libsvn_wc/update_editor.c
index fadd4fb..af926d7 100644
--- a/subversion/libsvn_wc/update_editor.c
+++ b/subversion/libsvn_wc/update_editor.c
@@ -492,7 +492,7 @@
svn_wc_adm_access_t *adm_access;
svn_stringbuf_t *log_item = svn_stringbuf_create ("", pool);
- SVN_ERR (svn_wc_adm_open (&adm_access, pb->path, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, pb->path, TRUE, TRUE, pool));
SVN_ERR (svn_wc__open_adm_file (&log_fp,
pb->path,
SVN_WC__ADM_LOG,
@@ -758,7 +758,8 @@
svn_stringbuf_t *entry_accum = svn_stringbuf_create ("", db->pool);
/* Lock down the administrative area */
- SVN_ERR (svn_wc_adm_open (&adm_access, db->path, TRUE, db->pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, db->path, TRUE, FALSE,
+ db->pool));
/* Open log file */
SVN_ERR (svn_wc__open_adm_file (&log_fp,
@@ -1192,7 +1193,7 @@
/* Lock the parent directory while we change things. If for some
reason the parent isn't under version control, this function will
bomb out. */
- SVN_ERR (svn_wc_adm_open (&adm_access, parent_dir, TRUE, pool));
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent_dir, TRUE, FALSE, pool));
/*
When this function is called on file F, we assume the following
diff --git a/subversion/libsvn_wc/wc.h b/subversion/libsvn_wc/wc.h
index 17e4a1b..c087a74 100644
--- a/subversion/libsvn_wc/wc.h
+++ b/subversion/libsvn_wc/wc.h
@@ -142,11 +142,11 @@
/* PATH to directory which contains the administrative area */
const char *path;
- enum svn_wc_adm_access_type {
+ enum svn_wc__adm_access_type {
- /* SVN_WC_ADM_ACCESS_UNLOCKED indicates no lock is held allowing
+ /* SVN_WC__ADM_ACCESS_UNLOCKED indicates no lock is held allowing
read-only access without cacheing. */
- svn_wc_adm_access_unlocked,
+ svn_wc__adm_access_unlocked,
#if 0 /* How cacheing might work one day */
@@ -158,14 +158,18 @@
### area (consider running svn_wc_status on some other user's
### working copy). */
- /* SVN_WC_ADM_ACCESS_READ_LOCK indicates that read-only access and
+ /* SVN_WC__ADM_ACCESS_READ_LOCK indicates that read-only access and
cacheing are allowed. */
- svn_wc_adm_access_read_lock,
+ svn_wc__adm_access_read_lock,
#endif
- /* SVN_WC_ADM_ACCESS_WRITE_LOCK indicates that read-write access and
+ /* SVN_WC__ADM_ACCESS_WRITE_LOCK indicates that read-write access and
cacheing are allowed. */
- svn_wc_adm_access_write_lock
+ svn_wc__adm_access_write_lock,
+
+ /* SVN_WC__ADM_ACCESS_CLOSED indicates that the baton has been
+ closed. */
+ svn_wc__adm_access_closed
} type;
@@ -184,6 +188,13 @@
apr_hash_t *entries;
#endif
+ /* PARENT access baton, may be NULL. */
+ svn_wc_adm_access_t *parent;
+
+ /* CHILDREN is a hash of svn_wc_adm_access_t* keyed on char*
+ representing the path to sub-directories that are also locked. */
+ apr_hash_t *children;
+
/* POOL is used to allocate cached items, they need to persist for the
lifetime of this access baton */
apr_pool_t *pool;
diff --git a/subversion/tests/clients/cmdline/copy_tests.py b/subversion/tests/clients/cmdline/copy_tests.py
index 0541799..f3569e7 100755
--- a/subversion/tests/clients/cmdline/copy_tests.py
+++ b/subversion/tests/clients/cmdline/copy_tests.py
@@ -470,6 +470,39 @@
return 0
+# Takes out working-copy locks for A/B2 and child A/B2/E. At one stage
+# during issue 749 the second lock cause an already-locked error.
+def copy_modify_commit(sbox):
+ "copy a directory hierarchy and modify before commit"
+
+ if sbox.build():
+ return 1
+
+ wc_dir = sbox.wc_dir
+ outlines, errlines = svntest.main.run_svn(None, 'cp',
+ wc_dir + '/A/B', wc_dir + '/A/B2',
+ '-m', 'fooogle')
+ if errlines:
+ print "Whoa, failed to copy A/B to A/B2"
+ return 1
+
+ alpha_path = os.path.join(wc_dir, 'A', 'B2', 'E', 'alpha')
+ svntest.main.file_append(alpha_path, "modified alpha")
+
+ expected_output = svntest.wc.State(wc_dir, {
+ 'A/B2' : Item(verb='Adding'),
+ 'A/B2/E/alpha' : Item(verb='Sending'),
+ })
+
+ if svntest.actions.run_and_verify_commit (wc_dir,
+ expected_output,
+ None,
+ None,
+ None, None,
+ None, None,
+ wc_dir):
+ return 1
+
########################################################################
# Run the tests
@@ -481,6 +514,7 @@
receive_copy_in_update,
resurrect_deleted_dir,
no_copy_overwrites,
+ copy_modify_commit,
]
if __name__ == '__main__':
