Submitter | Georges Racinet |
---|---|
Date | Jan. 24, 2019, 4:23 a.m. |
Message ID | <47d5458a4c32e20de4af.1548303833@ishtar> |
Download | mbox | patch |
Permalink | /patch/37953/ |
State | Accepted |
Headers | show |
Comments
On Wed, Jan 23, 2019 at 23:23:53 -0500, Georges Racinet wrote: ... > diff -r a35cfd592a90 -r 47d5458a4c32 tests/test-rust-ancestor.py > --- a/tests/test-rust-ancestor.py Wed Jan 23 07:47:04 2019 -0500 > +++ b/tests/test-rust-ancestor.py Wed Jan 23 07:49:36 2019 -0500 > @@ -1,6 +1,10 @@ > from __future__ import absolute_import > import sys > import unittest > +from mercurial import ( > + error, > + node, > + ) Is there supposed to be whitespace before the ) ? I'm not all that familiar with the python coding style. I know, such important feedback... :) Jeff. > > try: > from mercurial import rustext > @@ -153,6 +157,12 @@ > # rust-cpython issues appropriate str instances for Python 2 and 3 > self.assertEqual(exc.args, ('ParentOutOfRange', 1)) > > + def testwdirunsupported(self): > + # trying to access ancestors of the working directory raises > + # WdirUnsupported directly > + idx = self.parseindex() > + with self.assertRaises(error.WdirUnsupported): > + list(AncestorsIterator(idx, [node.wdirrev], -1, False)) > > if __name__ == '__main__': > import silenttestrunner > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On 1/24/19 11:11 AM, Josef 'Jeff' Sipek wrote: > On Wed, Jan 23, 2019 at 23:23:53 -0500, Georges Racinet wrote: > ... >> diff -r a35cfd592a90 -r 47d5458a4c32 tests/test-rust-ancestor.py >> --- a/tests/test-rust-ancestor.py Wed Jan 23 07:47:04 2019 -0500 >> +++ b/tests/test-rust-ancestor.py Wed Jan 23 07:49:36 2019 -0500 >> @@ -1,6 +1,10 @@ >> from __future__ import absolute_import >> import sys >> import unittest >> +from mercurial import ( >> + error, >> + node, >> + ) > Is there supposed to be whitespace before the ) ? I'm not all that familiar > with the python coding style. I'm not sure about that, but the coding style [1] doesn't seem to say much about that case, and, since it's cited as a lose reference, pep8 accepts both. The indentation you're seeing is just what my editor does by default. I didn't do anything else than making test-check-* happy [1] https://www.mercurial-scm.org/wiki/CodingStyle#Whitespace_and_syntax > I know, such important feedback... :) No worries ;-)
On Thu, Jan 24, 2019 at 15:41:56 -0500, Georges Racinet wrote: > On 1/24/19 11:11 AM, Josef 'Jeff' Sipek wrote: > > > On Wed, Jan 23, 2019 at 23:23:53 -0500, Georges Racinet wrote: > > ... > >> diff -r a35cfd592a90 -r 47d5458a4c32 tests/test-rust-ancestor.py > >> --- a/tests/test-rust-ancestor.py Wed Jan 23 07:47:04 2019 -0500 > >> +++ b/tests/test-rust-ancestor.py Wed Jan 23 07:49:36 2019 -0500 > >> @@ -1,6 +1,10 @@ > >> from __future__ import absolute_import > >> import sys > >> import unittest > >> +from mercurial import ( > >> + error, > >> + node, > >> + ) > > Is there supposed to be whitespace before the ) ? I'm not all that familiar > > with the python coding style. > > I'm not sure about that, but the coding style [1] doesn't seem to say > much about that case, and, since it's cited as a lose reference, pep8 > accepts both. > > The indentation you're seeing is just what my editor does by default. I > didn't do anything else than making test-check-* happy Fair enough. Sorry for the noise. :) For the little it is worth, I didn't see anything wrong with the rest of the series. Jeff.
On Thu, 24 Jan 2019 11:11:41 -0500, Josef 'Jeff' Sipek wrote: > On Wed, Jan 23, 2019 at 23:23:53 -0500, Georges Racinet wrote: > ... > > diff -r a35cfd592a90 -r 47d5458a4c32 tests/test-rust-ancestor.py > > --- a/tests/test-rust-ancestor.py Wed Jan 23 07:47:04 2019 -0500 > > +++ b/tests/test-rust-ancestor.py Wed Jan 23 07:49:36 2019 -0500 > > @@ -1,6 +1,10 @@ > > from __future__ import absolute_import > > import sys > > import unittest > > +from mercurial import ( > > + error, > > + node, > > + ) > > Is there supposed to be whitespace before the ) ? I'm not all that familiar > with the python coding style. Queued, thanks. Actually we prefer ")" at the same level of the starting statement. Emacs can be configured as such: (setq py-closing-list-dedents-bos t)
Patch
diff -r a35cfd592a90 -r 47d5458a4c32 mercurial/revlog.py --- a/mercurial/revlog.py Wed Jan 23 07:47:04 2019 -0500 +++ b/mercurial/revlog.py Wed Jan 23 07:49:36 2019 -0500 @@ -896,8 +896,6 @@ common = [nullrev] if rustext is not None: - # TODO: WdirUnsupported should be raised instead of GraphError - # if common includes wdirrev return rustext.ancestor.MissingAncestors(self.index, common) return ancestor.incrementalmissingancestors(self.parentrevs, common) diff -r a35cfd592a90 -r 47d5458a4c32 rust/hg-cpython/src/cindex.rs --- a/rust/hg-cpython/src/cindex.rs Wed Jan 23 07:47:04 2019 -0500 +++ b/rust/hg-cpython/src/cindex.rs Wed Jan 23 07:49:36 2019 -0500 @@ -16,7 +16,7 @@ use self::python_sys::PyCapsule_Import; use cpython::{PyClone, PyErr, PyObject, PyResult, Python}; -use hg::{Graph, GraphError, Revision}; +use hg::{Graph, GraphError, Revision, WORKING_DIRECTORY_REVISION}; use libc::c_int; use std::ffi::CStr; use std::mem::transmute; @@ -86,6 +86,9 @@ impl Graph for Index { /// wrap a call to the C extern parents function fn parents(&self, rev: Revision) -> Result<[Revision; 2], GraphError> { + if rev == WORKING_DIRECTORY_REVISION { + return Err(GraphError::WorkingDirectoryUnsupported); + } let mut res: [c_int; 2] = [0; 2]; let code = unsafe { (self.parents)( diff -r a35cfd592a90 -r 47d5458a4c32 tests/test-rust-ancestor.py --- a/tests/test-rust-ancestor.py Wed Jan 23 07:47:04 2019 -0500 +++ b/tests/test-rust-ancestor.py Wed Jan 23 07:49:36 2019 -0500 @@ -1,6 +1,10 @@ from __future__ import absolute_import import sys import unittest +from mercurial import ( + error, + node, + ) try: from mercurial import rustext @@ -153,6 +157,12 @@ # rust-cpython issues appropriate str instances for Python 2 and 3 self.assertEqual(exc.args, ('ParentOutOfRange', 1)) + def testwdirunsupported(self): + # trying to access ancestors of the working directory raises + # WdirUnsupported directly + idx = self.parseindex() + with self.assertRaises(error.WdirUnsupported): + list(AncestorsIterator(idx, [node.wdirrev], -1, False)) if __name__ == '__main__': import silenttestrunner