Skip to content

Commit dffe11c

Browse files
cpovirkGoogle Java Core Libraries
cpovirk
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 (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
1 parent 5041fbe commit dffe11c

File tree

13 files changed

+287
-184
lines changed

13 files changed

+287
-184
lines changed

‎android/guava-testlib/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@
6060
</dependencies>
6161
<build>
6262
<plugins>
63+
<plugin>
64+
<groupId>org.mvnsearch</groupId>
65+
<artifactId>toolchains-maven-plugin</artifactId>
66+
</plugin>
67+
<plugin>
68+
<artifactId>maven-toolchains-plugin</artifactId>
69+
</plugin>
6370
<plugin>
6471
<artifactId>maven-compiler-plugin</artifactId>
6572
</plugin>

‎android/guava-tests/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@
6666
</dependencies>
6767
<build>
6868
<plugins>
69+
<plugin>
70+
<groupId>org.mvnsearch</groupId>
71+
<artifactId>toolchains-maven-plugin</artifactId>
72+
</plugin>
73+
<plugin>
74+
<artifactId>maven-toolchains-plugin</artifactId>
75+
</plugin>
6976
<plugin>
7077
<artifactId>maven-compiler-plugin</artifactId>
7178
</plugin>

‎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

+7
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@
5757
</resource>
5858
</resources>
5959
<plugins>
60+
<plugin>
61+
<groupId>org.mvnsearch</groupId>
62+
<artifactId>toolchains-maven-plugin</artifactId>
63+
</plugin>
64+
<plugin>
65+
<artifactId>maven-toolchains-plugin</artifactId>
66+
</plugin>
6067
<plugin>
6168
<artifactId>maven-jar-plugin</artifactId>
6269
<configuration>

‎android/guava/src/com/google/common/hash/BloomFilter.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ public final class BloomFilter<T extends @Nullable Object> implements Predicate<
7474
*
7575
* <p>Implementations should be collections of pure functions (i.e. stateless).
7676
*/
77-
interface Strategy extends Serializable {
77+
interface Strategy extends java.io.Serializable {
7878

7979
/**
8080
* Sets {@code numHashFunctions} bits of the given bit array, by hashing a user element.

‎android/pom.xml

+110-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,7 +127,7 @@
122127
<plugins>
123128
<plugin>
124129
<artifactId>maven-compiler-plugin</artifactId>
125-
<version>3.8.1</version>
130+
<version>3.13.0</version>
126131
<configuration>
127132
<source>1.8</source>
128133
<target>1.8</target>
@@ -139,7 +144,32 @@
139144
<arg>doesnotexist</arg>
140145
<!-- https://errorprone.info/docs/installation#maven -->
141146
<arg>-XDcompilePolicy=simple</arg>
142-
<!-- -Xplugin:ErrorProne is set conditionally by a profile. -->
147+
148+
<!-- https://errorprone.info/docs/installation#maven -->
149+
<!-- TODO(cpovirk): Enable NullArgumentForNonNullParameter for
150+
prod code. It's disabled automatically for "test code"
151+
(which is good: our tests have intentional violations), but
152+
Error Prone doesn't know it's building test code unless we
153+
pass -XepCompilingTestOnlyCode, and that argument needs to
154+
be passed as part of the same <arg> as -Xplugin:ErrorProne,
155+
and I gave up trying to figure out how to do that for test
156+
compilation only. -->
157+
<arg>-Xplugin:ErrorProne -Xep:NullArgumentForNonNullParameter:OFF -Xep:Java8ApiChecker:ERROR</arg>
158+
<!-- https://github.com/google/error-prone/blob/f8e33bc460be82ab22256a7ef8b979d7a2cacaba/docs/installation.md#jdk-16 -->
159+
<!-- TODO(cpovirk): Use .mvn/jvm.config instead (per
160+
https://errorprone.info/docs/installation#maven) if it can
161+
be made not to interfere with JDK8 or if we stop building
162+
with JDK8. -->
163+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED</arg>
164+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED</arg>
165+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED</arg>
166+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED</arg>
167+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED</arg>
168+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED</arg>
169+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED</arg>
170+
<arg>-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED</arg>
171+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED</arg>
172+
<arg>-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED</arg>
143173
</compilerArgs>
144174
<annotationProcessorPaths>
145175
<path>
@@ -157,6 +187,70 @@
157187
<fork>true</fork>
158188
</configuration>
159189
</plugin>
190+
<plugin>
191+
<groupId>org.mvnsearch</groupId>
192+
<artifactId>toolchains-maven-plugin</artifactId>
193+
<version>4.5.0</version>
194+
<executions>
195+
<!--
196+
We can apparently have only one <jdk> per execution: Others are silently ignored :(
197+
To properly test this, you need to remove existing toolchains:
198+
rm -rf ~/.m2/jdks/ ~/.m2/toolchains.xml
199+
(But don't run that if you have put something into ~/.m2/toolchains.xml yourself.)
200+
-->
201+
<execution>
202+
<id>download-11</id>
203+
<goals>
204+
<goal>toolchain</goal>
205+
</goals>
206+
<configuration>
207+
<toolchains>
208+
<jdk>
209+
<version>11</version>
210+
<vendor>zulu</vendor>
211+
</jdk>
212+
</toolchains>
213+
</configuration>
214+
</execution>
215+
<execution>
216+
<id>download-22-and-surefire-version</id>
217+
<goals>
218+
<goal>toolchain</goal>
219+
</goals>
220+
<configuration>
221+
<toolchains>
222+
<jdk>
223+
<version>22</version>
224+
<vendor>zulu</vendor>
225+
</jdk>
226+
<testJdk>
227+
<version>${surefire.toolchain.version}</version>
228+
<vendor>zulu</vendor>
229+
</testJdk>
230+
</toolchains>
231+
</configuration>
232+
</execution>
233+
</executions>
234+
</plugin>
235+
<plugin>
236+
<artifactId>maven-toolchains-plugin</artifactId>
237+
<version>3.2.0</version>
238+
<executions>
239+
<execution>
240+
<goals>
241+
<goal>toolchain</goal>
242+
</goals>
243+
</execution>
244+
</executions>
245+
<configuration>
246+
<toolchains>
247+
<jdk>
248+
<version>22</version>
249+
<vendor>zulu</vendor>
250+
</jdk>
251+
</toolchains>
252+
</configuration>
253+
</plugin>
160254
<plugin>
161255
<artifactId>maven-jar-plugin</artifactId>
162256
<version>3.2.0</version>
@@ -176,7 +270,7 @@
176270
<dependency>
177271
<groupId>org.codehaus.plexus</groupId>
178272
<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. -->
273+
<!-- 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). -->
180274
<version>3.4.1</version>
181275
</dependency>
182276
</dependencies>
@@ -219,8 +313,13 @@
219313
</plugin>
220314
<plugin>
221315
<artifactId>maven-javadoc-plugin</artifactId>
222-
<version>3.5.0</version>
316+
<version>3.8.0</version>
223317
<configuration>
318+
<!-- TODO: b/286965322 - Use a newer version (probably the default toolchain we set up?) if it doesn't break Javadoc (including causing trouble for our later run of JDiff, which we do outside Maven, during snapshots/releases). -->
319+
<jdkToolchain>
320+
<version>11</version>
321+
<vendor>zulu</vendor>
322+
</jdkToolchain>
224323
<quiet>true</quiet>
225324
<notimestamp>true</notimestamp>
226325
<encoding>UTF-8</encoding>
@@ -231,7 +330,7 @@
231330
<additionalOption>-Xdoclint:-html</additionalOption>
232331
</additionalOptions>
233332
<linksource>true</linksource>
234-
<source>${java.specification.version}</source>
333+
<source>8</source>
235334
<additionalJOption>${maven-javadoc-plugin.additionalJOptions}</additionalJOption>
236335
</configuration>
237336
<executions>
@@ -251,8 +350,12 @@
251350
</plugin>
252351
<plugin>
253352
<artifactId>maven-surefire-plugin</artifactId>
254-
<version>2.7.2</version>
353+
<version>3.3.1</version>
255354
<configuration>
355+
<jdkToolchain>
356+
<version>${surefire.toolchain.version}</version>
357+
<vendor>zulu</vendor>
358+
</jdkToolchain>
256359
<includes>
257360
<include>${test.include}</include>
258361
</includes>
@@ -394,90 +497,6 @@
394497
</test.add.opens>
395498
</properties>
396499
</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>
481500
<profile>
482501
<id>javac-for-jvm18plus</id>
483502
<activation>

‎guava-gwt/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@
129129
</dependencies>
130130
<build>
131131
<plugins>
132+
<plugin>
133+
<groupId>org.mvnsearch</groupId>
134+
<artifactId>toolchains-maven-plugin</artifactId>
135+
</plugin>
136+
<plugin>
137+
<artifactId>maven-toolchains-plugin</artifactId>
138+
</plugin>
132139
<plugin>
133140
<artifactId>maven-compiler-plugin</artifactId>
134141
<configuration>

‎guava-testlib/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@
6060
</dependencies>
6161
<build>
6262
<plugins>
63+
<plugin>
64+
<groupId>org.mvnsearch</groupId>
65+
<artifactId>toolchains-maven-plugin</artifactId>
66+
</plugin>
67+
<plugin>
68+
<artifactId>maven-toolchains-plugin</artifactId>
69+
</plugin>
6370
<plugin>
6471
<artifactId>maven-compiler-plugin</artifactId>
6572
</plugin>

‎guava-tests/pom.xml

+7
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@
7272
</dependencies>
7373
<build>
7474
<plugins>
75+
<plugin>
76+
<groupId>org.mvnsearch</groupId>
77+
<artifactId>toolchains-maven-plugin</artifactId>
78+
</plugin>
79+
<plugin>
80+
<artifactId>maven-toolchains-plugin</artifactId>
81+
</plugin>
7582
<plugin>
7683
<artifactId>maven-compiler-plugin</artifactId>
7784
</plugin>

‎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.

0 commit comments

Comments
 (0)