-
-
Notifications
You must be signed in to change notification settings - Fork 7
Generate OSGi infos in the manifest #131
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
base: main
Are you sure you want to change the base?
Conversation
@mpkorstanje I actually would like to do that for other java based cucumber jars as well but though its good to start with something really simple, can you review and give feedback if it sounds feasible? |
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.
We've had OSGI meta info in the past, but because the project itself doesn't use OSGI and thus not test in an OSGI container, this resulted in a constant stream of broken packages. We simply didn't notice the problems until someone reported them (cucumber/cucumber-jvm#1404, cucumber/common#412).
So either:
- OSGI consumers repackage our jars in a way that suits them.
- We start testing in an OSGI container.
Both keep the feedback loop small. But at present I would prefer the first, because then it keeps the OSGI specific concerns away from an otherwise non-OSGI project.
That said, I'm not opposed to providing OSGI information all together. But I would like a more elegant, non-fragile, testable solution that also doesn't require knowing much, if anything about OSGI.
With Java 8 reaching EOL soon, we can start using the JPMS. Is there a way we can seamlessly translate that too an OSGI definition? Because we will test with the JPMS, that should also cover a programmatically derived OSGI definition.
java/pom.xml
Outdated
<bnd> | ||
<![CDATA[ | ||
Export-Package: io.cucumber.gherkin.utils.*;-noimport:=true | ||
Import-Package: * |
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.
This looks incredibly fragile. While for a single package module it is fine. I imagine that if there are multiple packages, this gets messy quickly.
Also, would it be possible to extract this to a file instead?
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.
This looks incredibly fragile. While for a single package module it is fine.
I must confess I don't understand what part of it?
Also, would it be possible to extract this to a file instead?
Yes one can extract it into a bnd.bnd
file for example. I often find that a bit more disturbing for projects not using OSGi/bnd otherwise.
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.
I must confess I don't understand what part of it?
- Not tested anywhere.
Export-Package
duplicates the automatic module name. That configuration now exists in two places. Same problem once JPMS is used.- A magic
-noimport:=true
flag. - A magic
Import-Package: *
statement.
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.
Not tested anywhere
I have reviewed the result and it all looks fine to me, and I trust the tool to do the right thing, so I'm not sure what kind of test is in your mind?
Export-Package duplicates the automatic module name
I really hope that you do not use io.cucumber.gherkin.utils.*
as the module name, and as this relates to the root namespace of the packages this is also not related to that name in any case... I can name my module the-foo
and have it contain packages named a.b.c.foo
so I don't think it has to match anyways (but I'm not a JPMS expert!)
A magic -noimport:=true flag.
A magic Import-Package: * statement.
These all are bnd instructions and nothing "magic" so I do not really get why it is fragile to use the notation of a tool? e.g. everything in cucumber can be seen "magic" as well of course its all well defined?!?
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.
These all are bnd instructions and nothing "magic"
That is exactly the magic I'm talking about. 😆
Anyway I've pushed 372d879 to remove the worst of the duplication.
This will work because the recommendations for following module names are from the top of my head:
- Use reverse DNS like package names.
- For a group of packages, use the name of the super package.
We can't follow these rules perfectly because io.cucumber.gherkin
also exists, but there currently no overlap.
java/pom.xml
Outdated
<plugin> | ||
<groupId>biz.aQute.bnd</groupId> | ||
<artifactId>bnd-maven-plugin</artifactId> | ||
<version>6.4.0</version> |
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.
If applied to all projects, version management can be done through the cucumber-parent
pom.
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.
If applied to all projects, version management can be done through the
cucumber-parent
pom.
Yes it would be good to have it at the parent.
Can you give an example here? Of course I would expect people to report issues for some special cases, but that's usually not a problem. The biggest problem usually are split packages (what will be an issue with JPMS as well of course!)
I'm not sure what people expect from such test but from the linked issue it is way tooo complex and goes far beyond what I try to archive here. Running feature files with cucumber inside OSGi is of course an interesting part, but that's really something one should do in an own module/project and is also not an easy task. If that is of interest for cucumber community I could probably help with that but such feature would be more a plugin to cucumber than the other way round. What we would need here is more a way to load and use the classes to allow such use case what is a much smaller scope and does not need a full fledged testing container. It would probably be nice that everything resolves, but nothing that a usual cucumber-devloper should need to worry about.
If JPMS is interesting for you the could maybe benefit from both, as the bnd plugin is able to generate the JPMS infos as well: https://bnd.bndtools.org/chapters/330-jpms.html I just didn't considered this as everything seems to be Java 8 at the moment. |
It's in the linked issues but you'll have to read between the lines a bit. We've had a module named
We've tried to maintain it for a bit, but telling people to ignore a certain type of failure isn't great either.
And of-course every module needs it, so that generates a stream of issues:
I hope that clarifies why I've soured on OSGI. And to re-iterate, I'm not opposed to adding OSGI information all together. But I would like a more elegant, non-fragile, testable solution that also doesn't require knowing much, if anything about OSGI.
I'm currently looking for a is a solution that is amongst other things, testable. So if the OSGI definitions are not correct, I want to know that after running
I'm looking for something in the opposite direction.
It still is at the moment, but you can find the rationale for upgrading in cucumber/cucumber-jvm#2962. |
I had a look at the JUnit 5 project to see how they resolved that problem. There seems to be a lot of custom programming done in Gradle to make that work nicely. And after reading just the introduction https://bnd.bndtools.org I'm not expecting stellar Maven support. 😄
You'd hope that people who want you to provide meta-information for their container system would make it easy and painless for you to provide it, not come at you with an attitude. |
So pax-exam is a tool for testing OSGi container and does not releate in any way to generation of OSGi metadata, but how does it "break packages"? I can understand that it might break the developers workflow in having to upgrade that thing, but as mentioned this is almost total overkill anyways for the thing that should be archived here... So you are actually facing different issues here that steems maybe from the fact how it was implemented was not that optimal in the past.
I think we can forget about the "testable" here, because I think it requires some kind of definition what/how to test. So if you looking at my proposed solution here (and forget about the frustration in the past maybe) I have chosen one that:
So what is the worst can happen? There might need to be some things adjusted for the sake of OSGi compatibility and then you can ask the people to do that for you (as I did it here) and then again stop to care as it does not affects you (as a no OSGi user). Given that I wonder, how much simpler a solution has to be to be considered here?
If that would be the case, then the tool would be buggy. I think no one ever "verifies" class files... or jar files or ... produced by a tool because then you need to verify that the verifiers is correct and verify that and so on. So I think at this point we need to make the assumption that the generated meta-data is correct (and I have no evidence from the thousands of people using that ever reported a complete failure of the tool).
As OSGi has richer metadata than JPMS and also things that can not be expressed in JPMS I really doubt something exits. Also often JPMS infos written manually are just wrong or incomplete.
Well that's how gradle users supposed to do their work :-P
Well as a matter of fact, bndlib is most used in maven (e.g. Tycho, Felix Bundle Plugin and a like), but please don't take the mind or attitude of a project to be representative for OSGi (what is homed here https://www.osgi.org/) what is of course tool agnostic. Still bndlib is a good tool regardless of some its questionable statements in the documentation. |
The complexity I'd prefer would be adding a single Maven plugin without configuration, then have everything work out of the box. If there is a JPMS module, then that is translated automatically. If it is a regular Jar, then import everything and export all the packages. And preferably all that is written to a manifest in Now I've played around with |
java/pom.xml
Outdated
<plugin> | ||
<groupId>biz.aQute.bnd</groupId> | ||
<artifactId>bnd-maven-plugin</artifactId> | ||
<version>6.4.0</version> |
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.
@laeubi any reason not to use v7.1?
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.
@laeubi any reason not to use v7.1?
This requires the build to use Java 17, I think that still work to build for java 8 then but don't wanted to change to much at once.
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.
Okay, please upgrade and you can drop 11 from the matrix. We don't build against 8 for a long time already.
BTW for reference
That's sadly a restriction of the maven-jar plugin and is required to have it more independent of each others. |
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.
I'm confident enough that this will work without too much fuss.
@laeubi please upgrade the plugin to the latest version and add a change long entry before merging. Semverwise, I think this would be an ### Added
header, but correct me if you think I got that wrong.
@mpkorstanje thanks for reviewing this, I think this is actually only a micro change, nothing changes API wise for consumers. Also many thanks for consideration I know it could be a bit confusing for non-OSGi users and therefore I really like to understand how we can improve things here, I'm currently investigate writing a small tool that checks the dependencies of the produced metadata so we even have a little check. And maybe first the cucumber-messages (that are the only dependency here) should be addressed so we get a proper version range automatically, so there is no need to merge this immediately, just let me know if a release is planned anyways, then it could be a good time to include this as well (maybe improve on it later on). Some things to check:
|
That's probably sensible to do. But the ordering of operations isn't important though. It can be done before or after this PR is merged.
Inline style. Then we can reuse the Automatic Module name. That will allow easier copy/paste. |
Releases aren't planned. They happen ad-hoc, when there is something to release.
I'm not 100% following this. |
@mpkorstanje sorry for confusion. So first I started on a small mojo that allows to verify some basic things (e.g. symbolic name is there and version is given) and then analyze the generated manifest if it can be consumed easily, this is something we can add after the next Tycho release as an additional way to get confidence: when I run this tool against my PR it currently gives me this report:
As you can see there is one warning because If I do that locally the output is as follows and all checks are green:
As you see it now uses a version range for the package what makes it less likely to break if there is ever a major change in the requirement, so results in better metadata. So to summarize it seems best for me:
I hope it makes it more clear! |
Yes! Thank you. |
For the first step I now created: |
@mpkorstanje I tried to push branch directly but it says:
I think this is because I have not yet comitted to this repo before? |
Try again! Looks like some defaults that were applied to all repos are no longer applied by default. |
🤔 What's changed?
This enables to embed OSGi metadata into the manifest of the produced jar file.
⚡️ What's your motivation?
I use this in an Eclipse plugin and currently need to repack the original cucumber jars into an OSGi bundle, but adding the info at the source has much benefits: