( )⚙ D7322 index: add a `has_node` method (API)

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7322

index: add a `has_node` method (API)
ClosedPublic

Authored by marmoute on Nov 8 2019, 8:33 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG0c659fc20207: index: add a `has_node` method (API)
rHGec1228364a62: index: add a `has_node` method (API)
Summary

The new index.has_node(node) is to be preferred over:

`node in index.nodemap`.

This get us closer to be able to remove the nodemap attribute of the index.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

marmoute created this revision.Nov 8 2019, 8:33 AM
indygreg requested changes to this revision.Nov 8 2019, 2:44 PM
indygreg added inline comments.
mercurial/cext/revlog.c
2071

index_contains() can return -1 in many cases (including invalid input), which will cast to True. So this implementation needs improved.

This revision now requires changes to proceed.Nov 8 2019, 2:44 PM
marmoute added inline comments.Nov 8 2019, 7:31 PM
mercurial/cext/revlog.c
2071

Good catch ! updating the patch.

marmoute updated this revision to Diff 17820.Nov 8 2019, 7:34 PM
martinvonz added a subscriber: martinvonz.Nov 8 2019, 8:25 PM

I think this needs to update the version number in mercurial/cext/parsers.c and in mercurial/policy.py. That will make hg fall back to pure code if it doesn't find the right version of the C code, making updating across this commit easier.

