From dd4b410bc003da3d6154d48860b4107cc7440f02 Mon Sep 17 00:00:00 2001 From: Rene Groeschke Date: Tue, 23 Jul 2024 15:25:07 +0200 Subject: [PATCH] [Gradle] Remove default Environment variables setup for LoggedExec tasks (#110375) We do not implicitly rely on all different kind of env variables when leveraging Gradle Configuration Cache this makes reusing config cache more reliable and improves cache hits * configure default env variables for logged exec task * Fix antfixturestop constructor * Make LoggedExec cc compatible (fix integtests) * Fix spotless --- .../gradle/internal/AntFixtureStop.groovy | 10 ++++-- .../internal/info/GlobalBuildInfoPlugin.java | 35 ------------------- .../org/elasticsearch/gradle/LoggedExec.java | 34 ++++++++++++++++-- 3 files changed, 39 insertions(+), 40 deletions(-) diff --git a/build-tools-internal/src/main/groovy/org/elasticsearch/gradle/internal/AntFixtureStop.groovy b/build-tools-internal/src/main/groovy/org/elasticsearch/gradle/internal/AntFixtureStop.groovy index e454d2ee38ff..658e2623cbbd 100644 --- a/build-tools-internal/src/main/groovy/org/elasticsearch/gradle/internal/AntFixtureStop.groovy +++ b/build-tools-internal/src/main/groovy/org/elasticsearch/gradle/internal/AntFixtureStop.groovy @@ -13,6 +13,7 @@ import org.elasticsearch.gradle.OS import org.elasticsearch.gradle.internal.test.AntFixture import org.gradle.api.file.FileSystemOperations import org.gradle.api.file.ProjectLayout +import org.gradle.api.provider.ProviderFactory import org.gradle.api.tasks.Internal import org.gradle.process.ExecOperations @@ -24,14 +25,17 @@ abstract class AntFixtureStop extends LoggedExec implements FixtureStop { AntFixture fixture @Inject - AntFixtureStop(ProjectLayout projectLayout, ExecOperations execOperations, FileSystemOperations fileSystemOperations) { - super(projectLayout, execOperations, fileSystemOperations) + AntFixtureStop(ProjectLayout projectLayout, + ExecOperations execOperations, + FileSystemOperations fileSystemOperations, + ProviderFactory providerFactory) { + super(projectLayout, execOperations, fileSystemOperations, providerFactory) } void setFixture(AntFixture fixture) { assert this.fixture == null this.fixture = fixture; - final Object pid = "${ -> this.fixture.pid }" + final Object pid = "${-> this.fixture.pid}" onlyIf("pidFile exists") { fixture.pidFile.exists() } doFirst { logger.info("Shutting down ${fixture.name} with pid ${pid}") diff --git a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java index e61bbefc9a97..e6059c729e8c 100644 --- a/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java +++ b/build-tools-internal/src/main/java/org/elasticsearch/gradle/internal/info/GlobalBuildInfoPlugin.java @@ -48,7 +48,6 @@ import java.io.UncheckedIOException; import java.nio.file.Files; import java.time.ZoneOffset; import java.time.ZonedDateTime; -import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.Optional; @@ -315,36 +314,6 @@ public class GlobalBuildInfoPlugin implements Plugin { return env == null ? Optional.empty() : Optional.of(new File(env)); } - @NotNull - private String resolveJavaHomeFromEnvVariable(String javaHomeEnvVar) { - Provider javaHomeNames = providers.gradleProperty("org.gradle.java.installations.fromEnv"); - // Provide a useful error if we're looking for a Java home version that we haven't told Gradle about yet - Arrays.stream(javaHomeNames.get().split(",")) - .filter(s -> s.equals(javaHomeEnvVar)) - .findFirst() - .orElseThrow( - () -> new GradleException( - "Environment variable '" - + javaHomeEnvVar - + "' is not registered with Gradle installation supplier. Ensure 'org.gradle.java.installations.fromEnv' is " - + "updated in gradle.properties file." - ) - ); - String versionedJavaHome = System.getenv(javaHomeEnvVar); - if (versionedJavaHome == null) { - final String exceptionMessage = String.format( - Locale.ROOT, - "$%s must be set to build Elasticsearch. " - + "Note that if the variable was just set you " - + "might have to run `./gradlew --stop` for " - + "it to be picked up. See https://github.com/elastic/elasticsearch/issues/31399 details.", - javaHomeEnvVar - ); - throw new GradleException(exceptionMessage); - } - return versionedJavaHome; - } - @NotNull private File resolveJavaHomeFromToolChainService(String version) { Property value = objectFactory.property(JavaLanguageVersion.class).value(JavaLanguageVersion.of(version)); @@ -354,10 +323,6 @@ public class GlobalBuildInfoPlugin implements Plugin { return javaLauncherProvider.get().getMetadata().getInstallationPath().getAsFile(); } - private static String getJavaHomeEnvVarName(String version) { - return "JAVA" + version + "_HOME"; - } - public static String getResourceContents(String resourcePath) { try ( BufferedReader reader = new BufferedReader(new InputStreamReader(GlobalBuildInfoPlugin.class.getResourceAsStream(resourcePath))) diff --git a/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java b/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java index 6087482db278..3a425d11ccf1 100644 --- a/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java +++ b/build-tools/src/main/java/org/elasticsearch/gradle/LoggedExec.java @@ -17,6 +17,8 @@ import org.gradle.api.logging.Logging; import org.gradle.api.provider.ListProperty; import org.gradle.api.provider.MapProperty; import org.gradle.api.provider.Property; +import org.gradle.api.provider.Provider; +import org.gradle.api.provider.ProviderFactory; import org.gradle.api.tasks.Input; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Optional; @@ -92,17 +94,45 @@ public abstract class LoggedExec extends DefaultTask implements FileSystemOperat private String output; @Inject - public LoggedExec(ProjectLayout projectLayout, ExecOperations execOperations, FileSystemOperations fileSystemOperations) { + public LoggedExec( + ProjectLayout projectLayout, + ExecOperations execOperations, + FileSystemOperations fileSystemOperations, + ProviderFactory providerFactory + ) { this.projectLayout = projectLayout; this.execOperations = execOperations; this.fileSystemOperations = fileSystemOperations; getWorkingDir().convention(projectLayout.getProjectDirectory().getAsFile()); // For now mimic default behaviour of Gradle Exec task here - getEnvironment().putAll(System.getenv()); + setupDefaultEnvironment(providerFactory); getCaptureOutput().convention(false); getSpoolOutput().convention(false); } + /** + * We explicitly configure the environment variables that are passed to the executed process. + * This is required to make sure that the build cache and Gradle configuration cache is correctly configured + * can be reused across different build invocations. + * */ + private void setupDefaultEnvironment(ProviderFactory providerFactory) { + getEnvironment().putAll(providerFactory.environmentVariablesPrefixedBy("BUILDKITE")); + getEnvironment().putAll(providerFactory.environmentVariablesPrefixedBy("GRADLE_BUILD_CACHE")); + getEnvironment().putAll(providerFactory.environmentVariablesPrefixedBy("VAULT")); + Provider javaToolchainHome = providerFactory.environmentVariable("JAVA_TOOLCHAIN_HOME"); + if (javaToolchainHome.isPresent()) { + getEnvironment().put("JAVA_TOOLCHAIN_HOME", javaToolchainHome); + } + Provider javaRuntimeHome = providerFactory.environmentVariable("RUNTIME_JAVA_HOME"); + if (javaRuntimeHome.isPresent()) { + getEnvironment().put("RUNTIME_JAVA_HOME", javaRuntimeHome); + } + Provider path = providerFactory.environmentVariable("PATH"); + if (path.isPresent()) { + getEnvironment().put("PATH", path); + } + } + @TaskAction public void run() { boolean spoolOutput = getSpoolOutput().get();