Handle pushing merges of which LHS parent is older revision of branch path.
authorJelmer Vernooij <jelmer@samba.org>
Sat, 15 Sep 2007 03:31:14 +0000 (05:31 +0200)
committerJelmer Vernooij <jelmer@samba.org>
Sat, 15 Sep 2007 03:31:14 +0000 (05:31 +0200)
NEWS
branchprops.py
commit.py
logwalker.py
repository.py
tests/test_checkout.py
tests/test_logwalker.py
tests/test_push.py

diff --git a/NEWS b/NEWS
index 3455452038ac47b9d32ad8e4af7e1a959ba746fc..5808973ba40b390299e841d46f5dbd19304bcedd 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,9 @@ bzr-svn 0.4.3 2007-09-12
 
    * Use write groups in fetch as required by the packs branch.
 
+   * Handle pushing merges of which LHS parent is older revision of 
+     branch path.
+
   INTERNALS
 
    * Track moving parents correctly in follow_path().
index 1e118d88a1e2dae46a2a4d622182646104f5b005..949c2895ac47120c79c137ef40f7a03411e4c506 100644 (file)
@@ -68,7 +68,8 @@ class BranchPropertyList:
         assert path is not None
         assert isinstance(path, str)
         assert isinstance(origrevnum, int) and origrevnum >= 0
-        revnum = self.log.find_latest_change(path, origrevnum)
+        revnum = self.log.find_latest_change(path, origrevnum, 
+                                             include_parents=True)
         assert revnum is not None, \
                 "can't find latest change for %r:%r" % (path, origrevnum)
 
index 44878db0d3a482775a84a031593289856cac39e0..751f9822ed7786a4c4c1a69df388288876d57ece 100644 (file)
--- a/commit.py
+++ b/commit.py
@@ -108,10 +108,6 @@ class SvnCommitBuilder(RootCommitBuilder):
         if revision_id is not None:
             self._record_revision_id(revision_id)
 
-        # At least one of the parents has to be the last revision on the 
-        # mainline in Subversion.
-        assert (self.base_revid is None or self.base_revid == parents[0])
-
         if old_inv is None:
             if self.base_revid is None:
                 self.old_inv = Inventory(root_id=None)
@@ -363,7 +359,7 @@ class SvnCommitBuilder(RootCommitBuilder):
             self.editor.close_directory(child_baton, self.pool)
 
     def open_branch_batons(self, root, elements, existing_elements, 
-                           base_path, base_rev):
+                           base_path, base_rev, replace_existing):
         """Open a specified directory given a baton for the repository root.
 
         :param root: Baton for the repository root
@@ -371,6 +367,7 @@ class SvnCommitBuilder(RootCommitBuilder):
         :param existing_elements: List of directory names that exist
         :param base_path: Path to base top-level branch on
         :param base_rev: Revision of path to base top-level branch on
+        :param replace_existing: Whether the current branch should be replaced
         """
         ret = [root]
 
@@ -392,12 +389,12 @@ class SvnCommitBuilder(RootCommitBuilder):
         # This needs to also check that base_rev was the latest version of 
         # branch_path.
         if (len(existing_elements) == len(elements) and 
-            base_path.strip("/") == "/".join(elements).strip("/")):
+            not replace_existing):
             ret.append(self.editor.open_directory(
                 "/".join(elements), ret[-1], base_rev, self.pool))
         else: # Branch has to be created
             # Already exists, old copy needs to be removed
-            if len(existing_elements) == len(elements):
+            if replace_existing:
                 self.editor.delete_entry("/".join(elements), -1, ret[-1])
             if base_path is not None:
                 base_url = urlutils.join(self.repository.transport.svn_url, base_path)
@@ -425,6 +422,7 @@ class SvnCommitBuilder(RootCommitBuilder):
             self.author = author
         
         bp_parts = self.branch.get_branch_path().split("/")
+        repository_latest_revnum = self.repository.transport.get_latest_revnum()
         lock = self.repository.transport.lock_write(".")
 
         try:
@@ -435,10 +433,18 @@ class SvnCommitBuilder(RootCommitBuilder):
                 message.encode("utf-8"), done, None, False)
 
             root = self.editor.open_root(self.base_revnum)
