Handle corner cases pulling pushed changes with directory renames.
authorJelmer Vernooij <jelmer@samba.org>
Wed, 24 Oct 2007 16:14:16 +0000 (18:14 +0200)
committerJelmer Vernooij <jelmer@samba.org>
Wed, 24 Oct 2007 16:14:16 +0000 (18:14 +0200)
NEWS
fetch.py
fileids.py
tests/test_push.py

diff --git a/NEWS b/NEWS
index dd3b8ec2e29e12ff2d0474e8e039a5c46f043b38..8ad221ec9a4aa1744b4ab9b4b556f80ae6a60a2d 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -54,6 +54,9 @@ bzr-svn 0.4.4 UNRELEASED
 
    * Show proper exception when lightweight checkout is out of date.
 
+   * Fix pulling in changes push earlier by bzr-svn with directory renames 
+     of directories containing files. (#153347)
+
   DOCUMENTATION
 
    * Add simple FAQ file. (#144388)
index 1c40451c0feb9bc2125eab09930c0b062239bb2c..bec33b1545a8351346bf9e39f1631afd6cf5c3ae 100644 (file)
--- a/fetch.py
+++ b/fetch.py
@@ -22,7 +22,6 @@ from bzrlib.revision import Revision, NULL_REVISION
 from bzrlib.repository import InterRepository
 from bzrlib.trace import mutter
 
-from copy import copy
 from cStringIO import StringIO
 import md5
 
@@ -58,7 +57,7 @@ class RevisionBuildEditor(svn.delta.Editor):
                  svn_revprops, id_map, scheme):
         self.branch_path = branch_path
         self.old_inventory = prev_inventory
-        self.inventory = copy(prev_inventory)
+        self.inventory = prev_inventory.copy()
         self.revid = revid
         self.id_map = id_map
         self.scheme = scheme
@@ -136,10 +135,21 @@ class RevisionBuildEditor(svn.delta.Editor):
             return self.id_map[new_path]
         return generate_file_id(self.source, self.revid, new_path)
 
+    def _rename(self, file_id, parent_id, path):
+        # Only rename if not right yet
+        if (self.inventory[file_id].parent_id == parent_id and 
+            self.inventory[file_id].name == urlutils.basename(path)):
+            return
+        self.inventory.rename(file_id, parent_id, urlutils.basename(path))
+
     def delete_entry(self, path, revnum, parent_id, pool):
         path = path.decode("utf-8")
         if path in self._premature_deletes:
+            # Delete recursively
             self._premature_deletes.remove(path)
+            for p in self._premature_deletes.copy():
+                if p.startswith("%s/" % path):
+                    self._premature_deletes.remove(p)
         else:
             self.inventory.remove_recursive_id(self._get_old_id(parent_id, path))
 
@@ -159,12 +169,13 @@ class RevisionBuildEditor(svn.delta.Editor):
             # This directory was moved here from somewhere else, but the 
             # other location hasn't been removed yet. 
             if copyfrom_path is None:
-                # FIXME: This should never happen!
+                # This should ideally never happen!
                 copyfrom_path = self.old_inventory.id2path(file_id)
+                mutter('no copyfrom path set, assuming %r' % copyfrom_path)
             assert copyfrom_path == self.old_inventory.id2path(file_id)
             assert copyfrom_path not in self._premature_deletes
             self._premature_deletes.add(copyfrom_path)
-            self.inventory.rename(file_id, parent_id, urlutils.basename(path))
+            self._rename(file_id, parent_id, path)
             ie = self.inventory[file_id]
         else:
             ie = self.inventory.add_path(path, 'directory', file_id)
@@ -266,12 +277,14 @@ class RevisionBuildEditor(svn.delta.Editor):
             # This file was moved here from somewhere else, but the 
             # other location hasn't been removed yet. 
             if copyfrom_path is None:
-                # FIXME: This should never happen!
+                # This should ideally never happen
                 copyfrom_path = self.old_inventory.id2path(self.file_id)
+                mutter('no copyfrom path set, assuming %r' % copyfrom_path)
             assert copyfrom_path == self.old_inventory.id2path(self.file_id)
             assert copyfrom_path not in self._premature_deletes
             self._premature_deletes.add(copyfrom_path)
-            self.inventory.rename(self.file_id, parent_id, urlutils.basename(path))
+            # No need to rename if it's already in the right spot
+            self._rename(self.file_id, parent_id, path)
         return path
 
     def open_file(self, path, parent_id, base_revnum, pool):
index 3d52fd3cbf054e9db881cb9bf3b4613a249e485a..2cb5baa9a84dc24738bbfcd56306b54b6f79b46e 100644 (file)
@@ -114,7 +114,7 @@ class FileIdMap(object):
         :param revnum: Revno for revision in which changes happened
         :param branch: Branch path where changes happened
         :param global_changes: Dict with global changes that happened
-        :param renames: List of renames
+        :param renames: List of renames (known file ids for particular paths)
         :param scheme: Branching scheme
         """
         changes = get_local_changes(global_changes, scheme,
@@ -130,11 +130,11 @@ class FileIdMap(object):
         revid = self.repos.generate_revision_id(revnum, branch, str(scheme))
 
         def new_file_id(x):
-            if renames.has_key(x):
-                return renames[x]
             return generate_file_id(self.repos, revid, x)
          
-        return self._apply_changes(new_file_id, changes, get_children)
+        idmap = self._apply_changes(new_file_id, changes, get_children)
+        idmap.update(renames)
+        return idmap
 
     def get_map(self, uuid, revnum, branch, renames_cb, scheme):
         """Make sure the map is up to date until revnum."""
@@ -190,14 +190,12 @@ class FileIdMap(object):
 
                 parent_revs = next_parent_revs
 
-                renames = renames_cb(revid)
-
                 def new_file_id(x):
-                    if renames.has_key(x):
-                        return renames[x]
                     return generate_file_id(self.repos, revid, x)
                 
                 revmap = self._apply_changes(new_file_id, changes, find_children)
+                revmap.update(renames_cb(revid))
+
                 for p in changes:
                     if changes[p][0] == 'M' and not revmap.has_key(p):
                         revmap[p] = map[p][0]
index 01704373aec0a03f1da7e923fcbbfe43c0b1bea4..08c2e1f769c7ae4a06850f7a407ac40d35f585a8 100644 (file)
@@ -515,7 +515,58 @@ class PushNewBranchTests(TestCaseWithSubversionRepository):
         self.assertFalse(tree.inventory.has_filename("registry.moved/generic.c"))
         os.mkdir("n")
         BzrDir.open(repos_url+"/trunk").sprout("n")
+
+    def test_rename_dir_changing_contents(self):
+        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", "dirid")
+        bzrwt.add("registry/generic.c", "origid")
+        revid1 = bzrwt.commit("Add initial directory + file")
+        bzrwt.rename_one("registry/generic.c", "registry/c.c")
+        self.build_tree({'c/registry/generic.c': "Tour2"})
+        bzrwt.add("registry/generic.c", "newid")
+        revid2 = bzrwt.commit("Other change")
+        bzrwt.rename_one("registry", "registry.moved")
+        revid3 = bzrwt.commit("Rename")
+        newdir = BzrDir.open(repos_url+"/trunk")
+        newbranch = newdir.import_branch(bzrwt.branch)
+        def check(b):
+            self.assertEquals([revid1, revid2, revid3], b.revision_history())
+            tree = b.repository.revision_tree(revid3)
+            self.assertEquals("origid", tree.path2id("registry.moved/c.c"))
+            self.assertEquals("newid", tree.path2id("registry.moved/generic.c"))
+            self.assertEquals("dirid", tree.path2id("registry.moved"))
+        check(newbranch)
+        os.mkdir("n")
+        BzrDir.open(repos_url+"/trunk").sprout("n")
+        copybranch = Branch.open("n")
+        check(copybranch)
     
+    def test_rename_dir(self):
+        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", "dirid")
+        bzrwt.add("registry/generic.c", "origid")
+        revid1 = bzrwt.commit("Add initial directory + file")
+        bzrwt.rename_one("registry", "registry.moved")
+        revid2 = bzrwt.commit("Rename")
+        newdir = BzrDir.open(repos_url+"/trunk")
+        newbranch = newdir.import_branch(bzrwt.branch)
+        def check(b):
+            self.assertEquals([revid1, revid2], b.revision_history())
+            tree = b.repository.revision_tree(revid2)
+            self.assertEquals("origid", tree.path2id("registry.moved/generic.c"))
+            self.assertEquals("dirid", tree.path2id("registry.moved"))
+        check(newbranch)
+        os.mkdir("n")
+        BzrDir.open(repos_url+"/trunk").sprout("n")
+        copybranch = Branch.open("n")
+        check(copybranch)
+
     def test_push_non_lhs_parent(self):        
         repos_url = self.make_client("a", "dc")
         bzrwt = BzrDir.create_standalone_workingtree("c",