Patchwork [4,of,4] rebase: exclude obsoletes without a successor in destination (issue5300)

# HG changeset patch
# User Denis Laxalde <denis@laxalde.org>
# Date 1510489041 -3600
#      Sun Nov 12 13:17:21 2017 +0100
# Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
# Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
# EXP-Topic rebase-obsolete
rebase: exclude obsoletes without a successor in destination (issue5300)

In the following example, when trying to rebase 3:: onto 2, the rebase
will abort with "this rebase will cause divergence from: 4":

    o  7 f
    |
    | o  6 e
    | |
    | o  5 d'
    | |
    x |  4 d (rewritten as 5)
    |/
    o  3 c
    |
    | o  2 x
    | |
    o |  1 b
    |/
    o  0 a

By excluding obsolete changesets without a successor in destination (4
in the example above) and their descendants, we make rebase work in this
case, thus giving:

    o  11 e
    |
    o  10 d'
    |
    o  9 c
    |
    o  8 b
    |
    | o  7 f
    | |
    | | x  6 e (rewritten using rebase as 11)
    | | |
    | | x  5 d' (rewritten using rebase as 10)
    | | |
    | x |  4 d
    | |/
    | x  3 c (rewritten using rebase as 9)
    | |
    o |  2 x
    | |
    | x  1 b (rewritten using rebase as 8)
    |/
    o  0 a

where branch 4:: is left behind while branch 5:: is rebased as expected.

The rationale is that users may not be interested in rebasing orphan
changesets when specifying a rebase set that include them but would
still want "stable" ones to be rebased. Currently, the user is suggested
to allow divergence (but probably does not want it) or they must specify
a rebase set excluding problematic changesets (which might be a bit
cumbersome). The approach proposed here corresponds to "Option 2" in
https://www.mercurial-scm.org/wiki/CEDRebase.

.. feature::

   Let 'hg rebase' avoid content-divergence when obsolete changesets are
   present in the rebase set by skipping possible sources of divergence.


We extend _computeobsoletenotrebased() so that it also return a set of
obsolete changesets in rebaseset without a successor in destination.
This 'obsoletewithoutsuccessorindestination' is then stored as an
attribute of rebaseruntime and used in _performrebasesubset() to:

* filter out descendants of these changesets from the revisions to
  rebase;
* issue a message about these revisions being skipped.

This only occurs if 'evolution.allowdivergence' option is off and
'rebaseskipobsolete' is on.

On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
> # HG changeset patch
> # User Denis Laxalde <denis@laxalde.org>
> # Date 1510489041 -3600
> #      Sun Nov 12 13:17:21 2017 +0100
> # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
> # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
> # EXP-Topic rebase-obsolete
> rebase: exclude obsoletes without a successor in destination (issue5300)

I took patches 1-3. I think this is right. I'm 90% sure this is
right. But I'm a little tired, and so I want Martin's more careful eye
on it if he has time.

