- support the case where a mergepoint has a branchpoint immediately

following it; this will be the norm in the case where parallel
branches refer to each other as dependencies.

this means we need to limit for from/to revisions based on current
heads / ancestors of those heads whenever we merge/unmerge.

2 files changed, 238 insertions, 40 deletions

diff --git a/alembic/migration.py b/alembic/migration.py
index 7d509af..7d854ba 100644
--- a/alembic/migration.py
+++ b/alembic/migration.py
@@ -472,7 +472,7 @@ class HeadMaintainer(object):
         elif step.should_merge_branches(self.heads):
             # delete revs, update from rev, update to rev
             (delete_revs, update_from_rev,
-             update_to_rev) = step.merge_branch_idents
+             update_to_rev) = step.merge_branch_idents(self.heads)
             log.debug(
                 "merge, delete %s, update %s to %s",
                 delete_revs, update_from_rev, update_to_rev)
@@ -481,7 +481,7 @@ class HeadMaintainer(object):
             self._update_version(update_from_rev, update_to_rev)
         elif step.should_unmerge_branches(self.heads):
             (update_from_rev, update_to_rev,
-             insert_revs) = step.unmerge_branch_idents
+             insert_revs) = step.unmerge_branch_idents(self.heads)
             log.debug(
                 "unmerge, insert %s, update %s to %s",
                 insert_revs, update_from_rev, update_to_rev)
@@ -489,7 +489,7 @@ class HeadMaintainer(object):
                 self._insert_version(insrev)
             self._update_version(update_from_rev, update_to_rev)
         else:
-            from_, to_ = step.update_version_num
+            from_, to_ = step.update_version_num(self.heads)
             log.debug("update %s to %s", from_, to_)
             self._update_version(from_, to_)
 
@@ -512,22 +512,6 @@ class MigrationStep(object):
         return not self.is_upgrade
 
     @property
