Re: [PATCH] Add size information to svn_client_info()

On Sun, 17 Jun 2007, Stefan Küng wrote:

> [[[
> Make svn_client_info() return the size or working size of a file or
> folder too since this information is already available and comes at no
> cost.
>
> * subversion/include/svn_client.h
> (struct svn_info_t): Add members working_size and size.
> (global): Add new define for unknown size
>
> * subversion/libsvn_client/info.c
> (build_info_from_dirent, build_info_from_entry): Adjust accordingly.
> ]]]
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 25433)
> +++ subversion/include/svn_client.h (working copy)
...
> @@ -3821,8 +3828,26 @@
> /* @since New in 1.5. */
> const char *changelist;
> svn_depth_t depth;
> +
> + /**
> + * Size of the file after being translated into local representation,
> + * or:
> + * @c SVN_WC_ENTRY_WORKING_SIZE_UNKOWN if unknown.
> + * @since New in 1.5.
> + */
> + apr_size_t working_size;
> /** @} */
...
> Index: subversion/libsvn_client/info.c
> ===================================================================
> --- subversion/libsvn_client/info.c (revision 25433)
> +++ subversion/libsvn_client/info.c (working copy)
> @@ -54,6 +54,7 @@
> tmpinfo->last_changed_author = dirent->last_author;
> tmpinfo->lock = lock;
> tmpinfo->depth = svn_depth_unknown;
> + tmpinfo->size = dirent->size;

It appears that we need to set tmpinfo->working_size to
SVN_WC_ENTRY_WORKING_SIZE_UNKOWN.

>
> *info = tmpinfo;
> return SVN_NO_ERROR;
> @@ -92,6 +93,8 @@
> tmpinfo->conflict_wrk = entry->conflict_wrk;
> tmpinfo->prejfile = entry->prejfile;
> tmpinfo->changelist = entry->changelist;
> + tmpinfo->working_size = entry->working_size;
> + tmpinfo->size = SVN_INFO_SIZE_UNKNOWN;
>
> /* lock stuff */
> if (entry->lock_token) /* the token is the critical bit. */

I tweaked the doc strings as well (revised patch attached).

I find the bare "size" variable name a little confusing, since for a
directory I'd expect that to list the number of children or something
else useful. Perhaps "repos_file_size" and "working_file_size" would
be more appropriate? Or are we leaving the door open to re-purpose
these fields at a later time?

I think this patch addresses all of the issues raised by glasser,
except for:

stefan wrote:
> glasser wrote:
> >Should these be printed in "svn info"?
>
> I can't really say, I'm not using the CL client that often. I'm also not
> sure if you have some naming conventions for the xml output, so I've
> left that one out.

AIUI, the command-line client would display either the repository file
size, or the translated WC file size, but not both. I don't see
display of this variable file size field as a compelling advantage for
the command-line UI, but nor would I be adverse to seeing it added.

This segways into the last question I have about this change: How do
we test it? We don't currently have a subversion/tests/libsvn_client/
module, because testing of the command-line client has been
traditionally carried out via Python scripts in
subversion/tests/cmdline/ (e.g. info_tests.py). However, if we don't
expose this function via the command-line client, we'll probably
ending up growing API-level tests for libsvn_client. This is not the
first time we're running into this for 1.5 -- Hyrum's commutative
author and revision reporting APIs will also be collecting some
information (for 'blame') that we don't display via the command-line
UI.