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

Broken link in Javadoc #5457

Closed
cowwoc opened this issue Mar 31, 2021 · 15 comments
Closed

Broken link in Javadoc #5457

cowwoc opened this issue Mar 31, 2021 · 15 comments
Assignees
Labels
P2 package=general type=defect Bug, not working as expected

Comments

@cowwoc
Copy link

cowwoc commented Mar 31, 2021

  1. Go to https://guava.dev/releases/30.1.1-jre/api/docs/
  2. Search for "unsignedbytes"
  3. Get redirected to https://guava.dev/releases/30.1.1-jre/api/docs/undefined/com/google/common/primitives/UnsignedBytes.html which returns HTTP 404.
@cpovirk
Copy link
Member

cpovirk commented Mar 31, 2021

Thanks. We'll need to do the same thing as google/truth#670 (which might also be what we did for google/error-prone#1825, but I haven't looked).

@cpovirk cpovirk self-assigned this Mar 31, 2021
@cpovirk cpovirk added P2 package=general type=defect Bug, not working as expected labels Mar 31, 2021
@cpovirk
Copy link
Member

cpovirk commented Mar 31, 2021

(As noted on the Truth issue, you might like an alternative that we added, which is to navigate to guava.dev/unsignedbytes. But we'll fix search, too.)

@cowwoc
Copy link
Author

cowwoc commented Mar 31, 2021

@cpovirk I looked at the issue you linked to but it sounds like everyone is implementing a workaround instead of fixing the underlying problem. Did anyone file a bug report against the Javadoc tool?

@cpovirk
Copy link
Member

cpovirk commented Mar 31, 2021

It looks like it: JDK-8215291.

@cowwoc
Copy link
Author

cowwoc commented Mar 31, 2021

@cpovirk Good find. I'll add it to the Stackoverflow discussion.

@ddadon10
Copy link

Hi, any news about this issue? I use Guava's Javadoc often and this bug is very annoying.

Thanks :)

@sumitsawant
Copy link

@cowwoc Can I take a look ?

@cowwoc
Copy link
Author

cowwoc commented Oct 13, 2021

@sumitsawant Yes, go ahead.

@sumitsawant
Copy link

Hey @cowwoc / @cpovirk Can anyone assign this to me ?

@cowwoc
Copy link
Author

cowwoc commented Oct 13, 2021

I don't have the necessary permission. I'll let @cpovirk do it.

@cpovirk cpovirk assigned sumitsawant and unassigned cpovirk Oct 19, 2021
@Marcono1234
Copy link
Contributor

An alternative solution might be to use the jdkToolchain property to specify that the Javadoc should be created using a newer JDK version, e.g. by specifying:

<jdkToolchain>
  <version>17</version>
</jdkToolchain>

You would then either have to build with JDK 17, or create a ~/.m2/toolchains.xml file, see Maven Toolchains documentation.

dmitry-timofeev pushed a commit to dmitry-timofeev/guava that referenced this issue Dec 1, 2021
Fixed Javadoc search feature on JDK 11, currently used in our
scripts updating snapshot Javadocs on guava.dev and building
Guava releases, by adding no-module-directories.

Note that this option is not present in javadoc tool from JDK 8
and 13+, making javadoc generation there *not supported*.
Whilst we could solve that with profiles, the extra complexity
is deemed unnecessary given we plan to eventually migrate to 17+,
removing this option (and supported for any JDKs before 17).

Fixes google#5457

TESTED=Locally with `mvn javadoc:javadoc` for `guava` module and its android flavour
dmitry-timofeev pushed a commit to dmitry-timofeev/guava that referenced this issue Dec 3, 2021
Fixed Javadoc search feature on JDK 11, which is currently used in our
scripts updating snapshot Javadocs on guava.dev and building
Guava releases, by adding `no-module-directories` flag.

This option is not present in javadoc tool from JDK 8 and 13+,
hence we use profiles to conditionally pass this flag on 9-12 only.

Note that on JDK 17 javadoc generation does not work,
as it seems to have changed the warning on LVTI usage in sources
to an error.

Fixes google#5457

TESTED=Locally with `mvn javadoc:javadoc` for `guava` module and its android flavour
copybara-service bot pushed a commit that referenced this issue Dec 3, 2021
Fixed Javadoc search feature on JDK 11, which is currently used in our
scripts updating snapshot Javadocs on guava.dev and building
Guava releases, by adding `no-module-directories` flag.

This option is not present in javadoc tool from JDK 8 and 13+,
hence we use profiles to conditionally pass this flag on 9-12 only.

