On 14. 1. 26 19:41, Nathan Hartman wrote:
On Wed, Jan 14, 2026 at 9:34 AM Branko Čibej <[email protected]> wrote: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 P.S.: Got caught out by Thunderbird ignoring Reply-To again. Sorry. Hi, If I understand this change correctly, if anything ever goes wrong and causes the assertion to trigger, it would trigger immediately as soon as any 'svn checkout --compatible-version=ARG' command is run, regardless of ARG. I believe the test suite would catch that.
I thought so, too. I think I'll add an assertion to svn_client_wc_version_from_format(), too.
Conceptually I like this refactoring; agreed it's better than codifying the knowledge in different ways in different places. (though I'm probably not the someone with the more insight you were hoping for) Oh, one question though: why is this change is on the better-pristines branch rather than trunk?
I happened to catch the bug on that branch because I added a new format version in libsvn_wc without also updating libsvn_client, so I just fixed it there. Only later did I realise that this could be done on trunk, too. I'll cherry-pick that commit to trunk (without the new format version).
It's not something that would be needed for 1.5.0, though. It might make sense to also port to the branch, just to make future merges easier.
-- Brane
