[RFC] Hook for branch nicknames?

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 11-09-24 08:08 PM, Neil Martinsen-Burrell wrote:
> There is a request (http://pad.lv/797578) for bzr-colo to give 
> branches a more helpful nick than "trunk".  It is possible to do
> this by setting explicit nicknames in all of the operations that
> create new colocated branches, but it would be nicer to satisfy the
> request in an extensible way.

Why is that?  Do you want this behaviour to apply to non-colocated
branches?

> The bug suggests using hooks and I have a prototype that does that,
> but I wanted feedback on whether I have the right approach (never
> having used hooks much).

It doesn't sound like your approach is the same as the suggested
approach; the suggested approach would set an explicit nick at branch
creation time.  That would use the post_branch_init hook.

One advantage of doing it at creation time is that you can do
expensive operations to determine the nick, knowing that they will
only be invoked once, instead of every time the nick is looked up.

Another option is to give the branch directory the same name as the
working tree directory, instead of "trunk".  That way, you don't even
need to override the default behaviour.  This is what bzr-pipeline does.

> Default nicknames originate from BranchConfig._get_nickname where
> it returns urlutils.unescape(self.branch.base.split('/')[-2].  This
> is the behavior that I would like to override with a hook.  I have
> added a HookPoint to Branch.hooks named 'automatic_nick'  (name
> follows 'automatic_tag' but could easily be 'default_nick') for
> callables that take a branch as input and return the nick.  Then,
> we can get the current behavior by registering the above by default
> and calling Branch.hooks['automatic_nick'] from _get_nickname.
> 
> My concern is when bzr-colo or another plugin registers a
> different hook for 'automatic_nick', then
> Branch.hooks['automatic_nick'] is now a list of two or more
> possibilities.  Should we always use the first hook in that list
> (presumably the most recently registered)?

With hooks like the MergeDirectiveHooks callback, the output of one
hook is provided as input to the next.  The default hook could choose
to provide a nick only in cases where one had not already been generated.

> Is there a way to use the hook infrastructure for situations where
> we wish to override behavior?

I don't think our hooks mechanism is suitable for an approach where
one hook is supposed to override all previous hooks.  As you say, it's
subject to ordering issues.  And if multiple plugins try to override
that hook, who's to say which should succeed?

I recommend:
- - Just change the default bzr-colo behaviour to generate the branch
name from the working tree name.

- - If you don't want to, use the post_branch_init hook to set an
explicit nick.

- - If you want to do something extensible, make get_nickname's fallback
behaviour extensible by just splitting it out into a method.  Those
wishing to override the behaviour can just monkey-patch the method.
It's not not our normal approach, but any override pattern is
equivalent to monkey-patching.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk5+fo4ACgkQ0F+nu1YWqI3/vwCfeMhmJWAQQe/S/5UYvW5r7hLZ
l0sAnjASR9fiQfqkI0aaOu3AljsTr5T6
=NOVV
-----END PGP SIGNATURE-----