On Fri, Apr 5, 2013 at 6:47 PM, <danielsh_at_apache.org> wrote:
> Author: danielsh
> Date: Fri Apr 5 14:47:37 2013
> New Revision: 1464993
>
> URL: http://svn.apache.org/r1464993
> Log:
> 'svnadmin info': sketch the public API.
>
[..]
Duplicating my comments from IRC inline.
> +typedef struct svn_fs_info_t {
> +
> + /** @see svn_fs_type() */
> + const char *fs_type;
> +
> + /** @see svn_fs_get_uuid() */
> + const char *uuid;
> +
> + /** @see svn_fs_youngest_rev() */
> + svn_revnum_t youngest;
> +
> + /** Filesystem format number: an integer that increases when incompatible
> + * changes are made (such as by #svn_fs_upgrade). */
> + int fs_format;
> +
> + /** The oldest Subversion GA release that can read and write this
> + * filesystem. */
> + svn_version_t *supports_version;
> +
> +#if 0
> + /* Potential future feature. */
> + svn_boolean_t is_write_locked;
> +#endif
> +
> + /** Filesystem backend (#fs_type) -specific information.
> + * @see SVN_FS_FSFS_INFO_* */
> + apr_hash_t *fsap_info;
> +
What is the values of this hash table? Always strings? What about
integers or booleans?
> + /** List of user-serviceable config files.
> + * Elements are dirents (as const char *). */
> + apr_array_header_t *config_files;
I don't see real use why config_files information can be useful in
svnadmin info command. So I suggest to remove it until we need it.
> +
> +} svn_fs_info_t;
> +
> +/**
> + * Set @a *info to an info struct describing @a fs.
> + *
> + * @see #svn_fs_info_t
> + *
> + * @since New in 1.8.
> + */
> +svn_error_t *
> +svn_fs_info(const svn_fs_info_t **info,
> + svn_fs_t *fs,
> + apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
> +
> +/**
> + * Return a duplicate of @a info, allocated in @a pool. No part of the new
> + * structure will be shared with @a info.
> + *
> + * @since New in 1.8.
> + */
> +svn_fs_info_t *
> +svn_fs_info_dup(const svn_fs_info_t *info,
> + apr_pool_t *result_pool);
> +
> +/** @name FSFS-specific #svn_fs_info_t information.
> + * @since New in 1.8.
> + * @{
> + */
> +
> +/** Value: shard size (as int), or 0 if the filesystem is
> + * not currently sharded. */
> +#define SVN_FS_FSFS_INFO_SHARDED "sharded"
> +
> +/** Value: abspath to rep-cache.db, or absent if that doesn't exist.
> + @note Do not modify the db schema or tables!
> + */
> +#define SVN_FS_FSFS_INFO_REP_CACHE_PATH "rep-cache-path"
I suggest to replace it with boolean REP_CACHE_ENABLED. In most cases
user is interested wheither rep caching is enabled or not.
rep-cache.db stored in stable location.
[...]
> + */
> +typedef struct svn_repos_info_t {
> +
> + /** Repository format number: an integer that increases when incompatible
> + * changes are made (such as by #svn_repos_upgrade). */
> + int repos_format;
> +
> + /** The oldest Subversion GA release that can read and write this
> + * repository. */
> + svn_version_t *supports_version;
> +
> + /** Set of basenames of hook scripts which have been installed.
> + * Keys are C strings such as "post-commit", values are undefined. */
> + apr_hash_t *hooks_enabled;
> +
> + /** Set of basenames of hook scripts which are respected by this version of
> + * Subversion. Keys are C strings such as "post-commit", values are
> + * undefined.
> + *
> + * @note Hooks are sometimes extended (e.g., by passing additional arguments
> + * to them). In the future we might extend the semantics of this hash to
> + * describe that case, for example by adding keys or defining a meaning for
> + * the values.
> + */
> + apr_hash_t *hooks_known;
> +
Do we really need information about hooks in svnadmin info command?
What is the behavior is hook file is present but empty? I suggest to
remove this information from svn_repos_info_t and implemented as
different API if needed.
--
Ivan Zhakov
Received on 2013-04-05 17:41:10 CEST