Improve isolation for nested Gradle runs

.gitlab-ci.yml

@@ -224,6 +224,7 @@ default:
       org.gradle.java.installations.auto-detect=false
       org.gradle.java.installations.auto-download=false
       org.gradle.java.installations.fromEnv=$JAVA_HOMES
+      org.gradle.console=colored
       EOF
     - mkdir -p .gradle
     - export GRADLE_USER_HOME=$(pwd)/.gradle
@@ -461,6 +462,7 @@ test_published_artifacts:
     - export GRADLE_OPTS="-Dorg.gradle.jvmargs='-Xms2G -Xmx2G -XX:ErrorFile=/tmp/hs_err_pid%p.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp'"
     - ./gradlew publishToMavenLocal $GRADLE_ARGS
     - cd test-published-dependencies
+    - printf '\norg.gradle.console=colored\n' >> gradle.properties
     - export GRADLE_OPTS="-Dorg.gradle.jvmargs='-Xms1G -Xmx1G -XX:ErrorFile=/tmp/hs_err_pid%p.log -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=/tmp'"
     - ./gradlew --version
     - ./gradlew check --info $GRADLE_ARGS

build-logic/smoke-test/src/main/kotlin/datadog/buildlogic/smoketest/NestedGradleBuild.kt

@@ -2,6 +2,7 @@ package datadog.buildlogic.smoketest
 
 import org.gradle.api.Action
 import org.gradle.api.DefaultTask
+import org.gradle.api.GradleException
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.FileTree
 import org.gradle.api.file.RegularFile
@@ -26,6 +27,8 @@ import org.gradle.jvm.toolchain.JavaLauncher
 import org.gradle.jvm.toolchain.JavaToolchainService
 import org.gradle.kotlin.dsl.newInstance
 import org.gradle.tooling.GradleConnector
+import java.io.File
+import java.util.concurrent.TimeUnit
 import javax.inject.Inject
 
 /**
@@ -103,9 +106,10 @@ abstract class NestedGradleBuild @Inject constructor(
   abstract val buildCacheEnabled: Property<Boolean>
 
   /**
-   * Extra environment variables for the nested Gradle daemon. Merged on top of the outer
-   * process environment — set a key to override an inherited value. The nested build script
-   * sees these via `System.getenv()` like any normal environment variable.
+   * Extra environment variables for the nested Gradle daemon. Merged on top of the outer process
+   * environment; Gradle launcher variables are reserved by this task so nested builds do not
+   * inherit incompatible outer-build settings. The nested build script sees these via
+   * `System.getenv()` like any normal environment variable.
    */
   @get:Input
   abstract val environment: MapProperty<String, String>
