Plugin Development Help

[Alex Lewis]

Jelmer Vernooij <jelmer <at> samba.org> writes:

> 
> On Wed, Jul 25, 2012 at 09:31:23AM +0000, Alex Lewis wrote:
> > Thanks Jelmer!
> > 
> > Jelmer Vernooij <jelmer <at> samba.org> writes:
> > > That's probably the easiest thing to do. You could also just override
> > > the implementation of the entire commands, as bzr-loom is doing for
> > > some commands. That works, but may break if other plugins are trying
> > > to do the same thing.
> > 
> > If possible I think I'd like to avoid having to copy the implementation of 
> > cmd_commit into my plugin. It just feels a bit brittle when Bazaar is 
changed, 
> > incompatibility with other plugins, etc. I'd like to keep my plugin as 
simple 
> > and compatible as possible.
> I'm not sure if there is an easy way around that. If your plugin is
> just transforming one option into another, or adding some extra
> behaviour before or after the command is run, then you don't need to
> copy the implementation.
> 
> It seems what you are trying to do requires modification of one of the
> local variables of cmd_commit.run, so I don't see any other way at the
> moment to avoid copying the body of cmd_commit.run.
> 
> Of course, another option would be for the bzr implementation to be
> extended to allow properties to be passed in.
> 

I'd be happy to try to make those changes if that were an option? I've had a 
look at the code and I guess options could be:
* Add a new parameter to cmd_commit.run revprops=None
  - Would this be changing the API and thus breaking existing code or would 
specifying
    the default value be enough to keep existing code happy?
* Update mutable_tree to provide some way to update revprops from within a 
start_commit hook.
* Introduce a new hook somewhere (possibly mutabletree)
* Others?

I think for the short term I'll completely override the cmd_commit.run method so 
I can get further. I'll add the --task-id option to the log command and do 
whatever I need to do to process a merge. Hopefully that will indicate what 
would be desirable hook points which I'd try to implement in Bazaar and make a 
proposal.

> > > Adding it to Commit.revprops sounds reasonable to me. I'm a bit unsure
> > > though how you would want to do this exactly. If you're overriding
> > > cmd_commit anyway to add a new option, wouldn't it make more sense to
> > > update the revprops dictionary in cmd_commit.run ?
> > Ok this is probably my lack of python knowledge showing here. I've 
overridden 
> > cmd_commit, added my new option to the "takes_options" dictionary, 
overridden 
> > "run" which processes **kw to find the value for my new option, then calls 
> > super(cmd_commit, self).run(*args, **kw) to finish off the command. 
Obviously at 
> > this point my task-id value is lost as it isn't passed in any way to the 
super 
> > implementation. 
> It shouldn't be passed to the super implementation - it doesn't know
> anything about your task-id option. You would instead want to do
> something specific in your implementation which uses that option.
> 
> > The super implementation of cmd_commit defines the "properties" dictionary 
> > within the run() function, which it populates with 'bugs' if present and 
passes 
> > that into tree.commit(...) as the revprops argument. As "properties" is 
> > defined/initialised inside cmd_commit.run() I thought I wouldn't be able to 
add 
> > my task-id to that dictionary. Is that correct? If I'm wrong how do I add my 
> > task-id to properties/revprops before it gets sent to tree.commit?
> No, you can't access that dictionary from your implementation (at
> least not without a significant amount of introspection, which is due
> to breakage too).
> 

Ok good that's what I thought. 

Thanks Jelmer for all your help.