search

jakartaee / inject

Apache License 2.0
17 stars 17 forks source link

An alternate way to build Java 8 source/target jar with Java 8 module… #25

Closed starksm64 closed 3 years ago

starksm64 commented 3 years ago

…-info.class

This is the approach that has been discussed on the platform dev list

Signed-off-by: Scott M Stark starksm64@gmail.com

Ladicek commented 3 years ago

I haven't seen that conversation, but relying on compiling the same source twice sounds rather brittle to me. I find the Moditect approach easier to understand and more robust. But I'm not married to it, if there's a "mandate" to use this approach, that sounds fine to me too.

starksm64 commented 3 years ago

The conversation has been on the platform-dev list. We will discuss this in the platform call tomorrow.

starksm64 commented 3 years ago

The conclusion on the call is that if a spec API jar has no actual requirements on the Java 11 SE APIs, the module-info.class should be provided in the root of the jar as per the mechanism in this PR. A multi-release jar should only be used if a spec API jar has some mechanism via a helper class, etc. to call the Java 11 SE APIs.

Ladicek commented 3 years ago

OK so I'm not terribly fond of this double compilation approach. The Moditect Maven plugin makes it super-simple to put the module-info.class into the root of the JAR, just delete the <jvmVersion>9</jvmVersion> line. In any case, if we have to do this, then we just do it.

Also CC @rbygrave @overheadhunter @Emily-Jiang

overheadhunter commented 3 years ago

While I'm fine with the approach of two compilations (using the same myself), I need to stress again, that the pre-modular Java world (including Android) may not like to find a Java 9 class inside the root of the jar.

I personally don't use Java 8 and neither do I use build tools or frameworks with fancy class loaders that load every .class they find in a jar. So I don't care. But some library vendors do care, as they don't want to risk breaking things.

Yes, specs allow to put the module-info into the root. And yes, we could argue that aforementioned tools that don't comply with specs are just stupid. But this would be a bad rationale to put compatibility at risk. So far I'm missing arguments against MR-Jars.

rbygrave commented 3 years ago

put compatibility at risk.

Yes. In the prior PR discussion we settled on MR Jar due to the compatibility risk.

So far I'm missing arguments against MR-Jars.

Yes. It would be good to know these arguments so that it's more obvious to us why they outweigh the compatibility risk that is mitigated by using MR Jar.

Emily-Jiang commented 3 years ago

I think the pushback for MR jars is that it overly complicate things. Overall concensus is that module-info under root is much simplier. Besides, some specs might put module-info under version/9 and others put version/11 etc, which causes issues to scan for these locations.

For more info regarding this, please refer to the minutes for the Jakarta platform call yesterday.

Emily-Jiang commented 3 years ago

@starksm64 I am with @Ladicek . I don't think you need to make so many changes. IIUC, just move the module-info under root and job done. You can still use the existing plugin and still use MR jar, which is allowed.

Emily-Jiang commented 3 years ago

I am not sure whether we have to produce a MR jar. From yesterday's call:

module-info must be placed in the root of the JAR The four specs from JDK 8 already support module-info: Activation, XML Binding, XML web services, SOAP attachments, jsonp, mail, persistence, jsonb In case a spec wants/needs to support previous versions of Java, a multi-release JAR may be required for that spec (e.g., JAXB in EE 9/9.1) Multi-release JAR is not needed just for adding the module-info

overheadhunter commented 3 years ago

I think the pushback for MR jars is that it overly complicate things. Overall concensus is that module-info under root is much simplier.

When you say "overly complicate", you're implying that there is a simpler solution with the same result. Sure, a module-info in the root is "simpler", but as mentioned multiple times, the results are not the same.

Besides, some specs might put module-info under version/9 and others put version/11 etc, which causes issues to scan for these locations.

Actually, it is an official standard for any module-aware JRE (and any other JRE or tool does not interact with module-info anyway). Therefore it is safe to assume that any tool that knows MR-Jars also knows module-info and vice versa.

What issues would that be? You don't need to decide to put versions into 9 and/or 11. At compile time, you simply store version-specific classes using the oldest supported version (potentially root). At runtime you load classes from all supported versions, ignore anything newer and resolve conflicts by using the newest supported version. You don't even need to worry about this, it is the JREs job to make these decisions.

just move the module-info under root and job done. You can still use the existing plugin and still use MR jar

That is a contradiction. Either you store it in the root or use the multi-version approach.

Let me explain my motivation in pushing this towards maximum compatibility: As said before, some library vendors (looking especially at Google here) don't want breaking changes. And as I use Google's Dagger library, I have an interest in Dagger switching from javax.inject to jakarta.inject. However, they won't do so, if jakarta.inject breaks downstream builds for users, who're stuck with Java 8. Specs are cool, but the reality is that some tools don't give a f** about specs and just load everything that ends with `.classinside a jar (maybe even from withinMETA-INFbut at least this exists a little longer thanmodule-info.class`, so apparently it got excluded).

starksm64 commented 3 years ago

The arguments against MR jars were:

  1. There is at least one bug in their handling that has not been addressed in SE 11.
  2. Builds that combine jars ala fat jars, etc. would need to deal with multiple locations from which to filter module-info.

