( )βš™ D10028 sidedata-exchange: add `wanted_sidedata` and `sidedata_computers` to repos

This is an archive of the discontinued Mercurial Phabricator instance.

sidedata-exchange: add `wanted_sidedata` and `sidedata_computers` to repos
ClosedPublic

Authored by Alphare on Feb 19 2021, 6:16 AM.

Details

Summary

Each repo will advertise the sidedata categories it requires (categories being
unique and canonical), and have a set of "computers", functions to generate
sidedata from (repo, revlog, rev, previous_sidedata), for a given category.
The set of computers can be a superset of the set of the wanted categories, but
not smaller: repos are expected to be coherent in their handling of sidedata.

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 19 2021, 6:16 AM
baymax updated this revision to Diff 25995.Mar 1 2021, 11:52 AM

βœ… refresh by Heptapod after a successful CI run (πŸ™ πŸ’š)

marmoute requested changes to this revision.Mar 2 2021, 2:00 PM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/metadata.py
819–825

We should use a registrar here (or at least a small helper function), that way, changing the implementation/processing will be simple in the futur.

What do you think ?

This revision now requires changes to proceed.Mar 2 2021, 2:00 PM
Alphare marked an inline comment as done.Mar 4 2021, 10:04 AM
Alphare updated this revision to Diff 26091.Mar 4 2021, 10:18 AM
marmoute requested changes to this revision.Mar 9 2021, 11:36 AM
marmoute added inline comments.
mercurial/bundle2.py
1815

Can you add a small docstring to clarify the semantic of the function ?

mercurial/changegroup.py
948

If you want to test for None, explicitly test for None (is None). Otherwise you introduce subtle bugs for our future self.

mercurial/exchange.py
423–437

Why do we need multiples layer of loop here . I would expect an API to return a the appropriate object for a categories and that's it.

So, what is going on here ?

1630

Same question here.

This revision now requires changes to proceed.Mar 9 2021, 11:36 AM
Alphare marked 2 inline comments as done.Mar 10 2021, 1:42 PM
Alphare added inline comments.
mercurial/exchange.py
423–437

Categories are advertised as a flat list, but can affect any or all of the revlog kinds. This checks that a sidedata computer is registered for at least one revlog kind.

Alphare updated this revision to Diff 26222.Mar 10 2021, 2:12 PM
marmoute accepted this revision.Mar 11 2021, 1:13 PM
marmoute added inline comments.
mercurial/exchange.py
423–437

Ha, yes…

marmoute requested changes to this revision.Mar 11 2021, 1:21 PM
marmoute added inline comments.
mercurial/exchange.py
1635–1643

If I understand correctly, this piece of code will be reached only if a the local client flag some sidedata as "locally wanted" without having the means to generate them. right ? The message could be clearer:

m = _(b'sidedata category requested by local side without local support: '%s'%s'")
This revision now requires changes to proceed.Mar 11 2021, 1:21 PM
Alphare marked 3 inline comments as done.Mar 12 2021, 5:34 AM
Alphare updated this revision to Diff 26256.Mar 12 2021, 6:34 AM
marmoute accepted this revision.Mar 12 2021, 1:10 PM
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.