-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add OSGi support #9
base: main
Are you sure you want to change the base?
Conversation
@@ -8,7 +8,7 @@ | |||
<version>7</version> | |||
</parent> | |||
|
|||
<packaging>jar</packaging> | |||
<packaging>bundle</packaging> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What impact does this have on how the library is consumed via Maven Central for non-OSGI users?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bundle is a jar with a OSGi manifest so should make no difference at all
@mwanji Do you think this new |
@davidvanlaatum Thanks for working on this. I've never used OSGI so I'll probably have some stupid questions while reviewing this. First one... Is there anything special the user will need to do that should go into the README? |
(I’m no one here but I see the changes do not conform to the code style of the rest of the project) |
If I was writing from scratch I would have written a service class that can be registered in the OSGi service registry rather than static functions but without some major refactoring can't really do that so settled for just making it work in an environment where a single bundle can not see the contents of all jars without going via the OSGi framework. Basicly what I have done should have no impact on users other than it will work in a OSGi container. As for extra stuff for OSGi users they can just use the same as normal which is not ideal but a start |
@yuchi As long as there aren't any tabs, I'm not too concerned about the code style. ;) At some point we should drop in a Maven plugin to format the code or at least validate the style. |
What do you mean by "do not conform to the code style of the rest of the project" ? |
I tried very hard to keep the style right |
@davidvanlaatum the only thing I found is the too liberal whitespace around parens.
@jamesward sorry. It’s just my code-style-cop instinct kicking in :( |
ah thats the convention I follow at work its subconscious ;) didn't even notice |
Example Servlet to register with OSGi http whiteboard @OsgiServiceProvider( classes = Servlet.class )
@Properties( {
@Property( name = "osgi.http.whiteboard.servlet.pattern", value = "/webjarsjs" ),
@Property( name = "osgi.http.whiteboard.servlet.name", value = "WebJars RequireJS Handler" )
} )
@Named( "WebJarsRequireJSServlet" )
public class WebJarsRequireJSServlet extends HttpServlet {
@Inject
private BundleContext bundleContext;
@Override
protected void doGet ( HttpServletRequest req, HttpServletResponse resp ) throws ServletException, IOException {
resp.setContentType ( "application/javascript" );
try ( ServletOutputStream out = resp.getOutputStream () ) {
out.print (
RequireJS.generateSetupJavaScript ( Collections.singletonList ( "/webjars/" ) ) );
}
}
@PostConstruct
public void init () {
RequireJS.setResourceLocator ( new OSGIResourceLocatorImpl ( bundleContext ) );
}
} |
@davidvanlaatum Think it would be helpful to have that in the README? |
Added to webjars-locator pull |
Am still relatively new to OSGi myself so will send another PR if I find a better way (or think of one) |
Had an idea overnight on how to remove the need to do @PostConstruct
public void init () {
RequireJS.setResourceLocator ( new OSGIResourceLocatorImpl ( bundleContext ) );
} Ill try to make the change when I get home from work tonight |
have made the changes. Much happier now that its using its own BundleContext |
Now to make the webjars them selves OSGi compatible ;) |
Cool. I'd still like to get @mwanji to review this before I merge. |
Any news? Also can someone point me in the right direction for making changes to the automatic webjars? |
I've just sent @mwanji and email. Maybe he isn't getting the notifications here. |
Sorry, I saw the initial notification, then forgot to take a look. I haven't had time to test the changes yet, but in the past I have had problems (at least in Eclipse) with Maven artifacts packaged as "bundle". I'll try to test this soon, but might be able to do so only early next week.
I've looked at the diff and it seems like the BundleUrlProtocolHandler and the new classes it introduces could be in their own project (which would eliminate any potential packaging problem). Then, only the small changes to the WebJarAssetLocator would be necessary (the new constructor that takes a classloader makes sense, considering the various environments the locator might be running in, eg. application, OSGi or during a Maven build). |
If you do this though you wont be able to load the jar in OSGi. You need to do one of:
3 not being a very good option and 2 not being much better |
We should be able to maintain the jar artifact type while still adding the appropriate manifest headers. If you take a look at the example here it shows how to have the default jar plugin include the adapted manifest from the bundle plugin. http://felix.apache.org/documentation/subprojects/apache-felix-maven-bundle-plugin-bnd.html . See the "Adding OSGi metadata to existing projects without changing the packaging type " section. The main magic is the supportedProjectTypes tag along with the jar plugin reference to pull in the generated version. |
Initial support for OSGi best I can do for now without restructuring.
About to send a pull for webjars-locator as well.