Continuing issue #749. Make locks associate together in groups
instead of in parent-child hierarchies. Parent-child is still used
when closing. Directory locking is now separate from committable
harvesting.
* subversion/include/svn_wc.h
(svn_wc_adm_open, svn_wc_adm_retrieve, svn_wc_adm_close): Document
new behavior.
* subversion/libsvn_wc/wc.h
(struct svn_wc_adm_access_t): Remove parent member. Rename children
member to set.
* subversion/libsvn_wc/lock.c
(svn_wc__adm_access_child): Deleted.
(svn_wc__adm_access_alloc): Remove parent parameter and change all
callers.
(svn_wc_adm_open): Add the associated access baton to the hash after
creating the hash. Use temporary hash for opening children.
(svn_wc_adm_retrieve): Don't call svn_wc__adm_access_child.
(svn_wc__do_adm_close): Copy children to array before closing them.
* subversion/libsvn_client/client.h
(svn_client__harvest_committables, svn_client__get_copy_committables):
Remove locked_dirs parameter.
* subversion/libsvn_client/commit_util.c
(lock_dir): Deleted.
(harvest_committables): Revert 2554 for this file. Also remove locked_dirs
parameter, and remove all directory locking.
(svn_client__harvest_committables, svn_client__get_copy_committables):
Remove locked_dirs parameter and directory locking.
* subversion/libsvn_client/commit.c
(unlock_dirs): Deleted.
(svn_client_commit): Remove locked_dirs. Open an access baton.
Close the baton intead of calling unlock_dirs.
* subversion/libsvn_client/copy.c
(unlock_dirs): Deleted.
(wc_to_repos_copy): Remove locked_dirs. Open an access baton.
Close the baton intead of calling unlock_dirs.
git-svn-id: https://svn.apache.org/repos/asf/subversion/branches/issue-749-dev@842699 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/subversion/include/svn_wc.h b/subversion/include/svn_wc.h
index b58df44..3dab4a9 100644
--- a/subversion/include/svn_wc.h
+++ b/subversion/include/svn_wc.h
@@ -53,66 +53,67 @@
/*** Locking/Opening/Closing ***/
/* Baton for access to working copy administrative area. One day all such
- access will require a baton, we're not there yet. */
+ access will require a baton, we're not there yet.
+
+ Access batons can be grouped into sets, by passing an existing open
+ baton when opening a new baton. Given one baton in a set, other batons
+ may be retrieved. This allows an entire hierarchy to be locked, and
+ then the set of batons can be passed around by passing a single baton.
+ */
typedef struct svn_wc_adm_access_t svn_wc_adm_access_t;
/* 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.
+ access. If PATH refers to a directory that is already locked then the
+ error SVN_ERR_WC_LOCKED will be returned.
- 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 ASSOCIATED is an open access baton then ADM_ACCESS will be added to
+ the set containing ASSOCIATED. ASSOCIATED can be NULL, in which case
+ ADM_ACCESS is the start of a new set.
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.
+ PATH will be locked. All the access batons will become part of the set
+ containing ADM_ACCESS. This is an all-or-nothing option, if it is not
+ possible to lock the entire tree then an error will be returned and
+ ADM_ACCESS will be invalid.
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. */
+ cleared, it will be closed automatically at that point, and removed from
+ its set. A baton closed in this way will not remove physical locks from
+ the working copy. */
svn_error_t *svn_wc_adm_open (svn_wc_adm_access_t **adm_access,
- svn_wc_adm_access_t *parent_access,
+ svn_wc_adm_access_t *associated,
const char *path,
svn_boolean_t write_lock,
svn_boolean_t tree_lock,
apr_pool_t *pool);
-/* 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.
+/* Return an access baton in ADM_ACCESS associated with PATH. PATH must be
+ a directory that is locked and part of the set containing the ASSOCIATED
+ access baton.
- ### I can't think of a good name for this function. */
+ POOL is used only for local processing it is not used for the batons. */
svn_error_t *svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access,
- svn_wc_adm_access_t *parent_access,
+ svn_wc_adm_access_t *associated,
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. */
+
+/* Give up the access baton ADM_ACCESS, and its lock if any. This will
+ recursively close any batons in the same set that are subdirectories of
+ ADM_ACCESS. 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 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);
+
/* Set *LOCKED to non-zero if PATH is locked, else set it to zero. */
svn_error_t *svn_wc_locked (svn_boolean_t *locked,
const char *path,
diff --git a/subversion/libsvn_client/client.h b/subversion/libsvn_client/client.h
index c7a1eb5..b93c5f9 100644
--- a/subversion/libsvn_client/client.h
+++ b/subversion/libsvn_client/client.h
@@ -329,7 +329,6 @@
found in TARGETS will not be crawled for modifications. */
svn_error_t *
svn_client__harvest_committables (apr_hash_t **committables,
- apr_hash_t **locked_dirs,
const char *parent_dir,
apr_array_header_t *targets,
svn_boolean_t nonrecursive,
@@ -344,7 +343,6 @@
to a new repository URL (NEW_URL). */
svn_error_t *
svn_client__get_copy_committables (apr_hash_t **committables,
- apr_hash_t **locked_dirs,
const char *new_url,
const char *target,
apr_pool_t *pool);
diff --git a/subversion/libsvn_client/commit.c b/subversion/libsvn_client/commit.c
index 87a7bc8..5f35078 100644
--- a/subversion/libsvn_client/commit.c
+++ b/subversion/libsvn_client/commit.c
@@ -605,32 +605,6 @@
static svn_error_t *
-unlock_dirs (apr_hash_t *locked_dirs,
- apr_pool_t *pool)
-{
- apr_hash_index_t *hi;
-
- /* Split if there's nothing to be done. */
- if (! locked_dirs)
- return SVN_NO_ERROR;
-
- /* Clean up any locks. */
- for (hi = apr_hash_first (pool, locked_dirs); hi; hi = apr_hash_next (hi))
- {
- const void *key;
- void *val;
- svn_wc_adm_access_t *adm_access;
-
- apr_hash_this (hi, &key, NULL, &val);
- adm_access = val;
- SVN_ERR (svn_wc_adm_close (adm_access));
- }
-
- return SVN_NO_ERROR;
-}
-
-
-static svn_error_t *
remove_tmpfiles (apr_hash_t *tempfiles,
apr_pool_t *pool)
{
@@ -747,7 +721,8 @@
const char *base_dir;
const char *base_url;
apr_array_header_t *rel_targets;
- apr_hash_t *committables, *locked_dirs, *tempfiles = NULL;
+ apr_hash_t *committables, *tempfiles = NULL;
+ svn_wc_adm_access_t *base_dir_access;
apr_array_header_t *commit_items;
apr_status_t apr_err = 0;
apr_file_t *xml_hnd = NULL;
@@ -755,6 +730,7 @@
svn_error_t *bump_err = NULL, *cleanup_err = NULL;
svn_boolean_t use_xml = (xml_dst && xml_dst[0]) ? TRUE : FALSE;
svn_boolean_t commit_in_progress = FALSE;
+ svn_wc_entry_t *base_entry;
const char *display_dir = ".";
int notify_path_offset;
int i;
@@ -762,6 +738,21 @@
/* Condense the target list. */
SVN_ERR (svn_path_condense_targets (&base_dir, &rel_targets, targets, pool));
+ /* Eeek! To get an access baton we need a directory name. If the user
+ explicitly commits a deleted file that has been physically removed
+ from the working copy (as is normal for deleted files), then base_dir
+ will be the file name not a directory name. To identify this case we
+ need to get the entry for base_dir before we get the access baton. I
+ wonder where this logic should live? */
+ SVN_ERR (svn_wc_entry (&base_entry, base_dir, TRUE, pool));
+ if (base_entry->kind == svn_node_dir)
+ SVN_ERR (svn_wc_adm_open (&base_dir_access, NULL, base_dir, TRUE, TRUE,
+ pool));
+ else
+ SVN_ERR (svn_wc_adm_open (&base_dir_access, NULL,
+ svn_path_remove_component_nts (base_dir, pool),
+ TRUE, TRUE, pool));
+
/* If we calculated only a base_dir and no relative targets, this
must mean that we are being asked to commit a single directory.
In order to do this properly, we need to anchor our commit up one
@@ -790,7 +781,6 @@
/* Crawl the working copy for commit items. */
if ((cmt_err = svn_client__harvest_committables (&committables,
- &locked_dirs,
base_dir,
rel_targets,
nonrecursive,
@@ -917,13 +907,9 @@
adm_access_path = item->path;
else
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)
- /* This should *never* happen */
- return svn_error_createf(SVN_ERR_GENERAL, 0, NULL, pool,
- "BUG: no lock for %s",
- item->path);
+ if ((bump_err = svn_wc_adm_retrieve (&adm_access, base_dir_access,
+ adm_access_path, pool)))
+ goto cleanup;
if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD)
&& (item->kind == svn_node_dir)
@@ -948,10 +934,6 @@
svn_pool_destroy (subpool);
}
- /* Unlock the locked directories. */
- if (! ((unlock_err = unlock_dirs (locked_dirs, pool))))
- locked_dirs = NULL;
-
/* If we were committing into XML, close the xml file. */
if (use_xml)
{
@@ -980,9 +962,8 @@
if (commit_in_progress)
editor->abort_edit (edit_baton); /* ignore return value */
- /* Unlock any remaining locked dirs. */
- if (locked_dirs)
- unlock_err = unlock_dirs (locked_dirs, pool);
+ /* ### Under what conditions should we remove the locks? */
+ unlock_err = svn_wc_adm_close (base_dir_access);
/* Remove any outstanding temporary text-base files. */
cleanup_err = remove_tmpfiles (tempfiles, pool);
diff --git a/subversion/libsvn_client/commit_util.c b/subversion/libsvn_client/commit_util.c
index 05fbb73..f9c6a7d 100644
--- a/subversion/libsvn_client/commit_util.c
+++ b/subversion/libsvn_client/commit_util.c
@@ -47,35 +47,6 @@
/*** Harvesting Commit Candidates ***/
-/* If DIR isn't already in the LOCKED_DIRS hash, attempt to lock it.
- If the lock is successful, add DIR to the LOCKED_DIRS hash. Use
- the hash's pool for adding new items, and POOL for any other
- allocations. */
-static svn_error_t *
-lock_dir (apr_hash_t *locked_dirs,
- const char *dir,
- apr_pool_t *pool)
-{
- apr_pool_t *hash_pool = apr_hash_pool_get (locked_dirs);
-
- if (! apr_hash_get (locked_dirs, dir, APR_HASH_KEY_STRING))
- {
- /* 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);
- }
- return SVN_NO_ERROR;
-}
-
-
/* Add a new commit candidate (described by all parameters except
`COMMITTABLES') to the COMMITABLES hash. */
static void
@@ -137,7 +108,6 @@
added with history as URL. */
static svn_error_t *
harvest_committables (apr_hash_t *committables,
- apr_hash_t *locked_dirs,
const char *path,
const char *url,
const char *copyfrom_url,
@@ -146,7 +116,6 @@
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? */
@@ -299,37 +268,25 @@
/* Now, if this is something to commit, add it to our list. */
if (state_flags)
{
- /* 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));
-
/* Finally, add the committable item. */
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;
}
- if (entries)
+ /* 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)))
{
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.
-
- ### 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. */
+ "this dir" entry. */
for (hi = apr_hash_first (subpool, entries);
hi;
hi = apr_hash_next (hi))
@@ -372,16 +329,6 @@
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. */
@@ -393,7 +340,7 @@
/* Recurse. */
SVN_ERR (harvest_committables
- (committables, locked_dirs, full_path,
+ (committables, full_path,
used_url ? used_url : this_entry->url,
this_cf_url,
(svn_wc_entry_t *)val,
@@ -401,7 +348,6 @@
adds_only,
copy_mode,
FALSE,
- lock_subdirs,
subpool));
svn_pool_clear (loop_pool);
@@ -416,7 +362,6 @@
svn_error_t *
svn_client__harvest_committables (apr_hash_t **committables,
- apr_hash_t **locked_dirs,
const char *parent_dir,
apr_array_header_t *targets,
svn_boolean_t nonrecursive,
@@ -427,9 +372,6 @@
/* Create the COMMITTABLES hash. */
*committables = apr_hash_make (pool);
- /* Create the LOCKED_DIRS hash. */
- *locked_dirs = apr_hash_make (pool);
-
/* ### Would be nice to use an iteration pool here, but need to
first look into lifetime issues w/ anything passed to
harvest_committables() and possibly stored by it. */
@@ -516,9 +458,9 @@
target);
/* Handle our TARGET. */
- SVN_ERR (harvest_committables (*committables, *locked_dirs, target,
+ SVN_ERR (harvest_committables (*committables, target,
url, NULL, entry, NULL, FALSE, FALSE,
- nonrecursive, FALSE, pool));
+ nonrecursive, pool));
i++;
}
@@ -530,7 +472,6 @@
svn_error_t *
svn_client__get_copy_committables (apr_hash_t **committables,
- apr_hash_t **locked_dirs,
const char *new_url,
const char *target,
apr_pool_t *pool)
@@ -540,9 +481,6 @@
/* Create the COMMITTABLES hash. */
*committables = apr_hash_make (pool);
- /* Create the LOCKED_DIRS hash. */
- *locked_dirs = apr_hash_make (pool);
-
/* Read the entry for TARGET. */
SVN_ERR (svn_wc_entry (&entry, target, FALSE, pool));
if (! entry)
@@ -550,9 +488,9 @@
(SVN_ERR_ENTRY_NOT_FOUND, 0, NULL, pool, target);
/* Handle our TARGET. */
- SVN_ERR (harvest_committables (*committables, *locked_dirs, target,
+ SVN_ERR (harvest_committables (*committables, target,
new_url, entry->url, entry, NULL,
- FALSE, TRUE, FALSE, FALSE, pool));
+ FALSE, TRUE, FALSE, pool));
return SVN_NO_ERROR;
}
diff --git a/subversion/libsvn_client/copy.c b/subversion/libsvn_client/copy.c
index 437a0fb..dc4eaeb 100644
--- a/subversion/libsvn_client/copy.c
+++ b/subversion/libsvn_client/copy.c
@@ -362,32 +362,6 @@
static svn_error_t *
-unlock_dirs (apr_hash_t *locked_dirs,
- apr_pool_t *pool)
-{
- apr_hash_index_t *hi;
-
- /* Split if there's nothing to be done. */
- if (! locked_dirs)
- return SVN_NO_ERROR;
-
- /* Clean up any locks. */
- for (hi = apr_hash_first (pool, locked_dirs); hi; hi = apr_hash_next (hi))
- {
- const void *key;
- void *val;
- svn_wc_adm_access_t *adm_access;
-
- apr_hash_this (hi, &key, NULL, &val);
- adm_access = val;
- SVN_ERR (svn_wc_adm_close (adm_access));
- }
-
- return SVN_NO_ERROR;
-}
-
-
-static svn_error_t *
remove_tmpfiles (apr_hash_t *tempfiles,
apr_pool_t *pool)
{
@@ -489,7 +463,8 @@
svn_revnum_t committed_rev = SVN_INVALID_REVNUM;
const char *committed_date = NULL;
const char *committed_author = NULL;
- apr_hash_t *committables, *locked_dirs, *tempfiles = NULL;
+ apr_hash_t *committables, *tempfiles = NULL;
+ svn_wc_adm_access_t *adm_access;
apr_array_header_t *commit_items;
svn_error_t *cmt_err = NULL, *unlock_err = NULL, *cleanup_err = NULL;
svn_boolean_t commit_in_progress = FALSE;
@@ -504,6 +479,9 @@
if (svn_path_is_empty_nts (parent))
parent = ".";
+ /* ### Do we need locks for a wc->repos copy? */
+ SVN_ERR (svn_wc_adm_open (&adm_access, NULL, parent, TRUE, TRUE, pool));
+
/* Split the DST_URL into an anchor and target. */
svn_path_split_nts (dst_url, &anchor, &target, pool);
@@ -551,7 +529,6 @@
/* Crawl the working copy for commit items. */
if ((cmt_err = svn_client__get_copy_committables (&committables,
- &locked_dirs,
base_url,
base_path,
pool)))
@@ -608,9 +585,8 @@
if (session)
ra_lib->close (session);
- /* Unlock any remaining locked dirs. */
- if (locked_dirs)
- unlock_err = unlock_dirs (locked_dirs, pool);
+ /* ### Under what conditions should we remove the locks? */
+ unlock_err = svn_wc_adm_close (adm_access);
/* Remove any outstanding temporary text-base files. */
if (tempfiles)
diff --git a/subversion/libsvn_wc/lock.c b/subversion/libsvn_wc/lock.c
index 668e389..960d479 100644
--- a/subversion/libsvn_wc/lock.c
+++ b/subversion/libsvn_wc/lock.c
@@ -93,14 +93,12 @@
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->set = NULL;
lock->lock_exists = FALSE;
/* ### Some places lock with a path that is not canonical, we need
### cannonical paths for reliable parent-child determination */
@@ -120,8 +118,7 @@
{
svn_error_t *err;
svn_wc_adm_access_t *lock
- = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, NULL, path,
- pool);
+ = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, path, pool);
err = svn_wc__lock (lock, 0, pool);
if (err)
@@ -137,33 +134,9 @@
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,
+ svn_wc_adm_access_t *associated,
const char *path,
svn_boolean_t write_lock,
svn_boolean_t tree_lock,
@@ -171,12 +144,14 @@
{
svn_wc_adm_access_t *lock;
- if (parent_access)
+ if (associated && associated->set)
{
- SVN_ERR (svn_wc__adm_access_child (&lock, parent_access, path,
- pool));
+ lock = apr_hash_get (associated->set, path, APR_HASH_KEY_STRING);
if (lock)
- /* Already locked */
+ /* 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. */
return svn_error_createf (SVN_ERR_WC_LOCKED, 0, NULL, pool,
"directory already locked (%s)",
path);
@@ -185,8 +160,8 @@
/* Need to create a new lock */
if (write_lock)
{
- lock = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock,
- parent_access, path, pool);
+ lock = svn_wc__adm_access_alloc (svn_wc__adm_access_write_lock, path,
+ pool);
SVN_ERR (svn_wc__lock (lock, 0, pool));
lock->lock_exists = TRUE;
}
@@ -201,36 +176,36 @@
"lock path is not a directory (%s)",
path);
- lock = svn_wc__adm_access_alloc (svn_wc__adm_access_unlocked,
- parent_access, path, pool);
+ lock = svn_wc__adm_access_alloc (svn_wc__adm_access_unlocked, path, pool);
}
- lock->parent = parent_access;
- if (lock->parent)
+ if (associated)
{
- if (! lock->parent->children)
- lock->parent->children = apr_hash_make (lock->parent->pool);
+ if (! associated->set)
+ {
+ associated->set = apr_hash_make (associated->pool);
+ apr_hash_set (associated->set, associated->path, APR_HASH_KEY_STRING,
+ associated);
+ }
-#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);
+ lock->set = associated->set;
+ apr_hash_set (lock->set, 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. */
+ /* ### Use this code to initialise the cache? */
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));
+
+ /* Use a temporary hash until all children have been opened. */
+ if (associated)
+ lock->set = apr_hash_make (subpool);
+
+ /* Open the tree */
for (hi = apr_hash_first (subpool, entries); hi; hi = apr_hash_next (hi))
{
void *val;
@@ -246,16 +221,39 @@
continue;
entry_path = svn_path_join (lock->path, entry->name, subpool);
- /* Use the main lock's pool here, it needs to persist */
+ /* Don't use the subpool pool here, the lock 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. */
+ /* This closes all the children in temporary hash as well */
svn_wc_adm_close (lock);
+ svn_pool_destroy (subpool);
return svn_err;
}
}
+
+ /* Switch from temporary hash to permanent hash */
+ if (associated)
+ {
+ for (hi = apr_hash_first (subpool, lock->set);
+ hi;
+ hi = apr_hash_next (hi))
+ {
+ const void *key;
+ void *val;
+ const char *entry_path;
+ svn_wc_adm_access_t *entry_access;
+
+ apr_hash_this (hi, &key, NULL, &val);
+ entry_path = key;
+ entry_access = val;
+ apr_hash_set (associated->set, entry_path, APR_HASH_KEY_STRING,
+ entry_access);
+ entry_access->set = associated->set;
+ }
+ lock->set = associated->set;
+ }
svn_pool_destroy (subpool);
}
@@ -265,11 +263,16 @@
svn_error_t *
svn_wc_adm_retrieve (svn_wc_adm_access_t **adm_access,
- svn_wc_adm_access_t *parent_access,
+ svn_wc_adm_access_t *associated,
const char *path,
apr_pool_t *pool)
{
- SVN_ERR (svn_wc__adm_access_child (adm_access, parent_access, path, pool));
+ if (associated->set)
+ *adm_access = apr_hash_get (associated->set, path, APR_HASH_KEY_STRING);
+ else if (! strcmp (associated->path, path))
+ *adm_access = associated;
+ else
+ *adm_access = NULL;
if (! *adm_access)
return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, 0, NULL, pool,
"directory not locked (%s)",
@@ -287,17 +290,39 @@
svn_wc__adm_access_pool_cleanup);
/* Close children */
- if (adm_access->children)
+ if (adm_access->set)
{
- for (hi = apr_hash_first (adm_access->pool, adm_access->children);
+ /* The documentation says that modifying a hash while iterating over
+ it is allowed but unpredictable! So, first loop to identify and
+ copy children, second loop to close them. */
+ int i;
+ apr_array_header_t *children = apr_array_make (adm_access->pool, 1,
+ sizeof (adm_access));
+
+ for (hi = apr_hash_first (adm_access->pool, adm_access->set);
hi;
hi = apr_hash_next (hi))
{
void *val;
- svn_wc_adm_access_t *child_access;
+ svn_wc_adm_access_t *associated;
+ const char *name;
apr_hash_this (hi, NULL, NULL, &val);
- child_access = val;
- SVN_ERR (svn_wc__do_adm_close (child_access, preserve_lock));
+ associated = val;
+ name = svn_path_is_child (adm_access->path, associated->path,
+ adm_access->pool);
+ if (name)
+ {
+ *(svn_wc_adm_access_t**)apr_array_push (children) = associated;
+ /* Deleting current element is allowed and predictable */
+ apr_hash_set (adm_access->set, associated->path,
+ APR_HASH_KEY_STRING, NULL);
+ }
+ }
+ for (i = 0; i < children->nelts; ++i)
+ {
+ svn_wc_adm_access_t *child = APR_ARRAY_IDX(children, i,
+ svn_wc_adm_access_t*);
+ SVN_ERR (svn_wc__do_adm_close (child, preserve_lock));
}
}
@@ -313,10 +338,9 @@
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);
+ /* Detach from set */
+ if (adm_access->set)
+ apr_hash_set (adm_access->set, adm_access->path, APR_HASH_KEY_STRING, NULL);
return SVN_NO_ERROR;
}
diff --git a/subversion/libsvn_wc/wc.h b/subversion/libsvn_wc/wc.h
index c087a74..9f7300f 100644
--- a/subversion/libsvn_wc/wc.h
+++ b/subversion/libsvn_wc/wc.h
@@ -188,12 +188,9 @@
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;
+ /* SET is a hash of svn_wc_adm_access_t* keyed on char* representing the
+ path to other directories that are also locked. */
+ apr_hash_t *set;
/* POOL is used to allocate cached items, they need to persist for the
lifetime of this access baton */
