Re: svn commit: r1899014 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
On Fri, Mar 18, 2022 at 12:01 PM Daniel Shahaf wrote: > > Jun Omae wrote on Fri, Mar 18, 2022 at 11:27:16 +0900: > > Hi, > > > > On Fri, Mar 18, 2022 at 9:44 AM Daniel Shahaf > > wrote: > > > > > > Could someone test this on Windows, please? I suspect read_wc_formats() > > > (added in r1899012) returns paths with os.sep, but the expectations > > > added in this commit use '/', so something will need to convert. > > > > > > Thanks, > > > > > > Daniel > > > > I got 3 failures from testing r1899017 on Windows. dav-tests.log is > > attached. > > See also > > https://github.com/jun66j5/subversion/runs/5594602036?check_suite_focus=true#step:7:2816 > > > > Thank you! > > Does r1899019 fix this? (resend to dev@...) Yeah, the failures go away on tests for r1899019. [[[ Summary of test results: 2593 tests PASSED 127 tests SKIPPED 77 tests XFAILED (17 WORK-IN-PROGRESS) Python version: 3.7.9. SUMMARY: All tests successful ]]] https://github.com/jun66j5/subversion/runs/5595682577?check_suite_focus=true#step:7:2816 -- Jun Omae (大前 潤)
Re: Issue #525/#4892: on only fetching the pristines we really need
On Mon, Mar 14, 2022 at 6:48 AM Julian Foad wrote: > > Dear dev community, and especially Karl and Mark: > > A plea to test the current design/implementation. Feedback so far on the 'pristines-on-demand-on-mwf' branch as of r1898990: tl;dr: Pretty darn good for a first cut! I merged pristines-on-demand-on-mwf to trunk, built, and tried it on a real workload. Admittedly I did not run the test suite (though I plan to). Instead, every step along the way, I compared the 1.15+i525pod WC directory contents (excluding .svn) to those of a parallel 1.14 WC checked out and operated upon by a 1.14 client and things seem to be working correctly, at least insofar as actual files and their contents are concerned. Repo access was via "svn://" (svnserve). As of r1898990 it merged cleanly to trunk and built successfully (on Debian Linux). The "real workload" mentioned consisted of a working copy containing several svn:externals; I checked that out, but most operations were done within one of the nested WCs, which contains the majority of the data. That WC contains a total of 19161 versioned files. When checked out without i525pod: Total 38141 files including pristines occupying 512M. When checked out with i525pod: Total 19166 files occupying 270M, and SVN correctly set the parent and nested WCs to f32 format. The versioned contents are source files; not exactly a huge WC by today's standards but believe it or not this makes a big difference for me, as I often operate on WCs this size directly on embedded systems whose storage is somewhat constrained, and also on ramdrives, where it's nice to cut usage in half. I performed operations: checkout, info, switch, log, patch, update (including restore), cleanup, merge, diff, status, commit. I performed several commits. To verify that nothing became hosed here, I then performed a fresh checkout with a 1.14 client and compared its contents to the expected known state, which was the contents of the 1.15+i525pod WC. Also I ran 'svnadmin verify' on the repo (which is on a separate machine) and it verified successfully. (Since the commits succeeded, I didn't expect otherwise.) Speed of client operations didn't feel noticeably slower; maybe just a little bit for some operations for which pristines had to be fetched, but as the files in question were not huge, it was not very impactful. I understand that this will probably be more noticeable when the WC contains huge files that are modified but don't have a convenient way to test that at the moment. I used the plaintext credential cache and was not prompted for passwords at any time. (I should check with a credential method that does result in prompts and follow up... I understand it may currently prompt twice for one invocation.) One thing I noticed: With i525pod enabled, svn does not delete empty .svn/pristine/??/ subdirectories when no longer needed. (Not sure what the correct terminology is for those 2-digit-hex-code dirs.) It does purge the pristine files within these subdirectories, just not the subdirectories themselves. They even survive a 'svn cleanup --vacuum-pristines'. I haven't yet looked at the code to see whether non-i525pod SVN ever deletes these or not, or if there's a simple fix. I don't consider this a showstopper or a terribly big deal but unless this is expected behavior, I'd like to at least file an issue for it. More below inline... > We are worried that the current design won't be acceptable because it > has poor behaviour in a particular use case. > > The use case involved running "svn update" at the root of the WC. (It > didn't explicitly say that. More precisely, it implied the update target > tree contains the huge locally modified file.) As I said above, I didn't have a convenient way to test this use case but it would be informative to conjure one up (or wait for feedback from users with such a use case). > Using this new feature necessarily requires some adjustments to user > expectations and work flow. > > What if we ask the user to limit their "svn update" to target the > particular files/paths that they need to update, keeping their huge > locally modified file out of its scope? Examples: > > svn update readme.txt > svn update small-docs/ > # BUT NOT: svn update the-whole-wc/ > > Then we side-step the issue. It only fetches pristines for modified > files that are within the tree scope of the specified targets. (This is > how it works already, not a proposal.) > > OK that's not optimal but it might be sufficient. We could suggest that as a workaround in the release notes and point out that this could be optimized gradually in future releases. > (Of course there are further concerns, such as what happens if the user > starts an update at the WC root, then cancels it as it's taking too > long: can we gracefully recover? Fine, we can look at those concerns.) > > I can go ahead with further work on changing the design if required, but > I am concerned that might not be the best use of res
Re: svn commit: r1899014 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
Jun Omae wrote on Fri, Mar 18, 2022 at 11:27:16 +0900: > Hi, > > On Fri, Mar 18, 2022 at 9:44 AM Daniel Shahaf wrote: > > > > Could someone test this on Windows, please? I suspect read_wc_formats() > > (added in r1899012) returns paths with os.sep, but the expectations > > added in this commit use '/', so something will need to convert. > > > > Thanks, > > > > Daniel > > I got 3 failures from testing r1899017 on Windows. dav-tests.log is attached. > See also > https://github.com/jun66j5/subversion/runs/5594602036?check_suite_focus=true#step:7:2816 > Thank you! Does r1899019 fix this?
Re: svn commit: r1899014 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
Hi, On Fri, Mar 18, 2022 at 9:44 AM Daniel Shahaf wrote: > > Could someone test this on Windows, please? I suspect read_wc_formats() > (added in r1899012) returns paths with os.sep, but the expectations > added in this commit use '/', so something will need to convert. > > Thanks, > > Daniel I got 3 failures from testing r1899017 on Windows. dav-tests.log is attached. See also https://github.com/jun66j5/subversion/runs/5594602036?check_suite_focus=true#step:7:2816 [[[ FAIL: upgrade_tests.py 2: upgrade with externals FAIL: upgrade_tests.py 7: test upgrading a working copy created with 1.0.0 FAIL: upgrade_tests.py 35: test upgrading 1.0.0 working copy with externals Summary of test results: 2590 tests PASSED 127 tests SKIPPED 77 tests XFAILED (17 WORK-IN-PROGRESS) 3 tests FAILED Python version: 3.7.9. SUMMARY: Some tests failed ]]] The build uses the following libraries: [[[ Found apr 1.6.5 Found apr-util 1.6.1 Found apr_memcache 1.6.1 Found expat 2.4.1 Found httpd 2.4.52 Found libintl 0.21.0 Found mod_dav 2.4.52 Found openssl 1.1.1m Found perl 5.32.1 Found ruby 2.5.0 Found serf 1.4.0 Found sqlite 3.38.1 Found swig 4.0.2 Found zlib 1.2.11 Using bundled lz4 1.7.5 Using bundled utf8proc 2.1.0 ]]] -- Jun Omae (大前 潤) dav-fails.log Description: Binary data
Re: Issue #525/#4892: on only fetching the pristines we really need
Hi -- I've just absorbed all of this thread that was new since the last time I read it (that's a lot! :-) ). I agree with Julian's judgement that we should just ship the MVP version of our issue #525 solution with 'svn update' fetching pristines for locally-modified files. While it's not ideal, it's also not a showstopper, and I don't think it's worth increasing time-to-ship for this feature over this relatively minor point. 1) It's "just" a performance issue, not a correctness issue. 2) It can be improved in the future. 3) For users who would be bitten by it, there are several easy workarounds available: target one's updates to avoid stimulating the fetch-pristine behavior; or, copy a file locally before operating on it; or, sequence one's work so as to only have one modified file around at a given time. The 525-enabled Subversion will still be a huge win, even with this small blemish. Many thanks to Evgeny for pointing out the complexities, likewise to Julian for his very patient explanations and re-explanations. And I'd like to personally thank Mark for fighting the good fight :-). It sounds like we have consensus that this is an implementation-driven behavior not a usage-driven behavior, so if/when one day we go to fix it at least we'll already have agreement on that as a goal. Best regards, -Karl On 14 Mar 2022, Julian Foad wrote: Dear dev community, and especially Karl and Mark: A plea to test the current design/implementation. I wonder if we are missing some perspective. We are worried that the current design won't be acceptable because it has poor behaviour in a particular use case. The use case involved running "svn update" at the root of the WC. (It didn't explicitly say that. More precisely, it implied the update target tree contains the huge locally modified file.) Using this new feature necessarily requires some adjustments to user expectations and work flow. What if we ask the user to limit their "svn update" to target the particular files/paths that they need to update, keeping their huge locally modified file out of its scope? Examples: svn update readme.txt svn update small-docs/ # BUT NOT: svn update the-whole-wc/ Then we side-step the issue. It only fetches pristines for modified files that are within the tree scope of the specified targets. (This is how it works already, not a proposal.) OK that's not optimal but it might be sufficient. (Of course there are further concerns, such as what happens if the user starts an update at the WC root, then cancels it as it's taking too long: can we gracefully recover? Fine, we can look at those concerns.) I can go ahead with further work on changing the design if required, but I am concerned that might not be the best use of resources. Also I don't know how to evaluate the balance of Evgeny's concerns about protocol level complexity of the alternative design, against the concerns about the present design. In other words pursuing that alternative seems riskier, while accepting the known down-sides of the current design is sub-optimal but seems less risky. Should we first test the current design and see if we can work with it, before going full steam ahead into changing the design? The current design/implementation (on branch 'pristines-on-demand-on-mwf') is in a working state. There are open issues that still need to be resolved, but it's complete enough to be ready for this level of testing. - Julian On 14 Mar 2022, Mark Phippard wrote: On Mon, Mar 14, 2022 at 6:48 AM Julian Foad wrote: Dear dev community, and especially Karl and Mark: A plea to test the current design/implementation. I wonder if we are missing some perspective. Hi Julian, I do not believe I can offer much in the way of testing, but I do want to reiterate that I am not objecting to the current state of the change. At least not in the veto sense. My work has taken me away from SVN. I just wanted to bring some user perspective into the conversation. I realize there may be considerations that make it not the best option to try to solve this. I will have to leave that up to you ... and Karl if he has helped fund this effort. Karl did a great job explaining why the current behavior will be unexpected to the user. I agree they may have workarounds they can use to manage it. How a user feels about those workarounds is hard to predict. Good luck Mark On 14 Mar 2022, Julian Foad wrote: Mark Phippard wrote: [...] I just wanted to bring some user perspective [...] Thanks, Mark. Understood. I also want to clarify that this is my pragmatic side speaking. For anyone who doesn't remember me saying this before, I'll repeat that my purist side would love to see us do the alternative, optimal, design, and work through the consequent protocol issues. That seems obviously to me more "Right", but is only useful if we could afford to follow it through to completion, and we don't hav
Re: multi-wc-format: upgrading externals
Daniel Shahaf wrote on Thu, 17 Mar 2022 23:02 +00:00:
> Daniel Shahaf wrote on Tue, Mar 08, 2022 at 10:57:17 +:
>> Julian Foad wrote on Wed, Mar 02, 2022 at 13:04:51 +:
>> > Daniel Shahaf wrote:
>> > > multi-wc-format/BRANCH-README mentioned this:
>> > >
>> > >> [*] New externals working copies must inherit the format from their
>> > >>parent working copy, because [...]
>> > >
>> > > Upgrading a parent working copy upgrades external wc's too. However,
>> > > upgrading an external succeeds. Judging by the quoted remark, should
>> > > «svn upgrade --compatible-version=$N /path/to/external» error out unless
>> > > the external's parent working copy is already at version $V?
>> >
>> > It isn't clear to me whether allowing it or disallowing it is more "right".
>> >
>> > Can anyone else chime in?
>> >
>>
>> Hmm. Considering that «svn update» recurses into externals by default,
>> but that nothing recurses upwards into parent wc's by default, perhaps
>> we should design things around making sure these two cases continue to
>> work? I.e., disallow selective upgrades that might make another
>> client's «svn update» of the outer wc fail because the outer wc and the
>> external wc are different formats?
>>
>> Following this train of thought, we'll forbid upgrading an external
>> without also upgrading a parent wc, but will entertain patches to make
>> «svn upgrade» _not_ descend into external wc's by default, should anyone
>> submit such. (I don't propose we add this ourselves for the MVP.)
>>
>
> Another perspective: If we aren't sure, we should make upgrading an
> external an error i n1.15, because that leaves users a workaround
> (upgrade the parent wc) and we can make it a non-error in the future,
> whereas if 1.15 allows upgrading only the external wc, backwards
> compatibility with that would be expected.
>
> If anyone thinks «svn upgrade /path/to/wc/path/to/external» should be
> allowed, do speak up.
How would one make «svn upgrade foo/bar» a failure if foo/bar is an
external within something? I guess by calling
svn_dirent_basename("foo/bar") and then running some svn_wc_* API on
that, but which? The same one that «svn status --depth=empty» uses, or
is there a better one?
Thanks,
Daniel
Re: svn commit: r1899014 - /subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
Could someone test this on Windows, please? I suspect read_wc_formats()
(added in r1899012) returns paths with os.sep, but the expectations
added in this commit use '/', so something will need to convert.
Thanks,
Daniel
danie...@apache.org wrote on Fri, 18 Mar 2022 00:40 +00:00:
> Author: danielsh
> Date: Fri Mar 18 00:40:29 2022
> New Revision: 1899014
>
> URL: http://svn.apache.org/viewvc?rev=1899014&view=rev
> Log:
> * subversion/tests/cmdline/upgrade_tests.py
> (upgrade_with_externals): Verify format numbers of upgraded externals.
> (check_formats): New.
> (check_format): Verify the argument type to guard against typos.
>
> Modified:
> subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
>
> Modified: subversion/trunk/subversion/tests/cmdline/upgrade_tests.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/upgrade_tests.py?rev=1899014&r1=1899013&r2=1899014&view=diff
> ==
> --- subversion/trunk/subversion/tests/cmdline/upgrade_tests.py (original)
> +++ subversion/trunk/subversion/tests/cmdline/upgrade_tests.py Fri Mar 18
> 00:40:29 2022
> @@ -102,11 +102,21 @@ def replace_sbox_repo_with_tarfile(sbox,
>shutil.move(os.path.join(extract_dir, dir), sbox.repo_dir)
>
> def check_format(sbox, expected_format):
> + assert isinstance(expected_format, int)
>formats = sbox.read_wc_formats()
>if formats[''] != expected_format:
> raise svntest.Failure("found format '%d'; expected '%d'; in wc '%s'" %
>(formats[''], expected_format, sbox.wc_dir))
>
> +def check_formats(sbox, expected_formats):
> + assert isinstance(expected_formats, dict)
> + formats = sbox.read_wc_formats()
> + ### If we ever need better error messages here, reuse
> run_and_verify_info().
> + if formats != expected_formats:
> +raise svntest.Failure("found format '%s'; expected '%s'; in wc '%s'" %
> + (formats, expected_formats, sbox.wc_dir))
> +
> +
> def check_pristine(sbox, files):
>for file in files:
> file_path = sbox.ospath(file)
> @@ -334,7 +344,18 @@ def upgrade_with_externals(sbox):
> 'upgrade', sbox.wc_dir)
>
># Actually check the format number of the upgraded working copy
> - check_format(sbox, get_current_format())
> + check_formats(sbox,
> + {relpath: get_current_format()
> + for relpath in (
> + '',
> + 'A/D/exdir_A',
> + 'A/D/exdir_A/G',
> + 'A/D/exdir_A/H',
> + 'A/D/x',
> + 'A/C/exdir_G',
> + 'A/C/exdir_H',
> + )})
> +
>check_pristine(sbox, ['iota', 'A/mu',
> 'A/D/x/lambda', 'A/D/x/E/alpha'])
Re: multi-wc-format: upgrading externals
Daniel Shahaf wrote on Tue, Mar 08, 2022 at 10:57:17 +: > Julian Foad wrote on Wed, Mar 02, 2022 at 13:04:51 +: > > Daniel Shahaf wrote: > > > multi-wc-format/BRANCH-README mentioned this: > > > > > >> [*] New externals working copies must inherit the format from their > > >>parent working copy, because [...] > > > > > > Upgrading a parent working copy upgrades external wc's too. However, > > > upgrading an external succeeds. Judging by the quoted remark, should > > > «svn upgrade --compatible-version=$N /path/to/external» error out unless > > > the external's parent working copy is already at version $V? > > > > It isn't clear to me whether allowing it or disallowing it is more "right". > > > > Can anyone else chime in? > > > > Hmm. Considering that «svn update» recurses into externals by default, > but that nothing recurses upwards into parent wc's by default, perhaps > we should design things around making sure these two cases continue to > work? I.e., disallow selective upgrades that might make another > client's «svn update» of the outer wc fail because the outer wc and the > external wc are different formats? > > Following this train of thought, we'll forbid upgrading an external > without also upgrading a parent wc, but will entertain patches to make > «svn upgrade» _not_ descend into external wc's by default, should anyone > submit such. (I don't propose we add this ourselves for the MVP.) > Another perspective: If we aren't sure, we should make upgrading an external an error i n1.15, because that leaves users a workaround (upgrade the parent wc) and we can make it a non-error in the future, whereas if 1.15 allows upgrading only the external wc, backwards compatibility with that would be expected. If anyone thinks «svn upgrade /path/to/wc/path/to/external» should be allowed, do speak up. Cheers, Daniel > Cheers, > > Daniel > > > In the meantime, I filed your question as > > https://subversion.apache.org/issue/4890 > > > > - Julian > >
Re: multi-wc-format: svn_wc__format_from_version()
Daniel Shahaf wrote: > It doesn't seem to be an old function; the docstring says (in a part I > snipped) it's new in 1.15. However, I don't think that changes the answer. > Done in r1899004. Oh, just older than my involvement then! Thanks. - Julian
Re: multi-wc-format: svn_wc__format_from_version()
Julian Foad wrote on Thu, 17 Mar 2022 20:33 +00:00: > That's an old function. "Characteristic" previously meant the only > format supported by a given client version. We should change the word > now. What should the function return now? The newest, I think: its > callers are upgrade and checkout; essentially it is used to implement > the --bikeshed=1.15 option. It doesn't seem to be an old function; the docstring says (in a part I snipped) it's new in 1.15. However, I don't think that changes the answer. Done in r1899004. Thanks, Julian. Daniel
Re: multi-wc-format: svn_wc__format_from_version()
That's an old function. "Characteristic" previously meant the only format supported by a given client version. We should change the word now. What should the function return now? The newest, I think: its callers are upgrade and checkout; essentially it is used to implement the --bikeshed=1.15 option. - Julian
multi-wc-format: svn_wc__format_from_version()
Here's the docstring:
[[[
/**
* Convert @a version to that version's characteristic working copy
* format, returned in @a format.
*
* A NULL @a version translates to the library's default version.
⋮
*/
svn_error_t *
svn_wc__format_from_version(int *format,
const svn_version_t* version,
apr_pool_t *scratch_pool);
]]]
Here's part of the implementation:
[[[
switch (version->minor)
{
case 0: /* Same as 1.3.x. */
case 1: /* Same as 1.3.x. */
case 2: /* Same as 1.3.x. */
case 3: *format = 4; break;
case 4: *format = 8; break;
case 5: *format = 9; break;
case 6: *format = 10; break;
case 7: *format = 29; break;
case 8: /* Same as 1.14.x. */
case 9: /* Same as 1.14.x. */
case 10: /* Same as 1.14.x. */
case 11: /* Same as 1.14.x. */
case 12: /* Same as 1.14.x. */
case 13: /* Same as 1.14.x. */
case 14: *format = 31; break;
case 15: /* Same as the current version. */
default: *format = SVN_WC__VERSION; break;
}
return SVN_NO_ERROR;
]]]
What does the term "characteristic" mean? Is it the oldest, newest, or
default wc format supported by @a version?
The implementation uses the newest, but that may have been an oversight
(cf. r1899000), and it's not clear to me from the callers what they
expect.
Cheers,
Daniel
Re: multi-wc-format review
Julian Foad wrote on Thu, Mar 17, 2022 at 11:10:32 +:
> Daniel Shahaf wrote:
> > + The upgraded working copy will be compatible with Subversion 1.8 and
> > + newer (this default may change ...
>
> Sure, +1, a bit clearer.
>
Committed.
> Also see Nathan's option-naming proposal at the end of this message.
Ack. We can still rename the option, but I didn't want to block committing
the usage message patch on that.
> > Format numbers are sequential. When upgrading from f30 to f40, there's
> > no way to skip f35. If we wanted that, we'd need some sort of
> > capability-like mechanism. Is that perhaps what you have in mind? That
> > a user might want to upgrade to 1.16 but not enable pristines-on-demand?
> > If so, we'll need a way to enable/disable pristines-on-demand that isn't
> > "format >= 32", as discussed previously.
>
> I think we will need that, yes.
OK. This is already tracked as SVN-4889 and marked as a blocker of
SVN-525.
> >> My point is, using the running software version as a proxy for a WC
> >> format introduces this ambiguity: [...]
> >> This is why I think we should do at least one of:
> >>
> >> - require the exact first-introduced version (1.8 or 1.15)
> >
> > I still don't like the idea of requiring users to figure out somehow
> > they should pass 1.8 when they want compatibility with 1.9. That's
> > a problem we should solve for them.
>
> Maybe. I can see it both ways. Maybe best to allow the flexibility in
> input ("=1.9") and make clear in the feedback (both immediate response
> from the command, and "svn info" kind of feedback) that the format
> chosen is compatible with "1.8".
`svn info` already does that:
% svn info
⋮
Working Copy Compatible With Version: 1.8
Working Copy Format: 31
The command's output idea is tracked in SVN-4885.
> [...]
> >> > 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.
> >
> > If it needs to be known by libsvn_client, then it should be in
> > include/private/… unless there is some reason it should be public?
>
> Indeed. I think "include/private" is right for now. Clients linking to
> libsvn_client will also need to know something about formats... I'm not
> sure whether to make these WC APIs public right away, so that any client
> developers can get on with using them. If not, if we would be expected
> to add alternative ways to access the info through libsvn_client public
> APIs (that hide WC format number details and expose the info another way
> (equivalent to the --bikeshed=1.15 UI and/or feature flags).
I'm afraid I'm having a hard time following your train of thought here:
I'm not sure which of the svn_wc__*/SVN_WC__* APIs mentioned upthread
you think to make public, and what kind of alternative you mean.
The cmdline client wc format logic doesn't use any private APIs. The
public libsvn_client API doesn't use format numbers either, other than
in svn_client_get_wc_formats_supported() and
svn_client_wc_version_from_format().
What questions might a client API consumer want to ask, that can't be
answered in a straightforward manner by the existing public APIs?
In the context of this branch, I guess the questions are "What clients
can read ?" and "What's the minimum format that
supports?". The former is answered by svn_client_wc_version_from_format().
The latter can be answered on by calling
svn_client_get_wc_formats_supported(), but doesn't seem to be
straightforward to answer on newer minor versions. So, perhaps we should
teach svn_client_get_wc_formats_supported() to take an svn_version_t*
parameter and return the formats supported by that version. Is this
useful enough to be included in the first release?
> Nathan Hartman wrote:
> > I wonder if user confusion can be mitigated / consistency could be
> > improved by calling it '--min-compatible-client=1.15' rather than
> > '--compatible-version'?
>
> That sounds good to me. That's both more understandable (to novice
> users) and more explicit.
No opinion here.
Datapoint: --compatible-version is the name used by «svnadmin create».
> > I am less fond of this second idea than the first one above
> > ('--min-compatible-client=1.15') because this second idea exposes
> > implementation details (f31, f32...), but this might nevertheless be
> > simpler for users as it eliminates any possibility for ambiguity.
>
> Yes, that's the dilemma. Probably the ideal would be
Re: Issue #525/#4892: on only fetching the pristines we really need
Johan Corveleyn wrote: >It's not specific to 'svn update' per se, but it's logical that it >leads to this discussion, because it is a (commonly used) case where >the pristine is not actually needed for the operation (if there is no >actual incoming update to the concerned file). 'svn diff' and 'svn >revert' cannot do their work without the pristine, but 'update without >an actual incoming edit'? > >And even with an 'incoming edit on update (on top of local mod)' it >might in theory be possible to delay the download of the full pristine >until after conflict-resolution decision (but I imagine that's even >more difficult to untangle). > >Also, why should 'svn update' be in the business of (silently) >restoring "the branch's invariant" (even when it does not need the >file), and not any other operation (like 'svn status -u' for example)? Thanks for adding all this rationale. +1 to it all. 100% agreed. - Julian
Re: Issue #525/#4892: on only fetching the pristines we really need
On Wed, Mar 16, 2022 at 9:59 PM Julian Foad wrote: > Daniel Shahaf wrote: > > Also, why is this specific to «svn update»? > > It's not specific to update. Update is a particular case that Karl cares > about so I looked at doing "update" first. Implementing this approach in one > subcommand at a time could be considered releasable incremental steps, > because each one is a further optimisation. It's not specific to 'svn update' per se, but it's logical that it leads to this discussion, because it is a (commonly used) case where the pristine is not actually needed for the operation (if there is no actual incoming update to the concerned file). 'svn diff' and 'svn revert' cannot do their work without the pristine, but 'update without an actual incoming edit'? And even with an 'incoming edit on update (on top of local mod)' it might in theory be possible to delay the download of the full pristine until after conflict-resolution decision (but I imagine that's even more difficult to untangle). Also, why should 'svn update' be in the business of (silently) restoring "the branch's invariant" (even when it does not need the file), and not any other operation (like 'svn status -u' for example)? -- Johan
Re: multi-wc-format review
Daniel Shahaf wrote:
> + The upgraded working copy will be compatible with Subversion 1.8 and
> + newer (this default may change ...
Sure, +1, a bit clearer.
Also see Nathan's option-naming proposal at the end of this message.
>> 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.
>
> In the described scenario, the user wanted any format that 1.15 can
> write to. They don't care which one, and accordingly we can leave it
> unspecified (an implementation detail) which would be selected. This
> user doesn't constrain us.
>
> Another user may expect --bikeshed=1.15 to select the _newest_ format
> 1.15 can write to. This user is certain to exist; they're the reason
> we're writing this feature.
>
> Yet another user may expect --bikeshed=1.15 to select the _oldest_
> format 1.15 can write to, you say in your next paragraph. However,
> I don't see why any user would expect that. [...]
Yes, all fair. I think I now can accept that the current meaning
(highest format supported by the specified client version) is OK.
>> Consider [...] --wc-format=$(/opt/bar/bin/svn --version
>> --show-item=wc-format-default)»
>
> Clearer, yes, but does this serve a use-case? Is there a user who would
> expect this to be possible? As opposed to just "give me something 1.15
> can read and write"?
Maybe there is no need.
> Format numbers are sequential. When upgrading from f30 to f40, there's
> no way to skip f35. If we wanted that, we'd need some sort of
> capability-like mechanism. Is that perhaps what you have in mind? That
> a user might want to upgrade to 1.16 but not enable pristines-on-demand?
> If so, we'll need a way to enable/disable pristines-on-demand that isn't
> "format >= 32", as discussed previously.
I think we will need that, yes.
>> Potentially the user can select from "-default", "-min", "-max" variants
>> of that option.
>
> KISS. [...]
Agreed; and I didn't mean we should expose that, just to use that idea
to help us think about the UI design.
> I suppose it's theoretically possible that there won't be a CLI
> incantation that will create f33; it will only be possible to create f32
> or f34. Is this a problem?
No problem. (If, say, a hypothetical 1.16 would add support for two new
formats (f33,f34), I can't imagine any reason why we'd ever do that and
also need to provide users the ability to specify the non-highest one
(f33 in this example) directly.
>> My point is, using the running software version as a proxy for a WC
>> format introduces this ambiguity: [...]
>> This is why I think we should do at least one of:
>>
>> - require the exact first-introduced version (1.8 or 1.15)
>
> I still don't like the idea of requiring users to figure out somehow
> they should pass 1.8 when they want compatibility with 1.9. That's
> a problem we should solve for them.
Maybe. I can see it both ways. Maybe best to allow the flexibility in
input ("=1.9") and make clear in the feedback (both immediate response
from the command, and "svn info" kind of feedback) that the format
chosen is compatible with "1.8".
>> - 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'
>
> Expecting a format number like this is basically asking the user to
> hardcode a magic number in their code, since there's no other way for
> them to learn the value "31".
OK.
[...]
>> > 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.
>
> If it needs to be known by libsvn_client, then it should be in
> include/private/… unless there is some reason it should be public?
Indeed. I think "include/private" is right for now. Clients linking to
libsvn_client will also need to know something about formats... I'm not
sure whether to make these WC APIs public right away, so that any client
developers can get on with using them. If not, if we would be expected
to add alternative ways to access the info through libsvn_client public
APIs (that hide WC format number details and expose the info another way
(equivalent to the --bikeshed=1.15 UI and/or feature flags)
