Skip to content

Commit c74128b

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Hard-code Java versions for plugins other than maven-surefire-plugin.
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
1 parent 5041fbe commit c74128b

File tree

6 files changed

+198
-184
lines changed

6 files changed

+198
-184
lines changed

android/guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ public void testClassesHaveOverrides() throws Exception {
7878
* well be a JDK bug.
7979
*/
8080
|| info.getName().contains("TypeTokenTest")
81+
/*
82+
* "IllegalAccess tried to access class
83+
* com.google.common.collect.testing.AbstractIteratorTester from class
84+
* com.google.common.collect.MultimapsTest"
85+
*
86+
* ...when we build with JDK 22 and run under JDK 8.
87+
*/
88+
|| info.getName().contains("MultimapsTest")
8189
/*
8290
* Luckily, we don't care about analyzing tests at all. We'd skip them all if we could do so
8391
* trivially, but it's enough to skip these ones.

android/guava/pom.xml

+5-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,11 @@
9999
</configuration>
100100
</plugin>
101101
<plugin>
102-
<artifactId>maven-compiler-plugin</artifactId>
102+
<groupId>org.mvnsearch</groupId>
103+
<artifactId>toolchains-maven-plugin</artifactId>
104+
</plugin>
105+
<plugin>
106+
<artifactId>maven-toolchains-plugin</artifactId>
103107
</plugin>
104108
<plugin>
105109
<artifactId>maven-source-plugin</artifactId>

android/pom.xml

+86-91
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,19 @@
1212
<description>Parent for guava artifacts</description>
1313
<url>https://github.com/google/guava</url>
1414
<properties>
15+
<!--
16+
We could override this to change which version we run tests under.
17+
However, I think that our -Djava.security.manager=allow profile is based on the *Maven* JDK.
18+
If we want to use overrides here, we should change that profile to also be based on surefire.toolchain.version.
19+
-->
20+
<surefire.toolchain.version>${java.specification.version}</surefire.toolchain.version>
1521
<!-- Override this with -Dtest.include="**/SomeTest.java" on the CLI -->
1622
<test.include>%regex[.*.class]</test.include>
1723
<truth.version>1.4.4</truth.version>
1824
<jsr305.version>3.0.2</jsr305.version>
1925
<checker.version>3.43.0</checker.version>
2026
<errorprone.version>2.28.0</errorprone.version>
2127
<j2objc.version>3.0.0</j2objc.version>
22-
<javac.version>9+181-r4173-1</javac.version>
2328
<!-- Empty for all JDKs but 9-12 -->
2429
<maven-javadoc-plugin.additionalJOptions></maven-javadoc-plugin.additionalJOptions>
2530
<project.build.outputTimestamp>2024-01-02T00:00:00Z</project.build.outputTimestamp>
@@ -122,8 +127,11 @@
122127
<plugins>
123128
<plugin>
124129
<artifactId>maven-compiler-plugin</artifactId>
125-
<version>3.8.1</version>
130+
<version>3.13.0</version>
126131
<configuration>
132+
<jdkToolchain>
133+
<version>22</version>
134+
</jdkToolchain>
127135
<source>1.8</source>
128136
<target>1.8</target>
129137
<encoding>UTF-8</encoding>
@@ -139,7 +147,32 @@
139147
<arg>doesnotexist</arg>
140148
<!-- https://errorprone.info/docs/installation#maven -->
141149
<arg>-XDcompilePolicy=simple</arg>
142-
<!-- -Xplugin:ErrorProne is set conditionally by a profile. -->
150+
151+
<!-- https://errorprone.info/docs/installation#maven -->
152+
<!-- TODO(cpovirk): Enable NullArgumentForNonNullParameter for
153+
prod code. It's disabled automatically for "test code"
154+
(which is good: our tests have intentional violations), but
155+
Error Prone doesn't know it's building test code unless we
156+
pass -XepCompilingTestOnlyCode, and that argument needs to
157+
be passed as part of the same <arg> as -Xplugin:ErrorProne,
158+
and I gave up trying to figure out how to do that for test
159+
compilation only. -->
160+
<arg>-Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR</arg>
161+
<!-- https://github.com/google/error-prone/blob/f8e33bc460be82ab22256a7ef8b979d7a2cacaba/docs/installation.md#jdk-16 -->
162+
<!-- TODO(cpovirk): Use .mvn/jvm.config instead (per
163+
https://errorprone.info/docs/installation#maven) if it can
164+
be made not to interfere with JDK8 or if we stop building
165+
with JDK8. -->
166+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
167+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
168+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
169+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
170+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
171+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
172+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
173+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
174+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
175+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
143176
</compilerArgs>
144177
<annotationProcessorPaths>
145178
<path>
@@ -157,6 +190,50 @@
157190
<fork>true</fork>
158191
</configuration>
159192
</plugin>
193+
<plugin>
194+
<groupId>org.mvnsearch</groupId>
195+
<artifactId>toolchains-maven-plugin</artifactId>
196+
<version>4.5.0</version>
197+
<executions>
198+
<execution>
199+
<goals>
200+
<goal>toolchain</goal>
201+
</goals>
202+
</execution>
203+
</executions>
204+
<configuration>
205+
<toolchains>
206+
<jdk>
207+
<version>11</version>
208+
</jdk>
209+
<jdk>
210+
<version>22</version>
211+
</jdk>
212+
<testJdk>
213+
<version>${surefire.toolchain.version}</version>
214+
</testJdk>
215+
</toolchains>
216+
</configuration>
217+
</plugin>
218+
<plugin>
219+
<artifactId>maven-toolchains-plugin</artifactId>
220+
<version>3.2.0</version>
221+
<executions>
222+
<execution>
223+
<goals>
224+
<goal>toolchain</goal>
225+
</goals>
226+
</execution>
227+
</executions>
228+
<configuration>
229+
<toolchains>
230+
<jdk>
231+
<!-- TODO: b/286965322 - Use a newer version if it doesn't break Javadoc or whatever (including our run of JDiff, which we do outside Maven, during snapshots/releases). -->
232+
<version>11</version>
233+
</jdk>
234+
</toolchains>
235+
</configuration>
236+
</plugin>
160237
<plugin>
161238
<artifactId>maven-jar-plugin</artifactId>
162239
<version>3.2.0</version>
@@ -176,7 +253,7 @@
176253
<dependency>
177254
<groupId>org.codehaus.plexus</groupId>
178255
<artifactId>plexus-io</artifactId>
179-
<!-- DO NOT UPGRADE this past 3.4.1 until https://github.com/codehaus-plexus/plexus-io/issues/109 is fixed. -->
256+
<!-- 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). -->
180257
<version>3.4.1</version>
181258
</dependency>
182259
</dependencies>
@@ -219,7 +296,7 @@
219296
</plugin>
220297
<plugin>
221298
<artifactId>maven-javadoc-plugin</artifactId>
222-
<version>3.5.0</version>
299+
<version>3.8.0</version>
223300
<configuration>
224301
<quiet>true</quiet>
225302
<notimestamp>true</notimestamp>
@@ -231,7 +308,6 @@
231308
<additionalOption>-Xdoclint:-html</additionalOption>
232309
</additionalOptions>
233310
<linksource>true</linksource>
234-
<source>${java.specification.version}</source>
235311
<additionalJOption>${maven-javadoc-plugin.additionalJOptions}</additionalJOption>
236312
</configuration>
237313
<executions>
@@ -251,8 +327,11 @@
251327
</plugin>
252328
<plugin>
253329
<artifactId>maven-surefire-plugin</artifactId>
254-
<version>2.7.2</version>
330+
<version>3.3.1</version>
255331
<configuration>
332+
<jdkToolchain>
333+
<version>${surefire.toolchain.version}</version>
334+
</jdkToolchain>
256335
<includes>
257336
<include>${test.include}</include>
258337
</includes>
@@ -394,90 +473,6 @@
394473
</test.add.opens>
395474
</properties>
396475
</profile>
397-
<profile>
398-
<id>javac9-for-jdk8</id>
399-
<activation>
400-
<jdk>1.8</jdk>
401-
</activation>
402-
<build>
403-
<plugins>
404-
<plugin>
405-
<artifactId>maven-compiler-plugin</artifactId>
406-
<configuration>
407-
<!-- Under JDK8, we continue to use errorprone's javac9 (even
408-
though we don't run Error Prone itself, which no longer
409-
supports JDK8!).
410-
411-
Why? At some point, presumably after
412-
https://github.com/google/guava/commit/e06a8cec65815599e510d7f9c1ea9d2a8eaa438a,
413-
builds with JDK8 began failing animal-sniffer with the error:
414-
415-
Failed to check signatures: Bad class file .../CollectionFuture$ListFuture.class
416-
417-
One way of dealing with that would be to disable
418-
animal-sniffer. And that would be fine for our -jre builds:
419-
If we're building with JDK8, then clearly we're sticking to
420-
JDK8 APIs. However, I assume (but did not confirm) that we'd
421-
have the same issue with our -android builds, which need
422-
animal-sniffer so that they can check that we're sticking to
423-
JDK6-like APIs.
424-
425-
So instead, we use javac9, which doesn't lead to this error.
426-
-->
427-
<compilerArgs combine.children="append">
428-
<arg>-J-Xbootclasspath/p:${settings.localRepository}/com/google/errorprone/javac/${javac.version}/javac-${javac.version}.jar</arg>
429-
</compilerArgs>
430-
</configuration>
431-
</plugin>
432-
</plugins>
433-
</build>
434-
</profile>
435-
<profile>
436-
<id>run-error-prone</id>
437-
<activation>
438-
<!--
439-
Error Prone requires 11+: https://errorprone.info/docs/installation
440-
We skip 12-15 because of https://github.com/google/error-prone/issues/3540.
441-
-->
442-
<jdk>[11,12),[16,)</jdk>
443-
</activation>
444-
<build>
445-
<plugins>
446-
<plugin>
447-
<artifactId>maven-compiler-plugin</artifactId>
448-
<configuration>
449-
<compilerArgs combine.children="append">
450-
<!-- https://errorprone.info/docs/installation#maven -->
451-
<!-- TODO(cpovirk): Enable NullArgumentForNonNullParameter for
452-
prod code. It's disabled automatically for "test code"
453-
(which is good: our tests have intentional violations), but
454-
Error Prone doesn't know it's building test code unless we
455-
pass -XepCompilingTestOnlyCode, and that argument needs to
456-
be passed as part of the same <arg> as -Xplugin:ErrorProne,
457-
and I gave up trying to figure out how to do that for test
458-
compilation only. -->
459-
<arg>-Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR</arg>
460-
<!-- https://github.com/google/error-prone/blob/f8e33bc460be82ab22256a7ef8b979d7a2cacaba/docs/installation.md#jdk-16 -->
461-
<!-- TODO(cpovirk): Use .mvn/jvm.config instead (per
462-
https://errorprone.info/docs/installation#maven) if it can
463-
be made not to interfere with JDK8 or if we stop building
464-
with JDK8. -->
465-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
466-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
467-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
468-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
469-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
470-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
471-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
472-
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
473-
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
474-
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
475-
</compilerArgs>
476-
</configuration>
477-
</plugin>
478-
</plugins>
479-
</build>
480-
</profile>
481476
<profile>
482477
<id>javac-for-jvm18plus</id>
483478
<activation>

guava-tests/test/com/google/common/collect/WriteReplaceOverridesTest.java

+8
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ public void testClassesHaveOverrides() throws Exception {
7878
* well be a JDK bug.
7979
*/
8080
|| info.getName().contains("TypeTokenTest")
81+
/*
82+
* "IllegalAccess tried to access class
83+
* com.google.common.collect.testing.AbstractIteratorTester from class
84+
* com.google.common.collect.MultimapsTest"
85+
*
86+
* ...when we build with JDK 22 and run under JDK 8.
87+
*/
88+
|| info.getName().contains("MultimapsTest")
8189
/*
8290
* Luckily, we don't care about analyzing tests at all. We'd skip them all if we could do so
8391
* trivially, but it's enough to skip these ones.

guava/pom.xml

+5-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,11 @@
9999
</configuration>
100100
</plugin>
101101
<plugin>
102-
<artifactId>maven-compiler-plugin</artifactId>
102+
<groupId>org.mvnsearch</groupId>
103+
<artifactId>toolchains-maven-plugin</artifactId>
104+
</plugin>
105+
<plugin>
106+
<artifactId>maven-toolchains-plugin</artifactId>
103107
</plugin>
104108
<plugin>
105109
<artifactId>maven-source-plugin</artifactId>

0 commit comments

Comments
 (0)