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
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.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 17865

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)):