Merge lp:~jelmer/bzr/remove-branch-history into lp:bzr

[Jelmer Vernooij]

Remove Branch.revision_history() and Branch.set_revision_history().

Finally (!) remove Branch.revision_history and Branch.set_revision_history.

The code in bzrlib.tests.per_branch.test_bound_sftp isn't actually used anymore, but I updated it for when we want to re-enable it.

[Vincent Ladeuil (vila)]

Nice job !

Watch for conflicts in test_bound_sftp deleted by gz in https://code.launchpad.net/~gz/bzr/delete_skipped_bound_sftp_tests_728252/+merge/97388

review: Approve

[lp:~jelmer/bzr/remove-branch-history]

6506. By Jelmer Vernooij on 2012-03-14

Merge bzr.dev.

=== modified file 'bzrlib/branch.py'
--- bzrlib/branch.py	2012-03-14 10:52:42 +0000
+++ bzrlib/branch.py	2012-03-14 15:29:20 +0000
@@ -766,24 +766,6 @@
         """Print `file` to stdout."""
         raise NotImplementedError(self.print_file)
 
-    @deprecated_method(deprecated_in((2, 4, 0)))
-    def set_revision_history(self, rev_history):
-        """See Branch.set_revision_history."""
-        self._set_revision_history(rev_history)
-
-    @needs_write_lock
-    def _set_revision_history(self, rev_history):
-        if len(rev_history) == 0:
-            revid = _mod_revision.NULL_REVISION
-        else:
-            revid = rev_history[-1]
-        if rev_history != self._lefthand_history(revid):
-            raise errors.NotLefthandHistory(rev_history)
-        self.set_last_revision_info(len(rev_history), revid)
-        self._cache_revision_history(rev_history)
-        for hook in Branch.hooks['set_rh']:
-            hook(self, rev_history)
-
     @needs_write_lock
     def set_last_revision_info(self, revno, revision_id):
         """Set the last revision of this branch.
@@ -1013,16 +995,6 @@
         """
         raise NotImplementedError(self._gen_revision_history)
 
-    @deprecated_method(deprecated_in((2, 5, 0)))
-    @needs_read_lock
-    def revision_history(self):
-        """Return sequence of revision ids on this branch.
-
-        This method will cache the revision history for as long as it is safe to
-        do so.
-        """
-        return self._revision_history()
-
     def _revision_history(self):
         if 'evil' in debug.debug_flags:
             mutter_callsite(3, "revision_history scales with history.")
@@ -1774,8 +1746,8 @@
 class BranchHooks(Hooks):
     """A dictionary mapping hook name to a list of callables for branch hooks.
 
-    e.g. ['set_rh'] Is the list of items to be called when the
-    set_revision_history function is invoked.
+    e.g. ['post_push'] Is the list of items to be called when the
+    push function is invoked.
     """
 
     def __init__(self):
@@ -1785,12 +1757,6 @@
         notified.
         """
         Hooks.__init__(self, "bzrlib.branch", "Branch.hooks")
-        self.add_hook('set_rh',
-            "Invoked whenever the revision history has been set via "
-            "set_revision_history. The api signature is (branch, "
-            "revision_history), and the branch will be write-locked. "
-            "The set_rh hook can be expensive for bzr to trigger, a better "
-            "hook to use is Branch.post_change_branch_tip.", (0, 15))
         self.add_hook('open',
             "Called with the Branch object that has been opened after a "
             "branch is opened.", (1, 8))
@@ -2747,12 +2713,6 @@
         else:
             return (0, _mod_revision.NULL_REVISION)
 
-    @deprecated_method(deprecated_in((2, 4, 0)))
-    @needs_write_lock
-    def set_revision_history(self, rev_history):
-        """See Branch.set_revision_history."""
-        self._set_revision_history(rev_history)
-
     def _set_revision_history(self, rev_history):
         if 'evil' in debug.debug_flags:
             mutter_callsite(3, "set_revision_history scales with history.")
@@ -2771,8 +2731,6 @@
         self._write_revision_history(rev_history)
         self._clear_cached_state()
         self._cache_revision_history(rev_history)
-        for hook in Branch.hooks['set_rh']:
-            hook(self, rev_history)
         if Branch.hooks['post_change_branch_tip']:
             self._run_post_change_branch_tip_hooks(old_revno, old_revid)
 

=== modified file 'bzrlib/log.py'
--- bzrlib/log.py	2011-12-19 13:23:58 +0000
+++ bzrlib/log.py	2012-03-14 15:29:20 +0000
@@ -138,15 +138,6 @@
         revno += 1
 
 
-def _enumerate_history(branch):
-    rh = []
-    revno = 1
-    for rev_id in branch.revision_history():
-        rh.append((revno, rev_id))
-        revno += 1
-    return rh
-
-
 def show_log(branch,
              lf,
              specific_fileid=None,

=== modified file 'bzrlib/remote.py'
--- bzrlib/remote.py	2012-02-23 19:45:15 +0000
+++ bzrlib/remote.py	2012-03-14 15:29:20 +0000
@@ -3792,26 +3792,6 @@
             raise errors.UnexpectedSmartServerResponse(response)
         self._run_post_change_branch_tip_hooks(old_revno, old_revid)
 
-    @symbol_versioning.deprecated_method(symbol_versioning.deprecated_in((2, 4, 0)))
-    @needs_write_lock
-    def set_revision_history(self, rev_history):
-        """See Branch.set_revision_history."""
-        self._set_revision_history(rev_history)
-
-    @needs_write_lock
-    def _set_revision_history(self, rev_history):
-        # Send just the tip revision of the history; the server will generate
-        # the full history from that.  If the revision doesn't exist in this
-        # branch, NoSuchRevision will be raised.
-        if rev_history == []:
-            rev_id = 'null:'
-        else:
-            rev_id = rev_history[-1]
-        self._set_last_revision(rev_id)
-        for hook in branch.Branch.hooks['set_rh']:
-            hook(self, rev_history)
-        self._cache_revision_history(rev_history)
-
     def _get_parent_location(self):
         medium = self._client._medium
         if medium._is_remote_before((1, 13)):
@@ -3949,8 +3929,18 @@
             except errors.UnknownSmartMethod:
                 medium._remember_remote_is_before((1, 6))
         self._clear_cached_state_of_remote_branch_only()
-        self._set_revision_history(self._lefthand_history(revision_id,
-            last_rev=last_rev,other_branch=other_branch))
+        graph = self.repository.get_graph()
+        (last_revno, last_revid) = self.last_revision_info()
+        known_revision_ids = [
+            (last_revid, last_revno),
+            (_mod_revision.NULL_REVISION, 0),
+            ]
+        if last_rev is not None:
+            if not graph.is_ancestor(last_rev, revision_id):
+                # our previous tip is not merged into stop_revision
+                raise errors.DivergedBranches(self, other_branch)
+        revno = graph.find_distance_to_null(revision_id, known_revision_ids)
+        self.set_last_revision_info(revno, revision_id)
 
     def set_push_location(self, location):
         self._set_config_location('push_location', location)

=== modified file 'bzrlib/smart/branch.py'
--- bzrlib/smart/branch.py	2011-12-19 13:23:58 +0000
+++ bzrlib/smart/branch.py	2012-03-14 15:29:20 +0000
@@ -299,13 +299,12 @@
 
     def do_tip_change_with_locked_branch(self, branch, new_last_revision_id):
         if new_last_revision_id == 'null:':
-            branch._set_revision_history([])
+            branch.set_last_revision_info(0, new_last_revision_id)
         else:
             if not branch.repository.has_revision(new_last_revision_id):
                 return FailedSmartServerResponse(
                     ('NoSuchRevision', new_last_revision_id))
-            branch._set_revision_history(branch._lefthand_history(
-                new_last_revision_id, None, None))
+            branch.generate_revision_history(new_last_revision_id, None, None)
         return SuccessfulSmartServerResponse(('ok',))
 
 

=== modified file 'bzrlib/tests/blackbox/test_hooks.py'
--- bzrlib/tests/blackbox/test_hooks.py	2011-11-28 18:57:59 +0000
+++ bzrlib/tests/blackbox/test_hooks.py	2012-03-14 15:29:20 +0000
@@ -39,17 +39,17 @@
     def test_hooks_with_unnamed_hook(self):
         self.make_branch('.')
         def foo(): return
-        Branch.hooks.install_named_hook('set_rh', foo, None)
+        Branch.hooks.install_named_hook('post_push', foo, None)
         out, err = self.run_bzr('hooks')
-        self._check_hooks_output(out, {'set_rh': ["No hook name"]})
+        self._check_hooks_output(out, {'post_push': ["No hook name"]})
 
     def test_hooks_with_named_hook(self):
         self.make_branch('.')
         def foo(): return
         name = "Foo Bar Hook"
-        Branch.hooks.install_named_hook('set_rh', foo, name)
+        Branch.hooks.install_named_hook('post_push', foo, name)
         out, err = self.run_bzr('hooks')
-        self._check_hooks_output(out, {'set_rh': [name]})
+        self._check_hooks_output(out, {'post_push': [name]})
 
     def test_hooks_no_branch(self):
         self.run_bzr('hooks')
@@ -57,19 +57,19 @@
     def test_hooks_lazy_with_unnamed_hook(self):
         self.make_branch('.')
         def foo(): return
-        Branch.hooks.install_named_hook_lazy('set_rh',
+        Branch.hooks.install_named_hook_lazy('post_push',
             'bzrlib.tests.blackbox.test_hooks',
             '_foo_hook',
             None)
         out, err = self.run_bzr('hooks')
-        self._check_hooks_output(out, {'set_rh': ["No hook name"]})
+        self._check_hooks_output(out, {'post_push': ["No hook name"]})
         
     def test_hooks_lazy_with_named_hook(self):
         self.make_branch('.')
         def foo(): return
-        Branch.hooks.install_named_hook_lazy('set_rh',
+        Branch.hooks.install_named_hook_lazy('post_push',
             'bzrlib.tests.blackbox.test_hooks',
             '_foo_hook',
             'hook has a name')
         out, err = self.run_bzr('hooks')
-        self._check_hooks_output(out, {'set_rh': ["hook has a name"]})
+        self._check_hooks_output(out, {'post_push': ["hook has a name"]})

=== modified file 'bzrlib/tests/per_branch/__init__.py'
--- bzrlib/tests/per_branch/__init__.py	2012-03-14 12:29:25 +0000
+++ bzrlib/tests/per_branch/__init__.py	2012-03-14 15:29:20 +0000
@@ -168,7 +168,6 @@
         'pull',
         'push',
         'reconcile',
-        'revision_history',
         'revision_id_to_dotted_revno',
         'revision_id_to_revno',
         'sprout',

=== added file 'bzrlib/tests/per_branch/test_bound_sftp.py.THIS'
--- bzrlib/tests/per_branch/test_bound_sftp.py.THIS	1970-01-01 00:00:00 +0000
+++ bzrlib/tests/per_branch/test_bound_sftp.py.THIS	2012-03-14 15:29:20 +0000
@@ -0,0 +1,331 @@
+# Copyright (C) 2005, 2006, 2007, 2009, 2010 Robey Pointer <[e-mail]>, 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
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+"""Tests for branches bound to an sftp branch."""
+
+
+import os
+
+from bzrlib import (
+    branch,
+    controldir,
+    errors,
+    tests,
+    )
+from bzrlib.tests import test_server
+from bzrlib.tests.matchers import RevisionHistoryMatches
+from bzrlib.transport import memory
+
+
+class BoundSFTPBranch(tests.TestCaseWithTransport):
+
+    def setUp(self):
+        tests.TestCaseWithTransport.setUp(self)
+        self.vfs_transport_factory = memory.MemoryServer
+        if self.transport_server is test_server.LocalURLServer:
+            self.transport_server = None
+
+    def create_branches(self):
+        self.build_tree(['base/', 'base/a', 'base/b'])
+        format = controldir.format_registry.make_bzrdir('knit')
+        try:
+            wt_base = controldir.ControlDir.create_standalone_workingtree(
+                self.get_url('base'), format=format)
+        except errors.NotLocalUrl:
+            raise tests.TestSkipped('Not a local URL')
+
+        b_base = wt_base.branch
+
+        wt_base.add('a')
+        wt_base.add('b')
+        wt_base.commit('first', rev_id='r@b-1')
+
+        wt_child = b_base.bzrdir.sprout('child').open_workingtree()
+        self.sftp_base = branch.Branch.open(self.get_url('base'))
+        wt_child.branch.bind(self.sftp_base)
+        # check the branch histories are ready for using in tests.
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+        return b_base, wt_child
+
+    def test_simple_binding(self):
+        self.build_tree(['base/', 'base/a', 'base/b', 'child/'])
+        try:
+            wt_base = controldir.ControlDir.create_standalone_workingtree(
+                self.get_url('base'))
+        except errors.NotLocalUrl:
+            raise tests.TestSkipped('Not a local URL')
+
+        wt_base.add('a')
+        wt_base.add('b')
+        wt_base.commit('first', rev_id='r@b-1')
+
+        b_base = wt_base.branch
+        # manually make a branch we can bind, because the default format
+        # may not be bindable-from, and we want to test the side effects etc
+        # of bondage.
+        format = controldir.format_registry.make_bzrdir('knit')
+        b_child = controldir.ControlDir.create_branch_convenience(
+            'child', format=format)
+        self.assertEqual(None, b_child.get_bound_location())
+        self.assertEqual(None, b_child.get_master_branch())
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+        b_child.bind(sftp_b_base)
+        self.assertEqual(sftp_b_base.base, b_child.get_bound_location())
+        # the bind must not have given b_child history:
+        self.assertThat(RevisionHistoryMatches([]), b_child)
+        # we should be able to update the branch at this point:
+        self.assertEqual(None, b_child.update())
+        # and now there must be history.
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_child)
+        # this line is more of a working tree test line, but - what the hey,
+        # it has work to do.
+        b_child.bzrdir.open_workingtree().update()
+        self.assertPathExists('child/a')
+        self.assertPathExists('child/b')
+
+        b_child.unbind()
+        self.assertEqual(None, b_child.get_bound_location())
+
+    def test_bound_commit(self):
+        b_base, wt_child = self.create_branches()
+
+        with open('child/a', 'wb') as f: f.write('new contents\n')
+        wt_child.commit('modified a', rev_id='r@c-2')
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), b_base)
+
+    def test_bound_commit_fails_when_out_of_date(self):
+        # Make sure commit fails if out of date.
+        b_base, wt_child = self.create_branches()
+
+        with open('base/a', 'wb') as f: f.write('new base contents\n')
+        b_base.bzrdir.open_workingtree().commit('base', rev_id='r@b-2')
+
+        with open('child/b', 'wb') as f: f.write('new b child contents\n')
+        self.assertRaises(errors.BoundBranchOutOfDate,
+                wt_child.commit, 'child', rev_id='r@c-2')
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+
+        # This is all that cmd_update does
+        wt_child.pull(sftp_b_base, overwrite=False)
+
+        wt_child.commit('child', rev_id='r@c-3')
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2', 'r@c-3']),
+                wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2', 'r@c-3']),
+                b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2', 'r@c-3']),
+                sftp_b_base)
+
+    def test_double_binding(self):
+        b_base, wt_child = self.create_branches()
+
+        wt_child2 = wt_child.branch.create_checkout('child2')
+
+        with open('child2/a', 'wb') as f: f.write('new contents\n')
+        self.assertRaises(errors.CommitToDoubleBoundBranch,
+                wt_child2.commit, 'child2', rev_id='r@d-2')
+
+    def test_unbinding(self):
+        b_base, wt_child = self.create_branches()
+
+        # TestCaseWithSFTPServer only allows you to connect one time
+        # to the SFTP server. So we have to create a connection and
+        # keep it around, so that it can be reused
+        __unused_t = self.get_transport()
+
+        wt_base = b_base.bzrdir.open_workingtree()
+        with open('base/a', 'wb') as f: f.write('new base contents\n')
+        wt_base.commit('base', rev_id='r@b-2')
+
+        with open('child/b', 'wb') as f: f.write('new b child contents\n')
+        self.assertRaises(errors.BoundBranchOutOfDate,
+                wt_child.commit, 'child', rev_id='r@c-2')
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+        wt_child.branch.unbind()
+        wt_child.commit('child', rev_id='r@c-2')
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2']), b_base)
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+        self.assertRaises(errors.DivergedBranches,
+                wt_child.branch.bind, sftp_b_base)
+
+    def test_commit_remote_bound(self):
+        # Make sure it is detected if the current base is bound during the
+        # objects lifetime, when the child goes to commit.
+        b_base, wt_child = self.create_branches()
+
+        b_base.bzrdir.sprout('newbase')
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+        sftp_b_newbase = branch.Branch.open(self.get_url('newbase'))
+
+        sftp_b_base.bind(sftp_b_newbase)
+
+        with open('child/a', 'wb') as f: f.write('new contents\n')
+        self.assertRaises(errors.CommitToDoubleBoundBranch,
+                wt_child.commit, 'failure', rev_id='r@c-2')
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), sftp_b_newbase)
+
+    def test_bind_diverged(self):
+        b_base, wt_child = self.create_branches()
+
+        wt_child.branch.unbind()
+        with open('child/a', 'ab') as f: f.write('child contents\n')
+        wt_child_rev = wt_child.commit('child', rev_id='r@c-2')
+
+        self.assertEqual(RevisionHistoryMatches(['r@b-1', 'r@c-2']),
+            wt_child.branch)
+        self.assertEqual(RevisionHistoryMatches(['r@b-1']), b_base)
+
+        with open('base/b', 'ab') as f: f.write('base contents\n')
+        b_base.bzrdir.open_workingtree().commit('base', rev_id='r@b-2')
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2']), b_base)
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+
+        self.assertRaises(errors.DivergedBranches,
+                wt_child.branch.bind, sftp_b_base)
+
+        wt_child.merge_from_branch(sftp_b_base)
+        self.assertEqual([wt_child_rev, 'r@b-2'], wt_child.get_parent_ids())
+        wt_child.commit('merged', rev_id='r@c-3')
+
+        # After a merge, trying to bind again should succeed but not push the
+        # new change.
+        wt_child.branch.bind(sftp_b_base)
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2']), b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2', 'r@c-3']),
+            wt_child.branch)
+
+    def test_bind_parent_ahead_preserves_parent(self):
+        b_base, wt_child = self.create_branches()
+
+        wt_child.branch.unbind()
+
+        with open('a', 'ab') as f: f.write('base changes\n')
+        wt_base = b_base.bzrdir.open_workingtree()
+        wt_base.commit('base', rev_id='r@b-2')
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@b-2']), b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+        wt_child.branch.bind(sftp_b_base)
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+
+        wt_child.branch.unbind()
+
+        # Check and make sure it also works if parent is ahead multiple
+        wt_base.commit('base 3', rev_id='r@b-3', allow_pointless=True)
+        wt_base.commit('base 4', rev_id='r@b-4', allow_pointless=True)
+        wt_base.commit('base 5', rev_id='r@b-5', allow_pointless=True)
+
+        self.assertThat(
+            RevisionHistoryMatches(['r@b-1', 'r@b-2', 'r@b-3', 'r@b-4', 'r@b-5']),
+            b_base)
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+
+        wt_child.branch.bind(sftp_b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), wt_child.branch)
+
+    def test_bind_child_ahead_preserves_child(self):
+        b_base, wt_child = self.create_branches()
+
+        wt_child.branch.unbind()
+
+        wt_child.commit('child', rev_id='r@c-2', allow_pointless=True)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+
+        sftp_b_base = branch.Branch.open(self.get_url('base'))
+        wt_child.branch.bind(sftp_b_base)
+
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+
+        # Check and make sure it also works if child is ahead multiple
+        wt_child.branch.unbind()
+        wt_child.commit('child 3', rev_id='r@c-3', allow_pointless=True)
+        wt_child.commit('child 4', rev_id='r@c-4', allow_pointless=True)
+        wt_child.commit('child 5', rev_id='r@c-5', allow_pointless=True)
+
+        self.assertThat(
+            RevisionHistoryMatches(['r@b-1', 'r@c-2', 'r@c-3', 'r@c-4', 'r@c-5']),
+            wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+
+        wt_child.branch.bind(sftp_b_base)
+        self.assertThat(RevisionHistoryMatches(['r@b-1']), b_base)
+
+    def test_commit_after_merge(self):
+        b_base, wt_child = self.create_branches()
+
+        # We want merge to be able to be a local only
+        # operation, because it does not alter the branch data.
+
+        # But we can't fail afterwards
+
+        wt_other = wt_child.bzrdir.sprout('other').open_workingtree()
+
+        with open('other/c', 'wb') as f: f.write('file c\n')
+        wt_other.add('c')
+        wt_other.commit('adding c', rev_id='r@d-2')
+
+        self.assertFalse(wt_child.branch.repository.has_revision('r@d-2'))
+        self.assertFalse(b_base.repository.has_revision('r@d-2'))
+
+        wt_child.merge_from_branch(wt_other.branch)
+
+        self.assertPathExists('child/c')
+        self.assertEqual(['r@d-2'], wt_child.get_parent_ids()[1:])
+        self.assertTrue(wt_child.branch.repository.has_revision('r@d-2'))
+        self.assertFalse(b_base.repository.has_revision('r@d-2'))
+
+        # Commit should succeed, and cause merged revisions to
+        # be pushed into base
+        wt_child.commit('merge other', rev_id='r@c-2')
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), wt_child.branch)
+        self.assertThat(RevisionHistoryMatches(['r@b-1', 'r@c-2']), b_base)
+        self.assertTrue(b_base.repository.has_revision('r@d-2'))
+
+    def test_commit_fails(self):
+        b_base, wt_child = self.create_branches()
+
+        with open('a', 'ab') as f: f.write('child adds some text\n')
+
+        # this deletes the branch from memory
+        del b_base
+        # and this moves it out of the way on disk
+        os.rename('base', 'hidden_base')
+
+        self.assertRaises(errors.BoundBranchConnectionFailure,
+                wt_child.commit, 'added text', rev_id='r@c-2')
+
+    # TODO: jam 20051231 We need invasive failure tests, so that we can show
+    #       performance even when something fails.
+
+

=== modified file 'bzrlib/tests/per_branch/test_branch.py'
--- bzrlib/tests/per_branch/test_branch.py	2012-02-23 23:26:35 +0000
+++ bzrlib/tests/per_branch/test_branch.py	2012-03-14 15:29:20 +0000
@@ -475,17 +475,6 @@
         checkout = source_branch.create_checkout('c')
         self.assertEqual(rev_id, checkout.last_revision())
 
-    def test_set_revision_history(self):
-        tree = self.make_branch_and_tree('a')
-        tree.commit('a commit', rev_id='rev1')
-        br = tree.branch
-        self.applyDeprecated(symbol_versioning.deprecated_in((2, 4, 0)),
-            br.set_revision_history, ["rev1"])
-        self.assertEquals(br.last_revision(), "rev1")
-        self.applyDeprecated(symbol_versioning.deprecated_in((2, 4, 0)),
-            br.set_revision_history, [])
-        self.assertEquals(br.last_revision(), 'null:')
-
     def test_heads_to_fetch(self):
         # heads_to_fetch is a method that returns a collection of revids that
         # need to be fetched to copy this branch into another repo.  At a

=== modified file 'bzrlib/tests/per_branch/test_check.py'
--- bzrlib/tests/per_branch/test_check.py	2011-04-19 14:17:24 +0000
+++ bzrlib/tests/per_branch/test_check.py	2012-03-14 15:29:20 +0000
@@ -44,18 +44,9 @@
 
         r5 = tree.commit('five')
         # Now, try to set an invalid history
-        try:
-            self.applyDeprecated(deprecated_in((2, 4, 0)),
-                tree.branch.set_revision_history, [r1, r2b, r5])
-            if tree.branch.last_revision_info() != (3, r5):
-                # RemoteBranch silently corrects an impossible revision
-                # history given to set_revision_history.  It can be tricked
-                # with set_last_revision_info though.
-                tree.branch.set_last_revision_info(3, r5)
-        except errors.NotLefthandHistory:
-            # Branch5 allows set_revision_history to be wrong
-            # Branch6 raises NotLefthandHistory, but we can force bogus stuff
-            # with set_last_revision_info
+        if getattr(tree.branch, "_set_revision_history", None) is not None:
+            tree.branch._set_revision_history([r1, r2b, r5])
+        else:
             tree.branch.set_last_revision_info(3, r5)
 
         tree.lock_read()

=== modified file 'bzrlib/tests/per_branch/test_get_revision_id_to_revno_map.py'
--- bzrlib/tests/per_branch/test_get_revision_id_to_revno_map.py	2011-04-19 14:17:24 +0000
+++ bzrlib/tests/per_branch/test_get_revision_id_to_revno_map.py	2012-03-14 15:29:20 +0000
@@ -81,24 +81,6 @@
         finally:
             branch.unlock()
 
-    def test_set_revision_history_when_locked(self):
-        """Calling set_revision_history should reset the cache."""
-        branch, calls = self.get_instrumented_branch()
-        branch.lock_write()
-        try:
-            self.assertEqual({'rev-1':(1,), 'rev-2':(2,), 'rev-3':(3,),
-                              'rev-1.1.1':(1,1,1)
-                             }, branch.get_revision_id_to_revno_map())
-            self.applyDeprecated(deprecated_in((2, 4, 0)),
-                branch.set_revision_history, ['rev-1', 'rev-2'])
-            self.assertEqual({'rev-1':(1,), 'rev-2':(2,)},
-                             branch.get_revision_id_to_revno_map())
-            self.assertEqual({'rev-1':(1,), 'rev-2':(2,)},
-                             branch.get_revision_id_to_revno_map())
-            self.assertEqual(['_gen_revno_map']*2, calls)
-        finally:
-            branch.unlock()
-
     def test_set_last_revision_info_when_locked(self):
         """Calling set_last_revision_info should reset the cache."""
         branch, calls = self.get_instrumented_branch()

=== modified file 'bzrlib/tests/per_branch/test_hooks.py'
--- bzrlib/tests/per_branch/test_hooks.py	2011-05-08 13:46:49 +0000
+++ bzrlib/tests/per_branch/test_hooks.py	2012-03-14 15:29:20 +0000
@@ -70,76 +70,6 @@
             self.assertEqual([expected_params], hook_calls)
 
 
-class TestSetRevisionHistoryHook(ChangeBranchTipTestCase):
-
-    def setUp(self):
-        self.hook_calls = []
-        super(TestSetRevisionHistoryHook, self).setUp()
-
-    def capture_set_rh_hook(self, branch, rev_history):
-        """Capture post set-rh hook calls to self.hook_calls.
-
-        The call is logged, as is some state of the branch.
-        """
-        self.hook_calls.append(
-            ('set_rh', branch, rev_history, branch.is_locked()))
-
-    def test_set_rh_empty_history(self):
-        branch = self.make_branch('source')
-        _mod_branch.Branch.hooks.install_named_hook(
-            'set_rh', self.capture_set_rh_hook, None)
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        expected_params = ('set_rh', branch, [], True)
-        self.assertHookCalls(expected_params, branch)
-
-    def test_set_rh_nonempty_history(self):
-        tree = self.make_branch_and_memory_tree('source')
-        tree.lock_write()
-        tree.add('')
-        tree.commit('another commit', rev_id='f\xc2\xb5')
-        tree.commit('empty commit', rev_id='foo')
-        tree.unlock()
-        branch = tree.branch
-        _mod_branch.Branch.hooks.install_named_hook(
-            'set_rh', self.capture_set_rh_hook, None)
-        # some branches require that their history be set to a revision in the
-        # repository
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, ['f\xc2\xb5'])
-        expected_params =('set_rh', branch, ['f\xc2\xb5'], True)
-        self.assertHookCalls(expected_params, branch)
-
-    def test_set_rh_branch_is_locked(self):
-        branch = self.make_branch('source')
-        _mod_branch.Branch.hooks.install_named_hook(
-            'set_rh', self.capture_set_rh_hook, None)
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        expected_params = ('set_rh', branch, [], True)
-        self.assertHookCalls(expected_params, branch)
-
-    def test_set_rh_calls_all_hooks_no_errors(self):
-        branch = self.make_branch('source')
-        _mod_branch.Branch.hooks.install_named_hook(
-            'set_rh', self.capture_set_rh_hook, None)
-        _mod_branch.Branch.hooks.install_named_hook(
-            'set_rh', self.capture_set_rh_hook, None)
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        expected_calls = [('set_rh', branch, [], True),
-            ('set_rh', branch, [], True),
-            ]
-        if isinstance(branch, remote.RemoteBranch):
-            # For a remote branch, both the server and the client will raise
-            # set_rh, and the server will do so first because that is where
-            # the change takes place.
-            self.assertEqual(expected_calls, self.hook_calls[2:])
-            self.assertEqual(4, len(self.hook_calls))
-        else:
-            self.assertEqual(expected_calls, self.hook_calls)
-
-
 class TestOpen(tests.TestCaseWithMemoryTransport):
 
     def capture_hook(self, branch):
@@ -389,12 +319,6 @@
         self.assertEqual(length, len(self.pre_hook_calls))
         self.assertEqual(length, len(self.post_hook_calls))
 
-    def test_set_revision_history(self):
-        branch = self.make_branch('')
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        self.assertPreAndPostHooksWereInvoked(branch, True)
-
     def test_set_last_revision_info(self):
         branch = self.make_branch('')
         branch.set_last_revision_info(0, revision.NULL_REVISION)

=== removed file 'bzrlib/tests/per_branch/test_revision_history.py'
--- bzrlib/tests/per_branch/test_revision_history.py	2011-10-05 12:45:41 +0000
+++ bzrlib/tests/per_branch/test_revision_history.py	1970-01-01 00:00:00 +0000
@@ -1,231 +0,0 @@
-# Copyright (C) 2007-2010 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
-# the Free Software Foundation; either version 2 of the License, or
-# (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
-
-"""Tests for Branch.revision_history and last_revision."""
-
-from bzrlib import (
-    branch,
-    errors,
-    revision as _mod_revision,
-    )
-from bzrlib.symbol_versioning import deprecated_in
-from bzrlib.tests import (
-    per_branch,
-    TestNotApplicable,
-    )
-
-
-class TestLastRevision(per_branch.TestCaseWithBranch):
-    """Tests for the last_revision property of the branch.
-    """
-
-    def test_set_last_revision_info(self):
-        # based on TestBranch.test_append_revisions, which uses the old
-        # append_revision api
-        wt = self.make_branch_and_tree('tree')
-        wt.commit('f', rev_id='rev1')
-        wt.commit('f', rev_id='rev2')
-        wt.commit('f', rev_id='rev3')
-        br = self.get_branch()
-        br.fetch(wt.branch)
-        br.set_last_revision_info(1, 'rev1')
-        self.assertEquals(
-            self.applyDeprecated(deprecated_in((2, 5, 0)), br.revision_history),
-            ["rev1",])
-        br.set_last_revision_info(3, 'rev3')
-        self.assertEquals(
-            self.applyDeprecated(deprecated_in((2, 5, 0)), br.revision_history),
-            ["rev1", "rev2", "rev3"])
-        # append_revision specifically prohibits some ids;
-        # set_last_revision_info currently does not
-        ## self.assertRaises(errors.ReservedId,
-        ##         br.set_last_revision_info, 4, 'current:')
-
-
-class TestRevisionHistoryCaching(per_branch.TestCaseWithBranch):
-    """Tests for the caching of branches' revision_history.
-
-    When locked, branches should avoid regenerating or rereading
-    revision_history by caching the last value of it.  This is safe because
-    the branch is locked, so nothing can change the revision_history
-    unexpectedly.
-
-    When not locked, obviously the revision_history will need to be regenerated
-    or reread each time.
-
-    We test if revision_history is using the cache by instrumenting the branch's
-    _gen_revision_history method, which is called by Branch.revision_history if
-    the branch does not have a cache of the revision history.
-    """
-
-    def get_instrumented_branch(self):
-        """Get a branch and monkey patch it to log calls to
-        _gen_revision_history.
-
-        :returns: a tuple of (the branch, list that calls will be logged to)
-        """
-        branch = self.get_branch()
-        calls = []
-        real_gen_revision_history = branch._gen_revision_history
-        def fake_gen_revision_history():
-            calls.append('_gen_revision_history')
-            return real_gen_revision_history()
-        branch._gen_revision_history = fake_gen_revision_history
-        return branch, calls
-
-    def test_revision_history_when_unlocked(self):
-        """Repeated calls to revision history will call _gen_revision_history
-        each time when the branch is not locked.
-        """
-        branch, calls = self.get_instrumented_branch()
-        # Repeatedly call revision_history.
-        self.applyDeprecated(deprecated_in((2, 5, 0)), branch.revision_history)
-        self.applyDeprecated(deprecated_in((2, 5, 0)), branch.revision_history)
-        self.assertEqual(
-            ['_gen_revision_history', '_gen_revision_history'], calls)
-
-    def test_revision_history_when_locked(self):
-        """Repeated calls to revision history will only call
-        _gen_revision_history once while the branch is locked.
-        """
-        branch, calls = self.get_instrumented_branch()
-        # Lock the branch, then repeatedly call revision_history.
-        branch.lock_read()
-        try:
-            self.applyDeprecated(deprecated_in((2, 5, 0)),
-                branch.revision_history)
-            self.applyDeprecated(deprecated_in((2, 5, 0)),
-                branch.revision_history)
-            self.assertEqual(['_gen_revision_history'], calls)
-        finally:
-            branch.unlock()
-
-    def test_set_revision_history_when_locked(self):
-        """When the branch is locked, calling set_revision_history should cache
-        the revision history so that a later call to revision_history will not
-        need to call _gen_revision_history.
-        """
-        branch, calls = self.get_instrumented_branch()
-        # Lock the branch, set the revision history, then repeatedly call
-        # revision_history.
-        branch.lock_write()
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        try:
-            self.applyDeprecated(deprecated_in((2, 5, 0)),
-                branch.revision_history)
-            self.assertEqual([], calls)
-        finally:
-            branch.unlock()
-
-    def test_set_revision_history_when_unlocked(self):
-        """When the branch is not locked, calling set_revision_history will not
-        cause the revision history to be cached.
-        """
-        branch, calls = self.get_instrumented_branch()
-        self.applyDeprecated(deprecated_in((2, 4, 0)),
-            branch.set_revision_history, [])
-        self.applyDeprecated(deprecated_in((2, 5, 0)),
-            branch.revision_history)
-        self.assertEqual(['_gen_revision_history'], calls)
-
-    def test_set_last_revision_info_when_locked(self):
-        """When the branch is locked, calling set_last_revision_info should
-        cache the last revision info so that a later call to last_revision_info
-        will not need the revision_history.  Thus the branch will not to call
-        _gen_revision_history in this situation.
-        """
-        a_branch, calls = self.get_instrumented_branch()
-        # Lock the branch, set the last revision info, then call
-        # last_revision_info.
-        a_branch.lock_write()
-        a_branch.set_last_revision_info(0, _mod_revision.NULL_REVISION)
-        del calls[:]
-        try:
-            a_branch.last_revision_info()
-            self.assertEqual([], calls)
-        finally:
-            a_branch.unlock()
-
-    def test_set_last_revision_info_none(self):
-        """Passing None to set_last_revision_info raises an exception."""
-        a_branch = self.get_branch()
-        # Lock the branch, set the last revision info, then call
-        # last_revision_info.
-        a_branch.lock_write()
-        self.addCleanup(a_branch.unlock)
-        self.assertRaises(errors.InvalidRevisionId,
-            a_branch.set_last_revision_info, 0, None)
-
-    def test_set_last_revision_info_uncaches_revision_history_for_format6(self):
-        """On format 6 branches, set_last_revision_info invalidates the revision
-        history cache.
-        """
-        if not isinstance(self.branch_format, branch.BzrBranchFormat6):
-            return
-        a_branch, calls = self.get_instrumented_branch()
-        # Lock the branch, cache the revision history.
-        a_branch.lock_write()
-        self.applyDeprecated(deprecated_in((2, 5, 0)),
-            a_branch.revision_history)
-        # Set the last revision info, clearing the cache.
-        a_branch.set_last_revision_info(0, _mod_revision.NULL_REVISION)
-        del calls[:]
-        try:
-            self.applyDeprecated(deprecated_in((2, 5, 0)),
-                a_branch.revision_history)
-            self.assertEqual(['_gen_revision_history'], calls)
-        finally:
-            a_branch.unlock()
-
-    def test_cached_revision_history_not_accidentally_mutable(self):
-        """When there's a cached version of the history, revision_history
-        returns a copy of the cached data so that callers cannot accidentally
-        corrupt the cache.
-        """
-        branch = self.get_branch()
-        # Lock the branch, then repeatedly call revision_history, mutating the
-        # results.
-        branch.lock_read()
-        try:
-            # The first time the data returned will not be in the cache.
-            history = self.applyDeprecated(deprecated_in((2, 5, 0)),
-                branch.revision_history)
-            history.append('one')
-            # The second time the data comes from the cache.
-            history = self.applyDeprecated(deprecated_in((2, 5, 0)),
-                branch.revision_history)
-            history.append('two')
-            # The revision_history() should still be unchanged, even though
-            # we've mutated the return values from earlier calls.
-            self.assertEqual([], self.applyDeprecated(
-                deprecated_in((2, 5, 0)), branch.revision_history))
-        finally:
-            branch.unlock()
-
-
-class TestRevisionHistory(per_branch.TestCaseWithBranch):
-
-    def test_parent_ghost(self):
-        tree = self.make_branch_and_tree('tree')
-        if not tree.branch.repository._format.supports_ghosts:
-            raise TestNotApplicable("repository format does not support ghosts")
-        tree.add_parent_tree_id('ghost-revision',
-                                allow_leftmost_as_ghost=True)
-        tree.commit('first non-ghost commit', rev_id='non-ghost-revision')
-        self.assertEqual(['non-ghost-revision'],
-                         self.applyDeprecated(deprecated_in((2, 5, 0)),
-                            tree.branch.revision_history))

=== modified file 'bzrlib/tests/per_workingtree/test_workingtree.py'
--- bzrlib/tests/per_workingtree/test_workingtree.py	2012-02-23 23:26:35 +0000
+++ bzrlib/tests/per_workingtree/test_workingtree.py	2012-03-14 15:29:20 +0000
@@ -343,19 +343,16 @@
         wt.set_last_revision('null:')
         wt.commit('A', allow_pointless=True, rev_id='A')
         self.assertEqual(['A'], wt.get_parent_ids())
-        # None is aways in the branch
+        # null: is aways in the branch
         wt.set_last_revision('null:')
         self.assertEqual([], wt.get_parent_ids())
         # and now we can set it to 'A'
         # because some formats mutate the branch to set it on the tree
         # we need to alter the branch to let this pass.
-        try:
-            self.applyDeprecated(symbol_versioning.deprecated_in((2, 4, 0)),
-                wt.branch.set_revision_history, ['A', 'B'])
-        except errors.NoSuchRevision, e:
-            self.assertEqual('B', e.revision)
+        if getattr(wt.branch, "_set_revision_history", None) is None:
             raise TestSkipped("Branch format does not permit arbitrary"
                               " history")
+        wt.branch._set_revision_history(['A', 'B'])
         wt.set_last_revision('A')
         self.assertEqual(['A'], wt.get_parent_ids())
         self.assertRaises(errors.ReservedId, wt.set_last_revision, 'A:')

=== modified file 'bzrlib/tests/test_branch.py'
--- bzrlib/tests/test_branch.py	2012-03-14 10:52:42 +0000
+++ bzrlib/tests/test_branch.py	2012-03-14 15:29:20 +0000
@@ -340,23 +340,6 @@
         self.assertPathDoesNotExist('a/.bzr/branch/bound')
         self.assertEqual('ftp://example.com', branch.get_bound_location())
 
-    def test_set_revision_history(self):
-        builder = self.make_branch_builder('.', format=self.get_format_name())
-        builder.build_snapshot('foo', None,
-            [('add', ('', None, 'directory', None))],
-            message='foo')
-        builder.build_snapshot('bar', None, [], message='bar')
-        branch = builder.get_branch()
-        branch.lock_write()
-        self.addCleanup(branch.unlock)
-        self.applyDeprecated(symbol_versioning.deprecated_in((2, 4, 0)),
-            branch.set_revision_history, ['foo', 'bar'])
-        self.applyDeprecated(symbol_versioning.deprecated_in((2, 4, 0)),
-                branch.set_revision_history, ['foo'])
-        self.assertRaises(errors.NotLefthandHistory,
-            self.applyDeprecated, symbol_versioning.deprecated_in((2, 4, 0)),
-            branch.set_revision_history, ['bar'])
-
     def do_checkout_test(self, lightweight=False):
         tree = self.make_branch_and_tree('source',
             format=self.get_format_name_subtree())
@@ -569,7 +552,6 @@
     def test_constructor(self):
         """Check that creating a BranchHooks instance has the right defaults."""
         hooks = _mod_branch.BranchHooks()
-        self.assertTrue("set_rh" in hooks, "set_rh not in %s" % hooks)
         self.assertTrue("post_push" in hooks, "post_push not in %s" % hooks)
         self.assertTrue("post_commit" in hooks, "post_commit not in %s" % hooks)
         self.assertTrue("pre_commit" in hooks, "pre_commit not in %s" % hooks)

=== modified file 'bzrlib/tests/test_remote.py'
--- bzrlib/tests/test_remote.py	2012-02-23 19:45:15 +0000
+++ bzrlib/tests/test_remote.py	2012-03-14 15:29:20 +0000
@@ -121,15 +121,6 @@
         self.local_wt.commit(message='test commit', rev_id=revid)
         self.assertTrue(repo.has_revision(revid))
 
-    def test_remote_branch_revision_history(self):
-        b = BzrDir.open_from_transport(self.transport).open_branch()
-        self.assertEqual([],
-            self.applyDeprecated(deprecated_in((2, 5, 0)), b.revision_history))
-        r1 = self.local_wt.commit('1st commit')
-        r2 = self.local_wt.commit('1st commit', rev_id=u'\xc8'.encode('utf8'))
-        self.assertEqual([r1, r2],
-            self.applyDeprecated(deprecated_in((2, 5, 0)), b.revision_history))
-
     def test_find_correct_format(self):
         """Should open a RemoteBzrDir over a RemoteTransport"""
         fmt = BzrDirFormat.find_format(self.transport)

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt	2012-03-14 14:39:16 +0000
+++ doc/en/release-notes/bzr-2.6.txt	2012-03-14 15:29:20 +0000
@@ -91,6 +91,10 @@
 * New convenience API method ``WorkingTree.get_config_stack``.
   (Jelmer Vernooij)
 
+* Remove deprecated ``Branch.set_revision_history`` and
+  ``Branch.revision_history`` methods and the ``set_rh``
+  hook on ``Branch``. (Jelmer Vernooij)
+
 * Remove 
   ``branch.PullResult.__int__`` deprecated in 2.3.0,
   ``branch.PushResult.__int__`` deprecated in 2.3.0,