Note that on JDK 17 javadoc generation does not work,
as it seems to have changed the warning on LVTI usage in sources
to an error.

Fixes #5457
Fixes #5800

RELNOTES=n/a
PiperOrigin-RevId: 413922237
copybara-service bot pushed a commit that referenced this issue Dec 3, 2021
Fixed Javadoc search feature on JDK 11, which is currently used in our
scripts updating snapshot Javadocs on guava.dev and building
Guava releases, by adding `no-module-directories` flag.

This option is not present in javadoc tool from JDK 8 and 13+,
hence we use profiles to conditionally pass this flag on 9-12 only.

Note that on JDK 17 javadoc generation does not work,
as it seems to have changed the warning on LVTI usage in sources
to an error.

Fixes #5457
Fixes #5800

RELNOTES=n/a
PiperOrigin-RevId: 413922237
copybara-service bot pushed a commit that referenced this issue Nov 15, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have problems under some versions, at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

Fixes #5801

RELNOTES=n/a
PiperOrigin-RevId: 488700624
copybara-service bot pushed a commit that referenced this issue Nov 15, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have [problems under some versions](google/error-prone#3540), at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

(I also disabled it for JDK10: As noted above, Error Prone [supports JDK11+](https://errorprone.info/docs/installation). And as noted further above, Maven doesn't get far enough with JDK10 to even start running Error Prone.)

Fixes #5801

RELNOTES=n/a
PiperOrigin-RevId: 488700624
copybara-service bot pushed a commit that referenced this issue Nov 15, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have [problems under some versions](google/error-prone#3540), at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

(I also disabled it for JDK10: As noted above, Error Prone [supports JDK11+](https://errorprone.info/docs/installation). And as noted further above, Maven doesn't get far enough with JDK10 to even start running Error Prone.)

Fixes #5801

RELNOTES=n/a
PiperOrigin-RevId: 488700624
copybara-service bot pushed a commit that referenced this issue Nov 16, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have [problems under some versions](google/error-prone#3540), at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

(I also disabled it for JDK10: As noted above, Error Prone [supports JDK11+](https://errorprone.info/docs/installation). And as noted further above, Maven doesn't get far enough with JDK10 to even start running Error Prone.)

Fixes #5801

RELNOTES=n/a
PiperOrigin-RevId: 488700624
copybara-service bot pushed a commit that referenced this issue Nov 16, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have [problems under some versions](google/error-prone#3540), at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

(I also disabled it for JDK10: As noted above, Error Prone [supports JDK11+](https://errorprone.info/docs/installation). And as noted further above, Maven doesn't get far enough with JDK10 to even start running Error Prone.)

Fixes #5801

RELNOTES=n/a
PiperOrigin-RevId: 488700624
copybara-service bot pushed a commit that referenced this issue Nov 16, 2022
(Guava is already _usable_ under plenty of verions. This change affects only people who build it themselves.)

And run CI under JDK17. Maybe this will make CI painfully slow, but we'll see what happens. If we want to drop something, we should consider whether to revert 17 or to drop 11 instead (so as to maintain coverage at the endpoints of \[8, 17\]).

## Notes on some of the versions

### JDK9

I expected Error Prone to work, but I saw `invalid flag: -Xep:NullArgumentForNonNullParameter:OFF`, even though that flag is [already](https://github.com/google/guava/blob/166d8c0d8733d40914fb24f368cb587a92bddfe0/pom.xml#L515) part of [the same `<arg>`](google/error-prone#1086 (comment)), which works fine for other JDK versions. So I disabled Error Prone for that version.

Then I had a Javadoc problem with the `--no-module-directories` configuration from cl/413934851 (the fix for #5457). After reading [JDK-8215582](https://bugs.openjdk.org/browse/JDK-8215582) more carefully, I get the impression that that flag might not have been added until 11: "addressed in JDK 11, along with an option to revert to the old layout in case of need." So I disabled it for 9-10.

Then I ran into a problem similar to bazelbuild/bazel#6173 / [JDK-8184940](https://bugs.openjdk.java.net/browse/JDK-8184940). I'm not sure exactly what tool produced a file with a month of 0, but it happened only when building `guava-tests`. At that point, I gave up, though I left the 2 above workarounds in place.

### JDK10

This fails with some kind of problem finding a Guice dependency inside Maven. I didn't investigate.

### JDK15 and JDK16

These fail with [the `TreeMap` bug](https://bugs.openjdk.org/browse/JDK-8259622) that [our collection testers had detected](#5801 (comment)) but we never got around to reporting. Thankfully, it got reported and [fixed](openjdk/jdk@2c8e337) for JDK17. We could consider suppressing the tests under that version.

### JDK18, JDK19, and JDK20-early-access

These fail with [`SecurityManager` trouble](#5801 (comment)).

## Notes on the other actual changes

### `maven-javadoc-plugin`

I set up `maven-javadoc-plugin` to use `-source ${java.specification.version}`. Otherwise, it would [take the version from `maven-compiler-plugin`](#5801 (comment)). That's typically fine: Guava's source code targets Java 8, so `-source 8` "ought" to work. But it doesn't actually work because we also pass Javadoc the _JDK_ sources (so that `{@inheritdoc}` works better), which naturally can target whichever version of the JDK we're building with.

### Error Prone

While Error Prone is mostly usable [on JDK11+](https://errorprone.info/docs/installation), some of its checks have [problems under some versions](google/error-prone#3540), at least when they're reporting warnings.

This stems from its use of part of the Checker Framework, which [doesn't support JDKs in the gap between 11 and 17](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/framework/src/main/java/org/checkerframework/framework/source/SourceChecker.java#L553-L554). And specifically,  it looks like the Checker Framework is [trying to look up `BindingPatternTree` under any JDK12+](https://github.com/typetools/checker-framework/blob/c2d16b3409000ac2e2ca95b8b81ae11e42195308/javacutil/src/main/java/org/checkerframework/javacutil/TreeUtils.java#L131-L144). But `BindingPatternTree` (besides not being present at all [until JDK14](openjdk/jdk@229e0d1#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R41)) didn't declare that method [until JDK16](openjdk/jdk@18bc95b#diff-3db4b0ce4411c851bcf75d92ef4dadc7351debcf0f9b2c2623dc513923b45867R39).

Anyway, the problem we saw was [a `NoSuchMethodException` during the `AbstractReferenceEquality` call to `NullnessAnalysis.getNullness`](https://oss-fuzz-build-logs.storage.googleapis.com/log-a9d04aa2-8b5a-47ca-8066-7e6b38548064.txt), which uses Checker Framework dataflow.

To address that, I disabled Error Prone for the versions under which I'd expect the `BindingPatternTree` code to be a problem.

(I also disabled it for JDK10: As noted above, Error Prone [supports JDK11+](https://errorprone.info/docs/installation). And as noted further above, Maven doesn't get far enough with JDK10 to even start running Error Prone.)

Fixes #5801

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

cpovirk commented May 10, 2023

RE: Maven Toolchains: google/error-prone#3895 (comment)

[edit: maybe here]

@cpovirk
Copy link
Member

cpovirk commented Jul 24, 2024

I was just messing with toolchains a little, and I found that putting this in maven-compiler-plugin appears to make no difference:

            <jdkToolchain>
              <version>22</version>
            </jdkToolchain>

I do see some effect if I configure maven-toolchains-plugin (as follows in pluginManagement, then listed in plugins itself):

        <plugin>
          <artifactId>maven-toolchains-plugin</artifactId>
          <version>3.2.0</version>
          <executions>
            <execution>
              <goals>
                <goal>toolchain</goal>
              </goals>
            </execution>
          </executions>
          <configuration>
            <toolchains>
              <jdk>
                <version>11</version>
              </jdk>
            </toolchains>
          </configuration>
        </plugin>

But then we have the problem that we have to tell Maven where to find the toolchain.

I'm able to autodownload a couple with:

        <plugin>
          <groupId>org.mvnsearch</groupId>
          <artifactId>toolchains-maven-plugin</artifactId>
          <version>4.5.0</version>
          <executions>
            <execution>
              <goals>
                <goal>toolchain</goal>
              </goals>
            </execution>
          </executions>
          <configuration>
            <toolchains>
              <jdk>
                <version>11</version>
              </jdk>
              <jdk>
                <version>22</version>
              </jdk>
            </toolchains>
          </configuration>
        </plugin>

Then the request for 22 appears to take effect.

The configuration above presumably sets up the tests to run with 11, too, and that's not what we want (well, at least until we both set up the tests to run with 11 and set up the tests to run with 8 and probably other versions). But luckily, it's not working?

[INFO] Toolchain in surefire-plugin: JDK[/usr/local/google/home/cpovirk/.m2/jdks/openjdk-11.0.16_8]
[WARNING] Toolchains are ignored, 'executable' parameter is set to /usr/local/google/home/cpovirk/jdk-17.0.2/bin/java

Oh, but maybe it starts working if I upgrade Surefire to 3.3.1? So I should probably figure this out for real.

@cpovirk
Copy link
Member

cpovirk commented Jul 24, 2024

I'm now this far:

diff --git a/guava/pom.xml b/guava/pom.xml
index be0bab174e..cd2c7412da 100644
--- a/guava/pom.xml
+++ b/guava/pom.xml
@@ -99,7 +99,11 @@
         </configuration>
       </plugin>
       <plugin>
-        <artifactId>maven-compiler-plugin</artifactId>
+        <groupId>org.mvnsearch</groupId>
+        <artifactId>toolchains-maven-plugin</artifactId>
+      </plugin>
+      <plugin>
+        <artifactId>maven-toolchains-plugin</artifactId>
       </plugin>
       <plugin>
         <artifactId>maven-source-plugin</artifactId>
diff --git a/pom.xml b/pom.xml
index 7e634f7a64..ea4bba4af0 100644
--- a/pom.xml
+++ b/pom.xml
@@ -12,6 +12,7 @@
   <description>Parent for guava artifacts</description>
   <url>https://github.com/google/guava</url>
   <properties>
+    <surefire.toolchain.version>${java.specification.version}</surefire.toolchain.version>
     <!-- Override this with -Dtest.include="**/SomeTest.java" on the CLI -->
     <test.include>%regex[.*.class]</test.include>
     <truth.version>1.4.4</truth.version>
@@ -123,8 +124,11 @@
       <plugins>
         <plugin>
           <artifactId>maven-compiler-plugin</artifactId>
-          <version>3.8.1</version>
+          <version>3.13.0</version>
           <configuration>
+            <jdkToolchain>
+              <version>22</version>
+            </jdkToolchain>
             <source>1.8</source>
             <target>1.8</target>
             <encoding>UTF-8</encoding>
@@ -158,6 +162,50 @@
             <fork>true</fork>
           </configuration>
         </plugin>
+        <plugin>
+          <groupId>org.mvnsearch</groupId>
+          <artifactId>toolchains-maven-plugin</artifactId>
+          <version>4.5.0</version>
+          <executions>
+            <execution>
+              <goals>
+                <goal>toolchain</goal>
+              </goals>
+            </execution>
+          </executions>
+          <configuration>
+            <toolchains>
+              <jdk>
+                <version>11</version>
+              </jdk>
+              <jdk>
+                <version>22</version>
+              </jdk>
+              <testJdk>
+                <version>${surefire.toolchain.version}</version>
+              </testJdk>
+            </toolchains>
+          </configuration>
+        </plugin>
+        <plugin>
+          <artifactId>maven-toolchains-plugin</artifactId>
+          <version>3.2.0</version>
+          <executions>
+            <execution>
+              <goals>
+                <goal>toolchain</goal>
+              </goals>
+            </execution>
+          </executions>
+          <configuration>
+            <toolchains>
+              <jdk>
+                <!-- TODO: b/286965322 - Use a newer version if it doesn't break JDiff (or Javadoc or whatever). -->
+                <version>11</version>
+              </jdk>
+            </toolchains>
+          </configuration>
+        </plugin>
         <plugin>
           <artifactId>maven-jar-plugin</artifactId>
           <version>3.2.0</version>
@@ -177,7 +225,7 @@
             <dependency>
               <groupId>org.codehaus.plexus</groupId>
               <artifactId>plexus-io</artifactId>
-              <!-- DO NOT UPGRADE this past 3.4.1 until https://github.com/codehaus-plexus/plexus-io/issues/109 is fixed. -->
+              <!-- DO NOT UPGRADE this past 3.4.1 until https://github.com/codehaus-plexus/plexus-io/issues/109 is fixed (probably in 3.5.1). -->
               <version>3.4.1</version>
             </dependency>
           </dependencies>
@@ -246,8 +294,11 @@
         </plugin>
         <plugin>
           <artifactId>maven-surefire-plugin</artifactId>
-          <version>2.7.2</version>
+          <version>3.3.1</version>
           <configuration>
+            <jdkToolchain>
+              <version>${surefire.toolchain.version}</version>
+            </jdkToolchain>
             <includes>
               <include>${test.include}</include>
             </includes>

But that fails during maven-compiler-plugin (with no error message) when I run with JAVA_HOME pointing to openjdk-8u342. It also fails if I dial back the maven-compiler-plugin toolchain to 21 or even all the way to 12.

We could require a newer version to build....

@cpovirk
Copy link
Member

cpovirk commented Jul 24, 2024

Ah, the failure is probably from the maven-compiler-plugin <profile> we have....

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 package=general type=defect Bug, not working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants