This is an archive of the discontinued Mercurial Phabricator instance.

dirstate: move management of nonnormal sets into dirstate map
ClosedPublic

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

Details

Summary

The dirstate map owns the nonnormal sets, and so should be the class to update
them. A future implementation of the dirstate will manage these maps
differently.

The action of clearing ambiguous times is now entirely controlled by the
dirstate map, so it moves there too.

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:31 PM
mbolin added a subscriber: mbolin.Nov 9 2017, 8:09 PM
mbolin added inline comments.
mercurial/dirstate.py
1286

I would prefer to see all mutations to self._map go through a common code path so that we can override this behavior easier in Eden.

As it stands, when this logic is conflated, it makes it much harder for us to safely subclass dirstatemap in Eden. For reference, here's what we're doing today:

https://github.com/facebookexperimental/eden-hg/blob/master/eden/hg/eden/eden_dirstate_map.py

I strongly suspect that (if you agree with my feedback on D1340) this will need some adjustments to document the expanded behavior of dirstatemap WRT nonnormalset (whatever a nonnormalset is)

mbthomas added inline comments.Nov 11 2017, 8:17 AM
mercurial/dirstate.py
1286

I think what you want is a common tuple constructor hook point - eden_dirstate_map does build a different tuple, but it just sets it as a property of the map in the same way. I think we want to avoid another layer of indirection if we can.

Arguably dirstatetuple is already one of these, as it can be either a native tuple type, or the fast C-based one from parses.c, but maybe that's the wrong place for eden to hook as it's global to the module. Instead, we can do it by setting dirstatemap._tuplecls to dirstatetuple in the constructor, so the code here becomes more like:

self._map[f] = self._tuplecls(state, mode, size, mtime)

In eden you can make your own function and assign that to self._tuplecls in eden_dirstate_map.__init__.

mbthomas updated this revision to Diff 3438.Nov 13 2017, 7:18 AM
durin42 accepted this revision.Nov 13 2017, 5:43 PM
This revision is now accepted and ready to land.Nov 13 2017, 5:43 PM
mbthomas updated this revision to Diff 3515.Nov 15 2017, 4:08 AM
durin42 accepted this revision.Nov 17 2017, 5:22 PM
This revision was automatically updated to reflect the committed changes.