Rev 4537: Fix dirstate.set_state_from_inventory to have a more stable old_iterator, fixing rename problems in new trees. (Robert Collins, bug 395556) in http://bazaar.launchpad.net/~lifeless/bzr/bug-395556
Robert Collins
robertc at robertcollins.net
Wed Jul 15 02:13:30 BST 2009
At http://bazaar.launchpad.net/~lifeless/bzr/bug-395556
------------------------------------------------------------
revno: 4537
revision-id: robertc at robertcollins.net-20090715011320-coi9b5psw5d3tr2r
parent: pqm at pqm.ubuntu.com-20090714173313-3p3ytzlfuc3y2bm6
committer: Robert Collins <robertc at robertcollins.net>
branch nick: bug-395556
timestamp: Wed 2009-07-15 11:13:20 +1000
message:
Fix dirstate.set_state_from_inventory to have a more stable old_iterator, fixing rename problems in new trees. (Robert Collins, bug 395556)
=== modified file 'NEWS'
--- a/NEWS 2009-07-14 16:27:40 +0000
+++ b/NEWS 2009-07-15 01:13:20 +0000
@@ -25,6 +25,11 @@
* BranchBuilder now accepts timezone to avoid test failures in countries far
from GMT. (Vincent Ladeuil, #397716)
+* Renames to lexographically lower basenames in trees that have never been
+ committed to will no longer corrupt the dirstate. This was caused by an
+ aliasing bug in the dirstate set_state_from_inventory method.
+ (Robert Collins, #395556)
+
Improvements
************
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py 2009-07-10 05:33:07 +0000
+++ b/bzrlib/dirstate.py 2009-07-15 01:13:20 +0000
@@ -203,6 +203,7 @@
import bisect
import binascii
+from copy import deepcopy
import errno
import os
from stat import S_IEXEC
@@ -2436,8 +2437,11 @@
new_iterator = new_inv.iter_entries_by_dir()
# we will be modifying the dirstate, so we need a stable iterator. In
# future we might write one, for now we just clone the state into a
- # list - which is a shallow copy.
- old_iterator = iter(list(self._iter_entries()))
+ # list using a deep copy so that forward changes don't make the logic
+ # more complex. Using a shallow copy results in all entries being seen
+ # but the state of the entries being wrong, and that leads to stale
+ # entries being left behind.
+ old_iterator = iter(deepcopy(list(self._iter_entries())))
# both must have roots so this is safe:
current_new = new_iterator.next()
current_old = old_iterator.next()
=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- a/bzrlib/tests/blackbox/test_commit.py 2009-07-08 05:27:06 +0000
+++ b/bzrlib/tests/blackbox/test_commit.py 2009-07-15 01:13:20 +0000
@@ -639,25 +639,3 @@
out, err = self.run_bzr("commit tree/hello.txt")
last_rev = tree.branch.repository.get_revision(tree.last_revision())
self.assertEqual('save me some typing\n', last_rev.message)
-
- def test_commit_and_mv_dance_a(self):
- # see https://bugs.launchpad.net/bzr/+bug/395556
- tree = self.make_branch_and_tree(".")
- self.build_tree(["a"])
- tree.add("a")
- self.check_output("a => b\n", ["mv", "a", "b"])
- self.check_output("", ["commit", "-q", "-m", "Actually no, b"])
- self.check_output("b => a\n", ["mv", "b", "a"])
- self.check_output("", ["commit", "-q", "-m", "No, really, a"])
-
- def test_commit_and_mv_dance_b(self):
- # see https://bugs.launchpad.net/bzr/+bug/395556
- tree = self.make_branch_and_tree(".")
- self.build_tree(["b"])
- tree.add("b")
- self.check_output("b => a\n", ["mv", "b", "a"])
- self.check_output("", ["commit", "-q", "-m", "Actually no, a"])
- self.check_output("a => b\n", ["mv", "a", "b"])
- self.expectFailure("bug 395556: gives DuplicateFileId "
- "committing renames",
- self.check_output, "", ["commit", "-q", "-m", "No, really, b"])
=== modified file 'bzrlib/tests/per_workingtree/test_commit.py'
--- a/bzrlib/tests/per_workingtree/test_commit.py 2009-07-10 07:14:02 +0000
+++ b/bzrlib/tests/per_workingtree/test_commit.py 2009-07-15 01:13:20 +0000
@@ -602,29 +602,3 @@
revid = tree.commit('first post')
committed_tree = tree.basis_tree()
self.assertTrue(committed_tree.has_filename("newfile"))
-
- def test_commit_and_mv_dance_a(self):
- # should fail because of
- # <https://bugs.launchpad.net/bzr/+bug/395556> but apparently does
- # not, while the blackbox.test_commit equivalent does - maybe because
- # of different format combinations
- tree = self.make_branch_and_tree(".")
- self.build_tree(["a"])
- tree.add("a")
- tree.rename_one("a", "b")
- tree.commit("Actually no, b")
- tree.rename_one("b", "a")
- tree.commit("No, really, a")
-
- def test_commit_and_mv_dance_b(self):
- # should fail because of
- # <https://bugs.launchpad.net/bzr/+bug/395556> but apparently does
- # not, while the blackbox.test_commit equivalent does - maybe because
- # of different format combinations
- tree = self.make_branch_and_tree(".")
- self.build_tree(["b"])
- tree.add("b")
- tree.rename_one("b", "a")
- tree.commit("Actually no, a")
- tree.rename_one("a", "b")
- tree.commit("No, really, b")
=== modified file 'bzrlib/tests/test_dirstate.py'
--- a/bzrlib/tests/test_dirstate.py 2009-05-06 05:36:28 +0000
+++ b/bzrlib/tests/test_dirstate.py 2009-07-15 01:13:20 +0000
@@ -827,7 +827,6 @@
finally:
tree.unlock()
-
def test_set_state_from_inventory_mixed_paths(self):
tree1 = self.make_branch_and_tree('tree1')
self.build_tree(['tree1/a/', 'tree1/a/b/', 'tree1/a-b/',
@@ -942,7 +941,6 @@
finally:
state.unlock()
-
def test_set_parent_trees_no_content(self):
# set_parent_trees is a slow but important api to support.
tree1 = self.make_branch_and_memory_tree('tree1')
@@ -1238,6 +1236,38 @@
self.assertRaises(errors.BzrError,
state.add, '..', 'ass-id', 'directory', None, None)
+ def test_set_state_with_rename_b_a_bug_395556(self):
+ # bug 395556 uncovered a bug where the dirstate ends up with a false
+ # relocation record - in a tree with no parents there should be no
+ # absent or relocated records. This then leads to further corruption
+ # when a commit occurs, as the incorrect relocation gathers an
+ # incorrect absent in tree 1, and future changes go to pot.
+ tree1 = self.make_branch_and_tree('tree1')
+ self.build_tree(['tree1/b'])
+ tree1.lock_write()
+ try:
+ tree1.add(['b'], ['b-id'])
+ root_id = tree1.get_root_id()
+ inv = tree1.inventory
+ state = dirstate.DirState.initialize('dirstate')
+ try:
+ # Set the initial state with 'b'
+ state.set_state_from_inventory(inv)
+ inv.rename('b-id', root_id, 'a')
+ # Set the new state with 'a', which currently corrupts.
+ state.set_state_from_inventory(inv)
+ expected_result1 = [('', '', root_id, 'd'),
+ ('', 'a', 'b-id', 'f'),
+ ]
+ values = []
+ for entry in state._iter_entries():
+ values.append(entry[0] + entry[1][0][:1])
+ self.assertEqual(expected_result1, values)
+ finally:
+ state.unlock()
+ finally:
+ tree1.unlock()
+
class TestGetLines(TestCaseWithDirState):
More information about the bazaar-commits
mailing list