-
Notifications
You must be signed in to change notification settings - Fork 10
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
missing module-info.java #40
Comments
Need seems to be a strong word. What is that good for? I have never needed this in any project. |
Any libraries ( like networknt's json schema validator ) that use JPMS need this, otherwise you enter into a mess of --add-opens that makes larger builds really painful. I'm not sure how it is that you haven't been exposed to JPMS before but it's been pretty much universally adopted. If you're still largely working in java8 environments check the multi-release specifications int the META-INF - ( like the java8 PR i opened for you yesterday ). What this means in practice:
|
I largely use the later Java releases like Java 17 and 21 in my personal and professional projects. I have thus obviously been exposed to it, but I have yet to find it useful (more of a pain in the neck, tbh). It obviously doesn't hurt to add the module-info.java. (PS: Please see the code review comment) |
Is it intended that this new release's MANIFEST.MF is like this
but the previous version was like this:
|
It seems like it's hard to keep everyone happy. 😅 @merks, I’d recommend checking the release notes first, as they explain the intended rationale behind the changes in this release. I genuinely appreciate all the feedback and contributions, but I find myself in a situation where bug reports and PRs often conflict with each other, making it difficult to ensure compatibility for everyone. To clarify my position: I personally have no need for OSGi and, as a result, no way to properly test it. Let’s find a path forward that works for most users! |
If you're on java 11, having the module-info.java has no impact on folks that are running on the classpath. For folks that are building libraries or running on the modulepath, not having it makes the library unusable. I can't stress enough - there is no impact on folks not using the modulepath. The JPMS requirements and the OSGI requirements are completely orthogonal, and do not conflict in anyway. The original PR I opened against the JDK8 version would of maintained compatibility for everyone. I'm happy with the java11 requirement, but folks on the module path need a module descriptor. I have no opinion about the OSGi stuff, but i'm also not interfering with it in anyway. |
Thanks for your feedback! @merks What is you interest and perspective on this? Is OSGi the concern or the lack of a module descriptor? Or something entirely different. Please advise. I will likely reverse it to include the module descriptor and maintain the Java 1.8 compatibility and OSGi for the 1.12.x release, but I will give it some time to gather all requirements. |
Firstly compliments to your responsiveness! 🥇 Sorry for not reading notes. I just saw the one open issue and dived right in. For your background, I maintain Eclipse's Orbit project which produces a p2 update site with OSGi bundles for reuse across a bunch of Eclipse projects: https://download.eclipse.org/tools/orbit/simrel/orbit-aggregation/table.html We build this site from a number of parts. One of those parts is for artifacts available at Maven Central as OSGi bundles: The Orbit tools automatically find new versions to update that file. Before I commit the updates I generally try to build it locally, which fails like this now:
For artifacts that are not available as OSGi bundles, I need to use BND to wrap them, e.g., So I just wanted to know if I should "move" ethlo from Maven.target to MavenBND.target and then define the needed BND instructions for it. What I hope to avoid is to do that work, and then later it has an OSGi MANIFEST.MF again, maybe even with a different bundle symbolic name. I might not notice that the BND instructions I've defined are not actually being used later... So if you plan to restore the OSGi details, which has no potential adverse impact and merely enables consuming the jar as an OSGi bundle in an OSGi runtime, I'll wait for the next release. If your don't plan to do that, then I can move it, and generate the needed OSGi details. So I don't really have any hard requirements and I appreciate your quick answers. |
- Revisit the issue next time there is a new version. ethlo/itu#40
@merks Excellent, and thank you! I could perhaps ask you to code review the PR that will originate from this, considering that you seem to be an expert on the matter. To anyone reading this, I'm a bit torn on the Java 1.8 compatibility. I see that the https://github.com/networknt/json-schema-validator/blob/master/pom.xml#L72 which is probably the biggest project depending on this, maintain 1.8 compatibility. @stevehu could perhaps chime in on the matter? |
The networknt folks specifically declare dependencies on com.ethlo.time in their module info, which is the root of all this activity from me :) |
I'm happy to help if I can be of some assistance. Just @ mention me... |
I have added a new PR, #42, largely reverting all changes in 1.11.0. It would be great if you could test if it is ok or if other changes are needed @merks @lscoughlin 👍🏻 There is also a 1.12.0-SNAPSHOT available that you can test, built on this branch. |
Thank you guys for the valuable feedback and learning experience this has been. Version 1.12.0 is now released: https://github.com/ethlo/itu/releases/tag/v1.12.0 |
You still need a module-info.java for java platform modules ( not OSGI ).
The text was updated successfully, but these errors were encountered: