Excavator: Allow Java 17 language features in all modules (#2898)

svc-excavator-bot · web-flow · commit a1ef6e6fd71a · 2024-11-04T16:57:18.000Z
diff --git a/build.gradle b/build.gradle
@@ -67,8 +67,7 @@ allprojects {
 }
 
 javaVersions {
-    libraryTarget = 11
-    runtime = 11
+    libraryTarget = 17
 }
 
 jdks {
diff --git a/changelog/@unreleased/pr-2898.v2.yml b/changelog/@unreleased/pr-2898.v2.yml
@@ -0,0 +1,5 @@
+type: break
+break:
+  description: All modules now require Java 17.
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2898
diff --git a/gradle-baseline-java/build.gradle b/gradle-baseline-java/build.gradle
@@ -25,6 +25,7 @@ dependencies {
     implementation 'org.ow2.asm:asm'
     implementation 'com.googlecode.java-diff-utils:diffutils'
     implementation 'com.palantir.gradle.utils:lazily-configured-mapping'
+    implementation 'com.palantir.gradle.utils:environment-variables'
     implementation 'com.palantir.gradle.failure-reports:gradle-failure-reports-exceptions'
 
     runtimeOnly 'com.palantir.javaformat:gradle-palantir-java-format'
@@ -62,6 +63,7 @@ tasks.test.dependsOn tasks.publishToMavenLocal
 test {
     environment 'CIRCLE_ARTIFACTS', "${buildDir}/artifacts"
     environment 'CIRCLE_TEST_REPORTS', "${buildDir}/circle-reports"
+
     systemProperty 'ignoreDeprecations', 'true'
 }
 
diff --git a/gradle-baseline-java/src/main/java/com/palantir/baseline/plugins/BaselineCircleCi.java b/gradle-baseline-java/src/main/java/com/palantir/baseline/plugins/BaselineCircleCi.java
@@ -18,6 +18,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Splitter;
+import com.palantir.gradle.utils.environmentvariables.EnvironmentVariables;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
@@ -38,16 +39,21 @@ public final class BaselineCircleCi implements Plugin<Project> {
 
     @Override
     public void apply(Project project) {
-        configurePluginsForReports(project);
-        configurePluginsForArtifacts(project);
+        EnvironmentVariables environmentVariables = project.getObjects().newInstance(EnvironmentVariables.class);
+
+        configurePluginsForReports(project, environmentVariables);
+        configurePluginsForArtifacts(project, environmentVariables);
         Preconditions.checkState(
                 !project.getName().equals("project"),
                 "Please specify rootProject.name in your settings.gradle, otherwise CircleCI's"
                         + " checkout dir ('project') will be used instead.");
     }
 
-    private void configurePluginsForArtifacts(Project project) {
-        String circleArtifactsDir = System.getenv("CIRCLE_ARTIFACTS");
+    private void configurePluginsForArtifacts(Project project, EnvironmentVariables environmentVariables) {
+        String circleArtifactsDir = environmentVariables
+                .envVarOrFromTestingProperty("CIRCLE_ARTIFACTS")
+                .getOrNull();
+
         if (circleArtifactsDir == null) {
             project.getLogger().info("$CIRCLE_ARTIFACTS variable is not set, not configuring junit/profiling reports");
             return;
@@ -66,8 +72,11 @@ private void configurePluginsForArtifacts(Project project) {
                 }));
     }
 
-    private void configurePluginsForReports(Project project) {
-        String circleReportsDir = System.getenv("CIRCLE_TEST_REPORTS");
+    private void configurePluginsForReports(Project project, EnvironmentVariables environmentVariables) {
+        String circleReportsDir = environmentVariables
+                .envVarOrFromTestingProperty("CIRCLE_TEST_REPORTS")
+                .getOrNull();
+
         if (circleReportsDir == null) {
             return;
         }
diff --git a/gradle-baseline-java/src/main/java/com/palantir/baseline/plugins/BaselineImmutables.java b/gradle-baseline-java/src/main/java/com/palantir/baseline/plugins/BaselineImmutables.java
@@ -55,6 +55,13 @@ public Iterable<String> asArguments() {
                                                     : Collections.emptyList();
                                         }
                                     });
+
+                            // This *attempts* to make immutables work by add the exports to the fork options.
+                            // However, this only happens if the compilation is actually forked, which at the time
+                            // of writing is not always the case. Gradle will fork the compiler if the version of
+                            // Java required by the compiler is different to the one running the Gradle daemon. But
+                            // if they are the same, it will not fork and this extra export option **will have no
+                            // effect**.
                             javaCompileTask
                                     .getOptions()
                                     .getForkOptions()
diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineCircleCiJavaIntegrationTests.java b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineCircleCiJavaIntegrationTests.java
@@ -29,23 +29,25 @@
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.contrib.java.lang.system.EnvironmentVariables;
 import org.junit.rules.TemporaryFolder;
 
 public class BaselineCircleCiJavaIntegrationTests {
-
-    @Rule
-    public final EnvironmentVariables env = new EnvironmentVariables();
-
     @Rule
     public final TemporaryFolder projectDir = new TemporaryFolder();
 
     private File reportsDir;
 
     @Before
-    public void before() {
+    public void before() throws IOException {
         reportsDir = new File(projectDir.getRoot(), "circle/reports");
-        env.set("CIRCLE_TEST_REPORTS", reportsDir.toString());
+
+        java.nio.file.Files.writeString(
+                projectDir.newFile("gradle.properties").toPath(),
+                """
+                __TESTING=true
+                __TESTING_CIRCLE_TEST_REPORTS=%s
+                """
+                        .formatted(reportsDir.toString()));
 
         copyTestFile("build.gradle", projectDir, "build.gradle");
         copyTestFile("subproject.gradle", projectDir, "subproject/build.gradle");
diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineFormatIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineFormatIntegrationTest.groovy
@@ -37,9 +37,14 @@ class BaselineFormatIntegrationTest extends AbstractPluginTest {
                 new File("../gradle-baseline-java-config/resources"),
                 new File(projectDir, ".baseline"))
         // Disable copyright by default so we can test it individually
-        file('gradle.properties') << "com.palantir.baseline-format.copyright=false\n"
+        file('gradle.properties') << '''
+            com.palantir.baseline-format.copyright=false
+            # Required for the eclipse formatter. Delete once it's removed.
+            org.gradle.jvmargs = --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED
+        '''.stripIndent(true)
     }
 
+    // language=Gradle
     def standardBuildFile = '''
         plugins {
             id 'java'
@@ -49,7 +54,7 @@ class BaselineFormatIntegrationTest extends AbstractPluginTest {
             // to resolve the `palantirJavaFormat` configuration
             mavenCentral()
         }
-    '''.stripIndent()
+    '''.stripIndent(true)
 
     def noJavaBuildFile = '''
         plugins {
diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineModuleJvmArgsIntegrationTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/BaselineModuleJvmArgsIntegrationTest.groovy
@@ -48,7 +48,6 @@ class BaselineModuleJvmArgsIntegrationTest extends IntegrationSpec {
     '''.stripIndent(true)
 
     def setup() {
-        setFork(true)
         buildFile << standardBuildFile
     }
 
diff --git a/gradle-baseline-java/src/test/groovy/com/palantir/baseline/plugins/BaselineImmutablesTest.groovy b/gradle-baseline-java/src/test/groovy/com/palantir/baseline/plugins/BaselineImmutablesTest.groovy
@@ -22,21 +22,38 @@ import nebula.test.functional.ExecutionResult
 
 class BaselineImmutablesTest extends IntegrationSpec {
     private static final String IMMUTABLES = 'org.immutables:value:2.8.8'
-    private static final String IMMUTABLES_ANNOTATIONS = 'org.immutables:value:2.8.8:annotations'
+    private static final String IMMUTABLES_ANNOTATIONS = IMMUTABLES + ':annotations'
 
-    def 'inserts incremental compilation args into source sets that have immutables'() {
-        buildFile << """
+    def setup() {
+        // language=Gradle
+        buildFile << '''
             plugins {
                 id 'org.unbroken-dome.test-sets' version '4.0.0'
             }
-
+            
             apply plugin: 'com.palantir.baseline-immutables'
             apply plugin: 'java-library'
 
             repositories {
                 mavenCentral()
             }
             
+            task compileAll
+            
+            tasks.withType(JavaCompile) { javaCompile ->
+                doFirst {
+                    logger.lifecycle "Debug compiler args: \${javaCompile.name}: \${javaCompile.options.allCompilerArgs}"
+                    logger.lifecycle "Debug compiler fork args: \${javaCompile.name}: \${javaCompile.options.forkOptions.allJvmArgs}"
+                    logger.lifecycle "Debug compiler fork: \${javaCompile.name}: \${javaCompile.options.fork}"
+                }
+                                
+                tasks.compileAll.dependsOn javaCompile
+            }
+        '''.stripIndent(true)
+    }
+
+    def 'inserts incremental compilation args into source sets that have immutables'() {
+        buildFile << """
             testSets {
                 hasImmutables
                 doesNotHaveImmutables
@@ -57,16 +74,6 @@ class BaselineImmutablesTest extends IntegrationSpec {
 
                 onlyHasImmutablesAnnotationsAnnotationProcessor '$IMMUTABLES_ANNOTATIONS'
             }
-            
-            task compileAll
-            
-            tasks.withType(JavaCompile) { javaCompile ->
-                doFirst {
-                    logger.lifecycle "\${javaCompile.name}: \${javaCompile.options.allCompilerArgs}"
-                }
-                                
-                tasks.compileAll.dependsOn javaCompile
-            }
         """.stripIndent()
 
         ['main', 'hasImmutables', 'doesNotHaveImmutables', 'hasImmutablesAddedInAfterEvaluate', 'onlyHasImmutablesAnnotations'].each {
@@ -88,16 +95,15 @@ class BaselineImmutablesTest extends IntegrationSpec {
     }
 
     def 'Compatible with java #javaVersion'() {
+        // Context: https://github.com/immutables/immutables/issues/1379#issuecomment-1254224741
+
         when:
         buildFile << """
             apply plugin: 'com.palantir.baseline-java-versions'
-            apply plugin: 'com.palantir.baseline-immutables'
-            apply plugin: 'java-library'
-            repositories {
-                mavenCentral()
-            }
             tasks.withType(JavaCompile).configureEach({
-              options.compilerArgs += ['-Werror']
+                options.compilerArgs += ['-Werror']
+                // See comment about fork options in BaselineImmutables
+                options.fork = true
             })
             javaVersions {
                 libraryTarget = $javaVersion
@@ -106,27 +112,33 @@ class BaselineImmutablesTest extends IntegrationSpec {
                 annotationProcessor '$IMMUTABLES'
                 compileOnly '$IMMUTABLES_ANNOTATIONS'
             }
-        """.stripIndent()
-        writeJavaSourceFile("""
-        package com.palantir.one;
-        import com.palantir.two.ImmutableTwo;
-        import org.immutables.value.Value;
-        @Value.Immutable
-        public interface One {
-            ImmutableTwo two();
-        }
-        """.stripIndent())
-        writeJavaSourceFile("""
-        package com.palantir.two;
-        import org.immutables.value.Value;
-        @Value.Immutable
-        public interface Two {
-            String value();
-        }
-        """.stripIndent())
+        """.stripIndent(true)
+
+        // language=Java
+        writeJavaSourceFile('''
+            package com.palantir.one;
+            import com.palantir.two.ImmutableTwo;
+            import org.immutables.value.Value;
+            @Value.Immutable
+            public interface One {
+                ImmutableTwo two();
+            }
+        '''.stripIndent(true))
+
+        // language=Java
+        writeJavaSourceFile('''
+            package com.palantir.two;
+            import org.immutables.value.Value;
+            @Value.Immutable
+            public interface Two {
+                String value();
+            }
+        '''.stripIndent(true))
+
         then:
         ExecutionResult result = runTasks('compileJava')
         println(result.standardError)
+        println(result.standardOutput)
         result.success
 
         where:
@@ -135,13 +147,6 @@ class BaselineImmutablesTest extends IntegrationSpec {
 
     def 'handles an annotationProcesor source set extending from another one'() {
         buildFile << """
-            apply plugin: 'com.palantir.baseline-immutables'
-            apply plugin: 'java-library'
-
-            repositories {
-                mavenCentral()
-            }
-
             dependencies {
                 annotationProcessor '$IMMUTABLES'
                 compileOnly '$IMMUTABLES_ANNOTATIONS'
@@ -151,12 +156,6 @@ class BaselineImmutablesTest extends IntegrationSpec {
                 testAnnotationProcessor.extendsFrom annotationProcessor
                 testCompileOnly.extendsFrom compileOnly
             } 
-            
-            tasks.withType(JavaCompile) { javaCompile ->
-                doFirst {
-                    logger.lifecycle "\${javaCompile.name}: \${javaCompile.options.allCompilerArgs}"
-                }
-            }
         """.stripIndent(true)
 
         def testJava = 'src/test/java'
diff --git a/versions.lock b/versions.lock
@@ -28,7 +28,8 @@ com.googlecode.concurrent-trees:concurrent-trees:2.6.1 (1 constraints: 761166da)
 com.googlecode.java-diff-utils:diffutils:1.3.0 (1 constraints: 0605f935)
 com.googlecode.javaewah:JavaEWAH:1.1.12 (1 constraints: 750eee5e)
 com.palantir.gradle.failure-reports:gradle-failure-reports-exceptions:1.2.0 (1 constraints: 0505f635)
-com.palantir.gradle.utils:lazily-configured-mapping:0.1.0 (1 constraints: 0305ee35)
+com.palantir.gradle.utils:environment-variables:0.9.0 (1 constraints: 0b050636)
+com.palantir.gradle.utils:lazily-configured-mapping:0.9.0 (1 constraints: 0b050636)
 com.palantir.javaformat:gradle-palantir-java-format:1.1.0 (1 constraints: 0405f335)
 com.palantir.javaformat:palantir-java-format-spi:1.1.0 (1 constraints: 711560be)
 com.uber.nullaway:nullaway:0.12.1 (1 constraints: 36052a3b)
diff --git a/versions.props b/versions.props
@@ -15,7 +15,7 @@ org.ow2.asm:asm = 9.7.1
 com.google.errorprone:error_prone_* = 2.31.0
 com.googlecode.java-diff-utils:diffutils = 1.3.0
 com.puppycrawl.tools:checkstyle = 10.20.0
-com.palantir.gradle.utils:* = 0.1.0
+com.palantir.gradle.utils:* = 0.9.0
 com.uber.nullaway:nullaway = 0.12.1
 com.palantir.gradle.failure-reports:* = 1.2.0
 

Original file line number	Diff line number	Diff line change
`@@ -67,8 +67,7 @@ allprojects {`
`67`	`67`	`}`
`68`	`68`
`69`	`69`	`javaVersions {`
`70`		`- libraryTarget = 11`
`71`		`- runtime = 11`
	`70`	`+ libraryTarget = 17`
`72`	`71`	`}`
`73`	`72`
`74`	`73`	`jdks {`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,6 @@ class BaselineModuleJvmArgsIntegrationTest extends IntegrationSpec {`
`48`	`48`	`'''.stripIndent(true)`
`49`	`49`
`50`	`50`	`def setup() {`
`51`		`- setFork(true)`
`52`	`51`	`buildFile << standardBuildFile`
`53`	`52`	`}`
`54`	`53`