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

add jpms module info #19

Closed
wants to merge 1 commit into from

Conversation

h908714124
Copy link

Suggestion: Bump java version to 11, and add module-info to sources root.
This is one possible way to resolve #17.

@h908714124
Copy link
Author

Wow legal checks. FOSS sure is fun these days.

Copy link

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

I am not sure we want to add these. There needs to be an agreement on jakarta level about it before proceeding (i.e. all specs should have it or none for certain ee release).

Personally I am -1 because I simply consider JPMS a grand leap in wrong direction but that's mostly irrelevant personal opinion here :-)

@h908714124
Copy link
Author

@manovotn jpms is needed to use jpackage, to create custom runtime images. What's the alternative? Fat jars? Shade plugin?

@A248
Copy link

A248 commented Jul 8, 2021

I don't see any good reason to delay adding a module descriptor. Whether or not you like JPMS, it's here to stay, and it's best that projects provide compatibility with it, especially as adoption increases.

I am not sure we want to add these. There needs to be an agreement on jakarta level about it before proceeding (i.e. all specs should have it or none for certain ee release).

I don't quite follow your reasoning here. From a specification standpoint and for the purposes of declaring dependencies, what matters is the declared module name.

Adding a module descriptor is more akin to a feature enhancement with no effect on the API specification. The descriptor states qualities of an API which were already the case pre-modularization. For example, "internal" packages were never part of the API, and thus when modularization occurs, they're simply not exported. This may change runtime behavior, as JPMS enforces this encapsulation, but no changes to the specification took place, strictly speaking.

@manovotn
Copy link

manovotn commented Jul 8, 2021

My personal reason is irrelevant, like I said.

What I'd like to see prior to accepting this is an agreement that next EE version will have this for all artifacts/specs.
Besides, I am not the only one who can review and merge PRs. So an opinion of some other commiter would be welcome. You could also start a discussion on the mailing list which might draw attention of more people.

@overheadhunter
Copy link

overheadhunter commented Jul 8, 2021

GitHub drew my attention as well 😉

While I personally (again - irrelevant) would love to see this PR, we should keep in mind that Java 8 compatibility is requested elsewhere (see discussion in #18). It is my understanding that the debate covers both 1.x as well as 2.x jars. If this is correct, the only option to add a module-info.java would be a multi-release-jar.


There needs to be an agreement on jakarta level about it before proceeding (i.e. all specs should have it or none for certain ee release).

jakarta.annotation and jakarta.ws.rs have a module-info btw

@Emily-Jiang
Copy link
Contributor

I think this change will need to be added to the project release plan if it has not already. At the moment, on the platform calls, we are discussing the strategies towards bumping compile java version (would this require a major or minor release?). As for jpms, it is not required as yet but component specs can add it if they know what they are doing. I think we should wait to merge this PR till platform spec team agreed on a policy towards the java compile version. Is there a compiling reason to support jpms right now?

@h908714124
Copy link
Author

@Emily-Jiang I think jpackage requiring it is a compelling reason to use jpms right now. Many things depend on @Inject, including dagger which is foundational itself. Well, actually dagger depends on javax.annotation, but that project is inactive.
There are open dagger tickets to add jpms support, and to support jakarta annotations. As it stands, one cannot be achieved without the other. But even then, the former ticket will only be resolved when jakarta also adds a module descriptor.

@starksm64
Copy link
Contributor

It is the next topic that the Jakarta platform group needs to address, so we will update the api jar once that is concluded.

@msgilligan
Copy link

@h908714124 thanks for making the PR. My recommendation would be to set source/target Java version to 9. Although most people will probably want to use 11/LTS, there's really no advantage to making this JAR require Java 11.

@h908714124
Copy link
Author

Hey @msgilligan, thanks for the suggestion. Good point. However this PR can't be merged due to legal constraints, so I guess it's not even worth the effort.

@dwhitla
Copy link

dwhitla commented Sep 27, 2021

I am not sure we want to add these. There needs to be an agreement on jakarta level about it before proceeding (i.e. all specs should have it or none for certain ee release).

Personally I am -1 because I simply consider JPMS a grand leap in wrong direction but that's mostly irrelevant personal opinion here :-)

Honestly - what people may want based on their opinion of the direction taken by Oracle when releasing jigsaw is irrelevant. Right now the whole JavaEE (JakartaEE) ecosystem is broken for people forced to use Java11+. This costs almost nothing to implement but spares application developers budget-breaking amounts of time working around its absence using hacks like moditect. I just can't express how damned frustrated I am as a 22 year Java veteran with how JPMS is being handled by people who are supposed to be leading the community. It just breaks my brain how people are happy to let the language die because Oracle had the hide to try and fix a very real problem with the JVM but not use the existing poorly designed module systems. Java9 shipped almost 5 years ago - it happened - can we just move forward please.

@dwhitla
Copy link

dwhitla commented Sep 27, 2021

While I personally (again - irrelevant) would love to see this PR, we should keep in mind that Java 8 compatibility is requested elsewhere (see discussion in #18). It is my understanding that the debate covers both 1.x as well as 2.x jars. If this is correct, the only option to add a module-info.java would be a multi-release-jar.

This is not correct. Java8 and earlier ignore the module-info.class. The only requirement for Java8 compatibility is that the class version be no later than Java8. Shipping a multi-release jar would certainly allow it but is not a prerequisite unless you want multiple versions of a class which utilise language features not present in all JVM releases.

@dwhitla
Copy link

dwhitla commented Sep 27, 2021

I think jpackage requiring it is a compelling reason to use jpms right now.

Not to mention jlink, jdeps and GraalVM native image creation. And lets not leave out JavaFX which requires Java11+ and strongly pushes developers to Java16 in order to address bugs in earlier releases. The newer Java based technologies that keep Java competitive as a development platform and relevant as a language in many businesses are being hamstrung by what used to be Java's greatest strength - the OSS libraries. When Spring, JBoss and even the bloody EE reference implementation refuse to adopt a language specification it pretty much kills Java as a language for many of my clients. And I'm too old, too invested in Java and too angry to learn .Net just to stay employed.

@dwhitla
Copy link

dwhitla commented Sep 27, 2021

Can someone please elaborate on the nature of the legal hold up here?

@rbygrave
Copy link
Contributor

@dwhitla I believe the hold up is that @h908714124 has not signed the Eclipse ECA agreement. Is that correct @h908714124 ?

@h908714124 are you ok with me re-submitting your PR or would you be up to signing the eclipse eca agreement?

I would love to see this PR merged so I have signed the eca (at least I believe I have) and I am happy to re-submit this PR and see if we can get that merged. Any objections?

If we desire I am also happy to do an initial PR that stays with Java8 and probably uses moditech plugin to add the module-info (as that's what I've been doing myself elsewhere). We would then get the maintainers to release that before we then merge this second PR which bumps to Java 9 and get the maintainers to release again a second time.

Are people on board with this? Do we want the initial Java 8 release as well as the Java 9 one?

@rbygrave
Copy link
Contributor

rbygrave commented Sep 27, 2021

FYI:

@h908714124
Copy link
Author

Hey @rbygrave, how are you doing buddy? If you have already signed the ECA, then please go ahead and resubmit this PR. Thanks

@h908714124 h908714124 closed this Sep 27, 2021
@h908714124
Copy link
Author

h908714124 commented Sep 27, 2021

This PR is replaced by 21

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.

Add explicit module-info class file, to prevent resolving all automatic modules
9 participants