@@ -138,6 +142,7 @@ abstract class NestedGradleBuild @Inject constructor(
     val appDir = applicationDir.get().asFile
     val appBuildDirFile = applicationBuildDir.get().asFile
     val daemonJavaHome = javaLauncher.get().metadata.installationPath.asFile
+    val gradleUserHomeDir = createGradleUserHome()
 
     val args = buildList {
       add(if (buildCacheEnabled.get()) "--build-cache" else "--no-build-cache")
@@ -150,6 +155,7 @@ abstract class NestedGradleBuild @Inject constructor(
 
     val connector = GradleConnector.newConnector()
       .forProjectDirectory(appDir)
+      .useGradleUserHomeDir(gradleUserHomeDir)
       .apply {
         val distributionBaseUrl = gradleDistributionBaseUrl.orNull
         if (distributionBaseUrl.isNullOrBlank()) {
@@ -161,19 +167,111 @@ abstract class NestedGradleBuild @Inject constructor(
         }
       }
 
-    val extraEnv = environment.get()
-    val mergedEnv: Map<String, String>? =
-      if (extraEnv.isEmpty()) null else System.getenv() + extraEnv
-
-    connector.connect().use { connection ->
-      connection.newBuild()
-        .forTasks(*tasksToRun.get().toTypedArray())
-        .withArguments(args)
-        .setJavaHome(daemonJavaHome)
-        .apply { if (mergedEnv != null) setEnvironmentVariables(mergedEnv) }
-        .setStandardOutput(System.out)
-        .setStandardError(System.err)
-        .run()
+    val mergedEnv =
+      System.getenv() +
+        environment.get() +
+        mapOf(
+          "GRADLE_ARGS" to "",
+          "GRADLE_OPTS" to "",
+          "GRADLE_USER_HOME" to gradleUserHomeDir.absolutePath,
+        )
+
+    try {
+      connector.connect().use { connection ->
+        connection.newBuild()
+          .forTasks(*tasksToRun.get().toTypedArray())
+          .withArguments(args)
+          .setJavaHome(daemonJavaHome)
+          .setEnvironmentVariables(mergedEnv)
+          .setStandardOutput(System.out)
+          .setStandardError(System.err)
+          .run()
+      }
+    } finally {
+      stopGradleDaemon(appDir, gradleUserHomeDir, daemonJavaHome, mergedEnv)
+      deleteGradleUserHome(gradleUserHomeDir)
+    }
+  }
+
+  private fun stopGradleDaemon(
+    appDir: File,
+    gradleUserHomeDir: File,
+    daemonJavaHome: File,
+    environment: Map<String, String>,
+  ) {
+    val gradleExecutable = findGradleExecutable(gradleUserHomeDir)
+    if (gradleExecutable == null) {
+      logger.warn(
+        "Could not find nested Gradle executable under {} to stop its daemon",
+        gradleUserHomeDir.absolutePath,
+      )
+      return
+    }
+
+    try {
+      val processBuilder = ProcessBuilder(gradleExecutable.absolutePath, "--stop")
+        .directory(appDir)
+        .redirectOutput(ProcessBuilder.Redirect.INHERIT)
+        .redirectError(ProcessBuilder.Redirect.INHERIT)
+      processBuilder.environment().apply {
+        clear()
+        putAll(environment)
+        put("JAVA_HOME", daemonJavaHome.absolutePath)
+      }
+
+      val process = processBuilder.start()
+      if (!process.waitFor(GRADLE_STOP_TIMEOUT_SECONDS, TimeUnit.SECONDS)) {
+        process.destroyForcibly()
+        logger.warn("Timed out while stopping nested Gradle daemon")
+        return
+      }
+      val exitCode = process.exitValue()
+      if (exitCode != 0) {
+        logger.warn("Nested Gradle daemon stop exited with code {}", exitCode)
+      }
+    } catch (e: InterruptedException) {
+      Thread.currentThread().interrupt()
+      logger.warn(
+        "Interrupted while stopping nested Gradle daemon before deleting its user home",
+        e,
+      )
+    } catch (e: Exception) {
+      logger.warn("Could not stop nested Gradle daemon before deleting its user home", e)
     }
   }
+
+  private fun findGradleExecutable(gradleUserHomeDir: File): File? =
+    gradleUserHomeDir.walkTopDown().firstOrNull { file ->
+      file.isFile &&
+        file.name == gradleExecutableName() &&
+        file.parentFile?.name == "bin"
+    }
+
+  private fun gradleExecutableName(): String =
+    if (System.getProperty("os.name").lowercase().contains("windows")) {
+      "gradle.bat"
+    } else {
+      "gradle"
+    }
+
+  private fun createGradleUserHome(): File {
+    val directory = temporaryDir.resolve("gradle-user-home")
+    deleteGradleUserHome(directory)
+    if (!directory.mkdirs()) {
+      throw GradleException(
+        "Could not create nested Gradle user home: ${directory.absolutePath}",
+      )
+    }
+    return directory
+  }
+
+  private fun deleteGradleUserHome(directory: File) {
+    if (directory.exists() && !directory.deleteRecursively()) {
+      logger.warn("Could not delete nested Gradle user home: {}", directory.absolutePath)
+    }
+  }
+
+  private companion object {
+    const val GRADLE_STOP_TIMEOUT_SECONDS = 30L
+  }
 }

build-logic/smoke-test/src/main/kotlin/datadog/buildlogic/smoketest/SmokeTestAppExtension.kt

@@ -214,9 +214,10 @@ abstract class ApplicationSpec @Inject constructor() {
   abstract val buildArguments: ListProperty<String>
 
   /**
-   * Extra environment variables exposed to the nested Gradle daemon. Merged on top of the
-   * outer process environment — entries here override any inherited values with the same key.
-   * Use this for nested tooling that reads `JAVA_HOME`, `GRAALVM_HOME`, etc. from the env.
+   * Extra environment variables exposed to the nested Gradle daemon. Merged on top of the outer
+   * process environment; Gradle launcher variables are reserved by the nested build task so CI
+   * settings do not leak into pinned Gradle versions. Use this for nested tooling that reads
+   * `JAVA_HOME`, `GRAALVM_HOME`, etc. from the env.
    */
   abstract val environment: MapProperty<String, String>
 

build-logic/smoke-test/src/test/kotlin/datadog/buildlogic/smoketest/SmokeTestAppEndToEndTest.kt

@@ -3,13 +3,15 @@ package datadog.buildlogic.smoketest
 import org.assertj.core.api.Assertions.assertThat
 import org.gradle.testkit.runner.GradleRunner
 import org.gradle.testkit.runner.TaskOutcome
+import org.gradle.tooling.internal.consumer.DefaultGradleConnector
 import org.junit.jupiter.api.BeforeEach
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.io.TempDir
 import org.junit.jupiter.params.ParameterizedTest
 import org.junit.jupiter.params.provider.Arguments
 import org.junit.jupiter.params.provider.MethodSource
 import java.io.File
+import java.nio.file.Files
 import java.nio.file.Path
 
 /**
@@ -163,6 +165,76 @@ class SmokeTestAppEndToEndTest {
     assertThat(result.task(":customBuild")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
   }
 
+  @Test
+  fun `nested build clears inherited Gradle launcher environment`() {
+    writeOuterSettings()
+    val inheritedGradleUserHome = projectDir.resolve("inherited-gradle-user-home").toFile()
+    inheritedGradleUserHome.mkdirs()
+    outerBuild.writeText(
+      """
+      plugins {
+        java
+        id("dd-trace-java.smoke-test-app")
+      }
+
+      smokeTestApp {
+        javaLauncher.set(
+          javaToolchains.launcherFor { languageVersion.set(JavaLanguageVersion.of(${currentMajorJdk()})) }
+        )
+        application {
+          taskName.set("recordGradleEnvironment")
+          artifactPath.set("gradle-env.txt")
+          sysProperty.set("gradle.env.path")
+        }
+      }
+      """.trimIndent(),
+    )
+    writeInnerSettings()
+    writeInnerBuild(
+      """
+      tasks.register("recordGradleEnvironment") {
+        val out = layout.buildDirectory.file("gradle-env.txt")
+        outputs.file(out)
+        doLast {
+          out.get().asFile.writeText(
+            listOf(
+              "GRADLE_ARGS=${'$'}{System.getenv("GRADLE_ARGS") ?: "<null>"}",
+              "GRADLE_OPTS=${'$'}{System.getenv("GRADLE_OPTS") ?: "<null>"}",
+              "GRADLE_USER_HOME=${'$'}{System.getenv("GRADLE_USER_HOME") ?: "<null>"}",
+              "gradleUserHomeDir=${'$'}{gradle.gradleUserHomeDir.absolutePath}",
+            ).joinToString(System.lineSeparator())
+          )
+        }
+      }
+      """.trimIndent(),
+    )
+
+    val result = runner(
+      "recordGradleEnvironment",
+      environment = mapOf(
+        "GRADLE_ARGS" to "--info",
+        "GRADLE_OPTS" to "-Ddd.test.gradle.opts=inherited",
+        "GRADLE_USER_HOME" to inheritedGradleUserHome.absolutePath,
+      ),
+    ).build()
+
+    assertThat(result.task(":recordGradleEnvironment")?.outcome).isEqualTo(TaskOutcome.SUCCESS)
+    val envFile = File(projectDir.toFile(), "build/application/gradle-env.txt")
+    assertThat(envFile).exists()
+    val lines = envFile.readLines()
+    assertThat(lines).contains(
+      "GRADLE_ARGS=",
+      "GRADLE_OPTS=",
+    )
+    val gradleUserHomeEnv = lines.single { it.startsWith("GRADLE_USER_HOME=") }
+      .substringAfter("=")
+    val gradleUserHomeDir = lines.single { it.startsWith("gradleUserHomeDir=") }
+      .substringAfter("=")
+    assertThat(gradleUserHomeEnv).isEqualTo(gradleUserHomeDir)
+    assertThat(gradleUserHomeDir).isNotEqualTo(inheritedGradleUserHome.absolutePath)
+    assertThat(File(gradleUserHomeDir)).doesNotExist()
+  }
+
   /**
    * `buildCacheEnabled` defaults to `false` and is plumbed through to the nested daemon as
    * an explicit `--no-build-cache` / `--build-cache` argument. The inner build records
@@ -336,19 +408,47 @@ class SmokeTestAppEndToEndTest {
     )
   }
 
-  private fun runner(vararg args: String): GradleRunner =
+  private fun runner(
+    vararg args: String,
+    environment: Map<String, String>? = null,
+  ): GradleRunner =
     GradleRunner.create()
       .withProjectDir(projectDir.toFile())
       .withPluginClasspath()
       .withArguments(*args, "--stacktrace")
+      .withEnvironment(sanitizedGradleEnvironment(environment))
       .forwardOutput()
 
+  private fun sanitizedGradleEnvironment(
+    overrides: Map<String, String>? = null,
+  ): Map<String, String> =
+    System.getenv() +
+      mapOf(
+        "GRADLE_ARGS" to "",
+        "GRADLE_OPTS" to "",
+        "GRADLE_USER_HOME" to outerGradleUserHome.absolutePath,
+      ) +
+      (overrides ?: emptyMap())
+
   private fun currentMajorJdk(): Int =
     System.getProperty("java.specification.version").let {
       if (it.startsWith("1.")) it.substring(2).toInt() else it.toInt()
     }
 
   companion object {
+    private val outerGradleUserHome: File by lazy {
+      Files.createTempDirectory("smoke-test-app-gradle-user-home-").toFile().also { dir ->
+        Runtime.getRuntime().addShutdownHook(Thread {
+          try {
+            DefaultGradleConnector.close()
+          } catch (_: Exception) {
+            // best effort
+          }
+          dir.deleteRecursively()
+        })
+      }
+    }
+
     @JvmStatic
     fun buildCacheFlagCases(): List<Arguments> = listOf(
       // (scenario name, DSL line added to the `application { … }` block, expected

dd-smoke-tests/gradle/src/test/java/datadog/smoketest/GradleDaemonSmokeTest.java

@@ -25,7 +25,6 @@
 import org.gradle.tooling.internal.consumer.DefaultGradleConnector;
 import org.gradle.util.GradleVersion;
 import org.gradle.wrapper.Download;
-import org.gradle.wrapper.GradleUserHomeLookup;
 import org.gradle.wrapper.Install;
 import org.gradle.wrapper.PathAssembler;
 import org.gradle.wrapper.WrapperConfiguration;
@@ -286,7 +285,7 @@ private void ensureDependenciesDownloaded(String gradleVersion) {
               GradleVersion.current().getVersion(),
               GRADLE_DISTRIBUTION_NETWORK_TIMEOUT);
 
-      java.io.File userHomeDir = GradleUserHomeLookup.gradleUserHome();
+      java.io.File userHomeDir = testKitFolder.toFile();
       java.io.File projectDir = projectFolder.toFile();
       Install install = new Install(logger, download, new PathAssembler(userHomeDir, projectDir));
 
@@ -306,6 +305,9 @@ private void ensureDependenciesDownloaded(String gradleVersion) {
   private BuildResult runGradle(
       String gradleVersion, List<String> arguments, boolean successExpected) throws IOException {
     Map<String, String> buildEnv = new HashMap<>();
+    buildEnv.put("GRADLE_ARGS", "");
+    buildEnv.put("GRADLE_OPTS", "");
+    buildEnv.put("GRADLE_USER_HOME", testKitFolder.toString());
     buildEnv.put("GRADLE_VERSION", gradleVersion);
     buildEnv.put(
         GradleDistribution.GRADLE_DISTRIBUTION_URL_ENV,

dd-smoke-tests/gradle/src/test/java/datadog/smoketest/GradleLauncherSmokeTest.java

@@ -99,6 +99,7 @@ void stopGradleBuildDaemon() {
     Map<String, String> env = new HashMap<>();
     env.put("JAVA_HOME", JAVA_HOME);
     env.put("GRADLE_USER_HOME", gradleUserHome.toString());
+    env.put("GRADLE_ARGS", "");
     env.put("GRADLE_OPTS", "");
     ShellCommandExecutor shellCommandExecutor =
         new ShellCommandExecutor(projectFolder.toFile(), GRADLE_STOP_TIMEOUT_MILLIS, env);
@@ -114,7 +115,8 @@ private void givenGradleWrapper(String gradleVersion) throws Exception {
     Map<String, String> env = new HashMap<>();
     env.put("JAVA_HOME", JAVA_HOME);
     env.put("GRADLE_USER_HOME", gradleUserHome.toString());
-    // Avoid inheriting CI's GRADLE_OPTS which might be incompatible with the tested JVM.
+    // Avoid inheriting CI Gradle launcher settings that might be incompatible with this wrapper.
+    env.put("GRADLE_ARGS", "");
     env.put("GRADLE_OPTS", "");
     ShellCommandExecutor shellCommandExecutor =
         new ShellCommandExecutor(projectFolder.toFile(), GRADLE_BUILD_TIMEOUT_MILLIS, env);
@@ -139,6 +141,7 @@ private String whenRunningGradleLauncherWithJavaTracerInjected(String gradleDaem
     Map<String, String> env = new HashMap<>();
     env.put("JAVA_HOME", JAVA_HOME);
     env.put("GRADLE_USER_HOME", gradleUserHome.toString());
+    env.put("GRADLE_ARGS", "");
     env.put("GRADLE_OPTS", "-javaagent:" + AGENT_JAR);
     env.put("DD_CIVISIBILITY_ENABLED", "true");
     env.put("DD_CIVISIBILITY_AGENTLESS_ENABLED", "true");