( )⚙ D10927 dirstate: introduce a symbolic constant for the NONNORMAL marker

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10927

dirstate: introduce a symbolic constant for the NONNORMAL marker
ClosedPublic

Authored by marmoute on Jul 2 2021, 11:17 AM.

Details

Reviewers
SimonSapin
pulkit
Group Reviewers
hg-reviewers
Commits
rHG4ac418b4a6af: dirstate: introduce a symbolic constant for the NONNORMAL marker
Summary

This is going to be clearer and easier to track than -1. Ultimately I would
like to get ride of this special value everywhere but in the lower level,
however we need to clarify the API first. This changeset is part of such
clarification.

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

marmoute created this revision.Jul 2 2021, 11:17 AM
SimonSapin added a subscriber: SimonSapin.Jul 2 2021, 11:53 AM

+1 for having a named constant but I’m not sure about the NONNORMAL name. For example this size == -1 is also used together with state == 'r' (removed) where it means "was previously state == 'm' (merged)". The other use is in dirstate.normallookup "Mark a file normal, but possibly dirty" together with state == 'n', which does not match the constant’s doc-comment.

marmoute added a comment.Jul 2 2021, 2:54 PM
In D10927#167153, @SimonSapin wrote:

+1 for having a named constant but I’m not sure about the NONNORMAL name. For example this size == -1 is also used together with state == 'r' (removed) where it means "was previously state == 'm' (merged)". The other use is in dirstate.normallookup "Mark a file normal, but possibly dirty" together with state == 'n', which does not match the constant’s doc-comment.

The name is not great, but I don't have a much better idea and the ultimate goal of this series is make it disapear, so I am not too worried about bad name.

SimonSapin accepted this revision.Jul 2 2021, 6:24 PM

Sounds good

pulkit accepted this revision.Jul 3 2021, 7:39 AM
This revision is now accepted and ready to land.Jul 3 2021, 7:39 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (13 lines)

Diff 28759

mercurial/dirstate.py

_rangemask = 0x7FFFFFFF _rangemask = 0x7FFFFFFF
dirstatetuple = parsers.dirstatetuple dirstatetuple = parsers.dirstatetuple
# a special value used internally for `size` if the file come from the other parent # a special value used internally for `size` if the file come from the other parent
FROM_P2 = -2 FROM_P2 = -2
# a special value used internally for `size` if the file is modified/merged/added
NONNORMAL = -1
class repocache(filecache): class repocache(filecache):
"""filecache for files in .hg/""" """filecache for files in .hg/"""
def join(self, obj, fname): def join(self, obj, fname):
return obj._opener.join(fname) return obj._opener.join(fname)
def normallookup(self, f): def normallookup(self, f):
'''Mark a file normal, but possibly dirty.''' '''Mark a file normal, but possibly dirty.'''
if self._pl[1] != self._nodeconstants.nullid: if self._pl[1] != self._nodeconstants.nullid:
# if there is a merge going on and the file was either # if there is a merge going on and the file was either
# in state 'm' (-1) or coming from other parent (-2) before # in state 'm' (-1) or coming from other parent (-2) before
# being removed, restore that state. # being removed, restore that state.
entry = self._map.get(f) entry = self._map.get(f)
if entry is not None: if entry is not None:
if entry[0] == b'r' and entry[2] in (-1, FROM_P2): if entry[0] == b'r' and entry[2] in (NONNORMAL, FROM_P2):
source = self._map.copymap.get(f) source = self._map.copymap.get(f)
if entry[2] == -1: if entry[2] == NONNORMAL:
self.merge(f) self.merge(f)
elif entry[2] == FROM_P2: elif entry[2] == FROM_P2:
self.otherparent(f) self.otherparent(f)
if source: if source:
self.copy(source, f) self.copy(source, f)
return return
if entry[0] == b'm' or entry[0] == b'n' and entry[2] == FROM_P2: if entry[0] == b'm' or entry[0] == b'n' and entry[2] == FROM_P2:
return return
self._addpath(f, b'n', 0, -1, -1) self._addpath(f, b'n', 0, NONNORMAL, -1)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
def otherparent(self, f): def otherparent(self, f):
'''Mark as coming from the other parent, always dirty.''' '''Mark as coming from the other parent, always dirty.'''
if self._pl[1] == self._nodeconstants.nullid: if self._pl[1] == self._nodeconstants.nullid:
msg = _(b"setting %r to other parent only allowed in merges") % f msg = _(b"setting %r to other parent only allowed in merges") % f
raise error.Abort(msg) raise error.Abort(msg)
if f in self and self[f] == b'n': if f in self and self[f] == b'n':
# merge-like # merge-like
self._addpath(f, b'm', 0, FROM_P2, -1) self._addpath(f, b'm', 0, FROM_P2, -1)
else: else:
# add-like # add-like
self._addpath(f, b'n', 0, FROM_P2, -1) self._addpath(f, b'n', 0, FROM_P2, -1)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
def add(self, f): def add(self, f):
'''Mark a file added.''' '''Mark a file added.'''
self._addpath(f, b'a', 0, -1, -1) self._addpath(f, b'a', 0, NONNORMAL, -1)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
def remove(self, f): def remove(self, f):
'''Mark a file removed.''' '''Mark a file removed.'''
self._dirty = True self._dirty = True
oldstate = self[f] oldstate = self[f]
size = 0 size = 0
if self._pl[1] != self._nodeconstants.nullid: if self._pl[1] != self._nodeconstants.nullid:
entry = self._map.get(f) entry = self._map.get(f)
if entry is not None: if entry is not None:
# backup the previous state # backup the previous state
if entry[0] == b'm': # merge if entry[0] == b'm': # merge
size = -1 size = NONNORMAL
elif entry[0] == b'n' and entry[2] == FROM_P2: # other parent elif entry[0] == b'n' and entry[2] == FROM_P2: # other parent
size = FROM_P2 size = FROM_P2
self._map.otherparentset.add(f) self._map.otherparentset.add(f)
self._updatedfiles.add(f) self._updatedfiles.add(f)
self._map.removefile(f, oldstate, size) self._map.removefile(f, oldstate, size)
if size == 0: if size == 0:
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)