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
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, 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 17889

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