( )⚙ D10031 revlog-index: add `replace_sidedata_info` method

This is an archive of the discontinued Mercurial Phabricator instance.

revlog-index: add `replace_sidedata_info` method
ClosedPublic

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

Details

Summary

During a pull operation where the server does not provide sidedata, the client
that requires it should generate them on-the-fly. In the generic case, we need
to wait for the changelog + manifests + filelogs to be added, since we don't
know what the sidedata computers might need: this means rewriting the sidedata
of index entries from within the pull transaction (and no further back) right
after we've added them.

Both Python and C implementations only allow for rewriting the sidedata offset
and length for revs within the transaction where they were created.

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 25998.Mar 1 2021, 11:52 AM

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

marmoute requested changes to this revision.Mar 3 2021, 8:54 AM
marmoute added a subscriber: marmoute.
marmoute added inline comments.
mercurial/cext/revlog.c
513–530

Should we explicitly check that no value other than the sidedata have been changed ?

mercurial/pure/parsers.py
125–129

The changeset description state that we don't check the transaction thing in the python version. I assume that this is because we have some extra information in the C code that the python version don't have. Could we fix it ?

133–139

Same feedback as the C code, could we validate than we don't modify anything else than the side-data index ?

This revision now requires changes to proceed.Mar 3 2021, 8:54 AM
Alphare marked an inline comment as done.Mar 4 2021, 10:06 AM
Alphare added inline comments.
mercurial/cext/revlog.c
513–530

I changed it so the only arguments are sidedata length and offset

mercurial/pure/parsers.py
125–129

I misunderstood the Python implementation, this is now fixed.

Alphare edited the summary of this revision. (Show Details)Mar 4 2021, 10:18 AM
Alphare updated this revision to Diff 26094.
marmoute requested changes to this revision.Mar 9 2021, 11:52 AM

Great, I just have a final feedback. We should probably rename this method to something less generic than replace. What about replace_sidedata_info ?

This revision now requires changes to proceed.Mar 9 2021, 11:52 AM

Great, I just have a final feedback. We should probably rename this method to something less generic than replace. What about replace_sidedata_info ?

Sure

Alphare updated this revision to Diff 26225.Mar 10 2021, 2:12 PM
marmoute accepted this revision.Mar 11 2021, 1:16 PM
Alphare retitled this revision from revlog-index: add `replace` method to revlog-index: add `replace_sidedata_info` method.Mar 12 2021, 6:34 AM
Alphare edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.