This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2698

merge: use constants for merge state record types
ClosedPublic

Authored by indygreg on Mar 5 2018, 7:15 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGa532b2f54f95: merge: use constants for merge state record types
Summary

merge.py is using multiple discrete sets of 1 and 2 letter constants
to define types and behavior. To the uninitiated, the code is very
difficult to reason about. I didn't even realize there were multiple
sets of constants in play initially!

We begin our sanity injection with merge state records. The record
types (which are serialized to disk) are now defined in RECORD_*
constants.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Mar 5 2018, 7:15 PM
martinvonz added a subscriber: martinvonz.Mar 21 2018, 8:34 PM
martinvonz added inline comments.
mercurial/merge.py
50

Perhaps we should move that documentation to here so we don't have two lists of the same values that can end up getting out of sync? That would also make it easier to review this patch because the descriptions would be right next to the names you picked.

durin42 accepted this revision.Mar 24 2018, 3:07 PM
This revision is now accepted and ready to land.Mar 24 2018, 3:07 PM
Closed by commit rHGa532b2f54f95: merge: use constants for merge state record types (authored by indygreg). · Explain WhyMar 24 2018, 5:22 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (76 lines)

Diff 6658

mercurial/merge.py

_unpack = struct.unpack _unpack = struct.unpack
def _droponode(data): def _droponode(data):
# used for compatibility for v1 # used for compatibility for v1
bits = data.split('\0') bits = data.split('\0')
bits = bits[:-2] + bits[-1:] bits = bits[:-2] + bits[-1:]
return '\0'.join(bits) return '\0'.join(bits)
# Merge state record types. See ``mergestate`` docs for more.
martinvonzUnsubmitted
Not Done

Perhaps we should move that documentation to here so we don't have two lists of the same values that can end up getting out of sync? That would also make it easier to review this patch because the descriptions would be right next to the names you picked.

