Page MenuHomePhabricator

revlog: introduce v2 format
ClosedPublic

Authored by Alphare on Jan 20 2021, 3:45 PM.

Details

Summary

As documented in [1], this is still tentative and could be subject to change,
but we need to lay down the foundations in order to work on the next abstraction
layers.

[1] https://www.mercurial-scm.org/wiki/RevlogV2Plan

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.Jan 20 2021, 3:45 PM

Can we get space for extra flags for things not supported by the filesystem? I'm thinking specifically +x and symlink support on Windows.

https://www.mercurial-scm.org/wiki/DirState#Proposed_extensions

Can we get space for extra flags for things not supported by the filesystem? I'm thinking specifically +x and symlink support on Windows.
https://www.mercurial-scm.org/wiki/DirState#Proposed_extensions

That would be a dirstate thing, not a revlog thing, right ?

Can we get space for extra flags for things not supported by the filesystem? I'm thinking specifically +x and symlink support on Windows.
https://www.mercurial-scm.org/wiki/DirState#Proposed_extensions

That would be a dirstate thing, not a revlog thing, right ?

Oops, yeah. It’s been one of those days...

pulkit added a subscriber: pulkit.Jan 22 2021, 11:40 AM

Couple of changes from this patch can be separated and send individually as prepare for revlog v2 which will help speed up review.

mercurial/localrepo.py
3639 ↗(On Diff #25185)

This can be a separate patch and will be easy one to push.

mercurial/pure/parsers.py
45

IIUC, this change can also be a separate patch.

I'm not going to review this right now, but I have prior art at https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/093657.html and https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-May/097960.html that might be worth a read. I believe the wiki page on this format only contains a subset of the deficiencies I outlined in the 1st link.

Alphare marked an inline comment as done.Jan 28 2021, 10:02 AM

I'm not going to review this right now, but I have prior art at https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-February/093657.html and https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-May/097960.html that might be worth a read. I believe the wiki page on this format only contains a subset of the deficiencies I outlined in the 1st link.

Thanks for pasting these links. I'll look at them pretty soon, and we'll definitely have to account for whatever deficiencies I haven't covered in the plan page before we actually settle on a format.

This patch series is still valid for the short-term because it enables work around side-data and copytracing that would not be possible without a new format.

baymax updated this revision to Diff 25603.Feb 12 2021, 7:46 AM

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

baymax updated this revision to Diff 25615.Feb 12 2021, 6:58 PM

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

baymax updated this revision to Diff 25664.Feb 19 2021, 6:12 AM

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

durin42 added inline comments.
mercurial/revlogutils/constants.py
18

Was this change intentional? Don't we want to _not_ use 2 for this version until the format is locked down?

marmoute added inline comments.Feb 24 2021, 11:20 AM
mercurial/revlogutils/constants.py
18

+1 on Augie comment

durin42 requested changes to this revision.Feb 24 2021, 11:21 AM

(marking as request changes in that case)

This revision now requires changes to proceed.Feb 24 2021, 11:21 AM
baymax updated this revision to Diff 25964.Feb 25 2021, 11:45 AM

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

Alphare added inline comments.Feb 25 2021, 11:45 AM
mercurial/revlogutils/constants.py
18

Right, updated.

baymax updated this revision to Diff 26000.Mar 1 2021, 12:11 PM

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

marmoute requested changes to this revision.Mar 2 2021, 11:25 AM
marmoute added inline comments.
mercurial/pure/parsers.py
244–250

I don't think the meaning of these are documented anywhere, it would be nice to use the fact you know what they mean to document them.

mercurial/revlog.py
383

should the constant be factored beetween the previous file and this one ?

like indexformatv2 = struct.Struct(pureparser.Index2Mixin.index_format) in this case, the documentation would move to the pureparser module.

tests/test-revlog.t
25–28

Should this be in an independant changeset ? Should we bump it to something much hight than (3) to avoid the same problem in the next iteration ?

This revision now requires changes to proceed.Mar 2 2021, 11:25 AM
Alphare marked 2 inline comments as done.Mar 4 2021, 3:32 AM
Alphare updated this revision to Diff 26046.
Alphare added inline comments.Mar 4 2021, 3:34 AM
tests/test-revlog.t
25–28

I don't think this is worth another changeset, but I can definitely use a better value.

Alphare updated this revision to Diff 26084.Mar 4 2021, 10:11 AM
marmoute accepted this revision.Mar 9 2021, 10:43 AM

Thanks for the fix.

baymax updated this revision to Diff 26215.Mar 10 2021, 2:10 PM

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

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