-    def merge_branch_idents(self):
-        return (
-            # delete revs, update from rev, update to rev
-            list(self.from_revisions[0:-1]), self.from_revisions[-1],
-            self.to_revisions[0]
-        )
-
-    @property
-    def unmerge_branch_idents(self):
-        return (
-            # update from rev, update to rev, insert revs
-            self.from_revisions[0], self.to_revisions[-1],
-            list(self.to_revisions[0:-1])
-        )
-
-    @property
     def short_log(self):
         return "%s %s -> %s" % (
             self.name,
@@ -632,6 +616,48 @@ class RevisionStep(MigrationStep):
             # is a merge point
             return False
 
+    def merge_branch_idents(self, heads):
+        other_heads = set(heads).difference(self.from_revisions)
+
+        if other_heads:
+            ancestors = set(
+                r.revision for r in
+                self.revision_map._get_ancestor_nodes(
+                    self.revision_map.get_revisions(other_heads),
+                    check=False
+                )
+            )
+            from_revisions = list(
+                set(self.from_revisions).difference(ancestors))
+        else:
+            from_revisions = list(self.from_revisions)
+
+        return (
+            # delete revs, update from rev, update to rev
+            list(from_revisions[0:-1]), from_revisions[-1],
+            self.to_revisions[0]
+        )
+
+    def unmerge_branch_idents(self, heads):
+        other_heads = set(heads).difference([self.revision.revision])
+        if other_heads:
+            ancestors = set(
+                r.revision for r in
+                self.revision_map._get_ancestor_nodes(
+                    self.revision_map.get_revisions(other_heads),
+                    check=False
+                )
+            )
+            to_revisions = list(set(self.to_revisions).difference(ancestors))
+        else:
+            to_revisions = self.to_revisions
+
+        return (
+            # update from rev, update to rev, insert revs
+            self.from_revisions[0], to_revisions[-1],
+            to_revisions[0:-1]
+        )
+
     def should_create_branch(self, heads):
         if not self.is_upgrade:
             return False
@@ -673,13 +699,19 @@ class RevisionStep(MigrationStep):
 
         return False
 
-    @property
-    def update_version_num(self):
-        assert self._has_scalar_down_revision
+    def update_version_num(self, heads):
+        if not self._has_scalar_down_revision:
+            downrev = heads.intersection(self.revision._down_revision_tuple)
+            assert len(downrev) == 1, \
+                "Can't do an UPDATE because downrevision is ambiguous"
+            down_revision = list(downrev)[0]
+        else:
+            down_revision = self.revision.down_revision
+
         if self.is_upgrade:
-            return self.revision.down_revision, self.revision.revision
+            return down_revision, self.revision.revision
         else:
-            return self.revision.revision, self.revision.down_revision
+            return self.revision.revision, down_revision
 
     @property
     def delete_version_num(self):
@@ -728,12 +760,25 @@ class StampStep(MigrationStep):
         assert len(self.to_) == 1
         return self.to_[0]
 
-    @property
-    def update_version_num(self):
+    def update_version_num(self, heads):
         assert len(self.from_) == 1
         assert len(self.to_) == 1
         return self.from_[0], self.to_[0]
 
+    def merge_branch_idents(self, heads):
+        return (
+            # delete revs, update from rev, update to rev
+            list(self.from_[0:-1]), self.from_[-1],
+            self.to_[0]
+        )
+
+    def unmerge_branch_idents(self, heads):
+        return (
+            # update from rev, update to rev, insert revs
+            self.from_[0], self.to_[-1],
+            list(self.to_[0:-1])
+        )
+
     def should_delete_branch(self, heads):
         return self.is_downgrade and self.branch_move
 
diff --git a/tests/test_version_traversal.py b/tests/test_version_traversal.py
index 8fcf8e9..77aa516 100644
--- a/tests/test_version_traversal.py
+++ b/tests/test_version_traversal.py
@@ -206,11 +206,12 @@ class BranchedPathTest(MigrationTest):
         a, b, c1, d1, c2, d2 = (
             self.a, self.b, self.c1, self.d1, self.c2, self.d2
         )
+        heads = [self.d1.revision, self.c2.revision]
         revs = self.env._stamp_revs(
-            self.b.revision, [self.d1.revision, self.c2.revision])
+            self.b.revision, heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # DELETE d1 revision, UPDATE c2 to b
             ([self.d1.revision], self.c2.revision, self.b.revision)
         )
@@ -229,11 +230,12 @@ class BranchedPathTest(MigrationTest):
         a, b, c1, d1, c2, d2 = (
             self.a, self.b, self.c1, self.d1, self.c2, self.d2
         )
+        heads = [self.d1.revision, self.c2.revision]
         revs = self.env._stamp_revs(
-            "c2branch@head", [self.d1.revision, self.c2.revision])
+            "c2branch@head", heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # the c1branch remains unchanged
             ([], self.c2.revision, self.d2.revision)
         )
@@ -274,6 +276,153 @@ class BranchedPathTest(MigrationTest):
         )
 
 
