Skip to content

Commit 2c6e3b9

Browse files
cpovirkGoogle Java Core Libraries
authored and
Google Java Core Libraries
committed
Make it possible to actually use (e.g.) -Dsurefire.toolchain.version=21 to set the Java version for tests, and do so in CI. (The property value still _defaults_ to the JDK that you use to run Maven, so passing the flag is not _necessary_.)
This lets us use whatever Java version we want\[1\] for executing Maven itself—and thus for executing any plugins that aren't toolchain-aware. (I even hacked up our toolchain configuration to run tests under Java 24, and it worked.) I've changed our CI to do that, too, by always using Java 23. (This makes the explicit choice of Java 23 for _javac_ redundant under CI, as well as on our Google workstations. However, it can be useful for anyone who builds Guava locally while having an older JDK as the default version.) Notably, that means that we can use a new version (like Java 23) for running the GWT plugin, which lets us upgrade to GWT 2.12.1 (cl/711417161), which uses an Eclipse compiler with bytecode version 55 (Java 11)\[2\]. (This means that users who build `guava-gwt` locally with Java 8 will need to [upgrade their build JDK to at least Java 11](https://github.com/google/guava/issues/6549)—or skip building `guava-gwt` if they don't need it.) That GWT upgrade is in turn necessary in order for us to be able to use use [JSpecify annotations](jspecify/jspecify#239) from our GWT artifact. In theory, this could also prepare us for further changes, like more easily running tests under multiple versions during our release process (in addition to how we already do so for CI), though there are already other ways of doing that by hacking up our release script or `pom.xml`. And I'm not sure that I'd really want to do that, anyway. (See also some notes that I've added to cl/639128020.) I also did some related cleanup: - Our `--no-module-directories` configuration probably became Just Wrong back in cl/655647768: It should be based on the version that we use to build Javadoc (which is currently always 11), not the version that Maven is running under. It probably mostly doesn't matter because we probably mostly don't try to run Javadoc under other versions, but I'd like to be able to do so, so this is a step forward. - `-Djava.security.manager=allow` is not necessary now that we've cleaned up our usages of `SecurityManager`. (And that's fortunate, as it seems difficult to [active a profile based on a range of values of an arbitrary "my test JRE version" property](https://issues.apache.org/jira/browse/MNG-3328), as opposed to activate it based on the JDK that Maven is running under.) Fixes #7492 (by making `setup-java` download all the toolchains that we need) \[1\] I mean, I suspect that Maven support lags new JDKs somewhat, and I've seen problems with other versions (as documented in cl/488902996). But at least we don't have to use the same Java version for executing Maven itself as we do for executing tests. \[2\] That was causing this error: ``` [INFO] --- gwt:2.10.0:compile (gwt-compile) @ guava-gwt --- Error: [ERROR] Unexpected internal compiler error [INFO] java.lang.UnsupportedClassVersionError: org/eclipse/jdt/internal/compiler/classfmt/ClassFormatException has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0 [INFO] at java.lang.ClassLoader.defineClass1(Native Method) [INFO] at java.lang.ClassLoader.defineClass(ClassLoader.java:757) [INFO] at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:142) [INFO] at java.net.URLClassLoader.defineClass(URLClassLoader.java:473) [INFO] at java.net.URLClassLoader.access$100(URLClassLoader.java:74) [INFO] at java.net.URLClassLoader$1.run(URLClassLoader.java:369) [INFO] at java.net.URLClassLoader$1.run(URLClassLoader.java:363) [INFO] at java.security.AccessController.doPrivileged(Native Method) [INFO] at java.net.URLClassLoader.findClass(URLClassLoader.java:362) [INFO] at java.lang.ClassLoader.loadClass(ClassLoader.java:419) [INFO] at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352) [INFO] at java.lang.ClassLoader.loadClass(ClassLoader.java:352) [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder.doBuildFrom(CompilationStateBuilder.java:488) [INFO] at com.google.gwt.dev.javac.CompilationStateBuilder.buildFrom(CompilationStateBuilder.java:464) [INFO] at com.google.gwt.dev.cfg.ModuleDef.getCompilationState(ModuleDef.java:426) [INFO] at com.google.gwt.dev.Precompile.validate(Precompile.java:145) [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:184) [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:143) [INFO] at com.google.gwt.dev.Compiler.compile(Compiler.java:132) [INFO] at com.google.gwt.dev.Compiler$1.run(Compiler.java:110) [INFO] at com.google.gwt.dev.CompileTaskRunner.doRun(CompileTaskRunner.java:55) [INFO] at com.google.gwt.dev.CompileTaskRunner.runWithAppropriateLogger(CompileTaskRunner.java:50) [INFO] at com.google.gwt.dev.Compiler.main(Compiler.java:113) ``` RELNOTES=n/a PiperOrigin-RevId: 711476575
1 parent 4559277 commit 2c6e3b9

File tree

4 files changed

+248
-126
lines changed

4 files changed

+248
-126
lines changed

.github/workflows/ci.yml

+11-4
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,26 @@ jobs:
3737
access_token: ${{ github.token }}
3838
- name: 'Check out repository'
3939
uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
40-
- name: 'Set up JDK ${{ matrix.java }}'
40+
# When we specify multiple JDKs, the final one becomes the default, which is used to execute Maven itself.
41+
# Our Maven configuration then specifies different JDKs to use for some of the steps:
42+
# - 11 for building Javadoc
43+
# - 23 for running javac (to help people who build Guava locally and might not use a recent JDK to run Maven)
44+
# (Note that we also use Java 11 for running our Gradle integration tests. But we run those only when we are already going to run tests under Java 11.)
45+
- name: 'Set up JDKs'
4146
uses: actions/setup-java@8df1039502a15bceb9433410b1a100fbe190c53b # v4.5.0
42-
4347
with:
44-
java-version: ${{ matrix.java }}
48+
java-version: |
49+
${{ matrix.java }}
50+
11
51+
23
4552
distribution: 'zulu'
4653
cache: 'maven'
4754
- name: 'Install'
4855
shell: bash
4956
run: ./mvnw -B -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn install -U -DskipTests=true -f $ROOT_POM
5057
- name: 'Test'
5158
shell: bash
52-
run: ./mvnw -B -P!standard-with-extra-repos verify -U -Dmaven.javadoc.skip=true -f $ROOT_POM
59+
run: ./mvnw -B -P!standard-with-extra-repos verify -U -Dmaven.javadoc.skip=true -Dsurefire.toolchain.version=${{ matrix.java }} -f $ROOT_POM
5360
- name: 'Print Surefire reports'
5461
# Note: Normally a step won't run if the job has failed, but this causes it to
5562
if: ${{ failure() }}

android/pom.xml

+116-61
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,8 @@
1313
<url>https://github.com/google/guava</url>
1414
<properties>
1515
<!--
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.
16+
When building Guava, you can pass (e.g.) `-Dsurefire.toolchain.version=21` to change which version to run tests under.
17+
You may find that you need to use Java 11+ to *build* Guava, but it continues to work under Java 8, and you can run tests to verify that, as we do.
1918
-->
2019
<surefire.toolchain.version>${java.specification.version}</surefire.toolchain.version>
2120
<!-- Override this with -Dtest.include="**/SomeTest.java" on the CLI -->
@@ -28,7 +27,19 @@
2827
<maven-javadoc-plugin.additionalJOptions></maven-javadoc-plugin.additionalJOptions>
2928
<project.build.outputTimestamp>2024-01-02T00:00:00Z</project.build.outputTimestamp>
3029
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
31-
<test.add.opens></test.add.opens>
30+
<!--
31+
Some tests need reflective access to the internals of these packages. It is only the
32+
tests themselves and not the code being tested that needs that access, though there's no
33+
obvious way to ensure that.
34+
35+
We could consider arranging things so that only the tests we know need this would get
36+
the add-opens. Right now that doesn't seem worth the effort, though.
37+
-->
38+
<test.add.opens>
39+
--add-opens java.base/java.lang=ALL-UNNAMED
40+
--add-opens java.base/java.util=ALL-UNNAMED
41+
--add-opens java.base/sun.security.jca=ALL-UNNAMED
42+
</test.add.opens>
3243
<test.add.args></test.add.args>
3344
<module.status>integration</module.status>
3445
<variant.jvmEnvironment>android</variant.jvmEnvironment>
@@ -321,7 +332,7 @@
321332
<artifactId>maven-javadoc-plugin</artifactId>
322333
<version>3.11.1</version>
323334
<configuration>
324-
<!-- 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). -->
335+
<!-- TODO: b/286965322 - Use a newer version (probably the default javac 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). At that point, we'll likely want to remove the no-module-directories additionalJOption, as documented below. -->
325336
<jdkToolchain>
326337
<version>11</version>
327338
<vendor>zulu</vendor>
@@ -337,7 +348,14 @@
337348
</additionalOptions>
338349
<linksource>true</linksource>
339350
<source>8</source>
340-
<additionalJOption>${maven-javadoc-plugin.additionalJOptions}</additionalJOption>
351+
<!--
352+
Required to make symbol search work correctly in the generated pages under JDK 11-12.
353+
354+
This flag does not exist on 9-10 and 13+ (https://bugs.openjdk.java.net/browse/JDK-8215582).
355+
356+
Consider removing it once our Javadoc generation is migrated to a recent JDK.
357+
-->
358+
<additionalJOption>--no-module-directories</additionalJOption>
341359
</configuration>
342360
<executions>
343361
<execution>
@@ -448,76 +466,113 @@
448466
</dependencyManagement>
449467
<profiles>
450468
<profile>
451-
<id>sonatype-oss-release</id>
452-
<build>
453-
<plugins>
454-
<plugin>
455-
<artifactId>maven-gpg-plugin</artifactId>
456-
<version>3.0.1</version>
457-
<executions>
458-
<execution>
459-
<id>sign-artifacts</id>
460-
<phase>verify</phase>
461-
<goals>
462-
<goal>sign</goal>
463-
</goals>
464-
</execution>
465-
</executions>
466-
</plugin>
467-
</plugins>
469+
<id>sonatype-oss-release</id>
470+
<build>
471+
<plugins>
472+
<plugin>
473+
<artifactId>maven-gpg-plugin</artifactId>
474+
<version>3.0.1</version>
475+
<executions>
476+
<execution>
477+
<id>sign-artifacts</id>
478+
<phase>verify</phase>
479+
<goals>
480+
<goal>sign</goal>
481+
</goals>
482+
</execution>
483+
</executions>
484+
</plugin>
485+
</plugins>
468486
</build>
469487
</profile>
470488
<profile>
471-
<!--
472-
Passes JDK 11-12-specific `no-module-directories` flag to Javadoc tool,
473-
which is required to make symbol search work correctly in the generated
474-
pages.
475-
476-
This flag does not exist on 9-10 and 13+ (https://bugs.openjdk.java.net/browse/JDK-8215582).
477-
478-
Consider removing it once our release and test scripts are migrated to a recent JDK (17+).
479-
-->
480-
<id>javadocs-jdk11-12</id>
489+
<id>suppress-open-jre-modules-for-toolchain-1.8</id>
481490
<activation>
482-
<jdk>[11,13)</jdk>
491+
<property>
492+
<name>surefire.toolchain.version</name>
493+
<!-- the value provided by java.specification.version -->
494+
<value>1.8</value>
495+
</property>
483496
</activation>
484497
<properties>
485-
<maven-javadoc-plugin.additionalJOptions>--no-module-directories</maven-javadoc-plugin.additionalJOptions>
498+
<test.add.opens></test.add.opens>
486499
</properties>
487500
</profile>
488501
<profile>
489-
<id>open-jre-modules</id>
502+
<id>suppress-open-jre-modules-for-toolchain-8</id>
490503
<activation>
491-
<jdk>[9,]</jdk>
504+
<property>
505+
<name>surefire.toolchain.version</name>
506+
<!-- the value provided by GitHub CI (which maybe we could even change, but supporting "8" seems nice for any users who try pass that value manually) -->
507+
<value>8</value>
508+
</property>
492509
</activation>
493510
<properties>
494-
<!--
495-
Some tests need reflective access to the internals of these packages. It is only the
496-
tests themselves and not the code being tested that needs that access, though there's no
497-
obvious way to ensure that.
498-
499-
We could consider arranging things so that only the tests we know need this would get
500-
the add-opens. Right now that doesn't seem worth the effort, though.
501-
-->
502-
<test.add.opens>
503-
--add-opens java.base/java.lang=ALL-UNNAMED
504-
--add-opens java.base/java.util=ALL-UNNAMED
505-
--add-opens java.base/sun.security.jca=ALL-UNNAMED
506-
</test.add.opens>
511+
<test.add.opens></test.add.opens>
507512
</properties>
508513
</profile>
514+
<!-- The Gradle integration tests need to set JAVA_HOME to the location of Java 11. The following profile extracts that location. -->
509515
<profile>
510-
<id>javac-for-jvm18plus</id>
511-
<activation>
512-
<!--
513-
In order to build and run the tests against JDK 18+, we need to pass java.security.manager=allow, to make
514-
the deprecated 'java.lang.SecurityManager' available for use.
515-
-->
516-
<jdk>[18,]</jdk>
517-
</activation>
518-
<properties>
519-
<test.add.args>-Djava.security.manager=allow</test.add.args>
520-
</properties>
516+
<id>print-java-11-home</id>
517+
<build>
518+
<plugins>
519+
<plugin>
520+
<artifactId>maven-toolchains-plugin</artifactId>
521+
<executions>
522+
<execution>
523+
<id>select-java-11</id>
524+
<phase>initialize</phase>
525+
<goals>
526+
<goal>toolchain</goal>
527+
</goals>
528+
</execution>
529+
</executions>
530+
<configuration>
531+
<toolchains>
532+
<jdk>
533+
<version>11</version>
534+
<vendor>zulu</vendor>
535+
</jdk>
536+
</toolchains>
537+
</configuration>
538+
</plugin>
539+
<plugin>
540+
<groupId>com.diamondq.maven</groupId>
541+
<artifactId>javahome-resolver-maven-plugin</artifactId>
542+
<version>1.0.2</version>
543+
<executions>
544+
<execution>
545+
<id>resolve-java-11</id>
546+
<phase>initialize</phase>
547+
<goals>
548+
<goal>resolve</goal>
549+
</goals>
550+
</execution>
551+
</executions>
552+
</plugin>
553+
<plugin>
554+
<groupId>org.codehaus.mojo</groupId>
555+
<artifactId>exec-maven-plugin</artifactId>
556+
<version>3.5.0</version>
557+
<executions>
558+
<execution>
559+
<id>print-java-11-home</id>
560+
<phase>initialize</phase>
561+
<goals>
562+
<goal>exec</goal>
563+
</goals>
564+
<configuration>
565+
<executable>echo</executable>
566+
<arguments>
567+
<argument>${javaHome}</argument>
568+
</arguments>
569+
<outputFile>${project.build.directory}/java_11_home</outputFile>
570+
</configuration>
571+
</execution>
572+
</executions>
573+
</plugin>
574+
</plugins>
575+
</build>
521576
</profile>
522577
</profiles>
523578
</project>

0 commit comments

Comments
 (0)