Browse Source

[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
Rene Groeschke 1 year ago
parent
commit
dd4b410bc0

+ 7 - 3
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}")

+ 0 - 35
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<Project> {
         return env == null ? Optional.empty() : Optional.of(new File(env));
     }
 
-    @NotNull
-    private String resolveJavaHomeFromEnvVariable(String javaHomeEnvVar) {
-        Provider<String> 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<JavaLanguageVersion> value = objectFactory.property(JavaLanguageVersion.class).value(JavaLanguageVersion.of(version));
@@ -354,10 +323,6 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
         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)))

+ 32 - 2
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<String> javaToolchainHome = providerFactory.environmentVariable("JAVA_TOOLCHAIN_HOME");
+        if (javaToolchainHome.isPresent()) {
+            getEnvironment().put("JAVA_TOOLCHAIN_HOME", javaToolchainHome);
+        }
+        Provider<String> javaRuntimeHome = providerFactory.environmentVariable("RUNTIME_JAVA_HOME");
+        if (javaRuntimeHome.isPresent()) {
+            getEnvironment().put("RUNTIME_JAVA_HOME", javaRuntimeHome);
+        }
+        Provider<String> path = providerFactory.environmentVariable("PATH");
+        if (path.isPresent()) {
+            getEnvironment().put("PATH", path);
+        }
+    }
+
     @TaskAction
     public void run() {
         boolean spoolOutput = getSpoolOutput().get();