C. Michael Pilato wrote:
> Julian Foad <julianfoad@btopenworld.com> writes:
>
>>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.
>
> As a general sentiment, the code is often inconsistent here. Some
> functions let you pass the top-level ADM access baton, and a path.
> Some make you pass the individual item's baton, and they simply ask
> that baton for the path it represents.
OK. In this case it works OK with the top-level access baton so I will leave it at that.
>>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?
>
> And what functions are those?
remote_proplist, remote_propget, remote_propset (static in libsvn_client/prop_commands.c)
>>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.
>
> svn_wc_walk_entries() has room to grow -- it makes no claim to
> completeness. If you think a new "mode" for skipping added things, so
> be it.
Thanks. Actually, I think I had misunderstood the "include deleted items" parameter. I thought it referred to items scheduled for deletion, but it actually means items which have already been deleted, but their entry is still hanging around.
Testing entry->schedule seems to be the correct approach. I have done this now, in my updated patch which is attached.
>>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.
>
> Good plan.
I have committed, in r7743, a fix for the existing proplist and propget implementations, and regression tests for them.
- 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.
Index: subversion/libsvn_client/prop_commands.c
===================================================================
--- subversion/libsvn_client/prop_commands.c (revision 7743)
+++ subversion/libsvn_client/prop_commands.c (working copy)
@@ -261,79 +261,65 @@
}
-/* 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;
+
+ /* 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;
- for (hi = apr_hash_first (pool, entries); hi; hi = apr_hash_next (hi))
+ /* Ignore an entry if it does not exist at the time of interest. */
+ if (wb->pristine) /* BASE */
{
- 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;
+ if (entry->schedule == svn_wc_schedule_add)
+ return SVN_NO_ERROR;
+ }
+ else /* WC */
+ {
+ if (entry->schedule == svn_wc_schedule_delete)
+ return SVN_NO_ERROR;
+ }
- /* 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));
+ SVN_ERR (pristine_or_working_propval (&propval, wb->propname, path,
+ wb->base_access, wb->pristine,
+ apr_hash_pool_get (wb->props)));
- /* Process the entry if it exists at the time of interest. */
- if (current_entry->schedule
- != (pristine ? svn_wc_schedule_add : 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;
}
@@ -580,8 +566,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, FALSE, pool));
+ }
else
{
const svn_string_t *propval;
@@ -785,72 +781,57 @@
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)
+proplist_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;
+ struct proplist_walk_baton *wb = walk_baton;
- SVN_ERR (svn_wc_entries_read (&entries, adm_access, FALSE, pool));
+ /* 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;
- for (hi = apr_hash_first (pool, entries); hi; hi = apr_hash_next (hi))
+ /* Ignore an entry if it does not exist at the time of interest. */
+ if (wb->pristine) /* BASE */
{
- 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;
+ if (entry->schedule == svn_wc_schedule_add)
+ return SVN_NO_ERROR;
+ }
+ else /* WC */
+ {
+ if (entry->schedule == svn_wc_schedule_delete)
+ return SVN_NO_ERROR;
+ }
- /* 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));
+ path = apr_pstrdup (wb->props->pool, path);
+ SVN_ERR (add_to_proplist (wb->props, path, wb->base_access,
+ wb->pristine, wb->props->pool));
- /* Process the entry if it exists at the time of interest. */
- if (current_entry->schedule
- != (pristine ? svn_wc_schedule_add : 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,
@@ -951,7 +932,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, FALSE, 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 16:32:54 2003