Rev 2249: New branch hooks: post_push, post_pull, post_commit, post_uncommit. These in file:///home/robertc/source/baz/branch-hooks/

Robert Collins robertc at robertcollins.net
Tue Feb 6 02:34:33 GMT 2007


------------------------------------------------------------
revno: 2249
revision-id: robertc at robertcollins.net-20070206023342-hv5o5qh6pdktiwbc
parent: robertc at robertcollins.net-20070205175221-hi6pbqbplnekjoxh
committer: Robert Collins <robertc at robertcollins.net>
branch nick: branch-hooks
timestamp: Tue 2007-02-06 13:33:42 +1100
message:
  New branch hooks: post_push, post_pull, post_commit, post_uncommit. These
  complement the set_rh hook by allowing different actions, and awareness of
  the prior state of the branch, for operations where this matters.
  
  Fix the inability to do a bzr pull --overwrite of a heavyweight checkout.
  
  Add branch implementation tests for uncommit and commit.
  
  (Robert Collins)
added:
  bzrlib/tests/branch_implementations/test_commit.py test_commit.py-20070206022134-117z1i5b644p63r0-1
  bzrlib/tests/branch_implementations/test_uncommit.py test_uncommit.py-20070205180410-ge7058d9138mvq3x-1
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/branch.py               branch.py-20050309040759-e4baf4e0d046576e
  bzrlib/commit.py               commit.py-20050511101309-79ec1a0168e0e825
  bzrlib/tests/blackbox/test_bound_branches.py test_bound_branches.py-20051109215527-2373188ad566c205
  bzrlib/tests/branch_implementations/__init__.py __init__.py-20060123013057-b12a52c3f361daf4
  bzrlib/tests/branch_implementations/test_branch.py testbranch.py-20050711070244-121d632bc37d7253
  bzrlib/tests/branch_implementations/test_hooks.py test_hooks.py-20070129154855-blhpwxmvjs07waei-1
  bzrlib/tests/branch_implementations/test_pull.py test_pull.py-20060410103942-83c35b26657414fc
  bzrlib/tests/branch_implementations/test_push.py test_push.py-20070130153159-fhfap8uoifevg30j-1
  bzrlib/tests/test_branch.py    test_branch.py-20060116013032-97819aa07b8ab3b5
  bzrlib/uncommit.py             uncommit.py-20050626215513-5ec509fa425b305c
=== added file 'bzrlib/tests/branch_implementations/test_commit.py'
--- a/bzrlib/tests/branch_implementations/test_commit.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/branch_implementations/test_commit.py	2007-02-06 02:33:42 +0000
@@ -0,0 +1,114 @@
+# Copyright (C) 2007 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""Tests for the contract of commit on branches."""
+
+from bzrlib.branch import Branch
+from bzrlib import errors
+from bzrlib.tests.branch_implementations.test_branch import TestCaseWithBranch
+from bzrlib.revision import NULL_REVISION
+from bzrlib.transport import get_transport
+
+
+class TestCommit(TestCaseWithBranch):
+
+    def test_commit_nicks(self):
+        """Nicknames are committed to the revision"""
+        get_transport(self.get_url()).mkdir('bzr.dev')
+        wt = self.make_branch_and_tree('bzr.dev')
+        branch = wt.branch
+        branch.nick = "My happy branch"
+        wt.commit('My commit respect da nick.')
+        committed = branch.repository.get_revision(branch.last_revision())
+        self.assertEqual(committed.properties["branch-nick"], 
+                         "My happy branch")
+
+
+class TestCommitHook(TestCaseWithBranch):
+
+    def setUp(self):
+        self.hook_calls = []
+        TestCaseWithBranch.setUp(self)
+
+    def capture_post_commit_hook(self, local, master, old_revno,
+        old_revid, new_revno, new_revid):
+        """Capture post commit hook calls to self.hook_calls.
+        
+        The call is logged, as is some state of the two branches.
+        """
+        if local:
+            local_locked = local.is_locked()
+            local_base = local.base
+        else:
+            local_locked = None
+            local_base = None
+        self.hook_calls.append(
+            ('post_commit', local_base, master.base, old_revno, old_revid,
+             new_revno, new_revid, local_locked, master.is_locked()))
+
+    def test_post_commit_to_origin(self):
+        tree = self.make_branch_and_memory_tree('branch')
+        Branch.hooks.install_hook('post_commit',
+            self.capture_post_commit_hook)
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('a revision')
+        # should have had one notification, from origin, and
+        # have the branch locked at notification time.
+        self.assertEqual([
+            ('post_commit', None, tree.branch.base, 0, NULL_REVISION, 1, revid,
+             None, True)
+            ],
+            self.hook_calls)
+        tree.unlock()
+
+    def test_post_commit_bound(self):
+        master = self.make_branch('master')
+        tree = self.make_branch_and_memory_tree('local')
+        try:
+            tree.branch.bind(master)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
+        Branch.hooks.install_hook('post_commit',
+            self.capture_post_commit_hook)
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('a revision')
+        # with a bound branch, local is set.
+        self.assertEqual([
+            ('post_commit', tree.branch.base, master.base, 0, NULL_REVISION,
+             1, revid, True, True)
+            ],
+            self.hook_calls)
+        tree.unlock()
+
+    def test_post_commit_not_to_origin(self):
+        tree = self.make_branch_and_memory_tree('branch')
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('first revision')
+        Branch.hooks.install_hook('post_commit',
+            self.capture_post_commit_hook)
+        revid2 = tree.commit('second revision')
+        # having committed from up the branch, we should get the
+        # before and after revnos and revids correctly.
+        self.assertEqual([
+            ('post_commit', None, tree.branch.base, 1, revid, 2, revid2,
+             None, True)
+            ],
+            self.hook_calls)
+        tree.unlock()

