| Submitter | Jun Wu |
|---|---|
| Date | July 10, 2017, 6 p.m. |
| Message ID | <c839dcefb603c31236e6.1499709648@x1c> |
| Download | mbox | patch |
| Permalink | /patch/22211/ |
| State | Accepted |
| Headers | show |
Comments
On Mon, 10 Jul 2017 11:00:48 -0700, Jun Wu wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1499709400 25200 > # Mon Jul 10 10:56:40 2017 -0700 > # Node ID c839dcefb603c31236e6ed2c3d94fedfe4729d33 > # Parent 1e872b08a4e93b21e60cba695e4022bf71d4acf4 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r c839dcefb603 > revset: define successors revset Queued, thanks.
On Mon, 10 Jul 2017 14:00:48 -0400, Jun Wu <quark@fb.com> wrote: > # HG changeset patch > # User Jun Wu <quark@fb.com> > # Date 1499709400 25200 > # Mon Jul 10 10:56:40 2017 -0700 > # Node ID c839dcefb603c31236e6ed2c3d94fedfe4729d33 > # Parent 1e872b08a4e93b21e60cba695e4022bf71d4acf4 > # Available At https://bitbucket.org/quark-zju/hg-draft > # hg pull https://bitbucket.org/quark-zju/hg-draft -r > c839dcefb603 > revset: define successors revset > > This revset returns all successors, including transit nodes and the > source > nodes (to be consistent with existing revsets like "ancestors"). > > To filter out transit nodes, use `successors(X)-obsolete()`. > To filter out divergent case, use `successors(X)-divergent()-obsolete()`. > > The revset could be useful to define rebase destination, like: > `max(successors(BASE)-divergent()-obsolete())`. The `max` is to deal with > splits. > > There are other implementations where `successors` returns just one > level of > successors, and `allsuccessors` returns everything. I think `successors` > returning all successors by default is more user friendly. We have seen > cases in production where people use 1-level `successors` while they > really > want `allsuccessors`. So it seems better to just have one single revset > returning all successors by default to avoid user errors. > > In the future we might want to add `depth` keyword argument to it and for > other revsets like `ancestors` etc. Or even build some flexible indexing > syntax [1] to satisfy people having the depth limit requirement. > > [1]: > https://www.mercurial-scm.org/pipermail/mercurial-devel/2017-July/101140.html This will be more of an issue with precursors(), which I assume will be defined the same way for consistency. I tend to use the following to make sure that conflicts were resolved properly after rebasing: $ hg extdiff --patch --hidden -r "precursors($rev)" -r "$rev" It can be especially handy in a loop when a series is rebased and several commits need to be reviewed. Depth=1 probably solves that. I only mention it to provide an actual use case. Hopefully this remains working one way or the other in 4.3.
Excerpts from Matt Harbison's message of 2017-07-12 00:25:47 -0400: > This will be more of an issue with precursors(), which I assume will be > defined the same way for consistency. I tend to use the following to make > sure that conflicts were resolved properly after rebasing: > > $ hg extdiff --patch --hidden -r "precursors($rev)" -r "$rev" > > It can be especially handy in a loop when a series is rebased and several > commits need to be reviewed. Depth=1 probably solves that. I only > mention it to provide an actual use case. Hopefully this remains working > one way or the other in 4.3. Yes, I was aware of the use-case. We also have internal users wanting to compare with a previous version. Note: we have decided to use "predecessor" instead of "precursors" [1]. A "predecessors" revset is planned. However, I think the current obsstore API need some change to support depth better. Since I'm also planning on new (hash-preserving) obsstore format and API, I don't want to spend too much time on things only working for the format obsstore. The successors revset is mainly to be used together with my rebase work, where you can use: hg rebase -s 'unstable()-obsolete()' -d 'max(successors(max(roots(ALLSRC) & ::SRC)^)-obsolete()-divergent())' to basically achieve what "evolve --all" can do today, even with more flexibility like you can move the destination approximately to a head: hg rebase -s 'unstable()-obsolete()' -d 'max((successors(max(roots(ALLSRC) & ::SRC)^)-obsolete()-divergent())::)' (I'd wish that feature to be available in 4.3, but that's just a wish) Predecessor revset is not needed for that so it's less prioritized. [1]: https://www.mercurial-scm.org/wiki/CEDVocabulary
Excerpts from Jun Wu's message of 2017-07-11 21:54:36 -0700: > Excerpts from Matt Harbison's message of 2017-07-12 00:25:47 -0400: > > This will be more of an issue with precursors(), which I assume will be > > defined the same way for consistency. I tend to use the following to make > > sure that conflicts were resolved properly after rebasing: > > > > $ hg extdiff --patch --hidden -r "precursors($rev)" -r "$rev" > > > > It can be especially handy in a loop when a series is rebased and several > > commits need to be reviewed. Depth=1 probably solves that. I only > > mention it to provide an actual use case. Hopefully this remains working > > one way or the other in 4.3. > > Yes, I was aware of the use-case. We also have internal users wanting to > compare with a previous version. > > Note: we have decided to use "predecessor" instead of "precursors" [1]. > A "predecessors" revset is planned. However, I think the current obsstore > API need some change to support depth better. Since I'm also planning on new > (hash-preserving) obsstore format and API, I don't want to spend too much > time on things only working for the format obsstore. correction: s/format/current/ > > The successors revset is mainly to be used together with my rebase work, > where you can use: > > hg rebase -s 'unstable()-obsolete()' -d 'max(successors(max(roots(ALLSRC) & ::SRC)^)-obsolete()-divergent())' correction: s/rebase -s/rebase -r/, so does the command below. > to basically achieve what "evolve --all" can do today, even with more > flexibility like you can move the destination approximately to a head: > > hg rebase -s 'unstable()-obsolete()' -d 'max((successors(max(roots(ALLSRC) & ::SRC)^)-obsolete()-divergent())::)' > > (I'd wish that feature to be available in 4.3, but that's just a wish) > > Predecessor revset is not needed for that so it's less prioritized. > > [1]: https://www.mercurial-scm.org/wiki/CEDVocabulary
Patch
diff --git a/mercurial/revset.py b/mercurial/revset.py --- a/mercurial/revset.py +++ b/mercurial/revset.py @@ -20,4 +20,5 @@ from . import ( node, obsolete as obsmod, + obsutil, pathutil, phases, @@ -1827,4 +1828,26 @@ def subrepo(repo, subset, x): return subset.filter(matches, condrepr=('<subrepo %r>', pat)) +def _mapbynodefunc(repo, s, f): + """(repo, smartset, [node] -> [node]) -> smartset + + Helper method to map a smartset to another smartset given a function only + talking about nodes. Handles converting between rev numbers and nodes, and + filtering. + """ + cl = repo.unfiltered().changelog + torev = cl.rev + tonode = cl.node + nodemap = cl.nodemap + result = set(torev(n) for n in f(tonode(r) for r in s) if n in nodemap) + return smartset.baseset(result - repo.changelog.filteredrevs) + +@predicate('successors(set)', safe=True) +def successors(repo, subset, x): + """All successors for set, including the given set themselves""" + s = getset(repo, fullreposet(repo), x) + f = lambda nodes: obsutil.allsuccessors(repo.obsstore, nodes) + d = _mapbynodefunc(repo, s, f) + return subset & d + def _substringmatcher(pattern, casesensitive=True): kind, pattern, matcher = util.stringmatcher(pattern, diff --git a/tests/test-revset.t b/tests/test-revset.t --- a/tests/test-revset.t +++ b/tests/test-revset.t @@ -21,4 +21,5 @@ $ cat >> $HGRCPATH << EOF > [extensions] + > drawdag=$TESTDIR/drawdag.py > testrevset=$TESTTMP/testrevset.py > EOF @@ -4284,2 +4285,55 @@ Test repo.anyrevs with customized revset $ cd .. + +Test obsstore related revsets + + $ hg init repo1 + $ cd repo1 + $ cat <<EOF >> .hg/hgrc + > [experimental] + > evolution = createmarkers + > EOF + + $ hg debugdrawdag <<'EOS' + > F G + > |/ # split: B -> E, F + > B C D E # amend: B -> C -> D + > \|/ | # amend: F -> G + > A A Z # amend: A -> Z + > EOS + + $ hg log -r 'successors(Z)' -T '{desc}\n' + Z + + $ hg log -r 'successors(F)' -T '{desc}\n' + F + G + + $ hg tag --remove --local C D E F G + + $ hg log -r 'successors(B)' -T '{desc}\n' + B + D + E + G + + $ hg log -r 'successors(B)' -T '{desc}\n' --hidden + B + C + D + E + F + G + + $ hg log -r 'successors(B)-obsolete()' -T '{desc}\n' --hidden + D + E + G + + $ hg log -r 'successors(B+A)-divergent()' -T '{desc}\n' + A + Z + B + + $ hg log -r 'successors(B+A)-divergent()-obsolete()' -T '{desc}\n' + Z