On Jul 31, 2005, at 5:51 PM, Alexander Thomas wrote:
>
> /**
> - * Set @a *dirents to a newly allocated hash of entries for @a
> + * Set @a *dirents and *locks to a newly allocated hash of entries
> for @a
> * path_or_url at @a revision. The actual node revision selected is
> * determined by the path as it exists in @a peg_revision. If @a
> * peg_revision is @c svn_opt_revision_unspecified, then it defaults
> @@ -1810,18 +1810,36 @@
> * @a path_or_url is a file, return only the dirent for the file.
> If @a
> * path_or_url is non-existent, return @c SVN_ERR_FS_NOT_FOUND.
> *
> - * The hash maps entry names (<tt>const char *</tt>) to @c
> svn_dirent_t *'s.
> - * Do all allocation in @a pool.
> + * The dirents hash maps entry names (<tt>const char *</tt>) to
> + * @c svn_dirent_t *'s. Do all allocation in @a pool.
> *
> + * The locks hash maps entry names (<tt>const char *</tt>) to
> + * @c svn_lock_t *'s. Do all allocation in @a pool.
> + *
> * 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.
> *
> - * @since New in 1.2.
> + * @since New in 1.3.
> */
The svn_client_ls3() docstring needs to say that the locks hash is
optional.
> svn_error_t *
> +svn_client_ls3 (apr_hash_t **dirents,
> + apr_hash_t **locks,
> + const char *path_or_url,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +/**
> + * Similar to svn_client_ls3(), return only dirent's in the form
> hash.
"form" hash? I think that's a typo. I think the docstring just
needs to say, "same as svn_client_ls3, but always passes a NULL lock
hash."
> + *
> + * @deprecated Provided for backward compatibility with the 1.2 API.
> + */
> +svn_error_t *
> svn_client_ls2 (apr_hash_t **dirents,
> const char *path_or_url,
> const svn_opt_revision_t *peg_revision,
> Index: subversion/libsvn_client/ls.c
> ===================================================================
> --- subversion/libsvn_client/ls.c (revision 15448)
> +++ subversion/libsvn_client/ls.c (working copy)
> @@ -71,6 +71,79 @@
> }
>
> svn_error_t *
> +svn_client_ls3 (apr_hash_t **dirents,
> + apr_hash_t **locks,
> + const char *path_or_url,
> + const svn_opt_revision_t *peg_revision,
> + const svn_opt_revision_t *revision,
> + svn_boolean_t recurse,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + svn_ra_session_t *ra_session;
> + const char *url;
> + const char *repos_root;
> + svn_revnum_t rev;
> + apr_hash_index_t *hi;
> + const char *rel_path;
> + svn_node_kind_t kind;
> + apr_hash_t *new_locks;
> + apr_pool_t *subpool;
> +
> + SVN_ERR (svn_client_ls2 (dirents, path_or_url, peg_revision,
> + revision, recurse, ctx, pool));
> +
Here's the big problem: you've defined the new function in terms of
the old function, which is backwards. When you deprecate a function,
it means that subversion has to stop using it absolutely everywhere.
Our standard technique for rev'ving an API is to define the
deprecated function in terms of the new function. So svn_client_ls2
() should simply be a call to svn_client_ls3() with a NULL lock
hash. And what was formerly svn_client_ls2() should be renamed to
ls3, and extra lock features then added to it.
> + if (locks == NULL)
> + return SVN_NO_ERROR;
> +
> + /* Getting repository root. */
> + SVN_ERR (svn_client_url_from_path (&repos_root, "", pool));
> +
> + /* Get an RA plugin for this filesystem object. */
> + SVN_ERR (svn_client__ra_session_from_path (&ra_session, &rev,
> + &url, path_or_url,
> peg_revision,
> + revision, ctx, pool));
> +
... which means there will be no need to open a new ra_session,
right? :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Aug 8 15:30:03 2005