JDK-8222791 : Order of evaluation of -link params in Javadoc tool reversed -> regression with split packages
  • Type: Bug
  • Component: tools
  • Sub-Component: javadoc(tool)
  • Affected Version: 11.0.2,12
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2019-04-21
  • Updated: 2019-08-06
  • Resolved: 2019-07-22
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 14
14 b07Fixed
Related Reports
Relates :  
Description
We found this problem when we migrated Apache Lucene to minimum Java 11. Apache Lucene uses a documentation linter to check for broken links and so we recognized that bug very fast. Although it's a bad idea with modules to have split packages (org.apach.lucene.something in different jar files), we can't easily fix this (we are not yet using modules!). In short: Lucene JAR files have same package in different JARs.

We also add crosslinks in Javadocs, e.g., the analysis JAR has org.apache.lucene.analysis.standard, but also Lucene Core has it). In a third jar file we have links to a class in this package which resides in Lucene Core. Lucene adds "-link" to Javadoc command line when calling for each dependent Lucene module, staring with the "Lucene Core" one first.

In all old JDK versions up to Java 11.0.1, the order of "link" elements when calling Javadocs are evaluated left to right and only the first occurence of a java package is taken. Lucene passes the link element in the order "important" to "less important". Javadocs tool was taking the package from the first link that matches (so earlier package name wins).

Before JDK-8205593 the lookup map was populated by reading all the packages-files, checking if package is already in package lookup map and add it only if its not there. By that "first-wins" (it's only inserted when seen for first time). After the Javadocs JDK-8205593 patch, the containKey() was removed and all packages are added to map, so "last-wins".

Before: http://hg.openjdk.java.net/jdk/jdk/rev/f7d40158eb2f#l7.60 (see !containsKey)
After: http://hg.openjdk.java.net/jdk/jdk/rev/f7d40158eb2f#l7.196 (see the put at end, so if same package in same Jigsaw module "unnamed", its overwritten and so latest item wins)

So that means all of Lucene's Javadocs are broken after Java 11.0.2, which is horrible as it happens depending on which minor version of JDK you are using. 11 and 11.0.1 works, 11.0.2. is broken. So depending on the JDK version we produce incorrect links.

As it is unlikely to be fixed in Java 11, we added a workaround: We have a regex ANT task patching the element-list files after building the Javadocs and removing all lines in all Lucene JAR files that have duplicates from the Lucene Core JAR file.

But this should be fixed by adding the "containskey" back (see above, also works with the map of maps).
Comments
Thanks Jonathan, sorry for late response. Looks fine, I will test this (after disabling our workaround) with the next JDK preview build. About your question: No it's not really with split packages, as we do not use the module system at all. I just called it "split package" because it affects all project setups, where the same package exists in multiple compilation units.
25-07-2019

URL: https://hg.openjdk.java.net/jdk/jdk/rev/afe8584ac8d9 User: jjg Date: 2019-07-22 23:15:18 +0000
22-07-2019

[~uschindler] Just checking ... this is not about split packages as such, right? Fundamentally, it's just about different versions of a package being made available via -link, and the wrong version being chosen. The contents of the package don't really matter.
18-07-2019

With the javadoc test infrastructure, it should be relatively easy to set up some split packages and make sure that the right declaration ends up in the generated files. Note: it's not "just" split packages, it's multiple libraries with split packages. That being said, a minimal solution just requires package-list or element-list files; it's not relevant to the test that they be part of some fully-formed API documentation.
18-07-2019

Hi & thanks Jonathan, I have no idea what the best test for this is (as you need a quite complicated build setuip with split packages). Maybe just fix the bug with reference to the old commit? I know tests are good, but I am not sure if it's worth a test. Uwe P.S.: we are living with our "element-list regex hack" workaround for now (as we are targeted to java 11 and it's unlikely that a fixes comes for that version). It was only a problem as the regression occurred during a bugfix release of Java 11, so there is no easy fix for our build system without having complicated version logic in our Ant-based build.
12-06-2019

Agreed, at least in principle, with the fix in the preceding comment (https://bugs.openjdk.java.net/browse/JDK-8222791?focusedCommentId=14258924&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14258924) so this may be one of those cases where the test is much more complicated than the fix.
11-06-2019

General discussion, for the record. At least at the simple option-decoding level, the prevailing practice is "last one wins". But for items like types on a package-oriented path, the prevailing practice is "first one wins". In this case, there is obviously the strong parallel with paths (so first one wins) and the fact that this is a regression from earlier behavior
11-06-2019

Fix is to replace "put" by "putIfAbsent" in "readElementList": packageItems.computeIfAbsent(moduleName == null ? DocletConstants.DEFAULT_ELEMENT_NAME : moduleName, k -> new TreeMap<>()) .put(elemname, item); To: packageItems.computeIfAbsent(moduleName == null ? DocletConstants.DEFAULT_ELEMENT_NAME : moduleName, k -> new TreeMap<>()) .putIfAbsent(elemname, item);
21-04-2019