| Submitter | phabricator |
|---|---|
| Date | Nov. 8, 2019, 1:34 p.m. |
| Message ID | <differential-rev-PHID-DREV-b2malbktw7fsebz77zrl-req@mercurial-scm.org> |
| Download | mbox | patch |
| Permalink | /patch/42926/ |
| State | Superseded |
| Headers | show |
Comments
This revision now requires changes to proceed. indygreg added inline comments. indygreg requested changes to this revision. INLINE COMMENTS > revlog.c:2071 > + int ret = index_contains(self, args); > + return PyBool_FromLong((long)ret); > +} `index_contains()` can return `-1` in many cases (including invalid input), which will cast to `True`. So this implementation needs improved. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7322/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7322 To: marmoute, indygreg, #hg-reviewers Cc: mercurial-devel
marmoute added inline comments. INLINE COMMENTS > indygreg wrote in revlog.c:2071 > `index_contains()` can return `-1` in many cases (including invalid input), which will cast to `True`. So this implementation needs improved. Good catch ! updating the patch. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7322/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7322 To: marmoute, indygreg, #hg-reviewers Cc: mercurial-devel
martinvonz added a comment. 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. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7322/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7322 To: marmoute, indygreg, #hg-reviewers Cc: martinvonz, mercurial-devel
Patch
diff --git a/mercurial/revlog.py b/mercurial/revlog.py --- a/mercurial/revlog.py +++ b/mercurial/revlog.py @@ -213,6 +213,10 @@ nodemap[n] = r 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): self.nodemap[tup[7]] = len(self) super(revlogoldindex, self).append(tup) diff --git a/mercurial/pure/parsers.py b/mercurial/pure/parsers.py --- a/mercurial/pure/parsers.py +++ b/mercurial/pure/parsers.py @@ -55,6 +55,10 @@ nodemap[n] = r 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): if 'nodemap' in vars(self): for r in range(start, len(self)): diff --git a/mercurial/cext/revlog.c b/mercurial/cext/revlog.c --- a/mercurial/cext/revlog.c +++ b/mercurial/cext/revlog.c @@ -2065,6 +2065,12 @@ } } +static PyObject *index_m_has_node(indexObject *self, PyObject *args) +{ + int ret = index_contains(self, args); + return PyBool_FromLong((long)ret); +} + typedef uint64_t bitmask; /* @@ -2723,6 +2729,8 @@ {"clearcaches", (PyCFunction)index_clearcaches, METH_NOARGS, "clear the index caches"}, {"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, "compute phases"}, {"reachableroots2", (PyCFunction)reachableroots2, METH_VARARGS,
marmoute created this revision. Herald added a reviewer: indygreg. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REVISION 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D7322 AFFECTED FILES mercurial/cext/revlog.c mercurial/pure/parsers.py mercurial/revlog.py CHANGE DETAILS To: marmoute, indygreg, #hg-reviewers Cc: mercurial-devel