Thanks Julian.
This updated patch has all the changes.
I have removed some redundant portion of code in print_dirent() and
print_dirent_xml() in svn/list-cmd.c.
Thanks & Regards,
Vijayaguru
On Tuesday 27 November 2012 03:19 AM, Julian Foad wrote:
vijay <vi...@collab.net> wrote:
On Tuesday 13 November 2012 03:02 AM, Stefan Sperling wrote:
The two extra list_func() calls has gone away!
Attaching the updated patch that addresses your review comments.
Hi Vijay. I read through the patch and it looks good, I just have a few minor
comments:
Index: subversion/include/svn_client.h
===================================================================
- * @since New in 1.4.
+ * If svn_client_list3() was called with @a include_externals set to TRUE,
+ * @a external_parent_url and @a external_target will be set.
+ * @a external_parent_url is url of the directory which has the
+ * externals definitions. @a external_target is the target subdirectory of
+ * externals definitions.
Is @a external_target an abspath, or a 'relpath' relative to @a path, or
relative to the root of the entire 'list' operation, or what?
+ *
+ * If external_parent_url and external_target are defined, the item being
+ * listed is part of the external described by external_parent_url and
+ * external_target. Else, the item is not part of any external.
+ * Moreover, we will never mix items which are part of separate
+ * externals, and will always finish listing an external before listing
+ * the next one.
+
+ * @a pool may be used for temporary allocations.
+ *
+ * @since New in 1.8.
*/
+typedef svn_error_t *(*svn_client_list_func2_t)(
Index: subversion/libsvn_client/deprecated.c
===================================================================
+
+static void
Please put a doc string on each new function. It can be brief for a simple
function such as this.
+wrap_list_func(svn_client_list_func2_t *list_func2,
+ void **list_func2_baton,
+ svn_client_list_func_t list_func,
+ void *baton,
+ apr_pool_t *scratch_pool)
+{
+ struct list_func_wrapper_baton *lfwb = apr_palloc(scratch_pool,
+ sizeof(*lfwb));
This is allocating memory that will be returned as the result of this function,
so the pool shouldn't be 'scratch_pool' but 'result_pool'.
+
+ /* Set the user provided old format callback in the baton. */
+ lfwb->list_func1_baton = baton;
+ lfwb->list_func1 = list_func;
+
+ *list_func2_baton = lfwb;
+ *list_func2 = list_func_wrapper;
+}
Index: subversion/libsvn_client/client.h
===================================================================
+/* List external items defined on each external in EXTERNALS, a const char *
+ externals_parent_url(url of the directory which has the externals
+ definitions) of all externals mapping to the const char * externals_desc
The implementation treats the hash values as 'svn_string_t *' not 'const char
*'.
+ (externals description text). All other options are the same as those
+ passed to svn_client_list(). */
+svn_error_t *
+svn_client__list_externals(apr_hash_t *externals,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool);
Index: subversion/libsvn_client/externals.c
===================================================================
+svn_error_t *
+svn_client__list_externals(apr_hash_t *externals,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ apr_hash_index_t *hi;
+
+ for (hi = apr_hash_first(scratch_pool, externals);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *externals_parent_url = svn__apr_hash_index_key(hi);
+ svn_string_t *externals_desc = svn__apr_hash_index_val(hi);
+ apr_array_header_t *external_items;
+
+ svn_pool_clear(iterpool);
+
+ external_items = apr_array_make(iterpool, 1,
+ sizeof(svn_wc_external_item2_t*));
There's no need to initialize 'external_items', as the first parameter of
svn_wc_parse_externals_description3() is a pure output parameter. That is, the
function creates a new array.
+ SVN_ERR(svn_wc_parse_externals_description3(&external_items,
+ externals_parent_url,
+ externals_desc->data,
+ FALSE, iterpool));
+
+ if (! external_items->nelts)
+ continue;
+
+ SVN_ERR(list_external_items(external_items, externals_parent_url, depth,
+ dirent_fields, fetch_locks, list_func,
+ baton, ctx, iterpool));
+
+ }
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
Index: subversion/svn/main.c
===================================================================
{"include-externals", opt_include_externals, 0,
- N_("Also commit file and dir externals reached by\n"
- " "
- "recursion. This does not include externals with a\n"
- " "
- "fixed revision. (See the svn:externals property)")},
+ N_("include externals definitions")},
That change loses information that was previously shown for the 'commit'
command. We have a way to display a different description text for the same
option for a specific subcommand. It's done by adding a bit at the end of the
subcommand definition -- see the lines starting with '{{' in main.c.
- Julian
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1414005)
+++ subversion/include/svn_client.h (working copy)
@@ -5368,7 +5368,7 @@
* @{
*/
-/** The type of function invoked by svn_client_list2() to report the details
+/** The type of function invoked by svn_client_list3() to report the details
* of each directory entry being listed.
*
* @a baton is the baton that was passed to the caller. @a path is the
@@ -5378,11 +5378,45 @@
* the entry's lock, if it is locked and if lock information is being
* reported by the caller; otherwise @a lock is NULL. @a abs_path is the
* repository path of the top node of the list operation; it is relative to
- * the repository root and begins with "/". @a pool may be used for
- * temporary allocations.
+ * the repository root and begins with "/".
*
- * @since New in 1.4.
+ * If svn_client_list3() was called with @a include_externals set to TRUE,
+ * @a external_parent_url and @a external_target will be set.
+ * @a external_parent_url is url of the directory which has the
+ * externals definitions. @a external_target is the target subdirectory of
+ * externals definitions which is relative to the parent directory that holds
+ * the external item.
+ *
+ * If external_parent_url and external_target are defined, the item being
+ * listed is part of the external described by external_parent_url and
+ * external_target. Else, the item is not part of any external.
+ * Moreover, we will never mix items which are part of separate
+ * externals, and will always finish listing an external before listing
+ * the next one.
+
+ * @a pool may be used for temporary allocations.
+ *
+ * @since New in 1.8.
*/
+typedef svn_error_t *(*svn_client_list_func2_t)(
+ void *baton,
+ const char *path,
+ const svn_dirent_t *dirent,
+ const svn_lock_t *lock,
+ const char *abs_path,
+ const char *external_parent_url,
+ const char *external_target,
+ apr_pool_t *pool);
+
+/**
+ * Similar to #svn_client_list_func2_t, but without any information about
+ * externals definitions.
+ *
+ * @deprecated Provided for backward compatibility with the 1.7 API.
+ *
+ * @since New in 1.4
+ *
+ * */
typedef svn_error_t *(*svn_client_list_func_t)(void *baton,
const char *path,
const svn_dirent_t *dirent,
@@ -5406,6 +5440,10 @@
*
* If @a fetch_locks is TRUE, include locks when reporting directory entries.
*
+ * If @a include_externals is TRUE, also list all external items
+ * reached by recursion. @a depth value passed to the original list target
+ * applies for the externals also.
+ *
* Use @a pool for temporary allocations.
*
* Use authentication baton cached in @a ctx to authenticate against the
@@ -5422,8 +5460,30 @@
* otherwise simply bitwise OR together the combination of @c SVN_DIRENT_
* fields you care about.
*
+ * @since New in 1.8.
+ */
+svn_error_t *
+svn_client_list3(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_boolean_t include_externals,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+
+/** Similar to svn_client_list3(), but with @a include_externals set to FALSE,
+ * and using a #svn_client_list_func2_t as callback.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.7 API.
+ *
* @since New in 1.5.
*/
+SVN_DEPRECATED
svn_error_t *
svn_client_list2(const char *path_or_url,
const svn_opt_revision_t *peg_revision,
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 1414005)
+++ subversion/libsvn_client/client.h (working copy)
@@ -596,6 +596,58 @@
/* ---------------------------------------------------------------- */
+/*** List ***/
+
+/* List the file/directory entries for PATH_OR_URL at REVISION.
+ The actual node revision selected is determined by the path as
+ it exists in PEG_REVISION.
+
+ If DEPTH is svn_depth_infinity, then list all file and directory entries
+ recursively. Else if DEPTH is svn_depth_files, list all files under
+ PATH_OR_URL (if any), but not subdirectories. Else if DEPTH is
+ svn_depth_immediates, list all files and include immediate
+ subdirectories (at svn_depth_empty). Else if DEPTH is
+ svn_depth_empty, just list PATH_OR_URL with none of its entries.
+
+ DIRENT_FIELDS controls which fields in the svn_dirent_t's are
+ filled in. To have them totally filled in use SVN_DIRENT_ALL,
+ otherwise simply bitwise OR together the combination of SVN_DIRENT_*
+ fields you care about.
+
+ If FETCH_LOCKS is TRUE, include locks when reporting directory entries.
+
+ If INCLUDE_EXTERNALS is TRUE, also list all external items
+ reached by recursion. DEPTH value passed to the original list target
+ applies for the externals also. EXTERNAL_PARENT_URL is url of the
+ directory which has the externals definitions. EXTERNAL_TARGET is the
+ target subdirectory of externals definitions.
+
+ Report directory entries by invoking LIST_FUNC/BATON.
+ Pass EXTERNAL_PARENT_URL and EXTERNAL_TARGET to LIST_FUNC when external
+ items are listed, otherwise both are set to NULL.
+
+ Use authentication baton cached in CTX to authenticate against the
+ repository.
+
+ Use POOL for all allocations.
+*/
+svn_error_t *
+svn_client__list_internal(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_boolean_t include_externals,
+ const char *external_parent_url,
+ const char *external_target,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+/* ---------------------------------------------------------------- */
+
/*** Inheritable Properties ***/
/* Fetch the inherited properties for the base of LOCAL_ABSPATH as well
@@ -1035,6 +1087,22 @@
void *status_baton,
apr_pool_t *pool);
+
+/* List external items defined on each external in EXTERNALS, a const char *
+ externals_parent_url(url of the directory which has the externals
+ definitions) of all externals mapping to the svn_string_t * externals_desc
+ (externals description text). All other options are the same as those
+ passed to svn_client_list(). */
+svn_error_t *
+svn_client__list_externals(apr_hash_t *externals,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool);
+
/* Baton for svn_client__dirent_fetcher */
struct svn_client__dirent_fetcher_baton_t
{
Index: subversion/libsvn_client/deprecated.c
===================================================================
--- subversion/libsvn_client/deprecated.c (revision 1414005)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -1282,7 +1282,78 @@
}
/*** From list.c ***/
+
+/* Baton for use with wrap_list_func */
+struct list_func_wrapper_baton {
+ void *list_func1_baton;
+ svn_client_list_func_t list_func1;
+};
+
+/* This implements svn_client_list_func2_t */
+static svn_error_t *
+list_func_wrapper(void *baton,
+ const char *path,
+ const svn_dirent_t *dirent,
+ const svn_lock_t *lock,
+ const char *abs_path,
+ const char *external_parent_url,
+ const char *external_target,
+ apr_pool_t *scratch_pool)
+{
+ struct list_func_wrapper_baton *lfwb = baton;
+
+ if (lfwb->list_func1)
+ return lfwb->list_func1(lfwb->list_func1_baton, path, dirent,
+ lock, abs_path, scratch_pool);
+
+ return SVN_NO_ERROR;
+}
+
+/* Helper function for svn_client_list2(). It wraps old format baton
+ and callback function in list_func_wrapper_baton and
+ returns new baton and callback to use with svn_client_list3(). */
+static void
+wrap_list_func(svn_client_list_func2_t *list_func2,
+ void **list_func2_baton,
+ svn_client_list_func_t list_func,
+ void *baton,
+ apr_pool_t *result_pool)
+{
+ struct list_func_wrapper_baton *lfwb = apr_palloc(result_pool,
+ sizeof(*lfwb));
+
+ /* Set the user provided old format callback in the baton. */
+ lfwb->list_func1_baton = baton;
+ lfwb->list_func1 = list_func;
+
+ *list_func2_baton = lfwb;
+ *list_func2 = list_func_wrapper;
+}
+
svn_error_t *
+svn_client_list2(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ svn_client_list_func2_t list_func2;
+ void *list_func2_baton;
+
+ wrap_list_func(&list_func2, &list_func2_baton, list_func, baton, pool);
+
+ return svn_client_list3(path_or_url, peg_revision, revision, depth,
+ dirent_fields, fetch_locks,
+ FALSE /* include externals */,
+ list_func2, list_func2_baton, ctx, pool);
+}
+
+svn_error_t *
svn_client_list(const char *path_or_url,
const svn_opt_revision_t *peg_revision,
const svn_opt_revision_t *revision,
Index: subversion/libsvn_client/externals.c
===================================================================
--- subversion/libsvn_client/externals.c (revision 1414005)
+++ subversion/libsvn_client/externals.c (working copy)
@@ -1180,3 +1180,105 @@
return SVN_NO_ERROR;
}
+/* Walk through all the external items and list them. */
+static svn_error_t *
+list_external_items(apr_array_header_t *external_items,
+ const char *externals_parent_url,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ const char *externals_parent_repos_root_url;
+ apr_pool_t *iterpool;
+ int i;
+
+ SVN_ERR(svn_client_get_repos_root(&externals_parent_repos_root_url,
+ NULL /* uuid */,
+ externals_parent_url, ctx,
+ scratch_pool, scratch_pool));
+
+ iterpool = svn_pool_create(scratch_pool);
+
+ for (i = 0; i < external_items->nelts; i++)
+ {
+ const char *resolved_url;
+
+ svn_wc_external_item2_t *item =
+ APR_ARRAY_IDX(external_items, i, svn_wc_external_item2_t *);
+
+ svn_pool_clear(iterpool);
+
+ SVN_ERR(svn_wc__resolve_relative_external_url(
+ &resolved_url,
+ item,
+ externals_parent_repos_root_url,
+ externals_parent_url,
+ iterpool, iterpool));
+
+ /* List the external */
+ SVN_ERR(wrap_external_error(ctx, item->target_dir,
+ svn_client__list_internal(
+ resolved_url,
+ &item->peg_revision,
+ &item->revision,
+ depth, dirent_fields,
+ fetch_locks,
+ TRUE,
+ externals_parent_url,
+ item->target_dir,
+ list_func, baton, ctx,
+ iterpool),
+ iterpool));
+
+ }
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
+
+
+svn_error_t *
+svn_client__list_externals(apr_hash_t *externals,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *scratch_pool)
+{
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
+ apr_hash_index_t *hi;
+
+ for (hi = apr_hash_first(scratch_pool, externals);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *externals_parent_url = svn__apr_hash_index_key(hi);
+ svn_string_t *externals_desc = svn__apr_hash_index_val(hi);
+ apr_array_header_t *external_items;
+
+ svn_pool_clear(iterpool);
+
+ SVN_ERR(svn_wc_parse_externals_description3(&external_items,
+ externals_parent_url,
+ externals_desc->data,
+ FALSE, iterpool));
+
+ if (! external_items->nelts)
+ continue;
+
+ SVN_ERR(list_external_items(external_items, externals_parent_url, depth,
+ dirent_fields, fetch_locks, list_func,
+ baton, ctx, iterpool));
+
+ }
+ svn_pool_destroy(iterpool);
+
+ return SVN_NO_ERROR;
+}
+
Index: subversion/libsvn_client/list.c
===================================================================
--- subversion/libsvn_client/list.c (revision 1414005)
+++ subversion/libsvn_client/list.c (working copy)
@@ -47,7 +47,15 @@
LOCKS, if non-NULL, is a hash mapping const char * paths to svn_lock_t
objects and FS_PATH is the absolute filesystem path of the RA session.
- Use POOL for temporary allocations.
+ Use SCRATCH_POOL for temporary allocations.
+
+ If the caller passes EXTERNALS as non-NULL, populate the EXTERNALS
+ hash table whose keys are URLs of the directory which has externals
+ definitions, and whose values are the externals description text.
+ Allocate the hash's keys and values in RESULT_POOL.
+
+ EXTERNAL_PARENT_URL and EXTERNAL_TARGET are set when external items
+ are listed, otherwise both are set to NULL by the caller.
*/
static svn_error_t *
get_dir_contents(apr_uint32_t dirent_fields,
@@ -58,23 +66,31 @@
const char *fs_path,
svn_depth_t depth,
svn_client_ctx_t *ctx,
- svn_client_list_func_t list_func,
+ apr_hash_t *externals,
+ const char *external_parent_url,
+ const char *external_target,
+ svn_client_list_func2_t list_func,
void *baton,
- apr_pool_t *pool)
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
{
apr_hash_t *tmpdirents;
- apr_pool_t *iterpool = svn_pool_create(pool);
+ apr_pool_t *iterpool = svn_pool_create(scratch_pool);
apr_array_header_t *array;
svn_error_t *err;
+ apr_hash_t *prop_hash = NULL;
+ const svn_string_t *prop_val = NULL;
int i;
if (depth == svn_depth_empty)
return SVN_NO_ERROR;
-
- /* Get the directory's entries, but not its props. Ignore any
- not-authorized errors. */
- err = svn_ra_get_dir2(ra_session, &tmpdirents, NULL, NULL,
- dir, rev, dirent_fields, pool);
+
+ /* Get the directory's entries. If externals hash is non-NULL, get its
+ properties also. Ignore any not-authorized errors. */
+ err = svn_ra_get_dir2(ra_session, &tmpdirents, NULL,
+ externals ? &prop_hash : NULL,
+ dir, rev, dirent_fields, scratch_pool);
+
if (err && ((err->apr_err == SVN_ERR_RA_NOT_AUTHORIZED) ||
(err->apr_err == SVN_ERR_RA_DAV_FORBIDDEN)))
{
@@ -82,12 +98,29 @@
return SVN_NO_ERROR;
}
SVN_ERR(err);
+
+ /* Filter out svn:externals from all properties hash. */
+ if (prop_hash)
+ prop_val = apr_hash_get(prop_hash, SVN_PROP_EXTERNALS,
+ APR_HASH_KEY_STRING);
+ if (prop_val)
+ {
+ const char *url;
+ SVN_ERR(svn_ra_get_session_url(ra_session, &url, scratch_pool));
+
+ apr_hash_set(externals, svn_path_url_add_component2(url, dir,
+ result_pool),
+ APR_HASH_KEY_STRING, svn_string_dup(prop_val,
+ result_pool));
+ }
+
if (ctx->cancel_func)
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
/* Sort the hash, so we can call the callback in a "deterministic" order. */
- array = svn_sort__hash(tmpdirents, svn_sort_compare_items_lexically, pool);
+ array = svn_sort__hash(tmpdirents, svn_sort_compare_items_lexically,
+ scratch_pool);
for (i = 0; i < array->nelts; ++i)
{
svn_sort__item_t *item = &APR_ARRAY_IDX(array, i, svn_sort__item_t);
@@ -110,12 +143,17 @@
if (the_ent->kind == svn_node_file
|| depth == svn_depth_immediates
|| depth == svn_depth_infinity)
- SVN_ERR(list_func(baton, path, the_ent, lock, fs_path, iterpool));
-
+ SVN_ERR(list_func(baton, path, the_ent, lock, fs_path,
+ external_parent_url, external_target, iterpool));
+
+ /* If externals is non-NULL, populate the externals hash table
+ recursively for all directory entries. */
if (depth == svn_depth_infinity && the_ent->kind == svn_node_dir)
SVN_ERR(get_dir_contents(dirent_fields, path, rev,
- ra_session, locks, fs_path, depth, ctx,
- list_func, baton, iterpool));
+ ra_session, locks, fs_path, depth, ctx,
+ externals, external_parent_url,
+ external_target, list_func, baton,
+ result_pool, iterpool));
}
svn_pool_destroy(iterpool);
@@ -236,16 +274,19 @@
}
svn_error_t *
-svn_client_list2(const char *path_or_url,
- const svn_opt_revision_t *peg_revision,
- const svn_opt_revision_t *revision,
- svn_depth_t depth,
- apr_uint32_t dirent_fields,
- svn_boolean_t fetch_locks,
- svn_client_list_func_t list_func,
- void *baton,
- svn_client_ctx_t *ctx,
- apr_pool_t *pool)
+svn_client__list_internal(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_boolean_t include_externals,
+ const char *external_parent_url,
+ const char *external_target,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
{
svn_ra_session_t *ra_session;
svn_client__pathrev_t *loc;
@@ -253,7 +294,13 @@
const char *fs_path;
svn_error_t *err;
apr_hash_t *locks;
+ apr_hash_t *externals;
+ if (include_externals)
+ externals = apr_hash_make(pool);
+ else
+ externals = NULL;
+
/* We use the kind field to determine if we should recurse, so we
always need it. */
dirent_fields |= SVN_DIRENT_KIND;
@@ -295,14 +342,52 @@
SVN_ERR(list_func(baton, "", dirent, locks
? (apr_hash_get(locks, fs_path,
APR_HASH_KEY_STRING))
- : NULL, fs_path, pool));
+ : NULL, fs_path, external_parent_url,
+ external_target, pool));
if (dirent->kind == svn_node_dir
&& (depth == svn_depth_files
|| depth == svn_depth_immediates
|| depth == svn_depth_infinity))
SVN_ERR(get_dir_contents(dirent_fields, "", loc->rev, ra_session, locks,
- fs_path, depth, ctx, list_func, baton, pool));
-
+ fs_path, depth, ctx, externals,
+ external_parent_url, external_target, list_func,
+ baton, pool, pool));
+
+ /* We handle externals after listing entries under path_or_url, so that
+ handling external items (and any errors therefrom) doesn't delay
+ the primary operation. */
+ if (include_externals && apr_hash_count(externals))
+ {
+ /* The 'externals' hash populated by get_dir_contents() is processed
+ here. */
+ SVN_ERR(svn_client__list_externals(externals, depth, dirent_fields,
+ fetch_locks, list_func, baton,
+ ctx, pool));
+ }
+
return SVN_NO_ERROR;
}
+
+svn_error_t *
+svn_client_list3(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *revision,
+ svn_depth_t depth,
+ apr_uint32_t dirent_fields,
+ svn_boolean_t fetch_locks,
+ svn_boolean_t include_externals,
+ svn_client_list_func2_t list_func,
+ void *baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+
+ return svn_error_trace(svn_client__list_internal(path_or_url, peg_revision,
+ revision,
+ depth, dirent_fields,
+ fetch_locks,
+ include_externals,
+ NULL, NULL, list_func,
+ baton, ctx, pool));
+}
Index: subversion/svn/list-cmd.c
===================================================================
--- subversion/svn/list-cmd.c (revision 1414005)
+++ subversion/svn/list-cmd.c (working copy)
@@ -42,9 +42,14 @@
struct print_baton {
svn_boolean_t verbose;
svn_client_ctx_t *ctx;
+
+ /* To keep track of last seen external information. */
+ const char *last_external_parent_url;
+ const char *last_external_target;
+ svn_boolean_t in_external;
};
-/* This implements the svn_client_list_func_t API, printing a single
+/* This implements the svn_client_list_func2_t API, printing a single
directory entry in text format. */
static svn_error_t *
print_dirent(void *baton,
@@ -52,6 +57,8 @@
const svn_dirent_t *dirent,
const svn_lock_t *lock,
const char *abs_path,
+ const char *external_parent_url,
+ const char *external_target,
apr_pool_t *pool)
{
struct print_baton *pb = baton;
@@ -59,6 +66,9 @@
static const char *time_format_long = NULL;
static const char *time_format_short = NULL;
+ SVN_ERR_ASSERT((external_parent_url == NULL && external_target == NULL) ||
+ (external_parent_url && external_target));
+
if (time_format_long == NULL)
time_format_long = _("%b %d %H:%M");
if (time_format_short == NULL)
@@ -79,7 +89,25 @@
}
else
entryname = path;
+
+ if (external_parent_url && external_target)
+ {
+ if ((pb->last_external_parent_url == NULL
+ && pb->last_external_target == NULL)
+ || (strcmp(pb->last_external_parent_url, external_parent_url) != 0
+ || strcmp(pb->last_external_target, external_target) != 0))
+ {
+ SVN_ERR(svn_cmdline_printf(pool,
+ _("Listing external '%s'"
+ " defined on '%s':\n"),
+ external_target,
+ external_parent_url));
+ pb->last_external_parent_url = external_parent_url;
+ pb->last_external_target = external_target;
+ }
+ }
+
if (pb->verbose)
{
apr_time_t now = apr_time_now();
@@ -133,7 +161,7 @@
}
-/* This implements the svn_client_list_func_t API, printing a single dirent
+/* This implements the svn_client_list_func2_t API, printing a single dirent
in XML format. */
static svn_error_t *
print_dirent_xml(void *baton,
@@ -141,11 +169,16 @@
const svn_dirent_t *dirent,
const svn_lock_t *lock,
const char *abs_path,
+ const char *external_parent_url,
+ const char *external_target,
apr_pool_t *pool)
{
struct print_baton *pb = baton;
const char *entryname;
- svn_stringbuf_t *sb;
+ svn_stringbuf_t *sb = svn_stringbuf_create_empty(pool);
+
+ SVN_ERR_ASSERT((external_parent_url == NULL && external_target == NULL) ||
+ (external_parent_url && external_target));
if (strcmp(path, "") == 0)
{
@@ -160,9 +193,33 @@
if (pb->ctx->cancel_func)
SVN_ERR(pb->ctx->cancel_func(pb->ctx->cancel_baton));
+
+ if (external_parent_url && external_target)
+ {
+ if ((pb->last_external_parent_url == NULL
+ && pb->last_external_target == NULL)
+ || (strcmp(pb->last_external_parent_url, external_parent_url) != 0
+ || strcmp(pb->last_external_target, external_target) != 0))
+ {
+ if (pb->in_external)
+ {
+ /* The external item being listed is different from the previous
+ one, so close the tag. */
+ svn_xml_make_close_tag(&sb, pool, "external");
+ pb->in_external = FALSE;
+ }
- sb = svn_stringbuf_create_empty(pool);
+ svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "external",
+ "parent_url", external_parent_url,
+ "target", external_target,
+ NULL);
+ pb->last_external_parent_url = external_parent_url;
+ pb->last_external_target = external_target;
+ pb->in_external = TRUE;
+ }
+ }
+
svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "entry",
"kind", svn_cl__node_kind_str_xml(dirent->kind),
NULL);
@@ -223,6 +280,8 @@
struct print_baton pb;
svn_boolean_t seen_nonexistent_target = FALSE;
svn_error_t *err;
+ svn_error_t *externals_err = SVN_NO_ERROR;
+ struct svn_cl__check_externals_failed_notify_baton nwb;
SVN_ERR(svn_cl__args_to_target_array_print_reserved(&targets, os,
opt_state->targets,
@@ -264,12 +323,27 @@
if (opt_state->depth == svn_depth_unknown)
opt_state->depth = svn_depth_immediates;
+ if (opt_state->include_externals)
+ {
+ nwb.wrapped_func = ctx->notify_func2;
+ nwb.wrapped_baton = ctx->notify_baton2;
+ nwb.had_externals_error = FALSE;
+ ctx->notify_func2 = svn_cl__check_externals_failed_notify_wrapper;
+ ctx->notify_baton2 = &nwb;
+ }
+
/* For each target, try to list it. */
for (i = 0; i < targets->nelts; i++)
{
const char *target = APR_ARRAY_IDX(targets, i, const char *);
const char *truepath;
svn_opt_revision_t peg_revision;
+
+ /* Initialize the following variables for
+ every list target. */
+ pb.last_external_parent_url = NULL;
+ pb.last_external_target = NULL;
+ pb.in_external = FALSE;
svn_pool_clear(subpool);
@@ -288,11 +362,12 @@
SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
}
- err = svn_client_list2(truepath, &peg_revision,
+ err = svn_client_list3(truepath, &peg_revision,
&(opt_state->start_revision),
opt_state->depth,
dirent_fields,
(opt_state->xml || opt_state->verbose),
+ opt_state->include_externals,
opt_state->xml ? print_dirent_xml : print_dirent,
&pb, ctx, subpool);
@@ -314,20 +389,38 @@
if (opt_state->xml)
{
svn_stringbuf_t *sb = svn_stringbuf_create_empty(pool);
+
+ if (pb.in_external)
+ {
+ /* close the final external item's tag */
+ svn_xml_make_close_tag(&sb, pool, "external");
+ pb.in_external = FALSE;
+ }
+
svn_xml_make_close_tag(&sb, pool, "list");
SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
}
}
svn_pool_destroy(subpool);
+
+ if (opt_state->include_externals && nwb.had_externals_error)
+ {
+ externals_err = svn_error_create(SVN_ERR_CL_ERROR_PROCESSING_EXTERNALS,
+ NULL,
+ _("Failure occurred processing one or "
+ "more externals definitions"));
+ }
if (opt_state->xml && ! opt_state->incremental)
SVN_ERR(svn_cl__xml_print_footer("lists", pool));
if (seen_nonexistent_target)
- return svn_error_create(
- SVN_ERR_ILLEGAL_TARGET, NULL,
- _("Could not list all targets because some targets don't exist"));
+ {
+ err = svn_error_create(SVN_ERR_ILLEGAL_TARGET, NULL,
+ _("Could not list all targets because some targets don't exist"));
+ return svn_error_compose_create(externals_err, err);
+ }
else
- return SVN_NO_ERROR;
+ return svn_error_compose_create(externals_err, err);
}
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1414005)
+++ subversion/svn/main.c (working copy)
@@ -624,7 +624,9 @@
" If locked, the letter 'O'. (Use 'svn info URL' to see details)\n"
" Size (in bytes)\n"
" Date and time of the last commit\n"),
- {'r', 'v', 'R', opt_depth, opt_incremental, opt_xml} },
+ {'r', 'v', 'R', opt_depth, opt_incremental, opt_xml,
+ opt_include_externals },
+ {{opt_include_externals, N_("include externals definitions")}} },
{ "lock", svn_cl__lock, {0}, N_
("Lock working copy paths or URLs in the repository, so that\n"
Index: subversion/tests/cmdline/externals_tests.py
===================================================================
--- subversion/tests/cmdline/externals_tests.py (revision 1414005)
+++ subversion/tests/cmdline/externals_tests.py (working copy)
@@ -2131,7 +2131,7 @@
actions.run_and_verify_update(wc_dir, expected_output, expected_disk,
expected_status, None, None, None, None, None, True, wc_dir)
-def include_externals(sbox):
+def commit_include_externals(sbox):
"commit --include-externals"
# svntest.factory.make(sbox, """
# mkdir Z
@@ -2939,8 +2939,61 @@
actions.run_and_verify_svn2('OUTPUT', expected_stdout, [], 0, 'pg',
'svn:externals', wc_dir)
+@Issue(4225)
+def list_include_externals(sbox):
+ "list with --include-externals"
+
+ externals_test_setup(sbox)
+ wc_dir = sbox.wc_dir
+ repo_url = sbox.repo_url
+
+ svntest.actions.run_and_verify_svn(None, None, [],
+ 'checkout',
+ repo_url, wc_dir)
+ B_path = sbox.ospath("A/B")
+ C_path = sbox.ospath("A/C")
+
+ B_url = repo_url + "/A/B"
+ C_url = repo_url + "/A/C"
+
+ def list_external_string(path, url):
+ string = "Listing external" + " '" + path + "' " + "defined on" + " '" + \
+ url + "'" + ":"
+ return string
+
+ expected_stdout = verify.UnorderedOutput([
+ "E/" + "\n",
+ "F/" + "\n",
+ "lambda" + "\n",
+ list_external_string("gamma", B_url ) + "\n",
+ "gamma" + "\n"])
+
+ exit_code, stdout, stderr = svntest.actions.run_and_verify_svn2(
+ "OUTPUT", expected_stdout, [], 0, 'ls', '--include-externals', B_path)
+
+ exit_code, stdout, stderr = svntest.actions.run_and_verify_svn2(
+ "OUTPUT", expected_stdout, [], 0, 'ls', '--include-externals', B_url)
+
+ expected_stdout = verify.UnorderedOutput([
+ list_external_string("exdir_G", C_url)+ "\n",
+ "pi" + "\n",
+ "rho" + "\n",
+ "tau" + "\n",
+ list_external_string("exdir_H", C_url) + "\n",
+ "chi" + "\n",
+ "omega" + "\n",
+ "psi" + "\n"])
+
+ exit_code, stdout, stderr = svntest.actions.run_and_verify_svn2(
+ "OUTPUT", expected_stdout, [], 0, 'ls', '--include-externals', C_path)
+
+ exit_code, stdout, stderr = svntest.actions.run_and_verify_svn2(
+ "OUTPUT", expected_stdout, [], 0, 'ls', '--include-externals', C_url)
+
+
+
########################################################################
# Run the tests
@@ -2982,13 +3035,14 @@
file_externals_different_url,
file_external_in_unversioned,
copy_file_externals,
- include_externals,
+ commit_include_externals,
include_immediate_dir_externals,
shadowing,
remap_file_external_with_prop_del,
dir_external_with_dash_r_only,
url_to_wc_copy_of_externals,
duplicate_targets,
+ list_include_externals,
]
if __name__ == '__main__':
Fix issue #4225, "Add '--include-externals' option to svn list".
* subversion/include/svn_client.h
(svn_client_list_func2_t): New type used to notify externals information.
(svn_client_list_func_t): Deprecate type.
(svn_client_list3): New function, which has a new argument
include_externals.
(svn_client_list2): Deprecate it.
* subversion/libsvn_client/client.h
(svn_client__list_internal): New function.
(svn_client__list_externals): New function.
* subversion/libsvn_client/deprecated.c
(list_func_wrapper_baton): New struct to deprecate svn_client_list2().
(list_func_wrapper, wrap_list_func): Helper functions to deprecate
svn_client_list2().
(svn_client_list2): Call svn_client_list3 with include_externals set to
FALSE, and use svn_client_list_func2_t as callback implemented by
list_func_wrapper().
* subversion/libsvn_client/externals.c
(list_external_items): New static function that walks through all the
externals under list target recursively and list them using
svn_client_list3().
(svn_client__list_externals): New function to parse externals description
and list it using list_external_items().
* subversion/libsvn_client/list.c
(get_dir_contents): Populate the hash table 'externals'. Use
svn_client_list_func2_t instead of svn_client_list_func_t to report file
and directory entries. Use external_parent_url and external_target when
external items are being listed.
(svn_client__list_internal): New function. Same as svn_client_list3(), it
accepts few additional parameters that carries external information.
If include_externals is set, process all the externals which are populated
by get_dir_contents() using svn_client__list_externals().
(svn_client_list3): New function, thin wrapper around
svn_client__list_internal().
* subversion/svn/list-cmd.c
(print_baton): Add few structure members to keep track of last seen external
information.
(print_dirent): Implement svn_client_list_func2_t to control the output when
used with externals.
(print_dirent_xml): Implement svn_client_list_func2_t. Enclose the external
items in the element <external parent_url=.. target= ..><..></external>
(svn_cl__list): Call svn_client_list3(). Handle if there are any errors
during externals processing.
* subversion/svn/main.c
(svn_cl__cmd_table): Enable include_externals for 'list' and give a short
description about the option.
* subversion/tests/cmdline/externals_tests.py
(include_externals): Rename it to 'commit_include_externals'.
(list_include_externals): New test.
(test_list): Add a reference to the new test. Rename 'include_externals' to
'commit_include_externals'.
Patch by: Vijayaguru G <vijay{_AT_}collab.net>
Review by: stsp, julianfoad