Patchwork D7907: rebase: always be graft-like, not merge-like, also for merges

login
register
mail settings
Submitter phabricator
Date Jan. 16, 2020, 6:24 p.m.
Message ID <bef94c2032d4e86f4057e18cffa3be8b@localhost.localdomain>
Download mbox | patch
Permalink /patch/44449/
State Not Applicable
Headers show

Comments

phabricator - Jan. 16, 2020, 6:24 p.m.
martinvonz updated this revision to Diff 19377.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7907?vs=19343&id=19377

BRANCH
  default

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7907/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7907

AFFECTED FILES
  hgext/rebase.py
  tests/test-rebase-dest.t
  tests/test-rebase-newancestor.t

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/test-rebase-newancestor.t b/tests/test-rebase-newancestor.t
--- a/tests/test-rebase-newancestor.t
+++ b/tests/test-rebase-newancestor.t
@@ -68,11 +68,6 @@ 
 that is mixed up with the actual merge stuff and there is in general no way to
 separate them.
 
-Note: The dev branch contains _no_ changes to f-default. It might be unclear
-how rebasing of ancestor merges should be handled, but the current behavior
-with spurious prompts for conflicts in files that didn't change seems very
-wrong.
-
   $ hg init ancestor-merge
   $ cd ancestor-merge
 
@@ -133,16 +128,11 @@ 
   note: not rebasing 1:1d1a643d390e "dev: create branch", its destination already has all its changes
   rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
   rebasing 4:4b019212aaf6 "dev: merge default"
-  file 'f-default' was deleted in local [dest] but was modified in other [source].
-  You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
-  What do you want to do? c
+  note: not rebasing 4:4b019212aaf6 "dev: merge default", its destination already has all its changes
   rebasing 6:010ced67e558 "dev: merge default"
+  note: not rebasing 6:010ced67e558 "dev: merge default", its destination already has all its changes
   saved backup bundle to $TESTTMP/ancestor-merge/.hg/strip-backup/1d1a643d390e-4a6f6d17-rebase.hg
   $ hg tglog
-  o  6: de147e4f69cf 'dev: merge default'
-  |
-  o  5: eda7b7f46f5d 'dev: merge default'
-  |
   o  4: 3e075b1c0a40 'dev: f-dev stuff'
   |
   @  3: e08089805d82 'default: f-other stuff'
@@ -163,28 +153,8 @@ 
   > EOF
   rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
   rebasing 4:4b019212aaf6 "dev: merge default"
-  file 'f-default' was deleted in local [dest] but was modified in other [source].
-  You can use (c)hanged version, leave (d)eleted, or leave (u)nresolved.
-  What do you want to do? c
-  rebasing 6:010ced67e558 "dev: merge default"
-  saved backup bundle to $TESTTMP/ancestor-merge-2/.hg/strip-backup/ec2c14fb2984-827d7a44-rebase.hg
-  $ hg tglog
-  o  7: de147e4f69cf 'dev: merge default'
-  |
-  o  6: eda7b7f46f5d 'dev: merge default'
-  |
-  o  5: 3e075b1c0a40 'dev: f-dev stuff'
-  |
-  o  4: e08089805d82 'default: f-other stuff'
-  |
-  o  3: 462860db70a1 'default: remove f-default'
-  |
-  o  2: f157ecfd2b6b 'default: f-default stuff'
-  |
-  | o  1: 1d1a643d390e 'dev: create branch' dev
-  |/
-  o  0: e90e8eb90b6f 'default: create f-default'
-  
+  abort: rebasing 4:4b019212aaf6 will include unwanted changes from 1:1d1a643d390e
+  [255]
   $ cd ..
 
 
@@ -284,18 +254,7 @@ 
   rebasing 6:4c5f12f25ebe "merge rebase ancestors" (tip)
   resolving manifests
   removing other
-  note: merging f9daf77ffe76+ and 4c5f12f25ebe using bids from ancestors a60552eb93fb and f59da8fc0fcf
-  
-  calculating bids for ancestor a60552eb93fb
   resolving manifests
-  
-  calculating bids for ancestor f59da8fc0fcf
-  resolving manifests
-  
-  auction for merging merge bids
-   other: consensus for g
-  end of auction
-  
   getting other
   committing files:
   other
diff --git a/tests/test-rebase-dest.t b/tests/test-rebase-dest.t
--- a/tests/test-rebase-dest.t
+++ b/tests/test-rebase-dest.t
@@ -256,7 +256,7 @@ 
   > EOS
   rebasing 3:a4256619d830 "B" (B)
   rebasing 6:8e139e245220 "C" (C tip)
