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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Rebase works by updating to a commit and then grafting changes on
  top. However, before this patch, it would actually merge in changes
  instead of grafting them in in some cases. That is, it would use the
  common ancestor as base instead of using one of the parents. That
  seems wrong to me, so I'm changing it so `defineparents()` always
  returns a value for `base`.
 
  This fixes the bad behavior in test-rebase-newancestor.t, which was
  introduced in 65f215ea3e8e <https://phab.mercurial-scm.org/rHG65f215ea3e8e3be7bd13ac4b8f3b40568d12ec66> (tests: add test for rebasing merges with
  ancestors of the rebase destination, 2014-11-30).
 
  The difference in test-rebase-dest.t is because the files in the tip
  revision were A, D, E, F before this patch and A, D, F, G after it. I
  think both files should ideally be there.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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,24 +153,23 @@
   > 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
+  abort: rebasing 4:4b019212aaf6 will include unwanted changes from 1:1d1a643d390e
+  [255]
   $ hg tglog
-  o  7: de147e4f69cf 'dev: merge default'
+  @  8: 3e075b1c0a40 'dev: f-dev stuff'
+  |
+  o  7: e08089805d82 'default: f-other stuff'
   |
-  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  6: 010ced67e558 'dev: merge default' dev
+  |/|
+  o |  5: 462860db70a1 'default: remove f-default'
+  | |
+  | o  4: 4b019212aaf6 'dev: merge default' dev
+  |/|
+  o |  3: f157ecfd2b6b 'default: f-default stuff'
+  | |
+  | o  2: ec2c14fb2984 'dev: f-dev stuff' dev
+  | |
   | o  1: 1d1a643d390e 'dev: create branch' dev
   |/
   o  0: e90e8eb90b6f 'default: create f-default'
@@ -284,18 +273,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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
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

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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz added a comment.


  Reviewers: This patch is independent of D7827 <https://phab.mercurial-scm.org/D7827>, so please don't let that patch block this one.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz updated this revision to Diff 19699.

REPOSITORY
  rHG Mercurial

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

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

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
@@ -1678,22 +1678,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:
@@ -1754,10 +1738,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
@@ -1781,42 +1765,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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz updated this revision to Diff 19858.

REPOSITORY
  rHG Mercurial

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

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

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
@@ -1680,22 +1680,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:
@@ -1756,10 +1740,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
@@ -1783,42 +1767,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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
martinvonz updated this revision to Diff 20093.

REPOSITORY
  rHG Mercurial

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

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

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
@@ -1678,22 +1678,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:
@@ -1754,10 +1738,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
@@ -1781,42 +1765,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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

martinvonz (Martin von Zweigbergk)
In reply to this post by martinvonz (Martin von Zweigbergk)
Closed by commit rHG77bb38be00ea: rebase: always be graft-like, not merge-like, also for merges (authored by martinvonz).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

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

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

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
@@ -1678,22 +1678,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:
@@ -1754,10 +1738,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
@@ -1781,42 +1765,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))
 



To: martinvonz, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel