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

Inheriting Javadoc from the JDK is broken (as was _linking to_ the JDK) #6790

Open
cpovirk opened this issue Oct 18, 2023 · 11 comments
Open

Comments

@cpovirk
Copy link
Member

cpovirk commented Oct 18, 2023

e.g., https://guava.dev/BiMap has blank descriptions for put, putAll, and values, and it has no links for types like java.lang.Object and java.util.Map

@cpovirk
Copy link
Member Author

cpovirk commented Oct 18, 2023

I can reproduce this locally with ./mvnw clean javadoc:javadoc with JAVA_HOME pointed to Java 11. I can also reproduce it with mvn, so it's not likely to be related to my recent Maven Wrapper change.

@cpovirk
Copy link
Member Author

cpovirk commented Oct 18, 2023

The info disappeared at bdbcb37, so it's been almost a year :\

A change from around that time that looks plausibly related is 8a676ad. I could believe that the cause is java.specification.version.

@cpovirk
Copy link
Member Author

cpovirk commented Oct 18, 2023

Yes, I can fix it by removing the java.specification.version change. But that point, Javadoc prints a bunch of warnings under JDK 11, and it fails entirely (or at least pretty badly) under JDK 17.

I could believe that the problem is somehow related to modules. Roughly, the JDK sources might be "hidden" by the JDK modules?

I briefly tried messing with the path we pass to Javadoc, passing ${project.build.directory}/jdk-sources/java.base instead of just ${project.build.directory}/jdk-sources, but that didn't help.

I tried removing our exclusion of the JDK's module-info files, but then I got:

[ERROR] Creating an aggregated report for both named and unnamed modules is not possible.
[ERROR] Ensure that every module has a module descriptor or is a jar with a MANIFEST.MF containing an Automatic-Module-Name.
[ERROR] Fix the following projects:
[ERROR]  - com.google.guava:guava

So it's possible that things would go better if Guava were a real module (#2970). (Presumably Javadoc would be willing to inherit Javadoc across module boundaries.)

I could probably "fix" this by setting the Javadoc source version back to 8 for JDK 11 (where we know that it works). But we know that it doesn't work for JDK 17, so this would be only a temporary fix.

@cpovirk
Copy link
Member Author

cpovirk commented Oct 18, 2023

(Presumably Javadoc would be willing to inherit Javadoc across module boundaries.)

I guess I'm somewhat pessimistic about that, given that the sources probably are being hidden entirely.

But I could believe that we could do something with --patch-module or something. And in fact, after poking around a little, I found https://bugs.openjdk.org/browse/JDK-8300694. (That bug even notes that this worked for Guava at the latest release but not at head at the time :))

I'm not excited about trying to get this to work, but if anyone wants to try to wire it up, either the temporary solution of using <source>8</source> under JDK11 or the permanent(?) solution of figuring out --module-path, please be my guest.

@cpovirk
Copy link
Member Author

cpovirk commented Oct 18, 2023

Oh, and in all this, I've been forgetting about the problem linking to the JDK. I would guess (but haven't checked) that that problem is a result of the same commit as the other problem. It may be related to how we still link (well, try to link) to JDK 9, which of course supports the module system but does not yet include the module name in the path of the Javadoc it produces.

@cpovirk
Copy link
Member Author

cpovirk commented Mar 11, 2024

It looks like #7087 fixes linking in snapshot Javadoc by switching to Java 21 for snapshot-Javadoc generation.

If we want to make things work for our releases, then we can do that by using Java 21 for our releases, too. (Now, I've always been in the habit of using snapshot Javadoc, myself. But I don't know what most users do.)