=== added file 'bzrlib/tests/branch_implementations/test_uncommit.py'
--- a/bzrlib/tests/branch_implementations/test_uncommit.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/branch_implementations/test_uncommit.py	2007-02-06 02:33:42 +0000
@@ -0,0 +1,108 @@
+# Copyright (C) 2007 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""Tests for the contract of uncommit on branches.
+
+Note that uncommit currently is not a branch method; it should be.
+"""
+
+from bzrlib.branch import Branch
+from bzrlib import errors
+from bzrlib.tests.branch_implementations.test_branch import TestCaseWithBranch
+from bzrlib.uncommit import uncommit
+
+
+class TestUncommitHook(TestCaseWithBranch):
+
+    def setUp(self):
+        self.hook_calls = []
+        TestCaseWithBranch.setUp(self)
+
+    def capture_post_uncommit_hook(self, local, master, old_revno,
+        old_revid, new_revno, new_revid):
+        """Capture post uncommit hook calls to self.hook_calls.
+        
+        The call is logged, as is some state of the two branches.
+        """
+        if local:
+            local_locked = local.is_locked()
+            local_base = local.base
+        else:
+            local_locked = None
+            local_base = None
+        self.hook_calls.append(
+            ('post_uncommit', local_base, master.base, old_revno, old_revid,
+             new_revno, new_revid, local_locked, master.is_locked()))
+
+    def test_post_uncommit_to_origin(self):
+        tree = self.make_branch_and_memory_tree('branch')
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('a revision')
+        tree.unlock()
+        Branch.hooks.install_hook('post_uncommit',
+            self.capture_post_uncommit_hook)
+        uncommit(tree.branch)
+        # with nothing left we should still get a notification, and
+        # have the branch locked at notification time.
+        self.assertEqual([
+            ('post_uncommit', None, tree.branch.base, 1, revid,
+             0, None, None, True)
+            ],
+            self.hook_calls)
+
+    def test_post_uncommit_bound(self):
+        master = self.make_branch('master')
+        tree = self.make_branch_and_memory_tree('local')
+        try:
+            tree.branch.bind(master)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('a revision')
+        tree.unlock()
+        Branch.hooks.install_hook('post_uncommit',
+            self.capture_post_uncommit_hook)
+        uncommit(tree.branch)
+        # with nothing left we should still get a notification, and
+        # have the branch locked at notification time.
+        self.assertEqual([
+            ('post_uncommit', tree.branch.base, master.base, 1, revid,
+             0, None, True, True)
+            ],
+            self.hook_calls)
+
+    def test_post_uncommit_not_to_origin(self):
+        tree = self.make_branch_and_memory_tree('branch')
+        tree.lock_write()
+        tree.add('')
+        revid = tree.commit('first revision')
+        revid2 = tree.commit('second revision')
+        revid3 = tree.commit('third revision')
+        tree.unlock()
+        Branch.hooks.install_hook('post_uncommit',
+            self.capture_post_uncommit_hook)
+        uncommit(tree.branch, revno=2)
+        # having uncommitted from up the branch, we should get the
+        # before and after revnos and revids correctly.
+        self.assertEqual([
+            ('post_uncommit', None, tree.branch.base, 3, revid3,
+             1, revid, None, True)
+            ],
+            self.hook_calls)
+

=== modified file 'NEWS'
--- a/NEWS	2007-02-05 17:52:21 +0000
+++ b/NEWS	2007-02-06 02:33:42 +0000
@@ -22,16 +22,20 @@
     * ``bzr help global-options`` describes the global options.
       (Aaron Bentley)
 
+    * ``bzr pull --overwrite`` will now correctly overwrite checkouts.
+      (Robert Collins)
+
   INTERNALS:
 
     * Reserved ids (any revision-id ending in a colon) are rejected by
       versionedfiles, repositories, branches, and working trees
       (Aaron Bentley)
 
-    * New Branch hooks facility, with one initial hook 'set_rh' which triggers
-      whenever the revision history is set. This allows triggering on e.g.
-      push, pull, commit, and so on. Developed for use with the branchrss
-      plugin. See bzrlib.branch.BranchHooks for more details. (Robert Collins)
+    * New easier to use Branch hooks facility. There are five initial hooks,
+      all documented in bzrlib.branch.BranchHooks.__init__ - 'set_rh',
+      'post_push', 'post_pull', 'post_commit', 'post_uncommit'. These hooks
+      fire after the matching operation on a branch has taken place, and were
+      originally added for the branchrss plugin. (Robert Collins)
 
     * New method ``Branch.push()`` which should be used when pushing from a
       branch as it makes performance and policy decisions to match the UI

=== modified file 'bzrlib/branch.py'
--- a/bzrlib/branch.py	2007-02-05 17:52:21 +0000
+++ b/bzrlib/branch.py	2007-02-06 02:33:42 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2005, 2006 Canonical Ltd
+# Copyright (C) 2005, 2006, 2007 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
@@ -259,7 +259,7 @@
         if config is None:
             config = self.get_config()
         
-        return self.repository.get_commit_builder(self, parents, config, 
+        return self.repository.get_commit_builder(self, parents, config,
             timestamp, timezone, committer, revprops, revision_id)
 
     def get_master_branch(self):
@@ -750,11 +750,39 @@
         notified.
         """
         dict.__init__(self)