-            
+
+            replace_existing = False
+            if len(bp_parts) == len(existing_bp_parts):
+                if self.base_path.strip("/") != "/".join(bp_parts).strip("/"):
+                    replace_existing = True
+                elif self.base_revnum < self.repository._log.find_latest_change(self.branch.get_branch_path(), repository_latest_revnum, include_children=True):
+                    replace_existing = True
+
             # TODO: Accept create_prefix argument
             branch_batons = self.open_branch_batons(root, bp_parts,
-                existing_bp_parts, self.base_path, self.base_revnum)
+                existing_bp_parts, self.base_path, self.base_revnum,
+                replace_existing)
 
             # Make sure the root id is stored properly
             if (self.old_inv.root is None or 
@@ -645,18 +651,15 @@ def push(target, source, revision_id, validate=False):
         the source revision.
     """
     assert isinstance(source, Branch)
-    mutter('pushing %r' % (revision_id))
     rev = source.repository.get_revision(revision_id)
+    mutter('pushing %r (%r)' % (revision_id, rev.parent_ids))
 
+    # revision on top of which to commit
     if rev.parent_ids == []:
         base_revid = None
     else:
         base_revid = rev.parent_ids[0]
 
-    # revision on top of which to commit
-    assert (base_revid in rev.parent_ids or 
-            base_revid is None and rev.parent_ids == [])
-
     old_tree = source.repository.revision_tree(revision_id)
     base_tree = source.repository.revision_tree(base_revid)
 
index 0d767837340ff756a2e72a47294361f2f8d6c902..c03fd7f74da2355788c3cb61f97eef3c2d610b13 100644 (file)
@@ -231,7 +231,8 @@ class LogWalker(object):
             message = _escape_commit_message(base64.b64decode(message))
         return (author, message, date)
 
-    def find_latest_change(self, path, revnum, recurse=False):
+    def find_latest_change(self, path, revnum, include_parents=False,
+                           include_children=False):
         """Find latest revision that touched path.
 
         :param path: Path to check for changes
@@ -242,11 +243,12 @@ class LogWalker(object):
         if revnum > self.saved_revnum:
             self.fetch_revisions(revnum)
 
-        if recurse:
-            extra = " or path like '%s/%%'" % path.strip("/")
-        else:
-            extra = ""
-        query = "select rev from changed_path where (path='%s' or ('%s' like (path || '/%%') and (action = 'R' or action = 'A'))%s) and rev <= %d order by rev desc limit 1" % (path.strip("/"), path.strip("/"), extra, revnum)
+        extra = ""
+        if include_children:
+            extra += " or path like '%s/%%'" % path.strip("/")
+        if include_parents:
+            extra += " or ('%s' like (path || '/%%') and (action = 'R' or action = 'A'))" % path.strip("/")
+        query = "select rev from changed_path where (path='%s'%s) and rev <= %d order by rev desc limit 1" % (path.strip("/"), extra, revnum)
 
         row = self.db.execute(query).fetchone()
         if row is None and path == "":
index 628d7ed51d09c01478fd362606633fb864e7737c..297017675e1c465cf3064a712f008f125d827e7b 100644 (file)
@@ -1024,7 +1024,7 @@ class SvnRepository(Repository):
                         if paths[p][0] in ('R', 'D'):
                             del created_branches[p]
                             j = self._log.find_latest_change(p, i-1, 
-                                recurse=True)
+                                include_parents=True, include_children=True)
                             ret.append((p, j, False))
 
                         if paths[p][0] in ('A', 'R'): 
@@ -1037,7 +1037,8 @@ class SvnRepository(Repository):
                                 if c.startswith(p+"/"):
                                     del created_branches[c] 
                                     j = self._log.find_latest_change(c, i-1, 
-                                            recurse=True)
+                                            include_parents=True, 
+                                            include_children=True)
                                     ret.append((c, j, False))
                         if paths[p][0] in ('A', 'R'):
                             parents = [p]
@@ -1054,7 +1055,9 @@ class SvnRepository(Repository):
             pb.finished()
 
         for p in created_branches:
-            j = self._log.find_latest_change(p, revnum, recurse=True)
+            j = self._log.find_latest_change(p, revnum, 
+                                             include_parents=True,
+                                             include_children=True)
             if j is None:
                 j = created_branches[p]
             ret.append((p, j, True))
index 10718eb96b56e6bab1fdb1aa634d69d576bd38f5..bbb503de020675630dc4aa836e1cfac4abd1cdc2 100644 (file)
@@ -46,7 +46,7 @@ class TestCheckoutFormat(TestCase):
         self.format = SvnWorkingTreeDirFormat()
 
     def test_get_converter(self):
-        self.assertIsInstance(self.format.get_converter(), SvnConverter)
+        self.assertRaises(NotImplementedError, self.format.get_converter)
 
 
 class TestCheckout(TestCaseWithSubversionRepository):
index 485c88eb8d075d2e127af29ac5db9dde45161846..d76eac26fd7884b46809fcae2dd5546d21ff277d 100644 (file)
@@ -166,7 +166,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(0, walker.find_latest_change("", 1))
+        self.assertEqual(0, walker.find_latest_change("", 1, include_parents=True))
 
     def test_find_latest_parent(self):
         repos_url = self.make_client("a", "dc")
@@ -179,7 +179,8 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(2, walker.find_latest_change("tags/tmp/foo", 2))
+        self.assertEqual(2, walker.find_latest_change("tags/tmp/foo", 2, 
+                                                      include_parents=True))
 
     def test_find_latest_parent_just_modify(self):
         repos_url = self.make_client("a", "dc")
