This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10104

pure-parsers: document index class constants
ClosedPublic

Authored by Alphare on Mar 4 2021, 3:31 AM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGe83327af26f1: pure-parsers: document index class constants
Summary

This also adds the big endian prefix > to make the constants truly
platform-independent, even if no issue with this has been reported in the wild.

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.Mar 4 2021, 3:31 AM
pulkit accepted this revision.Mar 4 2021, 3:57 AM
This revision is now accepted and ready to land.Mar 4 2021, 3:57 AM
SimonSapin added a subscriber: SimonSapin.Mar 4 2021, 4:15 AM
SimonSapin added inline comments.
mercurial/pure/parsers.py
48–49

This description isn’t factually wrong, but wouldn’t it be a serious bug if the on-disk format ends up varying across different platforms?

SimonSapin added inline comments.Mar 4 2021, 6:22 AM
mercurial/pure/parsers.py
48–49

Correction: since index_format starts with >, it opts in both to big-endian and to "stardard sizes" so index_size is in fact platform-independent. See docs in https://docs.python.org/3/library/struct.html#format-characters

Maybe > should be added to the format string for big_int_size and int_size as well, just to be safe.

Alphare added inline comments.Mar 4 2021, 10:02 AM
mercurial/pure/parsers.py
48–49

Thanks for the catch, I'll add > to the other format strings

Alphare edited the summary of this revision. (Show Details)Mar 4 2021, 10:10 AM
Alphare added a child revision: D10107: requirements: also add a dotencode constant.
Alphare updated this revision to Diff 26080.
pulkit accepted this revision.Mar 5 2021, 3:37 AM
Alphare added a commit: rHGe83327af26f1: pure-parsers: document index class constants.Mar 5 2021, 4:34 AM
Closed by commit rHGe83327af26f1: pure-parsers: document index class constants (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/pure/parsers.py (9 lines)

Diff 26101

mercurial/pure/parsers.py

return int(q & 0xFFFF) return int(q & 0xFFFF)
def offset_type(offset, type): def offset_type(offset, type):
return int(int(offset) << 16 | type) return int(int(offset) << 16 | type)
class BaseIndexObject(object): class BaseIndexObject(object):
# Format of an index entry according to Python's `struct` language
index_format = b">Qiiiiii20s12x" index_format = b">Qiiiiii20s12x"
big_int_size = struct.calcsize(b'Q') # Size of a C unsigned long long int, platform independent
int_size = struct.calcsize(b'i') big_int_size = struct.calcsize(b'>Q')
# Size of a C long int, platform independent
SimonSapinUnsubmitted
Not Done

This description isn’t factually wrong, but wouldn’t it be a serious bug if the on-disk format ends up varying across different platforms?

SimonSapin: This description isn’t factually wrong, but wouldn’t it be a serious bug if the on-disk format…
SimonSapinUnsubmitted
Not Done

Correction: since index_format starts with >, it opts in both to big-endian and to "stardard sizes" so index_size is in fact platform-independent. See docs in https://docs.python.org/3/library/struct.html#format-characters

Maybe > should be added to the format string for big_int_size and int_size as well, just to be safe.

SimonSapin: Correction: since `index_format` starts with `>`, it opts in both to big-endian and to…
AlphareAuthorUnsubmitted
Done

Thanks for the catch, I'll add > to the other format strings

Alphare: Thanks for the catch, I'll add `>` to the other format strings
int_size = struct.calcsize(b'>i')
# Size of the entire index format
index_size = struct.calcsize(index_format) index_size = struct.calcsize(index_format)
# An empty index entry, used as a default value to be overridden, or nullrev
null_item = (0, 0, 0, -1, -1, -1, -1, nullid) null_item = (0, 0, 0, -1, -1, -1, -1, nullid)
@property @property
def nodemap(self): def nodemap(self):
msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]" msg = b"index.nodemap is deprecated, use index.[has_node|rev|get_rev]"
util.nouideprecwarn(msg, b'5.3', stacklevel=2) util.nouideprecwarn(msg, b'5.3', stacklevel=2)
return self._nodemap return self._nodemap