On Mar 8 2022, Daniel Shahaf wrote: >> By default Subversion will upgrade the working copy to a version >> compatible with Subversion 1.8 and newer. > > Are we assuming that future minor versions (1.16, 1.17, etc.) will all > continue to be able to read/write f31? This seems to be implied by > the language [...]
It seems desirable at this time, now that we have the mechanism to do so, that we would continue supporting that format for a good long while; but of course I would not promise indefinite support. What I was thinking is we can use language like that for 1.15, and then in a future version if and when we drop support, we can at the same time change the language (and if necessary the format reporting APIs) to report more specific lists of support. Specifically, I felt it was OK to use language like "and newer" in a given release of the tool, without being construed as a promise about versions newer than this version. If anyone thinks anything here could seriously mislead, point it out and let's change it. Daniel Shahaf wrote: > Julian Foad wrote on Thu, Mar 03, 2022 at 10:53:13 +0000: >> [...] it seems clear to me now that we need to expose >> [WC format numbers]. [...] > > Would you elaborate? [...] > What are the awkward semnatics? What are the inconsistencies? What > questions would API users be able to answer for themselves if we hand > them format numbers, that they can't easily answer with trunk@HEAD? Some examples below. >> If we're going to have version numbers as the 'compatible version' UI >> option, perhaps we should eliminate these issues by requiring to >> specify the exact first-introduced version, MAJOR.MINOR format (with >> no .PATCH, no -TAG). > > _Prima facie_ I would -0 this, because it should be possible to do > «/opt/foo/bin/svn upgrade --compatible-version=$(/opt/bar/bin/svn > --version --quiet)» even when bar is v1.9. I suppose in that example the intention is "I want it to be compatible with version <bar>, without me having to learn what formats <bar> supports." The current option parsing seems to have a design intent that reflects this usage. And it is a reasonable use case at first sight. Suppose '/opt/v1.19/bin/svn --version' outputs: * WC format 31, compatible with Subversion v1.8 and newer * WC format 32, compatible with Subversion v1.15 and newer Which WC format did our hypothetical user want? (Rhetorical.) The current implementation gives them the highest format compatible with the requested version. But contrast that to our planned behaviour of 1.15 which is to select the older format (31) by default. To me, that is already showing the cracks in this approach. Consider if we replace your example with (hypothetical UI): «/opt/foo/bin/svn upgrade --compatible-version=$(/opt/bar/bin/svn --version --show-item=wc-compatible-version-default)» which I intend should select compatible-version=1.8. That is getting a bit clearer, less ambiguous, isn't it? Then a step clearer: «/opt/foo/bin/svn upgrade --wc-format=$(/opt/bar/bin/svn --version --show-item=wc-format-default)» Potentially the user can select from "-default", "-min", "-max" variants of that option. (To be clear, I am not proposing we should support such a UI, just making a point about clearer semantics of any UI that deals with formats.) My point is, using the running software version as a proxy for a WC format introduces this ambiguity: is it going to mean the minimum version it supports, its default, or its highest supported format? The language "compatible version" is not clear. One version supports N formats, and one format is supported by M versions, in general. In saying 'compatible version 1.15' it's not necessarily clear to the user how the quoted software version relates to one of the formats that version supports. This is why I think we should do at least one of: - require the exact first-introduced version (1.8 or 1.15) - rename the option to use a less ambiguous language, to something like '--wc-format-version=1.8' (meaning the version in which this format was introduced) or '--wc-format=31' I think those both make it clearer for the user. We need especially to be aware of users encountering this option for the first time, and not deeply knowing what they are doing. A user wanting a format compatible with Subversion minor version <bar>, without considering that there may be more than one compatible format to choose from, may be misled if we silently pick one and don't make that clear. >> In exposing WC format numbers in the APIs, certainly we should treat >> them as opaque identifiers with no intrinsic meaning to their numeric >> value. We could of course also introduce non-numeric identifiers for >> them, for avoidance of 'magic constants' in source code. > > I'm not sure I understand what you have in mind here. Suppose we have > a non-numeric identifier SVN_WC_F31, what now? [...] I meant named constants would be source code, not for UI. Having svn(1) pass SVN_WC_F31 makes it much easier to search for such cases in the source code. The UI could still use --format=31 (note '=': I mean it should be a numeric parameter). >> I am not sure how best to distribute all these APIs between libsvn_wc >> and libsvn_client, and how public to make each of them. [...] >> >> * Move SVN_WC__VERSION and friends from wc.h, at least to the >> inter-library scope of 'include/private/svn_wc_private.h', [...] >> >> - version history of all the WC formats >> - SVN_WC__VERSION, SVN_WC__SUPPORTED_VERSION, SVN_WC__DEFAULT_VERSION >> - SVN_WC__*: minimum format for certain features >> - svn_wc__version_string_from_format() >> - SVN_WC__ERR_IS_NOT_CURRENT_WC() > > Why should we move any of that to include/private/? [except] > SVN_WC__SUPPORTED_VERSION and SVN_WC__VERSION [...] They are all a closely related family. The minimum format numbers for old (no longer supported) features don't need to be used outside libsvn_wc upgrade code, indeed. But the minimum format numbers for new features that are within the range of supported formats DO from now on need to be known by libsvn_client. A new one of them will be introduced with format 32: #define SVN_WC__PRISTINES_ON_DEMAND_VERSION 32 We could split up the list... or keep it all together. Thanks for the detailed review. - Julian
