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. 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? Cheers, Nathan
