You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2002/07/14 00:31:30 UTC
[PATCH] Issue 749 Parent-child locks
Hello
This patch implements a parent-child lock relationship. It passes
the regression tests, including Justin's new one. I'm posting it to
ask if this is the right way to go. One thing that worries me is the
memory usage, it's probably trivial at the moment, but if we start to
cache the entries will it prove to be too expensive?
Since my visitors' baby has stopped crying I'm going to try to get
some sleep now :)
Continuing issue #749. Make svn_wc_adm_access_t optionally
hierarchical.
Also revert r2504 as it's no longer needed.
* subversion/include/svn_error_codes.h (SVN_ERR_WC_INVALID_LOCK): Added.
* subversion/include/svn_wc.h
(svn_wc_adm_open): Add parent parameter.
(svn_wc_adm_close): Document new behavior.
* subversion/libsvn_wc/adm_ops.c
(svn_wc_process_committed): Pass parent to svn_wc_adm_open. Allocate
child batons from parent's pool and don't explicitly close them.
(svn_wc_delete, svn_wc_revert, svn_wc_remove_from_revision_control):
Pass parent to svn_wc_adm_open.
* subversion/libsvn_wc/log.c (log_do_delete_entry): Pass parent to
svn_wc_adm_open.
* subversion/libsvn_wc/props.c (svn_wc_merge_prop_diffs): Pass NULL
parent to svn_wc_adm_open.
* subversion/libsvn_wc/adm_files.c (init_adm): Pass NULL parent 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): Pass parent to svn_wc_adm_open.
* subversion/libsvn_wc/update_editor.c (delete_entry, close_directory,
svn_wc_install_file): Pass NULL parent to svn_wc_adm_open.
* subversion/libsvn_client/externals.c (relegate_external,
handle_external_item_change): Pass NULL parent 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.
(struct svn_wc_adm_access_t): Added parent and children members.
* subversion/libsvn_wc/lock.c
(svn_wc__adm_access_alloc): New function.
(svn_wc__adm_steal_write_lock): Use svn_wc__adm_access_alloc.
(svn_wc_adm_open): Add parent parameter and processing. Use
svn_wc__adm_access_alloc. For read locks check path is a directory.
(svn_wc_adm_close): Close children.
(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.
Index: ./subversion/include/svn_error_codes.h
===================================================================
--- ./subversion/include/svn_error_codes.h
+++ ./subversion/include/svn_error_codes.h Sun Jul 14 00:32:26 2002
@@ -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")
Index: ./subversion/include/svn_wc.h
===================================================================
--- ./subversion/include/svn_wc.h
+++ ./subversion/include/svn_wc.h Sun Jul 14 00:32:26 2002
@@ -59,14 +59,34 @@
/* 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 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
+ and not yet explicitly closed, then the baton previously opened 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.
+
+ POOL will be used to allocate memory for the baton and any subsequently
+ cached items. If ADM_ACCESS is to be closed automatically by closing
+ PARENT_ACCESS then POOL must have a sufficient lifetime.*/
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,
apr_pool_t *pool);
-/* Give up the access baton ADM_ACCESS, and its lock if any */
+/* 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. */
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
Index: ./subversion/libsvn_wc/props.c
===================================================================
--- ./subversion/libsvn_wc/props.c
+++ ./subversion/libsvn_wc/props.c Sun Jul 14 00:32:26 2002
@@ -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, pool));
SVN_ERR (svn_wc__open_adm_file (&log_fp, parent, SVN_WC__ADM_LOG,
(APR_WRITE | APR_CREATE), /* not excl */
pool));
Index: ./subversion/libsvn_wc/wc.h
===================================================================
--- ./subversion/libsvn_wc/wc.h
+++ ./subversion/libsvn_wc/wc.h Sun Jul 14 00:32:26 2002
@@ -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
read-only access without cacheing. */
- svn_wc_adm_access_unlocked,
+ svn_wc__adm_access_unlocked,
#if 0 /* How cacheing might work one day */
@@ -160,12 +160,12 @@
/* 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
cacheing are allowed. */
- svn_wc_adm_access_write_lock
+ svn_wc__adm_access_write_lock
} type;
@@ -183,6 +183,13 @@
ENTRIES will be NULL. */
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 */
Index: ./subversion/libsvn_wc/log.c
===================================================================
--- ./subversion/libsvn_wc/log.c
+++ ./subversion/libsvn_wc/log.c Sun Jul 14 00:32:26 2002
@@ -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_open (&adm_access, loggy->adm_access, full_path, TRUE,
+ loggy->pool));
err = svn_wc_remove_from_revision_control (adm_access,
SVN_WC_ENTRY_THIS_DIR,
TRUE, loggy->pool);
@@ -795,7 +796,8 @@
{
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_open (&entry_access, loggy->adm_access, pdir,
+ TRUE, pool));
}
if (base_name)
Index: ./subversion/libsvn_wc/adm_ops.c
===================================================================
--- ./subversion/libsvn_wc/adm_ops.c
+++ ./subversion/libsvn_wc/adm_ops.c Sun Jul 14 00:57:16 2002
@@ -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_open (&child_access, adm_access, this_path,
+ TRUE, 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,7 @@
### 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, pool));
SVN_ERR (svn_wc_remove_from_revision_control
(adm_access, SVN_WC_ENTRY_THIS_DIR, FALSE, pool));
@@ -1201,7 +1198,7 @@
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, pool));
}
else if (entry->kind == svn_node_dir)
{
@@ -1211,7 +1208,7 @@
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, pool));
}
/* Remove the item from revision control. */
@@ -1507,8 +1504,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_open (&entry_access, adm_access, entrypath,
+ TRUE, adm_access->pool));
err = svn_wc_remove_from_revision_control (entry_access,
SVN_WC_ENTRY_THIS_DIR,
destroy_wf, subpool);
@@ -1519,7 +1516,6 @@
}
else if (err)
return err;
- SVN_ERR (svn_wc_adm_close (entry_access));
}
}
Index: ./subversion/libsvn_wc/adm_files.c
===================================================================
--- ./subversion/libsvn_wc/adm_files.c
+++ ./subversion/libsvn_wc/adm_files.c Sun Jul 14 00:32:26 2002
@@ -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, pool));
/** Make subdirectories. ***/
Index: ./subversion/libsvn_wc/update_editor.c
===================================================================
--- ./subversion/libsvn_wc/update_editor.c
+++ ./subversion/libsvn_wc/update_editor.c Sun Jul 14 00:32:26 2002
@@ -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, pool));
SVN_ERR (svn_wc__open_adm_file (&log_fp,
pb->path,
SVN_WC__ADM_LOG,
@@ -758,7 +758,7 @@
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, db->pool));
/* Open log file */
SVN_ERR (svn_wc__open_adm_file (&log_fp,
@@ -1192,7 +1192,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, pool));
/*
When this function is called on file F, we assume the following
Index: ./subversion/libsvn_wc/lock.c
===================================================================
--- ./subversion/libsvn_wc/lock.c
+++ ./subversion/libsvn_wc/lock.c Sun Jul 14 00:32:26 2002
@@ -59,17 +59,33 @@
return svn_wc__remove_adm_file (path, pool, SVN_WC__ADM_LOCK, NULL);
}
+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;
+ 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)
@@ -87,25 +103,68 @@
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,
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)
+ {
+ const char *name;
+
+ /* PATH must be a child. We only need the result, not the allocated
+ name. */
+ 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)
+ {
+ lock = apr_hash_get (parent_access->children, path,
+ APR_HASH_KEY_STRING);
+ if (lock)
+ {
+ *adm_access = lock;
+ return SVN_NO_ERROR;
+ }
+ }
+ }
+ /* 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);
+
+ apr_hash_set (lock->parent->children, lock->path, APR_HASH_KEY_STRING,
+ lock);
}
*adm_access = lock;
@@ -115,7 +174,25 @@
svn_error_t *
svn_wc_adm_close (svn_wc_adm_access_t *adm_access)
{
- if (adm_access->type == svn_wc_adm_access_write_lock)
+ apr_hash_index_t *hi;
+
+ /* Close children */
+ if (adm_access->children)
+ {
+ 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_adm_close (child_access));
+ }
+ }
+
+ /* Physically unlock if required */
+ if (adm_access->type == svn_wc__adm_access_write_lock)
{
if (adm_access->lock_exists)
{
@@ -123,16 +200,21 @@
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_unlocked;
}
+ /* 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_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)
{
Index: ./subversion/libsvn_client/externals.c
===================================================================
--- ./subversion/libsvn_client/externals.c
+++ ./subversion/libsvn_client/externals.c Sun Jul 14 00:32:26 2002
@@ -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, pool));
err = svn_wc_remove_from_revision_control (adm_access,
SVN_WC_ENTRY_THIS_DIR,
TRUE,
@@ -378,7 +378,7 @@
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, ib->pool));
/* We don't use relegate_external() here, because we know that
nothing else in this externals description (at least) is
Index: ./subversion/libsvn_client/commit_util.c
===================================================================
--- ./subversion/libsvn_client/commit_util.c
+++ ./subversion/libsvn_client/commit_util.c Sun Jul 14 00:40:22 2002
@@ -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,
+ hash_pool));
apr_hash_set (locked_dirs, apr_pstrdup (hash_pool, dir),
APR_HASH_KEY_STRING, adm_access);
}
Index: ./subversion/libsvn_client/commit.c
===================================================================
--- ./subversion/libsvn_client/commit.c
+++ ./subversion/libsvn_client/commit.c Sun Jul 14 00:32:26 2002
@@ -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))
Index: ./subversion/tests/clients/cmdline/copy_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/copy_tests.py
+++ ./subversion/tests/clients/cmdline/copy_tests.py Sun Jul 14 01:11:13 2002
@@ -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__':
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Slowing code churn in libsvn_wc
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> Perhaps we should revert 2470 as it introduces a regression. The
> second commit in the following sequence
Yeah, that's one of the options on the table. Especially given your
recent comments in issue #792, we can't be certain that we fully
understand the cause of the Kevin's original commit bug either, except
that it appeared between 2452/2470 and 2504.
(So if we revert 2470, would we also then revert 2452, do you think?
I kind of think we should. It feels odd to leave the half-started
change in the code; better if all this stuff were cleanly restarted
after Alpha, instead of awkwardly bridging the release.)
Anyway, let's wait a bit on making this call. Ben is reproducing the
regression right now and taking a look, we'll come back with more
formal proposal for action in a bit.
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Slowing code churn in libsvn_wc
Posted by Philip Martin <ph...@codematters.co.uk>.
Karl Fogel <kf...@newton.ch.collab.net> writes:
> So I'd like to request that until Alpha is released, we confine
> ourselves to fixing bugs in the current libsvn_wc code, and not try to
> make any more forward progress on issue #749.
Perhaps we should revert 2470 as it introduces a regression. The
second commit in the following sequence
$ svn mkdir foo
$ svn mkdir foo/bar
$ touch foo/bar/zig
$ svn add foo/bar/zig
$ svn ci
$ svn cp foo foo2
$ echo x >> foo2/bar/zig
$ svn ci
fails with "foo2/bar already locked".
> Philip Martin <ph...@codematters.co.uk> writes:
> > This patch implements a parent-child lock relationship. It passes
> > the regression tests, including Justin's new one.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Slowing code churn in libsvn_wc
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
This is a "meta request":
I haven't had time yet to carefully look at the lock problems in
libsvn_wc; I've read all of Philip's and Justin's discussion, just
haven't had time to really digest the discussion and inspect the code
for myself. I don't think Ben Collins-Sussman has had time either,
nor has Mike Pilato. This is a pity, because we three probably know
libsvn_wc best and would be the obvious people to review the recent
code changes (diffidence or false humility would be useless here, so
I'm not even going to bother).
So I'd like to request that until Alpha is released, we confine
ourselves to fixing bugs in the current libsvn_wc code, and not try to
make any more forward progress on issue #749. The most recent commits
and patches to libsvn_wc have indeed been for fixing bugs, so my
request is pre-emptive, not reactive :-).
The problem is not that the commits are bad in themselves. It's just
that the most qualified reviewers are temporarily very busy with other
Alpha issues, and these are commits that deserve full attention,
nothing less. Therefore it makes sense to coordinate coding and
review times on this, until Alpha is out.
Thanks,
-Karl
Philip Martin <ph...@codematters.co.uk> writes:
> This patch implements a parent-child lock relationship. It passes
> the regression tests, including Justin's new one. I'm posting it to
> ask if this is the right way to go. One thing that worries me is the
> memory usage, it's probably trivial at the moment, but if we start to
> cache the entries will it prove to be too expensive?
>
> Since my visitors' baby has stopped crying I'm going to try to get
> some sleep now :)
>
>
> Continuing issue #749. Make svn_wc_adm_access_t optionally
> hierarchical.
>
> Also revert r2504 as it's no longer needed.
>
> [...]
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org