Skip to content
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

[Build] Generate OSGi metadata for JSVG #107

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

HannesWell
Copy link

@HannesWell HannesWell commented Dec 7, 2024

This change has the goal to extend the jar's META-INF/MANIFEST.MF generated during the build to contiain OSGi metadata so that JSVG can be used as bundle in OSGi runtimes out of the box.
This is needed for eclipse-platform/eclipse.platform.swt#1638

To generate OSGi metadata the bnd-tools library is used respectivly it's gradle plugin.
While I know how to configure bnd-tools, for JSVG it should actually be quite simple since there are no dependencies, I have almost zero knowledge about Gradle and how to integrate the bnd-gradle plugin into JSVG's build and failed in my attempts to make it work.
I tried to follow the guide provided at:
https://github.com/bndtools/bnd/tree/master/gradle-plugins#gradle-plugin-for-non-bnd-workspace-builds ff

@weisJ could you please advice me what I need to do in order to make the bnd-gradle plugin work?
Then i can work out a suitable bnd-configuration to generated proper OSGi metadata. Therefore this is currently a work-in-progress draft to get the work started.
Thanks in advance!

@HannesWell
Copy link
Author

HannesWell commented Dec 7, 2024

@Michael5601 FYI.

@weisJ if it would helpful, I can offer to have video-call, maybe together with @Michael5601, to make this work.
I had a private mail coversiation with Michael about this and he said, you are in contact. If you are interested, maybe Michael could start a private mail conversion for an appointment?

@weisJ
Copy link
Owner

weisJ commented Dec 8, 2024

I have just pushed a commit to your branch which makes the plugin work. You should now be able to configure the bundle properties.

@Michael5601
Copy link

@Michael5601 FYI.

@weisJ if it would helpful, I can offer to have video-call, maybe together with @Michael5601, to make this work. I had a private mail coversiation with Michael about this and he said, you are in contact. If you are interested, maybe Michael could start a private mail conversion for an appointment?

Thank you for the draft and the work you put into this already @HannesWell. Should I still create the appointment or is the problem fixed by the commit of @weisJ?

@HannesWell
Copy link
Author

I have just pushed a commit to your branch which makes the plugin work. You should now be able to configure the bundle properties.

Thanks a lot. I was now to configure the bundle task, but only through the manifest-attributes as described in the last section of
https://github.com/bndtools/bnd/tree/master/gradle-plugins#instructing-bnd-on-how-to-build-your-bundle

I also tried to use, what I think is the regular way, through a bnd element in the bundle task, but failed miserably. Sorry for that, but I have really zero knowledge about Gradle. I tried hard to find something through internet search, etc. but failed. I you have better advice, I would be very glad.

build.gradle.kts Outdated
Comment on lines 268 to 269
bundle {
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I added the following, the configuration was just ignored and the generated MANIFEST.MF in weisJ-jsvg\jsvg\build\libs\jsvg-1.6.2-SNAPSHOT.jar did had the content that should result from the specified instructions.

Suggested change
bundle {
}
bundle {
bnd("""
Bundle-SymbolicName: com.github.weisj.jsvg
-exportcontents: !*.impl.*,*
-removeheaders: Private-Package,Tool
""")
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why it is ignored. The documentation on this isn't very clear in how you are supposed to set things up. I have moved bundle setup into the gradle file of the jsvg itself (not the root) and now it works. See latest commit for this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now it works.

That's really awesome. Thank you.

I don't know why it is ignored.

I'm sorry, but I cannot answer that since by Gradle knowledge is almost zero and the more I tried to make this change work the more I felt it's not only close to but zero.

build.gradle.kts Outdated
// bnd instructions to generate OSGi metadata
attributes["Bundle-SymbolicName"] = "com.github.weisj.jsvg"
attributes["-exportcontents"] = "!*.impl.*,*"
attributes["-removeheaders"] = "Private-Package,Tool"
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the bnd instruction to a separate bundle task (?) here

Suggested change
}
}
bundle {
bnd("""
Bundle-SymbolicName: com.github.weisj.jsvg
-exportcontents: !*.impl.*,*
-removeheaders: Private-Package,Tool
""")
}

lead to a build failure with:

Could not determine the dependencies of task ':annotations:assemble'.
> Could not create task ':annotations:sourcesJar'.
   > Extension with name 'bundle' does not exist. Currently registered extension names: [ext]

@HannesWell
Copy link
Author

The current state is probably not beautiful, but seems to work, although I think some parts can be cleaned up?
If possible, it might still be cleaner to make the bnd element within a bundle task work?

@Michael5601 FYI.
@weisJ if it would helpful, I can offer to have video-call, maybe together with @Michael5601, to make this work. I had a private mail coversiation with Michael about this and he said, you are in contact. If you are interested, maybe Michael could start a private mail conversion for an appointment?

Thank you for the draft and the work you put into this already @HannesWell. Should I still create the appointment or is the problem fixed by the commit of @weisJ?

From my point of view this is not necessary, I hope we can sort out the remaining issues here. But if any one of you is still interested I'm open for it. :)

bundle {
bnd(
"Bundle-SymbolicName: com.github.weisj.jsvg2",
"-exportcontents: !*.impl.*,*",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only added the 2 for testing purposes. I guess -exportcontents relates to which classes are exposed as API? In this case not everything outside impl subpackages should be considered as being public. I have defined some exports in the module-info.java file. Though it is not up to date. I will get back to you with which packages at the moment are considered to be public.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess -exportcontents relates to which classes are exposed as API?

Yes exactly.

In this case not everything outside impl subpackages should be considered as being public. I have defined some exports in the module-info.java file. Though it is not up to date. I will get back to you with which packages at the moment are considered to be public.

It's no problem to update that list. Just let me know what should be in there.

Btw. you can also use bnd-tools to generate the JPMS module-info.class which ensures that OSGi and JPMS metadata are in sync:

If you are interested I can try to make that work too, hoping that my Gradle (non) skills don't hinder me from doing it.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's no problem to update that list. Just let me know what should be in there.

I'll get back to you once I have updated it. If I don't get to it soon enough I'll just adjust it myself.

Btw. you can also use bnd-tools to generate the JPMS module-info.class which ensures that OSGi and JPMS metadata are in sync:

If you are interested I can try to make that work too, hoping that my Gradle (non) skills don't hinder me from doing it.

Does this also ensure that code conforms to the generated module-info.java? It does sound intriguing not having to maintain two places, where these things are defined, but with the current setup the build will fail if the module-info.java is not respected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants