Skip to content

Commit

Permalink
[Build] Fix Concurrency issue in buildparams access (elastic#117552)
Browse files Browse the repository at this point in the history
Also provide caching support for buildparams provider
* Extract BuildParameterExtension public api into interface
* Make tests better readable
* Fix test flakyness
  • Loading branch information
breskeby committed Dec 13, 2024
1 parent 5a23da4 commit 1957078
Show file tree
Hide file tree
Showing 15 changed files with 419 additions and 190 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ class PublishPluginFuncTest extends AbstractGradleFuncTest {
// scm info only added for internal builds
internalBuild()
buildFile << """
buildParams.getGitOriginProperty().set("https://some-repo.com/repo.git")
buildParams.setGitOrigin("https://some-repo.com/repo.git")
apply plugin:'elasticsearch.java'
apply plugin:'elasticsearch.publish'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ private static void disableTransitiveDependenciesForSourceSet(Project project, S
public void configureCompile(Project project) {
project.getExtensions().getExtraProperties().set("compactProfile", "full");
JavaPluginExtension java = project.getExtensions().getByType(JavaPluginExtension.class);
if (buildParams.getJavaToolChainSpec().isPresent()) {
if (buildParams.getJavaToolChainSpec().getOrNull() != null) {
java.toolchain(buildParams.getJavaToolChainSpec().get());
}
java.setSourceCompatibility(buildParams.getMinimumRuntimeVersion());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

import org.elasticsearch.gradle.OS;
import org.elasticsearch.gradle.internal.conventions.util.Util;
import org.elasticsearch.gradle.internal.info.BuildParameterExtension;
import org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin;
import org.elasticsearch.gradle.internal.test.ErrorReportingTestListener;
import org.elasticsearch.gradle.internal.test.SimpleCommandLineArgumentProvider;
Expand All @@ -26,7 +25,6 @@
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.file.FileCollection;
import org.gradle.api.plugins.JavaPlugin;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.ProviderFactory;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;
Expand Down Expand Up @@ -55,8 +53,7 @@ public abstract class ElasticsearchTestBasePlugin implements Plugin<Project> {
@Override
public void apply(Project project) {
project.getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
Property<BuildParameterExtension> buildParams = loadBuildParams(project);

var buildParams = loadBuildParams(project);
project.getPluginManager().apply(GradleTestPolicySetupPlugin.class);
// for fips mode check
project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void apply(Project project) {
project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
project.getPlugins().apply(JvmToolchainsPlugin.class);
toolChainService = project.getExtensions().getByType(JavaToolchainService.class);
BuildParameterExtension buildParams = loadBuildParams(project).get();
var buildParams = loadBuildParams(project).get();
Boolean isCi = buildParams.isCi();
buildParams.getBwcVersions().forPreviousUnreleased((BwcVersions.UnreleasedVersionInfo unreleasedVersion) -> {
configureBwcProject(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import org.elasticsearch.gradle.internal.distribution.InternalElasticsearchDistributionTypes;
import org.elasticsearch.gradle.internal.docker.DockerSupportPlugin;
import org.elasticsearch.gradle.internal.docker.DockerSupportService;
import org.elasticsearch.gradle.internal.info.BuildParameterExtension;
import org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin;
import org.elasticsearch.gradle.util.GradleUtils;
import org.gradle.api.GradleException;
Expand Down Expand Up @@ -49,7 +48,7 @@ public void apply(Project project) {
// this is needed for isInternal
project.getRootProject().getPluginManager().apply(GlobalBuildInfoPlugin.class);
project.getRootProject().getPluginManager().apply(DockerSupportPlugin.class);
BuildParameterExtension buildParams = loadBuildParams(project).get();
var buildParams = loadBuildParams(project).get();

DistributionDownloadPlugin distributionDownloadPlugin = project.getPlugins().apply(DistributionDownloadPlugin.class);
Provider<DockerSupportService> dockerSupport = GradleUtils.getBuildService(
Expand All @@ -61,7 +60,7 @@ public void apply(Project project) {
);
registerInternalDistributionResolutions(
DistributionDownloadPlugin.getRegistrationsContainer(project),
buildParams.getBwcVersionsProperty()
buildParams.getBwcVersionsProvider()
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
package org.elasticsearch.gradle.internal;

import org.elasticsearch.gradle.VersionProperties;
import org.elasticsearch.gradle.internal.info.BuildParameterExtension;
import org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin;
import org.elasticsearch.gradle.testclusters.ElasticsearchCluster;
import org.elasticsearch.gradle.testclusters.TestClustersPlugin;
Expand All @@ -26,7 +25,7 @@ public class InternalTestClustersPlugin implements Plugin<Project> {
public void apply(Project project) {
project.getPlugins().apply(InternalDistributionDownloadPlugin.class);
project.getRootProject().getRootProject().getPlugins().apply(GlobalBuildInfoPlugin.class);
BuildParameterExtension buildParams = loadBuildParams(project).get();
var buildParams = loadBuildParams(project).get();
project.getRootProject().getPluginManager().apply(InternalReaperPlugin.class);
TestClustersPlugin testClustersPlugin = project.getPlugins().apply(TestClustersPlugin.class);
testClustersPlugin.setRuntimeJava(buildParams.getRuntimeJavaHome());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,175 +13,58 @@
import org.gradle.api.Action;
import org.gradle.api.JavaVersion;
import org.gradle.api.Task;
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.ProviderFactory;
import org.gradle.jvm.toolchain.JavaToolchainSpec;

import java.io.File;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.List;
import java.util.Random;
import java.util.concurrent.atomic.AtomicReference;

public abstract class BuildParameterExtension {
private final Provider<Boolean> inFipsJvm;
private final Provider<File> runtimeJavaHome;
private final Boolean isRuntimeJavaHomeSet;
private final List<JavaHome> javaVersions;
private final JavaVersion minimumCompilerVersion;
private final JavaVersion minimumRuntimeVersion;
private final JavaVersion gradleJavaVersion;
private final Provider<JavaVersion> runtimeJavaVersion;
private final Provider<? extends Action<JavaToolchainSpec>> javaToolChainSpec;
private final Provider<String> runtimeJavaDetails;
private final String gitRevision;
private transient AtomicReference<ZonedDateTime> buildDate = new AtomicReference<>();
private final String testSeed;
private final Boolean isCi;
private final Integer defaultParallel;
private final Boolean isSnapshotBuild;

public BuildParameterExtension(
ProviderFactory providers,
Provider<File> runtimeJavaHome,
Provider<? extends Action<JavaToolchainSpec>> javaToolChainSpec,
Provider<JavaVersion> runtimeJavaVersion,
boolean isRuntimeJavaHomeSet,
Provider<String> runtimeJavaDetails,
List<JavaHome> javaVersions,
JavaVersion minimumCompilerVersion,
JavaVersion minimumRuntimeVersion,
JavaVersion gradleJavaVersion,
String gitRevision,
String gitOrigin,
ZonedDateTime buildDate,
String testSeed,
boolean isCi,
int defaultParallel,
final boolean isSnapshotBuild,
Provider<BwcVersions> bwcVersions
) {
this.inFipsJvm = providers.systemProperty("tests.fips.enabled").map(BuildParameterExtension::parseBoolean);
this.runtimeJavaHome = runtimeJavaHome;
this.javaToolChainSpec = javaToolChainSpec;
this.runtimeJavaVersion = runtimeJavaVersion;
this.isRuntimeJavaHomeSet = isRuntimeJavaHomeSet;
this.runtimeJavaDetails = runtimeJavaDetails;
this.javaVersions = javaVersions;
this.minimumCompilerVersion = minimumCompilerVersion;
this.minimumRuntimeVersion = minimumRuntimeVersion;
this.gradleJavaVersion = gradleJavaVersion;
this.gitRevision = gitRevision;
this.testSeed = testSeed;
this.isCi = isCi;
this.defaultParallel = defaultParallel;
this.isSnapshotBuild = isSnapshotBuild;
this.getBwcVersionsProperty().set(bwcVersions);
this.getGitOriginProperty().set(gitOrigin);
}

private static boolean parseBoolean(String s) {
if (s == null) {
return false;
}
return Boolean.parseBoolean(s);
}

public boolean getInFipsJvm() {
return inFipsJvm.getOrElse(false);
}

public Provider<File> getRuntimeJavaHome() {
return runtimeJavaHome;
}

public void withFipsEnabledOnly(Task task) {
task.onlyIf("FIPS mode disabled", task1 -> getInFipsJvm() == false);
}

public Boolean getIsRuntimeJavaHomeSet() {
return isRuntimeJavaHomeSet;
}

public List<JavaHome> getJavaVersions() {
return javaVersions;
}

public JavaVersion getMinimumCompilerVersion() {
return minimumCompilerVersion;
}

public JavaVersion getMinimumRuntimeVersion() {
return minimumRuntimeVersion;
}

public JavaVersion getGradleJavaVersion() {
return gradleJavaVersion;
}

public Provider<JavaVersion> getRuntimeJavaVersion() {
return runtimeJavaVersion;
}

public Provider<? extends Action<JavaToolchainSpec>> getJavaToolChainSpec() {
return javaToolChainSpec;
}

public Provider<String> getRuntimeJavaDetails() {
return runtimeJavaDetails;
}

public String getGitRevision() {
return gitRevision;
}

public String getGitOrigin() {
return getGitOriginProperty().get();
}

public ZonedDateTime getBuildDate() {
ZonedDateTime value = buildDate.get();
if (value == null) {
value = ZonedDateTime.now(ZoneOffset.UTC);
if (buildDate.compareAndSet(null, value) == false) {
// If another thread initialized it first, return the initialized value
value = buildDate.get();
}
}
return value;
}

public String getTestSeed() {
return testSeed;
}

public Boolean isCi() {
return isCi;
}

public Integer getDefaultParallel() {
return defaultParallel;
}

public Boolean isSnapshotBuild() {
return isSnapshotBuild;
}

public BwcVersions getBwcVersions() {
return getBwcVersionsProperty().get();
}

public abstract Property<BwcVersions> getBwcVersionsProperty();

public abstract Property<String> getGitOriginProperty();

public Random getRandom() {
return new Random(Long.parseUnsignedLong(testSeed.split(":")[0], 16));
}

public Boolean isGraalVmRuntime() {
return runtimeJavaDetails.get().toLowerCase().contains("graalvm");
}

public interface BuildParameterExtension {
String EXTENSION_NAME = "buildParams";

boolean getInFipsJvm();

Provider<File> getRuntimeJavaHome();

void withFipsEnabledOnly(Task task);

Boolean getIsRuntimeJavaHomeSet();

List<JavaHome> getJavaVersions();

JavaVersion getMinimumCompilerVersion();

JavaVersion getMinimumRuntimeVersion();

JavaVersion getGradleJavaVersion();

Provider<JavaVersion> getRuntimeJavaVersion();

Provider<? extends Action<JavaToolchainSpec>> getJavaToolChainSpec();

Provider<String> getRuntimeJavaDetails();

String getGitRevision();

String getGitOrigin();

ZonedDateTime getBuildDate();

String getTestSeed();

Boolean isCi();

Integer getDefaultParallel();

Boolean isSnapshotBuild();

BwcVersions getBwcVersions();

Provider<BwcVersions> getBwcVersionsProvider();

Random getRandom();

Boolean isGraalVmRuntime();
}
Loading

0 comments on commit 1957078

Please sign in to comment.