From 2509793a308ca1d349dd519888e46ce5ed3881fc Mon Sep 17 00:00:00 2001 From: cpovirk Date: Wed, 24 Jul 2024 09:19:24 -0700 Subject: [PATCH] Hard-code Java versions for plugins other than `maven-surefire-plugin`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In particular: - Use JDK 22 for compilation to [avoid a JDK 11 bug](https://github.com/google/guava/issues/7331). - Another way to avoid that bug would be to use JDK 8, which [would also provide a `--release`-like compatibility guarantee](https://github.com/google/guava/issues/3990). However, that could complicate [using newer APIs conditionally](https://github.com/google/guava/issues/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](https://github.com/google/guava/pull/7109). - What matters might actually be the version used [by _JDiff_](https://github.com/google/guava/issues/6549#issuecomment-1761654083), 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](https://github.com/google/guava/issues/6790#issuecomment-2072445343). But this CL is already complicated enough.... - When we hard-code JDK 11, we need to remove the `${java.specification.version}` 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](https://github.com/google/guava/issues/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 `` (not just ``) 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](https://github.com/google/guava/issues/5457#issuecomment-2248203420).) So Fixes https://github.com/google/guava/issues/7331 RELNOTES=n/a PiperOrigin-RevId: 655592201 --- android/guava-testlib/pom.xml | 7 + android/guava-tests/pom.xml | 7 + .../collect/WriteReplaceOverridesTest.java | 8 + android/guava/pom.xml | 10 +- android/pom.xml | 200 ++++++++++-------- guava-gwt/pom.xml | 7 + guava-testlib/pom.xml | 7 + guava-tests/pom.xml | 7 + .../collect/WriteReplaceOverridesTest.java | 8 + guava/pom.xml | 10 +- pom.xml | 200 ++++++++++-------- 11 files changed, 283 insertions(+), 188 deletions(-) diff --git a/android/guava-testlib/pom.xml b/android/guava-testlib/pom.xml index 33dc15799d00..1963a08fcc65 100644 --- a/android/guava-testlib/pom.xml +++ b/android/guava-testlib/pom.xml @@ -60,6 +60,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-compiler-plugin diff --git a/android/guava-tests/pom.xml b/android/guava-tests/pom.xml index bf9bec45c651..12cf5de79bda 100644 --- a/android/guava-tests/pom.xml +++ b/android/guava-tests/pom.xml @@ -66,6 +66,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-compiler-plugin diff --git a/android/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java b/android/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java index bf10f5f75d14..d19299220ed4 100644 --- a/android/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java +++ b/android/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java @@ -78,6 +78,14 @@ public void testClassesHaveOverrides() throws Exception { * well be a JDK bug. */ || info.getName().contains("TypeTokenTest") + /* + * "IllegalAccess tried to access class + * com.google.common.collect.testing.AbstractIteratorTester from class + * com.google.common.collect.MultimapsTest" + * + * ...when we build with JDK 22 and run under JDK 8. + */ + || info.getName().contains("MultimapsTest") /* * Luckily, we don't care about analyzing tests at all. We'd skip them all if we could do so * trivially, but it's enough to skip these ones. diff --git a/android/guava/pom.xml b/android/guava/pom.xml index 501b3967264c..d69898787a66 100644 --- a/android/guava/pom.xml +++ b/android/guava/pom.xml @@ -57,6 +57,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-jar-plugin @@ -98,9 +105,6 @@ - - maven-compiler-plugin - maven-source-plugin diff --git a/android/pom.xml b/android/pom.xml index 954ae146a293..8ce6efb7ccc9 100644 --- a/android/pom.xml +++ b/android/pom.xml @@ -12,6 +12,12 @@ Parent for guava artifacts https://github.com/google/guava + + ${java.specification.version} %regex[.*.class] 1.4.4 @@ -19,7 +25,6 @@ 3.43.0 2.28.0 3.0.0 - 9+181-r4173-1 2024-01-02T00:00:00Z @@ -122,7 +127,7 @@ maven-compiler-plugin - 3.8.1 + 3.13.0 1.8 1.8 @@ -139,7 +144,32 @@ doesnotexist -XDcompilePolicy=simple - + + + + -Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR + + + -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED @@ -157,6 +187,70 @@ true + + org.mvnsearch + toolchains-maven-plugin + 4.5.0 + + + + download-11 + + toolchain + + + + + 11 + zulu + + + + + + download-22-and-surefire-version + + toolchain + + + + + 22 + zulu + + + ${surefire.toolchain.version} + zulu + + + + + + + + maven-toolchains-plugin + 3.2.0 + + + + toolchain + + + + + + + 22 + zulu + + + + maven-jar-plugin 3.2.0 @@ -176,7 +270,7 @@ org.codehaus.plexus plexus-io - + 3.4.1 @@ -219,8 +313,13 @@ maven-javadoc-plugin - 3.5.0 + 3.8.0 + + + 11 + zulu + true true UTF-8 @@ -231,7 +330,6 @@ -Xdoclint:-html true - ${java.specification.version} ${maven-javadoc-plugin.additionalJOptions} @@ -251,8 +349,12 @@ maven-surefire-plugin - 2.7.2 + 3.3.1 + + ${surefire.toolchain.version} + zulu + ${test.include} @@ -394,90 +496,6 @@ - - javac9-for-jdk8 - - 1.8 - - - - - maven-compiler-plugin - - - - -J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar - - - - - - - - run-error-prone - - - [11,12),[16,) - - - - - maven-compiler-plugin - - - - - -Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR - - - -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED - -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED - -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED - - - - - - javac-for-jvm18plus diff --git a/guava-gwt/pom.xml b/guava-gwt/pom.xml index 00c6421009db..afe4fa0bc461 100644 --- a/guava-gwt/pom.xml +++ b/guava-gwt/pom.xml @@ -129,6 +129,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-compiler-plugin diff --git a/guava-testlib/pom.xml b/guava-testlib/pom.xml index f60890e78e7a..6a696351e4c6 100644 --- a/guava-testlib/pom.xml +++ b/guava-testlib/pom.xml @@ -60,6 +60,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-compiler-plugin diff --git a/guava-tests/pom.xml b/guava-tests/pom.xml index 8596221d8316..f05a079b2f94 100644 --- a/guava-tests/pom.xml +++ b/guava-tests/pom.xml @@ -72,6 +72,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-compiler-plugin diff --git a/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java b/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java index bf10f5f75d14..d19299220ed4 100644 --- a/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java +++ b/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java @@ -78,6 +78,14 @@ public void testClassesHaveOverrides() throws Exception { * well be a JDK bug. */ || info.getName().contains("TypeTokenTest") + /* + * "IllegalAccess tried to access class + * com.google.common.collect.testing.AbstractIteratorTester from class + * com.google.common.collect.MultimapsTest" + * + * ...when we build with JDK 22 and run under JDK 8. + */ + || info.getName().contains("MultimapsTest") /* * Luckily, we don't care about analyzing tests at all. We'd skip them all if we could do so * trivially, but it's enough to skip these ones. diff --git a/guava/pom.xml b/guava/pom.xml index be0bab174e16..618cf9e4ddc3 100644 --- a/guava/pom.xml +++ b/guava/pom.xml @@ -57,6 +57,13 @@ + + org.mvnsearch + toolchains-maven-plugin + + + maven-toolchains-plugin + maven-jar-plugin @@ -98,9 +105,6 @@ - - maven-compiler-plugin - maven-source-plugin diff --git a/pom.xml b/pom.xml index 7e634f7a644e..9513f86074bb 100644 --- a/pom.xml +++ b/pom.xml @@ -12,6 +12,12 @@ Parent for guava artifacts https://github.com/google/guava + + ${java.specification.version} %regex[.*.class] 1.4.4 @@ -19,7 +25,6 @@ 3.43.0 2.28.0 3.0.0 - 9+181-r4173-1 2024-01-02T00:00:00Z @@ -123,7 +128,7 @@ maven-compiler-plugin - 3.8.1 + 3.13.0 1.8 1.8 @@ -140,7 +145,32 @@ doesnotexist -XDcompilePolicy=simple - + + + + -Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR + + + -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED + -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED + -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED @@ -158,6 +188,70 @@ true + + org.mvnsearch + toolchains-maven-plugin + 4.5.0 + + + + download-11 + + toolchain + + + + + 11 + zulu + + + + + + download-22-and-surefire-version + + toolchain + + + + + 22 + zulu + + + ${surefire.toolchain.version} + zulu + + + + + + + + maven-toolchains-plugin + 3.2.0 + + + + toolchain + + + + + + + 22 + zulu + + + + maven-jar-plugin 3.2.0 @@ -177,7 +271,7 @@ org.codehaus.plexus plexus-io - + 3.4.1 @@ -214,8 +308,13 @@ maven-javadoc-plugin - 3.5.0 + 3.8.0 + + + 11 + zulu + true true UTF-8 @@ -226,7 +325,6 @@ -Xdoclint:-html true - ${java.specification.version} ${maven-javadoc-plugin.additionalJOptions} @@ -246,8 +344,12 @@ maven-surefire-plugin - 2.7.2 + 3.3.1 + + ${surefire.toolchain.version} + zulu + ${test.include} @@ -389,90 +491,6 @@ - - javac9-for-jdk8 - - 1.8 - - - - - maven-compiler-plugin - - - - -J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar - - - - - - - - run-error-prone - - - [11,12),[16,) - - - - - maven-compiler-plugin - - - - - -Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR - - - -J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED - -J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED - -J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED - -J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED - - - - - - javac-for-jvm18plus