Page MenuHomePhabricator

sidedata: move to new sidedata storage in revlogv2
ClosedPublic

Authored by Alphare on Feb 15 2021, 5:29 AM.

Details

Summary

The current (experimental) sidedata system uses flagprocessors to signify the
presence and store/retrieve sidedata from the raw revlog data. This proved to be
quite fragile from an exchange perspective and a lot more complex than simply
having a dedicated space in the new revlog format.

This change does not handle exchange (ironically), so the test for amend - that
uses a bundle - is broken. This functionality is split into the next patches.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Feb 15 2021, 5:29 AM
baymax updated this revision to Diff 25989.Mar 1 2021, 11:51 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

marmoute requested changes to this revision.Mar 2 2021, 1:45 PM
marmoute added a subscriber: marmoute.

The request for change is actually about comment that are related to earlier changeset, but that I only understood in that changesets.

mercurial/revlog.py
2371–2372

what does merging revlog means ?

2373–2374

This is premature and we should not include this in this series.

2390–2392

Ha, so this is what this is. I don't think we should include them in the format at this point. The only new feature we have are the sidedata and we should focus on that.

This revision now requires changes to proceed.Mar 2 2021, 1:45 PM
Alphare added inline comments.Mar 3 2021, 6:52 AM
mercurial/revlog.py
2390–2392

Accounting for them in the format was easy enough, I don't think it's worth any more attention by reworking several changesets to remove them just to re-do that work in a few weeks.

marmoute added inline comments.Mar 3 2021, 7:29 AM
mercurial/revlog.py
2390–2392

It is unlikley for that work to be done "in a few week" and even if so, we have no garantee the current format fit the need for these data when that work is actually done. So is is really premature.

To me, the important part of this revlogv2 it to setup a more versatile codebase where adding more field/data get simpler. So, not having them part of the initial batch should be fine, as adding these one, or others, later should be simpler.

Alphare marked 3 inline comments as done.Mar 4 2021, 10:06 AM
Alphare updated this revision to Diff 26087.Mar 4 2021, 10:18 AM
marmoute accepted this revision.Mar 9 2021, 11:14 AM

Thanks for the update.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.