( )⚙ 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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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.