>
> In the following example, when trying to rebase 3:: onto 2, the rebase
> will abort with "this rebase will cause divergence from: 4":
>
>     o  7 f
>     |
>     | o  6 e
>     | |
>     | o  5 d'
>     | |
>     x |  4 d (rewritten as 5)
>     |/
>     o  3 c
>     |
>     | o  2 x
>     | |
>     o |  1 b
>     |/
>     o  0 a
>
> By excluding obsolete changesets without a successor in destination (4
> in the example above) and their descendants, we make rebase work in this
> case, thus giving:
>
>     o  11 e
>     |
>     o  10 d'
>     |
>     o  9 c
>     |
>     o  8 b
>     |
>     | o  7 f
>     | |
>     | | x  6 e (rewritten using rebase as 11)
>     | | |
>     | | x  5 d' (rewritten using rebase as 10)
>     | | |
>     | x |  4 d
>     | |/
>     | x  3 c (rewritten using rebase as 9)
>     | |
>     o |  2 x
>     | |
>     | x  1 b (rewritten using rebase as 8)
>     |/
>     o  0 a
>
> where branch 4:: is left behind while branch 5:: is rebased as expected.
>
> The rationale is that users may not be interested in rebasing orphan
> changesets when specifying a rebase set that include them but would
> still want "stable" ones to be rebased. Currently, the user is suggested
> to allow divergence (but probably does not want it) or they must specify
> a rebase set excluding problematic changesets (which might be a bit
> cumbersome). The approach proposed here corresponds to "Option 2" in
> https://www.mercurial-scm.org/wiki/CEDRebase.
>
> .. feature::
>
>    Let 'hg rebase' avoid content-divergence when obsolete changesets are
>    present in the rebase set by skipping possible sources of divergence.
>
>
> We extend _computeobsoletenotrebased() so that it also return a set of
> obsolete changesets in rebaseset without a successor in destination.
> This 'obsoletewithoutsuccessorindestination' is then stored as an
> attribute of rebaseruntime and used in _performrebasesubset() to:
>
> * filter out descendants of these changesets from the revisions to
>   rebase;
> * issue a message about these revisions being skipped.
>
> This only occurs if 'evolution.allowdivergence' option is off and
> 'rebaseskipobsolete' is on.
>
> diff --git a/hgext/rebase.py b/hgext/rebase.py
> --- a/hgext/rebase.py
> +++ b/hgext/rebase.py
> @@ -179,6 +179,7 @@ class rebaseruntime(object):
>          # other extensions
>          self.keepopen = opts.get('keepopen', False)
>          self.obsoletenotrebased = {}
> +        self.obsoletewithoutsuccessorindestination = set()
>
>      @property
>      def repo(self):
> @@ -311,9 +312,10 @@ class rebaseruntime(object):
>          if not self.ui.configbool('experimental', 'rebaseskipobsolete'):
>              return
>          obsoleteset = set(obsoleterevs)
> -        self.obsoletenotrebased = _computeobsoletenotrebased(self.repo,
> -                                    obsoleteset, destmap)
> +        self.obsoletenotrebased, self.obsoletewithoutsuccessorindestination = \
> +            _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
>          skippedset = set(self.obsoletenotrebased)
> +        skippedset.update(self.obsoletewithoutsuccessorindestination)
>          _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
>
>      def _prepareabortorcontinue(self, isabort):
> @@ -419,12 +421,26 @@ class rebaseruntime(object):
>      def _performrebasesubset(self, tr, subset, pos, total):
>          repo, ui, opts = self.repo, self.ui, self.opts
>          sortedrevs = repo.revs('sort(%ld, -topo)', subset)
> +        allowdivergence = self.ui.configbool(
> +            'experimental', 'evolution.allowdivergence')
> +        if not allowdivergence:
> +            sortedrevs -= repo.revs(
> +                'descendants(%ld) and not %ld',
> +                self.obsoletewithoutsuccessorindestination,
> +                self.obsoletewithoutsuccessorindestination,
> +            )
>          for rev in sortedrevs:
>              dest = self.destmap[rev]
>              ctx = repo[rev]
>              desc = _ctxdesc(ctx)
>              if self.state[rev] == rev:
>                  ui.status(_('already rebased %s\n') % desc)
> +            elif (not allowdivergence
> +                  and rev in self.obsoletewithoutsuccessorindestination):
> +                msg = _('note: not rebasing %s and its descendants as '
> +                        'this would cause divergences\n') % desc
> +                repo.ui.status(msg)
> +                self.skipped.add(rev)
>              elif rev in self.obsoletenotrebased:
>                  succ = self.obsoletenotrebased[rev]
>                  if succ is None:
> @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
>      return set(r for r in revs if repo[r].obsolete())
>
>  def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
> -    """return a mapping obsolete => successor for all obsolete nodes to be
> -    rebased that have a successors in the destination
> +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindestination).
> +
> +    `obsoletenotrebased` is a mapping mapping obsolete => successor for all
> +    obsolete nodes to be rebased given in `rebaseobsrevs`.
>
> -    obsolete => None entries in the mapping indicate nodes with no successor"""
> +    `obsoletewithoutsuccessorindestination` is a set with obsolete revisions
> +    without a successor in destination.
> +    """
>      obsoletenotrebased = {}
> +    obsoletewithoutsuccessorindestination = set([])
>
>      assert repo.filtername is None
>      cl = repo.changelog
> @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
>                  if cl.isancestor(succnode, destnode):
>                      obsoletenotrebased[srcrev] = nodemap[succnode]
>                      break
> +            else:
> +                # Unless 'srcrev' only has one successor and it's not in
> +                # rebaseset, we might skip it and its descendants as it has no
> +                # successor in destination. Otherwise, we'll issue the
> +                # divergence warning and abort.
> +                if (len(successors) > 1
> +                        and any(nodemap[s] in destmap
> +                                for s in successors if s != srcnode)):
> +                    obsoletewithoutsuccessorindestination.add(srcrev)
>
> -    return obsoletenotrebased
> +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
>
>  def summaryhook(ui, repo):
>      if not repo.vfs.exists('rebasestate'):
> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> --- a/tests/test-rebase-obsolete.t
> +++ b/tests/test-rebase-obsolete.t
> @@ -987,6 +987,208 @@ Create the changes that we will rebase
>    rebasing 21:7bdc8a87673d "dummy change" (tip)
>    $ cd ..
>
> +Divergence cases due to obsolete changesets
> +-------------------------------------------
> +
> +We should ignore branches with unstable changesets when they are based on an
> +obsolete changeset which successor is in rebase set.
> +
> +  $ hg init divergence
> +  $ cd divergence
> +  $ cat >> .hg/hgrc << EOF
> +  > [extensions]
> +  > strip =
> +  > [alias]
> +  > strip = strip --no-backup --quiet
> +  > [templates]
> +  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilities," ({instabilities})")}\n'
> +  > EOF
> +
> +  $ hg debugdrawdag <<EOF
> +  >   e   f
> +  >   |   |
> +  >   d'  d # replace: d -> d'
> +  >    \ /
> +  >     c
> +  >     |
> +  >   x b
> +  >    \|
> +  >     a
> +  > EOF
> +  $ hg log -G -r 'a'::
> +  o  7:1143e9adc121 f
> +  |
> +  | o  6:d60ebfa0f1cb e
> +  | |
> +  | o  5:027ad6c5830d d'
> +  | |
> +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
> +  |/
> +  o  3:a82ac2b38757 c
> +  |
> +  | o  2:630d7c95eff7 x
> +  | |
> +  o |  1:488e1b7e7341 b
> +  |/
> +  o  0:b173517d0057 a
> +
> +
> +Changeset d and its descendants are excluded to avoid divergence of d, which
> +would occur because the successor of d (d') is also in rebaseset. As a
> +consequence f (descendant of d) is left behind.
> +
> +  $ hg rebase -b 'e' -d 'x'
> +  rebasing 1:488e1b7e7341 "b" (b)
> +  rebasing 3:a82ac2b38757 "c" (c)
> +  rebasing 5:027ad6c5830d "d'" (d')
> +  rebasing 6:d60ebfa0f1cb "e" (e)
> +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this would cause divergences
> +  $ hg log -G -r 'a'::
> +  o  11:eb6d63fc4ed5 e
> +  |
> +  o  10:44d8c724a70c d'
> +  |
> +  o  9:d008e6b4d3fd c
> +  |
> +  o  8:67e8f4a16c49 b
> +  |
> +  | o  7:1143e9adc121 f
> +  | |
> +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
> +  | | |
> +  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
> +  | | |
> +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
> +  | |/
> +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
> +  | |
> +  o |  2:630d7c95eff7 x
> +  | |
> +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
> +  |/
> +  o  0:b173517d0057 a
> +
> +  $ hg strip -r 8:
> +
> +If the rebase set has an obsolete (d) with a unique successor (d') outside the
> +rebase set and not in destination, we still get the divergence warning. By
> +allowing divergence, we can perform the rebase.
> +
> +  $ hg rebase -r 'c'::'f' -d 'x'
> +  abort: this rebase will cause divergences from: 76be324c128b
> +  (to force the rebase please set experimental.evolution.allowdivergence=True)
> +  [255]
> +  $ hg rebase --config experimental.evolution.allowdivergence=true -r 'c'::'f' -d 'x'
> +  rebasing 3:a82ac2b38757 "c" (c)
> +  rebasing 4:76be324c128b "d" (d)
> +  rebasing 7:1143e9adc121 "f" (f tip)
> +  $ hg log -G -r 'a':: -T instabilities
> +  o  10:e1744ea07510 f
> +  |
> +  o  9:e2b36ea9a0a0 d (content-divergent)
> +  |
> +  o  8:6a0376de376e c
> +  |
> +  | x  7:1143e9adc121 f
> +  | |
> +  | | o  6:d60ebfa0f1cb e (orphan)
> +  | | |
> +  | | o  5:027ad6c5830d d' (orphan content-divergent)
> +  | | |
> +  | x |  4:76be324c128b d
> +  | |/
> +  | x  3:a82ac2b38757 c
> +  | |
> +  o |  2:630d7c95eff7 x
> +  | |
> +  | o  1:488e1b7e7341 b
> +  |/
> +  o  0:b173517d0057 a
> +
> +  $ hg strip -r 8:
> +
> +(Not skipping obsoletes means that divergence is allowed.)
> +
> +  $ hg rebase --config experimental.rebaseskipobsolete=false -r 'c'::'f' -d 'x'
> +  rebasing 3:a82ac2b38757 "c" (c)
> +  rebasing 4:76be324c128b "d" (d)
> +  rebasing 7:1143e9adc121 "f" (f tip)
> +
> +  $ hg strip -r 0:
> +
> +Similar test on a more complex graph
> +
> +  $ hg debugdrawdag <<EOF
> +  >       g
> +  >       |
> +  >   f   e
> +  >   |   |
> +  >   e'  d # replace: e -> e'
> +  >    \ /
> +  >     c
> +  >     |
> +  >   x b
> +  >    \|
> +  >     a
> +  > EOF
> +  $ hg log -G -r 'a':
> +  o  8:2876ce66c6eb g
> +  |
> +  | o  7:3ffec603ab53 f
> +  | |
> +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
> +  | |
> +  | o  5:63324dc512ea e'
> +  | |
> +  o |  4:76be324c128b d
> +  |/
> +  o  3:a82ac2b38757 c
> +  |
> +  | o  2:630d7c95eff7 x
> +  | |
> +  o |  1:488e1b7e7341 b
> +  |/
> +  o  0:b173517d0057 a
> +
> +  $ hg rebase -b 'f' -d 'x'
> +  rebasing 1:488e1b7e7341 "b" (b)
> +  rebasing 3:a82ac2b38757 "c" (c)
> +  rebasing 5:63324dc512ea "e'" (e')
> +  rebasing 7:3ffec603ab53 "f" (f)
> +  rebasing 4:76be324c128b "d" (d)
> +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this would cause divergences
> +  $ hg log -G -r 'a':
> +  o  13:a1707a5b7c2c d
> +  |
> +  | o  12:ef6251596616 f
> +  | |
> +  | o  11:b6f172e64af9 e'
> +  |/
> +  o  10:d008e6b4d3fd c
> +  |
> +  o  9:67e8f4a16c49 b
> +  |
> +  | o  8:2876ce66c6eb g
> +  | |
> +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
> +  | | |
> +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
> +  | | |
> +  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
> +  | | |
> +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
> +  | |/
> +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
> +  | |
> +  o |  2:630d7c95eff7 x
> +  | |
> +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
> +  |/
> +  o  0:b173517d0057 a
> +
> +
> +  $ cd ..
> +
>  Rebase merge where successor of one parent is equal to destination (issue5198)
>
>    $ hg init p1-succ-is-dest
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Mon, Nov 13, 2017 at 3:07 PM, Augie Fackler <raf@durin42.com> wrote:

> On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
> > # HG changeset patch
> > # User Denis Laxalde <denis@laxalde.org>
> > # Date 1510489041 -3600
> > #      Sun Nov 12 13:17:21 2017 +0100
> > # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
> > # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
> > # EXP-Topic rebase-obsolete
> > rebase: exclude obsoletes without a successor in destination (issue5300)
>

If I read it right, this line should be something like "rebase: exclude
*descendants of* obsoletes without..."


>
> I took patches 1-3. I think this is right. I'm 90% sure this is
> right. But I'm a little tired, and so I want Martin's more careful eye
> on it if he has time.
>

Thanks for asking. I think this makes sense. Thanks for working on this,
Denis!

I think a general thing to note is that if a commit in the rebase set has
become obsolete, then there's a conflict between the rebase invocation and
the obsmarkers. The rebase command tells us to move the descendants onto
the rebase destination, while the obsmarkers tell us to move them onto the
obsolete commit's successor. Since we don't keep track of some default
rebase destination (like git's upstream concept), any "hg rebase"
invocation is a one-off as far as hg is concerned, so I think it makes
sense to resolve the conflict in favor of the obsmarkers. That probably
didn't make much sense, so let me give an example:

o B'
|
o O
|
| o U
| |
| | o C
| | |
| | x B
| |/
| .
|/
o A

Let's say C's upstream was somehow configured to be U here, and that B had
been rebased onto O instead. If the user then runs "hg evolve -r C", you
could argue that there would be a similar conflict between rebase/upstream
that want to move C onto U and obsmarkers that want to move C onto B'.
However, since we don't have that feature, B' is the only natural
destination.


> >
> > In the following example, when trying to rebase 3:: onto 2, the rebase
> > will abort with "this rebase will cause divergence from: 4":
> >
> >     o  7 f
> >     |
> >     | o  6 e
> >     | |
> >     | o  5 d'
> >     | |
> >     x |  4 d (rewritten as 5)
> >     |/
> >     o  3 c
> >     |
> >     | o  2 x
> >     | |
> >     o |  1 b
> >     |/
> >     o  0 a
> >
> > By excluding obsolete changesets without a successor in destination (4
> > in the example above) and their descendants, we make rebase work in this
> > case, thus giving:
> >
> >     o  11 e
> >     |
> >     o  10 d'
> >     |
> >     o  9 c
> >     |
> >     o  8 b
> >     |
> >     | o  7 f
> >     | |
> >     | | x  6 e (rewritten using rebase as 11)
> >     | | |
> >     | | x  5 d' (rewritten using rebase as 10)
> >     | | |
> >     | x |  4 d
> >     | |/
> >     | x  3 c (rewritten using rebase as 9)
> >     | |
> >     o |  2 x
> >     | |
> >     | x  1 b (rewritten using rebase as 8)
> >     |/
> >     o  0 a
> >
> > where branch 4:: is left behind while branch 5:: is rebased as expected.
> >
> > The rationale is that users may not be interested in rebasing orphan
> > changesets when specifying a rebase set that include them but would
> > still want "stable" ones to be rebased. Currently, the user is suggested
> > to allow divergence (but probably does not want it) or they must specify
> > a rebase set excluding problematic changesets (which might be a bit
> > cumbersome). The approach proposed here corresponds to "Option 2" in
> > https://www.mercurial-scm.org/wiki/CEDRebase.
>

I think I'd still want "Option 3", but I don't think this patch prevents
that from happening later.


> >
> > .. feature::
> >
> >    Let 'hg rebase' avoid content-divergence when obsolete changesets are
> >    present in the rebase set by skipping possible sources of divergence.
>

Similar to what I said above, I think it would help to mention descendants
here. While "possible sources of divergence" is vague enough to be correct,
it would be helpful if we could clarify it.

>
> >
>

Just to make sure, the stuff below is not part of the release notes, is it?
I'm not familiar with the syntax, so I can't tell (and I'm too lazy to test
it).

> We extend _computeobsoletenotrebased() so that it also return a set of
> > obsolete changesets in rebaseset without a successor in destination.
> > This 'obsoletewithoutsuccessorindestination' is then stored as an
> > attribute of rebaseruntime and used in _performrebasesubset() to:
> >
> > * filter out descendants of these changesets from the revisions to
> >   rebase;
> > * issue a message about these revisions being skipped.
> >
> > This only occurs if 'evolution.allowdivergence' option is off and
> > 'rebaseskipobsolete' is on.
> >
> > diff --git a/hgext/rebase.py b/hgext/rebase.py
> > --- a/hgext/rebase.py
> > +++ b/hgext/rebase.py
> > @@ -179,6 +179,7 @@ class rebaseruntime(object):
> >          # other extensions
> >          self.keepopen = opts.get('keepopen', False)
> >          self.obsoletenotrebased = {}
> > +        self.obsoletewithoutsuccessorindestination = set()
> >
> >      @property
> >      def repo(self):
> > @@ -311,9 +312,10 @@ class rebaseruntime(object):
> >          if not self.ui.configbool('experimental',
> 'rebaseskipobsolete'):
> >              return
> >          obsoleteset = set(obsoleterevs)
> > -        self.obsoletenotrebased = _computeobsoletenotrebased(self.repo,
> > -                                    obsoleteset, destmap)
> > +        self.obsoletenotrebased, self.obsoletewithoutsuccessorindestination
> = \
> > +            _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
> >          skippedset = set(self.obsoletenotrebased)
> > +        skippedset.update(self.obsoletewithoutsuccessorindestination)
> >          _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
> >
> >      def _prepareabortorcontinue(self, isabort):
> > @@ -419,12 +421,26 @@ class rebaseruntime(object):
> >      def _performrebasesubset(self, tr, subset, pos, total):
> >          repo, ui, opts = self.repo, self.ui, self.opts
> >          sortedrevs = repo.revs('sort(%ld, -topo)', subset)
> > +        allowdivergence = self.ui.configbool(
> > +            'experimental', 'evolution.allowdivergence')
> > +        if not allowdivergence:
> > +            sortedrevs -= repo.revs(
> > +                'descendants(%ld) and not %ld',
> > +                self.obsoletewithoutsuccessorindestination,
> > +                self.obsoletewithoutsuccessorindestination,
> > +            )
> >          for rev in sortedrevs:
> >              dest = self.destmap[rev]
> >              ctx = repo[rev]
> >              desc = _ctxdesc(ctx)
> >              if self.state[rev] == rev:
> >                  ui.status(_('already rebased %s\n') % desc)
> > +            elif (not allowdivergence
> > +                  and rev in self.obsoletewithoutsuccessori
> ndestination):
> > +                msg = _('note: not rebasing %s and its descendants as '
> > +                        'this would cause divergences\n') % desc
> > +                repo.ui.status(msg)
> > +                self.skipped.add(rev)
> >              elif rev in self.obsoletenotrebased:
> >                  succ = self.obsoletenotrebased[rev]
> >                  if succ is None:
> > @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
> >      return set(r for r in revs if repo[r].obsolete())
> >
> >  def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
> > -    """return a mapping obsolete => successor for all obsolete nodes to
> be
> > -    rebased that have a successors in the destination
> > +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindest
> ination).
> > +
> > +    `obsoletenotrebased` is a mapping mapping obsolete => successor for
> all
> > +    obsolete nodes to be rebased given in `rebaseobsrevs`.
> >
> > -    obsolete => None entries in the mapping indicate nodes with no
> successor"""
> > +    `obsoletewithoutsuccessorindestination` is a set with obsolete
> revisions
> > +    without a successor in destination.
> > +    """
> >      obsoletenotrebased = {}
> > +    obsoletewithoutsuccessorindestination = set([])
> >
> >      assert repo.filtername is None
> >      cl = repo.changelog
> > @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
> >                  if cl.isancestor(succnode, destnode):
> >                      obsoletenotrebased[srcrev] = nodemap[succnode]
> >                      break
> > +            else:
> > +                # Unless 'srcrev' only has one successor and it's not in
> > +                # rebaseset, we might skip it and its descendants as it
> has no
> > +                # successor in destination. Otherwise, we'll issue the
> > +                # divergence warning and abort.
> > +                if (len(successors) > 1
>

This line doesn't seem necessary, but feel free to keep if you think it
helps readability (I find it slightly distracting).


>
> +                        and any(nodemap[s] in destmap
> > +                                for s in successors if s != srcnode)):
> > +                    obsoletewithoutsuccessorindestination.add(srcrev)
> >
> > -    return obsoletenotrebased
> > +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
> >
> >  def summaryhook(ui, repo):
> >      if not repo.vfs.exists('rebasestate'):
> > diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
> > --- a/tests/test-rebase-obsolete.t
> > +++ b/tests/test-rebase-obsolete.t
> > @@ -987,6 +987,208 @@ Create the changes that we will rebase
> >    rebasing 21:7bdc8a87673d "dummy change" (tip)
> >    $ cd ..
> >
> > +Divergence cases due to obsolete changesets
> > +-------------------------------------------
> > +
> > +We should ignore branches with unstable changesets when they are based
> on an
> > +obsolete changeset which successor is in rebase set.
> > +
> > +  $ hg init divergence
> > +  $ cd divergence
> > +  $ cat >> .hg/hgrc << EOF
> > +  > [extensions]
> > +  > strip =
> > +  > [alias]
> > +  > strip = strip --no-backup --quiet
> > +  > [templates]
> > +  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilities,"
> ({instabilities})")}\n'
> > +  > EOF
> > +
> > +  $ hg debugdrawdag <<EOF
> > +  >   e   f
> > +  >   |   |
> > +  >   d'  d # replace: d -> d'
> > +  >    \ /
> > +  >     c
> > +  >     |
> > +  >   x b
> > +  >    \|
> > +  >     a
> > +  > EOF
> > +  $ hg log -G -r 'a'::
> > +  o  7:1143e9adc121 f
> > +  |
> > +  | o  6:d60ebfa0f1cb e
> > +  | |
> > +  | o  5:027ad6c5830d d'
> > +  | |
> > +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
> > +  |/
> > +  o  3:a82ac2b38757 c
> > +  |
> > +  | o  2:630d7c95eff7 x
> > +  | |
> > +  o |  1:488e1b7e7341 b
> > +  |/
> > +  o  0:b173517d0057 a
> > +
> > +
> > +Changeset d and its descendants are excluded to avoid divergence of d,
> which
> > +would occur because the successor of d (d') is also in rebaseset. As a
> > +consequence f (descendant of d) is left behind.
> > +
> > +  $ hg rebase -b 'e' -d 'x'
> > +  rebasing 1:488e1b7e7341 "b" (b)
> > +  rebasing 3:a82ac2b38757 "c" (c)
> > +  rebasing 5:027ad6c5830d "d'" (d')
> > +  rebasing 6:d60ebfa0f1cb "e" (e)
> > +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this
> would cause divergences
>

The plural "divergences" sounds wrong to me, but I'm not sure. Augie (or
others), since you're a native English speaker, do you think it should be
singular or plural?


> > +  $ hg log -G -r 'a'::
> > +  o  11:eb6d63fc4ed5 e
> > +  |
> > +  o  10:44d8c724a70c d'
> > +  |
> > +  o  9:d008e6b4d3fd c
> > +  |
> > +  o  8:67e8f4a16c49 b
> > +  |
> > +  | o  7:1143e9adc121 f
> > +  | |
> > +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
> > +  | | |
> > +  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
> > +  | | |
> > +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
> > +  | |/
> > +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
> > +  | |
> > +  o |  2:630d7c95eff7 x
> > +  | |
> > +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
> > +  |/
> > +  o  0:b173517d0057 a
> > +
> > +  $ hg strip -r 8:
> > +
> > +If the rebase set has an obsolete (d) with a unique successor (d')
> outside the
> > +rebase set and not in destination, we still get the divergence warning.
> By
> > +allowing divergence, we can perform the rebase.
> > +
> > +  $ hg rebase -r 'c'::'f' -d 'x'
> > +  abort: this rebase will cause divergences from: 76be324c128b
> > +  (to force the rebase please set experimental.evolution.allowdi
> vergence=True)
> > +  [255]
> > +  $ hg rebase --config experimental.evolution.allowdivergence=true -r
> 'c'::'f' -d 'x'
> > +  rebasing 3:a82ac2b38757 "c" (c)
> > +  rebasing 4:76be324c128b "d" (d)
> > +  rebasing 7:1143e9adc121 "f" (f tip)
> > +  $ hg log -G -r 'a':: -T instabilities
> > +  o  10:e1744ea07510 f
> > +  |
> > +  o  9:e2b36ea9a0a0 d (content-divergent)
> > +  |
> > +  o  8:6a0376de376e c
> > +  |
> > +  | x  7:1143e9adc121 f
> > +  | |
> > +  | | o  6:d60ebfa0f1cb e (orphan)
> > +  | | |
> > +  | | o  5:027ad6c5830d d' (orphan content-divergent)
> > +  | | |
> > +  | x |  4:76be324c128b d
> > +  | |/
> > +  | x  3:a82ac2b38757 c
> > +  | |
> > +  o |  2:630d7c95eff7 x
> > +  | |
> > +  | o  1:488e1b7e7341 b
> > +  |/
> > +  o  0:b173517d0057 a
> > +
> > +  $ hg strip -r 8:
> > +
> > +(Not skipping obsoletes means that divergence is allowed.)
> > +
> > +  $ hg rebase --config experimental.rebaseskipobsolete=false -r
> 'c'::'f' -d 'x'
> > +  rebasing 3:a82ac2b38757 "c" (c)
> > +  rebasing 4:76be324c128b "d" (d)
> > +  rebasing 7:1143e9adc121 "f" (f tip)
> > +
> > +  $ hg strip -r 0:
> > +
> > +Similar test on a more complex graph
> > +
> > +  $ hg debugdrawdag <<EOF
> > +  >       g
> > +  >       |
> > +  >   f   e
> > +  >   |   |
> > +  >   e'  d # replace: e -> e'
> > +  >    \ /
> > +  >     c
> > +  >     |
> > +  >   x b
> > +  >    \|
> > +  >     a
> > +  > EOF
> > +  $ hg log -G -r 'a':
> > +  o  8:2876ce66c6eb g
> > +  |
> > +  | o  7:3ffec603ab53 f
> > +  | |
> > +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
> > +  | |
> > +  | o  5:63324dc512ea e'
> > +  | |
> > +  o |  4:76be324c128b d
> > +  |/
> > +  o  3:a82ac2b38757 c
> > +  |
> > +  | o  2:630d7c95eff7 x
> > +  | |
> > +  o |  1:488e1b7e7341 b
> > +  |/
> > +  o  0:b173517d0057 a
> > +
> > +  $ hg rebase -b 'f' -d 'x'
> > +  rebasing 1:488e1b7e7341 "b" (b)
> > +  rebasing 3:a82ac2b38757 "c" (c)
> > +  rebasing 5:63324dc512ea "e'" (e')
> > +  rebasing 7:3ffec603ab53 "f" (f)
> > +  rebasing 4:76be324c128b "d" (d)
> > +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this
> would cause divergences
> > +  $ hg log -G -r 'a':
> > +  o  13:a1707a5b7c2c d
> > +  |
> > +  | o  12:ef6251596616 f
> > +  | |
> > +  | o  11:b6f172e64af9 e'
> > +  |/
> > +  o  10:d008e6b4d3fd c
> > +  |
> > +  o  9:67e8f4a16c49 b
> > +  |
> > +  | o  8:2876ce66c6eb g
> > +  | |
> > +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
> > +  | | |
> > +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
> > +  | | |
> > +  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
> > +  | | |
> > +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
> > +  | |/
> > +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
> > +  | |
> > +  o |  2:630d7c95eff7 x
> > +  | |
> > +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
> > +  |/
> > +  o  0:b173517d0057 a
> > +
> > +
> > +  $ cd ..
> > +
> >  Rebase merge where successor of one parent is equal to destination
> (issue5198)
> >
> >    $ hg init p1-succ-is-dest
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Martin von Zweigbergk a écrit :
> On Mon, Nov 13, 2017 at 3:07 PM, Augie Fackler <raf@durin42.com> wrote:
> 
>> On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
>>> # HG changeset patch
>>> # User Denis Laxalde <denis@laxalde.org>
>>> # Date 1510489041 -3600
>>> #      Sun Nov 12 13:17:21 2017 +0100
>>> # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
>>> # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
>>> # EXP-Topic rebase-obsolete
>>> rebase: exclude obsoletes without a successor in destination (issue5300)
>>
> 
> If I read it right, this line should be something like "rebase: exclude
> *descendants of* obsoletes without..."

That's correct here and in the release note below.

> 
>>
>> I took patches 1-3. I think this is right. I'm 90% sure this is
>> right. But I'm a little tired, and so I want Martin's more careful eye
>> on it if he has time.
>>
> 
> Thanks for asking. I think this makes sense. Thanks for working on this,
> Denis!
> 
> I think a general thing to note is that if a commit in the rebase set has
> become obsolete, then there's a conflict between the rebase invocation and
> the obsmarkers. The rebase command tells us to move the descendants onto
> the rebase destination, while the obsmarkers tell us to move them onto the
> obsolete commit's successor. Since we don't keep track of some default
> rebase destination (like git's upstream concept), any "hg rebase"
> invocation is a one-off as far as hg is concerned, so I think it makes
> sense to resolve the conflict in favor of the obsmarkers. That probably
> didn't make much sense, so let me give an example:
> 
> o B'
> |
> o O
> |
> | o U
> | |
> | | o C
> | | |
> | | x B
> | |/
> | .
> |/
> o A
> 
> Let's say C's upstream was somehow configured to be U here, and that B had
> been rebased onto O instead. If the user then runs "hg evolve -r C", you
> could argue that there would be a similar conflict between rebase/upstream
> that want to move C onto U and obsmarkers that want to move C onto B'.
> However, since we don't have that feature, B' is the only natural
> destination.

I think I get this right, but I'm not sure if you actually expect me to
change something (apart for other explicit comments, which I'll address)?
Also, the change proposed here handles cases where we cannot rebase C
onto B' because both B and B' are part of the rebase set (and B' is not
in destination).

> 
>>>
>>> In the following example, when trying to rebase 3:: onto 2, the rebase
>>> will abort with "this rebase will cause divergence from: 4":
>>>
>>>      o  7 f
>>>      |
>>>      | o  6 e
>>>      | |
>>>      | o  5 d'
>>>      | |
>>>      x |  4 d (rewritten as 5)
>>>      |/
>>>      o  3 c
>>>      |
>>>      | o  2 x
>>>      | |
>>>      o |  1 b
>>>      |/
>>>      o  0 a
>>>
>>> By excluding obsolete changesets without a successor in destination (4
>>> in the example above) and their descendants, we make rebase work in this
>>> case, thus giving:
>>>
>>>      o  11 e
>>>      |
>>>      o  10 d'
>>>      |
>>>      o  9 c
>>>      |
>>>      o  8 b
>>>      |
>>>      | o  7 f
>>>      | |
>>>      | | x  6 e (rewritten using rebase as 11)
>>>      | | |
>>>      | | x  5 d' (rewritten using rebase as 10)
>>>      | | |
>>>      | x |  4 d
>>>      | |/
>>>      | x  3 c (rewritten using rebase as 9)
>>>      | |
>>>      o |  2 x
>>>      | |
>>>      | x  1 b (rewritten using rebase as 8)
>>>      |/
>>>      o  0 a
>>>
>>> where branch 4:: is left behind while branch 5:: is rebased as expected.
>>>
>>> The rationale is that users may not be interested in rebasing orphan
>>> changesets when specifying a rebase set that include them but would
>>> still want "stable" ones to be rebased. Currently, the user is suggested
>>> to allow divergence (but probably does not want it) or they must specify
>>> a rebase set excluding problematic changesets (which might be a bit
>>> cumbersome). The approach proposed here corresponds to "Option 2" in
>>> https://www.mercurial-scm.org/wiki/CEDRebase.
>>
> 
> I think I'd still want "Option 3", but I don't think this patch prevents
> that from happening later.
> 
> 
>>>
>>> .. feature::
>>>
>>>     Let 'hg rebase' avoid content-divergence when obsolete changesets are
>>>     present in the rebase set by skipping possible sources of divergence.
>>
> 
> Similar to what I said above, I think it would help to mention descendants
> here. While "possible sources of divergence" is vague enough to be correct,
> it would be helpful if we could clarify it.
> 
>>
>>>
>>
> 
> Just to make sure, the stuff below is not part of the release notes, is it?
> I'm not familiar with the syntax, so I can't tell (and I'm too lazy to test
> it).

The directive block only includes indented lines, so by de-denting the
text below, it will not be in the release note.

>> We extend _computeobsoletenotrebased() so that it also return a set of
>>> obsolete changesets in rebaseset without a successor in destination.
>>> This 'obsoletewithoutsuccessorindestination' is then stored as an
>>> attribute of rebaseruntime and used in _performrebasesubset() to:
>>>
>>> * filter out descendants of these changesets from the revisions to
>>>    rebase;
>>> * issue a message about these revisions being skipped.
>>>
>>> This only occurs if 'evolution.allowdivergence' option is off and
>>> 'rebaseskipobsolete' is on.
>>>
>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>> --- a/hgext/rebase.py
>>> +++ b/hgext/rebase.py
>>> @@ -179,6 +179,7 @@ class rebaseruntime(object):
>>>           # other extensions
>>>           self.keepopen = opts.get('keepopen', False)
>>>           self.obsoletenotrebased = {}
>>> +        self.obsoletewithoutsuccessorindestination = set()
>>>
>>>       @property
>>>       def repo(self):
>>> @@ -311,9 +312,10 @@ class rebaseruntime(object):
>>>           if not self.ui.configbool('experimental',
>> 'rebaseskipobsolete'):
>>>               return
>>>           obsoleteset = set(obsoleterevs)
>>> -        self.obsoletenotrebased = _computeobsoletenotrebased(self.repo,
>>> -                                    obsoleteset, destmap)
>>> +        self.obsoletenotrebased, self.obsoletewithoutsuccessorindestination
>> = \
>>> +            _computeobsoletenotrebased(self.repo, obsoleteset, destmap)
>>>           skippedset = set(self.obsoletenotrebased)
>>> +        skippedset.update(self.obsoletewithoutsuccessorindestination)
>>>           _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
>>>
>>>       def _prepareabortorcontinue(self, isabort):
>>> @@ -419,12 +421,26 @@ class rebaseruntime(object):
>>>       def _performrebasesubset(self, tr, subset, pos, total):
>>>           repo, ui, opts = self.repo, self.ui, self.opts
>>>           sortedrevs = repo.revs('sort(%ld, -topo)', subset)
>>> +        allowdivergence = self.ui.configbool(
>>> +            'experimental', 'evolution.allowdivergence')
>>> +        if not allowdivergence:
>>> +            sortedrevs -= repo.revs(
>>> +                'descendants(%ld) and not %ld',
>>> +                self.obsoletewithoutsuccessorindestination,
>>> +                self.obsoletewithoutsuccessorindestination,
>>> +            )
>>>           for rev in sortedrevs:
>>>               dest = self.destmap[rev]
>>>               ctx = repo[rev]
>>>               desc = _ctxdesc(ctx)
>>>               if self.state[rev] == rev:
>>>                   ui.status(_('already rebased %s\n') % desc)
>>> +            elif (not allowdivergence
>>> +                  and rev in self.obsoletewithoutsuccessori
>> ndestination):
>>> +                msg = _('note: not rebasing %s and its descendants as '
>>> +                        'this would cause divergences\n') % desc
>>> +                repo.ui.status(msg)
>>> +                self.skipped.add(rev)
>>>               elif rev in self.obsoletenotrebased:
>>>                   succ = self.obsoletenotrebased[rev]
>>>                   if succ is None:
>>> @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
>>>       return set(r for r in revs if repo[r].obsolete())
>>>
>>>   def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
>>> -    """return a mapping obsolete => successor for all obsolete nodes to
>> be
>>> -    rebased that have a successors in the destination
>>> +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindest
>> ination).
>>> +
>>> +    `obsoletenotrebased` is a mapping mapping obsolete => successor for
>> all
>>> +    obsolete nodes to be rebased given in `rebaseobsrevs`.
>>>
>>> -    obsolete => None entries in the mapping indicate nodes with no
>> successor"""
>>> +    `obsoletewithoutsuccessorindestination` is a set with obsolete
>> revisions
>>> +    without a successor in destination.
>>> +    """
>>>       obsoletenotrebased = {}
>>> +    obsoletewithoutsuccessorindestination = set([])
>>>
>>>       assert repo.filtername is None
>>>       cl = repo.changelog
>>> @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
>>>                   if cl.isancestor(succnode, destnode):
>>>                       obsoletenotrebased[srcrev] = nodemap[succnode]
>>>                       break
>>> +            else:
>>> +                # Unless 'srcrev' only has one successor and it's not in
>>> +                # rebaseset, we might skip it and its descendants as it
>> has no
>>> +                # successor in destination. Otherwise, we'll issue the
>>> +                # divergence warning and abort.
>>> +                if (len(successors) > 1
>>
> 
> This line doesn't seem necessary, but feel free to keep if you think it
> helps readability (I find it slightly distracting).

You're right, it's not needed.

> 
>>
>> +                        and any(nodemap[s] in destmap
>>> +                                for s in successors if s != srcnode)):
>>> +                    obsoletewithoutsuccessorindestination.add(srcrev)
>>>
>>> -    return obsoletenotrebased
>>> +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
>>>
>>>   def summaryhook(ui, repo):
>>>       if not repo.vfs.exists('rebasestate'):
>>> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
>>> --- a/tests/test-rebase-obsolete.t
>>> +++ b/tests/test-rebase-obsolete.t
>>> @@ -987,6 +987,208 @@ Create the changes that we will rebase
>>>     rebasing 21:7bdc8a87673d "dummy change" (tip)
>>>     $ cd ..
>>>
>>> +Divergence cases due to obsolete changesets
>>> +-------------------------------------------
>>> +
>>> +We should ignore branches with unstable changesets when they are based
>> on an
>>> +obsolete changeset which successor is in rebase set.
>>> +
>>> +  $ hg init divergence
>>> +  $ cd divergence
>>> +  $ cat >> .hg/hgrc << EOF
>>> +  > [extensions]
>>> +  > strip =
>>> +  > [alias]
>>> +  > strip = strip --no-backup --quiet
>>> +  > [templates]
>>> +  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilities,"
>> ({instabilities})")}\n'
>>> +  > EOF
>>> +
>>> +  $ hg debugdrawdag <<EOF
>>> +  >   e   f
>>> +  >   |   |
>>> +  >   d'  d # replace: d -> d'
>>> +  >    \ /
>>> +  >     c
>>> +  >     |
>>> +  >   x b
>>> +  >    \|
>>> +  >     a
>>> +  > EOF
>>> +  $ hg log -G -r 'a'::
>>> +  o  7:1143e9adc121 f
>>> +  |
>>> +  | o  6:d60ebfa0f1cb e
>>> +  | |
>>> +  | o  5:027ad6c5830d d'
>>> +  | |
>>> +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>> +  |/
>>> +  o  3:a82ac2b38757 c
>>> +  |
>>> +  | o  2:630d7c95eff7 x
>>> +  | |
>>> +  o |  1:488e1b7e7341 b
>>> +  |/
>>> +  o  0:b173517d0057 a
>>> +
>>> +
>>> +Changeset d and its descendants are excluded to avoid divergence of d,
>> which
>>> +would occur because the successor of d (d') is also in rebaseset. As a
>>> +consequence f (descendant of d) is left behind.
>>> +
>>> +  $ hg rebase -b 'e' -d 'x'
>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>> +  rebasing 5:027ad6c5830d "d'" (d')
>>> +  rebasing 6:d60ebfa0f1cb "e" (e)
>>> +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this
>> would cause divergences
>>
> 
> The plural "divergences" sounds wrong to me, but I'm not sure. Augie (or
> others), since you're a native English speaker, do you think it should be
> singular or plural?

It feels strange to me as well. I took it from the existing message
"this rebase will cause divergences from: <obsolete>" (which, maybe, was
also written by a non-native).

> 
>>> +  $ hg log -G -r 'a'::
>>> +  o  11:eb6d63fc4ed5 e
>>> +  |
>>> +  o  10:44d8c724a70c d'
>>> +  |
>>> +  o  9:d008e6b4d3fd c
>>> +  |
>>> +  o  8:67e8f4a16c49 b
>>> +  |
>>> +  | o  7:1143e9adc121 f
>>> +  | |
>>> +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
>>> +  | | |
>>> +  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
>>> +  | | |
>>> +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>> +  | |/
>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
>>> +  | |
>>> +  o |  2:630d7c95eff7 x
>>> +  | |
>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
>>> +  |/
>>> +  o  0:b173517d0057 a
>>> +
>>> +  $ hg strip -r 8:
>>> +
>>> +If the rebase set has an obsolete (d) with a unique successor (d')
>> outside the
>>> +rebase set and not in destination, we still get the divergence warning.
>> By
>>> +allowing divergence, we can perform the rebase.
>>> +
>>> +  $ hg rebase -r 'c'::'f' -d 'x'
>>> +  abort: this rebase will cause divergences from: 76be324c128b
>>> +  (to force the rebase please set experimental.evolution.allowdi
>> vergence=True)
>>> +  [255]
>>> +  $ hg rebase --config experimental.evolution.allowdivergence=true -r
>> 'c'::'f' -d 'x'
>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>> +  rebasing 4:76be324c128b "d" (d)
>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>> +  $ hg log -G -r 'a':: -T instabilities
>>> +  o  10:e1744ea07510 f
>>> +  |
>>> +  o  9:e2b36ea9a0a0 d (content-divergent)
>>> +  |
>>> +  o  8:6a0376de376e c
>>> +  |
>>> +  | x  7:1143e9adc121 f
>>> +  | |
>>> +  | | o  6:d60ebfa0f1cb e (orphan)
>>> +  | | |
>>> +  | | o  5:027ad6c5830d d' (orphan content-divergent)
>>> +  | | |
>>> +  | x |  4:76be324c128b d
>>> +  | |/
>>> +  | x  3:a82ac2b38757 c
>>> +  | |
>>> +  o |  2:630d7c95eff7 x
>>> +  | |
>>> +  | o  1:488e1b7e7341 b
>>> +  |/
>>> +  o  0:b173517d0057 a
>>> +
>>> +  $ hg strip -r 8:
>>> +
>>> +(Not skipping obsoletes means that divergence is allowed.)
>>> +
>>> +  $ hg rebase --config experimental.rebaseskipobsolete=false -r
>> 'c'::'f' -d 'x'
>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>> +  rebasing 4:76be324c128b "d" (d)
>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>> +
>>> +  $ hg strip -r 0:
>>> +
>>> +Similar test on a more complex graph
>>> +
>>> +  $ hg debugdrawdag <<EOF
>>> +  >       g
>>> +  >       |
>>> +  >   f   e
>>> +  >   |   |
>>> +  >   e'  d # replace: e -> e'
>>> +  >    \ /
>>> +  >     c
>>> +  >     |
>>> +  >   x b
>>> +  >    \|
>>> +  >     a
>>> +  > EOF
>>> +  $ hg log -G -r 'a':
>>> +  o  8:2876ce66c6eb g
>>> +  |
>>> +  | o  7:3ffec603ab53 f
>>> +  | |
>>> +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>> +  | |
>>> +  | o  5:63324dc512ea e'
>>> +  | |
>>> +  o |  4:76be324c128b d
>>> +  |/
>>> +  o  3:a82ac2b38757 c
>>> +  |
>>> +  | o  2:630d7c95eff7 x
>>> +  | |
>>> +  o |  1:488e1b7e7341 b
>>> +  |/
>>> +  o  0:b173517d0057 a
>>> +
>>> +  $ hg rebase -b 'f' -d 'x'
>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>> +  rebasing 5:63324dc512ea "e'" (e')
>>> +  rebasing 7:3ffec603ab53 "f" (f)
>>> +  rebasing 4:76be324c128b "d" (d)
>>> +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this
>> would cause divergences
>>> +  $ hg log -G -r 'a':
>>> +  o  13:a1707a5b7c2c d
>>> +  |
>>> +  | o  12:ef6251596616 f
>>> +  | |
>>> +  | o  11:b6f172e64af9 e'
>>> +  |/
>>> +  o  10:d008e6b4d3fd c
>>> +  |
>>> +  o  9:67e8f4a16c49 b
>>> +  |
>>> +  | o  8:2876ce66c6eb g
>>> +  | |
>>> +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
>>> +  | | |
>>> +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>> +  | | |
>>> +  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
>>> +  | | |
>>> +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
>>> +  | |/
>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
>>> +  | |
>>> +  o |  2:630d7c95eff7 x
>>> +  | |
>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
>>> +  |/
>>> +  o  0:b173517d0057 a
>>> +
>>> +
>>> +  $ cd ..
>>> +
>>>   Rebase merge where successor of one parent is equal to destination
>> (issue5198)
>>>
>>>     $ hg init p1-succ-is-dest
>>> _______________________________________________
>>> Mercurial-devel mailing list
>>> Mercurial-devel@mercurial-scm.org
>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>
>

On Tue, Nov 14, 2017 at 3:24 AM, Denis Laxalde <denis@laxalde.org> wrote:

> Martin von Zweigbergk a écrit :
>
>> On Mon, Nov 13, 2017 at 3:07 PM, Augie Fackler <raf@durin42.com> wrote:
>>
>> On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
>>>
>>>> # HG changeset patch
>>>> # User Denis Laxalde <denis@laxalde.org>
>>>> # Date 1510489041 -3600
>>>> #      Sun Nov 12 13:17:21 2017 +0100
>>>> # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
>>>> # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
>>>> # EXP-Topic rebase-obsolete
>>>> rebase: exclude obsoletes without a successor in destination (issue5300)
>>>>
>>>
>>>
>> If I read it right, this line should be something like "rebase: exclude
>> *descendants of* obsoletes without..."
>>
>
> That's correct here and in the release note below.
>
>
>
>>
>>> I took patches 1-3. I think this is right. I'm 90% sure this is
>>> right. But I'm a little tired, and so I want Martin's more careful eye
>>> on it if he has time.
>>>
>>>
>> Thanks for asking. I think this makes sense. Thanks for working on this,
>> Denis!
>>
>> I think a general thing to note is that if a commit in the rebase set has
>> become obsolete, then there's a conflict between the rebase invocation and
>> the obsmarkers. The rebase command tells us to move the descendants onto
>> the rebase destination, while the obsmarkers tell us to move them onto the
>> obsolete commit's successor. Since we don't keep track of some default
>> rebase destination (like git's upstream concept), any "hg rebase"
>> invocation is a one-off as far as hg is concerned, so I think it makes
>> sense to resolve the conflict in favor of the obsmarkers. That probably
>> didn't make much sense, so let me give an example:
>>
>> o B'
>> |
>> o O
>> |
>> | o U
>> | |
>> | | o C
>> | | |
>> | | x B
>> | |/
>> | .
>> |/
>> o A
>>
>> Let's say C's upstream was somehow configured to be U here, and that B had
>> been rebased onto O instead. If the user then runs "hg evolve -r C", you
>> could argue that there would be a similar conflict between rebase/upstream
>> that want to move C onto U and obsmarkers that want to move C onto B'.
>> However, since we don't have that feature, B' is the only natural
>> destination.
>>
>
> I think I get this right, but I'm not sure if you actually expect me to
> change something (apart for other explicit comments, which I'll address)?
>

Not asking for any changes. I just thought I'd mention an aspect I had not
thought about before.


> Also, the change proposed here handles cases where we cannot rebase C
> onto B' because both B and B' are part of the rebase set (and B' is not
> in destination).
>
>
>
>>
>>>> In the following example, when trying to rebase 3:: onto 2, the rebase
>>>> will abort with "this rebase will cause divergence from: 4":
>>>>
>>>>      o  7 f
>>>>      |
>>>>      | o  6 e
>>>>      | |
>>>>      | o  5 d'
>>>>      | |
>>>>      x |  4 d (rewritten as 5)
>>>>      |/
>>>>      o  3 c
>>>>      |
>>>>      | o  2 x
>>>>      | |
>>>>      o |  1 b
>>>>      |/
>>>>      o  0 a
>>>>
>>>> By excluding obsolete changesets without a successor in destination (4
>>>> in the example above) and their descendants, we make rebase work in this
>>>> case, thus giving:
>>>>
>>>>      o  11 e
>>>>      |
>>>>      o  10 d'
>>>>      |
>>>>      o  9 c
>>>>      |
>>>>      o  8 b
>>>>      |
>>>>      | o  7 f
>>>>      | |
>>>>      | | x  6 e (rewritten using rebase as 11)
>>>>      | | |
>>>>      | | x  5 d' (rewritten using rebase as 10)
>>>>      | | |
>>>>      | x |  4 d
>>>>      | |/
>>>>      | x  3 c (rewritten using rebase as 9)
>>>>      | |
>>>>      o |  2 x
>>>>      | |
>>>>      | x  1 b (rewritten using rebase as 8)
>>>>      |/
>>>>      o  0 a
>>>>
>>>> where branch 4:: is left behind while branch 5:: is rebased as expected.
>>>>
>>>> The rationale is that users may not be interested in rebasing orphan
>>>> changesets when specifying a rebase set that include them but would
>>>> still want "stable" ones to be rebased. Currently, the user is suggested
>>>> to allow divergence (but probably does not want it) or they must specify
>>>> a rebase set excluding problematic changesets (which might be a bit
>>>> cumbersome). The approach proposed here corresponds to "Option 2" in
>>>> https://www.mercurial-scm.org/wiki/CEDRebase.
>>>>
>>>
>>>
>> I think I'd still want "Option 3", but I don't think this patch prevents
>> that from happening later.
>>
>>
>>
>>>> .. feature::
>>>>
>>>>     Let 'hg rebase' avoid content-divergence when obsolete changesets
>>>> are
>>>>     present in the rebase set by skipping possible sources of
>>>> divergence.
>>>>
>>>
>>>
>> Similar to what I said above, I think it would help to mention descendants
>> here. While "possible sources of divergence" is vague enough to be
>> correct,
>> it would be helpful if we could clarify it.
>>
>>
>>>
>>>>
>>>
>> Just to make sure, the stuff below is not part of the release notes, is
>> it?
>> I'm not familiar with the syntax, so I can't tell (and I'm too lazy to
>> test
>> it).
>>
>
> The directive block only includes indented lines, so by de-denting the
> text below, it will not be in the release note.
>
>
> We extend _computeobsoletenotrebased() so that it also return a set of
>>>
>>>> obsolete changesets in rebaseset without a successor in destination.
>>>> This 'obsoletewithoutsuccessorindestination' is then stored as an
>>>> attribute of rebaseruntime and used in _performrebasesubset() to:
>>>>
>>>> * filter out descendants of these changesets from the revisions to
>>>>    rebase;
>>>> * issue a message about these revisions being skipped.
>>>>
>>>> This only occurs if 'evolution.allowdivergence' option is off and
>>>> 'rebaseskipobsolete' is on.
>>>>
>>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>>> --- a/hgext/rebase.py
>>>> +++ b/hgext/rebase.py
>>>> @@ -179,6 +179,7 @@ class rebaseruntime(object):
>>>>           # other extensions
>>>>           self.keepopen = opts.get('keepopen', False)
>>>>           self.obsoletenotrebased = {}
>>>> +        self.obsoletewithoutsuccessorindestination = set()
>>>>
>>>>       @property
>>>>       def repo(self):
>>>> @@ -311,9 +312,10 @@ class rebaseruntime(object):
>>>>           if not self.ui.configbool('experimental',
>>>>
>>> 'rebaseskipobsolete'):
>>>
>>>>               return
>>>>           obsoleteset = set(obsoleterevs)
>>>> -        self.obsoletenotrebased = _computeobsoletenotrebased(sel
>>>> f.repo,
>>>> -                                    obsoleteset, destmap)
>>>> +        self.obsoletenotrebased, self.obsoletewithoutsuccessori
>>>> ndestination
>>>>
>>> = \
>>>
>>>> +            _computeobsoletenotrebased(self.repo, obsoleteset,
>>>> destmap)
>>>>           skippedset = set(self.obsoletenotrebased)
>>>> +        skippedset.update(self.obsoletewithoutsuccessorindestination)
>>>>           _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
>>>>
>>>>       def _prepareabortorcontinue(self, isabort):
>>>> @@ -419,12 +421,26 @@ class rebaseruntime(object):
>>>>       def _performrebasesubset(self, tr, subset, pos, total):
>>>>           repo, ui, opts = self.repo, self.ui, self.opts
>>>>           sortedrevs = repo.revs('sort(%ld, -topo)', subset)
>>>> +        allowdivergence = self.ui.configbool(
>>>> +            'experimental', 'evolution.allowdivergence')
>>>> +        if not allowdivergence:
>>>> +            sortedrevs -= repo.revs(
>>>> +                'descendants(%ld) and not %ld',
>>>> +                self.obsoletewithoutsuccessorindestination,
>>>> +                self.obsoletewithoutsuccessorindestination,
>>>> +            )
>>>>           for rev in sortedrevs:
>>>>               dest = self.destmap[rev]
>>>>               ctx = repo[rev]
>>>>               desc = _ctxdesc(ctx)
>>>>               if self.state[rev] == rev:
>>>>                   ui.status(_('already rebased %s\n') % desc)
>>>> +            elif (not allowdivergence
>>>> +                  and rev in self.obsoletewithoutsuccessori
>>>>
>>> ndestination):
>>>
>>>> +                msg = _('note: not rebasing %s and its descendants as '
>>>> +                        'this would cause divergences\n') % desc
>>>> +                repo.ui.status(msg)
>>>> +                self.skipped.add(rev)
>>>>               elif rev in self.obsoletenotrebased:
>>>>                   succ = self.obsoletenotrebased[rev]
>>>>                   if succ is None:
>>>> @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
>>>>       return set(r for r in revs if repo[r].obsolete())
>>>>
>>>>   def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
>>>> -    """return a mapping obsolete => successor for all obsolete nodes to
>>>>
>>> be
>>>
>>>> -    rebased that have a successors in the destination
>>>> +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindest
>>>>
>>> ination).
>>>
>>>> +
>>>> +    `obsoletenotrebased` is a mapping mapping obsolete => successor for
>>>>
>>> all
>>>
>>>> +    obsolete nodes to be rebased given in `rebaseobsrevs`.
>>>>
>>>> -    obsolete => None entries in the mapping indicate nodes with no
>>>>
>>> successor"""
>>>
>>>> +    `obsoletewithoutsuccessorindestination` is a set with obsolete
>>>>
>>> revisions
>>>
>>>> +    without a successor in destination.
>>>> +    """
>>>>       obsoletenotrebased = {}
>>>> +    obsoletewithoutsuccessorindestination = set([])
>>>>
>>>>       assert repo.filtername is None
>>>>       cl = repo.changelog
>>>> @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
>>>>                   if cl.isancestor(succnode, destnode):
>>>>                       obsoletenotrebased[srcrev] = nodemap[succnode]
>>>>                       break
>>>> +            else:
>>>> +                # Unless 'srcrev' only has one successor and it's not
>>>> in
>>>> +                # rebaseset, we might skip it and its descendants as it
>>>>
>>> has no
>>>
>>>> +                # successor in destination. Otherwise, we'll issue the
>>>> +                # divergence warning and abort.
>>>>
>>>
Hmm, that actually doesn't sound like "successor in destination", it sounds
like "no successor in rebase set". It sounds like it'll miss this case:

$ hg hg rebase -b C -d E

o E
|
o B''
|
| o D
| |
| x B'
| |
| | o C
| | |
| | x B
| |/
| o A
|/
o X

I think it would be nice to *not* skip C or D in this case (IIUC, C would
be skipped by this patch). Maybe it's enough to check only the immediate
visible successors? I.e. using the "closest=True" thing that I think Boris
added recently.


> +                if (len(successors) > 1
>>>>
>>>
>>>
>> This line doesn't seem necessary, but feel free to keep if you think it
>> helps readability (I find it slightly distracting).
>>
>
> You're right, it's not needed.
>
>
>
>>
>>> +                        and any(nodemap[s] in destmap
>>>
>>>> +                                for s in successors if s != srcnode)):
>>>> +                    obsoletewithoutsuccessorindestination.add(srcrev)
>>>>
>>>
This does not seem to check the "only has one successor" that you
described. I'm not sure what the reason for that condition is, so I don't
know if the code or the description should be fixed (or if I just
misunderstood). I saw some comment suggesting that we don't care much about
splits yet, so I suppose the "only has one successor" is not about that?


>>>> -    return obsoletenotrebased
>>>> +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
>>>>
>>>>   def summaryhook(ui, repo):
>>>>       if not repo.vfs.exists('rebasestate'):
>>>> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
>>>> --- a/tests/test-rebase-obsolete.t
>>>> +++ b/tests/test-rebase-obsolete.t
>>>> @@ -987,6 +987,208 @@ Create the changes that we will rebase
>>>>     rebasing 21:7bdc8a87673d "dummy change" (tip)
>>>>     $ cd ..
>>>>
>>>> +Divergence cases due to obsolete changesets
>>>> +-------------------------------------------
>>>> +
>>>> +We should ignore branches with unstable changesets when they are based
>>>>
>>> on an
>>>
>>>> +obsolete changeset which successor is in rebase set.
>>>> +
>>>> +  $ hg init divergence
>>>> +  $ cd divergence
>>>> +  $ cat >> .hg/hgrc << EOF
>>>> +  > [extensions]
>>>> +  > strip =
>>>> +  > [alias]
>>>> +  > strip = strip --no-backup --quiet
>>>> +  > [templates]
>>>> +  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilit
>>>> ies,"
>>>>
>>> ({instabilities})")}\n'
>>>
>>>> +  > EOF
>>>> +
>>>> +  $ hg debugdrawdag <<EOF
>>>> +  >   e   f
>>>> +  >   |   |
>>>> +  >   d'  d # replace: d -> d'
>>>> +  >    \ /
>>>> +  >     c
>>>> +  >     |
>>>> +  >   x b
>>>> +  >    \|
>>>> +  >     a
>>>> +  > EOF
>>>> +  $ hg log -G -r 'a'::
>>>> +  o  7:1143e9adc121 f
>>>> +  |
>>>> +  | o  6:d60ebfa0f1cb e
>>>> +  | |
>>>> +  | o  5:027ad6c5830d d'
>>>> +  | |
>>>> +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>> +  |/
>>>> +  o  3:a82ac2b38757 c
>>>> +  |
>>>> +  | o  2:630d7c95eff7 x
>>>> +  | |
>>>> +  o |  1:488e1b7e7341 b
>>>> +  |/
>>>> +  o  0:b173517d0057 a
>>>> +
>>>> +
>>>> +Changeset d and its descendants are excluded to avoid divergence of d,
>>>>
>>> which
>>>
>>>> +would occur because the successor of d (d') is also in rebaseset. As a
>>>> +consequence f (descendant of d) is left behind.
>>>> +
>>>> +  $ hg rebase -b 'e' -d 'x'
>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>> +  rebasing 5:027ad6c5830d "d'" (d')
>>>> +  rebasing 6:d60ebfa0f1cb "e" (e)
>>>> +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this
>>>>
>>> would cause divergences
>>>
>>>
>> The plural "divergences" sounds wrong to me, but I'm not sure. Augie (or
>> others), since you're a native English speaker, do you think it should be
>> singular or plural?
>>
>
> It feels strange to me as well. I took it from the existing message
> "this rebase will cause divergences from: <obsolete>" (which, maybe, was
> also written by a non-native).
>
>
>
>> +  $ hg log -G -r 'a'::
>>>> +  o  11:eb6d63fc4ed5 e
>>>> +  |
>>>> +  o  10:44d8c724a70c d'
>>>> +  |
>>>> +  o  9:d008e6b4d3fd c
>>>> +  |
>>>> +  o  8:67e8f4a16c49 b
>>>> +  |
>>>> +  | o  7:1143e9adc121 f
>>>> +  | |
>>>> +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
>>>> +  | | |
>>>> +  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
>>>> +  | | |
>>>> +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>> +  | |/
>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
>>>> +  | |
>>>> +  o |  2:630d7c95eff7 x
>>>> +  | |
>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
>>>> +  |/
>>>> +  o  0:b173517d0057 a
>>>> +
>>>> +  $ hg strip -r 8:
>>>> +
>>>> +If the rebase set has an obsolete (d) with a unique successor (d')
>>>>
>>> outside the
>>>
>>>> +rebase set and not in destination, we still get the divergence warning.
>>>>
>>> By
>>>
>>>> +allowing divergence, we can perform the rebase.
>>>> +
>>>> +  $ hg rebase -r 'c'::'f' -d 'x'
>>>> +  abort: this rebase will cause divergences from: 76be324c128b
>>>> +  (to force the rebase please set experimental.evolution.allowdi
>>>>
>>> vergence=True)
>>>
>>>> +  [255]
>>>> +  $ hg rebase --config experimental.evolution.allowdivergence=true -r
>>>>
>>> 'c'::'f' -d 'x'
>>>
>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>> +  rebasing 4:76be324c128b "d" (d)
>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>> +  $ hg log -G -r 'a':: -T instabilities
>>>> +  o  10:e1744ea07510 f
>>>> +  |
>>>> +  o  9:e2b36ea9a0a0 d (content-divergent)
>>>> +  |
>>>> +  o  8:6a0376de376e c
>>>> +  |
>>>> +  | x  7:1143e9adc121 f
>>>> +  | |
>>>> +  | | o  6:d60ebfa0f1cb e (orphan)
>>>> +  | | |
>>>> +  | | o  5:027ad6c5830d d' (orphan content-divergent)
>>>> +  | | |
>>>> +  | x |  4:76be324c128b d
>>>> +  | |/
>>>> +  | x  3:a82ac2b38757 c
>>>> +  | |
>>>> +  o |  2:630d7c95eff7 x
>>>> +  | |
>>>> +  | o  1:488e1b7e7341 b
>>>> +  |/
>>>> +  o  0:b173517d0057 a
>>>> +
>>>> +  $ hg strip -r 8:
>>>> +
>>>> +(Not skipping obsoletes means that divergence is allowed.)
>>>> +
>>>> +  $ hg rebase --config experimental.rebaseskipobsolete=false -r
>>>>
>>> 'c'::'f' -d 'x'
>>>
>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>> +  rebasing 4:76be324c128b "d" (d)
>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>> +
>>>> +  $ hg strip -r 0:
>>>> +
>>>> +Similar test on a more complex graph
>>>> +
>>>> +  $ hg debugdrawdag <<EOF
>>>> +  >       g
>>>> +  >       |
>>>> +  >   f   e
>>>> +  >   |   |
>>>> +  >   e'  d # replace: e -> e'
>>>> +  >    \ /
>>>> +  >     c
>>>> +  >     |
>>>> +  >   x b
>>>> +  >    \|
>>>> +  >     a
>>>> +  > EOF
>>>> +  $ hg log -G -r 'a':
>>>> +  o  8:2876ce66c6eb g
>>>> +  |
>>>> +  | o  7:3ffec603ab53 f
>>>> +  | |
>>>> +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>> +  | |
>>>> +  | o  5:63324dc512ea e'
>>>> +  | |
>>>> +  o |  4:76be324c128b d
>>>> +  |/
>>>> +  o  3:a82ac2b38757 c
>>>> +  |
>>>> +  | o  2:630d7c95eff7 x
>>>> +  | |
>>>> +  o |  1:488e1b7e7341 b
>>>> +  |/
>>>> +  o  0:b173517d0057 a
>>>> +
>>>> +  $ hg rebase -b 'f' -d 'x'
>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>> +  rebasing 5:63324dc512ea "e'" (e')
>>>> +  rebasing 7:3ffec603ab53 "f" (f)
>>>> +  rebasing 4:76be324c128b "d" (d)
>>>> +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this
>>>>
>>> would cause divergences
>>>
>>>> +  $ hg log -G -r 'a':
>>>> +  o  13:a1707a5b7c2c d
>>>> +  |
>>>> +  | o  12:ef6251596616 f
>>>> +  | |
>>>> +  | o  11:b6f172e64af9 e'
>>>> +  |/
>>>> +  o  10:d008e6b4d3fd c
>>>> +  |
>>>> +  o  9:67e8f4a16c49 b
>>>> +  |
>>>> +  | o  8:2876ce66c6eb g
>>>> +  | |
>>>> +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
>>>> +  | | |
>>>> +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>> +  | | |
>>>> +  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
>>>> +  | | |
>>>> +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
>>>> +  | |/
>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
>>>> +  | |
>>>> +  o |  2:630d7c95eff7 x
>>>> +  | |
>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
>>>> +  |/
>>>> +  o  0:b173517d0057 a
>>>> +
>>>> +
>>>> +  $ cd ..
>>>> +
>>>>   Rebase merge where successor of one parent is equal to destination
>>>>
>>> (issue5198)
>>>
>>>>
>>>>     $ hg init p1-succ-is-dest
>>>> _______________________________________________
>>>> Mercurial-devel mailing list
>>>> Mercurial-devel@mercurial-scm.org
>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>>>
>>>
>>>
>>
>

Martin von Zweigbergk a écrit :
> On Tue, Nov 14, 2017 at 3:24 AM, Denis Laxalde <denis@laxalde.org> wrote:
> 
>> Martin von Zweigbergk a écrit :
>>
>>> On Mon, Nov 13, 2017 at 3:07 PM, Augie Fackler <raf@durin42.com> wrote:
>>>
>>> On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
>>>>
>>>>> # HG changeset patch
>>>>> # User Denis Laxalde <denis@laxalde.org>
>>>>> # Date 1510489041 -3600
>>>>> #      Sun Nov 12 13:17:21 2017 +0100
>>>>> # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
>>>>> # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
>>>>> # EXP-Topic rebase-obsolete
>>>>> rebase: exclude obsoletes without a successor in destination (issue5300)
>>>>>
>>>>
>>>>
>>> If I read it right, this line should be something like "rebase: exclude
>>> *descendants of* obsoletes without..."
>>>
>>
>> That's correct here and in the release note below.
>>
>>
>>
>>>
>>>> I took patches 1-3. I think this is right. I'm 90% sure this is
>>>> right. But I'm a little tired, and so I want Martin's more careful eye
>>>> on it if he has time.
>>>>
>>>>
>>> Thanks for asking. I think this makes sense. Thanks for working on this,
>>> Denis!
>>>
>>> I think a general thing to note is that if a commit in the rebase set has
>>> become obsolete, then there's a conflict between the rebase invocation and
>>> the obsmarkers. The rebase command tells us to move the descendants onto
>>> the rebase destination, while the obsmarkers tell us to move them onto the
>>> obsolete commit's successor. Since we don't keep track of some default
>>> rebase destination (like git's upstream concept), any "hg rebase"
>>> invocation is a one-off as far as hg is concerned, so I think it makes
>>> sense to resolve the conflict in favor of the obsmarkers. That probably
>>> didn't make much sense, so let me give an example:
>>>
>>> o B'
>>> |
>>> o O
>>> |
>>> | o U
>>> | |
>>> | | o C
>>> | | |
>>> | | x B
>>> | |/
>>> | .
>>> |/
>>> o A
>>>
>>> Let's say C's upstream was somehow configured to be U here, and that B had
>>> been rebased onto O instead. If the user then runs "hg evolve -r C", you
>>> could argue that there would be a similar conflict between rebase/upstream
>>> that want to move C onto U and obsmarkers that want to move C onto B'.
>>> However, since we don't have that feature, B' is the only natural
>>> destination.
>>>
>>
>> I think I get this right, but I'm not sure if you actually expect me to
>> change something (apart for other explicit comments, which I'll address)?
>>
> 
> Not asking for any changes. I just thought I'd mention an aspect I had not
> thought about before.
> 
> 
>> Also, the change proposed here handles cases where we cannot rebase C
>> onto B' because both B and B' are part of the rebase set (and B' is not
>> in destination).
>>
>>
>>
>>>
>>>>> In the following example, when trying to rebase 3:: onto 2, the rebase
>>>>> will abort with "this rebase will cause divergence from: 4":
>>>>>
>>>>>       o  7 f
>>>>>       |
>>>>>       | o  6 e
>>>>>       | |
>>>>>       | o  5 d'
>>>>>       | |
>>>>>       x |  4 d (rewritten as 5)
>>>>>       |/
>>>>>       o  3 c
>>>>>       |
>>>>>       | o  2 x
>>>>>       | |
>>>>>       o |  1 b
>>>>>       |/
>>>>>       o  0 a
>>>>>
>>>>> By excluding obsolete changesets without a successor in destination (4
>>>>> in the example above) and their descendants, we make rebase work in this
>>>>> case, thus giving:
>>>>>
>>>>>       o  11 e
>>>>>       |
>>>>>       o  10 d'
>>>>>       |
>>>>>       o  9 c
>>>>>       |
>>>>>       o  8 b
>>>>>       |
>>>>>       | o  7 f
>>>>>       | |
>>>>>       | | x  6 e (rewritten using rebase as 11)
>>>>>       | | |
>>>>>       | | x  5 d' (rewritten using rebase as 10)
>>>>>       | | |
>>>>>       | x |  4 d
>>>>>       | |/
>>>>>       | x  3 c (rewritten using rebase as 9)
>>>>>       | |
>>>>>       o |  2 x
>>>>>       | |
>>>>>       | x  1 b (rewritten using rebase as 8)
>>>>>       |/
>>>>>       o  0 a
>>>>>
>>>>> where branch 4:: is left behind while branch 5:: is rebased as expected.
>>>>>
>>>>> The rationale is that users may not be interested in rebasing orphan
>>>>> changesets when specifying a rebase set that include them but would
>>>>> still want "stable" ones to be rebased. Currently, the user is suggested
>>>>> to allow divergence (but probably does not want it) or they must specify
>>>>> a rebase set excluding problematic changesets (which might be a bit
>>>>> cumbersome). The approach proposed here corresponds to "Option 2" in
>>>>> https://www.mercurial-scm.org/wiki/CEDRebase.
>>>>>
>>>>
>>>>
>>> I think I'd still want "Option 3", but I don't think this patch prevents
>>> that from happening later.
>>>
>>>
>>>
>>>>> .. feature::
>>>>>
>>>>>      Let 'hg rebase' avoid content-divergence when obsolete changesets
>>>>> are
>>>>>      present in the rebase set by skipping possible sources of
>>>>> divergence.
>>>>>
>>>>
>>>>
>>> Similar to what I said above, I think it would help to mention descendants
>>> here. While "possible sources of divergence" is vague enough to be
>>> correct,
>>> it would be helpful if we could clarify it.
>>>
>>>
>>>>
>>>>>
>>>>
>>> Just to make sure, the stuff below is not part of the release notes, is
>>> it?
>>> I'm not familiar with the syntax, so I can't tell (and I'm too lazy to
>>> test
>>> it).
>>>
>>
>> The directive block only includes indented lines, so by de-denting the
>> text below, it will not be in the release note.
>>
>>
>> We extend _computeobsoletenotrebased() so that it also return a set of
>>>>
>>>>> obsolete changesets in rebaseset without a successor in destination.
>>>>> This 'obsoletewithoutsuccessorindestination' is then stored as an
>>>>> attribute of rebaseruntime and used in _performrebasesubset() to:
>>>>>
>>>>> * filter out descendants of these changesets from the revisions to
>>>>>     rebase;
>>>>> * issue a message about these revisions being skipped.
>>>>>
>>>>> This only occurs if 'evolution.allowdivergence' option is off and
>>>>> 'rebaseskipobsolete' is on.
>>>>>
>>>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>>>> --- a/hgext/rebase.py
>>>>> +++ b/hgext/rebase.py
>>>>> @@ -179,6 +179,7 @@ class rebaseruntime(object):
>>>>>            # other extensions
>>>>>            self.keepopen = opts.get('keepopen', False)
>>>>>            self.obsoletenotrebased = {}
>>>>> +        self.obsoletewithoutsuccessorindestination = set()
>>>>>
>>>>>        @property
>>>>>        def repo(self):
>>>>> @@ -311,9 +312,10 @@ class rebaseruntime(object):
>>>>>            if not self.ui.configbool('experimental',
>>>>>
>>>> 'rebaseskipobsolete'):
>>>>
>>>>>                return
>>>>>            obsoleteset = set(obsoleterevs)
>>>>> -        self.obsoletenotrebased = _computeobsoletenotrebased(sel
>>>>> f.repo,
>>>>> -                                    obsoleteset, destmap)
>>>>> +        self.obsoletenotrebased, self.obsoletewithoutsuccessori
>>>>> ndestination
>>>>>
>>>> = \
>>>>
>>>>> +            _computeobsoletenotrebased(self.repo, obsoleteset,
>>>>> destmap)
>>>>>            skippedset = set(self.obsoletenotrebased)
>>>>> +        skippedset.update(self.obsoletewithoutsuccessorindestination)
>>>>>            _checkobsrebase(self.repo, self.ui, obsoleteset, skippedset)
>>>>>
>>>>>        def _prepareabortorcontinue(self, isabort):
>>>>> @@ -419,12 +421,26 @@ class rebaseruntime(object):
>>>>>        def _performrebasesubset(self, tr, subset, pos, total):
>>>>>            repo, ui, opts = self.repo, self.ui, self.opts
>>>>>            sortedrevs = repo.revs('sort(%ld, -topo)', subset)
>>>>> +        allowdivergence = self.ui.configbool(
>>>>> +            'experimental', 'evolution.allowdivergence')
>>>>> +        if not allowdivergence:
>>>>> +            sortedrevs -= repo.revs(
>>>>> +                'descendants(%ld) and not %ld',
>>>>> +                self.obsoletewithoutsuccessorindestination,
>>>>> +                self.obsoletewithoutsuccessorindestination,
>>>>> +            )
>>>>>            for rev in sortedrevs:
>>>>>                dest = self.destmap[rev]
>>>>>                ctx = repo[rev]
>>>>>                desc = _ctxdesc(ctx)
>>>>>                if self.state[rev] == rev:
>>>>>                    ui.status(_('already rebased %s\n') % desc)
>>>>> +            elif (not allowdivergence
>>>>> +                  and rev in self.obsoletewithoutsuccessori
>>>>>
>>>> ndestination):
>>>>
>>>>> +                msg = _('note: not rebasing %s and its descendants as '
>>>>> +                        'this would cause divergences\n') % desc
>>>>> +                repo.ui.status(msg)
>>>>> +                self.skipped.add(rev)
>>>>>                elif rev in self.obsoletenotrebased:
>>>>>                    succ = self.obsoletenotrebased[rev]
>>>>>                    if succ is None:
>>>>> @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
>>>>>        return set(r for r in revs if repo[r].obsolete())
>>>>>
>>>>>    def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
>>>>> -    """return a mapping obsolete => successor for all obsolete nodes to
>>>>>
>>>> be
>>>>
>>>>> -    rebased that have a successors in the destination
>>>>> +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindest
>>>>>
>>>> ination).
>>>>
>>>>> +
>>>>> +    `obsoletenotrebased` is a mapping mapping obsolete => successor for
>>>>>
>>>> all
>>>>
>>>>> +    obsolete nodes to be rebased given in `rebaseobsrevs`.
>>>>>
>>>>> -    obsolete => None entries in the mapping indicate nodes with no
>>>>>
>>>> successor"""
>>>>
>>>>> +    `obsoletewithoutsuccessorindestination` is a set with obsolete
>>>>>
>>>> revisions
>>>>
>>>>> +    without a successor in destination.
>>>>> +    """
>>>>>        obsoletenotrebased = {}
>>>>> +    obsoletewithoutsuccessorindestination = set([])
>>>>>
>>>>>        assert repo.filtername is None
>>>>>        cl = repo.changelog
>>>>> @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
>>>>>                    if cl.isancestor(succnode, destnode):
>>>>>                        obsoletenotrebased[srcrev] = nodemap[succnode]
>>>>>                        break
>>>>> +            else:
>>>>> +                # Unless 'srcrev' only has one successor and it's not
>>>>> in
>>>>> +                # rebaseset, we might skip it and its descendants as it
>>>>>
>>>> has no
>>>>
>>>>> +                # successor in destination. Otherwise, we'll issue the
>>>>> +                # divergence warning and abort.
>>>>>
>>>>
> Hmm, that actually doesn't sound like "successor in destination", it sounds
> like "no successor in rebase set". It sounds like it'll miss this case:
> 
> $ hg hg rebase -b C -d E
> 
> o E
> |
> o B''
> |
> | o D
> | |
> | x B'
> | |
> | | o C
> | | |
> | | x B
> | |/
> | o A
> |/
> o X
> 
> I think it would be nice to *not* skip C or D in this case (IIUC, C would
> be skipped by this patch). Maybe it's enough to check only the immediate
> visible successors? I.e. using the "closest=True" thing that I think Boris
> added recently.
> 

This case appears to be handled independently of this patch (because b
has a successor in destination b''). So I get:

   o  10 d
   |
   | o  9 c
   |/
   o  8 a
   |
   | x  7 d (rewritten using rebase as 10)
   | |
   | | x  6 c (rewritten using rebase as 9)
   | | |
   o | |  5 e
   | | |
   | x |  4 b'
   | | |
   | | x  3 b
   | |/
   o |  2 b''
   | |
   | x  1 a (rewritten using rebase as 8)
   |/
   o  0 x

> 
>> +                if (len(successors) > 1
>>>>>
>>>>
>>>>
>>> This line doesn't seem necessary, but feel free to keep if you think it
>>> helps readability (I find it slightly distracting).
>>>
>>
>> You're right, it's not needed.
>>
>>
>>
>>>
>>>> +                        and any(nodemap[s] in destmap
>>>>
>>>>> +                                for s in successors if s != srcnode)):
>>>>> +                    obsoletewithoutsuccessorindestination.add(srcrev)
>>>>>
>>>>
> This does not seem to check the "only has one successor" that you
> described. I'm not sure what the reason for that condition is, so I don't
> know if the code or the description should be fixed (or if I just
> misunderstood). I saw some comment suggesting that we don't care much about
> splits yet, so I suppose the "only has one successor" is not about that?

Sorry, the comment is wrong (and the "if len(successors) > 1" is
useless). I'd rephrase it as: "If 'srcrev' has a successor in rebase set
but none in destination, we shall skip it and its descendants to avoid
divergence".
The check for "no successor in destination" is done through the "break"
in the loop above (and if the loop does not break, we enter the else
clause and check if there's a successor in rebase set).
The case for "a successor in rebase set but none in destination" still
produces the divergence warning and aborts (this is tested below the "If
the rebase set has an obsolete (d) with..." comment; though this one is
also wrongly talking about "unique successor).



>>>>> -    return obsoletenotrebased
>>>>> +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
>>>>>
>>>>>    def summaryhook(ui, repo):
>>>>>        if not repo.vfs.exists('rebasestate'):
>>>>> diff --git a/tests/test-rebase-obsolete.t b/tests/test-rebase-obsolete.t
>>>>> --- a/tests/test-rebase-obsolete.t
>>>>> +++ b/tests/test-rebase-obsolete.t
>>>>> @@ -987,6 +987,208 @@ Create the changes that we will rebase
>>>>>      rebasing 21:7bdc8a87673d "dummy change" (tip)
>>>>>      $ cd ..
>>>>>
>>>>> +Divergence cases due to obsolete changesets
>>>>> +-------------------------------------------
>>>>> +
>>>>> +We should ignore branches with unstable changesets when they are based
>>>>>
>>>> on an
>>>>
>>>>> +obsolete changeset which successor is in rebase set.
>>>>> +
>>>>> +  $ hg init divergence
>>>>> +  $ cd divergence
>>>>> +  $ cat >> .hg/hgrc << EOF
>>>>> +  > [extensions]
>>>>> +  > strip =
>>>>> +  > [alias]
>>>>> +  > strip = strip --no-backup --quiet
>>>>> +  > [templates]
>>>>> +  > instabilities = '{rev}:{node|short} {desc|firstline}{if(instabilit
>>>>> ies,"
>>>>>
>>>> ({instabilities})")}\n'
>>>>
>>>>> +  > EOF
>>>>> +
>>>>> +  $ hg debugdrawdag <<EOF
>>>>> +  >   e   f
>>>>> +  >   |   |
>>>>> +  >   d'  d # replace: d -> d'
>>>>> +  >    \ /
>>>>> +  >     c
>>>>> +  >     |
>>>>> +  >   x b
>>>>> +  >    \|
>>>>> +  >     a
>>>>> +  > EOF
>>>>> +  $ hg log -G -r 'a'::
>>>>> +  o  7:1143e9adc121 f
>>>>> +  |
>>>>> +  | o  6:d60ebfa0f1cb e
>>>>> +  | |
>>>>> +  | o  5:027ad6c5830d d'
>>>>> +  | |
>>>>> +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>>> +  |/
>>>>> +  o  3:a82ac2b38757 c
>>>>> +  |
>>>>> +  | o  2:630d7c95eff7 x
>>>>> +  | |
>>>>> +  o |  1:488e1b7e7341 b
>>>>> +  |/
>>>>> +  o  0:b173517d0057 a
>>>>> +
>>>>> +
>>>>> +Changeset d and its descendants are excluded to avoid divergence of d,
>>>>>
>>>> which
>>>>
>>>>> +would occur because the successor of d (d') is also in rebaseset. As a
>>>>> +consequence f (descendant of d) is left behind.
>>>>> +
>>>>> +  $ hg rebase -b 'e' -d 'x'
>>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>> +  rebasing 5:027ad6c5830d "d'" (d')
>>>>> +  rebasing 6:d60ebfa0f1cb "e" (e)
>>>>> +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as this
>>>>>
>>>> would cause divergences
>>>>
>>>>
>>> The plural "divergences" sounds wrong to me, but I'm not sure. Augie (or
>>> others), since you're a native English speaker, do you think it should be
>>> singular or plural?
>>>
>>
>> It feels strange to me as well. I took it from the existing message
>> "this rebase will cause divergences from: <obsolete>" (which, maybe, was
>> also written by a non-native).
>>
>>
>>
>>> +  $ hg log -G -r 'a'::
>>>>> +  o  11:eb6d63fc4ed5 e
>>>>> +  |
>>>>> +  o  10:44d8c724a70c d'
>>>>> +  |
>>>>> +  o  9:d008e6b4d3fd c
>>>>> +  |
>>>>> +  o  8:67e8f4a16c49 b
>>>>> +  |
>>>>> +  | o  7:1143e9adc121 f
>>>>> +  | |
>>>>> +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
>>>>> +  | | |
>>>>> +  | | x  5:027ad6c5830d d' (rewritten using rebase as 10:44d8c724a70c)
>>>>> +  | | |
>>>>> +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>>> +  | |/
>>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
>>>>> +  | |
>>>>> +  o |  2:630d7c95eff7 x
>>>>> +  | |
>>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
>>>>> +  |/
>>>>> +  o  0:b173517d0057 a
>>>>> +
>>>>> +  $ hg strip -r 8:
>>>>> +
>>>>> +If the rebase set has an obsolete (d) with a unique successor (d')
>>>>>
>>>> outside the
>>>>
>>>>> +rebase set and not in destination, we still get the divergence warning.
>>>>>
>>>> By
>>>>
>>>>> +allowing divergence, we can perform the rebase.
>>>>> +
>>>>> +  $ hg rebase -r 'c'::'f' -d 'x'
>>>>> +  abort: this rebase will cause divergences from: 76be324c128b
>>>>> +  (to force the rebase please set experimental.evolution.allowdi
>>>>>
>>>> vergence=True)
>>>>
>>>>> +  [255]
>>>>> +  $ hg rebase --config experimental.evolution.allowdivergence=true -r
>>>>>
>>>> 'c'::'f' -d 'x'
>>>>
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>>> +  $ hg log -G -r 'a':: -T instabilities
>>>>> +  o  10:e1744ea07510 f
>>>>> +  |
>>>>> +  o  9:e2b36ea9a0a0 d (content-divergent)
>>>>> +  |
>>>>> +  o  8:6a0376de376e c
>>>>> +  |
>>>>> +  | x  7:1143e9adc121 f
>>>>> +  | |
>>>>> +  | | o  6:d60ebfa0f1cb e (orphan)
>>>>> +  | | |
>>>>> +  | | o  5:027ad6c5830d d' (orphan content-divergent)
>>>>> +  | | |
>>>>> +  | x |  4:76be324c128b d
>>>>> +  | |/
>>>>> +  | x  3:a82ac2b38757 c
>>>>> +  | |
>>>>> +  o |  2:630d7c95eff7 x
>>>>> +  | |
>>>>> +  | o  1:488e1b7e7341 b
>>>>> +  |/
>>>>> +  o  0:b173517d0057 a
>>>>> +
>>>>> +  $ hg strip -r 8:
>>>>> +
>>>>> +(Not skipping obsoletes means that divergence is allowed.)
>>>>> +
>>>>> +  $ hg rebase --config experimental.rebaseskipobsolete=false -r
>>>>>
>>>> 'c'::'f' -d 'x'
>>>>
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>>> +
>>>>> +  $ hg strip -r 0:
>>>>> +
>>>>> +Similar test on a more complex graph
>>>>> +
>>>>> +  $ hg debugdrawdag <<EOF
>>>>> +  >       g
>>>>> +  >       |
>>>>> +  >   f   e
>>>>> +  >   |   |
>>>>> +  >   e'  d # replace: e -> e'
>>>>> +  >    \ /
>>>>> +  >     c
>>>>> +  >     |
>>>>> +  >   x b
>>>>> +  >    \|
>>>>> +  >     a
>>>>> +  > EOF
>>>>> +  $ hg log -G -r 'a':
>>>>> +  o  8:2876ce66c6eb g
>>>>> +  |
>>>>> +  | o  7:3ffec603ab53 f
>>>>> +  | |
>>>>> +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>>> +  | |
>>>>> +  | o  5:63324dc512ea e'
>>>>> +  | |
>>>>> +  o |  4:76be324c128b d
>>>>> +  |/
>>>>> +  o  3:a82ac2b38757 c
>>>>> +  |
>>>>> +  | o  2:630d7c95eff7 x
>>>>> +  | |
>>>>> +  o |  1:488e1b7e7341 b
>>>>> +  |/
>>>>> +  o  0:b173517d0057 a
>>>>> +
>>>>> +  $ hg rebase -b 'f' -d 'x'
>>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>> +  rebasing 5:63324dc512ea "e'" (e')
>>>>> +  rebasing 7:3ffec603ab53 "f" (f)
>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>> +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as this
>>>>>
>>>> would cause divergences
>>>>
>>>>> +  $ hg log -G -r 'a':
>>>>> +  o  13:a1707a5b7c2c d
>>>>> +  |
>>>>> +  | o  12:ef6251596616 f
>>>>> +  | |
>>>>> +  | o  11:b6f172e64af9 e'
>>>>> +  |/
>>>>> +  o  10:d008e6b4d3fd c
>>>>> +  |
>>>>> +  o  9:67e8f4a16c49 b
>>>>> +  |
>>>>> +  | o  8:2876ce66c6eb g
>>>>> +  | |
>>>>> +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
>>>>> +  | | |
>>>>> +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>>> +  | | |
>>>>> +  | | x  5:63324dc512ea e' (rewritten using rebase as 11:b6f172e64af9)
>>>>> +  | | |
>>>>> +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
>>>>> +  | |/
>>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
>>>>> +  | |
>>>>> +  o |  2:630d7c95eff7 x
>>>>> +  | |
>>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
>>>>> +  |/
>>>>> +  o  0:b173517d0057 a
>>>>> +
>>>>> +
>>>>> +  $ cd ..
>>>>> +
>>>>>    Rebase merge where successor of one parent is equal to destination
>>>>>
>>>> (issue5198)
>>>>
>>>>>
>>>>>      $ hg init p1-succ-is-dest
>>>>> _______________________________________________
>>>>> Mercurial-devel mailing list
>>>>> Mercurial-devel@mercurial-scm.org
>>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>>>>
>>>>
>>>>
>>>
>>
>

On Tue, Nov 14, 2017 at 1:21 PM, Denis Laxalde <denis@laxalde.org> wrote:

> Martin von Zweigbergk a écrit :
>
>> On Tue, Nov 14, 2017 at 3:24 AM, Denis Laxalde <denis@laxalde.org> wrote:
>>
>> Martin von Zweigbergk a écrit :
>>>
>>> On Mon, Nov 13, 2017 at 3:07 PM, Augie Fackler <raf@durin42.com> wrote:
>>>>
>>>> On Sun, Nov 12, 2017 at 06:06:37PM +0100, Denis Laxalde wrote:
>>>>
>>>>>
>>>>> # HG changeset patch
>>>>>> # User Denis Laxalde <denis@laxalde.org>
>>>>>> # Date 1510489041 -3600
>>>>>> #      Sun Nov 12 13:17:21 2017 +0100
>>>>>> # Node ID 25d1891bd300bc527cb7e878532c8512ff5489de
>>>>>> # Parent  1d16e28225e01d2fac9f31ed058ae1f4c00bb9da
>>>>>> # EXP-Topic rebase-obsolete
>>>>>> rebase: exclude obsoletes without a successor in destination
>>>>>> (issue5300)
>>>>>>
>>>>>>
>>>>>
>>>>> If I read it right, this line should be something like "rebase: exclude
>>>> *descendants of* obsoletes without..."
>>>>
>>>>
>>> That's correct here and in the release note below.
>>>
>>>
>>>
>>>
>>>> I took patches 1-3. I think this is right. I'm 90% sure this is
>>>>> right. But I'm a little tired, and so I want Martin's more careful eye
>>>>> on it if he has time.
>>>>>
>>>>>
>>>>> Thanks for asking. I think this makes sense. Thanks for working on
>>>> this,
>>>> Denis!
>>>>
>>>> I think a general thing to note is that if a commit in the rebase set
>>>> has
>>>> become obsolete, then there's a conflict between the rebase invocation
>>>> and
>>>> the obsmarkers. The rebase command tells us to move the descendants onto
>>>> the rebase destination, while the obsmarkers tell us to move them onto
>>>> the
>>>> obsolete commit's successor. Since we don't keep track of some default
>>>> rebase destination (like git's upstream concept), any "hg rebase"
>>>> invocation is a one-off as far as hg is concerned, so I think it makes
>>>> sense to resolve the conflict in favor of the obsmarkers. That probably
>>>> didn't make much sense, so let me give an example:
>>>>
>>>> o B'
>>>> |
>>>> o O
>>>> |
>>>> | o U
>>>> | |
>>>> | | o C
>>>> | | |
>>>> | | x B
>>>> | |/
>>>> | .
>>>> |/
>>>> o A
>>>>
>>>> Let's say C's upstream was somehow configured to be U here, and that B
>>>> had
>>>> been rebased onto O instead. If the user then runs "hg evolve -r C", you
>>>> could argue that there would be a similar conflict between
>>>> rebase/upstream
>>>> that want to move C onto U and obsmarkers that want to move C onto B'.
>>>> However, since we don't have that feature, B' is the only natural
>>>> destination.
>>>>
>>>>
>>> I think I get this right, but I'm not sure if you actually expect me to
>>> change something (apart for other explicit comments, which I'll address)?
>>>
>>>
>> Not asking for any changes. I just thought I'd mention an aspect I had not
>> thought about before.
>>
>>
>> Also, the change proposed here handles cases where we cannot rebase C
>>> onto B' because both B and B' are part of the rebase set (and B' is not
>>> in destination).
>>>
>>>
>>>
>>>
>>>> In the following example, when trying to rebase 3:: onto 2, the rebase
>>>>>> will abort with "this rebase will cause divergence from: 4":
>>>>>>
>>>>>>       o  7 f
>>>>>>       |
>>>>>>       | o  6 e
>>>>>>       | |
>>>>>>       | o  5 d'
>>>>>>       | |
>>>>>>       x |  4 d (rewritten as 5)
>>>>>>       |/
>>>>>>       o  3 c
>>>>>>       |
>>>>>>       | o  2 x
>>>>>>       | |
>>>>>>       o |  1 b
>>>>>>       |/
>>>>>>       o  0 a
>>>>>>
>>>>>> By excluding obsolete changesets without a successor in destination (4
>>>>>> in the example above) and their descendants, we make rebase work in
>>>>>> this
>>>>>> case, thus giving:
>>>>>>
>>>>>>       o  11 e
>>>>>>       |
>>>>>>       o  10 d'
>>>>>>       |
>>>>>>       o  9 c
>>>>>>       |
>>>>>>       o  8 b
>>>>>>       |
>>>>>>       | o  7 f
>>>>>>       | |
>>>>>>       | | x  6 e (rewritten using rebase as 11)
>>>>>>       | | |
>>>>>>       | | x  5 d' (rewritten using rebase as 10)
>>>>>>       | | |
>>>>>>       | x |  4 d
>>>>>>       | |/
>>>>>>       | x  3 c (rewritten using rebase as 9)
>>>>>>       | |
>>>>>>       o |  2 x
>>>>>>       | |
>>>>>>       | x  1 b (rewritten using rebase as 8)
>>>>>>       |/
>>>>>>       o  0 a
>>>>>>
>>>>>> where branch 4:: is left behind while branch 5:: is rebased as
>>>>>> expected.
>>>>>>
>>>>>> The rationale is that users may not be interested in rebasing orphan
>>>>>> changesets when specifying a rebase set that include them but would
>>>>>> still want "stable" ones to be rebased. Currently, the user is
>>>>>> suggested
>>>>>> to allow divergence (but probably does not want it) or they must
>>>>>> specify
>>>>>> a rebase set excluding problematic changesets (which might be a bit
>>>>>> cumbersome). The approach proposed here corresponds to "Option 2" in
>>>>>> https://www.mercurial-scm.org/wiki/CEDRebase.
>>>>>>
>>>>>>
>>>>>
>>>>> I think I'd still want "Option 3", but I don't think this patch
>>>> prevents
>>>> that from happening later.
>>>>
>>>>
>>>>
>>>> .. feature::
>>>>>>
>>>>>>      Let 'hg rebase' avoid content-divergence when obsolete changesets
>>>>>> are
>>>>>>      present in the rebase set by skipping possible sources of
>>>>>> divergence.
>>>>>>
>>>>>>
>>>>>
>>>>> Similar to what I said above, I think it would help to mention
>>>> descendants
>>>> here. While "possible sources of divergence" is vague enough to be
>>>> correct,
>>>> it would be helpful if we could clarify it.
>>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>> Just to make sure, the stuff below is not part of the release notes, is
>>>> it?
>>>> I'm not familiar with the syntax, so I can't tell (and I'm too lazy to
>>>> test
>>>> it).
>>>>
>>>>
>>> The directive block only includes indented lines, so by de-denting the
>>> text below, it will not be in the release note.
>>>
>>>
>>> We extend _computeobsoletenotrebased() so that it also return a set of
>>>
>>>>
>>>>> obsolete changesets in rebaseset without a successor in destination.
>>>>>> This 'obsoletewithoutsuccessorindestination' is then stored as an
>>>>>> attribute of rebaseruntime and used in _performrebasesubset() to:
>>>>>>
>>>>>> * filter out descendants of these changesets from the revisions to
>>>>>>     rebase;
>>>>>> * issue a message about these revisions being skipped.
>>>>>>
>>>>>> This only occurs if 'evolution.allowdivergence' option is off and
>>>>>> 'rebaseskipobsolete' is on.
>>>>>>
>>>>>> diff --git a/hgext/rebase.py b/hgext/rebase.py
>>>>>> --- a/hgext/rebase.py
>>>>>> +++ b/hgext/rebase.py
>>>>>> @@ -179,6 +179,7 @@ class rebaseruntime(object):
>>>>>>            # other extensions
>>>>>>            self.keepopen = opts.get('keepopen', False)
>>>>>>            self.obsoletenotrebased = {}
>>>>>> +        self.obsoletewithoutsuccessorindestination = set()
>>>>>>
>>>>>>        @property
>>>>>>        def repo(self):
>>>>>> @@ -311,9 +312,10 @@ class rebaseruntime(object):
>>>>>>            if not self.ui.configbool('experimental',
>>>>>>
>>>>>> 'rebaseskipobsolete'):
>>>>>
>>>>>                return
>>>>>>            obsoleteset = set(obsoleterevs)
>>>>>> -        self.obsoletenotrebased = _computeobsoletenotrebased(sel
>>>>>> f.repo,
>>>>>> -                                    obsoleteset, destmap)
>>>>>> +        self.obsoletenotrebased, self.obsoletewithoutsuccessori
>>>>>> ndestination
>>>>>>
>>>>>> = \
>>>>>
>>>>> +            _computeobsoletenotrebased(self.repo, obsoleteset,
>>>>>> destmap)
>>>>>>            skippedset = set(self.obsoletenotrebased)
>>>>>> +        skippedset.update(self.obsoletewithoutsuccessorindestination
>>>>>> )
>>>>>>            _checkobsrebase(self.repo, self.ui, obsoleteset,
>>>>>> skippedset)
>>>>>>
>>>>>>        def _prepareabortorcontinue(self, isabort):
>>>>>> @@ -419,12 +421,26 @@ class rebaseruntime(object):
>>>>>>        def _performrebasesubset(self, tr, subset, pos, total):
>>>>>>            repo, ui, opts = self.repo, self.ui, self.opts
>>>>>>            sortedrevs = repo.revs('sort(%ld, -topo)', subset)
>>>>>> +        allowdivergence = self.ui.configbool(
>>>>>> +            'experimental', 'evolution.allowdivergence')
>>>>>> +        if not allowdivergence:
>>>>>> +            sortedrevs -= repo.revs(
>>>>>> +                'descendants(%ld) and not %ld',
>>>>>> +                self.obsoletewithoutsuccessorindestination,
>>>>>> +                self.obsoletewithoutsuccessorindestination,
>>>>>> +            )
>>>>>>            for rev in sortedrevs:
>>>>>>                dest = self.destmap[rev]
>>>>>>                ctx = repo[rev]
>>>>>>                desc = _ctxdesc(ctx)
>>>>>>                if self.state[rev] == rev:
>>>>>>                    ui.status(_('already rebased %s\n') % desc)
>>>>>> +            elif (not allowdivergence
>>>>>> +                  and rev in self.obsoletewithoutsuccessori
>>>>>>
>>>>>> ndestination):
>>>>>
>>>>> +                msg = _('note: not rebasing %s and its descendants as
>>>>>> '
>>>>>> +                        'this would cause divergences\n') % desc
>>>>>> +                repo.ui.status(msg)
>>>>>> +                self.skipped.add(rev)
>>>>>>                elif rev in self.obsoletenotrebased:
>>>>>>                    succ = self.obsoletenotrebased[rev]
>>>>>>                    if succ is None:
>>>>>> @@ -1616,11 +1632,16 @@ def _filterobsoleterevs(repo, revs):
>>>>>>        return set(r for r in revs if repo[r].obsolete())
>>>>>>
>>>>>>    def _computeobsoletenotrebased(repo, rebaseobsrevs, destmap):
>>>>>> -    """return a mapping obsolete => successor for all obsolete nodes
>>>>>> to
>>>>>>
>>>>>> be
>>>>>
>>>>> -    rebased that have a successors in the destination
>>>>>> +    """Return (obsoletenotrebased, obsoletewithoutsuccessorindest
>>>>>>
>>>>>> ination).
>>>>>
>>>>> +
>>>>>> +    `obsoletenotrebased` is a mapping mapping obsolete => successor
>>>>>> for
>>>>>>
>>>>>> all
>>>>>
>>>>> +    obsolete nodes to be rebased given in `rebaseobsrevs`.
>>>>>>
>>>>>> -    obsolete => None entries in the mapping indicate nodes with no
>>>>>>
>>>>>> successor"""
>>>>>
>>>>> +    `obsoletewithoutsuccessorindestination` is a set with obsolete
>>>>>>
>>>>>> revisions
>>>>>
>>>>> +    without a successor in destination.
>>>>>> +    """
>>>>>>        obsoletenotrebased = {}
>>>>>> +    obsoletewithoutsuccessorindestination = set([])
>>>>>>
>>>>>>        assert repo.filtername is None
>>>>>>        cl = repo.changelog
>>>>>> @@ -1641,8 +1662,17 @@ def _computeobsoletenotrebased(repo, reb
>>>>>>                    if cl.isancestor(succnode, destnode):
>>>>>>                        obsoletenotrebased[srcrev] = nodemap[succnode]
>>>>>>                        break
>>>>>> +            else:
>>>>>> +                # Unless 'srcrev' only has one successor and it's not
>>>>>> in
>>>>>> +                # rebaseset, we might skip it and its descendants as
>>>>>> it
>>>>>>
>>>>>> has no
>>>>>
>>>>> +                # successor in destination. Otherwise, we'll issue the
>>>>>> +                # divergence warning and abort.
>>>>>>
>>>>>>
>>>>> Hmm, that actually doesn't sound like "successor in destination", it
>> sounds
>> like "no successor in rebase set". It sounds like it'll miss this case:
>>
>> $ hg hg rebase -b C -d E
>>
>> o E
>> |
>> o B''
>> |
>> | o D
>> | |
>> | x B'
>> | |
>> | | o C
>> | | |
>> | | x B
>> | |/
>> | o A
>> |/
>> o X
>>
>> I think it would be nice to *not* skip C or D in this case (IIUC, C would
>> be skipped by this patch). Maybe it's enough to check only the immediate
>> visible successors? I.e. using the "closest=True" thing that I think Boris
>> added recently.
>>
>>
> This case appears to be handled independently of this patch (because b
> has a successor in destination b''). So I get:


Oh, right, that makes sense.


>
>
>   o  10 d
>   |
>   | o  9 c
>   |/
>   o  8 a
>   |
>   | x  7 d (rewritten using rebase as 10)
>   | |
>   | | x  6 c (rewritten using rebase as 9)
>   | | |
>   o | |  5 e
>   | | |
>   | x |  4 b'
>   | | |
>   | | x  3 b
>   | |/
>   o |  2 b''
>   | |
>   | x  1 a (rewritten using rebase as 8)
>   |/
>   o  0 x
>
>
>> +                if (len(successors) > 1
>>>
>>>>
>>>>>>
>>>>>
>>>>> This line doesn't seem necessary, but feel free to keep if you think it
>>>> helps readability (I find it slightly distracting).
>>>>
>>>>
>>> You're right, it's not needed.
>>>
>>>
>>>
>>>
>>>> +                        and any(nodemap[s] in destmap
>>>>>
>>>>> +                                for s in successors if s != srcnode)):
>>>>>> +                    obsoletewithoutsuccessorindest
>>>>>> ination.add(srcrev)
>>>>>>
>>>>>>
>>>>> This does not seem to check the "only has one successor" that you
>> described. I'm not sure what the reason for that condition is, so I don't
>> know if the code or the description should be fixed (or if I just
>> misunderstood). I saw some comment suggesting that we don't care much
>> about
>> splits yet, so I suppose the "only has one successor" is not about that?
>>
>
> Sorry, the comment is wrong (and the "if len(successors) > 1" is
> useless). I'd rephrase it as: "If 'srcrev' has a successor in rebase set
> but none in destination, we shall skip it and its descendants to avoid
> divergence".
> The check for "no successor in destination" is done through the "break"
> in the loop above (and if the loop does not break, we enter the else
> clause and check if there's a successor in rebase set).
> The case for "a successor in rebase set but none in destination" still
> produces the divergence warning and aborts (this is tested below the "If
> the rebase set has an obsolete (d) with..." comment; though this one is
> also wrongly talking about "unique successor).


Ah, it's both of those conditions ("not in destination" and "in rebase
set"). Thanks for explaining. Just update the commit message and the code
comments and this patch will look good to me then, I think. Thanks again
for working on this.


>
>
>
>
> -    return obsoletenotrebased
>>>>>> +    return obsoletenotrebased, obsoletewithoutsuccessorindestination
>>>>>>
>>>>>>    def summaryhook(ui, repo):
>>>>>>        if not repo.vfs.exists('rebasestate'):
>>>>>> diff --git a/tests/test-rebase-obsolete.t
>>>>>> b/tests/test-rebase-obsolete.t
>>>>>> --- a/tests/test-rebase-obsolete.t
>>>>>> +++ b/tests/test-rebase-obsolete.t
>>>>>> @@ -987,6 +987,208 @@ Create the changes that we will rebase
>>>>>>      rebasing 21:7bdc8a87673d "dummy change" (tip)
>>>>>>      $ cd ..
>>>>>>
>>>>>> +Divergence cases due to obsolete changesets
>>>>>> +-------------------------------------------
>>>>>> +
>>>>>> +We should ignore branches with unstable changesets when they are
>>>>>> based
>>>>>>
>>>>>> on an
>>>>>
>>>>> +obsolete changeset which successor is in rebase set.
>>>>>> +
>>>>>> +  $ hg init divergence
>>>>>> +  $ cd divergence
>>>>>> +  $ cat >> .hg/hgrc << EOF
>>>>>> +  > [extensions]
>>>>>> +  > strip =
>>>>>> +  > [alias]
>>>>>> +  > strip = strip --no-backup --quiet
>>>>>> +  > [templates]
>>>>>> +  > instabilities = '{rev}:{node|short}
>>>>>> {desc|firstline}{if(instabilit
>>>>>> ies,"
>>>>>>
>>>>>> ({instabilities})")}\n'
>>>>>
>>>>> +  > EOF
>>>>>> +
>>>>>> +  $ hg debugdrawdag <<EOF
>>>>>> +  >   e   f
>>>>>> +  >   |   |
>>>>>> +  >   d'  d # replace: d -> d'
>>>>>> +  >    \ /
>>>>>> +  >     c
>>>>>> +  >     |
>>>>>> +  >   x b
>>>>>> +  >    \|
>>>>>> +  >     a
>>>>>> +  > EOF
>>>>>> +  $ hg log -G -r 'a'::
>>>>>> +  o  7:1143e9adc121 f
>>>>>> +  |
>>>>>> +  | o  6:d60ebfa0f1cb e
>>>>>> +  | |
>>>>>> +  | o  5:027ad6c5830d d'
>>>>>> +  | |
>>>>>> +  x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>>>> +  |/
>>>>>> +  o  3:a82ac2b38757 c
>>>>>> +  |
>>>>>> +  | o  2:630d7c95eff7 x
>>>>>> +  | |
>>>>>> +  o |  1:488e1b7e7341 b
>>>>>> +  |/
>>>>>> +  o  0:b173517d0057 a
>>>>>> +
>>>>>> +
>>>>>> +Changeset d and its descendants are excluded to avoid divergence of
>>>>>> d,
>>>>>>
>>>>>> which
>>>>>
>>>>> +would occur because the successor of d (d') is also in rebaseset. As a
>>>>>> +consequence f (descendant of d) is left behind.
>>>>>> +
>>>>>> +  $ hg rebase -b 'e' -d 'x'
>>>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>>> +  rebasing 5:027ad6c5830d "d'" (d')
>>>>>> +  rebasing 6:d60ebfa0f1cb "e" (e)
>>>>>> +  note: not rebasing 4:76be324c128b "d" (d) and its descendants as
>>>>>> this
>>>>>>
>>>>>> would cause divergences
>>>>>
>>>>>
>>>>> The plural "divergences" sounds wrong to me, but I'm not sure. Augie
>>>> (or
>>>> others), since you're a native English speaker, do you think it should
>>>> be
>>>> singular or plural?
>>>>
>>>>
>>> It feels strange to me as well. I took it from the existing message
>>> "this rebase will cause divergences from: <obsolete>" (which, maybe, was
>>> also written by a non-native).
>>>
>>>
>>>
>>> +  $ hg log -G -r 'a'::
>>>>
>>>>> +  o  11:eb6d63fc4ed5 e
>>>>>> +  |
>>>>>> +  o  10:44d8c724a70c d'
>>>>>> +  |
>>>>>> +  o  9:d008e6b4d3fd c
>>>>>> +  |
>>>>>> +  o  8:67e8f4a16c49 b
>>>>>> +  |
>>>>>> +  | o  7:1143e9adc121 f
>>>>>> +  | |
>>>>>> +  | | x  6:d60ebfa0f1cb e (rewritten using rebase as 11:eb6d63fc4ed5)
>>>>>> +  | | |
>>>>>> +  | | x  5:027ad6c5830d d' (rewritten using rebase as
>>>>>> 10:44d8c724a70c)
>>>>>> +  | | |
>>>>>> +  | x |  4:76be324c128b d (rewritten using replace as 5:027ad6c5830d)
>>>>>> +  | |/
>>>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 9:d008e6b4d3fd)
>>>>>> +  | |
>>>>>> +  o |  2:630d7c95eff7 x
>>>>>> +  | |
>>>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 8:67e8f4a16c49)
>>>>>> +  |/
>>>>>> +  o  0:b173517d0057 a
>>>>>> +
>>>>>> +  $ hg strip -r 8:
>>>>>> +
>>>>>> +If the rebase set has an obsolete (d) with a unique successor (d')
>>>>>>
>>>>>> outside the
>>>>>
>>>>> +rebase set and not in destination, we still get the divergence
>>>>>> warning.
>>>>>>
>>>>>> By
>>>>>
>>>>> +allowing divergence, we can perform the rebase.
>>>>>> +
>>>>>> +  $ hg rebase -r 'c'::'f' -d 'x'
>>>>>> +  abort: this rebase will cause divergences from: 76be324c128b
>>>>>> +  (to force the rebase please set experimental.evolution.allowdi
>>>>>>
>>>>>> vergence=True)
>>>>>
>>>>> +  [255]
>>>>>> +  $ hg rebase --config experimental.evolution.allowdivergence=true
>>>>>> -r
>>>>>>
>>>>>> 'c'::'f' -d 'x'
>>>>>
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>>>> +  $ hg log -G -r 'a':: -T instabilities
>>>>>> +  o  10:e1744ea07510 f
>>>>>> +  |
>>>>>> +  o  9:e2b36ea9a0a0 d (content-divergent)
>>>>>> +  |
>>>>>> +  o  8:6a0376de376e c
>>>>>> +  |
>>>>>> +  | x  7:1143e9adc121 f
>>>>>> +  | |
>>>>>> +  | | o  6:d60ebfa0f1cb e (orphan)
>>>>>> +  | | |
>>>>>> +  | | o  5:027ad6c5830d d' (orphan content-divergent)
>>>>>> +  | | |
>>>>>> +  | x |  4:76be324c128b d
>>>>>> +  | |/
>>>>>> +  | x  3:a82ac2b38757 c
>>>>>> +  | |
>>>>>> +  o |  2:630d7c95eff7 x
>>>>>> +  | |
>>>>>> +  | o  1:488e1b7e7341 b
>>>>>> +  |/
>>>>>> +  o  0:b173517d0057 a
>>>>>> +
>>>>>> +  $ hg strip -r 8:
>>>>>> +
>>>>>> +(Not skipping obsoletes means that divergence is allowed.)
>>>>>> +
>>>>>> +  $ hg rebase --config experimental.rebaseskipobsolete=false -r
>>>>>>
>>>>>> 'c'::'f' -d 'x'
>>>>>
>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>>> +  rebasing 7:1143e9adc121 "f" (f tip)
>>>>>> +
>>>>>> +  $ hg strip -r 0:
>>>>>> +
>>>>>> +Similar test on a more complex graph
>>>>>> +
>>>>>> +  $ hg debugdrawdag <<EOF
>>>>>> +  >       g
>>>>>> +  >       |
>>>>>> +  >   f   e
>>>>>> +  >   |   |
>>>>>> +  >   e'  d # replace: e -> e'
>>>>>> +  >    \ /
>>>>>> +  >     c
>>>>>> +  >     |
>>>>>> +  >   x b
>>>>>> +  >    \|
>>>>>> +  >     a
>>>>>> +  > EOF
>>>>>> +  $ hg log -G -r 'a':
>>>>>> +  o  8:2876ce66c6eb g
>>>>>> +  |
>>>>>> +  | o  7:3ffec603ab53 f
>>>>>> +  | |
>>>>>> +  x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>>>> +  | |
>>>>>> +  | o  5:63324dc512ea e'
>>>>>> +  | |
>>>>>> +  o |  4:76be324c128b d
>>>>>> +  |/
>>>>>> +  o  3:a82ac2b38757 c
>>>>>> +  |
>>>>>> +  | o  2:630d7c95eff7 x
>>>>>> +  | |
>>>>>> +  o |  1:488e1b7e7341 b
>>>>>> +  |/
>>>>>> +  o  0:b173517d0057 a
>>>>>> +
>>>>>> +  $ hg rebase -b 'f' -d 'x'
>>>>>> +  rebasing 1:488e1b7e7341 "b" (b)
>>>>>> +  rebasing 3:a82ac2b38757 "c" (c)
>>>>>> +  rebasing 5:63324dc512ea "e'" (e')
>>>>>> +  rebasing 7:3ffec603ab53 "f" (f)
>>>>>> +  rebasing 4:76be324c128b "d" (d)
>>>>>> +  note: not rebasing 6:e36fae928aec "e" (e) and its descendants as
>>>>>> this
>>>>>>
>>>>>> would cause divergences
>>>>>
>>>>> +  $ hg log -G -r 'a':
>>>>>> +  o  13:a1707a5b7c2c d
>>>>>> +  |
>>>>>> +  | o  12:ef6251596616 f
>>>>>> +  | |
>>>>>> +  | o  11:b6f172e64af9 e'
>>>>>> +  |/
>>>>>> +  o  10:d008e6b4d3fd c
>>>>>> +  |
>>>>>> +  o  9:67e8f4a16c49 b
>>>>>> +  |
>>>>>> +  | o  8:2876ce66c6eb g
>>>>>> +  | |
>>>>>> +  | | x  7:3ffec603ab53 f (rewritten using rebase as 12:ef6251596616)
>>>>>> +  | | |
>>>>>> +  | x |  6:e36fae928aec e (rewritten using replace as 5:63324dc512ea)
>>>>>> +  | | |
>>>>>> +  | | x  5:63324dc512ea e' (rewritten using rebase as
>>>>>> 11:b6f172e64af9)
>>>>>> +  | | |
>>>>>> +  | x |  4:76be324c128b d (rewritten using rebase as 13:a1707a5b7c2c)
>>>>>> +  | |/
>>>>>> +  | x  3:a82ac2b38757 c (rewritten using rebase as 10:d008e6b4d3fd)
>>>>>> +  | |
>>>>>> +  o |  2:630d7c95eff7 x
>>>>>> +  | |
>>>>>> +  | x  1:488e1b7e7341 b (rewritten using rebase as 9:67e8f4a16c49)
>>>>>> +  |/
>>>>>> +  o  0:b173517d0057 a
>>>>>> +
>>>>>> +
>>>>>> +  $ cd ..
>>>>>> +
>>>>>>    Rebase merge where successor of one parent is equal to destination
>>>>>>
>>>>>> (issue5198)
>>>>>
>>>>>
>>>>>>      $ hg init p1-succ-is-dest
>>>>>> _______________________________________________
>>>>>> Mercurial-devel mailing list
>>>>>> Mercurial-devel@mercurial-scm.org
>>>>>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>