Page MenuHomePhabricator

node: introduce nodeconstants class
ClosedPublic

Authored by joerg.sonnenberger on Jan 13 2021, 10:55 AM.

Details

Summary

In preparing for moving from SHA1 hashes to a modern hash function,
place nullid and other constant magic vules in a class. Provide the
active set of constants in the repository and push it down. Provide
nullid directly in strategic places like the repository as it is
accessed very often. This changeset introduces the API change, but not
the mechanical replacement of the node.py attributes itself.

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

This is the API changing part of D9465 without all the followup changes.

So, I'm still not sold on the "only one hash function in a repo, ever" constraint of the design this seems to imply. Is there a plan page or anything or is this all in your head?

I'm aiming more for "only one hash size in the repository" and the magic constants staying the same independent of the hash function. The hash itself is planned to be tagged, i.e. 248bit for the hash function and the trailing byte as type tag. The SHA1 replacement would use "01" and in principle allow for existing SHA1 nodes to embedded directly into the larger 256bit space as type "00".

durin42 requested changes to this revision.Mar 16 2021, 5:52 PM

Fair enough. This is ready, but needs rebased.

This revision now requires changes to proceed.Mar 16 2021, 5:52 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.

This made test-git-interop.t fail more than before. Can you send a fix? The end of the traceback looks like this:

+    File "/usr/local/google/home/martinvonz/hg/hgext/git/gitlog.py", line 221, in changelogrevision
+      return hgchangelog._changelogrevision(extra={})
+  TypeError: __init__() missing 1 required positional argument: 'manifest'
+  [1]