julianf...@apache.org wrote on Thu, Feb 24, 2022 at 15:58:11 -0000:
> Multi-WC-format: Clarify the supported versions display.
>
> This patch:
>
> - Changes the APIs for querying the default WC format and the supported WC
> formats;
> - clarifies the display of supported WC formats in 'svn --version'.
>
> API changes:
>
> - Remove svn_client_supported_wc_version().
> - Add svn_client_default_wc_version().
> - Add svn_client_supported_wc_formats() and a type it returns.
>
Looks good. Just a few comments:
> CLI changes:
>
> Old display in 'svn --version':
>
> | Supported working copy (WC) versions: from 1.8 to 1.15
>
> New display in 'svn --version':
>
> | Supported working copy (WC) versions:
> |
> | * compatible with Subversion v1.8 to v1.15 (WC format 31)
> | * compatible with Subversion v1.15 (WC format 32)
Nice!
> The list style, along with inclusion of the WC format number, helps show
> that each line describes a distinct format. Users sometimes also need to
> know about WC format numbers, and the 'version' command is an appropriate
> place to show these. The presentation style matches that used for the lists
> of RA modules and credential caches.
>
> * subversion/include/svn_client.h,
> subversion/libsvn_client/upgrade.c
> (svn_client_checkout4,
> svn_client_upgrade2): Update doc string.
> (svn_client_supported_wc_version): Remove.
> (svn_client_default_wc_version,
> svn_client_wc_format_t,
> svn_client_supported_wc_formats): New.
>
> * subversion/libsvn_client/checkout.c
> (svn_client_checkout4): Update caller: use svn_client_default_wc_version().
>
> * subversion/libsvn_wc/wc.h
> (SVN_WC__SUPPORTED_VERSION): Update doc string.
>
> * subversion/svn/help-cmd.c
> (print_supported_wc_formats): New.
> (svn_cl__help): Use it to display supported WC formats as a list.
>
> * subversion/svn/svn.c
> (parse_compatible_version): Update caller: use
> svn_client_supported_wc_formats().
>
> * subversion/tests/cmdline/getopt_tests_data/svn--version_stdout,
> subversion/tests/cmdline/getopt_tests_data/svn--version--verbose_stdout
> Update expected output.
> +++ subversion/trunk/subversion/include/svn_client.h Thu Feb 24 15:58:10 2022
> @@ -4428,13 +4428,40 @@ svn_client_upgrade(const char *wcroot_di
> apr_pool_t *scratch_pool);
>
> /**
> - * Returns the version related to the earliest supported
> + * Returns the version related to the library's default
> * working copy metadata format.
> *
> * @since New in 1.15.
> */
> const svn_version_t *
> -svn_client_supported_wc_version(void);
> +svn_client_default_wc_version(apr_pool_t *result_pool);
> +
> +/**
> + * Information about a WC version.
> + *
> + * Only the @c .major and @c .minor version fields are significant: so a
> + * version_max value of 1.15.0 for example means "up to 1.15.x".
Missing @since.
Missing warning not to manually allocate structs of this type since
fields may be added in the future.
> + */
> +typedef struct svn_client_wc_format_t {
> + /* Oldest version of svn libraries known to support this WC version */
> + const svn_version_t *version_min;
> + /* Newest version of svn libraries known to support this WC version. */
> + const svn_version_t *version_max;
> + /* The WC format number of this format, as defined by libsvn_wc. */
> + int wc_format;
> +} svn_client_wc_format_t;
> +
> +/**
> + * Returns a list of the WC formats supported by the client library.
> + *
> + * The list is sorted from oldest to newest, and terminated by an entry
> + * containing all null/zero fields.
> + *
> + * The returned data are allocated in @a result_pool and/or statically.
Missing @since.
> + */
> +const svn_client_wc_format_t *
> +svn_client_supported_wc_formats(apr_pool_t *result_pool,
> + apr_pool_t *scratch_pool);
> +++ subversion/trunk/subversion/libsvn_client/upgrade.c Thu Feb 24 15:58:10
> 2022
> @@ -41,6 +41,7 @@
>
> #include "svn_private_config.h"
> #include "private/svn_wc_private.h"
> +#include "../libsvn_wc/wc.h"
I don't think libsvn_client is allowed to use this header.
libsvn_foo/bar.h headers are for internal use by libsvn_foo, not for
interlibrary use; that would be include/private/. We generally follow
this, too:
% grep -R 'include.*[.][.]/libsvn' subversion/lib* | grep -Eo '[<"].*[">]' |
sort | uniq -c
14 "../../libsvn_fs/fs-loader.h"
2 "../libsvn_delta/delta.h"
53 "../libsvn_fs/fs-loader.h"
1 "../libsvn_fs_base/fs_init.h"
1 "../libsvn_fs_fs/fs_init.h"
1 "../libsvn_fs_x/fs_init.h"
24 "../libsvn_ra/ra_loader.h"
3 "../libsvn_ra/wrapper_template.h"
%
The RA/FS ones are presumably related to the pluggable design of those
layers. (Sorry, I haven't got time to confirm this in detail.)
The delta.h one is in FSFS/FSX, which, as the comment there claims, use
SVN_DELTA_WINDOW_SIZE and nothing else declared or #define'd in that
header.
> +++
> subversion/trunk/subversion/tests/cmdline/getopt_tests_data/svn--version_stdout
> Thu Feb 24 15:58:10 2022
> @@ -6,7 +6,10 @@ This software consists of contributions
> see the NOTICE file for more information.
> Subversion is open source software, see http://subversion.apache.org/
>
> -Supported working copy (WC) versions: from 1.8 to 1.15
> +Supported working copy (WC) formats:
> +
> +* compatible with Subversion v1.8 to v1.15 (WC format 31)
> +* compatible with Subversion v1.15 (WC format 32)
>
Bikeshed, but the bullet point parses as a sentence fragment. Suggest
instead:
* format 31, compatible with Subversion v1.8 to v1.15
> The following repository access (RA) modules are available:
>
Cheers,
Daniel
