-
Notifications
You must be signed in to change notification settings - Fork 975
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
feat: support jpms in annotations
module
#2302
Conversation
cc / @cpovirk |
4468bf8
to
5e62dcf
Compare
- feat: add `module-info.java` to `annotations` - chore: wire up build support for building `java9` root in MRJAR - chore: apply java9 jpms changes to both maven and makefile build Signed-off-by: Sam Gammon <sam@elide.ventures>
5e62dcf
to
d0aafde
Compare
|
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.
Thank you very much for this contribution, and all your notes regarding it. I'll work with the Guava team when this is committed so their next release uses this version.
@tomball Thank you for the quick response! And for all of your hard work on J2ObjC. 😄 I'll keep an eye on this in case you need any changes from me. |
Thanks, @sgammon and @tomball! Before your PR yesterday, I had been playing with the alternative of replacing Guava's So it will be great to have |
@cpovirk If 10 annotation classes are still too much, I can offer a PR which links users to Proguard ;) |
@tomball Thank you both for the quick review and merge. Made my day! 🥳 |
:) In fairness, my sense is that some of the concerns are things like compliance overhead and the need to include another project if you're building from source. But a lot of objections do sound like cases that would be best served with Proguard.... |
Right, that's one reason none of these annotations have
RetentionPolicy.RUNTIME -- they have nothing to do with code execution,
only with compiler (j2objc)
generation.
…On Fri, Mar 8, 2024 at 11:42 AM Chris Povirk ***@***.***> wrote:
:)
In fairness, my sense is that some of the concerns are things like
compliance overhead and the need to include another project if you're
building from source. But a lot of objections do sound like cases that
would be best served with Proguard....
—
Reply to this email directly, view it on GitHub
<#2302 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAW2JL64GPJULBKAL7DQODLYXIIDDAVCNFSM6AAAAABEL2XSQ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBWGMYDOOBQHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@tomball Happy meals were exciting to me when you started coding in Java. So I will defer to you, this is why |
Summary
This PR adds a
module-info.java
definition to theannotations
package, and adjusts the build to produce a MRJAR. The result is a JAR artifact which (1) remains safely usable on JDK 1.8, and (2) helps enable modular Java builds for consumers of the annotations.Downstream, Guava depends on these annotations, and many artifacts depend on Guava. As a result, many projects that don't otherwise use J2ObjC or ship for iOS rely on J2ObjC adopting a module definition like this one.
I've made the minimum suite of changes possible here to maintain both the
make
andmaven
-based builds. The two produce nearly identical JARs, save for the Maven Compiler Plugin stamping the JAR manifest differently with its own message. I've cleaned and performed a full rebuild of the project to ensure the JARs are properly usable.For now, I will build and issue the JAR artifacts at the version
2.8-jpms
, and make them available for downstream testing. I think releasing the annotations under a version postfix like that might help projects opt-in before this ships for real.Changelog
module-info.java
toannotations
java9
root in a multi-release jarRelates to: google/guava#2970