[MERGE] add a hook for Branch.set_last_revision_info()

[James Henstridge]

On 09/04/2008, Aaron Bentley <aaron at aaronbentley.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
>  Hash: SHA1
>
>
>  James Henstridge wrote:
>  > On IRC Aaron suggested that the branch being changed be exposed as a
>  > member of the params object passed to the hook rather than as a
>  > separate argument.  The attached bundle makes that change.
>
>
> Thanks.
>
>  bb:resubmit
>
>  > +    def _make_branch_tip_hook_params(self, new_revno, new_revision_id):
>  > +        """Construct a Params object for passing to *branch_tip hooks."""
>  > +        if Branch.hooks['post_change_branch_tip']:
>  > +            old_revno, old_revision_id = self.last_revision_info()
>  > +            return ChangeBranchTipParams(
>  > +                self, old_revno, new_revno, old_revision_id, new_revision_id)
>  > +        else:
>  > +            return None
>
>  For myself, I'd prefer to invoke _make_branch_tip_hook_params after
>  updating to the new revno and revision_id, since they're only
>  *predicted* values at this point.  But that's probably just anal.  You
>  don't have to make this change; it's just a thought.

Well, branch.last_revision_info() needs to be captured prior to
updating the tip, so something needs to be done before hand.  I guess
you're suggesting not to trust the method arguments for the new
revno/revid?

I guess that makes sense, so I've changed the implementation to use
last_revision_info() to determine both the old and new revision info.


>  > @@ -1395,10 +1447,12 @@
>  >          configured to check constraints on history, in which case this may not
>  >          be permitted.
>  >          """
>  > +        hook_params = self._make_branch_tip_hook_params(revno, revision_id)
>  >          revision_id = _mod_revision.ensure_null(revision_id)
>
>  ^^ _make_branch_tip_hook_params should not be invoked before
>  ensure_null.  ensure_null makes sure the revision_id is a string, not None.
>
>  > @@ -1853,10 +1907,12 @@
>  >
>  >      @needs_write_lock
>  >      def set_last_revision_info(self, revno, revision_id):
>  > +        hook_params = self._make_branch_tip_hook_params(revno, revision_id)
>  >          revision_id = _mod_revision.ensure_null(revision_id)
>
>  ^^^ wrong order again.

Fixed both of these.


>  > +post_change_branch_tip
>  > +----------------------
>  > +
>  > +Run after a branch tip has been changed but while the branch is still
>  > +write-locked. Note that push, pull, commit and uncommit all invoke this hook.
>  > +
>  > +The hook signature is (params), where params is an object containing
>  > +the members
>  > +
>  > +  branch
>  > +    The branch whose tip has been changed.
>  > +
>  > +  old_revno
>  > +    The revision number (eg 10) of the branch before the change.
>
>  ^^^ Maybe mention that this is always an integer, never a dotted revno?

Added a sentence to the end of this section to that effect.

>
>  > +class TestPostChangeBranchTip(TestCaseWithMemoryTransport):
>  > +
>  > +    def setUp(self):
>  > +        self.hook_calls = []
>  > +        TestCaseWithMemoryTransport.setUp(self)
>  > +
>  > +    def capture_post_change_branch_tip_hook(self, params):
>  > +        """Capture post_change_branch_tip hook calls to self.hook_calls.
>  > +
>  > +        The call is logged, as is some state of the branch.
>  > +        """
>  > +        self.hook_calls.append((params, params.branch.is_locked()))
>  > +
>  > +    def test_post_change_branch_tip_empty_history(self):
>  > +        branch = self.make_branch('source')
>  > +        Branch.hooks.install_hook('post_change_branch_tip',
>  > +                                  self.capture_post_change_branch_tip_hook)
>
>  It looks like this hook is never being removed.  But if not, how are the
>  remaining tests passing?

The test infrastructure ensures that Branch.hooks is empty for tests.
The other existing hooks tests depend on this behaviour, so it
shouldn't be a problem here either.

Attached is an updated bundle.

James.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bzr-post-change-branch-tip-v3.patch
Type: text/x-diff
Size: 24812 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20080409/f5f05416/attachment-0001.bin