+class BranchFromMergepointTest(MigrationTest):
+    """this is a form that will come up frequently in the
+    "many independent roots with cross-dependencies" case.
+
+    """
+
+    @classmethod
+    def setup_class(cls):
+        cls.env = env = staging_env()
+        cls.a1 = env.generate_revision(util.rev_id(), '->a1', refresh=True)
+        cls.b1 = env.generate_revision(util.rev_id(), 'a1->b1', refresh=True)
+        cls.c1 = env.generate_revision(util.rev_id(), 'b1->c1', refresh=True)
+
+        cls.a2 = env.generate_revision(
+            util.rev_id(), '->a2', head=(),
+            refresh=True)
+        cls.b2 = env.generate_revision(
+            util.rev_id(), 'a2->b2', head=cls.a2.revision, refresh=True)
+        cls.c2 = env.generate_revision(
+            util.rev_id(), 'b2->c2', head=cls.b2.revision, refresh=True)
+
+        # mergepoint between c1, c2
+        # d1 dependent on c2
+        cls.d1 = env.generate_revision(
+            util.rev_id(), 'd1', head=(cls.c1.revision, cls.c2.revision),
+            refresh=True)
+
+        # but then c2 keeps going into d2
+        cls.d2 = env.generate_revision(
+            util.rev_id(), 'd2', head=cls.c2.revision,
+            refresh=True, splice=True)
+
+    def test_mergepoint_to_only_one_side_upgrade(self):
+        a1, b1, c1, a2, b2, c2, d1, d2 = (
+            self.a1, self.b1, self.c1, self.a2, self.b2, self.c2,
+            self.d1, self.d2
+        )
+
+        self._assert_upgrade(
+            d1.revision, (d2.revision, b1.revision),
+            [self.up_(c1), self.up_(d1)],
+            set([d2.revision, d1.revision])
+        )
+
+    def test_mergepoint_to_only_one_side_downgrade(self):
+        a1, b1, c1, a2, b2, c2, d1, d2 = (
+            self.a1, self.b1, self.c1, self.a2, self.b2, self.c2,
+            self.d1, self.d2
+        )
+
+        self._assert_downgrade(
+            b1.revision, (d2.revision, d1.revision),
+            [self.down_(d1), self.down_(c1)],
+            set([d2.revision, b1.revision])
+        )
+
+class BranchFrom3WayMergepointTest(MigrationTest):
+    """this is a form that will come up frequently in the
+    "many independent roots with cross-dependencies" case.
+
+    """
+
+    @classmethod
+    def setup_class(cls):
+        cls.env = env = staging_env()
+        cls.a1 = env.generate_revision(util.rev_id(), '->a1', refresh=True)
+        cls.b1 = env.generate_revision(util.rev_id(), 'a1->b1', refresh=True)
+        cls.c1 = env.generate_revision(util.rev_id(), 'b1->c1', refresh=True)
+
+        cls.a2 = env.generate_revision(
+            util.rev_id(), '->a2', head=(),
+            refresh=True)
+        cls.b2 = env.generate_revision(
+            util.rev_id(), 'a2->b2', head=cls.a2.revision, refresh=True)
+        cls.c2 = env.generate_revision(
+            util.rev_id(), 'b2->c2', head=cls.b2.revision, refresh=True)
+
+        cls.a3 = env.generate_revision(
+            util.rev_id(), '->a3', head=(),
+            refresh=True)
+        cls.b3 = env.generate_revision(
+            util.rev_id(), 'a3->b3', head=cls.a3.revision, refresh=True)
+        cls.c3 = env.generate_revision(
+            util.rev_id(), 'b3->c3', head=cls.b3.revision, refresh=True)
+
+        # mergepoint between c1, c2, c3
+        # d1 dependent on c2, c3
+        cls.d1 = env.generate_revision(
+            util.rev_id(), 'd1', head=(
+                cls.c1.revision, cls.c2.revision, cls.c3.revision),
+            refresh=True)
+
+        # but then c2 keeps going into d2
+        cls.d2 = env.generate_revision(
+            util.rev_id(), 'd2', head=cls.c2.revision,
+            refresh=True, splice=True)
+
+        # c3 keeps going into d3
+        cls.d3 = env.generate_revision(
+            util.rev_id(), 'd3', head=cls.c3.revision,
+            refresh=True, splice=True)
+
+    def test_mergepoint_to_only_one_side_upgrade(self):
+        a1, b1, c1, a2, b2, c2, d1, d2, a3, b3, c3, d3 = (
+            self.a1, self.b1, self.c1, self.a2, self.b2, self.c2,
+            self.d1, self.d2, self.a3, self.b3, self.c3, self.d3
+        )
+
+        self._assert_upgrade(
+            d1.revision, (d3.revision, d2.revision, b1.revision),
+            [self.up_(c1), self.up_(d1)],
+            set([d3.revision, d2.revision, d1.revision])
+        )
+
+    def test_mergepoint_to_only_one_side_downgrade(self):
+        a1, b1, c1, a2, b2, c2, d1, d2, a3, b3, c3, d3 = (
+            self.a1, self.b1, self.c1, self.a2, self.b2, self.c2,
+            self.d1, self.d2, self.a3, self.b3, self.c3, self.d3
+        )
+
+        self._assert_downgrade(
+            b1.revision, (d3.revision, d2.revision, d1.revision),
+            [self.down_(d1), self.down_(c1)],
+            set([d3.revision, d2.revision, b1.revision])
+        )
+
+    def test_mergepoint_to_two_sides_upgrade(self):
+        a1, b1, c1, a2, b2, c2, d1, d2, a3, b3, c3, d3 = (
+            self.a1, self.b1, self.c1, self.a2, self.b2, self.c2,
+            self.d1, self.d2, self.a3, self.b3, self.c3, self.d3
+        )
+
+        self._assert_upgrade(
+            d1.revision, (d3.revision, b2.revision, b1.revision),
+            [self.up_(c2), self.up_(c1), self.up_(d1)],
+            # this will merge b2 and b1 into d1
+            set([d3.revision, d1.revision])
+        )
+
+        # but then!  b2 will break out again if we keep going with it
+        self._assert_upgrade(
+            d2.revision, (d3.revision, d1.revision),
+            [self.up_(d2)],
+            set([d3.revision, d2.revision, d1.revision])
+        )
+
+
 class ForestTest(MigrationTest):
     @classmethod
     def setup_class(cls):
