While trying to make all operations proceed in sorted order, I noticed that recursive propget, proplist and propset all do their own recursion, and should probably make use of "svn_wc_walk_entries" to do it for them (for BASE and WC).
The attached _unfinished_ patch does this for propget and proplist. It works about as well as the existing code, but I'd like to know if I'm on the right track before I finish it. Questions:
1. Is it OK to use, in each directory, the adm. access baton for the top of the tree, or should I be getting an access baton for each directory individually as the original code did? I don't know the pros and cons of this.
/* SVN_ERR (svn_wc_adm_retrieve (&dir_access, wb->base_access, path, pool)); <- necessary? */
SVN_ERR (add_to_proplist (wb->props, path, wb->base_access /* or dir_access ? */,
wb->pristine, wb->props->pool));
2. The remote (repository) versions of these functions are also doing their own recursion. Is there a better or equivalent way in which those should be written?
3. When I am walking the WC, I want to omit entries that are scheduled for deletion, and svn_wc_walk_entries has a parameter to select this behaviour. When I am walking BASE, I want to omit entries that are scheduled for addition. There is no option for this. I will have to examine the scheduling of each item (and I have not done this yet). Perhaps this inconsistency is justified on the basis of "most common usage", as presumably more code paths look at the WC than at BASE, but I'm not sure. Just provoking thought.
4. The existing code omits scheduled-delete items even if it is operating on BASE, which is wrong. I might commit this new regression test first, in XFAIL state, and then, in a second commit, change the tree-walking code and change the test to normal.
Log message:
[[[
Use a common library function to walk the working copy for recursive local
propget and proplist, rather than implementing recursion manually. This does
not save much code, but keeps the recursion logic in one place.
* subversion/libsvn_client/prop_commands.c
(recursive_propget, recursive_proplist): Deleted.
(propget_walk_baton, proplist_walk_baton): New structures.
(propget_walk_cb, proplist_walk_cb): New callback functions.
(svn_client_propget, svn_client_proplist):
Use svn_wc_walk_entries instead of recursing into the WC manually.
### Done "get" and "list"; should do "set" too.
### Bug: A sheduled-delete item shows up in a recursive WC propget/list.
* subversion/tests/clients/cmdline/prop_tests.py
(recursive_base_wc_ops): New test for recursive propget and proplist in BASE
and WC.
(test_list): Add the new test, initially as XFAIL.
]]]
- Julian
Use a common library function to walk the working copy for recursive local
propget and proplist, rather than implementing recursion manually. This does
not save much code, but keeps the recursion logic in one place.
* subversion/libsvn_client/prop_commands.c
(recursive_propget, recursive_proplist): Deleted.
(propget_walk_baton, proplist_walk_baton): New structures.
(propget_walk_cb, proplist_walk_cb): New callback functions.
(svn_client_propget, svn_client_proplist):
Use svn_wc_walk_entries instead of recursing into the WC manually.
### Done "get" and "list"; should do "set" too.
### Bug: A sheduled-delete item shows up in a recursive WC propget/list.
* subversion/tests/clients/cmdline/prop_tests.py
(recursive_base_wc_ops): New test for recursive propget and proplist in BASE
and WC.
(test_list): Add the new test, initially as XFAIL.
Index: subversion/tests/clients/cmdline/prop_tests.py
===================================================================
--- subversion/tests/clients/cmdline/prop_tests.py (revision 7713)
+++ subversion/tests/clients/cmdline/prop_tests.py (working copy)
@@ -899,6 +899,69 @@
check_prop('prop_binx', mu_path_bak, [prop_binx])
check_prop('prop_binx', A_path_bak, [prop_binx])
+#----------------------------------------------------------------------
+
+def recursive_base_wc_ops(sbox):
+ "recursive property operations in BASE and WC"
+
+ # Bootstrap
+ sbox.build()
+ wc_dir = sbox.wc_dir
+
+ # Files with which to test, in alphabetical order
+ f_add = os.path.join('A', 'added')
+ f_del = os.path.join('A', 'mu')
+ f_keep= os.path.join('iota')
+ fp_add = os.path.join(wc_dir, f_add)
+ fp_del = os.path.join(wc_dir, f_del)
+ fp_keep= os.path.join(wc_dir, f_keep)
+
+ # Set up properties
+ svntest.main.run_svn(None, 'propset', 'p', 'old-del', fp_del)
+ svntest.main.run_svn(None, 'propset', 'p', 'old-keep',fp_keep)
+ svntest.main.run_svn(None, 'commit', '-m', '', wc_dir)
+ svntest.main.file_append(fp_add, 'blah')
+ svntest.main.run_svn(None, 'add', fp_add)
+ svntest.main.run_svn(None, 'propset', 'p', 'new-add', fp_add)
+ svntest.main.run_svn(None, 'propset', 'p', 'new-del', fp_del)
+ svntest.main.run_svn(None, 'propset', 'p', 'new-keep',fp_keep)
+ svntest.main.run_svn(None, 'del', '--force', fp_del)
+
+ # Ensure that each line of output contains the corresponding string of
+ # expected_out, and that errput is empty.
+ def verify_output(expected_out, output, errput):
+ if errput != []:
+ print 'Error: stderr:'
+ print errput
+ raise svntest.Failure
+ output.sort()
+ ln = 0
+ for line in output:
+ if ((line.find(expected_out[ln]) == -1) or
+ (line != '' and expected_out[ln] == '')):
+ print 'Error: expected keywords: ', expected_out
+ print ' actual full output:', output
+ raise svntest.Failure
+ ln = ln + 1
+
+ # Test recursive proplist
+ output, errput = svntest.main.run_svn(None, 'proplist', '-R', '-v', wc_dir,
+ '-rBASE')
+ verify_output([ 'old-del', 'old-keep', 'Properties on ', 'Properties on ' ],
+ output, errput)
+
+ output, errput = svntest.main.run_svn(None, 'proplist', '-R', '-v', wc_dir)
+ verify_output([ 'new-add', 'new-keep', 'Properties on ', 'Properties on ' ],
+ output, errput)
+
+ # Test recursive propget
+ output, errput = svntest.main.run_svn(None, 'propget', '-R', 'p', wc_dir,
+ '-rBASE')
+ verify_output([ 'old-del', 'old-keep' ], output, errput)
+
+ output, errput = svntest.main.run_svn(None, 'propget', '-R', 'p', wc_dir)
+ verify_output([ 'new-add', 'new-keep' ], output, errput)
+
########################################################################
# Run the tests
@@ -921,6 +984,7 @@
Skip(revprop_change, (os.name != 'posix')),
prop_value_conversions,
binary_props,
+ XFail(recursive_base_wc_ops),
]
if __name__ == '__main__':
Index: subversion/libsvn_client/prop_commands.c
===================================================================
--- subversion/libsvn_client/prop_commands.c (revision 7713)
+++ subversion/libsvn_client/prop_commands.c (working copy)
@@ -261,77 +261,54 @@
}
-/* Helper for svn_client_propget.
+/* Machinery for an automated entries walk... */
+
+struct propget_walk_baton
+{
+ const char *propname; /* The name of the property to get. */
+ svn_boolean_t pristine; /* Select base rather than working props. */
+ svn_wc_adm_access_t *base_access; /* Access for the tree being walked. */
+ apr_hash_t *props; /* Out: mapping of (path:propval). */
+};
+
+/* An entries-walk callback for svn_client_propget.
*
- * Starting from the path associated with ADM_ACCESS, populate PROPS
- * with the values of property PROPNAME. If PRISTINE is true, use the
- * base values, else use working values.
+ * For the path given by PATH and ENTRY,
+ * within the WALK_BATON which is of type "struct propget_walk_baton",
+ * populate wb->PROPS with the values of property wb->PROPNAME.
+ * If wb->PRISTINE is true, use the base value, else use the working value.
*
- * The keys of PROPS will be 'const char *' paths, rooted at the
+ * The keys of wb->PROPS will be 'const char *' paths, rooted at the
* path svn_wc_adm_access_path(ADM_ACCESS), and the values are
* 'const svn_string_t *' property values.
*/
static svn_error_t *
-recursive_propget (apr_hash_t *props,
- const char *propname,
- svn_boolean_t pristine,
- svn_wc_adm_access_t *adm_access,
- apr_pool_t *pool)
+propget_walk_cb (const char *path,
+ const svn_wc_entry_t *entry,
+ void *walk_baton,
+ apr_pool_t *pool)
{
- apr_hash_t *entries;
- apr_hash_index_t *hi;
- SVN_ERR (svn_wc_entries_read (&entries, adm_access, FALSE, pool));
+ struct propget_walk_baton *wb = walk_baton;
+ const svn_string_t *propval;
- for (hi = apr_hash_first (pool, entries); hi; hi = apr_hash_next (hi))
- {
- const void *key;
- const char *keystring;
- void * val;
- const char *current_entry_name;
- const char *full_entry_path;
- const svn_wc_entry_t *current_entry;
-
- apr_hash_this (hi, &key, NULL, &val);
- keystring = key;
- current_entry = val;
-
- if (! strcmp (keystring, SVN_WC_ENTRY_THIS_DIR))
- current_entry_name = NULL;
- else
- current_entry_name = keystring;
-
- /* Compute the complete path of the entry */
- if (current_entry_name)
- full_entry_path = svn_path_join (svn_wc_adm_access_path (adm_access),
- current_entry_name, pool);
- else
- full_entry_path = apr_pstrdup (pool,
- svn_wc_adm_access_path (adm_access));
+ /* We're going to receive dirents twice; we want to ignore the
+ first one (where it's a child of a parent dir), and only use
+ the second one (where we're looking at THIS_DIR.) */
+ if ((entry->kind == svn_node_dir)
+ && (strcmp (entry->name, SVN_WC_ENTRY_THIS_DIR)))
+ return SVN_NO_ERROR;
+
+ /*SVN_ERR (svn_wc_adm_retrieve (&dir_access, wb->base_access, path, pool));*/
+ SVN_ERR (pristine_or_working_propval (&propval, wb->propname, path,
+ wb->base_access, wb->pristine,
+ apr_hash_pool_get (wb->props)));
- if (current_entry->schedule != svn_wc_schedule_delete)
- {
- if (current_entry->kind == svn_node_dir && current_entry_name)
- {
- svn_wc_adm_access_t *dir_access;
- SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access,
- full_entry_path, pool));
- SVN_ERR (recursive_propget (props, propname, pristine,
- dir_access, pool));
- }
- else
- {
- const svn_string_t *propval;
-
- SVN_ERR (pristine_or_working_propval (&propval, propname,
- full_entry_path,
- adm_access,
- pristine, pool));
- if (propval)
- apr_hash_set (props, full_entry_path,
- APR_HASH_KEY_STRING, propval);
- }
- }
+ if (propval)
+ {
+ path = apr_pstrdup (apr_hash_pool_get (wb->props), path);
+ apr_hash_set (wb->props, path, APR_HASH_KEY_STRING, propval);
}
+
return SVN_NO_ERROR;
}
@@ -578,8 +555,18 @@
/* Fetch, recursively or not. */
if (recurse && (node->kind == svn_node_dir))
- SVN_ERR (recursive_propget (*props, propname, pristine,
- adm_access, pool));
+ {
+ static const svn_wc_entry_callbacks_t walk_callbacks = { propget_walk_cb };
+ struct propget_walk_baton wb;
+
+ wb.base_access = adm_access;
+ wb.props = *props;
+ wb.propname = propname;
+ wb.pristine = pristine;
+
+ SVN_ERR (svn_wc_walk_entries (target, adm_access,
+ &walk_callbacks, &wb, pristine, pool));
+ }
else
{
const svn_string_t *propval;
@@ -783,70 +770,46 @@
return SVN_NO_ERROR;
}
-/* Helper for svn_client_proplist.
+/* Machinery for an automated entries walk... */
+
+struct proplist_walk_baton
+{
+ svn_boolean_t pristine; /* Select base rather than working props. */
+ svn_wc_adm_access_t *base_access; /* Access for the tree being walked. */
+ apr_array_header_t *props; /* Out: array of svn_client_proplist_item_t. */
+};
+
+/* An entries-walk callback for svn_client_proplist.
*
- * Starting from the path associated with ADM_ACCESS, populate PROPS
- * with the values of property PROPNAME. If PRISTINE is true, use the
- * base values, else use working values.
- *
- * The keys of PROPS will be 'const char *' paths, rooted at the
- * path svn_wc_adm_access_path(ADM_ACCESS), and the values are
- * 'const svn_string_t *' property values.
+ * For the path given by PATH and ENTRY,
+ * within the WALK_BATON which is of type "struct proplist_walk_baton",
+ * populate wb->PROPS with a svn_client_proplist_item_t for each path.
+ * If wb->PRISTINE is true, use the base values, else use the working values.
*/
static svn_error_t *
-recursive_proplist (apr_array_header_t *props,
- svn_wc_adm_access_t *adm_access,
- svn_boolean_t pristine,
- apr_pool_t *pool)
-{
- apr_hash_t *entries;
- apr_hash_index_t *hi;
-
- SVN_ERR (svn_wc_entries_read (&entries, adm_access, FALSE, pool));
-
- for (hi = apr_hash_first (pool, entries); hi; hi = apr_hash_next (hi))
- {
- const void *key;
- const char *keystring;
- void * val;
- const char *current_entry_name;
- const char *full_entry_path;
- const svn_wc_entry_t *current_entry;
-
- apr_hash_this (hi, &key, NULL, &val);
- keystring = key;
- current_entry = val;
-
- if (! strcmp (keystring, SVN_WC_ENTRY_THIS_DIR))
- current_entry_name = NULL;
- else
- current_entry_name = keystring;
+proplist_walk_cb (const char *path,
+ const svn_wc_entry_t *entry,
+ void *walk_baton,
+ apr_pool_t *pool)
+{
+ struct proplist_walk_baton *wb = walk_baton;
+
+ /* We're going to receive dirents twice; we want to ignore the
+ first one (where it's a child of a parent dir), and only use
+ the second one (where we're looking at THIS_DIR.) */
+ if ((entry->kind == svn_node_dir)
+ && (strcmp (entry->name, SVN_WC_ENTRY_THIS_DIR)))
+ return SVN_NO_ERROR;
+
+ /*SVN_ERR (svn_wc_adm_retrieve (&dir_access, wb->base_access, path, pool));*/
+ path = apr_pstrdup (wb->props->pool, path);
+ SVN_ERR (add_to_proplist (wb->props, path, wb->base_access,
+ wb->pristine, wb->props->pool));
- /* Compute the complete path of the entry */
- if (current_entry_name)
- full_entry_path = svn_path_join (svn_wc_adm_access_path (adm_access),
- current_entry_name, pool);
- else
- full_entry_path = apr_pstrdup (pool,
- svn_wc_adm_access_path (adm_access));
-
- if (current_entry->schedule != svn_wc_schedule_delete)
- {
- if (current_entry->kind == svn_node_dir && current_entry_name)
- {
- svn_wc_adm_access_t *dir_access;
- SVN_ERR (svn_wc_adm_retrieve (&dir_access, adm_access,
- full_entry_path, pool));
- SVN_ERR (recursive_proplist (props, dir_access, pristine, pool));
- }
- else
- SVN_ERR (add_to_proplist (props, full_entry_path, adm_access,
- pristine, pool));
- }
- }
return SVN_NO_ERROR;
}
+
/* Note: this implementation is very similar to svn_client_propget. */
svn_error_t *
svn_client_proplist (apr_array_header_t **props,
@@ -947,7 +910,17 @@
/* Fetch, recursively or not. */
if (recurse && (node->kind == svn_node_dir))
- SVN_ERR (recursive_proplist (*props, adm_access, pristine, pool));
+ {
+ static const svn_wc_entry_callbacks_t walk_callbacks = { proplist_walk_cb };
+ struct proplist_walk_baton wb;
+
+ wb.base_access = adm_access;
+ wb.props = *props;
+ wb.pristine = pristine;
+
+ SVN_ERR (svn_wc_walk_entries (target, adm_access,
+ &walk_callbacks, &wb, pristine, pool));
+ }
else
SVN_ERR (add_to_proplist (*props, target, adm_access, pristine, pool));
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Nov 14 05:00:11 2003