[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: (for sparse-directories) Question about listing directories.

From: Karl Fogel <kfogel_at_red-bean.com>
Date: 2007-04-10 09:46:45 CEST

kfogel@tigris.org writes:
> Upgrade svn_client_list() with recurse to svn_client_list2() with depth.
>
> [...]
>
> * subversion/svn/list-cmd.c
> (print_dirent, print_dirent_xml): When verbose, print this-dir
> entry instead of ignoring it.
>
> [...]

Okay, I committed this in r24510. Before r24510, 'svn ls' behavior
was like so:

   $ svn ls
   [ ...would show nothing about '.' in output... ]
   $ svn ls .
   [ ...would show nothing about '.' in output... ]
   $ svn ls -v
   [ ...would show nothing about '.' in output... ]
   $ svn ls -v .
   [ ...would show nothing about '.' in output... ]
   $ svn ls -v SOME_DIR
   [ ...would show nothing about SOME_DIR itself in output... ]
   $

After r24510, the '-v' versions of the above commands would show '.'
or SOME_DIR in the output. The rationale for this is that in the '-v'
case there's actually some information to show, and if you were to
invoke

   $ svn ls --depth=empty .

or

   $ svn ls --depth=empty SOME_SUBDIR

...you'd presumably want to see that information.

Now, don't panic! :-)

We can tweak this new behavior any way we want. I can revert it to
the old behavior in a jiffy. Let's decide what we want.

In IRC, jszakmeister and I were speculating that the old behavior was
meant to match Unix 'ls', which also doesn't show anything about its
target when the target is a directory (unless you pass the -d flag).

Options right now are:

   1. Keep current (new) behavior.

   2. Revert, so that target directory is never shown in output.

   3. Make it so that --depth=empty is like Unix 'ls -d' (i.e., shows
      target directory), but for all other depths target directory is
      not shown.

   4. Something else?

I lean toward (1) or (3). If the r24510 behavior is considered an
incompatible change, then IMHO we should do (3), since --depth=empty
clearly cannot have any compatibility concerns.

Thoughts?

-Karl

> Modified:
> trunk/notes/sparse-directories.txt
> trunk/subversion/include/svn_client.h
> trunk/subversion/libsvn_client/list.c
> trunk/subversion/svn/list-cmd.c
>
> Modified: trunk/notes/sparse-directories.txt
> URL: http://svn.collab.net/viewvc/svn/trunk/notes/sparse-directories.txt?pathrev=24510&r1=24509&r2=24510
> ==============================================================================
> --- trunk/notes/sparse-directories.txt (original)
> +++ trunk/notes/sparse-directories.txt Tue Apr 10 00:23:51 2007
> @@ -381,7 +381,6 @@
> that all of these should be switched to depth, but the
> question needs some more consideration:
>
> - svn_client_list() # Yes, probably should take depth.
> svn_client_import2() # Note: takes 'nonrecursive' right now
> svn_client_revert() # Any point taking depth?
> svn_client_commit4() # Is manual control of depth needed here?
>
> Modified: trunk/subversion/include/svn_client.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/include/svn_client.h?pathrev=24510&r1=24509&r2=24510
> ==============================================================================
> --- trunk/subversion/include/svn_client.h (original)
> +++ trunk/subversion/include/svn_client.h Tue Apr 10 00:23:51 2007
> @@ -3279,7 +3279,7 @@
> * @{
> */
>
> -/** Invoked by svn_client_list() for each @a path with its @a dirent and,
> +/** Invoked by svn_client_list2() for each @a path with its @a dirent and,
> * if @a path is locked, its @a lock. @a abs_path is the filesystem path
> * to which @a path is relative. @a baton is the baton passed to the
> * caller. @a pool may be used for temporary allocations.
> @@ -3314,18 +3314,38 @@
> * Use authentication baton cached in @a ctx to authenticate against the
> * repository.
> *
> - * If @a recurse is true (and @a path_or_url is a directory) this will
> - * be a recursive operation.
> - *
> - * ### TODO(sd): This really should take depth instead of recurse, but
> - * ### one thing at a time, one thing at a time...
> + * If @a depth is @c svn_depth_empty, list just @a path_or_url itself.
> + * If @a depth is @c svn_depth_files, list @a path_or_url and its file
> + * entries. If @c svn_depth_immediates, list its immediate file and
> + * directory entries. If @c svn_depth_infinity, list file entries and
> + * recurse (with @c svn_depth_infinity) on directory entries.
> *
> * @a dirent_fields controls which fields in the @c svn_dirent_t's are
> * filled in. To have them totally filled in use @c SVN_DIRENT_ALL,
> * otherwise simply bitwise OR together the combination of @c SVN_DIRENT_
> * fields you care about.
> *
> + * @since New in 1.5.
> + */
> +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);
> +
> +/* Like svn_client_list2(), but with @a recurse instead of @a depth.
> + * If @a recurse is true, pass @c svn_depth_files for @a depth; else
> + * pass @c svn_depth_infinity.
> + *
> * @since New in 1.4.
> + *
> + * @deprecated Provided for backward compatibility with the 1.4 API.
> */
> svn_error_t *
> svn_client_list(const char *path_or_url,
>
> Modified: trunk/subversion/libsvn_client/list.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/list.c?pathrev=24510&r1=24509&r2=24510
> ==============================================================================
> --- trunk/subversion/libsvn_client/list.c (original)
> +++ trunk/subversion/libsvn_client/list.c Tue Apr 10 00:23:51 2007
> @@ -32,7 +32,13 @@
> /* Get the directory entries of DIR at REV (relative to the root of
> RA_SESSION), getting at least the fields specified by DIRENT_FIELDS.
> Use the cancellation function/baton of CTX to check for cancellation.
> - If RECURSE is TRUE, recurse into child directories.
> +
> + If DEPTH is svn_depth_empty, return immediately. If DEPTH is
> + svn_depth_files, invoke LIST_FUNC on the file entries with BATON;
> + if svn_depth_immediates, invoke it on file and directory entries;
> + if svn_depth_infinity, invoke it on file and directory entries and
> + recurse into the directory entries with the same depth.
> +
> 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.
> @@ -44,7 +50,7 @@
> svn_ra_session_t *ra_session,
> apr_hash_t *locks,
> const char *fs_path,
> - svn_boolean_t recurse,
> + svn_depth_t depth,
> svn_client_ctx_t *ctx,
> svn_client_list_func_t list_func,
> void *baton,
> @@ -55,6 +61,9 @@
> apr_array_header_t *array;
> int i;
>
> + if (depth == svn_depth_empty)
> + return;
> +
> /* Get the directory's entries, but not its props. */
> SVN_ERR(svn_ra_get_dir2(ra_session, &tmpdirents, NULL, NULL,
> dir, rev, dirent_fields, pool));
> @@ -83,11 +92,14 @@
> else
> lock = NULL;
>
> - SVN_ERR(list_func(baton, path, the_ent, lock, fs_path, iterpool));
> + 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));
>
> - if (recurse && the_ent->kind == svn_node_dir)
> + 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, recurse, ctx,
> + ra_session, locks, fs_path, depth, ctx,
> list_func, baton, iterpool));
> }
>
> @@ -96,16 +108,16 @@
> }
>
> svn_error_t *
> -svn_client_list(const char *path_or_url,
> - const svn_opt_revision_t *peg_revision,
> - const svn_opt_revision_t *revision,
> - svn_boolean_t recurse,
> - 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_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_ra_session_t *ra_session;
> svn_revnum_t rev;
> @@ -246,13 +258,40 @@
> APR_HASH_KEY_STRING))
> : NULL, fs_path, pool));
>
> - if (dirent->kind == svn_node_dir)
> + 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, "", rev, ra_session, locks,
> - fs_path, recurse, ctx, list_func, baton, pool));
> + fs_path, depth, ctx, list_func, baton, pool));
>
> return SVN_NO_ERROR;
> }
>
> +svn_error_t *
> +svn_client_list(const char *path_or_url,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + 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)
> +{
> + return svn_client_list(path_or_url,
> + peg_revision,
> + revision,
> + SVN_DEPTH_FROM_RECURSE(recurse),
> + dirent_fields,
> + fetch_locks,
> + list_func,
> + baton,
> + ctx,
> + pool);
> +}
> +
> /* Baton used by compatibility wrapper svn_client_ls3. */
> struct ls_baton {
> apr_hash_t *dirents;
>
> Modified: trunk/subversion/svn/list-cmd.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/svn/list-cmd.c?pathrev=24510&r1=24509&r2=24510
> ==============================================================================
> --- trunk/subversion/svn/list-cmd.c (original)
> +++ trunk/subversion/svn/list-cmd.c Tue Apr 10 00:23:51 2007
> @@ -61,7 +61,10 @@
> {
> if (dirent->kind == svn_node_file)
> entryname = svn_path_basename(abs_path, pool);
> + else if (pb->verbose)
> + entryname = ".";
> else
> + /* Don't bother to list if no useful information will be shown. */
> return SVN_NO_ERROR;
> }
> else
> @@ -140,7 +143,10 @@
> {
> if (dirent->kind == svn_node_file)
> entryname = svn_path_basename(abs_path, pool);
> + else if (pb->verbose)
> + entryname = ".";
> else
> + /* Don't bother to list if no useful information will be shown. */
> return SVN_NO_ERROR;
> }
> else
> @@ -275,13 +281,13 @@
> SVN_ERR(svn_cl__error_checked_fputs(sb->data, stdout));
> }
>
> - SVN_ERR(svn_client_list(truepath, &peg_revision,
> - &(opt_state->start_revision),
> - SVN_DEPTH_TO_RECURSE(opt_state->depth),
> - dirent_fields,
> - (opt_state->xml || opt_state->verbose),
> - opt_state->xml ? print_dirent_xml : print_dirent,
> - &pb, ctx, subpool));
> + SVN_ERR(svn_client_list2(truepath, &peg_revision,
> + &(opt_state->start_revision),
> + opt_state->depth,
> + dirent_fields,
> + (opt_state->xml || opt_state->verbose),
> + opt_state->xml ? print_dirent_xml : print_dirent,
> + &pb, ctx, subpool));
>
> if (opt_state->xml)
> {
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 10 09:47:11 2007

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.