On Tuesday 13 November 2012 03:02 AM, Stefan Sperling wrote:
Attached the updated patch and log message.

+      /* Notify that we're about to handle an external. */
+      SVN_ERR(list_func(baton, NULL, NULL, NULL, NULL,
+                        externals_parent_url,
+                        item->target_dir, iterpool));

The docstring of svn_client_list_func2_t doesn't say if it is valid
for path or dirent to be NULL.

However, you're passing NULL for these parameters before listing the
external. Do we really need these two extra list_func() calls before
and after listing the external? I was expecting them to go away.



The two extra list_func() calls has gone away!

Attaching the updated patch that addresses your review comments.

Thanks & Regards,
Vijayaguru
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h     (revision 1413464)
+++ 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,44 @@
  * 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.
+ *
+ * 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 +5439,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 +5459,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 1413464)
+++ 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 const char * 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 1413464)
+++ subversion/libsvn_client/deprecated.c       (working copy)
@@ -1282,7 +1282,75 @@
 }
 
 /*** 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;
+}
+
+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 *scratch_pool)
+{
+  struct list_func_wrapper_baton *lfwb = apr_palloc(scratch_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 1413464)
+++ subversion/libsvn_client/externals.c        (working copy)
@@ -1180,3 +1180,108 @@
   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);
+
+      external_items = apr_array_make(iterpool, 1, 
+                                      sizeof(svn_wc_external_item2_t*));
+
+      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 1413464)
+++ 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 1413464)
+++ 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,35 @@
   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 (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 (strcmp(path, "") == 0 && dirent->kind == svn_node_dir 
+              && ! pb->verbose)
+            {
+              /* Don't bother to list if no useful information 
+                 will be shown. */
+              return SVN_NO_ERROR;
+            }
+
+          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 (time_format_long == NULL)
     time_format_long = _("%b %d %H:%M");
   if (time_format_short == NULL)
@@ -133,7 +169,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,12 +177,50 @@
                  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 (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 (strcmp(path, "") == 0 && dirent->kind == svn_node_dir)
+            {
+              /* Don't bother to list if no useful information 
+                 will be shown. */
+              return SVN_NO_ERROR;
+            }
+
+          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;
+            }
+
+          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;
+        }
+    }
+
   if (strcmp(path, "") == 0)
     {
       if (dirent->kind == svn_node_file)
@@ -163,8 +237,6 @@
   if (pb->ctx->cancel_func)
     SVN_ERR(pb->ctx->cancel_func(pb->ctx->cancel_baton));
 
-  sb = svn_stringbuf_create_empty(pool);
-
   svn_xml_make_open_tag(&sb, pool, svn_xml_normal, "entry",
                         "kind", svn_cl__node_kind_str_xml(dirent->kind),
                         NULL);
@@ -225,6 +297,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,
@@ -266,12 +340,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);
 
@@ -290,11 +379,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);
 
@@ -316,20 +406,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 1413464)
+++ subversion/svn/main.c       (working copy)
@@ -358,11 +358,7 @@
                        "                             "
                        "Please run 'svn update' instead.")},
   {"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")},
   {"show-inherited-props", opt_show_inherited_props, 0,
                        N_("retrieve target's inherited properties")},
   {"search", opt_search, 1,
@@ -624,7 +620,8 @@
      "    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} },
 
   { "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 1413464)
+++ 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__options): Short description about include_externals.
  (svn_cl__cmd_table): Enable include_externals for 'list'.

* 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

Reply via email to