-  o    8: 51e2ce92e06a C
+  o    8: d7d1169e9b1c C
   |\
   | o    7: 2ed0c8546285 B
   | |\
diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -1676,22 +1676,6 @@ 
             elif p in state and state[p] > 0:
                 np = state[p]
 
-            # "bases" only record "special" merge bases that cannot be
-            # calculated from changelog DAG (i.e. isancestor(p, np) is False).
-            # For example:
-            #
-            #   B'   # rebase -s B -d D, when B was rebased to B'. dest for C
-            #   | C  # is B', but merge base for C is B, instead of
-            #   D |  # changelog.ancestor(C, B') == A. If changelog DAG and
-            #   | B  # "state" edges are merged (so there will be an edge from
-            #   |/   # B to B'), the merge base is still ancestor(C, B') in
-            #   A    # the merged graph.
-            #
-            # Also see https://bz.mercurial-scm.org/show_bug.cgi?id=1950#c8
-            # which uses "virtual null merge" to explain this situation.
-            if isancestor(p, np):
-                bases[i] = nullrev
-
             # If one parent becomes an ancestor of the other, drop the ancestor
             for j, x in enumerate(newps[:i]):
                 if x == nullrev:
@@ -1752,10 +1736,10 @@ 
     # But our merge base candidates (D and E in above case) could still be
     # better than the default (ancestor(F, Z) == null). Therefore still
     # pick one (so choose p1 above).
-    if sum(1 for b in set(bases) if b != nullrev) > 1:
+    if sum(1 for b in set(bases) if b != nullrev and b not in newps) > 1:
         unwanted = [None, None]  # unwanted[i]: unwanted revs if choose bases[i]
         for i, base in enumerate(bases):
-            if base == nullrev:
+            if base == nullrev or base in newps:
                 continue
             # Revisions in the side (not chosen as merge base) branch that
             # might contain "surprising" contents
@@ -1779,42 +1763,40 @@ 
                     )
                 )
 
-        # Choose a merge base that has a minimal number of unwanted revs.
-        l, i = min(
-            (len(revs), i)
-            for i, revs in enumerate(unwanted)
-            if revs is not None
-        )
-
-        # The merge will include unwanted revisions. Abort now. Revisit this if
-        # we have a more advanced merge algorithm that handles multiple bases.
-        if l > 0:
-            unwanteddesc = _(b' or ').join(
-                (
-                    b', '.join(b'%d:%s' % (r, repo[r]) for r in revs)
-                    for revs in unwanted
-                    if revs is not None
-                )
-            )
-            raise error.Abort(
-                _(b'rebasing %d:%s will include unwanted changes from %s')
-                % (rev, repo[rev], unwanteddesc)
+        if any(revs is not None for revs in unwanted):
+            # Choose a merge base that has a minimal number of unwanted revs.
+            l, i = min(
+                (len(revs), i)
+                for i, revs in enumerate(unwanted)
+                if revs is not None
             )
 
-        # newps[0] should match merge base if possible. Currently, if newps[i]
-        # is nullrev, the only case is newps[i] and newps[j] (j < i), one is
-        # the other's ancestor. In that case, it's fine to not swap newps here.
-        # (see CASE-1 and CASE-2 above)
-        if i != 0:
-            if newps[i] != nullrev:
-                newps[0], newps[i] = newps[i], newps[0]
-            bases[0], bases[i] = bases[i], bases[0]
+            # The merge will include unwanted revisions. Abort now. Revisit this if
+            # we have a more advanced merge algorithm that handles multiple bases.
+            if l > 0:
+                unwanteddesc = _(b' or ').join(
+                    (
+                        b', '.join(b'%d:%s' % (r, repo[r]) for r in revs)
+                        for revs in unwanted
+                        if revs is not None
+                    )
+                )
+                raise error.Abort(
+                    _(b'rebasing %d:%s will include unwanted changes from %s')
+                    % (rev, repo[rev], unwanteddesc)
+                )
+
+            # newps[0] should match merge base if possible. Currently, if newps[i]
+            # is nullrev, the only case is newps[i] and newps[j] (j < i), one is
+            # the other's ancestor. In that case, it's fine to not swap newps here.
+            # (see CASE-1 and CASE-2 above)
+            if i != 0:
+                if newps[i] != nullrev:
+                    newps[0], newps[i] = newps[i], newps[0]
+                bases[0], bases[i] = bases[i], bases[0]
 
     # "rebasenode" updates to new p1, use the corresponding merge base.
-    if bases[0] != nullrev:
-        base = bases[0]
-    else:
-        base = None
+    base = bases[0]
 
     repo.ui.debug(b" future parents are %d and %d\n" % tuple(newps))