@@ -194,7 +195,8 @@ class TestLogWalker(TestCaseWithSubversionRepository):
         self.client_commit("dc", "My Message3")
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
-        self.assertEqual(2, walker.find_latest_change("tags/tmp/foo", 3))
+        self.assertEqual(2, walker.find_latest_change("tags/tmp/foo", 3,
+                                                      include_parents=True))
 
     def test_find_latest_parentmoved(self):
         repos_url = self.make_client("a", "dc")
@@ -207,7 +209,8 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertIs(2, walker.find_latest_change("bla/tmp", 2))
+        self.assertIs(2, walker.find_latest_change("bla/tmp", 2, 
+                                                   include_parents=True))
 
     def test_find_latest_nonexistant(self):
         repos_url = self.make_client("a", "dc")
@@ -220,8 +223,8 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertIs(None, walker.find_latest_change("bloe", 2))
-        self.assertIs(None, walker.find_latest_change("bloe/bla", 2))
+        self.assertIs(None, walker.find_latest_change("bloe", 2, include_parents=True))
+        self.assertIs(None, walker.find_latest_change("bloe/bla", 2, include_parents=True))
 
     def test_find_latest_change(self):
         repos_url = self.make_client("a", "dc")
@@ -231,7 +234,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(1, walker.find_latest_change("branches", 1))
+        self.assertEqual(1, walker.find_latest_change("branches", 1, include_parents=True))
 
     def test_find_latest_change_children(self):
         repos_url = self.make_client("a", "dc")
@@ -244,7 +247,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(1, walker.find_latest_change("branches", 2))
+        self.assertEqual(1, walker.find_latest_change("branches", 2, include_parents=True))
 
     def test_find_latest_change_prop(self):
         repos_url = self.make_client("a", "dc")
@@ -259,7 +262,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(2, walker.find_latest_change("branches", 3))
+        self.assertEqual(2, walker.find_latest_change("branches", 3, include_parents=True))
 
     def test_find_latest_change_file(self):
         repos_url = self.make_client("a", "dc")
@@ -274,7 +277,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(3, walker.find_latest_change("branches/foo", 3))
+        self.assertEqual(3, walker.find_latest_change("branches/foo", 3, include_parents=True))
 
     def test_find_latest_change_newer(self):
         repos_url = self.make_client("a", "dc")
@@ -289,7 +292,7 @@ class TestLogWalker(TestCaseWithSubversionRepository):
 
         walker = logwalker.LogWalker(transport=SvnRaTransport(repos_url))
 
-        self.assertEqual(2, walker.find_latest_change("branches/foo", 2))
+        self.assertEqual(2, walker.find_latest_change("branches/foo", 2, include_parents=True))
 
     def test_follow_history_branch_replace(self):
         repos_url = self.make_client("a", "dc")
index 4c31dbc6bb41efbe8022cf194c35fe860ef9c541..6e5630867a2ff1c754f847c168d0fc6abd4a155b 100644 (file)
@@ -452,3 +452,44 @@ class PushNewBranchTests(TestCaseWithSubversionRepository):
 
         os.mkdir("n")
         BzrDir.open(repos_url+"/trunk").sprout("n")
+
+    def test_push_unnecessary_merge(self):        
+        from bzrlib import debug
+        debug.debug_flags.add('transport')
+        debug.debug_flags.add('commit')
+        repos_url = self.make_client("a", "dc")
+        bzrwt = BzrDir.create_standalone_workingtree("c", 
+            format=format.get_rich_root_format())
+        self.build_tree({'c/registry/generic.c': "Tour"})
+        bzrwt.add("registry")
+        bzrwt.add("registry/generic.c")
+        revid1 = bzrwt.commit("Add initial directory + file", 
+                              rev_id="initialrevid")
+
+        # Push first branch into Subversion
+        newdir = BzrDir.open(repos_url+"/trunk")
+        newbranch = newdir.import_branch(bzrwt.branch)
+
+        # Should create dc/trunk
+        self.client_update("dc")
+
+        self.build_tree({'dc/trunk/registry/generic.c': "DE"})
+        self.client_commit("dc", "Change copied branch")[0]
+        self.client_update("dc")
+        merge_revid = newdir.find_repository().generate_revision_id(2, "trunk", "trunk0")
+
+        # Merge 
+        self.build_tree({'c/registry/generic.c': "DE"})
+        bzrwt.add_pending_merge(merge_revid)
+        revid2 = bzrwt.commit("Merge something", rev_id="mergerevid")
+
+        trunk = Branch.open(repos_url + "/trunk")
+        trunk.pull(bzrwt.branch)
+
+        self.assertEquals([revid1, revid2], trunk.revision_history())
+        self.client_update("dc")
+        self.assertEquals(
+                '1 initialrevid\n2 mergerevid\n',
+                self.client_get_prop("dc/trunk", SVN_PROP_BZR_REVISION_ID+"trunk0"))
+
+