-
Notifications
You must be signed in to change notification settings - Fork 423
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
Clean up OSGi metadata #60
Conversation
This change does remove "sun.misc" and "sun.nio.ch" from the list of imported packages. Signed-off-by: Jens Reimann <jreimann@redhat.com>
Should we fix the duplicate package export as well? This commit just cleans up the imports. |
I will. I just thought I will start with the easier one 😉 |
Signed-off-by: Jens Reimann <jreimann@redhat.com>
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.
Changing the packaging type to 'bundle' and relying on the bundle plugin makes me uncomfortable. The docs for the plugin say by doing this you're (potentially) changing what gets packed in the jar files as well.
I've been trying to model OSGi support off of what the netty project does, which seems both minimal and non-intrusive.
Well I have made good experience with the bundle plugin ... no issues up until now. But I'll understand .. I will try to switch back and still keep it simple. I did find another issue though .. there is a reference to "com.sun.managment". That may cause troubles since this package could be missing. And I am not sure what will happen with Java 9 regarding that. |
|
Could I wrap this into some sort of "test if possible, don't fail otherwise" section? |
Then I could make the import optional ... and nothing would fail. |
You could. I don't see any way to make sure it doesn't appear in the imports other than using reflection to check if the class is available and also to invoke the |
This change does use the maven-bundle-plugin in order to create OSGi metadata. It falls back to the default configuration, only overriding this when really necessary. Signed-off-by: Jens Reimann <jreimann@redhat.com>
50dbb8a
to
4e393e5
Compare
No, I wouldn't use reflection for that. Since it doesn't appear in the class definition it should be fine just catching |
I updated the PR ... better that way? 😉 |
Damn ... I need to do some more testing with Karaf for that. |
I meant for Java 9 compatibility - even trying to import a com.sun package should result in a compilation error if we're not allowed access to that module, no? |
Hm .. that is a good point... I have no clue how Java 9 would behave when I comes to compiling that. I guess you just gave me a reason to finally install Java 9 😁 |
I just pushed a new update. That let's Karaf validate the OSGi metadata successfully. But I will try an actually install the bundle. So give me a bit more time. |
Ok .. it is getting close. But now I run into a "uses" issue:
|
This one seems to be the issue:
|
This change does prevent the maven-bundle-plugin from importing 'javax.annotation' in order to prevent a "uses violation". Signed-off-by: Jens Reimann <jreimann@redhat.com>
Yeah, you're on your own with the weird OSGi errors ;) I really have no experience with OSGi, and while I find it to be interesting and somewhat compelling in theory, the reality/implementation seems overly complicated and messy. |
I know .. and I am just talking to myself in order to remember 😉 This was tricky. And although I seem to have a solution, I don't like the situation. The JSR 305 annotation live in the same package As is seems right now JSR 305 will never make into Java. At least not as it is now. But I do see the value of "findbugs". Maybe there is a different way. |
I am doing one final test now on an actual target. Docker already is fine. |
Ok ... looks good! Ready to merge, if you like. |
Thanks! |
This change does remove "sun.misc" and "sun.nio.ch" from the list of
imported packages.
Signed-off-by: Jens Reimann jreimann@redhat.com