Handle corner cases pulling pushed changes with directory renames.

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.
 
 
    * 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)
   DOCUMENTATION
 
    * Add simple FAQ file. (#144388)

diff --git a/fetch.py b/fetch.py
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 bzrlib.repository import InterRepository
 from bzrlib.trace import mutter
 

-from copy import copy

 from cStringIO import StringIO
 import md5
 
 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
                  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
         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)
 
             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:
     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)
             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))
 
         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:
             # 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)
                 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)
             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)
             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:
             # 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)
                 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)
             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):
         return path
 
     def open_file(self, path, parent_id, base_revnum, pool):

diff --git a/fileids.py b/fileids.py
index 3d52fd3cbf054e9db881cb9bf3b4613a249e485a..2cb5baa9a84dc24738bbfcd56306b54b6f79b46e 100644 (file)
--- a/fileids.py
+++ b/fileids.py
@@ -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 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,
         :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):
         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 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."""
 
     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
 
 
                 parent_revs = next_parent_revs
 

-                renames = renames_cb(revid)
-

                 def new_file_id(x):
                 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)
                     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]
                 for p in changes:
                     if changes[p][0] == 'M' and not revmap.has_key(p):
                         revmap[p] = map[p][0]

diff --git a/tests/test_push.py b/tests/test_push.py
index 01704373aec0a03f1da7e923fcbbfe43c0b1bea4..08c2e1f769c7ae4a06850f7a407ac40d35f585a8 100644 (file)
--- a/tests/test_push.py
+++ b/tests/test_push.py
@@ -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")
         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", 
     def test_push_non_lhs_parent(self):        
         repos_url = self.make_client("a", "dc")
         bzrwt = BzrDir.create_standalone_workingtree("c",