D5438: rust-cpython: implementing Graph using C parents function
This revision was automatically updated to reflect the committed changes. Closed by commit rHG4c25038c112c: rust-cpython: implement Graph using C parents function (authored by gracinet, committed by ). CHANGED PRIOR TO COMMIT https://phab.mercurial-scm.org/D5438?vs=12949=12967#toc REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5438?vs=12949=12967 REVISION DETAIL https://phab.mercurial-scm.org/D5438 AFFECTED FILES mercurial/cext/revlog.c rust/hg-cpython/src/cindex.rs rust/hg-cpython/src/lib.rs CHANGE DETAILS diff --git a/rust/hg-cpython/src/lib.rs b/rust/hg-cpython/src/lib.rs --- a/rust/hg-cpython/src/lib.rs +++ b/rust/hg-cpython/src/lib.rs @@ -21,8 +21,10 @@ #[macro_use] extern crate cpython; extern crate hg; +extern crate libc; mod ancestors; +mod cindex; mod exceptions; py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| { diff --git a/rust/hg-cpython/src/cindex.rs b/rust/hg-cpython/src/cindex.rs new file mode 100644 --- /dev/null +++ b/rust/hg-cpython/src/cindex.rs @@ -0,0 +1,121 @@ +// cindex.rs +// +// Copyright 2018 Georges Racinet +// +// This software may be used and distributed according to the terms of the +// GNU General Public License version 2 or any later version. + +//! Bindings to use the Index defined by the parsers C extension +//! +//! Ideally, we should use an Index entirely implemented in Rust, +//! but this will take some time to get there. +#[cfg(feature = "python27")] +extern crate python27_sys as python_sys; +#[cfg(feature = "python3")] +extern crate python3_sys as python_sys; + +use self::python_sys::PyCapsule_Import; +use cpython::{PyErr, PyObject, PyResult, Python}; +use hg::{Graph, GraphError, Revision}; +use libc::c_int; +use std::ffi::CStr; +use std::mem::transmute; + +type IndexParentsFn = unsafe extern "C" fn( +index: *mut python_sys::PyObject, +rev: c_int, +ps: *mut [c_int; 2], +) -> c_int; + +/// A `Graph` backed up by objects and functions from revlog.c +/// +/// This implementation of the `Graph` trait, relies on (pointers to) +/// - the C index object (`index` member) +/// - the `index_get_parents()` function (`parents` member) +/// +/// # Safety +/// +/// The C index itself is mutable, and this Rust exposition is **not +/// protected by the GIL**, meaning that this construct isn't safe with respect +/// to Python threads. +/// +/// All callers of this `Index` must acquire the GIL and must not release it +/// while working. +/// +/// # TODO find a solution to make it GIL safe again. +/// +/// This is non trivial, and can wait until we have a clearer picture with +/// more Rust Mercurial constructs. +/// +/// One possibility would be to a `GILProtectedIndex` wrapper enclosing +/// a `Python<'p>` marker and have it be the one implementing the +/// `Graph` trait, but this would mean the `Graph` implementor would become +/// likely to change between subsequent method invocations of the `hg-core` +/// objects (a serious change of the `hg-core` API): +/// either exposing ways to mutate the `Graph`, or making it a non persistent +/// parameter in the relevant methods that need one. +/// +/// Another possibility would be to introduce an abstract lock handle into +/// the core API, that would be tied to `GILGuard` / `Python<'p>` +/// in the case of the `cpython` crate bindings yet could leave room for other +/// mechanisms in other contexts. + +pub struct Index { +index: PyObject, +parents: IndexParentsFn, +} + +impl Index { +pub fn new(py: Python, index: PyObject) -> PyResult { +Ok(Index { +index: index, +parents: decapsule_parents_fn(py)?, +}) +} +} + +impl Graph for Index { +/// wrap a call to the C extern parents function +fn parents(, rev: Revision) -> Result<[Revision; 2], GraphError> { +let mut res: [c_int; 2] = [0; 2]; +let code = unsafe { +(self.parents)( +self.index.as_ptr(), +rev as c_int, + res as *mut [c_int; 2], +) +}; +match code { +0 => Ok(res), +_ => Err(GraphError::ParentOutOfRange(rev)), +} +} +} + +/// Return the `index_get_parents` function of the parsers C Extension module. +/// +/// A pointer to the function is stored in the `parsers` module as a +/// standard [Python capsule](https://docs.python.org/2/c-api/capsule.html). +/// +/// This function retrieves the capsule and casts the function pointer +/// +/// Casting function pointers is one of the rare cases of +/// legitimate use cases of `mem::transmute()` (see +/// https://doc.rust-lang.org/std/mem/fn.transmute.html of +/// `mem::transmute()`. +/// It is inappropriate for architectures where +/// function and data pointer sizes differ (so-called "Harvard +/// architectures"), but these are nowadays mostly DSPs +/// and microcontrollers, hence out of our scope. +fn decapsule_parents_fn(py: Python) ->
D5440: rust: core implementation for lazyancestors
This revision was automatically updated to reflect the committed changes. Closed by commit rHGef54bd33b476: rust: core implementation for lazyancestors (authored by gracinet, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5440?vs=12954=12968 REVISION DETAIL https://phab.mercurial-scm.org/D5440 AFFECTED FILES rust/hg-core/src/ancestors.rs rust/hg-core/src/lib.rs rust/hg-cpython/src/cindex.rs CHANGE DETAILS diff --git a/rust/hg-cpython/src/cindex.rs b/rust/hg-cpython/src/cindex.rs --- a/rust/hg-cpython/src/cindex.rs +++ b/rust/hg-cpython/src/cindex.rs @@ -15,7 +15,7 @@ extern crate python3_sys as python_sys; use self::python_sys::PyCapsule_Import; -use cpython::{PyErr, PyObject, PyResult, Python}; +use cpython::{PyClone, PyErr, PyObject, PyResult, Python}; use hg::{Graph, GraphError, Revision}; use libc::c_int; use std::ffi::CStr; @@ -59,7 +59,6 @@ /// the core API, that would be tied to `GILGuard` / `Python<'p>` /// in the case of the `cpython` crate bindings yet could leave room for other /// mechanisms in other contexts. - pub struct Index { index: PyObject, parents: IndexParentsFn, @@ -74,6 +73,16 @@ } } +impl Clone for Index { +fn clone() -> Self { +let guard = Python::acquire_gil(); +Index { +index: self.index.clone_ref(guard.python()), +parents: self.parents.clone(), +} +} +} + impl Graph for Index { /// wrap a call to the C extern parents function fn parents(, rev: Revision) -> Result<[Revision; 2], GraphError> { diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs --- a/rust/hg-core/src/lib.rs +++ b/rust/hg-core/src/lib.rs @@ -3,7 +3,7 @@ // This software may be used and distributed according to the terms of the // GNU General Public License version 2 or any later version. mod ancestors; -pub use ancestors::{AncestorsIterator, MissingAncestors}; +pub use ancestors::{AncestorsIterator, LazyAncestors, MissingAncestors}; /// Mercurial revision numbers /// diff --git a/rust/hg-core/src/ancestors.rs b/rust/hg-core/src/ancestors.rs --- a/rust/hg-core/src/ancestors.rs +++ b/rust/hg-core/src/ancestors.rs @@ -25,6 +25,15 @@ stoprev: Revision, } +/// Lazy ancestors set, backed by AncestorsIterator +pub struct LazyAncestors { +graph: G, +containsiter: AncestorsIterator, +initrevs: Vec, +stoprev: Revision, +inclusive: bool, +} + pub struct MissingAncestors { graph: G, bases: HashSet, @@ -98,9 +107,31 @@ } Ok(false) } + +pub fn peek() -> Option { +self.visit.peek().map(|| r) +} + +/// Tell if the iterator is about an empty set +/// +/// The result does not depend whether the iterator has been consumed +/// or not. +/// This is mostly meant for iterators backing a lazy ancestors set +pub fn is_empty() -> bool { +if self.visit.len() > 0 { +return false; +} +if self.seen.len() > 1 { +return false; +} +// at this point, the seen set is at most a singleton. +// If not `self.inclusive`, it's still possible that it has only +// the null revision +self.seen.is_empty() || self.seen.contains(_REVISION) +} } -/// Main implementation. +/// Main implementation for the iterator /// /// The algorithm is the same as in `_lazyancestorsiter()` from `ancestors.py` /// with a few non crucial differences: @@ -137,6 +168,49 @@ } } +impl LazyAncestors { +pub fn new( +graph: G, +initrevs: impl IntoIterator, +stoprev: Revision, +inclusive: bool, +) -> Result { +let v: Vec = initrevs.into_iter().collect(); +Ok(LazyAncestors { +graph: graph.clone(), +containsiter: AncestorsIterator::new( +graph, +v.iter().cloned(), +stoprev, +inclusive, +)?, +initrevs: v, +stoprev: stoprev, +inclusive: inclusive, +}) +} + +pub fn contains( self, rev: Revision) -> Result { +self.containsiter.contains(rev) +} + +pub fn is_empty() -> bool { +self.containsiter.is_empty() +} + +pub fn iter() -> AncestorsIterator { +// the arguments being the same as for self.containsiter, we know +// for sure that AncestorsIterator constructor can't fail +AncestorsIterator::new( +self.graph.clone(), +self.initrevs.iter().cloned(), +self.stoprev, +self.inclusive, +) +.unwrap() +} +} + impl MissingAncestors { pub fn new(graph: G, bases: impl IntoIterator) -> Self { let mut bases: HashSet = bases.into_iter().collect(); @@ -407,7 +481,40 @@ assert!(!lazy.contains(NULL_REVISION).unwrap()); } +#[test] +fn test_peek() { +let
D5439: rust-cpython: binding for AncestorsIterator
This revision was automatically updated to reflect the committed changes. Closed by commit rHGd9f439fcdb4c: rust-cpython: binding for AncestorsIterator (authored by gracinet, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5439?vs=12953=12966 REVISION DETAIL https://phab.mercurial-scm.org/D5439 AFFECTED FILES rust/hg-cpython/src/ancestors.rs tests/test-rust-ancestor.py CHANGE DETAILS diff --git a/tests/test-rust-ancestor.py b/tests/test-rust-ancestor.py --- a/tests/test-rust-ancestor.py +++ b/tests/test-rust-ancestor.py @@ -1,19 +1,46 @@ from __future__ import absolute_import +import sys import unittest try: from mercurial import rustext +rustext.__name__ # trigger immediate actual import except ImportError: rustext = None +else: +# this would fail already without appropriate ancestor.__package__ +from mercurial.rustext.ancestor import AncestorsIterator try: from mercurial.cext import parsers as cparsers except ImportError: cparsers = None +# picked from test-parse-index2, copied rather than imported +# so that it stays stable even if test-parse-index2 changes or disappears. +data_non_inlined = ( +b'\x00\x00\x00\x01\x00\x00\x00\x00\x00\x01D\x19' +b'\x00\x07e\x12\x00\x00\x00\x00\x00\x00\x00\x00\xff\xff\xff\xff' +b'\xff\xff\xff\xff\xd1\xf4\xbb\xb0\xbe\xfc\x13\xbd\x8c\xd3\x9d' +b'\x0f\xcd\xd9;\x8c\x07\x8cJ/\x00\x00\x00\x00\x00\x00\x00\x00\x00' +b'\x00\x00\x00\x00\x00\x00\x01D\x19\x00\x00\x00\x00\x00\xdf\x00' +b'\x00\x01q\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x00\xff' +b'\xff\xff\xff\xc1\x12\xb9\x04\x96\xa4Z1t\x91\xdfsJ\x90\xf0\x9bh' +b'\x07l&\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00' +b'\x00\x01D\xf8\x00\x00\x00\x00\x01\x1b\x00\x00\x01\xb8\x00\x00' +b'\x00\x01\x00\x00\x00\x02\x00\x00\x00\x01\xff\xff\xff\xff\x02\n' +b'\x0e\xc6&\xa1\x92\xae6\x0b\x02i\xfe-\xe5\xbao\x05\xd1\xe7\x00' +b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x01F' +b'\x13\x00\x00\x00\x00\x01\xec\x00\x00\x03\x06\x00\x00\x00\x01' +b'\x00\x00\x00\x03\x00\x00\x00\x02\xff\xff\xff\xff\x12\xcb\xeby1' +b'\xb6\r\x98B\xcb\x07\xbd`\x8f\x92\xd9\xc4\x84\xbdK\x00\x00\x00' +b'\x00\x00\x00\x00\x00\x00\x00\x00\x00' +) + + @unittest.skipIf(rustext is None or cparsers is None, - "rustext.ancestor or the C Extension parsers module " - "it relies on is not available") + "rustext or the C Extension parsers module " + "ancestor relies on is not available") class rustancestorstest(unittest.TestCase): """Test the correctness of binding to Rust code. @@ -27,11 +54,52 @@ Algorithmic correctness is asserted by the Rust unit tests. """ -def testmodule(self): -self.assertTrue('DAG' in rustext.ancestor.__doc__) +def parseindex(self): +return cparsers.parse_index2(data_non_inlined, False)[0] + +def testiteratorrevlist(self): +idx = self.parseindex() +# checking test assumption about the index binary data: +self.assertEqual({i: (r[5], r[6]) for i, r in enumerate(idx)}, + {0: (-1, -1), + 1: (0, -1), + 2: (1, -1), + 3: (2, -1)}) +ait = AncestorsIterator(idx, [3], 0, True) +self.assertEqual([r for r in ait], [3, 2, 1, 0]) + +ait = AncestorsIterator(idx, [3], 0, False) +self.assertEqual([r for r in ait], [2, 1, 0]) + +def testrefcount(self): +idx = self.parseindex() +start_count = sys.getrefcount(idx) + +# refcount increases upon iterator init... +ait = AncestorsIterator(idx, [3], 0, True) +self.assertEqual(sys.getrefcount(idx), start_count + 1) +self.assertEqual(next(ait), 3) + +# and decreases once the iterator is removed +del ait +self.assertEqual(sys.getrefcount(idx), start_count) + +# and removing ref to the index after iterator init is no issue +ait = AncestorsIterator(idx, [3], 0, True) +del idx +self.assertEqual([r for r in ait], [3, 2, 1, 0]) def testgrapherror(self): -self.assertTrue('GraphError' in dir(rustext)) +data = (data_non_inlined[:64 + 27] + +b'\xf2' + +data_non_inlined[64 + 28:]) +idx = cparsers.parse_index2(data, False)[0] +with self.assertRaises(rustext.GraphError) as arc: +AncestorsIterator(idx, [1], -1, False) +exc = arc.exception +self.assertIsInstance(exc, ValueError) +# rust-cpython issues appropriate str instances for Python 2 and 3 +self.assertEqual(exc.args, ('ParentOutOfRange', 1)) if __name__ == '__main__': diff --git a/rust/hg-cpython/src/ancestors.rs b/rust/hg-cpython/src/ancestors.rs --- a/rust/hg-cpython/src/ancestors.rs +++
D5441: rust-cpython: binding for LazyAncestors
yuja added a comment. > +/// The purpose of this module is to hide identifiers from other Rust users > +/// > +/// Some of the identifiers we are defining are meant for consumption > +/// from Python with other naming conventions. For instance, `py_class!` > +/// does not let us make a distinction between the Python and the Rust name, > +/// and that name ends up in particular in the `__class__` attribute, so that > +/// renaming it inside its Python module is not enough. > +/// > +/// For Rust consumers, we will reexport these definitions, following the > +/// appropriate naming convention. > +mod concealed_detail { > +use super::*; > + > +py_class!(pub class lazyancestors |py| { Does the class name actually matter? Personally I don't care if `lazyancestors()` function returns a `LazyAncestors` object. We'll anyway need a wrapper function to make pure ancestors and rustext ancestors compatible. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5441 To: gracinet, #hg-reviewers, kevincox Cc: yuja, durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5441: rust-cpython: binding for LazyAncestors
> +/// The purpose of this module is to hide identifiers from other Rust users > +/// > +/// Some of the identifiers we are defining are meant for consumption > +/// from Python with other naming conventions. For instance, `py_class!` > +/// does not let us make a distinction between the Python and the Rust name, > +/// and that name ends up in particular in the `__class__` attribute, so that > +/// renaming it inside its Python module is not enough. > +/// > +/// For Rust consumers, we will reexport these definitions, following the > +/// appropriate naming convention. > +mod concealed_detail { > +use super::*; > + > +py_class!(pub class lazyancestors |py| { Does the class name actually matter? Personally I don't care if `lazyancestors()` function returns a `LazyAncestors` object. We'll anyway need a wrapper function to make pure ancestors and rustext ancestors compatible. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5442: rust-cpython: using the new bindings from Python
yuja added a comment. > +try: > +from . import rustext Touch e.g. rustext.__doc__ here to get around the lazy importer. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5442 To: gracinet, indygreg, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5442: rust-cpython: using the new bindings from Python
> +try: > +from . import rustext Touch e.g. rustext.__doc__ here to get around the lazy importer. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5473: config: register evolution.stackaliases as a config option
Oh, yes. It is extension specific configuration. Abandoning this one. On Mon, Dec 24, 2018 at 8:41 AM yuja (Yuya Nishihara) < phabrica...@mercurial-scm.org> wrote: > yuja added a comment. > > > > +coreconfigitem('experimental', 'evolution.stackaliases', > > +default=True, > > +) > > No `stackaliases` found in core. Maybe it's an extension-specific config? > > REPOSITORY > rHG Mercurial > > REVISION DETAIL > https://phab.mercurial-scm.org/D5473 > > To: khanchi97, #hg-reviewers > Cc: yuja, mercurial-devel > ___ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)
valentin.gatienbaron added a comment. Thanks! > IIRC, listkeys is a newer command than lookup. If the peer doesn't support listkeys, I suspect this batch query would fail. In that case, maybe listkeys has to be skipped if the peer doesn't support it and if --bookmark is not specified. listkeys shouldn't need compatibility check in the caller, because it's defined like this in wireprotov1peer.py (https://www.mercurial-scm.org/repo/hg-committed/file/tip/mercurial/wireprotov1peer.py#l381): @batchable def listkeys(self, namespace): if not self.capable('pushkey'): yield {}, None ... REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5449 To: valentin.gatienbaron, #hg-reviewers Cc: yuja, indygreg, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5475: branch:Added option to show branch name of a given rev (Issue5948)
yuja added a comment. > Thanks for the info @yuja. I'm thinking to move --rev/-r to branches command and replacing --show/-s that I've made to --rev/-r in the branch command itself. I'm not sure if I get it, but my idea is to add `hg branches -r/--rev` that selects branches to be listed by revisions. So, `hg branch -srREV` in your original patch will be equivalent to `hg branches -qrREV` (-q to suppress detailed output.) REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5475 To: navaneeth.suresh, #hg-reviewers Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5475: branch:Added option to show branch name of a given rev (Issue5948)
> Thanks for the info @yuja. I'm thinking to move --rev/-r to branches > command and replacing --show/-s that I've made to --rev/-r in the branch > command itself. I'm not sure if I get it, but my idea is to add `hg branches -r/--rev` that selects branches to be listed by revisions. So, `hg branch -srREV` in your original patch will be equivalent to `hg branches -qrREV` (-q to suppress detailed output.) ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)
This revision was automatically updated to reflect the committed changes. Closed by commit rHGbad05a6afdc8: pull: fix inconsistent view of bookmarks during pull (issue4700) (authored by valentin.gatienbaron, committed by ). CHANGED PRIOR TO COMMIT https://phab.mercurial-scm.org/D5449?vs=12938=12965#toc REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5449?vs=12938=12965 REVISION DETAIL https://phab.mercurial-scm.org/D5449 AFFECTED FILES mercurial/commands.py tests/test-bookmarks-pushpull.t CHANGE DETAILS diff --git a/tests/test-bookmarks-pushpull.t b/tests/test-bookmarks-pushpull.t --- a/tests/test-bookmarks-pushpull.t +++ b/tests/test-bookmarks-pushpull.t @@ -673,12 +673,13 @@ adding manifests adding file changes added 1 changesets with 1 changes to 1 files + updating bookmark Y new changesets 0d60821d2197 (1 drafts) (run 'hg update' to get a working copy) $ hg book @ 1:0d2164f0ce0d X 1:0d2164f0ce0d - * Y 5:35d1ef0a8d1b + * Y 6:0d60821d2197 Z 1:0d2164f0ce0d $ hg -R $TESTTMP/pull-race book @ 1:0d2164f0ce0d diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -4414,49 +4414,47 @@ revs, checkout = hg.addbranchrevs(repo, other, branches, opts.get('rev')) - pullopargs = {} -if opts.get('bookmark'): -if not revs: -revs = [] -# The list of bookmark used here is not the one used to actually -# update the bookmark name. This can result in the revision pulled -# not ending up with the name of the bookmark because of a race -# condition on the server. (See issue 4689 for details) -remotebookmarks = other.listkeys('bookmarks') + +nodes = None +if opts['bookmark'] or revs: +# The list of bookmark used here is the same used to actually update +# the bookmark names, to avoid the race from issue 4689 and we do +# all lookup and bookmark queries in one go so they see the same +# version of the server state (issue 4700). +nodes = [] +fnodes = [] +revs = revs or [] +if revs and not other.capable('lookup'): +err = _("other repository doesn't support revision lookup, " +"so a rev cannot be specified.") +raise error.Abort(err) +with other.commandexecutor() as e: +fremotebookmarks = e.callcommand('listkeys', { +'namespace': 'bookmarks' +}) +for r in revs: +fnodes.append(e.callcommand('lookup', {'key': r})) +remotebookmarks = fremotebookmarks.result() remotebookmarks = bookmarks.unhexlifybookmarks(remotebookmarks) pullopargs['remotebookmarks'] = remotebookmarks for b in opts['bookmark']: b = repo._bookmarks.expandname(b) if b not in remotebookmarks: raise error.Abort(_('remote bookmark %s not found!') % b) -revs.append(hex(remotebookmarks[b])) - -if revs: -try: -# When 'rev' is a bookmark name, we cannot guarantee that it -# will be updated with that name because of a race condition -# server side. (See issue 4689 for details) -oldrevs = revs -revs = [] # actually, nodes -for r in oldrevs: -with other.commandexecutor() as e: -node = e.callcommand('lookup', {'key': r}).result() - -revs.append(node) -if r == checkout: -checkout = node -except error.CapabilityError: -err = _("other repository doesn't support revision lookup, " -"so a rev cannot be specified.") -raise error.Abort(err) +nodes.append(remotebookmarks[b]) +for i, rev in enumerate(revs): +node = fnodes[i].result() +nodes.append(node) +if rev == checkout: +checkout = node wlock = util.nullcontextmanager() if opts.get('update'): wlock = repo.wlock() with wlock: pullopargs.update(opts.get('opargs', {})) -modheads = exchange.pull(repo, other, heads=revs, +modheads = exchange.pull(repo, other, heads=nodes, force=opts.get('force'), bookmarks=opts.get('bookmark', ()),
D5476: merge: modify the logical statement
This revision was automatically updated to reflect the committed changes. Closed by commit rHG6faaf3a1c6ec: merge: modify the logical statement (authored by khanchi97, committed by ). REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5476?vs=12963=12964 REVISION DETAIL https://phab.mercurial-scm.org/D5476 AFFECTED FILES mercurial/merge.py CHANGE DETAILS diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -1538,8 +1538,8 @@ unresolvedcount = attr.ib() def isempty(self): -return (not self.updatedcount and not self.mergedcount -and not self.removedcount and not self.unresolvedcount) +return not (self.updatedcount or self.mergedcount +or self.removedcount or self.unresolvedcount) def emptyactions(): """create an actions dict, to be populated and passed to applyupdates()""" To: khanchi97, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)
yuja added a comment. Queued, thanks. I have a concern about compatibility with ancient Mercurial. Can you check it and send a follow-up as needed? > I had to change the handling of the case where the server doesn't > support the lookup query, because if it fails, it would otherwise make > fremotebookmark.result() block forever. This is due to > wireprotov1peer.peerexecutor.sendcommands's behavior (it fills a > single future if any query fails synchronously and leaves all other > futures unchanged), but I don't know if the fix is to cancel all other > futures, or to keep going with the other queries. @indygreg > +if opts['bookmark'] or revs: > +# The list of bookmark used here is the same used to actually update > +# the bookmark names, to avoid the race from issue 4689 and we do > +# all lookup and bookmark queries in one go so they see the same > +# version of the server state (issue 4700). > +nodes = [] > +fnodes = [] > +revs = revs or [] > +if revs and not other.capable('lookup'): > +err = _("other repository doesn't support revision lookup, " > +"so a rev cannot be specified.") > +raise error.Abort(err) > +with other.commandexecutor() as e: > +fremotebookmarks = e.callcommand('listkeys', { > +'namespace': 'bookmarks' > +}) > +for r in revs: > +fnodes.append(e.callcommand('lookup', {'key': r})) IIRC, `listkeys` is a newer command than `lookup`. If the peer doesn't support `listkeys`, I suspect this batch query would fail. In that case, maybe `listkeys` has to be skipped if the peer doesn't support it and if --bookmark is not specified. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5449 To: valentin.gatienbaron, #hg-reviewers Cc: yuja, indygreg, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5449: pull: fix inconsistent view of bookmarks during pull (issue4700)
Queued, thanks. I have a concern about compatibility with ancient Mercurial. Can you check it and send a follow-up as needed? > I had to change the handling of the case where the server doesn't > support the lookup query, because if it fails, it would otherwise make > fremotebookmark.result() block forever. This is due to > wireprotov1peer.peerexecutor.sendcommands's behavior (it fills a > single future if any query fails synchronously and leaves all other > futures unchanged), but I don't know if the fix is to cancel all other > futures, or to keep going with the other queries. @indygreg > +if opts['bookmark'] or revs: > +# The list of bookmark used here is the same used to actually > update > +# the bookmark names, to avoid the race from issue 4689 and we do > +# all lookup and bookmark queries in one go so they see the same > +# version of the server state (issue 4700). > +nodes = [] > +fnodes = [] > +revs = revs or [] > +if revs and not other.capable('lookup'): > +err = _("other repository doesn't support revision lookup, " > +"so a rev cannot be specified.") > +raise error.Abort(err) > +with other.commandexecutor() as e: > +fremotebookmarks = e.callcommand('listkeys', { > +'namespace': 'bookmarks' > +}) > +for r in revs: > +fnodes.append(e.callcommand('lookup', {'key': r})) IIRC, `listkeys` is a newer command than `lookup`. If the peer doesn't support `listkeys`, I suspect this batch query would fail. In that case, maybe `listkeys` has to be skipped if the peer doesn't support it and if --bookmark is not specified. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5473: config: register evolution.stackaliases as a config option
yuja added a comment. > +coreconfigitem('experimental', 'evolution.stackaliases', > +default=True, > +) No `stackaliases` found in core. Maybe it's an extension-specific config? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5473 To: khanchi97, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5473: config: register evolution.stackaliases as a config option
> +coreconfigitem('experimental', 'evolution.stackaliases', > +default=True, > +) No `stackaliases` found in core. Maybe it's an extension-specific config? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 6 V2] extensions: import the exthelper class from evolve 980565468003 (API)
On Sun, 23 Dec 2018 08:27:42 -0500, Yuya Nishihara wrote: On Sun, 23 Dec 2018 01:15:46 -0500, Matt Harbison wrote: # HG changeset patch # User Matt Harbison # Date 1545530784 18000 # Sat Dec 22 21:06:24 2018 -0500 # Node ID ffab2010329f111c2237ebbb64be650a1b0301d8 # Parent e9c606fef203621755d75b0434574a8a60ffd0ff extensions: import the exthelper class from evolve 980565468003 (API) Queued with several pyflakes fixes, thanks. Weird. Pyflakes on Windows doesn't catch any of that. Sorry about that. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5475: branch:Added option to show branch name of a given rev (Issue5948)
navaneeth.suresh added a comment. Thanks for the info @yuja. I'm thinking to move --rev/-r to branches command and replacing --show/-s that I've made to --rev/-r in the branch command itself. How's that sound to you guys, @pulkit @yuja ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5475 To: navaneeth.suresh, #hg-reviewers Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5475: branch:Added option to show branch name of a given rev (Issue5948)
pulkit added a comment. In https://phab.mercurial-scm.org/D5475#81060, @yuja wrote: > > _('set branch name even if it shadows an existing branch')), > > ('C', 'clean', None, _('reset branch name to parent branch name')), > > ('r', 'rev', [], _('change branches of the given revs (EXPERIMENTAL)')), > > > > + ('s', 'show', None, _('show branch name of the given rev')) > > > > ], > > _('[-fC] [NAME]'), > > helpcategory=command.CATEGORY_CHANGE_ORGANIZATION) > > > > @@ -1097,6 +1098,11 @@ > > > > ui.write("%s\n" % repo.dirstate.branch()) > > return > > > > > > +elif opts.get('show') and label: > > +ctx = scmutil.revsingle(repo, label) > > +ui.write("%s\n" % ctx.branch()) > > +return > > How about adding the -r/--rev option to the branches command instead? > > I came to this idea while working on the issue4505, tag sorting. I haven't sent > the patch yet because I noticed we would also want to sort tags > lexicographically, which can't be achieved by the revset. But at least, tags, > branches, and bookmarks can be filtered by the -r/--rev option. > > https://bz.mercurial-scm.org/show_bug.cgi?id=4505 > > branch, tag, and bookmark (without -l) are basically the commands to mutate > the repository, and the -r option of the branch command is highly experimental. > IIRC, @pulkit regrets of adding `branch -r`. Yep I do. I don't think it's a good UI. It should either be part of commands like metaedit, or should be moved to a change-branch extension for now. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5475 To: navaneeth.suresh, #hg-reviewers Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5475: branch:Added option to show branch name of a given rev (Issue5948)
yuja added subscribers: pulkit, yuja. yuja added a comment. > _('set branch name even if it shadows an existing branch')), > ('C', 'clean', None, _('reset branch name to parent branch name')), > ('r', 'rev', [], _('change branches of the given revs (EXPERIMENTAL)')), > > + ('s', 'show', None, _('show branch name of the given rev')) > > ], > _('[-fC] [NAME]'), > helpcategory=command.CATEGORY_CHANGE_ORGANIZATION) > > @@ -1097,6 +1098,11 @@ > > ui.write("%s\n" % repo.dirstate.branch()) > return > > > +elif opts.get('show') and label: > +ctx = scmutil.revsingle(repo, label) > +ui.write("%s\n" % ctx.branch()) > +return How about adding the -r/--rev option to the branches command instead? I came to this idea while working on the issue4505, tag sorting. I haven't sent the patch yet because I noticed we would also want to sort tags lexicographically, which can't be achieved by the revset. But at least, tags, branches, and bookmarks can be filtered by the -r/--rev option. https://bz.mercurial-scm.org/show_bug.cgi?id=4505 branch, tag, and bookmark (without -l) are basically the commands to mutate the repository, and the -r option of the branch command is highly experimental. IIRC, @pulkit regrets of adding `branch -r`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5475 To: navaneeth.suresh, #hg-reviewers Cc: yuja, pulkit, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D5475: branch:Added option to show branch name of a given rev (Issue5948)
> _('set branch name even if it shadows an existing branch')), > ('C', 'clean', None, _('reset branch name to parent branch name')), > ('r', 'rev', [], _('change branches of the given revs (EXPERIMENTAL)')), > + ('s', 'show', None, _('show branch name of the given rev')) > ], > _('[-fC] [NAME]'), > helpcategory=command.CATEGORY_CHANGE_ORGANIZATION) > @@ -1097,6 +1098,11 @@ > ui.write("%s\n" % repo.dirstate.branch()) > return > > +elif opts.get('show') and label: > +ctx = scmutil.revsingle(repo, label) > +ui.write("%s\n" % ctx.branch()) > +return How about adding the -r/--rev option to the branches command instead? I came to this idea while working on the issue4505, tag sorting. I haven't sent the patch yet because I noticed we would also want to sort tags lexicographically, which can't be achieved by the revset. But at least, tags, branches, and bookmarks can be filtered by the -r/--rev option. https://bz.mercurial-scm.org/show_bug.cgi?id=4505 branch, tag, and bookmark (without -l) are basically the commands to mutate the repository, and the -r option of the branch command is highly experimental. IIRC, @pulkit regrets of adding `branch -r`. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH 1 of 6 V2] extensions: import the exthelper class from evolve 980565468003 (API)
On Sun, 23 Dec 2018 01:15:46 -0500, Matt Harbison wrote: > # HG changeset patch > # User Matt Harbison > # Date 1545530784 18000 > # Sat Dec 22 21:06:24 2018 -0500 > # Node ID ffab2010329f111c2237ebbb64be650a1b0301d8 > # Parent e9c606fef203621755d75b0434574a8a60ffd0ff > extensions: import the exthelper class from evolve 980565468003 (API) Queued with several pyflakes fixes, thanks. > +def wrapcommand(self, command, extension=None, opts=None): > +"""Decorated function is a command wrapper > + > +The name of the command must be given as the decorator argument. > +The wrapping is installed during `uisetup`. > + > +If the second option `extension` argument is provided, the wrapping > +will be applied in the extension commandtable. This argument must be > a > +string that will be searched using `extension.find` if not found and > +Abort error is raised. If the wrapping applies to an extension, it is > +installed during `extsetup`. > + > +example:: > + > +@eh.wrapcommand('summary') > +def wrapsummary(orig, ui, repo, *args, **kwargs): > +ui.note('Barry!') > +return orig(ui, repo, *args, **kwargs) > + > +The `opts` argument allows specifying additional arguments for the > +command. > + > +""" > +def dec(wrapper): > +if opts is None: > +opts = [] It shadows the outer 'opts' variable. Moved out of the dec function. > +def addattr(self, container, funcname): > +"""Decorated function is to be added to the container > + > +This function takes two arguments, the container and the name of the > +function to wrap. The wrapping is performed during `uisetup`. > + > +example:: > + > +@eh.function(context.changectx, 'babar') > +def babar(ctx): > +return 'babar' in ctx.description This example doesn't look nice since we discourage rewriting classes. https://www.mercurial-scm.org/wiki/WritingExtensions#Wrapping_methods_on_the_ui_and_repo_classes ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5476: merge: modify the logical statement
khanchi97 created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5476 AFFECTED FILES mercurial/merge.py CHANGE DETAILS diff --git a/mercurial/merge.py b/mercurial/merge.py --- a/mercurial/merge.py +++ b/mercurial/merge.py @@ -1538,8 +1538,8 @@ unresolvedcount = attr.ib() def isempty(self): -return (not self.updatedcount and not self.mergedcount -and not self.removedcount and not self.unresolvedcount) +return not (self.updatedcount or self.mergedcount +or self.removedcount or self.unresolvedcount) def emptyactions(): """create an actions dict, to be populated and passed to applyupdates()""" To: khanchi97, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5474: obsutil: fix the issue5686
khanchi97 updated this revision to Diff 12962. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D5474?vs=12956=12962 REVISION DETAIL https://phab.mercurial-scm.org/D5474 AFFECTED FILES mercurial/obsutil.py tests/test-obsmarker-template.t tests/test-obsolete.t CHANGE DETAILS diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t --- a/tests/test-obsolete.t +++ b/tests/test-obsolete.t @@ -935,38 +935,38 @@ $ rm access.log errors.log #endif -Several troubles on the same changeset (create an unstable and bumped changeset) +Several troubles on the same changeset (create an unstable and bumped and content-divergent changeset) $ hg debugobsolete `getid obsolete_e` obsoleted 1 changesets 2 new orphan changesets $ hg debugobsolete `getid original_c` `getid babar` 1 new phase-divergent changesets - $ hg log --config ui.logtemplate= -r 'phasedivergent() and orphan()' + 2 new content-divergent changesets + $ hg log --config ui.logtemplate= -r 'phasedivergent() and orphan() and contentdivergent()' changeset: 7:50c51b361e60 user:test date:Thu Jan 01 00:00:00 1970 + - instability: orphan, phase-divergent + instability: orphan, phase-divergent, content-divergent summary: add babar - test the "obsolete" templatekw $ hg log -r 'obsolete()' 6:3de5eca88c00 (draft *obsolete*) [ ] add obsolete_e [pruned] test the "troubles" templatekw $ hg log -r 'phasedivergent() and orphan()' - 7:50c51b361e60 (draft orphan phase-divergent) [ ] add babar + 7:50c51b361e60 (draft orphan phase-divergent content-divergent) [ ] add babar test the default cmdline template $ hg log -T default -r 'phasedivergent()' changeset: 7:50c51b361e60 user:test date:Thu Jan 01 00:00:00 1970 + - instability: orphan, phase-divergent + instability: orphan, phase-divergent, content-divergent summary: add babar $ hg log -T default -r 'obsolete()' @@ -981,18 +981,18 @@ test the obsolete labels $ hg log --config ui.logtemplate= --color=debug -r 'phasedivergent()' - [log.changeset changeset.draft changeset.unstable instability.orphan instability.phase-divergent|changeset: 7:50c51b361e60] + [log.changeset changeset.draft changeset.unstable instability.orphan instability.phase-divergent instability.content-divergent|changeset: 7:50c51b361e60] [log.user|user:test] [log.date|date:Thu Jan 01 00:00:00 1970 +] - [log.instability|instability: orphan, phase-divergent] + [log.instability|instability: orphan, phase-divergent, content-divergent] [log.summary|summary: add babar] $ hg log -T default -r 'phasedivergent()' --color=debug - [log.changeset changeset.draft changeset.unstable instability.orphan instability.phase-divergent|changeset: 7:50c51b361e60] + [log.changeset changeset.draft changeset.unstable instability.orphan instability.phase-divergent instability.content-divergent|changeset: 7:50c51b361e60] [log.user|user:test] [log.date|date:Thu Jan 01 00:00:00 1970 +] - [log.instability|instability: orphan, phase-divergent] + [log.instability|instability: orphan, phase-divergent, content-divergent] [log.summary|summary: add babar] @@ -1019,13 +1019,14 @@ $ hg up -r 'phasedivergent() and orphan()' 1 files updated, 0 files merged, 1 files removed, 0 files unresolved $ hg summary - parent: 7:50c51b361e60 (orphan, phase-divergent) + parent: 7:50c51b361e60 (orphan, phase-divergent, content-divergent) add babar branch: default commit: (clean) update: 2 new changesets (update) phases: 4 draft orphan: 2 changesets + content-divergent: 2 changesets phase-divergent: 1 changesets $ hg up -r 'obsolete()' 0 files updated, 0 files merged, 1 files removed, 0 files unresolved @@ -1037,22 +1038,26 @@ update: 3 new changesets (update) phases: 4 draft orphan: 2 changesets + content-divergent: 2 changesets phase-divergent: 1 changesets test debugwhyunstable output $ hg debugwhyunstable 50c51b361e60 orphan: obsolete parent 3de5eca88c00aa039da7399a220f4a5221faa585 phase-divergent: immutable predecessor 245bde4270cd1072a27757984f9cda8ba26f08ca + content-divergent: 6f96419950729f3671185b847352890f074f7557 (draft) predecessor 245bde4270cd1072a27757984f9cda8ba26f08ca test whyunstable template keyword $ hg log -r 50c51b361e60 -T '{whyunstable}\n' orphan: obsolete parent 3de5eca88c00 phase-divergent: immutable predecessor 245bde4270cd + content-divergent: 3:6f9641995072 (draft) predecessor 245bde4270cd $ hg log -r 50c51b361e60 -T '{whyunstable % "{instability}: {reason} {node|shortest}\n"}' orphan: obsolete parent 3de5 phase-divergent: immutable predecessor 245b + content-divergent: predecessor 245b #if serve @@ -1076,36 +1081,43 @@ check changeset with instabilities $ get-with-headers.py
D5474: obsutil: fix the issue5686
khanchi97 added a comment. @av6 Thanks for the review. I will update it. INLINE COMMENTS > av6 wrote in test-obsmarker-template.t:2511 > The new output looks somewhat more correct, but not quite. This changeset > definitely wasn't just pruned, but it also wasn't rewritten either: it was > split, and you can see correct `hg fatelog` output in the block below this > one. The difference between them is wdir parent is different and the fatelog > below has --hidden flag. > > So is it expected that this patch doesn't fully fix this particular issue? wdir parent is not making a difference here. If we pass `--hidden` here without updating the wdir parent, it will show the same result as block below. Maybe, this changed output is correct as it including those revision only which are visible to user. So when we pass `--hidden` that hidden cset is visible and it outputs that it was a split operation. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5474 To: khanchi97, #hg-reviewers Cc: av6, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D5475: branch:Added option to show branch name of a given rev (Issue5948)
navaneeth.suresh created this revision. Herald added a subscriber: mercurial-devel. Herald added a reviewer: hg-reviewers. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5475 AFFECTED FILES mercurial/commands.py CHANGE DETAILS diff --git a/mercurial/commands.py b/mercurial/commands.py --- a/mercurial/commands.py +++ b/mercurial/commands.py @@ -1054,6 +1054,7 @@ _('set branch name even if it shadows an existing branch')), ('C', 'clean', None, _('reset branch name to parent branch name')), ('r', 'rev', [], _('change branches of the given revs (EXPERIMENTAL)')), + ('s', 'show', None, _('show branch name of the given rev')) ], _('[-fC] [NAME]'), helpcategory=command.CATEGORY_CHANGE_ORGANIZATION) @@ -1097,6 +1098,11 @@ ui.write("%s\n" % repo.dirstate.branch()) return +elif opts.get('show') and label: +ctx = scmutil.revsingle(repo, label) +ui.write("%s\n" % ctx.branch()) +return + with repo.wlock(): if opts.get('clean'): label = repo[None].p1().branch() To: navaneeth.suresh, #hg-reviewers Cc: mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel