Pristines-on-demand: OK to merge to trunk?

[Julian Foad]

TL;DR: are we OK to merge the pristines feature
('pristines-on-demand-on-mwf' branch) to trunk soon, like early next week?

As said in "A status review" [1] in the long thread "A two-part vision
for Subversion and large binary objects.", next steps are reviewing and
handling the outstanding issues, and proposing merge to trunk. I think
these can be done in parallel as I don't see any that would block a
merge to trunk. So here is the proposal to merge to trunk, and then
complete the remaining work on trunk.

It feels to me like there is general consensus that this feature is
taking a form that will be acceptable for a first release of it (while
not perfect), and consensus for proceeding to get it into trunk and
subsequently including it in the next release. I'm too close to it to
make an independent assessment. Can anybody else comment?

If no objections, I plan to merge to trunk early next week.

[1] on dev@, 2022-04-05, 
https://lists.apache.org/thread/lm98og8jqonffcs250q5y3ft5r5qlmk5

Re: Pristines-on-demand: fix disabled tests (#4891)

[Julian Foad]

Johan, that's a great start, thanks.

Now can you run the test suite with --wc-format-version=1.15? On Unixy
systems that is done by, for example:

$ make svnserveautocheck WC_FORMAT_VERSION=1.15 ...

Jun Omae wrote:
>> FAIL:  diff_tests.py 48: svn diff --diff-cmd provides the correct arguments
> 
> I've posted patch for the failure.
> See https://lists.apache.org/thread/2o0xtqfzy9xg8wzxscj2wb641p2kyo9c

Thank you, Jun Omae. Sorry, I missed that before. Committed now in r1899645.

- Julian

Re: A two-part vision for Subversion and large binary objects.

[Julian Foad]

Julian Foad wrote:
> Pristines (#525):
>  - #4888 authz denied during textbase sync
>(an edge case issue, not sure if it's a blocker)
>  - #4889 per-WC config
>(wanted)
>  - #4891 fix disabled tests
>(a few different edge cases; much of the analysis is posted in the issue)
> 
> Getting multi-wc-format ready for release (#4883):
>  - #4885 WC upgraded and not-upgraded notifications
>(still open for some nice-to-haves, but probably done enough for MVP)
>  - #4886 config for default WC version for checkout & upgrade
>()
>  - #4887 clarify/unify option names for compatible-version
>(perhaps change '--compatible-version' to '--wc-compatible-version'
> or '--min-compatible-client')
>  - API review; thread: "multi-wc-format review"
>(state is APIs are mostly private and a bit messy; not clear what,
> if anything, we would want to change)

Further updates:

#4888: demoted to non-blocker
#4889: blocker, in progress
#4891: blocker, in progress (I've processed a bunch of the sub-issues in it)
#4885: done enough (now non-blocker)
#4886: not sure (currently marked non-blocker)
#4887: not sure (currently marked blocker)
API review: not sure
Merge to trunk: new thread "Pristines-on-demand: OK to merge to trunk?"

In #4891 "fix disabled tests", the remaining sub-issues don't look like
show-stoppers. Likely we will soon demote it to non-blocker.

Issue #4889 "per-WC config" is the subject of Johan's new dev@ post
"Pristines-on-demand=enabled == format 32?". We already concurred that
it's wise to decouple "pristines-on-demand mode is enabled in this WC"
from "the WC format is (at least) 32 so can support that mode".
. This may be considered
higher priority than fixing the remaining tests. I previously drafted a
proof-of-concept for such a config setting. I'm going to spend two or
three hours and see if I can complete an acceptable minimal version of it.

This (#4889) conceptually also relates to #4886 "config for default WC
version for checkout & upgrade"; I am not yet sure if both are
separately necessary.

Two other issues Karl and I discussed were:

* regression tests:
  -> current status is devs need to run test suite both with and without
the new '--wc-format-version=1.15' knob;
  -> this adds another knob to the several existing knobs;
  -> the resulting exponential increase in test runs is a concern but
not a new problem in itself;
  -> we should make build bots run that combination.
  -> Filed as #4898 "Pristines-on-demand: make buildbots test it"

* simplified user documentation:
  -> not sure, maybe existing is sufficient initially (just needs to be
put where users can find it?);
  -> maybe someone else will be able to rewrite into a simpler, more
digestible form?

Re: Pristines-on-demand: fix disabled tests (#4891)

[Johan Corveleyn]

On Thu, Apr 7, 2022 at 11:31 AM Julian Foad  wrote:
> Now can you run the test suite with --wc-format-version=1.15? On Unixy
> systems that is done by, for example:
>
> $ make svnserveautocheck WC_FORMAT_VERSION=1.15 ...

Okay, passing that option to win-tests.py (the test runner on Windows) failed.
Should be fixed now, with r1899654.

I suppose not passing the option uses wc-format-version=1.15
automatically? Or what is the default? Is format 1.8 the same as 1.14
and anything in between?

Should I run with 'None' and 1.8? Or 1.8 and 1.15?

-- 
Johan

Re: Pristines-on-demand: OK to merge to trunk?

[Julian Foad]

Nathan Hartman wrote:
> The branch worked for me when I last tested it and I saw no glaring
> issues so I have no objections to merging it soon. That said, I would
> encourage, if at all feasible, that we try to do two things first:
> decouple the format 32 and pod525 feature, and decide what the
> user-facing feature and its CLI switches should be called so that our
> internal "plumbing" names won't stick forever as the user-visible
> terms... If we can only do one of these, it should probably be the naming.

For "decouple the format 32 and pod525 feature", see email thread
"Pristines-on-demand=enabled == format 32?" 
.

Naming
==

What naming do we want? I'm perhaps too close to the code. Some
potentially good ideas have been floated in the past, none very
obviously best but perhaps we can pick a good one from them. Some
suggestions were along the lines of:

  * "bare WC"
  * "blob-optimised WC"
  * "[no] local cache" (of text-bases)
  * "[no] trade space for speed"

(just off the top of my head).

The feature name hardly appears in the UI, so far. The places I can
think of are:

  * a progress notification: "Fetching text bases...",
  * perhaps a dedicated option name for checkout/upgrade, as being
discussed in the "decouple" thread mentioned above;
  * release notes;
  * user guide (where currently a place-holder 'POD525' is used for the
feature name).

- Julian

Re: Pristines-on-demand: fix disabled tests (#4891)

[Johan Corveleyn]

On Thu, Apr 7, 2022 at 11:31 AM Julian Foad  wrote:
> Jun Omae wrote:
> >> FAIL:  diff_tests.py 48: svn diff --diff-cmd provides the correct arguments
> >
> > I've posted patch for the failure.
> > See https://lists.apache.org/thread/2o0xtqfzy9xg8wzxscj2wb641p2kyo9c
>
> Thank you, Jun Omae. Sorry, I missed that before. Committed now in r1899645.

Thanks, confirmed fixed on Windows.

-- 
Johan

Re: Pristines-on-demand: fix disabled tests (#4891)

[Julian Foad]

Johan Corveleyn wrote:
>Should be fixed now, with r1899654.

Great! Thanks.

>I suppose not passing the option uses wc-format-version=1.15

No...

>automatically? Or what is the default? Is format 1.8 the same as 1.14
>and anything in between?

Default would be the format '31' which is 1.8 through 1.14.

- Julian

Re: Call for release signatures

[Stefan Sperling]

On Thu, Apr 07, 2022 at 10:07:15AM -0400, Nathan Hartman wrote:
> > We should have enough signatures according to ASF rules (need 3 signatures
> > by PMC members), but not for our own cross-platform testing requirements
> > (there is only one windows signature so far).
> >
> > Should we fail to meet our own higher standard, we can fall back on ASF
> > rules, meaning we could release with the signatures we have available now.
> >
> > While not required this time around, the RM could in principle count
> > their own signature against the 3 required by ASF.
> >
> > I don't mean to discourage further testing/signing; if you can test these
> > releases on Windows or other platforms which have not yet been covered in
> > some way, this would be very welcome. Testing from people outside the PMC
> > is also always welcome, even though a signature won't be useful in this
> > case.
> >
> > Cheers,
> > Stefan
> >
> I'm a little bit confused... Per HACKING we need three +1 votes from PMC
> members, at least one of which is for each platform (Windows and Unix). We
> have already met this requirement for both 1.14.2 and 1.10.8, haven't we?

Oh, so the rules have been updated? Sorry for the confusion. I still had
the old rules in mind, which required 3 votes for each *nix and windows.

Re: Pristines-on-demand=enabled == format 32?

[Julian Foad]

Johan Corveleyn wrote:
> Ah, yes, I think that makes #4889 a blocker.

Well, I'm having a hard time deciding what exactly we need and why.

I previously said "it's pretty clear it needs to be uncoupled" but
actually just now I've dived into it for a couple of hours, coding and
thinking, and it's not at all clear what this means.

Is it mainly about UI level naming in "checkout" and "upgrade" -- in
other words, that we would prefer the user to say

  "svn checkout --enable-POD525"

instead of (or in addition to) "--compatible-version=1.15"? And we would
not need to support the combination of "=1.15" without "--enable-POD525"
(in other words the feature is still coupled to the format in the
implementation), but require it to be specified explicitly and error out
if it isn't, in order to set a precedent that that's the option you'll
also be needing to use in future versions?

('POD525' used here as a place-holder for the feature name that is to be 
decided.)

If it's that kind of UI-level naming issue, then we probably want to
also put corresponding entry in "svn info" saying "POD525 enabled?
yes/no". And anywhere else the idea is exposed in the UI, if anywhere.

And/or, is it that we want to put internal APIs in place that let the
higher level code layers ask "is POD525 enabled?" and not have to change
those callers when 1.16/new-wc-format-33 comes around? But I don't see a
strong need for that. We are not making a strong case for any of this to
be exposed in public APIs at this time at all and the internal API calls
can surely be updated as and when needed.

And/or, is it that we really need users to be able to create a format-32
(v=1.15) WC that doesn't have POD525 enabled? I am pretty sure that is
not the case.

Am I missing some other need? What seems to be the problem really?


> I tried to suggest a slightly more flexible per-WC-config [...]

I appreciate your suggestions. I think you are on the right track for
forward-thinking and architecturally sound design. It's something we can
take into account when naming the parts and designing the config options.

- Julian

Re: Call for release signatures

[Stefan Sperling]

On Thu, Apr 07, 2022 at 09:37:08AM -0400, Mark Phippard wrote:
> Just a reminder, the 1.10.8 and 1.14.2 releases are posted and
> available for testing and signatures. Please try to get them completed
> by this Sunday.
> 
> The plan is to make the release available on Tuesday April 12.
> 
> Thanks
> 
> Mark
> 

We should have enough signatures according to ASF rules (need 3 signatures
by PMC members), but not for our own cross-platform testing requirements
(there is only one windows signature so far).

Should we fail to meet our own higher standard, we can fall back on ASF
rules, meaning we could release with the signatures we have available now.

While not required this time around, the RM could in principle count
their own signature against the 3 required by ASF.

I don't mean to discourage further testing/signing; if you can test these
releases on Windows or other platforms which have not yet been covered in
some way, this would be very welcome. Testing from people outside the PMC
is also always welcome, even though a signature won't be useful in this case.

Cheers,
Stefan

Re: Call for release signatures

[Nathan Hartman]

On Thu, Apr 7, 2022 at 9:49 AM Stefan Sperling  wrote:

> On Thu, Apr 07, 2022 at 09:37:08AM -0400, Mark Phippard wrote:
> > Just a reminder, the 1.10.8 and 1.14.2 releases are posted and
> > available for testing and signatures. Please try to get them completed
> > by this Sunday.
> >
> > The plan is to make the release available on Tuesday April 12.
> >
> > Thanks
> >
> > Mark
> >
>
> We should have enough signatures according to ASF rules (need 3 signatures
> by PMC members), but not for our own cross-platform testing requirements
> (there is only one windows signature so far).
>
> Should we fail to meet our own higher standard, we can fall back on ASF
> rules, meaning we could release with the signatures we have available now.
>
> While not required this time around, the RM could in principle count
> their own signature against the 3 required by ASF.
>
> I don't mean to discourage further testing/signing; if you can test these
> releases on Windows or other platforms which have not yet been covered in
> some way, this would be very welcome. Testing from people outside the PMC
> is also always welcome, even though a signature won't be useful in this
> case.
>
> Cheers,
> Stefan
>
I'm a little bit confused... Per HACKING we need three +1 votes from PMC
members, at least one of which is for each platform (Windows and Unix). We
have already met this requirement for both 1.14.2 and 1.10.8, haven't we?

Nevertheless I agree it is good to have  additional testing and signatures,
for both platforms, and interested community members can and should feel
free to participate.

Cheers,
Nathan

Call for release signatures

[Mark Phippard]

Just a reminder, the 1.10.8 and 1.14.2 releases are posted and
available for testing and signatures. Please try to get them completed
by this Sunday.

The plan is to make the release available on Tuesday April 12.

Thanks

Mark

Re: Call for release signatures

[Mark Phippard]

On Thu, Apr 7, 2022 at 10:07 AM Nathan Hartman  wrote:
>
> On Thu, Apr 7, 2022 at 9:49 AM Stefan Sperling  wrote:
>>
>> On Thu, Apr 07, 2022 at 09:37:08AM -0400, Mark Phippard wrote:
>> > Just a reminder, the 1.10.8 and 1.14.2 releases are posted and
>> > available for testing and signatures. Please try to get them completed
>> > by this Sunday.
>> >
>> > The plan is to make the release available on Tuesday April 12.
>> >
>> > Thanks
>> >
>> > Mark
>> >
>>
>> We should have enough signatures according to ASF rules (need 3 signatures
>> by PMC members), but not for our own cross-platform testing requirements
>> (there is only one windows signature so far).
>>
>> Should we fail to meet our own higher standard, we can fall back on ASF
>> rules, meaning we could release with the signatures we have available now.
>>
>> While not required this time around, the RM could in principle count
>> their own signature against the 3 required by ASF.
>>
>> I don't mean to discourage further testing/signing; if you can test these
>> releases on Windows or other platforms which have not yet been covered in
>> some way, this would be very welcome. Testing from people outside the PMC
>> is also always welcome, even though a signature won't be useful in this case.
>>
>> Cheers,
>> Stefan
>
> I'm a little bit confused... Per HACKING we need three +1 votes from PMC 
> members, at least one of which is for each platform (Windows and Unix). We 
> have already met this requirement for both 1.14.2 and 1.10.8, haven't we?
>
> Nevertheless I agree it is good to have  additional testing and signatures, 
> for both platforms, and interested community members can and should feel free 
> to participate.

Sorry for the confusion  I was not implying we *need* more
signatures, it was as you said a call for anyone who wants to sign the
release to get it submitted by this Sunday.

Mark

Re: Pristines-on-demand: OK to merge to trunk?

[Nathan Hartman]

On Thu, Apr 7, 2022 at 7:43 AM Julian Foad  wrote:

> TL;DR: are we OK to merge the pristines feature
> ('pristines-on-demand-on-mwf' branch) to trunk soon, like early next week?
>
> As said in "A status review" [1] in the long thread "A two-part vision
> for Subversion and large binary objects.", next steps are reviewing and
> handling the outstanding issues, and proposing merge to trunk. I think
> these can be done in parallel as I don't see any that would block a
> merge to trunk. So here is the proposal to merge to trunk, and then
> complete the remaining work on trunk.
>
> It feels to me like there is general consensus that this feature is
> taking a form that will be acceptable for a first release of it (while
> not perfect), and consensus for proceeding to get it into trunk and
> subsequently including it in the next release. I'm too close to it to
> make an independent assessment. Can anybody else comment?
>
> If no objections, I plan to merge to trunk early next week.
>
> [1] on dev@, 2022-04-05,
> https://lists.apache.org/thread/lm98og8jqonffcs250q5y3ft5r5qlmk5
>
>
The branch worked for me when I last tested it and I saw no glaring issues
so I have no objections to merging it soon. That said, I would encourage,
if at all feasible, that we try to do two things first: decouple the format
32 and pod525 feature, and decide what the user-facing feature and its CLI
switches should be called so that our internal "plumbing" names won't stick
forever as the user-visible terms... If we can only do one of these, it
should probably be the naming.

Cheers,
Nathan

Re: Pristines-on-demand: fix disabled tests (#4891)

[Johan Corveleyn]

Okay, with --wc-format-version=1.8, I now get these (X)FAILs and
XPASSes (fails_wc-format-version1.8.log in attachment):

[[[
XFAIL: diff-diff3-test 18: 3-way merge, double add
XFAIL: dirent_uri-test 47: test match with RFC 6125 s. 6.4.3 Rule 3
XFAIL: op-depth-test 42: mixed_rev_move
   [[needs different libsvn_wc entry point]]
XFAIL: op-depth-test 56: commit_moved_away_descendant
XFAIL: op-depth-test 68: move retract (issue 4336)
XFAIL: op-depth-test 69: move/delete file externals (issue 4293)
XFAIL: op-depth-test 75: move more than once, revert intermediate
XFAIL: op-depth-test 79: del4: delete AAA
XFAIL: op-depth-test 80: del4: add AAA
XFAIL: op-depth-test 81: del4: replace AAA
XFAIL: op-depth-test 83: del4: replace self AAA
XFAIL: op-depth-test 85: move4: delete AAA
XFAIL: op-depth-test 87: move4: replace AAA
XFAIL: op-depth-test 86: move4: add AAA
XFAIL: op-depth-test 89: move4: replace self AAA
XFAIL: op-depth-test 95: move within mixed move
XFAIL: basic_tests.py 9: basic corruption detection on update
   [[Relies on wc.text_base_path()]]
XFAIL: basic_tests.py 63: peg rev resolution on non-existent wc paths
XFAIL: blame_tests.py 15: blame -g handles changes from empty mergeinfo
XFAIL: changelist_tests.py 5: diff --changelist (wc-wc and repos-wc)
XFAIL: commit_tests.py 66: last changed of copied subdir
XFAIL: commit_tests.py 74: commit sees tree conflict on unversioned path
XFAIL: copy_tests.py 105: copy and move conflicts
XFAIL: depth_tests.py 49: deleted & moved items left untouched
XFAIL: depth_tests.py 50: unversioned files in excluded directory
XFAIL: diff_tests.py 77: diff repo to wc of a copy
XFAIL: diff_tests.py 90: diff unversioned files in git format
XFAIL: diff_tests.py 92: diff summary repo wc local copy unmodified
XFAIL: diff_tests.py 94: diff git format copy
XFAIL: export_tests.py 11: export working copy at base revision
XFAIL: externals_tests.py 25: update that modifies a file external
XFAIL: externals_tests.py 39: file external remap segfaults due to deleted props
XFAIL: externals_tests.py 44: move with file externals
XFAIL: externals_tests.py 49: file externals versioned obstruction
XFAIL: externals_tests.py 68: check file external recorded info
XFAIL: log_tests.py 46: log --use-merge-history --search
XFAIL: log_tests.py 47: log --use-merge-history --xml
XFAIL: merge_automatic_tests.py 16: cherry2_fwd
XFAIL: merge_automatic_tests.py 17: cherry3_fwd
XFAIL: merge_tests.py 49: avoid repeated merges for cyclic merging
XFAIL: merge_tests.py 64: merge target with non inheritable mergeinfo
XFAIL: merge_tests.py 114: don't inherit bogus mergeinfo
XFAIL: merge_tests.py 115: don't inherit bogus working mergeinfo
XFAIL: patch_tests.py 52: hunks that overlap
XFAIL: patch_tests.py 78: patching a specific merge
XFAIL: patch_tests.py 80: patch empty prop
XFAIL: patch_tests.py 81: patch working copy root
XFAIL: patch_tests.py 82: patch working copy root
XFAIL: pegrev_parse_tests.py 11: add file '.@tau' without pegrev escape
   [[The error message mentions '@tau' instead of '.@tau']]
XFAIL: pegrev_parse_tests.py 23: add file 'E/@tau' without pegrev escape
   [[The error message mentions 'E@tau' instead of 'E/@tau']]
XFAIL: pegrev_parse_tests.py 25: add file 'E/.@tau' without pegrev escape
   [[The error message mentions 'E@tau' instead of 'E/.@tau']]
XFAIL: pegrev_parse_tests.py 28: add file 'E/@' without pegrev escape
   [[The error message is E29 but should be E125001]]
XFAIL: pegrev_parse_tests.py 39: create directory '.@T' without pegrev escape
   [[The error message mentions '@T' instead of '.@T']]
XFAIL: pegrev_parse_tests.py 49: create directory 'E/@T' without pegrev escape
   [[The error message mentions 'E@T' instead of 'E/@T']]
XFAIL: pegrev_parse_tests.py 51: create directory 'E/.@T' without pegrev escape
   [[The error message mentions 'E@T' instead of 'E/.@T']]
XFAIL: pegrev_parse_tests.py 52: create directory 'E/@' without pegrev escape
   [[Reports error that E exists but should be E125001 for E/@]]
XFAIL: pegrev_parse_tests.py 63: remove '.@kappa' without pegrev escape
   [[The error message mentions '@kappa' instead of '.@kappa']]
XFAIL: pegrev_parse_tests.py 77: remove 'B/@beta' without pegrev escape
   [[The error message mentions 'B@beta' instead of 'B/@beta']]
XFAIL: pegrev_parse_tests.py 79: remove 'D/.@delta' without pegrev escape
   [[The error message mentions 'D@delta' instead of 'D/.@delta']]
XFAIL: pegrev_parse_tests.py 80: remove 'B/@' without pegrev escape
   [[Removes B instead of reporting E125001 for B/@]]
XFAIL: pegrev_parse_tests.py 81: remove missing 'E/@' without pegrev escape
   [[Removes E instead of reporting ENOENT or E125001 for E/@]]
XFAIL: pegrev_parse_tests.py 82: remove missing '@/@' without pegrev escape
   [[Removes @ instead of reporting ENOENT or E125001 for @/@]]
XFAIL: pegrev_parse_tests.py 83: rename 'iota' to 'E/@tau with pegrev escape
   [[Rename creates 'E/@tau@' instead of 

Re: Pristines-on-demand: fix disabled tests (#4891)

[Jun Omae]

On Thu, Apr 7, 2022 at 2:47 PM Johan Corveleyn  wrote:
> This is in fails.log for that one FAIL:
> [[[
> ...
>   File "C:\Python39\lib\sre_parse.py", line 426, in _escape
> raise source.error("bad escape %s" % escape, len(escape))
> re.error: bad escape \c at position 32 (line 1, column 33)
> FAIL:  diff_tests.py 48: svn diff --diff-cmd provides the correct arguments
> ]]]

I've posted patch for the failure.
See https://lists.apache.org/thread/2o0xtqfzy9xg8wzxscj2wb641p2kyo9c

-- 
Jun Omae  (大前 潤)