[patch] bug #43959, bug in commit of merges in checkouts

Martin Pool mbp at canonical.com
Thu May 11 08:24:08 BST 2006


Fix and test case for a bug reported yesterday by Erik where commit of
merged revisions fails in a checkout directory.  The root problem is
that we used the wrong revision object to access the repository and so
saw slightly out-of-date information.

Because this substantially impairs checkouts I'm going to make an 
0.8.1 release with this fix soon; Robert & I did it in pair so that
should do for review.  (But comments are still welcome.)

(As future work this class of errors could probably raise an assertion,
if you try to access an object using a transaction that doesn't
correspond to a lock on the object.)

-- 
Martin
-------------- next part --------------
=== modified file 'bzrlib/commit.py'
--- bzrlib/commit.py	
+++ bzrlib/commit.py	
@@ -512,7 +512,7 @@
             if ie.revision is None:
                 change = ie.snapshot(self.rev_id, path, previous_entries,
                                      self.work_tree, self.weave_store,
-                                     self.branch.get_transaction())
+                                     self.branch.repository.get_transaction())
             else:
                 change = "unchanged"
             self.reporter.snapshot_change(change, path)

=== modified file 'bzrlib/inventory.py'
--- bzrlib/inventory.py	
+++ bzrlib/inventory.py	
@@ -656,21 +656,21 @@
         self.text_sha1 = None
         self.executable = None
 
-    def _snapshot_text(self, file_parents, work_tree, weave_store, transaction):
+    def _snapshot_text(self, file_parents, work_tree, versionedfile_store, transaction):
         """See InventoryEntry._snapshot_text."""
-        mutter('storing file {%s} in revision {%s}',
-               self.file_id, self.revision)
+        mutter('storing text of file {%s} in revision {%s} into %r',
+               self.file_id, self.revision, versionedfile_store)
         # special case to avoid diffing on renames or 
         # reparenting
         if (len(file_parents) == 1
             and self.text_sha1 == file_parents.values()[0].text_sha1
             and self.text_size == file_parents.values()[0].text_size):
             previous_ie = file_parents.values()[0]
-            versionedfile = weave_store.get_weave(self.file_id, transaction)
+            versionedfile = versionedfile_store.get_weave(self.file_id, transaction)
             versionedfile.clone_text(self.revision, previous_ie.revision, file_parents.keys())
         else:
             new_lines = work_tree.get_file(self.file_id).readlines()
-            self._add_text_to_weave(new_lines, file_parents.keys(), weave_store,
+            self._add_text_to_weave(new_lines, file_parents.keys(), versionedfile_store,
                                     transaction)
             self.text_sha1 = sha_strings(new_lines)
             self.text_size = sum(map(len, new_lines))

=== modified file 'bzrlib/repository.py'
--- bzrlib/repository.py	
+++ bzrlib/repository.py	
@@ -166,6 +166,10 @@
         self.control_weaves = control_store
         # TODO: make sure to construct the right store classes, etc, depending
         # on whether escaping is required.
+
+    def __repr__(self):
+        return '%s(%r)' % (self.__class__.__name__, 
+                           self.bzrdir.transport.base)
 
     def is_locked(self):
         return self.control_files.is_locked()

=== modified file 'bzrlib/tests/blackbox/test_checkout.py'
--- bzrlib/tests/blackbox/test_checkout.py	
+++ bzrlib/tests/blackbox/test_checkout.py	
@@ -1,5 +1,4 @@
-# Copyright (C) 2005 by Canonical Ltd
-# -*- coding: utf-8 -*-
+# Copyright (C) 2005, 2006 by Canonical Ltd
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by

=== modified file 'bzrlib/tests/blackbox/test_commit.py'
--- bzrlib/tests/blackbox/test_commit.py	
+++ bzrlib/tests/blackbox/test_commit.py	
@@ -186,3 +186,35 @@
         self.assertEqualDiff('', out)
         self.assertEqualDiff('bzr: ERROR: Cannot perform local-only commits '
                              'on unbound branches.\n', err)
+
+    def test_commit_a_text_merge_in_a_checkout(self):
+        # checkouts perform multiple actions in a transaction across bond
+        # branches and their master, and have been observed to fail in the
+        # past. This is a user story reported to fail in bug #43959 where 
+        # a merge done in a checkout (using the update command) failed to
+        # commit correctly.
+        self.run_bzr('init', 'trunk')
+
+        self.run_bzr('checkout', 'trunk', 'u1')
+        self.build_tree_contents([('u1/hosts', 'initial contents')])
+        self.run_bzr('add', 'u1/hosts')
+        self.run_bzr('commit', '-m', 'add hosts', 'u1')
+
+        self.run_bzr('checkout', 'trunk', 'u2')
+        self.build_tree_contents([('u2/hosts', 'altered in u2')])
+        self.run_bzr('commit', '-m', 'checkin from u2', 'u2')
+
+        # make an offline commits
+        self.build_tree_contents([('u1/hosts', 'first offline change in u1')])
+        self.run_bzr('commit', '-m', 'checkin offline', '--local', 'u1')
+
+        # now try to pull in online work from u2, and then commit our offline
+        # work as a merge
+        # retcode 1 as we expect a text conflict
+        self.run_bzr('update', 'u1', retcode=1)
+        self.run_bzr('resolved', 'u1/hosts')
+        # add a text change here to represent resolving the merge conflicts in
+        # favour of a new version of the file not identical to either the u1
+        # version or the u2 version.
+        self.build_tree_contents([('u1/hosts', 'merge resolution\n')])
+        self.run_bzr('commit', '-m', 'checkin merge of the offline work from u1', 'u1')

=== modified file 'bzrlib/tests/test_commit.py'
--- bzrlib/tests/test_commit.py	
+++ bzrlib/tests/test_commit.py	
@@ -1,4 +1,4 @@
-# Copyright (C) 2005 by Canonical Ltd
+# Copyright (C) 2005, 2006 by Canonical Ltd
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -392,3 +392,33 @@
             self.assertRaises(LockContention, wt.commit, 'silly')
         finally:
             master_branch.unlock()
+
+    def test_commit_bound_merge(self):
+        # see bug #43959; commit of a merge in a bound branch fails to push
+        # the new commit into the master
+        master_branch = self.make_branch('master')
+        bound_tree = self.make_branch_and_tree('bound')
+        bound_tree.branch.bind(master_branch)
+
+        self.build_tree_contents([('bound/content_file', 'initial contents\n')])
+        bound_tree.add(['content_file'])
+        bound_tree.commit(message='woo!')
+
+        other_bzrdir = master_branch.bzrdir.sprout('other')
+        other_tree = other_bzrdir.open_workingtree()
+
+        # do a commit to the the other branch changing the content file so
+        # that our commit after merging will have a merged revision in the
+        # content file history.
+        self.build_tree_contents([('other/content_file', 'change in other\n')])
+        other_tree.commit('change in other')
+
+        # do a merge into the bound branch from other, and then change the
+        # content file locally to force a new revision (rather than using the
+        # revision from other). This forces extra processing in commit.
+        self.merge(other_tree.branch, bound_tree)
+        self.build_tree_contents([('bound/content_file', 'change in bound\n')])
+
+        # before #34959 was fixed, this failed with 'revision not present in
+        # weave' when trying to implicitly push from the bound branch to the master
+        bound_tree.commit(message='commit of merge in bound tree')



More information about the bazaar mailing list