This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8801

obsstore: refactor v1 logic to fix 32 byte hash support
ClosedPublic

Authored by joerg.sonnenberger on Jul 23 2020, 2:34 PM.

Details

Reviewers
Alphare
pulkit
Group Reviewers
hg-reviewers
Commits
rHG145cfe84d3e4: obsstore: refactor v1 logic to fix 32 byte hash support
Summary

Refactor the v1 logic to determine the node parsing based on the flag.
Move the predecessor out of the fixed part and handle it like the other
nodes, removing most of the duplicated code for parsing 20/32 bytes
hashes.

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

pulkit added subscribers: marmoute, pulkit.Aug 27 2020, 4:03 AM

@marmoute this is something you know more about.

Alphare accepted this revision.Sep 10 2020, 4:15 AM
Alphare added a subscriber: Alphare.

Other than my "nit but please do it" comment, this looks fine.

mercurial/obsolete.py
414

I would add a comment explaining the magic 1

pulkit accepted this revision.Sep 16 2020, 3:32 AM
This revision is now accepted and ready to land.Sep 16 2020, 3:32 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/obsolete.py (84 lines)

Diff 22647

mercurial/obsolete.py

# - N*(20 or 32) bytes: successors changesets identifiers. # - N*(20 or 32) bytes: successors changesets identifiers.
# #
# - P*(20 or 32) bytes: parents of the predecessors changesets. # - P*(20 or 32) bytes: parents of the predecessors changesets.
# #
# - M*(uint8, uint8): size of all metadata entries (key and value) # - M*(uint8, uint8): size of all metadata entries (key and value)
# #
# - remaining bytes: the metadata, each (key, value) pair after the other. # - remaining bytes: the metadata, each (key, value) pair after the other.
_fm1version = 1 _fm1version = 1
_fm1fixed = b'>IdhHBBB20s' _fm1fixed = b'>IdhHBBB'
_fm1nodesha1 = b'20s' _fm1nodesha1 = b'20s'
_fm1nodesha256 = b'32s' _fm1nodesha256 = b'32s'
_fm1nodesha1size = _calcsize(_fm1nodesha1) _fm1nodesha1size = _calcsize(_fm1nodesha1)
_fm1nodesha256size = _calcsize(_fm1nodesha256) _fm1nodesha256size = _calcsize(_fm1nodesha256)
_fm1fsize = _calcsize(_fm1fixed) _fm1fsize = _calcsize(_fm1fixed)
_fm1parentnone = 3 _fm1parentnone = 3
_fm1parentshift = 14 _fm1parentshift = 14
_fm1parentmask = _fm1parentnone << _fm1parentshift _fm1parentmask = _fm1parentnone << _fm1parentshift
unpack = _unpack unpack = _unpack
# Loop on markers # Loop on markers
ufixed = struct.Struct(_fm1fixed).unpack ufixed = struct.Struct(_fm1fixed).unpack
while off < stop: while off < stop:
# read fixed part # read fixed part
o1 = off + fsize o1 = off + fsize
t, secs, tz, flags, numsuc, numpar, nummeta, prec = ufixed(data[off:o1]) t, secs, tz, flags, numsuc, numpar, nummeta = ufixed(data[off:o1])
if flags & sha2flag: if flags & sha2flag:
# FIXME: prec was read as a SHA1, needs to be amended nodefmt = sha2fmt
nodesize = sha2size
# read 0 or more successors
if numsuc == 1:
o2 = o1 + sha2size
sucs = (data[o1:o2],)
else: else:
o2 = o1 + sha2size * numsuc nodefmt = sha1fmt
sucs = unpack(sha2fmt * numsuc, data[o1:o2]) nodesize = sha1size
(prec,) = unpack(nodefmt, data[o1 : o1 + nodesize])
o1 += nodesize
# read parents
if numpar == noneflag:
o3 = o2
parents = None
elif numpar == 1:
o3 = o2 + sha2size
parents = (data[o2:o3],)
else:
o3 = o2 + sha2size * numpar
parents = unpack(sha2fmt * numpar, data[o2:o3])
else:
# read 0 or more successors # read 0 or more successors
if numsuc == 1: if numsuc == 1:
o2 = o1 + sha1size o2 = o1 + nodesize
sucs = (data[o1:o2],) sucs = (data[o1:o2],)
else: else:
o2 = o1 + sha1size * numsuc o2 = o1 + nodesize * numsuc
sucs = unpack(sha1fmt * numsuc, data[o1:o2]) sucs = unpack(nodefmt * numsuc, data[o1:o2])
# read parents # read parents
if numpar == noneflag: if numpar == noneflag:
o3 = o2 o3 = o2
parents = None parents = None
elif numpar == 1: elif numpar == 1:
o3 = o2 + sha1size o3 = o2 + nodesize
parents = (data[o2:o3],) parents = (data[o2:o3],)
else: else:
o3 = o2 + sha1size * numpar o3 = o2 + nodesize * numpar
parents = unpack(sha1fmt * numpar, data[o2:o3]) parents = unpack(nodefmt * numpar, data[o2:o3])
# read metadata # read metadata
off = o3 + metasize * nummeta off = o3 + metasize * nummeta
metapairsize = unpack(b'>' + (metafmt * nummeta), data[o3:off]) metapairsize = unpack(b'>' + (metafmt * nummeta), data[o3:off])
metadata = [] metadata = []
for idx in pycompat.xrange(0, len(metapairsize), 2): for idx in pycompat.xrange(0, len(metapairsize), 2):
o1 = off + metapairsize[idx] o1 = off + metapairsize[idx]
o2 = o1 + metapairsize[idx + 1] o2 = o1 + metapairsize[idx + 1]
metadata.append((data[off:o1], data[o1:o2])) metadata.append((data[off:o1], data[o1:o2]))
off = o2 off = o2
yield (prec, sucs, flags, tuple(metadata), (secs, tz * 60), parents) yield (prec, sucs, flags, tuple(metadata), (secs, tz * 60), parents)
def _fm1encodeonemarker(marker): def _fm1encodeonemarker(marker):
pre, sucs, flags, metadata, date, parents = marker pre, sucs, flags, metadata, date, parents = marker
# determine node size # determine node size
_fm1node = _fm1nodesha1 _fm1node = _fm1nodesha1
if flags & usingsha256: if flags & usingsha256:
_fm1node = _fm1nodesha256 _fm1node = _fm1nodesha256
numsuc = len(sucs) numsuc = len(sucs)
numextranodes = numsuc numextranodes = 1 + numsuc
AlphareUnsubmitted
Not Done