Frankly the JPMS tech continues to be an experiment when it comes to user facing applications. We can simply produce two api artifacts and let users run their own experiments. The main jar would be the module-info.class in the root to be consistent with what the rest of the Jakarta specs are doing. A second MR jar with a mr qualifier can be produced in addition.

starksm64 commented 3 years ago

OK so I'm not terribly fond of this double compilation approach. The Moditect Maven plugin makes it super-simple to put the module-info.class into the root of the JAR, just delete the <jvmVersion>9</jvmVersion> line. In any case, if we have to do this, then we just do it.

But it is introducing another plugin to set the module-info.class info rather than using the approach of adding the module-info.java to the source tree as is being done by every other spec project. My least favorite activity when looking at a spec project build is trying to figure out what non-basic plugins are doing.

rbygrave commented 3 years ago

We can simply produce two api artifacts and let users run their own experiments.

I like this, to me this sounds like the best approach and sounds similar to what was mooted in a previous PR (The Java 11 PR we closed).

To be more explicit, what I think was mooted was to release 2 artifacts:

  1. First artifact (version 2.0.1) that uses Java 8 and is a MR Jar to provide the best compatibility with folks in Java 8 land.
  2. Second artifact (version 3.0.0) mooted to bump to Java 11 (or Java 9+) and has module-info at the source root and doesn't need any special tooling at all - nice and simple. The variation is that this second artifact could actually still stay using java 8 (this PR) rather than bump to java 11.

With 2 artifacts we then support both cases nicely and people can choose.

So the plan would be to:

overheadhunter commented 3 years ago

I like the idea of two different artifacts, but let me reframe it:

  1. One artifact for the "new era": module-info.class at the jar's root, jakarta namespace, consumers will most likely be on JDK 9+ anyway
  2. One legacy artifact, which is targeting consumers still on JDK 8 and javax.inject - JDK 8 doesn't even know about mr jars, therefore it can not contain bugs when handling mr jars (mentioned by @starksm64), therefore (please hear me out) this artifact should be a MR-Jar:

This approach should combine the most benefits:

Ladicek commented 3 years ago

Frankly, I'm tired of rehashing the same arguments over and over again. (And I'm tired of having to point out an intrinsic inconsistency in the argument that "some tools scan all .class files, therefore we can't have module-info.class in the root, but we can put it info META-INF/versions, because surely those tools will ignore that location".)

Here's a proposal: we produce 2 JARs, but not 2.0.1 and 3.0. We produce 2.0.1 with module-info.class in the root. We also produce 2.0.1-nomod that will not include module-info.class at all.

How does that sound?

EDIT: I see the previous comment contains a similar proposal. I believe we should just do what I described, because the MR JAR does not address the fear of breaking changes. Tools that scan for .class files and pick up /module-info.class cannot be expected to ignore /META-INF/versions/9/module-info.class, simply because the reality is that some tools don't give a f*** about specs and just load everything.

overheadhunter commented 3 years ago

Frankly, I'm tired of rehashing the same arguments over and over again. [...]

Me too. I'm not personally interested in these discussions, I just want to benefit from a modular jar.

But if downstream maintainers of some of the most wide-spread Java libs out there are unwilling to adapt such change, I will never know the true promise of this module-info. Therefore I feel the need to bring attention to those fears expressed by said library maintainers.

If this is not allowed, we can of course just close our eyes and live in a bubble where everything adheres specs. And you can believe me, I would love to live in this bubble as well.

Here's a proposal: we produce 2 JARs, but not 2.0.1 and 3.0. We produce 2.0.1 with module-info.class in the root. We also produce 2.0.1-nomod that will not include module-info.class at all.

How does that sound?

Maybe I wasn't clear enough: There is no need for any such compromise. For the jakarta namespace it is safe to assume downstream projects are on a (mostly) modern tech stack. In other words: Just put the god damn module info into the jar's root and that's it. No need for a separate release.

The whole point of this PR is to reduce alleged complexity. This goal is hardly achieved by committing to maintaining two separate flavours of the same lib.

Emily-Jiang commented 3 years ago

I would just say this: just produce a jar with module-info.class under root. Full stop! If someone has problems, they can either remove the module-info.class from the jar themselves or raise an issue here and we then produce another jar. I don't like the idea to producing a swiss knife just in case. Since it is so easy and simple to do a service release, just do one and wait for feedback. If we got some request, then add another one. There is no other specs releasing multiple api jars as far as I know. If we want to release two jars, another conversation on the platform needs to happen again. It takes time. I am not sure it worth it before we have a concrete use case - at the moment, it is based on some possible misuage.

starksm64 commented 3 years ago

So a 3.0.0 or even a 2.1.0 release is off the table for EE10 because both of those require a release plan with a specification committee vote. The initial deadline for specifications is Oct 15, so all we can do is a point service release.

This is what we'll do:

  1. Merge this PR and release it as the 2.0.1 version for EE10.
  2. Create a separate release 2.0.1.MR multi-release jar using the previous PR approach and state there is no commitment to maintain that approach unless there is overwhelming feedback that it is required due to issues with legacy libraries.
  3. Create a 1.0.4.MR multi-release jar of the javax.* namespace API.