If we switch to Java 21, we'll have to be careful to continue to generate bytecode that works under older versions (which -target doesn't guarantee). Fortunately, Animal Sniffer usually accomplishes that, such as with a recent problem with SequencedCollection in Java 21+. (See also #6903 and #6832.) That's more important for releases than for snapshots, but we would of course prefer not to break snapshots under older JDKs, either :)

@cpovirk
Copy link
Member Author

cpovirk commented Mar 11, 2024

(Snapshot docs are built by a different workflow task/whatever than snapshots themselves, so we could also use Java 21 for one but not the other. Releases currently use the same Java version for each.)

copybara-service bot pushed a commit that referenced this issue Mar 11, 2024
Drops the JDK source steps involved with Javadoc builds, as suggested in #7089. There will be some build warnings related to Javadoc until subsequent PRs are merged.

Signed-off-by: Sam Gammon <[email protected]>

Gives up on the "inherting" half of #6790.

Fixes #7092

RELNOTES=n/a
PiperOrigin-RevId: 614681928
copybara-service bot pushed a commit that referenced this issue Mar 11, 2024
Drops the JDK source steps involved with Javadoc builds, as suggested in #7089. There will be some build warnings related to Javadoc until subsequent PRs are merged.

Signed-off-by: Sam Gammon <[email protected]>

Gives up on the "inherting" half of #6790.

Fixes #7092

RELNOTES=n/a
PiperOrigin-RevId: 614693592
copybara-service bot pushed a commit that referenced this issue Mar 12, 2024
The main changes is that tests need `-Djava.security.manager=allow`.

Fixes and closes #7065

Helps with #6790

Fixes #6245 (at least the remaining parts that we actually care about)

Fixes #7087

Signed-off-by: Sam Gammon <[email protected]>
RELNOTES=n/a
PiperOrigin-RevId: 614671411
copybara-service bot pushed a commit that referenced this issue Mar 12, 2024
The main changes is that tests need `-Djava.security.manager=allow`.

Fixes and closes #7065

Helps with #6790

Fixes #6245 (at least the remaining parts that we actually care about)

Fixes #7087

Signed-off-by: Sam Gammon <[email protected]>
RELNOTES=n/a
PiperOrigin-RevId: 614671411
copybara-service bot pushed a commit that referenced this issue Mar 12, 2024
The main changes is that tests need `-Djava.security.manager=allow`.

Fixes and closes #7065

Helps with #6790

Fixes #6245 (at least the remaining parts that we actually care about)

Fixes #7087

Signed-off-by: Sam Gammon <[email protected]>
RELNOTES=n/a
PiperOrigin-RevId: 614671411
copybara-service bot pushed a commit that referenced this issue Mar 12, 2024
The main changes is that tests need `-Djava.security.manager=allow`.

Fixes and closes #7065

Helps with #6790

Fixes #6245 (at least the remaining parts that we actually care about)

Fixes #7087

Signed-off-by: Sam Gammon <[email protected]>
RELNOTES=n/a
PiperOrigin-RevId: 615151162
@cpovirk
Copy link
Member Author

cpovirk commented Mar 13, 2024

Wait, no, #7087 (aka #7102 aka f2b8c4f) causes Javadoc generation to fail?

Error: Exception in thread "main" java.lang.NoClassDefFoundError: com/sun/javadoc/Doclet
	at java.base/java.lang.ClassLoader.defineClass1(Native Method)
	at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1027)
	at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:150)
	at java.base/java.net.URLClassLoader.defineClass(URLClassLoader.java:524)
	at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:427)
	at java.base/java.net.URLClassLoader$1.run(URLClassLoader.java:421)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:714)
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:420)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:593)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	at jdk.javadoc/jdk.javadoc.internal.tool.Start.loadDocletClass(Start.java:800)
	at jdk.javadoc/jdk.javadoc.internal.tool.Start.preprocess(Start.java:771)
	at jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:376)
	at jdk.javadoc/jdk.javadoc.internal.tool.Start.begin(Start.java:347)
	at jdk.javadoc/jdk.javadoc.internal.tool.Main.execute(Main.java:57)
	at jdk.javadoc/jdk.javadoc.internal.tool.Main.main(Main.java:46)
Caused by: java.lang.ClassNotFoundException: com.sun.javadoc.Doclet
	at java.base/java.net.URLClassLoader.findClass(URLClassLoader.java:445)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:593)
	at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:526)
	... 16 more

I had tested something locally, but I guess it wasn't that. For now, I'll roll back to JDK 11.

copybara-service bot pushed a commit that referenced this issue Mar 13, 2024
(i.e., roll back a small portion of cl/615151162)

