This is an archive of the discontinued Mercurial Phabricator instance.

Differential D1339

dirstate: don't remove normallookup files from nonnormalset
ClosedPublic

Authored by mbthomas on Nov 8 2017, 12:30 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHG1664dc7ccd8a: dirstate: don't remove normallookup files from nonnormalset
Summary

The dirstate typically tries to keep the nonnormalset and otherparentset
up-to-date when making changes to the dirstate. In the case of files marked
normallookup, however, it erroneously removes the file from the nonnormalset,
after _addpath had just added it.

This doesn't seem to matter at the moment, as nothing relies on the
nonnormalset being correct at this point, however a future re-implementations
of the dirstate map will require this to be kept up-to-date correctly.

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

mbthomas created this revision.Nov 8 2017, 12:30 PM
indygreg added a subscriber: indygreg.Nov 10 2017, 12:23 AM

I was going to review this series. But I fear my dirstate knowledge is not good enough to understand the implications of all the changes. So I don't plan to look at it unless another reviewer can't be found.

durin42 added a subscriber: durin42.Nov 10 2017, 5:03 PM

Based on a very shaky understanding of the code gained by spending 10 minutes looking around at uses of nonnormalset, I think this is right.

I'd appreciate a patch that describes what all these attributes are intended to contain, especially if there's about to be some api-ish contracts going on between this and other code (even if there's no stability contract)

durin42 accepted this revision.Nov 10 2017, 5:03 PM
This revision is now accepted and ready to land.Nov 10 2017, 5:03 PM
Closed by commit rHG1664dc7ccd8a: dirstate: don't remove normallookup files from nonnormalset (authored by mbthomas). · Explain WhyNov 10 2017, 6:03 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/dirstate.py (2 lines)

Diff 3413

mercurial/dirstate.py

self.otherparent(f) self.otherparent(f)
if source: if source:
self.copy(source, f) self.copy(source, f)
return return
if entry[0] == 'm' or entry[0] == 'n' and entry[2] == -2: if entry[0] == 'm' or entry[0] == 'n' and entry[2] == -2:
return return
self._addpath(f, 'n', 0, -1, -1) self._addpath(f, 'n', 0, -1, -1)
self._map.copymap.pop(f, None) self._map.copymap.pop(f, None)
if f in self._map.nonnormalset:
self._map.nonnormalset.remove(f)
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] == nullid: if self._pl[1] == nullid:
raise error.Abort(_("setting %r to other parent " raise error.Abort(_("setting %r to other parent "
"only allowed in merges") % f) "only allowed in merges") % f)
if f in self and self[f] == 'n': if f in self and self[f] == 'n':
# merge-like # merge-like