Re: svn commit: r1931314 - in subversion/branches/better-pristines/subversion: include/private libsvn_client libsvn_wc tests/cmdline/svntest

On 14. 1. 26 14:36, [email protected] wrote:
> Author: brane
> Date: Wed Jan 14 13:36:52 2026
> New Revision: 1931314
>
> Log:
> On the better-pristines branch: Move all knowledge of the relation between
> working copy formats and Subversion versions into libsvn_wc. Having them
> in two places was slightly unmaintainable if the format numbers changed.
[...]
> @@ -288,10 +276,14 @@ svn_client_get_wc_formats_supported(apr_
>   const svn_version_t *
>   svn_client_oldest_wc_version(apr_pool_t *result_pool)
>   {
> -  /* NOTE: For consistency, always return the version of the client
> -     that first introduced the format. */
> -  static const svn_version_t version = { 1, 8, 0, NULL };
> -  return &version;
> +  const svn_wc__version_info_t *const version_info =
> +    svn_wc__version_info_from_format(SVN_WC__SUPPORTED_VERSION);
> +
> +  /* NOTE: This can be the "null" version "0.0.0" and that's an
> +           unrecoverable programming error. */
> +  SVN_ERR_ASSERT_NO_RETURN(version_info->text != NULL);
> +
> +  return &version_info->version;
>   }

I have a question for someone with a bit more insight into the 
multi-wc-version support code. On the better-pristines branch, I removed 
these hard-coded constants and move the format -> version mapping to 
libsvn_wc, to keep it in one place.

As you can see above, I added assertions. They should never trigger if 
the format versions and their mapping are defined correctly, and I think 
it's better to abort() than to return NULL and crash instead.

So, any opinions on this change? Note that 
svn_client_wc_version_from_format() still returns NULL if our internal 
data are invalid which, IMO, is not so nice and also harder to track 
down than an assertion.

-- Brane