It turns out that Javadoc snapshots [fail when using Java 21](#6790 (comment)).

RELNOTES=n/a
PiperOrigin-RevId: 615518606
copybara-service bot pushed a commit that referenced this issue Mar 13, 2024
(i.e., roll back a small portion of cl/615151162)

It turns out that Javadoc snapshots [fail when using Java 21](#6790 (comment)).

RELNOTES=n/a
PiperOrigin-RevId: 615532809
@cpovirk
Copy link
Member Author

cpovirk commented Apr 23, 2024

Linking to the JDK works with a Java 21 (and perhaps earlier versions), so that's a reason to generate docs with a newer version once we can. (We'd want to think about both what we do for snapshots and what we do for releases.) Inheriting docs from the JDK doesn't work automatically, so I'd probably continue not to worry about it.

The only javadoc-21 blocker I remember offhand is JDiff, which I'm assuming is responsible for the error reported just above.

copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to remove the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592.

(See also [these notes](#5457 (comment)).)

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655592201
copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to remove the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592. I assume that we'll now get the default from the JDK whose Javadoc binary we're using, which (again) will now be 11. That seems fine. We could consider setting it to 8 to match our normal build (which I thought I had remembered was the `maven-javadoc-plugin` default, but I don't think I'm seeing that, at least not under our current versions), but I don't see much downside to 11—or even to newer versions that we might someday use for Maven Javadoc generation, given that we keep the code compiling under new versions already.

Some other thing I'm wondering:

- I wonder if we should activate(?) some of the plugins, including the new toolchain plugins, in the `<plugins>` (not just `<pluginManagement>`) section of the parent `pom.xml`. Might that save us from having to do so in each separate `pom.xml`? (We might actually mostly get away with activating(?) them only in the main `guava` build: That _downloads and registers_ the toolchains, and then at least the other projects' _per-plugin_ toolchain configuration probably finds them? But for the more general configuration to work, I think we at least need to activate(?) `maven-toolchains-plugin` in each? I haven't experimented a ton with this.)

(See also [these notes](#5457 (comment)).)

So

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655592201
copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to remove the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592. I assume that we'll now get the default from the JDK whose Javadoc binary we're using, which (again) will now be 11. That seems fine. We could consider setting it to 8 to match our normal build (which I thought I had remembered was the `maven-javadoc-plugin` default, but I don't think I'm seeing that, at least not under our current versions), but I don't see much downside to 11—or even to newer versions that we might someday use for Maven Javadoc generation, given that we keep the code compiling under new versions already.

Some other thing I'm wondering:

- I wonder if we should activate(?) some of the plugins, including the new toolchain plugins, in the `<plugins>` (not just `<pluginManagement>`) section of the parent `pom.xml`. Might that save us from having to do so in each separate `pom.xml`? (We might actually mostly get away with activating(?) them only in the main `guava` build: That _downloads and registers_ the toolchains, and then at least the other projects' _per-plugin_ toolchain configuration probably finds them? But for the more general configuration to work, I think we at least need to activate(?) `maven-toolchains-plugin` in each? I haven't experimented a ton with this.)

(See also [these notes](#5457 (comment)).)

So

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655592201
copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to change the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592. I first tried going with the default from the JDK whose Javadoc binary we're using, which (again) will now be 11. But that led to a problem in `org.codehaus.plexus.languages.java.jpms.CmdModuleNameExtractor`, which apparently tries to look up the module name for the `-source 11` run but uses the Maven run's JDK instead of the Javadoc toolchain or Maven toolchain. So now I've set it to 8 to match what we use for `maven-compiler-plugin`. (I _thought_ I had remembered that `maven-javadoc-plugin` defaulted to matching `maven-compiler-plugin`, even though that's weird. Maybe the two actually just read from the same Maven property or something, or maybe the behavior changed.)

Some other thing I'm wondering:

- I wonder if we should activate(?) some of the plugins, including the new toolchain plugins, in the `<plugins>` (not just `<pluginManagement>`) section of the parent `pom.xml`. Might that save us from having to do so in each separate `pom.xml`? (We might actually mostly get away with activating(?) them only in the main `guava` build: That _downloads and registers_ the toolchains, and then at least the other projects' _per-plugin_ toolchain configuration probably finds them? But for the more general configuration to work, I think we at least need to activate(?) `maven-toolchains-plugin` in each? I haven't experimented a ton with this.)
- I forgot the other thing while I was typing :(

(See also [these notes](#5457 (comment)).)

So

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655592201
copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation (also, for any other [affected plugins](https://maven.apache.org/guides/mini/guide-using-toolchains.html#prerequisites)) to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet. There are probably other simplifications that we could perform, as well, such as `maven-javadoc-plugin.additionalJOptions`.
   - Originally, I'd set up this CL to explicitly set only the toolchain of `maven-compiler-plugin` to 22. I had it using 11 for any other plugins (just Animal Sniffer, maybe?), I think from when I was trying to get toolchains to take effect at all. I've since changed this CL to set the _default_ toolchain to 22 while still including overrides for `maven-javadoc-plugin` and `maven-surefire-plugin`.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to change the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592. I first tried going with the default from the JDK whose Javadoc binary we're using, which (again) will now be 11. But that led to a problem in `org.codehaus.plexus.languages.java.jpms.CmdModuleNameExtractor`, which apparently tries to look up the module name for the `-source 11` run but uses the Maven run's JDK instead of the Javadoc toolchain or Maven toolchain. So now I've set it to 8 to match what we use for `maven-compiler-plugin`. (I _thought_ I had remembered that `maven-javadoc-plugin` defaulted to matching `maven-compiler-plugin`, even though that's weird. Maybe the two actually just read from the same Maven property or something, or maybe the behavior changed.)

Some other thing I'm wondering:

- I wonder if we should activate(?) some of the plugins, including the new toolchain plugins, in the `<plugins>` (not just `<pluginManagement>`) section of the parent `pom.xml`. Might that save us from having to do so in each separate `pom.xml`? (We might actually mostly get away with activating(?) them only in the main `guava` build: That _downloads and registers_ the toolchains, and then at least the other projects' _per-plugin_ toolchain configuration probably finds them? But for the more general configuration to work, I think we at least need to activate(?) `maven-toolchains-plugin` in each? I haven't experimented a ton with this.)
- I forgot the other thing while I was typing :(

(See also [these notes](#5457 (comment)).)

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655592201
copybara-service bot pushed a commit that referenced this issue Jul 24, 2024
In particular:

- Use JDK 22 for compilation (also, for any other [affected plugins](https://maven.apache.org/guides/mini/guide-using-toolchains.html#prerequisites)) to [avoid a JDK 11 bug](#7331).
   - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](#3990). However, that could complicate [using newer APIs conditionally](#6549). And of course we'd expect JDK 8 to be buggier than JDK 22. (In fact, we still have a workaround or two for JDK 8 bugs (with a brand new one coming in cl/655556207), and we could now remove those—assuming that none of our users use JDK 8 to build Guava outside of our Maven build.) JDK 22 also supports new versions of Error Prone, while JDK 8 does not.
   - This change also allows us to simplify our Error Prone configuration, which until now needed different profiles in order to support both JDK 8 and JDK 9+. We could now upgrade Error Prone, but I haven't done so yet. There are probably other simplifications that we could perform, as well, such as `maven-javadoc-plugin.additionalJOptions`.
   - Originally, I'd set up this CL to explicitly set only the toolchain of `maven-compiler-plugin` to 22. I had it using 11 for any other plugins (just Animal Sniffer, maybe?), I think from when I was trying to get toolchains to take effect at all. I've since changed this CL to set the _default_ toolchain to 22 while still including overrides for `maven-javadoc-plugin` and `maven-surefire-plugin`.
- Continue to use JDK 11 for Javadoc, as [we're doing now](https://github.com/google/guava/blob/5041fbe61965a73ea269c7c24ea746d89bd1b1ba/.github/workflows/ci.yml#L89-L99) because of [problems with at least JDK 21](#7109).
   - What matters might actually be the version used [by _JDiff_](#6549 (comment)), which comes from the version in the linked `ci.yml` file. But since we're using JDK 11 currently for docs in general, I'm sticking with that for now. Still, we should consider [upgrading the version used for Javadoc generation](#6790 (comment)). But this CL is already complicated enough....
   - When we hard-code JDK 11, we need to change the `<source>${java.specification.version}</source>` line: That would otherwise set (for example) `-source 17` when running Maven under JDK 17, and JDK 11 wouldn't recognize it. As I recall, the `java.specification.version` usage was from the days in which we tried to inherit Javadoc from the JDK. Inheritance had [stopped working](#6790), and we ripped it out in cl/614693592. I first tried going with the default from the JDK whose Javadoc binary we're using, which (again) will now be 11. But that led to a problem in `org.codehaus.plexus.languages.java.jpms.CmdModuleNameExtractor`, which apparently tries to look up the module name for the `-source 11` run but uses the Maven run's JDK instead of the Javadoc toolchain or Maven toolchain. So now I've set it to 8 to match what we use for `maven-compiler-plugin`. (I _thought_ I had remembered that `maven-javadoc-plugin` defaulted to matching `maven-compiler-plugin`, even though that's weird. Maybe the two actually just read from the same Maven property or something, or maybe the behavior changed.)

Some other thing I'm wondering:

- I wonder if we should activate(?) some of the plugins, including the new toolchain plugins, in the `<plugins>` (not just `<pluginManagement>`) section of the parent `pom.xml`. Might that save us from having to do so in each separate `pom.xml`? (We might actually mostly get away with activating(?) them only in the main `guava` build: That _downloads and registers_ the toolchains, and then at least the other projects' _per-plugin_ toolchain configuration probably finds them? But for the more general configuration to work, I think we at least need to activate(?) `maven-toolchains-plugin` in each? I haven't experimented a ton with this.)
- I forgot the other thing while I was typing :(

(See also [these notes](#5457 (comment)).)

Fixes #7331

RELNOTES=n/a
PiperOrigin-RevId: 655647768
copybara-service bot pushed a commit to google/truth that referenced this issue Nov 14, 2024
I had hoped that maybe this would help with the error that I saw when building with the Javadoc from JDK 21:

```
[ERROR] error: Unexpected redirection for URL https://docs.oracle.com/javase/7/docs/api/element-list to https://docs.oracle.com/javase/7/docs/api/
```

It turns out not to help with that.

However, the change does lead us to produce actual links to JDK types in e.g., http://truth.dev/StringSubject. (This sounds like it's _different_ from what I previously saw in Guava, where [_generating Javadoc with JDK 21_ (while _not_ updating the version we _link_ to)](google/guava#6790 (comment)) _also_ fixed links. I guess I should consider updating Guava to link to Java 21, too.)

Our old behavior of linking to Java 7 arguably made some sense when that was the minimum version that we supported. But we've [required Java 8 for a while now](https://github.com/google/truth/releases/tag/v1.1.4). And really, _some_ even newer version seems likely to be useful to most users, who have largely upgraded. And given that Google search is already not great at finding Javadoc at all (let alone new versions), maybe it's good for us to get into the practice of linking to newer versions to see if that helps improve their rankings??

RELNOTES=n/a
PiperOrigin-RevId: 696624054
copybara-service bot pushed a commit to google/truth that referenced this issue Nov 14, 2024
I had hoped that maybe this would help with the error that I saw when building with the Javadoc from JDK 21:

```
[ERROR] error: Unexpected redirection for URL https://docs.oracle.com/javase/7/docs/api/element-list to https://docs.oracle.com/javase/7/docs/api/
```

It turns out not to help with that.

However, the change does lead us to produce actual links to JDK types in e.g., http://truth.dev/StringSubject. (This sounds like it's _different_ from what I previously saw in Guava, where [_generating Javadoc with JDK 21_ (while _not_ updating the version we _link_ to)](google/guava#6790 (comment)) _also_ fixed links. I guess I should consider updating Guava to link to Java 21, too.)

Our old behavior of linking to Java 7 arguably made some sense when that was the minimum version that we supported. But we've [required Java 8 for a while now](https://github.com/google/truth/releases/tag/v1.1.4). And really, _some_ even newer version seems likely to be useful to most users, who have largely upgraded. And given that Google search is already not great at finding Javadoc at all (let alone new versions), maybe it's good for us to get into the practice of linking to newer versions to see if that helps improve their rankings??

RELNOTES=n/a
PiperOrigin-RevId: 696632561
copybara-service bot pushed a commit that referenced this issue Nov 15, 2024
(Compare cl/696632561 for Truth. I verified that the Truth change worked in post-submit CI, and (as you'll see) I did some work to verify that this Guava change works in local testing.)

This CL addresses the "linking" part of #6790 but not the "inheriting" part.

I was having trouble getting this to work with the JDK in the `<linkoffline>` section, so I moved it to the `<link>` section. (But not before first accidentally creating a _second_ `<link>` section, which Maven seems to have chosen to ignore without error. See also my other complaints in a comment in the actual `pom.xml`.) Fortunately, we don't need to use `<linkoffline>` for the JDK anymore because we no longer need to have the JDK in the same section as JSR-305 now that `javax.annotation` isn't part of the JDK anymore (and thus there's no longer a conflict about whether the JDK or JSR-305 gets to "claim" it and thus there's no need to resolve it through ordering, which we can control only within sections, not across sections).

Somewhere in the course of working through that, I upgraded `maven-javadoc-plugin` to 3.11.1. That may or may not have turned out to be necessary, but it's probably more likely to help than to hurt.

RELNOTES=n/a
PiperOrigin-RevId: 696636564
copybara-service bot pushed a commit that referenced this issue Nov 15, 2024
(Compare cl/696632561 for Truth. I verified that the Truth change worked in post-submit CI, and (as you'll see) I did some work to verify that this Guava change works in local testing.)

This CL addresses the "linking" part of #6790 but not the "inheriting" part.

I was having trouble getting this to work with the JDK in the `<linkoffline>` section, so I moved it to the `<link>` section. (But not before first accidentally creating a _second_ `<link>` section, which Maven seems to have chosen to ignore without error. See also my other complaints in a comment in the actual `pom.xml`.) Fortunately, we don't need to use `<linkoffline>` for the JDK anymore because we no longer need to have the JDK in the same section as JSR-305 now that `javax.annotation` isn't part of the JDK anymore (and thus there's no longer a conflict about whether the JDK or JSR-305 gets to "claim" it and thus there's no need to resolve it through ordering, which we can control only within sections, not across sections).

Somewhere in the course of working through that, I upgraded `maven-javadoc-plugin` to 3.11.1. That may or may not have turned out to be necessary, but it's probably more likely to help than to hurt.

RELNOTES=n/a
PiperOrigin-RevId: 696975523
@cpovirk cpovirk changed the title Inheriting Javadoc from the JDK _and_ linking to the JDK broke at some point Inheriting Javadoc from the JDK is broken (as was _linking to_ the JDK) Nov 15, 2024
@cpovirk
Copy link
Member Author

cpovirk commented Nov 20, 2024

I tried using the Java 21 version of Javadoc again after #7516, and it looks like that still works OK (but presumably also still fails if we use it for JDiff). But I do see an error from Javadoc itself if I go all the way to Java 23, mirroring our approach of running the newest javac:

[ERROR] error: Unexpected redirection for URL https://docs.oracle.com/javase/21/docs/api/element-list to https://docs.oracle.com/en/java/javase/21/docs/api/index.html

Changing our configuration to link to Java 23 doesn't help:

[ERROR] error: Unexpected redirection for URL https://docs.oracle.com/javase/23/docs/api/element-list to https://docs.oracle.com/en/java/javase/23/docs/api/index.html

So we probably wouldn't go past Java 21 for Javadoc yet.

(There are also some warnings, which might already arise with Java 11's Javadoc but which (if so) are hidden by repeated warnings: "[WARNING] javadoc: warning - The code being documented uses packages in the unnamed module, but the packages defined in https://docs.oracle.com/en/java/javase/21/docs/api/ are in named modules.")

[edit: As already discussed somewhere or other, using jdk-based activation is the wrong approach for setting (e.g.) Javadoc flags now that we use toolchains. We'd likely have to modify that if we were to change the version we use and/or the JDK that our doc-generating CI job uses to start Maven.]

@cpovirk
Copy link
Member Author

cpovirk commented Nov 20, 2024

The inability to inherit also leads to these warnings (if we dial up Javadoc warnings enough):

guava/src/com/google/common/util/concurrent/AbstractFuture.java:433: warning: overridden methods do not document exception type java.util.concurrent.CancellationException (package java.util.concurrent class CancellationException)
   * @throws CancellationException {@inheritDoc}
     ^
guava/src/com/google/common/util/concurrent/AbstractFuture.java:547: warning: overridden methods do not document exception type java.util.concurrent.CancellationException (package java.util.concurrent class CancellationException)
   * @throws CancellationException {@inheritDoc}
     ^

I wondered if we could avoid that by omitting the @throws clauses entirely, but it turns out that we can't: I confirmed with a self-contained project that unchecked exceptions appear in the subclass Javadoc only if we use @throws there.

[edit: similarly in ListeningExecutorService with RejectedExecutionException]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant