> Thanks, will be merging this today after I do some minor cleanup and
> fix the Maven build. We can look at the bundle scanning changes in
a
> separate issue. Wrt. your other questions:
Maybe I went a bit too far, since I
pushed also proposed implementations that you probably wont' like:
- Bundle.loadClass("javax.inject.Inject")
or Bundle.loadClass("com.google.inject.Inject") to select an
extendable bundle
- Moved the utility to select
the beanScanning from Main to BeanScanning, but also changed a bit behaviour
to use a ClassSpace to look for an index file before falling back to ON
From what you wrote in this message,
I now understand you won't like them because they may lead to lazy-bundles
getting activated unnecessarily. I thought this was not a problem because
the activator bundle tracker defaults to ACTIVE. Let me know if you want
me to rollback those changes, if so I'll do later today.
At the moment it tracks ACTIVE, it used to track STARTING and ACTIVE so it could also scan lazy plug-ins and activate them (but only if appropriate) and at some point that feature will come back.
For the moment I think it's better to separate out these changes rather than roll them all into this refactoring item; we can discuss them in a separate patch/issue (will be easier for people to follow). Also, but this should not be an issue,
I proposed a change to make the selection of an extendable bundle pluggable.
Very simple, based on an interface + default implementation which can be
overriden by means of a system property + fragment attached to the sisu
bundle.
Sounds reasonable to me. > > I was wondering if InjectorPublisher may be used for the
> > purpose, also given that the add/remove methods of
> > MutableBeanLocator are deprecated.
>
> Very true, BindingPublisher (the SPI interface) is a possible option.
If you can wait one or two days before
merging, I can try to make the change: I could make the service tracker
look for BindingPublisher in the OSGi registry, and have the bundle tracker
register an InjectorPublisher.
No problem, go ahead. > The main downside of this is it
causes eager class-loading, which
> leads to lazy-bundles getting activated unnecessarily. One suggestion
from
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=394734 was to use the
> wiring API which would let us check visibility without eager class-loading.
Again, if you can wait one or two days
I can try to read that bugzilla entry and make the changes later today
or tomorrow.
>
> > BundleProperties to get properties from the bundle
> > context as opposed to getting properties from the
> > environment.
>
> The main reason for using BundleProperties over System properties
is
> that it also includes properties set on the framework instance.
> However, this is likely only of interest to people who run multiple
> frameworks in the same process and even then they'd probably use
> ConfigAdmin so I'll think about simplifying this.
Just let me know if you can wait and
want me to remove it.
Sure, zap it :) > > Not sure I'll have much time this week to write tests,
> > but in case I had, would it be possible to use
> > TinyBundles to test the extender?
>
> The current plan is to migrate the tests over to http://wiki.
> eclipse.org/Tycho/Packaging_Types#eclipse-test-plugin after this
> release. Would need to see if/how TinyBundles works with that, and
> also formally request to use it as a test dependency.
"After this release" means
you don't mind not having tests for the extender in this release, right?
Correct wrt. automated tests - I manually test the activator code with some local projects before making a release. Thanks
GianMaria.
|
