Skip to content

Commit

Permalink
[Gradle] Remove static use of BuildParams (elastic#115122)
Browse files Browse the repository at this point in the history
Static fields dont do well in Gradle with configuration cache enabled.

- Use buildParams extension in build scripts
- Keep BuildParams.ci for now for easy serverless migration
-  Tweak testing doc
  • Loading branch information
breskeby authored Nov 15, 2024
1 parent 4d33849 commit 13c8aae
Show file tree
Hide file tree
Showing 185 changed files with 965 additions and 615 deletions.
8 changes: 4 additions & 4 deletions TESTING.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ You can run a group of YAML test by using wildcards:
--tests "org.elasticsearch.test.rest.ClientYamlTestSuiteIT.test {yaml=index/*/*}"
---------------------------------------------------------------------------

or
or

---------------------------------------------------------------------------
./gradlew :rest-api-spec:yamlRestTest \
Expand Down Expand Up @@ -564,8 +564,8 @@ Sometimes a backward compatibility change spans two versions.
A common case is a new functionality that needs a BWC bridge in an unreleased versioned of a release branch (for example, 5.x).
Another use case, since the introduction of serverless, is to test BWC against main in addition to the other released branches.
To do so, specify the `bwc.refspec` remote and branch to use for the BWC build as `origin/main`.
To test against main, you will also need to create a new version in link:./server/src/main/java/org/elasticsearch/Version.java[Version.java],
increment `elasticsearch` in link:./build-tools-internal/version.properties[version.properties], and hard-code the `project.version` for ml-cpp
To test against main, you will also need to create a new version in link:./server/src/main/java/org/elasticsearch/Version.java[Version.java],
increment `elasticsearch` in link:./build-tools-internal/version.properties[version.properties], and hard-code the `project.version` for ml-cpp
in link:./x-pack/plugin/ml/build.gradle[ml/build.gradle].

In general, to test the changes, you can instruct Gradle to build the BWC version from another remote/branch combination instead of pulling the release branch from GitHub.
Expand Down Expand Up @@ -625,7 +625,7 @@ For specific YAML rest tests one can use
For disabling entire types of tests for subprojects, one can use for example:

------------------------------------------------
if (BuildParams.inFipsJvm){
if (buildParams.inFipsJvm) {
// This test cluster is using a BASIC license and FIPS 140 mode is not supported in BASIC
tasks.named("javaRestTest").configure{enabled = false }
}
Expand Down
3 changes: 1 addition & 2 deletions benchmarks/build.gradle
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import org.elasticsearch.gradle.internal.info.BuildParams
import org.elasticsearch.gradle.internal.test.TestUtil

/*
Expand Down Expand Up @@ -78,7 +77,7 @@ tasks.register("copyPainless", Copy) {
}

tasks.named("run").configure {
executable = "${BuildParams.runtimeJavaHome}/bin/java"
executable = "${buildParams.runtimeJavaHome.get()}/bin/java"
args << "-Dplugins.dir=${buildDir}/plugins" << "-Dtests.index=${buildDir}/index"
dependsOn "copyExpression", "copyPainless", configurations.nativeLib
systemProperty 'es.nativelibs.path', TestUtil.getTestLibraryPath(file("../libs/native/libraries/build/platform/").toString())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,12 @@ public abstract class GUtils {
public static String capitalize(String s) {
return s.substring(0, 1).toUpperCase(Locale.ROOT) + s.substring(1);
}

public static <T> T elvis(T given, T fallback) {
if (given == null) {
return fallback;
} else {
return given;
}
}
}
11 changes: 7 additions & 4 deletions build-tools-internal/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -386,10 +386,13 @@ tasks.named("jar") {

spotless {
java {
// IDEs can sometimes run annotation processors that leave files in
// here, causing Spotless to complain. Even though this path ought not
// to exist, exclude it anyway in order to avoid spurious failures.
toggleOffOn()

// workaround for https://github.com/diffplug/spotless/issues/2317
//toggleOffOn()
target project.fileTree("src/main/java") {
include '**/*.java'
exclude '**/DockerBase.java'
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,10 @@

package org.elasticsearch.gradle.internal

import org.elasticsearch.gradle.Architecture
import org.elasticsearch.gradle.fixtures.AbstractGitAwareGradleFuncTest
import org.gradle.testkit.runner.TaskOutcome
import spock.lang.IgnoreIf
import spock.lang.Unroll

/*
* Test is ignored on ARM since this test case tests the ability to build certain older BWC branches that we don't support on ARM
*/
@IgnoreIf({ Architecture.current() == Architecture.AARCH64 })
class InternalDistributionBwcSetupPluginFuncTest extends AbstractGitAwareGradleFuncTest {

def setup() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,7 @@ class PublishPluginFuncTest extends AbstractGradleFuncTest {
// scm info only added for internal builds
internalBuild()
buildFile << """
BuildParams.init { it.setGitOrigin("https://some-repo.com/repo.git") }
buildParams.getGitOriginProperty().set("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 @@ -161,7 +161,7 @@ class SnykDependencyMonitoringGradlePluginFuncTest extends AbstractGradleInterna
},
"target": {
"remoteUrl": "http://acme.org",
"branch": "unknown"
"branch": "$version"
},
"targetReference": "$version",
"projectAttributes": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
package org.elasticsearch.gradle.internal.test.rest

import spock.lang.IgnoreIf
import spock.lang.IgnoreRest

import org.elasticsearch.gradle.VersionProperties
import org.elasticsearch.gradle.fixtures.AbstractRestResourcesFuncTest
Expand All @@ -20,16 +21,16 @@ import org.gradle.testkit.runner.TaskOutcome
class LegacyYamlRestTestPluginFuncTest extends AbstractRestResourcesFuncTest {

def setup() {
configurationCacheCompatible = true
buildApiRestrictionsDisabled = true
}


def "yamlRestTest does nothing when there are no tests"() {
given:
internalBuild()
buildFile << """
plugins {
id 'elasticsearch.legacy-yaml-rest-test'
}
apply plugin: 'elasticsearch.legacy-yaml-rest-test'
"""

when:
Expand Down Expand Up @@ -136,7 +137,7 @@ class LegacyYamlRestTestPluginFuncTest extends AbstractRestResourcesFuncTest {
"""

when:
def result = gradleRunner("yamlRestTest", "--console", 'plain', '--stacktrace').buildAndFail()
def result = gradleRunner("yamlRestTest", "--console", 'plain').buildAndFail()

then:
result.task(":distribution:archives:integ-test-zip:buildExpanded").outcome == TaskOutcome.SUCCESS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import java.time.LocalDateTime;

import org.elasticsearch.gradle.Architecture
import org.elasticsearch.gradle.OS
import org.elasticsearch.gradle.internal.info.BuildParams

import java.lang.management.ManagementFactory
import java.time.LocalDateTime
Expand All @@ -34,12 +33,15 @@ develocity {
publishing.onlyIf { false }
}

def fips = buildParams.inFipsJvm
def gitRevision = buildParams.gitRevision

background {
tag OS.current().name()
tag Architecture.current().name()

// Tag if this build is run in FIPS mode
if (BuildParams.inFipsJvm) {
if (fips) {
tag 'FIPS'
}

Expand Down Expand Up @@ -92,8 +94,8 @@ develocity {
link 'Source', "${prBaseUrl}/tree/${System.getenv('BUILDKITE_COMMIT')}"
link 'Pull Request', "https://github.com/${repository}/pull/${prId}"
} else {
value 'Git Commit ID', BuildParams.gitRevision
link 'Source', "https://github.com/${repository}/tree/${BuildParams.gitRevision}"
value 'Git Commit ID', gitRevision
link 'Source', "https://github.com/${repository}/tree/${gitRevision}"
}

buildFinished { result ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

import org.elasticsearch.gradle.Version
import org.elasticsearch.gradle.internal.ElasticsearchTestBasePlugin
import org.elasticsearch.gradle.internal.info.BuildParams
import org.elasticsearch.gradle.internal.test.rest.InternalJavaRestTestPlugin
import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask

Expand All @@ -19,7 +18,7 @@ ext.bwcTaskName = { Version version ->

def bwcTestSnapshots = tasks.register("bwcTestSnapshots") {
if (project.bwc_tests_enabled) {
dependsOn tasks.matching { task -> BuildParams.bwcVersions.unreleased.any { version -> bwcTaskName(version) == task.name } }
dependsOn tasks.matching { task -> buildParams.bwcVersions.unreleased.any { version -> bwcTaskName(version) == task.name } }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ import org.elasticsearch.gradle.testclusters.StandaloneRestIntegTestTask
import org.elasticsearch.gradle.testclusters.TestClustersAware
import org.elasticsearch.gradle.testclusters.TestDistribution

// Common config when running with a FIPS-140 runtime JVM
if (BuildParams.inFipsJvm) {
//apply plugin: org.elasticsearch.gradle.internal.info.GlobalBuildInfoPlugin

// Common config when running with a FIPS-140 runtime JVM
if (buildParams.inFipsJvm) {
allprojects {
String javaSecurityFilename = BuildParams.runtimeJavaDetails.toLowerCase().contains('oracle') ? 'fips_java_oracle.security' : 'fips_java.security'
String javaSecurityFilename = buildParams.runtimeJavaDetails.toLowerCase().contains('oracle') ? 'fips_java_oracle.security' : 'fips_java.security'
File fipsResourcesDir = new File(project.buildDir, 'fips-resources')
File fipsSecurity = new File(fipsResourcesDir, javaSecurityFilename)
File fipsPolicy = new File(fipsResourcesDir, 'fips_java.policy')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ if (providers.systemProperty('idea.active').getOrNull() == 'true') {
idea {
project {
vcs = 'Git'
jdkName = BuildParams.minimumCompilerVersion.majorVersion
jdkName = buildParams.minimumCompilerVersion.majorVersion

settings {
delegateActions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import org.elasticsearch.gradle.Architecture
import org.elasticsearch.gradle.OS
import org.elasticsearch.gradle.VersionProperties
import org.elasticsearch.gradle.internal.info.BuildParams
import org.elasticsearch.gradle.internal.precommit.ThirdPartyAuditPrecommitPlugin
import org.elasticsearch.gradle.internal.precommit.ThirdPartyAuditTask
import org.elasticsearch.gradle.internal.test.rest.RestTestBasePlugin
Expand All @@ -27,8 +26,8 @@ configure(allprojects) {
JvmVendorSpec.matching(VersionProperties.bundledJdkVendor)
}
project.tasks.withType(Test).configureEach { Test test ->
if (BuildParams.getIsRuntimeJavaHomeSet()) {
test.executable = "${BuildParams.runtimeJavaHome}/bin/java" +
if (buildParams.getIsRuntimeJavaHomeSet()) {
test.executable = "${buildParams.runtimeJavaHome.get()}/bin/java" +
(OS.current() == OS.WINDOWS ? '.exe' : '')
} else {
test.javaLauncher = javaToolchains.launcherFor {
Expand All @@ -41,7 +40,7 @@ configure(allprojects) {
}
project.plugins.withId("elasticsearch.testclusters") { testClustersPlugin ->
project.plugins.withId("elasticsearch.internal-testclusters") { internalPlugin ->
if (BuildParams.getIsRuntimeJavaHomeSet() == false) {
if (buildParams.getIsRuntimeJavaHomeSet() == false) {
// If no runtime java home is set, use the bundled JDK for test clusters
testClustersPlugin.setRuntimeJava(launcher.map { it.metadata.installationPath.asFile })
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ dependencies {
newJar project(":libs:${project.name}")
}

BuildParams.bwcVersions.withIndexCompatible({ it.onOrAfter(Version.fromString(ext.stableApiSince))
buildParams.bwcVersions.withIndexCompatible({ it.onOrAfter(Version.fromString(ext.stableApiSince))
&& it != VersionProperties.elasticsearchVersion
}) { bwcVersion, baseName ->

BwcVersions.UnreleasedVersionInfo unreleasedVersion = BuildParams.bwcVersions.unreleasedInfo(bwcVersion)
BwcVersions.UnreleasedVersionInfo unreleasedVersion = buildParams.bwcVersions.unreleasedInfo(bwcVersion)

configurations {
"oldJar${baseName}" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import groovy.lang.Closure;

import org.elasticsearch.gradle.internal.conventions.util.Util;
import org.elasticsearch.gradle.internal.info.BuildParams;
import org.elasticsearch.gradle.internal.info.BuildParameterExtension;
import org.elasticsearch.gradle.internal.precommit.JarHellPrecommitPlugin;
import org.elasticsearch.gradle.internal.test.HistoricalFeaturesMetadataPlugin;
import org.elasticsearch.gradle.plugin.PluginBuildPlugin;
Expand All @@ -39,6 +39,7 @@ public void apply(Project project) {
project.getPluginManager().apply(JarHellPrecommitPlugin.class);
project.getPluginManager().apply(ElasticsearchJavaPlugin.class);
project.getPluginManager().apply(HistoricalFeaturesMetadataPlugin.class);
boolean isCi = project.getRootProject().getExtensions().getByType(BuildParameterExtension.class).isCi();
// Clear default dependencies added by public PluginBuildPlugin as we add our
// own project dependencies for internal builds
// TODO remove once we removed default dependencies from PluginBuildPlugin
Expand All @@ -54,7 +55,7 @@ public void apply(Project project) {
.set("addQaCheckDependencies", new Closure<Project>(BaseInternalPluginBuildPlugin.this, BaseInternalPluginBuildPlugin.this) {
public void doCall(Project proj) {
// This is only a convenience for local developers so make this a noop when running in CI
if (BuildParams.isCi() == false) {
if (isCi == false) {
proj.afterEvaluate(project1 -> {
// let check depend on check tasks of qa sub-projects
final var checkTaskProvider = project1.getTasks().named("check");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import org.elasticsearch.gradle.LoggedExec;
import org.elasticsearch.gradle.OS;
import org.elasticsearch.gradle.Version;
import org.elasticsearch.gradle.internal.info.BuildParams;
import org.gradle.api.Action;
import org.gradle.api.GradleException;
import org.gradle.api.Project;
Expand Down Expand Up @@ -47,6 +46,7 @@ public class BwcSetupExtension {
private final ProviderFactory providerFactory;
private final JavaToolchainService toolChainService;
private final Provider<BwcVersions.UnreleasedVersionInfo> unreleasedVersionInfo;
private final Boolean isCi;

private Provider<File> checkoutDir;

Expand All @@ -56,14 +56,16 @@ public BwcSetupExtension(
ProviderFactory providerFactory,
JavaToolchainService toolChainService,
Provider<BwcVersions.UnreleasedVersionInfo> unreleasedVersionInfo,
Provider<File> checkoutDir
Provider<File> checkoutDir,
Boolean isCi
) {
this.project = project;
this.objectFactory = objectFactory;
this.providerFactory = providerFactory;
this.toolChainService = toolChainService;
this.unreleasedVersionInfo = unreleasedVersionInfo;
this.checkoutDir = checkoutDir;
this.isCi = isCi;
}

TaskProvider<LoggedExec> bwcTask(String name, Action<LoggedExec> configuration) {
Expand All @@ -80,7 +82,8 @@ TaskProvider<LoggedExec> bwcTask(String name, Action<LoggedExec> configuration,
toolChainService,
name,
configuration,
useUniqueUserHome
useUniqueUserHome,
isCi
);
}

Expand All @@ -93,7 +96,8 @@ private static TaskProvider<LoggedExec> createRunBwcGradleTask(
JavaToolchainService toolChainService,
String name,
Action<LoggedExec> configAction,
boolean useUniqueUserHome
boolean useUniqueUserHome,
boolean isCi
) {
return project.getTasks().register(name, LoggedExec.class, loggedExec -> {
loggedExec.dependsOn("checkoutBwcBranch");
Expand All @@ -104,7 +108,7 @@ private static TaskProvider<LoggedExec> createRunBwcGradleTask(
spec.getParameters().getCheckoutDir().set(checkoutDir);
}).flatMap(s -> getJavaHome(objectFactory, toolChainService, Integer.parseInt(s))));

if (BuildParams.isCi() && OS.current() != OS.WINDOWS) {
if (isCi && OS.current() != OS.WINDOWS) {
// TODO: Disabled for now until we can figure out why files are getting corrupted
// loggedExec.getEnvironment().put("GRADLE_RO_DEP_CACHE", System.getProperty("user.home") + "/gradle_ro_cache");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.gradle.Version;
import org.elasticsearch.gradle.VersionProperties;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
Expand Down Expand Up @@ -60,15 +61,16 @@
* We are then able to map the unreleased version to branches in git and Gradle projects that are capable of checking
* out and building them, so we can include these in the testing plan as well.
*/
public class BwcVersions {

public class BwcVersions implements Serializable {

private static final Pattern LINE_PATTERN = Pattern.compile(
"\\W+public static final Version V_(\\d+)_(\\d+)_(\\d+)(_alpha\\d+|_beta\\d+|_rc\\d+)?.*\\);"
);
private static final String GLIBC_VERSION_ENV_VAR = "GLIBC_VERSION";

private final Version currentVersion;
private final List<Version> versions;
private final transient List<Version> versions;
private final Map<Version, UnreleasedVersionInfo> unreleased;

public BwcVersions(List<String> versionLines) {
Expand Down
Loading

0 comments on commit 13c8aae

Please sign in to comment.