martinvonz: Perhaps we should move that documentation to here so we don't have two lists of the same values…
RECORD_LOCAL = b'L'
RECORD_OTHER = b'O'
RECORD_MERGED = b'F'
RECORD_CHANGEDELETE_CONFLICT = b'C'
RECORD_MERGE_DRIVER_MERGE = b'D'
RECORD_PATH_CONFLICT = b'P'
RECORD_MERGE_DRIVER_STATE = b'm'
RECORD_FILE_VALUES = b'f'
RECORD_LABELS = b'l'
RECORD_OVERRIDE = b't'
RECORD_UNSUPPORTED_MANDATORY = b'X'
RECORD_UNSUPPORTED_ADVISORY = b'x'
class mergestate(object): class mergestate(object):
'''track 3-way merge state of individual files '''track 3-way merge state of individual files
The merge state is stored on disk when needed. Two files are used: one with The merge state is stored on disk when needed. Two files are used: one with
an old format (version 1), and one with a new format (version 2). Version 2 an old format (version 1), and one with a new format (version 2). Version 2
stores a superset of the data in version 1, including new kinds of records stores a superset of the data in version 1, including new kinds of records
in the future. For more about the new format, see the documentation for in the future. For more about the new format, see the documentation for
`_readrecordsv2`. `_readrecordsv2`.
for var in ('localctx', 'otherctx'): for var in ('localctx', 'otherctx'):
if var in vars(self): if var in vars(self):
delattr(self, var) delattr(self, var)
self._readmergedriver = None self._readmergedriver = None
self._mdstate = 's' self._mdstate = 's'
unsupported = set() unsupported = set()
records = self._readrecords() records = self._readrecords()
for rtype, record in records: for rtype, record in records:
if rtype == 'L': if rtype == RECORD_LOCAL:
self._local = bin(record) self._local = bin(record)
elif rtype == 'O': elif rtype == RECORD_OTHER:
self._other = bin(record) self._other = bin(record)
elif rtype == 'm': elif rtype == RECORD_MERGE_DRIVER_STATE:
bits = record.split('\0', 1) bits = record.split('\0', 1)
mdstate = bits[1] mdstate = bits[1]
if len(mdstate) != 1 or mdstate not in 'ums': if len(mdstate) != 1 or mdstate not in 'ums':
# the merge driver should be idempotent, so just rerun it # the merge driver should be idempotent, so just rerun it
mdstate = 'u' mdstate = 'u'
self._readmergedriver = bits[0] self._readmergedriver = bits[0]
self._mdstate = mdstate self._mdstate = mdstate
elif rtype in 'FDCP': elif rtype in (RECORD_MERGED, RECORD_CHANGEDELETE_CONFLICT,
RECORD_PATH_CONFLICT, RECORD_MERGE_DRIVER_MERGE):
bits = record.split('\0') bits = record.split('\0')
self._state[bits[0]] = bits[1:] self._state[bits[0]] = bits[1:]
elif rtype == 'f': elif rtype == RECORD_FILE_VALUES:
filename, rawextras = record.split('\0', 1) filename, rawextras = record.split('\0', 1)
extraparts = rawextras.split('\0') extraparts = rawextras.split('\0')
extras = {} extras = {}
i = 0 i = 0
while i < len(extraparts): while i < len(extraparts):
extras[extraparts[i]] = extraparts[i + 1] extras[extraparts[i]] = extraparts[i + 1]
i += 2 i += 2
self._stateextras[filename] = extras self._stateextras[filename] = extras
elif rtype == 'l': elif rtype == RECORD_LABELS:
labels = record.split('\0', 2) labels = record.split('\0', 2)
self._labels = [l for l in labels if len(l) > 0] self._labels = [l for l in labels if len(l) > 0]
elif not rtype.islower(): elif not rtype.islower():
unsupported.add(rtype) unsupported.add(rtype)
self._results = {} self._results = {}
self._dirty = False self._dirty = False
if unsupported: if unsupported:
v2records = self._readrecordsv2() v2records = self._readrecordsv2()
if self._v1v2match(v1records, v2records): if self._v1v2match(v1records, v2records):
return v2records return v2records
else: else:
# v1 file is newer than v2 file, use it # v1 file is newer than v2 file, use it
# we have to infer the "other" changeset of the merge # we have to infer the "other" changeset of the merge
# we cannot do better than that with v1 of the format # we cannot do better than that with v1 of the format
mctx = self._repo[None].parents()[-1] mctx = self._repo[None].parents()[-1]
v1records.append(('O', mctx.hex())) v1records.append((RECORD_OTHER, mctx.hex()))
# add place holder "other" file node information # add place holder "other" file node information
# nobody is using it yet so we do no need to fetch the data # nobody is using it yet so we do no need to fetch the data
# if mctx was wrong `mctx[bits[-2]]` may fails. # if mctx was wrong `mctx[bits[-2]]` may fails.
for idx, r in enumerate(v1records): for idx, r in enumerate(v1records):
if r[0] == 'F': if r[0] == RECORD_MERGED:
bits = r[1].split('\0') bits = r[1].split('\0')
bits.insert(-2, '') bits.insert(-2, '')
v1records[idx] = (r[0], '\0'.join(bits)) v1records[idx] = (r[0], '\0'.join(bits))
return v1records return v1records
def _v1v2match(self, v1records, v2records): def _v1v2match(self, v1records, v2records):
oldv2 = set() # old format version of v2 record oldv2 = set() # old format version of v2 record
for rec in v2records: for rec in v2records:
if rec[0] == 'L': if rec[0] == RECORD_LOCAL:
oldv2.add(rec) oldv2.add(rec)
elif rec[0] == 'F': elif rec[0] == RECORD_MERGED:
# drop the onode data (not contained in v1) # drop the onode data (not contained in v1)
oldv2.add(('F', _droponode(rec[1]))) oldv2.add((RECORD_MERGED, _droponode(rec[1])))
for rec in v1records: for rec in v1records:
if rec not in oldv2: if rec not in oldv2:
return False return False
else: else:
return True return True
def _readrecordsv1(self): def _readrecordsv1(self):
"""read on disk merge state for version 1 file """read on disk merge state for version 1 file
returns list of record [(TYPE, data), ...] returns list of record [(TYPE, data), ...]
Note: the "F" data from this file are one entry short Note: the "F" data from this file are one entry short
(no "other file node" entry) (no "other file node" entry)
""" """
records = [] records = []
try: try:
f = self._repo.vfs(self.statepathv1) f = self._repo.vfs(self.statepathv1)
for i, l in enumerate(f): for i, l in enumerate(f):
if i == 0: if i == 0:
records.append(('L', l[:-1])) records.append((RECORD_LOCAL, l[:-1]))
else: else:
records.append(('F', l[:-1])) records.append((RECORD_MERGED, l[:-1]))
f.close() f.close()
except IOError as err: except IOError as err:
if err.errno != errno.ENOENT: if err.errno != errno.ENOENT:
raise raise
return records return records
def _readrecordsv2(self): def _readrecordsv2(self):
"""read on disk merge state for version 2 file """read on disk merge state for version 2 file
end = len(data) end = len(data)
while off < end: while off < end:
rtype = data[off:off + 1] rtype = data[off:off + 1]
off += 1 off += 1
length = _unpack('>I', data[off:(off + 4)])[0] length = _unpack('>I', data[off:(off + 4)])[0]
off += 4 off += 4
record = data[off:(off + length)] record = data[off:(off + length)]
off += length off += length
if rtype == 't': if rtype == RECORD_OVERRIDE:
rtype, record = record[0:1], record[1:] rtype, record = record[0:1], record[1:]
records.append((rtype, record)) records.append((rtype, record))
f.close() f.close()
except IOError as err: except IOError as err:
if err.errno != errno.ENOENT: if err.errno != errno.ENOENT:
raise raise
return records return records
"""Write current state on disk (if necessary)""" """Write current state on disk (if necessary)"""
if self._dirty: if self._dirty:
records = self._makerecords() records = self._makerecords()
self._writerecords(records) self._writerecords(records)
self._dirty = False self._dirty = False
def _makerecords(self): def _makerecords(self):
records = [] records = []
records.append(('L', hex(self._local))) records.append((RECORD_LOCAL, hex(self._local)))
records.append(('O', hex(self._other))) records.append((RECORD_OTHER, hex(self._other)))
if self.mergedriver: if self.mergedriver:
records.append(('m', '\0'.join([ records.append((RECORD_MERGE_DRIVER_STATE, '\0'.join([
self.mergedriver, self._mdstate]))) self.mergedriver, self._mdstate])))
# Write out state items. In all cases, the value of the state map entry # Write out state items. In all cases, the value of the state map entry
# is written as the contents of the record. The record type depends on # is written as the contents of the record. The record type depends on
# the type of state that is stored, and capital-letter records are used # the type of state that is stored, and capital-letter records are used
# to prevent older versions of Mercurial that do not support the feature # to prevent older versions of Mercurial that do not support the feature
# from loading them. # from loading them.
for filename, v in self._state.iteritems(): for filename, v in self._state.iteritems():
if v[0] == 'd': if v[0] == 'd':
# Driver-resolved merge. These are stored in 'D' records. # Driver-resolved merge. These are stored in 'D' records.
records.append(('D', '\0'.join([filename] + v))) records.append((RECORD_MERGE_DRIVER_MERGE,
'\0'.join([filename] + v)))
elif v[0] in ('pu', 'pr'): elif v[0] in ('pu', 'pr'):
# Path conflicts. These are stored in 'P' records. The current # Path conflicts. These are stored in 'P' records. The current
# resolution state ('pu' or 'pr') is stored within the record. # resolution state ('pu' or 'pr') is stored within the record.
records.append(('P', '\0'.join([filename] + v))) records.append((RECORD_PATH_CONFLICT,
'\0'.join([filename] + v)))
elif v[1] == nullhex or v[6] == nullhex: elif v[1] == nullhex or v[6] == nullhex:
# Change/Delete or Delete/Change conflicts. These are stored in # Change/Delete or Delete/Change conflicts. These are stored in
# 'C' records. v[1] is the local file, and is nullhex when the # 'C' records. v[1] is the local file, and is nullhex when the
# file is deleted locally ('dc'). v[6] is the remote file, and # file is deleted locally ('dc'). v[6] is the remote file, and
# is nullhex when the file is deleted remotely ('cd'). # is nullhex when the file is deleted remotely ('cd').
records.append(('C', '\0'.join([filename] + v))) records.append((RECORD_CHANGEDELETE_CONFLICT,
'\0'.join([filename] + v)))
else: else:
# Normal files. These are stored in 'F' records. # Normal files. These are stored in 'F' records.
records.append(('F', '\0'.join([filename] + v))) records.append((RECORD_MERGED,
'\0'.join([filename] + v)))
for filename, extras in sorted(self._stateextras.iteritems()): for filename, extras in sorted(self._stateextras.iteritems()):
rawextras = '\0'.join('%s\0%s' % (k, v) for k, v in rawextras = '\0'.join('%s\0%s' % (k, v) for k, v in
extras.iteritems()) extras.iteritems())
records.append(('f', '%s\0%s' % (filename, rawextras))) records.append((RECORD_FILE_VALUES,
'%s\0%s' % (filename, rawextras)))
if self._labels is not None: if self._labels is not None:
labels = '\0'.join(self._labels) labels = '\0'.join(self._labels)
records.append(('l', labels)) records.append((RECORD_LABELS, labels))
return records return records
def _writerecords(self, records): def _writerecords(self, records):
"""Write current state on disk (both v1 and v2)""" """Write current state on disk (both v1 and v2)"""
self._writerecordsv1(records) self._writerecordsv1(records)
self._writerecordsv2(records) self._writerecordsv2(records)
def _writerecordsv1(self, records): def _writerecordsv1(self, records):
"""Write current state on disk in a version 1 file""" """Write current state on disk in a version 1 file"""
f = self._repo.vfs(self.statepathv1, 'wb') f = self._repo.vfs(self.statepathv1, 'wb')
irecords = iter(records) irecords = iter(records)
lrecords = next(irecords) lrecords = next(irecords)
assert lrecords[0] == 'L' assert lrecords[0] == RECORD_LOCAL
f.write(hex(self._local) + '\n') f.write(hex(self._local) + '\n')
for rtype, data in irecords: for rtype, data in irecords:
if rtype == 'F': if rtype == RECORD_MERGED:
f.write('%s\n' % _droponode(data)) f.write('%s\n' % _droponode(data))
f.close() f.close()
def _writerecordsv2(self, records): def _writerecordsv2(self, records):
"""Write current state on disk in a version 2 file """Write current state on disk in a version 2 file
See the docstring for _readrecordsv2 for why we use 't'.""" See the docstring for _readrecordsv2 for why we use 't'."""
# these are the records that all version 2 clients can read # these are the records that all version 2 clients can read
whitelist = 'LOF' allowlist = (RECORD_LOCAL, RECORD_OTHER, RECORD_MERGED)
f = self._repo.vfs(self.statepathv2, 'wb') f = self._repo.vfs(self.statepathv2, 'wb')
for key, data in records: for key, data in records:
assert len(key) == 1 assert len(key) == 1
if key not in whitelist: if key not in allowlist:
key, data = 't', '%s%s' % (key, data) key, data = RECORD_OVERRIDE, '%s%s' % (key, data)
format = '>sI%is' % len(data) format = '>sI%is' % len(data)
f.write(_pack(format, key, len(data), data)) f.write(_pack(format, key, len(data), data))
f.close() f.close()
def add(self, fcl, fco, fca, fd): def add(self, fcl, fco, fca, fd):
"""add a new (potentially?) conflicting file the merge state """add a new (potentially?) conflicting file the merge state
fcl: file context for local, fcl: file context for local,
fco: file context for remote, fco: file context for remote,