( )⚙ D7324 index: add a `rev` method (API)

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7324

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

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

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGbd87114ce341: index: add a `rev` method (API)
rHGb43a685edbc7: index: add a `rev` method (API)
Summary

The new index.rev(node) is to be preferred over using `node
index.nodemap[node]`.

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, 3:06 PM

The code is fine. But since I've already left a number of comments on this series, I thought I'd ask about raising a better exception than RevlogError. I thought we had dedicated exceptions for revision not existing?

This revision now requires changes to proceed.Nov 8 2019, 3:06 PM
marmoute requested review of this revision.Nov 8 2019, 7:36 PM
In D7324#107591, @indygreg wrote:

The code is fine. But since I've already left a number of comments on this series, I thought I'd ask about raising a better exception than RevlogError. I thought we had dedicated exceptions for revision not existing?

This is the exception currently raised by nodemap access and recognised by higher level code (that turn it into a better exception). Maybe this should change ? but this is out of scope from this series in my opinion.

marmoute updated this revision to Diff 17822.Nov 8 2019, 8:10 PM
marmoute updated this revision to Diff 17837.Nov 9 2019, 12:15 AM
martinvonz added a subscriber: martinvonz.Nov 9 2019, 3:35 AM

Perhaps it's best to bump the version number in this patch and the get_rev patch as well? I don't feel strongly, but I'll queue all the has_node patches to start with.

marmoute updated this revision to Diff 17883.Nov 9 2019, 8:13 AM
indygreg requested changes to this revision.Nov 9 2019, 10:45 AM

I'm fine ignoring the RevlogError quirk, since it already exists.

This revision now requires changes to proceed.Nov 9 2019, 10:45 AM
indygreg accepted this revision.Nov 9 2019, 10:46 AM
This revision is now accepted and ready to land.Nov 9 2019, 10:46 AM
marmoute added a commit: rHGb43a685edbc7: index: add a `rev` method (API).Nov 9 2019, 10:56 AM
Closed by commit rHGb43a685edbc7: index: add a `rev` method (API) (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 17883

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 = 14; static const int version = 15;
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

static PyObject *index_m_has_node(indexObject *self, PyObject *args) static PyObject *index_m_has_node(indexObject *self, PyObject *args)
{ {
int ret = index_contains(self, args); int ret = index_contains(self, args);
if (ret < 0) if (ret < 0)
return NULL; return NULL;
return PyBool_FromLong((long)ret); return PyBool_FromLong((long)ret);
} }
static PyObject *index_m_rev(indexObject *self, PyObject *val)
{
char *node;
int rev;
if (node_check(val, &node) == -1)
return NULL;
rev = index_find_node(self, node, 20);
if (rev >= -1)
return PyInt_FromLong(rev);
if (rev == -2)
raise_revlog_error();
return NULL;
}
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,
{"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, {"has_node", (PyCFunction)index_m_has_node, METH_O,
"return True if the node exist in the index"}, "return True if the node exist in the index"},
{"rev", (PyCFunction)index_m_rev, METH_O,
"return `rev` associated with a node or raise RevlogError"},
{"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'): 14, ('cext', 'parsers'): 15,
} }
# 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

n = self[r][7] n = self[r][7]
nodemap[n] = r nodemap[n] = r
return nodemap return nodemap
def has_node(self, node): def has_node(self, node):
"""return True if the node exist in the index""" """return True if the node exist in the index"""
return node in self.nodemap return node in self.nodemap
def rev(self, node):
"""return a revision for a node
If the node is unknown, raise a RevlogError"""
return self.nodemap[node]
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

n = self[r][7] n = self[r][7]
nodemap[n] = r nodemap[n] = r
return nodemap return nodemap
def has_node(self, node): def has_node(self, node):
"""return True if the node exist in the index""" """return True if the node exist in the index"""
return node in self.nodemap return node in self.nodemap
def rev(self, node):
"""return a revision for a node
If the node is unknown, raise a RevlogError"""
return self.nodemap[node]
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)):