I would add a comment explaining the magic 1

Alphare: I would add a comment explaining the magic 1
if parents is None: if parents is None:
numpar = _fm1parentnone numpar = _fm1parentnone
else: else:
numpar = len(parents) numpar = len(parents)
numextranodes += numpar numextranodes += numpar
formatnodes = _fm1node * numextranodes formatnodes = _fm1node * numextranodes
formatmeta = _fm1metapair * len(metadata) formatmeta = _fm1metapair * len(metadata)
format = _fm1fixed + formatnodes + formatmeta format = _fm1fixed + formatnodes + formatmeta
* encode metadata * encode metadata
If you are a human writing code creating marker you want to use the If you are a human writing code creating marker you want to use the
`createmarkers` function in this module instead. `createmarkers` function in this module instead.
return True if a new marker have been added, False if the markers return True if a new marker have been added, False if the markers
already existed (no op). already existed (no op).
""" """
flag = int(flag)
if metadata is None: if metadata is None:
metadata = {} metadata = {}
if date is None: if date is None:
if b'date' in metadata: if b'date' in metadata:
# as a courtesy for out-of-tree extensions # as a courtesy for out-of-tree extensions
date = dateutil.parsedate(metadata.pop(b'date')) date = dateutil.parsedate(metadata.pop(b'date'))
elif ui is not None: elif ui is not None:
date = ui.configdate(b'devel', b'default-date') date = ui.configdate(b'devel', b'default-date')
if date is None: if date is None:
date = dateutil.makedate() date = dateutil.makedate()
else: else:
date = dateutil.makedate() date = dateutil.makedate()
if flag & usingsha256:
if len(prec) != 32:
raise ValueError(prec)
for succ in succs:
if len(succ) != 32:
raise ValueError(succ)
else:
if len(prec) != 20: if len(prec) != 20:
raise ValueError(prec) raise ValueError(prec)
for succ in succs: for succ in succs:
if len(succ) != 20: if len(succ) != 20:
raise ValueError(succ) raise ValueError(succ)
if prec in succs: if prec in succs:
raise ValueError( raise ValueError(
'in-marker cycle with %s' % pycompat.sysstr(node.hex(prec)) 'in-marker cycle with %s' % pycompat.sysstr(node.hex(prec))
) )
metadata = tuple(sorted(pycompat.iteritems(metadata))) metadata = tuple(sorted(pycompat.iteritems(metadata)))
for k, v in metadata: for k, v in metadata:
try: try:
# might be better to reject non-ASCII keys # might be better to reject non-ASCII keys
k.decode('utf-8') k.decode('utf-8')
v.decode('utf-8') v.decode('utf-8')
except UnicodeDecodeError: except UnicodeDecodeError:
raise error.ProgrammingError( raise error.ProgrammingError(
b'obsstore metadata must be valid UTF-8 sequence ' b'obsstore metadata must be valid UTF-8 sequence '
b'(key = %r, value = %r)' b'(key = %r, value = %r)'
% (pycompat.bytestr(k), pycompat.bytestr(v)) % (pycompat.bytestr(k), pycompat.bytestr(v))
) )
marker = (bytes(prec), tuple(succs), int(flag), metadata, date, parents) marker = (bytes(prec), tuple(succs), flag, metadata, date, parents)
return bool(self.add(transaction, [marker])) return bool(self.add(transaction, [marker]))
def add(self, transaction, markers): def add(self, transaction, markers):
"""Add new markers to the store """Add new markers to the store
Take care of filtering duplicate. Take care of filtering duplicate.
Return the number of new marker.""" Return the number of new marker."""
if self._readonly: if self._readonly: