Re: svn commit: r1898389 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/upgrade.c svn/info-cmd.c svn/svn.c

[Daniel Shahaf <d....@daniel.shahaf.name>]

julianfoad@apache.org wrote on Thu, Feb 24, 2022 at 20:06:32 -0000:
> Multi-WC-format, issue #4884: add svn info --show-item=wc-compatible-version.
> 
> +++ subversion/trunk/subversion/include/svn_client.h Thu Feb 24 20:06:32 2022
> @@ -4451,6 +4451,14 @@ typedef struct svn_client_wc_format_t {
> +/** Return the version of the Subversion library that first supported
> + * the given WC format, @a wc_format.
> + */
> +const svn_version_t *
> +svn_client__wc_version_from_format(int wc_format,
> +                                   apr_pool_t *result_pool,
> +                                   apr_pool_t *scratch_pool);
> +
> +++ subversion/trunk/subversion/libsvn_client/upgrade.c Thu Feb 24 20:06:32 2022
> @@ -200,6 +200,34 @@ svn_client_upgrade2(const char *path,
> +const svn_version_t *
> +svn_client__wc_version_from_format(int wc_format,
> +                                   apr_pool_t *result_pool,
> +                                   apr_pool_t *scratch_pool)
> +{
> +  static const svn_version_t
> +    version_1_0  = { 1, 0, 0, NULL },
> +    version_1_4  = { 1, 4, 0, NULL },
> +    version_1_5  = { 1, 5, 0, NULL },
> +    version_1_6  = { 1, 6, 0, NULL },
> +    version_1_7  = { 1, 7, 0, NULL },
> +    version_1_8  = { 1, 8, 0, NULL },
> +    version_1_15 = { 1, 15, 0, NULL };
> +
> +  switch (wc_format)
> +    {
> +      case  4: return &version_1_0;
> +      case  8: return &version_1_4;
> +      case  9: return &version_1_5;
> +      case 10: return &version_1_6;
> +      case SVN_WC__WC_NG_VERSION: return &version_1_7;

SVN_WC__WC_NG_VERSION is declared in wc.h, which this file isn't allowed
to use.

Why does this function special-case f12 over all other format numbers
only created by unreleased trunk revisions?  Why is f12 handled
differently to f6, f11, f13, f25, and f30?  It seems to me we should
either return NULL for all of them, or return the appropriate
svn_version_t value for each of them (and for all other integers between
11 and 28), as needed.

> +      case 29: return &version_1_7;
> +      case 31: return &version_1_8;
> +      case 32: return &version_1_15;
> +    }
> +  return NULL;
> +}

> +++ subversion/trunk/subversion/svn/info-cmd.c Thu Feb 24 20:06:32 2022
> @@ -397,6 +399,8 @@ static const info_item_map_t info_item_m
>      { SVN__STATIC_STRING("depth"),               info_item_depth },
>      { SVN__STATIC_STRING("changelist"),          info_item_changelist },
>      { SVN__STATIC_STRING("wc-format"),           info_item_wc_format },
> +    { SVN__STATIC_STRING("wc-compatible-version"),
> +                                                 info_item_wc_compatible_version },

The trailing comma is not C89-compatible and used to break the Windows
build.

>    };

Cheers,

Daniel

Re: svn commit: r1898389 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/upgrade.c svn/info-cmd.c svn/svn.c

[Julian Foad <ju...@apache.org>]

Daniel Shahaf wrote:
>> +      case SVN_WC__WC_NG_VERSION: return &version_1_7;
> 
> SVN_WC__WC_NG_VERSION is declared in wc.h, which this file isn't allowed
> to use.

Ack. Similar cases mentioned elsewhere; I'll fix the private include
later as part of API public/private changes.

> Why does this function special-case f12 over all other format numbers

I don't know. I copied it from the existing
'svn_wc__version_string_from_format()'. It seems to me it is legacy (for
developers around 1.7 era) and should be removed from both. I'll remove
it from this one at least.

r1898510.

>> +    { SVN__STATIC_STRING("wc-compatible-version"),
>> +      info_item_wc_compatible_version },
> 
> The trailing comma is not C89-compatible and used to break the Windows
> build.

We use a trailing comma in arrays in Subversion. I think it can stay.

- Julian