+        # Introduced in 0.15:
         # invoked whenever the revision history has been set
         # with set_revision_history. The api signature is
         # (branch, revision_history), and the branch will
-        # be write-locked. Introduced in 0.15.
+        # be write-locked.
         self['set_rh'] = []
+        # invoked after a push operation completes.
+        # the api signature is
+        # (source, local, master, old_revno, old_revid, new_revno, new_revid)
+        # where local is the local branch or None, master is the target 
+        # master branch, and the rest should be self explanatory. The source
+        # is read locked and the target branches write locked. Source will
+        # be the local low-latency branch.
+        self['post_push'] = []
+        # invoked after a pull operation completes.
+        # the api signature is
+        # (source, local, master, old_revno, old_revid, new_revno, new_revid)
+        # where local is the local branch or None, master is the target 
+        # master branch, and the rest should be self explanatory. The source
+        # is read locked and the target branches write locked. The local
+        # branch is the low-latency branch.
+        self['post_pull'] = []
+        # invoked after a commit operation completes.
+        # the api signature is 
+        # (local, master, old_revno, old_revid, new_revno, new_revid)
+        # old_revid is NULL_REVISION for the first commit to a branch.
+        self['post_commit'] = []
+        # invoked after a uncommit operation completes.
+        # the api signature is
+        # (local, master, old_revno, old_revid, new_revno, new_revid) where
+        # local is the local branch or None, master is the target branch,
+        # and an empty branch recieves new_revno of 0, new_revid of None.
+        self['post_uncommit'] = []
 
     def install_hook(self, hook_name, a_callable):
         """Install a_callable in to the hook hook_name.
@@ -1249,11 +1277,18 @@
         return self.bzrdir.open_workingtree()
 
     @needs_write_lock
-    def pull(self, source, overwrite=False, stop_revision=None):
-        """See Branch.pull."""
+    def pull(self, source, overwrite=False, stop_revision=None,
+        _hook_master=None, _run_hooks=True):
+        """See Branch.pull.
+
+        :param _hook_master: Private parameter - set the branch to 
+            be supplied as the master to push hooks.
+        :param _run_hooks: Private parameter - allow disabling of
+            hooks, used when pushing to a master branch.
+        """
         source.lock_read()
         try:
-            old_count = self.last_revision_info()[0]
+            old_count, old_tip = self.last_revision_info()
             try:
                 self.update_revisions(source, stop_revision)
             except DivergedBranches:
@@ -1261,17 +1296,33 @@
                     raise
             if overwrite:
                 self.set_revision_history(source.revision_history())
-            new_count = self.last_revision_info()[0]
+            new_count, new_tip = self.last_revision_info()
+            if _run_hooks:
+                if _hook_master:
+                    _hook_local = self
+                else:
+                    _hook_master = self
+                    _hook_local = None
+                for hook in Branch.hooks['post_pull']:
+                    hook(source, _hook_local, _hook_master, old_count, old_tip,
+                        new_count, new_tip)
             return new_count - old_count
         finally:
             source.unlock()
 
     @needs_read_lock
-    def push(self, target, overwrite=False, stop_revision=None):
-        """See Branch.push."""
+    def push(self, target, overwrite=False, stop_revision=None,
+        _hook_master=None, _run_hooks=True):
+        """See Branch.push.
+        
+        :param _hook_master: Private parameter - set the branch to 
+            be supplied as the master to push hooks.
+        :param _run_hooks: Private parameter - allow disabling of
+            hooks, used when pushing to a master branch.
+        """
         target.lock_write()
         try:
-            old_count = len(target.revision_history())
+            old_count, old_tip = target.last_revision_info()
             try:
                 target.update_revisions(self, stop_revision)
             except DivergedBranches:
@@ -1279,7 +1330,16 @@
                     raise
             if overwrite:
                 target.set_revision_history(self.revision_history())
-            new_count = len(target.revision_history())
+            new_count, new_tip = target.last_revision_info()
+            if _run_hooks:
+                if _hook_master:
+                    _hook_local = target
+                else:
+                    _hook_master = target
+                    _hook_local = None
+                for hook in Branch.hooks['post_push']:
+                    hook(self, _hook_local, _hook_master, old_count, old_tip,
+                        new_count, new_tip)
             return new_count - old_count
         finally:
             target.unlock()
@@ -1361,32 +1421,53 @@
                                          _repository=_repository)
         
     @needs_write_lock
-    def pull(self, source, overwrite=False, stop_revision=None):
-        """Extends branch.pull to be bound branch aware."""
+    def pull(self, source, overwrite=False, stop_revision=None,
+        _run_hooks=True):
+        """Extends branch.pull to be bound branch aware.
+        
+        :param _run_hooks: Private parameter used to force hook running
+            off during bound branch double-pushing.
+        """
         bound_location = self.get_bound_location()
-        if source.base != bound_location:
+        master_branch = None
+        if bound_location and source.base != bound_location:
             # not pulling from master, so we need to update master.
             master_branch = self.get_master_branch()
-            if master_branch:
-                master_branch.pull(source)
-                source = master_branch
-        return super(BzrBranch5, self).pull(source, overwrite, stop_revision)
+            master_branch.lock_write()
+        try:
+            if master_branch:
+                # pull from source into master.
+                master_branch.pull(source, overwrite, stop_revision,
+                    _run_hooks=False)
+            return super(BzrBranch5, self).pull(source, overwrite,
+                stop_revision, _hook_master=master_branch,
+                _run_hooks=_run_hooks)
+        finally:
+            if master_branch:
+                master_branch.unlock()
 
     @needs_write_lock
     def push(self, target, overwrite=False, stop_revision=None):
         """Updates branch.push to be bound branch aware."""
         bound_location = target.get_bound_location()
-        if target.base != bound_location:
+        master_branch = None
+        if bound_location and target.base != bound_location:
             # not pushing to master, so we need to update master.
             master_branch = target.get_master_branch()
+            master_branch.lock_write()
+        try:
             if master_branch:
                 # push into the master from this branch.
                 super(BzrBranch5, self).push(master_branch, overwrite,
-                    stop_revision)
-        # and push into the target branch from this. Note that we push from
-        # this branch again, because its considered the highest bandwidth
-        # repository.
-        return super(BzrBranch5, self).push(target, overwrite, stop_revision)
+                    stop_revision, _run_hooks=False)
+            # and push into the target branch from this. Note that we push from
+            # this branch again, because its considered the highest bandwidth
+            # repository.
+            return super(BzrBranch5, self).push(target, overwrite,
+                stop_revision, _hook_master=master_branch)
+        finally:
+            if master_branch:
+                master_branch.unlock()
 
     def get_bound_location(self):
         try:

=== modified file 'bzrlib/commit.py'
--- a/bzrlib/commit.py	2007-02-01 14:47:47 +0000
+++ b/bzrlib/commit.py	2007-02-06 02:33:42 +0000
@@ -60,6 +60,7 @@
     errors,
     tree,
     )
+from bzrlib.branch import Branch
 import bzrlib.config
 from bzrlib.errors import (BzrError, PointlessCommit,
                            ConflictsInTree,
@@ -331,6 +332,8 @@
             # now the work tree is up to date with the branch
             
             self.reporter.completed(self.branch.revno(), self.rev_id)
+            # old style commit hooks - should be deprecated ? (obsoleted in
+            # 0.15)
             if self.config.post_commit() is not None:
                 hooks = self.config.post_commit().split(' ')
                 # this would be nicer with twisted.python.reflect.namedAny
@@ -339,6 +342,25 @@
                                   {'branch':self.branch,
                                    'bzrlib':bzrlib,
                                    'rev_id':self.rev_id})
+            # new style commit hooks:
+            if not self.bound_branch:
+                hook_master = self.branch
+                hook_local = None
+            else:
+                hook_master = self.master_branch
+                hook_local = self.branch
+            new_revno = self.branch.revno()
+            # With bound branches, when the master is behind the local branch,
+            # the 'old_revno' and old_revid values here are incorrect.
+            # XXX: FIXME ^. RBC 20060206
+            old_revno = new_revno - 1
+            if self.parents:
+                old_revid = self.parents[0]
+            else:
+                old_revid = bzrlib.revision.NULL_REVISION
+            for hook in Branch.hooks['post_commit']:
+                hook(hook_local, hook_master, old_revno, old_revid, new_revno,
+                    self.rev_id)
             self._emit_progress_update()
         finally:
             self._cleanup()

=== modified file 'bzrlib/tests/blackbox/test_bound_branches.py'
--- a/bzrlib/tests/blackbox/test_bound_branches.py	2007-01-05 17:12:03 +0000
+++ b/bzrlib/tests/blackbox/test_bound_branches.py	2007-02-06 02:33:42 +0000
@@ -349,7 +349,8 @@
 
         bzr('cat-revision', new_rev_id)
 
-    def test_pull_overwrite_fails(self):
+    def test_pull_overwrite(self):
+        # XXX: This test should be moved to branch-implemenations/test_pull
         bzr = self.run_bzr
         self.create_branches()
 
@@ -373,17 +374,8 @@
         self.check_revno(2)
         self.check_revno(2, '../base')
 
-        # It might be possible that we want pull --overwrite to
-        # actually succeed.
-        # If we want it, just change this test to make sure that 
-        # both base and child are updated properly
-        bzr('pull', '--overwrite', '../other', retcode=3)
-
-        # It should fail without changing the local revision
-        self.check_revno(2)
-        self.check_revno(2, '../base')
-
-    # TODO: jam 20051230 Test that commit & pull fail when the branch we 
-    #       are bound to is not available
-
-
+        bzr('pull', '--overwrite', '../other')
+
+        # both the local and master should have been updated.
+        self.check_revno(4)
+        self.check_revno(4, '../base')

=== modified file 'bzrlib/tests/branch_implementations/__init__.py'
--- a/bzrlib/tests/branch_implementations/__init__.py	2007-02-05 17:52:21 +0000
+++ b/bzrlib/tests/branch_implementations/__init__.py	2007-02-06 02:33:42 +0000
@@ -42,6 +42,7 @@
         'bzrlib.tests.branch_implementations.test_bound_sftp',
         'bzrlib.tests.branch_implementations.test_branch',
         'bzrlib.tests.branch_implementations.test_break_lock',
+        'bzrlib.tests.branch_implementations.test_commit',
         'bzrlib.tests.branch_implementations.test_hooks',
         'bzrlib.tests.branch_implementations.test_http',
         'bzrlib.tests.branch_implementations.test_last_revision_info',
@@ -50,6 +51,7 @@
         'bzrlib.tests.branch_implementations.test_permissions',
         'bzrlib.tests.branch_implementations.test_pull',
         'bzrlib.tests.branch_implementations.test_push',
+        'bzrlib.tests.branch_implementations.test_uncommit',
         'bzrlib.tests.branch_implementations.test_update',
         ]
     adapter = BranchTestProviderAdapter(

=== modified file 'bzrlib/tests/branch_implementations/test_branch.py'
--- a/bzrlib/tests/branch_implementations/test_branch.py	2007-01-11 17:20:25 +0000
+++ b/bzrlib/tests/branch_implementations/test_branch.py	2007-02-06 02:33:42 +0000
@@ -327,17 +327,6 @@
         branch.nick = u"\u1234"
         self.assertEqual(branch.nick, u"\u1234")
 
-    def test_commit_nicks(self):
-        """Nicknames are committed to the revision"""
-        get_transport(self.get_url()).mkdir('bzr.dev')
-        wt = self.make_branch_and_tree('bzr.dev')
-        branch = wt.branch
-        branch.nick = "My happy branch"
-        wt.commit('My commit respect da nick.')
-        committed = branch.repository.get_revision(branch.last_revision())
-        self.assertEqual(committed.properties["branch-nick"], 
-                         "My happy branch")
-
     def test_create_open_branch_uses_repository(self):
         try:
             repo = self.make_repository('.', shared=True)

=== modified file 'bzrlib/tests/branch_implementations/test_hooks.py'
--- a/bzrlib/tests/branch_implementations/test_hooks.py	2007-01-30 10:41:04 +0000
+++ b/bzrlib/tests/branch_implementations/test_hooks.py	2007-02-06 02:33:42 +0000
@@ -20,48 +20,47 @@
 from bzrlib.tests import TestCaseWithMemoryTransport
 
 
-class TestPushHook(TestCaseWithMemoryTransport):
+class TestSetRevisionHistoryHook(TestCaseWithMemoryTransport):
 
     def setUp(self):
         self.hook_calls = []
         TestCaseWithMemoryTransport.setUp(self)
 
     def capture_set_rh_hook(self, branch, rev_history):
-        """Capture post push hook calls to self.hook_calls.
+        """Capture post set-rh hook calls to self.hook_calls.
         
-        The call is logged, as is some state of the two branches.
+        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')
-        Branch.hooks['set_rh'].append(self.capture_set_rh_hook)
+        Branch.hooks.install_hook('set_rh', self.capture_set_rh_hook)
         branch.set_revision_history([])
         self.assertEqual(self.hook_calls,
             [('set_rh', branch, [], True)])
 
     def test_set_rh_nonempty_history(self):
         branch = self.make_branch('source')
-        Branch.hooks['set_rh'].append(self.capture_set_rh_hook)
+        Branch.hooks.install_hook('set_rh', self.capture_set_rh_hook)
         branch.set_revision_history([u'foo'])
         self.assertEqual(self.hook_calls,
             [('set_rh', branch, [u'foo'], True)])
 
     def test_set_rh_branch_is_locked(self):
         branch = self.make_branch('source')
-        Branch.hooks['set_rh'].append(self.capture_set_rh_hook)
+        Branch.hooks.install_hook('set_rh', self.capture_set_rh_hook)
         branch.set_revision_history([])
         self.assertEqual(self.hook_calls,
             [('set_rh', branch, [], True)])
 
     def test_set_rh_calls_all_hooks_no_errors(self):
         branch = self.make_branch('source')
-        Branch.hooks['set_rh'].append(self.capture_set_rh_hook)
-        Branch.hooks['set_rh'].append(self.capture_set_rh_hook)
+        Branch.hooks.install_hook('set_rh', self.capture_set_rh_hook)
+        Branch.hooks.install_hook('set_rh', self.capture_set_rh_hook)
         branch.set_revision_history([])
         self.assertEqual(self.hook_calls,
             [('set_rh', branch, [], True),
              ('set_rh', branch, [], True),
             ])
-

=== modified file 'bzrlib/tests/branch_implementations/test_pull.py'
--- a/bzrlib/tests/branch_implementations/test_pull.py	2007-01-30 20:58:25 +0000
+++ b/bzrlib/tests/branch_implementations/test_pull.py	2007-02-06 02:33:42 +0000
@@ -20,10 +20,12 @@
 
 from bzrlib.branch import Branch
 from bzrlib import errors
-from bzrlib.tests import TestCaseWithTransport
-
-
-class TestPull(TestCaseWithTransport):
+from bzrlib.memorytree import MemoryTree
+from bzrlib.revision import NULL_REVISION
+from bzrlib.tests.branch_implementations.test_branch import TestCaseWithBranch
+
+
+class TestPull(TestCaseWithBranch):
 
     def test_pull_convergence_simple(self):
         # when revisions are pulled, the left-most accessible parents must 
@@ -77,3 +79,82 @@
         # try to pull, which should raise a BoundBranchConnectionFailure.
         self.assertRaises(errors.BoundBranchConnectionFailure,
                 checkout.branch.pull, other.branch)
+
+
+class TestPullHook(TestCaseWithBranch):
+
+    def setUp(self):
+        self.hook_calls = []
+        TestCaseWithBranch.setUp(self)
+
+    def capture_post_pull_hook(self, source, local, master, old_revno,
+        old_revid, new_revno, new_revid):
+        """Capture post pull hook calls to self.hook_calls.
+        
+        The call is logged, as is some state of the two branches.
+        """
+        if local:
+            local_locked = local.is_locked()
+            local_base = local.base
+        else:
+            local_locked = None
+            local_base = None
+        self.hook_calls.append(
+            ('post_pull', source, local_base, master.base, old_revno, old_revid,
+             new_revno, new_revid, source.is_locked(), local_locked,
+             master.is_locked()))
+
+    def test_post_pull_empty_history(self):
+        target = self.make_branch('target')
+        source = self.make_branch('source')
+        Branch.hooks.install_hook('post_pull', self.capture_post_pull_hook)
+        target.pull(source)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_pull', source, None, target.base, 0, NULL_REVISION,
+             0, NULL_REVISION, True, None, True)
+            ],
+            self.hook_calls)
+
+    def test_post_pull_bound_branch(self):
+        # pulling to a bound branch should pass in the master branch to the
+        # hook, allowing the correct number of emails to be sent, while still
+        # allowing hooks that want to modify the target to do so to both 
+        # instances.
+        target = self.make_branch('target')
+        local = self.make_branch('local')
+        try:
+            local.bind(target)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
+        source = self.make_branch('source')
+        Branch.hooks.install_hook('post_pull', self.capture_post_pull_hook)
+        local.pull(source)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_pull', source, local.base, target.base, 0, NULL_REVISION,
+             0, NULL_REVISION, True, True, True)
+            ],
+            self.hook_calls)
+
+    def test_post_pull_nonempty_history(self):
+        target = self.make_branch_and_memory_tree('target')
+        target.lock_write()
+        target.add('')
+        rev1 = target.commit('rev 1')
+        target.unlock()
+        sourcedir = target.bzrdir.clone(self.get_url('source'))
+        source = MemoryTree.create_on_branch(sourcedir.open_branch())
+        rev2 = source.commit('rev 2')
+        Branch.hooks.install_hook('post_pull', self.capture_post_pull_hook)
+        target.branch.pull(source.branch)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_pull', source.branch, None, target.branch.base, 1, rev1,
+             2, rev2, True, None, True)
+            ],
+            self.hook_calls)

=== modified file 'bzrlib/tests/branch_implementations/test_push.py'
--- a/bzrlib/tests/branch_implementations/test_push.py	2007-01-30 20:58:25 +0000
+++ b/bzrlib/tests/branch_implementations/test_push.py	2007-02-06 02:33:42 +0000
@@ -20,10 +20,12 @@
 
 from bzrlib.branch import Branch
 from bzrlib import errors
-from bzrlib.tests import TestCaseWithTransport
-
-
-class TestPush(TestCaseWithTransport):
+from bzrlib.memorytree import MemoryTree
+from bzrlib.revision import NULL_REVISION
+from bzrlib.tests.branch_implementations.test_branch import TestCaseWithBranch
+
+
+class TestPush(TestCaseWithBranch):
 
     def test_push_convergence_simple(self):
         # when revisions are pushed, the left-most accessible parents must 
@@ -57,8 +59,13 @@
     def test_push_to_checkout_updates_master(self):
         """Pushing into a checkout updates the checkout and the master branch"""
         master_tree = self.make_branch_and_tree('master')
-        rev1 = master_tree.commit('master')
-        checkout = master_tree.branch.create_checkout('checkout')
+        checkout = self.make_branch_and_tree('checkout')
+        try:
+            checkout.branch.bind(master_tree.branch)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
+        rev1 = checkout.commit('master')
 
         other = master_tree.branch.bzrdir.sprout('other').open_workingtree()
         rev2 = other.commit('other commit')
@@ -69,7 +76,12 @@
 
     def test_push_raises_specific_error_on_master_connection_error(self):
         master_tree = self.make_branch_and_tree('master')
-        checkout = master_tree.branch.create_checkout('checkout')
+        checkout = self.make_branch_and_tree('checkout')
+        try:
+            checkout.branch.bind(master_tree.branch)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
         other = master_tree.branch.bzrdir.sprout('other').open_workingtree()
         # move the branch out of the way on disk to cause a connection
         # error.
@@ -77,3 +89,82 @@
         # try to push, which should raise a BoundBranchConnectionFailure.
         self.assertRaises(errors.BoundBranchConnectionFailure,
                 other.branch.push, checkout.branch)
+
+
+class TestPushHook(TestCaseWithBranch):
+
+    def setUp(self):
+        self.hook_calls = []
+        TestCaseWithBranch.setUp(self)
+
+    def capture_post_push_hook(self, source, local, master, old_revno,
+        old_revid, new_revno, new_revid):
+        """Capture post push hook calls to self.hook_calls.
+        
+        The call is logged, as is some state of the two branches.
+        """
+        if local:
+            local_locked = local.is_locked()
+            local_base = local.base
+        else:
+            local_locked = None
+            local_base = None
+        self.hook_calls.append(
+            ('post_push', source, local_base, master.base, old_revno, old_revid,
+             new_revno, new_revid, source.is_locked(), local_locked,
+             master.is_locked()))
+
+    def test_post_push_empty_history(self):
+        target = self.make_branch('target')
+        source = self.make_branch('source')
+        Branch.hooks.install_hook('post_push', self.capture_post_push_hook)
+        source.push(target)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_push', source, None, target.base, 0, NULL_REVISION,
+             0, NULL_REVISION, True, None, True)
+            ],
+            self.hook_calls)
+
+    def test_post_push_bound_branch(self):
+        # pushing to a bound branch should pass in the master branch to the
+        # hook, allowing the correct number of emails to be sent, while still
+        # allowing hooks that want to modify the target to do so to both 
+        # instances.
+        target = self.make_branch('target')
+        local = self.make_branch('local')
+        try:
+            local.bind(target)
+        except errors.UpgradeRequired:
+            # cant bind this format, the test is irrelevant.
+            return
+        source = self.make_branch('source')
+        Branch.hooks.install_hook('post_push', self.capture_post_push_hook)
+        source.push(local)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_push', source, local.base, target.base, 0, NULL_REVISION,
+             0, NULL_REVISION, True, True, True)
+            ],
+            self.hook_calls)
+
+    def test_post_push_nonempty_history(self):
+        target = self.make_branch_and_memory_tree('target')
+        target.lock_write()
+        target.add('')
+        rev1 = target.commit('rev 1')
+        target.unlock()
+        sourcedir = target.bzrdir.clone(self.get_url('source'))
+        source = MemoryTree.create_on_branch(sourcedir.open_branch())
+        rev2 = source.commit('rev 2')
+        Branch.hooks.install_hook('post_push', self.capture_post_push_hook)
+        source.branch.push(target.branch)
+        # with nothing there we should still get a notification, and
+        # have both branches locked at the notification time.
+        self.assertEqual([
+            ('post_push', source.branch, None, target.branch.base, 1, rev1,
+             2, rev2, True, None, True)
+            ],
+            self.hook_calls)

=== modified file 'bzrlib/tests/test_branch.py'
--- a/bzrlib/tests/test_branch.py	2007-01-30 11:52:30 +0000
+++ b/bzrlib/tests/test_branch.py	2007-02-06 02:33:42 +0000
@@ -171,6 +171,10 @@
         """Check that creating a BranchHooks instance has the right defaults."""
         hooks = bzrlib.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("post_pull" in hooks, "post_pull not in %s" % hooks)
+        self.assertTrue("post_uncommit" in hooks, "post_uncommit not in %s" % hooks)
 
     def test_installed_hooks_are_BranchHooks(self):
         """The installed hooks object should be a BranchHooks."""

=== modified file 'bzrlib/uncommit.py'
--- a/bzrlib/uncommit.py	2006-09-05 09:49:40 +0000
+++ b/bzrlib/uncommit.py	2007-02-06 02:33:42 +0000
@@ -14,11 +14,13 @@
 # along with this program; if not, write to the Free Software
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
-"""Remove the last revision from the history of the current branch.
-"""
+"""Remove the last revision from the history of the current branch."""
+
+# TODO: make the guts of this methods on tree, branch.
 
 import os
 
+from bzrlib.branch import Branch
 from bzrlib.errors import BoundBranchOutOfDate
 
 
@@ -51,10 +53,14 @@
             raise BoundBranchOutOfDate(branch, master)
         if revno is None:
             revno = len(rh)
+        old_revno, old_tip = branch.last_revision_info()
+        new_revno = revno -1
 
         files_to_remove = []
         for r in range(revno-1, len(rh)):
             rev_id = rh.pop()
+            # NB: performance would be better using the revision graph rather
+            # than the whole revision.
             rev = branch.repository.get_revision(rev_id)
             # When we finish popping off the pending merges, we want
             # them to stay in the order that they used to be.
@@ -70,10 +76,19 @@
             if master is not None:
                 master.set_revision_history(rh)
             branch.set_revision_history(rh)
+            new_tip = branch.last_revision()
+            if master is None:
+                hook_local = None
+                hook_master = branch
+            else:
+                hook_local = branch
+                hook_master = master
+            for hook in Branch.hooks['post_uncommit']:
+                hook(hook_local, hook_master, old_revno, old_tip, new_revno,
+                    new_tip)
             if tree is not None:
-                branch_tip = branch.last_revision()
-                if branch_tip is not None:
-                    parents = [branch.last_revision()]
+                if new_tip is not None:
+                    parents = [new_tip]
                 else:
                     parents = []
                 parents.extend(reversed(pending_merges))



More information about the bazaar-commits mailing list