marmoute updated this revision to Diff 17835.Nov 9 2019, 12:15 AM
marmoute added a commit: rHGec1228364a62: index: add a `has_node` method (API).Nov 9 2019, 3:38 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGec1228364a62: index: add a `has_node` method (API) (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 17835

mercurial/cext/parsers.c

{"fm1readmarkers", fm1readmarkers, METH_VARARGS, {"fm1readmarkers", fm1readmarkers, METH_VARARGS,
"parse v1 obsolete markers\n"}, "parse v1 obsolete markers\n"},
{NULL, NULL}}; {NULL, NULL}};
void dirs_module_init(PyObject *mod); void dirs_module_init(PyObject *mod);
void manifest_module_init(PyObject *mod); void manifest_module_init(PyObject *mod);
void revlog_module_init(PyObject *mod); void revlog_module_init(PyObject *mod);
static const int version = 13; static const int version = 14;
static void module_init(PyObject *mod) static void module_init(PyObject *mod)
{ {
PyObject *capsule = NULL; PyObject *capsule = NULL;
PyModule_AddIntConstant(mod, "version", version); PyModule_AddIntConstant(mod, "version", version);
/* This module constant has two purposes. First, it lets us unit test /* This module constant has two purposes. First, it lets us unit test
* the ImportError raised without hard-coding any error text. This * the ImportError raised without hard-coding any error text. This

mercurial/cext/revlog.c

return -1; return -1;
case -2: case -2:
return 0; return 0;
default: default:
return 1; return 1;
} }
} }
static PyObject *index_m_has_node(indexObject *self, PyObject *args)
{
int ret = index_contains(self, args);
if (ret < 0)
indygregUnsubmitted
Not Done

index_contains() can return -1 in many cases (including invalid input), which will cast to True. So this implementation needs improved.

indygreg: `index_contains()` can return `-1` in many cases (including invalid input), which will cast to…
marmouteAuthorUnsubmitted
Done

Good catch ! updating the patch.

marmoute: Good catch ! updating the patch.
return NULL;
return PyBool_FromLong((long)ret);
}
typedef uint64_t bitmask; typedef uint64_t bitmask;
/* /*
* Given a disjoint set of revs, return all candidates for the * Given a disjoint set of revs, return all candidates for the
* greatest common ancestor. In revset notation, this is the set * greatest common ancestor. In revset notation, this is the set
* "heads(::a and ::b and ...)" * "heads(::a and ::b and ...)"
*/ */
static PyObject *find_gca_candidates(indexObject *self, const int *revs, static PyObject *find_gca_candidates(indexObject *self, const int *revs,
{"ancestors", (PyCFunction)index_ancestors, METH_VARARGS, {"ancestors", (PyCFunction)index_ancestors, METH_VARARGS,
"return the gca set of the given revs"}, "return the gca set of the given revs"},
{"commonancestorsheads", (PyCFunction)index_commonancestorsheads, {"commonancestorsheads", (PyCFunction)index_commonancestorsheads,
METH_VARARGS, METH_VARARGS,
"return the heads of the common ancestors of the given revs"}, "return the heads of the common ancestors of the given revs"},
{"clearcaches", (PyCFunction)index_clearcaches, METH_NOARGS, {"clearcaches", (PyCFunction)index_clearcaches, METH_NOARGS,
"clear the index caches"}, "clear the index caches"},
{"get", (PyCFunction)index_m_get, METH_VARARGS, "get an index entry"}, {"get", (PyCFunction)index_m_get, METH_VARARGS, "get an index entry"},
{"has_node", (PyCFunction)index_m_has_node, METH_O,
"return True if the node exist in the index"},
{"computephasesmapsets", (PyCFunction)compute_phases_map_sets, METH_VARARGS, {"computephasesmapsets", (PyCFunction)compute_phases_map_sets, METH_VARARGS,
"compute phases"}, "compute phases"},
{"reachableroots2", (PyCFunction)reachableroots2, METH_VARARGS, {"reachableroots2", (PyCFunction)reachableroots2, METH_VARARGS,
"reachableroots"}, "reachableroots"},
{"headrevs", (PyCFunction)index_headrevs, METH_VARARGS, {"headrevs", (PyCFunction)index_headrevs, METH_VARARGS,
"get head revisions"}, /* Can do filtering since 3.2 */ "get head revisions"}, /* Can do filtering since 3.2 */
{"headrevsfiltered", (PyCFunction)index_headrevs, METH_VARARGS, {"headrevsfiltered", (PyCFunction)index_headrevs, METH_VARARGS,
"get filtered head revisions"}, /* Can always do filtering */ "get filtered head revisions"}, /* Can always do filtering */

mercurial/policy.py

# keep in sync with "version" in C modules # keep in sync with "version" in C modules
_cextversions = { _cextversions = {
('cext', 'base85'): 1, ('cext', 'base85'): 1,
('cext', 'bdiff'): 3, ('cext', 'bdiff'): 3,
('cext', 'mpatch'): 1, ('cext', 'mpatch'): 1,
('cext', 'osutil'): 4, ('cext', 'osutil'): 4,
('cext', 'parsers'): 13, ('cext', 'parsers'): 14,
} }
# map import request to other package or module # map import request to other package or module
_modredirects = { _modredirects = {
('cext', 'charencode'): ('cext', 'parsers'), ('cext', 'charencode'): ('cext', 'parsers'),
('cffi', 'base85'): ('pure', 'base85'), ('cffi', 'base85'): ('pure', 'base85'),
('cffi', 'charencode'): ('pure', 'charencode'), ('cffi', 'charencode'): ('pure', 'charencode'),
('cffi', 'parsers'): ('pure', 'parsers'), ('cffi', 'parsers'): ('pure', 'parsers'),

mercurial/pure/parsers.py

@util.propertycache @util.propertycache
def nodemap(self): def nodemap(self):
nodemap = revlogutils.NodeMap({nullid: nullrev}) nodemap = revlogutils.NodeMap({nullid: nullrev})
for r in range(0, len(self)): for r in range(0, len(self)):
n = self[r][7] n = self[r][7]
nodemap[n] = r nodemap[n] = r
return nodemap return nodemap
def has_node(self, node):
"""return True if the node exist in the index"""
return node in self.nodemap
def _stripnodes(self, start): def _stripnodes(self, start):
if 'nodemap' in vars(self): if 'nodemap' in vars(self):
for r in range(start, len(self)): for r in range(start, len(self)):
n = self[r][7] n = self[r][7]
del self.nodemap[n] del self.nodemap[n]
def clearcaches(self): def clearcaches(self):
self.__dict__.pop('nodemap', None) self.__dict__.pop('nodemap', None)

mercurial/revlog.py

@util.propertycache @util.propertycache
def nodemap(self): def nodemap(self):
nodemap = revlogutils.NodeMap({nullid: nullrev}) nodemap = revlogutils.NodeMap({nullid: nullrev})
for r in range(0, len(self)): for r in range(0, len(self)):
n = self[r][7] n = self[r][7]
nodemap[n] = r nodemap[n] = r
return nodemap return nodemap
def has_node(self, node):
"""return True if the node exist in the index"""
return node in self.nodemap
def append(self, tup): def append(self, tup):
self.nodemap[tup[7]] = len(self) self.nodemap[tup[7]] = len(self)
super(revlogoldindex, self).append(tup) super(revlogoldindex, self).append(tup)
def __delitem__(self, i): def __delitem__(self, i):
if not isinstance(i, slice) or not i.stop == -1 or i.step is not None: if not isinstance(i, slice) or not i.stop == -1 or i.step is not None:
raise ValueError(b"deleting slices only supports a:-1 with step 1") raise ValueError(b"deleting slices only supports a:-1 with step 1")
for r in pycompat.xrange(i.start, len(self)): for r in pycompat.xrange(i.start, len(self)):