@@ -334,10 +483,11 @@ class MergedPathTest(MigrationTest):
             self.a, self.b, self.c1, self.d1, self.c2, self.d2,
             self.e, self.f
         )
-        revs = self.env._stamp_revs(self.c2.revision, [self.e.revision])
+        heads = [self.e.revision]
+        revs = self.env._stamp_revs(self.c2.revision, heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # no deletes, UPDATE e to c2
             ([], self.e.revision, self.c2.revision)
         )
@@ -347,10 +497,11 @@ class MergedPathTest(MigrationTest):
             self.a, self.b, self.c1, self.d1, self.c2, self.d2,
             self.e, self.f
         )
-        revs = self.env._stamp_revs(self.a.revision, [self.e.revision])
+        heads = [self.e.revision]
+        revs = self.env._stamp_revs(self.a.revision, heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # no deletes, UPDATE e to c2
             ([], self.e.revision, self.a.revision)
         )
@@ -363,7 +514,7 @@ class MergedPathTest(MigrationTest):
         revs = self.env._stamp_revs(self.e.revision, [self.c2.revision])
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents([self.c2.revision]),
             # no deletes, UPDATE e to c2
             ([], self.c2.revision, self.e.revision)
         )
@@ -380,7 +531,7 @@ class MergedPathTest(MigrationTest):
             "c2branch@head", [self.d1.revision, self.c2.revision])
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents([self.d1.revision, self.c2.revision]),
             # DELETE d1 revision, UPDATE c2 to e
             ([self.d1.revision], self.c2.revision, self.f.revision)
         )
@@ -390,11 +541,12 @@ class MergedPathTest(MigrationTest):
             self.a, self.b, self.c1, self.d1, self.c2, self.d2,
             self.e, self.f
         )
+        heads = [self.d1.revision, self.c2.revision]
         revs = self.env._stamp_revs(
-            self.e.revision, [self.d1.revision, self.c2.revision])
+            self.e.revision, heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # DELETE d1 revision, UPDATE c2 to e
             ([self.d1.revision], self.c2.revision, self.e.revision)
         )
@@ -404,10 +556,11 @@ class MergedPathTest(MigrationTest):
             self.a, self.b, self.c1, self.d1, self.c2, self.d2,
             self.e, self.f
         )
-        revs = self.env._stamp_revs(self.e.revision, [self.b.revision])
+        heads = [self.b.revision]
+        revs = self.env._stamp_revs(self.e.revision, heads)
         eq_(len(revs), 1)
         eq_(
-            revs[0].merge_branch_idents,
+            revs[0].merge_branch_idents(heads),
             # no deletes, UPDATE e to c2
             ([], self.b.revision, self.e.revision)
         )