-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
The project's generated MR JAR is not spec-compliant #346
Comments
@anthonyvdotbe Sorry but which "The JAR spec" do you mean by that? So this is a no-issue and I hereby close it. Wherever you found that sentence it is either outdated or misleading. |
@keilw this one, which is listed as one of the Java SE 11 specifications here Again, the spec states:
So the MR JAR of this project is not spec-compliant. And in case you're wondering: the one in Java SE 16 has the same text, so it's not outdated. Please reopen this issue. |
I'm not an expert on this topic, though it seems to me, the observation is correct, stating that Indriya's MR jar is not spec [1] compliant. I think there is little room on how to interpret
Which leads to the question, what to do about that. I'm also curious, can we ignore the spec? (That would be the easy way out.) [1] https://docs.oracle.com/en/java/javase/16/docs/specs/jar/jar.html |
This comment was marked as resolved.
This comment was marked as resolved.
@andi-huber thanks for your consideration
The spec also states:
In other words: javac/jar/java are allowed to assume MR JARs are well-formed, without being required to verify that their assumptions hold. So the spec can't be ignored, because malformed MR JARs will lead to issues such as the one in https://github.com/unitsofmeasurement/uom-demos/tree/master/console/java13 (e.g. during compilation, javac is allowed not to consider the versioned directories, because the public API must be the same across versions anyway). |
I'm not involved with that particular demo, but that might be a valid observation, no objections to that. |
Hi @keilw - seems to me, this is more serious than we might have thought at first glance. But again I'm by no means the expert here. |
@andi-huber Thanks for the input. The only other expert here (you are at least when it comes to the inner workings of Indriya but if you haven't dealt with JPMS or Multi-Release-JARs you are not alone;-) seems @nipafx because I not only saw, reviewed and approved his DWX talk his GitHub profile shows decent commits (just like that other GeoAPI case believe me, it is a bit like Football or other training and if someone lacks that kind of excersise they are usually either a total newcomer or someone who stopped coding ;-) while @anthonyvdotbe's profile is almost blank with 79 commits compared to our 1k+ to 2.5k+ in your case. First and most importantly Indriya is NOT the API, so the whole talking about APIs does not apply here. #301 is independent of the demo case and was raised for an affected Java version like 11 so there clearly are JDK bugs and they were not fixed till some later version. As for the question about the spec: There is no need to reopen this and those who at least read, review or even co-author specifications themselves will understand. |
I was under the impression that API in the context of a jar, does mean something very specific. But fair enough, I'm happy to learn. |
Glad it allowed you (and hopefully a few others, too) to learn a bit about the sometimes tedious and often very "political" way some of these specs are created rather than just bore you. I guess if @desruisseaux has the time he could also tell you a little more about how this works in the OGC although you may have got a little "taste" of that helping those GeoTools guys with their bugs and just as much, there have been far more ugly and "colorful" discussions between Spec Leads of e.g. what's now "Jakarta Inject" (when it was still under the JCP lead by other people) and CDI over the roles and responsibilities, than what we heard in the GeoTools issue or from some of its contributors ;-O |
This comment was marked as resolved.
This comment was marked as resolved.
@nipafx No it isn't, there is no point in making an enum that's only used by the implementation (see below) available in a Java version <12, sorry but that would be a waste of our resources and we have more important things to help our users with than this. It works in the Java versions from 13 on where it needs to, there is no value making the same enum available in earlier Java versions down to 8 because it would there be a no-op and useless. What makes the whole ticket even more ridiculous is, that That
That constant is completely |
This comment was marked as resolved.
This comment was marked as resolved.
You keep failing to understand this is and never WAS an API in the first place. It was merely an implementation detail yet you keep talking about "API".
Please see earlier, https://bugs.openjdk.java.net/browse/JDK-8207162 was linked here numerous times in this ticket and it's not the only one (also https://bugs.openjdk.java.net/browse/JDK-8210502 referenced there) The issue is between JPMS and multi-release JARs. Without |
Hello Werner. I have not examined this issue closely. But my reading of this report is that the Indriya JAR file does not provide the exact same set of public classes, methods and fields (we can ignore private things) for all JDK versions targeted in the JAR. If this is the case, this is indeed a risk of unpredictable behaviour at runtime. In order to analyse this issue, maybe a first step would be to list which classes/methods/fields are different? |
Martin, |
@desruisseaux, @nipafx These JDK bug(s) exist all the way to Java 15. I don't really have time or care which exact bug numbers they have in the JDK tracker, feel free to investigate yourself but fact is they were not addressed until Java 16, see console/java16, a merge of prior "java13" and "java14" demos also containing the format demo. It's good to know for Jakarta EE, that there is a version that allows combining JPMS via |
Unfortunately the JDK contributors often have a slightly different focus and priorities on the next Garbage Collector, Multi-Core in the Cloud, 20 different variations of Not sure if either of you even read the full spec including @nipafx because
Explicitly states the
Earlier also states, there can be classes depending on Java 8 or Java 9 so if you have a class that deals with So Indriya never violated anything, it works perfectly as the spec intends to at least on recent enough Java versions.
|
This comment was marked as resolved.
This comment was marked as resolved.
Werner, would it be possible to run the following command:
where As a side question for my information: is Indriya embedded with JEE? |
@nipafx As mentioned there are at least two different problems and most of these exist up to Java 15.
There is no need to "try" that because you probably never looked at these two classes. Again you keep babbling about the enum but the enum does not matter!!! It does not matter that this has an additional factory method getCompactInstance() because even if it was there, those are static methods and not part of an "API" which also the implementation doesn't even expose, it is not the API, I know the JDK sometimes does not make a clear difference be Unless you remove the @desruisseaux |
I just did the following test on a clone of Indriya project:
I got the following output:
So it it seems that Indriya has a public method for Java 12 which is not present on older Java version. Is it intended? |
Yes, because So if a method like |
If Indriya had a In the case where a method signature is invokable by all Java versions, we are at risk of
Admittedly we still have an exception, but it is a |
Either way that does not solve the issue that the JDK just gets it wrong if a |
@desruisseaux Until Java 16 that case only worked without a JPMS module-info declared in the consuming app. Not sure @nipafx if you also plan to talk about those things in your talk, but the JDK itself is most sketchy and inconsistent in applying and enforcing its own specs otherwise the behavior of modular and non-modular multi-release JARs would have been the same ever since they got introduced. Whether actual bug tickets were filed for any of that or not. |
This comment was marked as resolved.
This comment was marked as resolved.
@nipafx Thanks for replicating at least some of what we already did earlier.
That is a problem/bug of a Maven plugin (bnd or Felix), I guess it's documented somewhere, but they have not fixed that yet, you should build it with Java 14 and not 16, because the highest toolchain is also 14, see the CI build it works without problems. Of course the demo is succesful in Java 16 even with the overloaded methods that are Java 12 specific like So the spec from Java 16 is applied differently and that is the most important learning from this, beside the fact (which you also did not bother to replicate but we did it earlier) that the JVM behaved for non-modular multi-release JARs like that ever since at least Java 13 while only for modular multi-release JARs it behaved differently. After 16 this is no longer the case. If you or @anthonyvdotbe chose to ignore that or still have blind faith in what some spec that Oracle was just too lazy or busy to update says there is nothing to prevent you but as he even filed that ticket from some forgotten Jakarta EE ticket the important takeaway for Jakarta EE is what you can do with multi-release JARs after Java 16 and what you cannot do before that at least not without side-effects. |
This comment was marked as resolved.
This comment was marked as resolved.
Please point to the exact changes in Java 16 you believe are responsible for the different behavior
After Java 16:
With exact identical code, so Java 16 tolerates the overloading with new arguments (or even a new method that did not exist in the prior versions of the class), thus what "magical change" would you say is responsible for that?
Again remove the module-info and see the "broken" JAR was always accepted by JDK 13-15 or even below 13. How many specs do you review and ask the authors to change them (if they missed something) on a daily basis? How many have you written at Oracle???? You talk at conferences about what your colleagues like Brian Goetz, Mark Reinhold or others define, so please ask them before making assumptions. |
This comment was marked as resolved.
This comment was marked as resolved.
Well I do, and I also reviewed the very JPMS, see https://jcp.org/en/jsr/results?id=5959. While you just admitted yourself at Oracle you're merely a foot-soldier and if you can't find anybody in your company who would answer the question of changed behavior between Java 15 and 16, no worries I can. |
This comment was marked as resolved.
This comment was marked as resolved.
Thanks but your services are no longer needed, I'm going to ask the people responsible e.g. the Spec Leads about the obvious change of behavior (allowing an overloaded method in a multi-release-JAR where that was different before) |
I think that @nipafx has a good point about having the same API for all versions. I'm not familiar with what changed in JDK 16, but it seems to me that making the public API identical will be safer in any cases, isn't it? I also feel uncomfortable about authority argument. They often depend on a particular context, and can distract from the technical discussion. Do we have an agreement on the following technical points?
I did not tested the experiment that @nipafx did above, where he got the demo working. But if this approach work, what would be the drawback of fixing the issue reported here? Can we unblock him and instead gives this issue a rest for a few days before to reconsider? |
No he has nothing to add to this anymore just repeated noise I'm afraid. The only method for study purposes is deprecated, so are others, we don't just have the Java SE spec, there are others especially Unicode which is the reason why |
@nipafx The JBS issue with the fix to javac in JDK 16 is JDK-8235229. You are correct that the classes in a multi-release JAR file should be exactly the same across versions. |
I'm not familiar with Indriya API so I can only have superficial opinion on this issue; I let you manage. But while the discussion has wandered in different directions, I did not saw a reason to doubt about @nipafx technical competence. I feel that his contributions in other parts of "unitsofmeasurement" (if he wanted to) would be appreciable. I find unfortunate to loose his possible inputs in Seshat for example… |
Just see @AlanBateman's input, thanks a lot, that was useful and pretty close to the Spec Leads ;-) Thanks @AlanBateman and others for pointing @nipafx in the right direction so we got the answer asked pretty much at the beginning of this thread without having to read coffee grounds or make assumptions. It is probably even more useful for other specs like Jakarta EE and its user guides, etc. where in the past we already pointed to some of the Java SE specs but were asked to elaborate a little more about modularity and/or multi-release JARs. We'll keep him silenced here because everyone else is welcome and able to provide input in a more compact and precise manner like Alan did. And maybe after a few days or so cooling period I guess we may revert it. |
Well, I would like to have him welcomed at least in |
No it does not work in a particular sub-project but what kinds of contributions do you mean, code?` |
Not necessarily code. Bug reports are appreciated as well, or hints such as pointing to some specifications. |
You'll probably get more constructive input by the others involved including Alan, but after the 3 days he's welcome to participate again, hopefully a little more efficient. So even with the term "foot-soldier" may have been a little harsh, it still hits the point. And before BEA and the JRockit team (much of it including Flight Recorder came from there, we were asked to sell its predecessors like "BEA Guardian" while I worked there) was taken over by Oracle, I worked there as a "DRE" ("Developer Relationship Expert" I believe) in 2006, the only external contractor BEA accepted in the EMEA region at least back then. |
After we discussed this matter amongst ourselves and agreed that, as a courtesy to others who might read this issue, it’s important to point out you had misunderstood the MR JAR spec, @nipafx volunteered to communicate the answer to you because he had brought this issue to our attention and, frankly, no one who had interacted with you in the past would. @AlanBateman then bravely agreed to chime in briefly, but that’s it. |
The spec had alway been a little ambiguous especially Modular multi-release JAR files:
A module descriptor is generally treated no differently to any other class or resource file. A module descriptor may be present under a versioned area but not present under the top-level directory. This ensures, for example, only Java 8 versioned classes can be present under the top-level directory while Java 9 versioned classes (including, or perhaps only, the module descriptor) can be present under the 9 versioned directory. And while @nipafx kept insisting that the module-info may also be in the top-level (Java 8) folder under the right circumstances (e.g. we do this in the JSR 354 RI Moneta) placing that in Java 9+ folders is perfectly spec-compliant, see above.
Explicitly states "Library and framework developers can gradually migrate to and support new Java features while still supporting the old features." so one would think that e.g. the same as adding a new method like We had an earlier bug report #301 precisely caused by JDK-8235229 as it looks like. This was also referenced from the currennt one, but nobody ever mentioned that OpenJDK ticket until @AlanBateman did today. The only other tickets mentioned were unrelated and @nipafx seems to have mistaken them or not been aware of JDK-8235229. You @pron also wrote in jakarta-platform#329 but only about support for Java 8 or 11, never mentioning JDK-8235229 while that's pretty essential to a working Of course there are worse specs at least some that have hundreds or more optional aspects, just take JWT as one example, which is why I cautioned @m-reza-rahman and others about using it directly in Jakarta EE as opposed to a more "fluent" spec and API like MP-JWT. |
Just to be perfectly clear, JDK-8235229 is incidental to this issue, even though it may have caused some confusion about its origin as it's caused the underlying problem to manifest differently on different JDK versions. As @nipafx and others repeatedly tried to explain, the JAR specification currently requires that the public/protected signature of exported classes be identical for all versions for the JAR file to be well-formed and behave correctly in all situations; that a specific ill-formed JAR happens to incidentally behave correctly in some situations on some JDK versions is irrelevant. The |
While it may not be directly related to this one (which was just created without an actual problem based on discussions in Jakarta EE) it most certainly was for #301. I also pointed to it in jakartaee-platform#329 only briefly mentioning the examples (which reproduce BOTH at least until Java 16) but instead of getting to the root cause of #301 @anthonyvdotbe thought to help by creating this one. @anthonyvdotbe also quoted Alex Buckley from some JDK mailing list:
But not only is that precisely what the whole JDK does, there is no separate API JAR (and for some specs like JSR 310 there isn't even a clear separation of the API and API elements reference their own implementation while e.g. the Collections API was much better at this) even with smaller modules API and implementation are molded closer together than in Spring, there are also several Jakarta EE specs where the implementations cannot really separate the API, especially Faces. While in theory and in the academic world many things may sound great the reality of software projects and libraries is often different. This was more of an academic discurse or discussions in different parts of the project or I remember we also had with Brian or Mark Reinhold along the lines of JSRs like the JPMS (one of his professors was Stallman btw ;-D) so there is no more need here because the only theorecial problem (nobody ever hit it or filed an issue here) was preemptively addressed, but there is more actual work in Jakarta EE. And instead of beating this dead horse even further, what about #348? |
Thanks @pron for the clarification about JDK-8235229 and the links to tracking system. |
@desruisseaux please also have a look at #348 because you know (since you contributed a lot there) unit-api has one way to create a "dual-release JAR" (for Java 8+9) using So is there tool-support and if using "jar" instead of the maven-jar-plugin where is this done succesfully at the moment? |
This #349 is how that ticket here should have looked like. Unfortunately @anthonyvdotbe splashed something here without knowing enough or anything about the particular JSR as he also did in Jakarta EE before creating this only from a ticket there which the maintainers of Jakarta EE had pretty much ignored and nobody in the Spec Committee (myself included) ever heard about or took notice. It gave us answers to the much more crucial ticket #301 where there was a workaround until Java 16 and we still need it as fallback, but CompactNumberFormat and the related method or implementation details may never have been used by anybody hence the whold discussion here was about something less than 0.5% or fewer users ever hit, and so far none have. It seems like an excuse for both @anthonyvdotbe or @nipafx that they just read or talk about specs, but next time please also try to read THIS spec before you even create such a ticket in the first place or spend much time on it. It's up to the maintainers of the project or spec to prioritize, @AlanBateman or @pron may know why e.g. JDK-8235229 took between Java 11 and 16 to resolve, but "Created: 2019-12-01 08:10" sounds like it was only spotted more than a year after 11 came out. |
The JAR spec says:
However, this is currently not the case, since the Java SE 12 version adds additional API. This leads to issues such as the one described at https://github.com/unitsofmeasurement/uom-demos/tree/master/console/java13 (where it is incorrectly qualified as a JDK issue).
The text was updated successfully, but these errors were encountered: