Branch.set_last_revision() runs hooks even if last revision didn't change

[Jelmer Vernooij]

  affects bzr
  status confirmed
  importance low

Branch.set_last_revision() will always run the update_branch_tip hooks,
even if the branch tip hook hasn't actually changed.

[Robert Collins (lifeless)]

On Sun, 2010-02-07 at 15:15 +0000, Jelmer Vernooij wrote:
> Public bug reported:
>
> affects bzr
> status confirmed
> importance low
>
> Branch.set_last_revision() will always run the update_branch_tip hooks,
> even if the branch tip hook hasn't actually changed.

I'm not sure that this really is a bug: It's true that its a no-op, but
I'm struggling to think of reasons why it /shouldn't/ run the hooks (or
equally why it should).

-Rob

[Jelmer Vernooij (jelmer)]

On Mon, 2010-02-08 at 21:31 +0000, Robert Collins wrote:
> On Sun, 2010-02-07 at 15:15 +0000, Jelmer Vernooij wrote:
> > Public bug reported:
> >
> > affects bzr
> > status confirmed
> > importance low
> >
> > Branch.set_last_revision() will always run the update_branch_tip hooks,
> > even if the branch tip hook hasn't actually changed.
>
> I'm not sure that this really is a bug: It's true that its a no-op, but
> I'm struggling to think of reasons why it /shouldn't/ run the hooks (or
> equally why it should).
So, the reason why I reported this was because I see notification popups
for branches that haven't actually changed. This is because e.g. bzr-svn
and bzr-git will call set_last_revision() if they did not fetch any new
revisions.

Arguably this could be fixed in a number of different places:
 * bzr-git/bzr-hg/bzr-svn could check last_revision() before calling
set_last_revision()
 * bzrlib could avoid running the hooks if the branch tip did not
actually change
 * bzr-dbus could not send notifications if the old and new revid are
the same
 * bzr-notify could ignore notifications for branches that didn't
actually change

I think fixing bzrlib makes the most sense, since the name
'post_change_branch_tip' suggests that that hook is only ever executed
if the branch tip actually changes and I can't think of a reason why you
*would* want that hook to be fired if the branch tip didn't change.

Cheers,

Jelmer