Skip to content

Update build to use gradle wrapper 6.8 #65596

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jan 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
package org.elasticsearch.gradle

import org.elasticsearch.gradle.fixtures.AbstractGradleFuncTest
import org.elasticsearch.gradle.transform.SymbolicLinkPreservingUntarTransform
import org.gradle.testkit.runner.TaskOutcome
import spock.lang.Unroll

Expand Down Expand Up @@ -61,17 +60,21 @@ class DistributionDownloadPluginFuncTest extends AbstractGradleFuncTest {
"""

when:
def runner = gradleRunner('clean', 'setupDistro', '-i')
def guh = new File(testProjectDir.getRoot(), "gradle-user-home").absolutePath;
def runner = gradleRunner('clean', 'setupDistro', '-i', '-g', guh)
def result = withMockedDistributionDownload(version, platform, runner) {
// initial run
build()
def firstRun = build()
assertOutputContains(firstRun.output, "Unpacking elasticsearch-${version}-linux-x86_64.tar.gz " +
"using SymbolicLinkPreservingUntarTransform")
// 2nd invocation
build()
}

then:
result.task(":setupDistro").outcome == TaskOutcome.SUCCESS
assertOutputContains(result.output, "Skipping ${SymbolicLinkPreservingUntarTransform.class.simpleName}")
assertOutputMissing(result.output, "Unpacking elasticsearch-${version}-linux-x86_64.tar.gz " +
"using SymbolicLinkPreservingUntarTransform")
}

def "transforms are reused across projects"() {
Expand Down Expand Up @@ -109,7 +112,7 @@ class DistributionDownloadPluginFuncTest extends AbstractGradleFuncTest {
then:
result.tasks.size() == 3
result.output.count("Unpacking elasticsearch-${version}-linux-x86_64.tar.gz " +
"using SymbolicLinkPreservingUntarTransform.") == 1
"using SymbolicLinkPreservingUntarTransform") == 1
}

private boolean assertExtractedDistroCreated(String relativePath) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,4 @@ class ElasticsearchJavaPluginFuncTest extends AbstractGradleFuncTest {
then:
gradleRunner("help").build()
}

private File someJavaSource() {
file("src/main/java/org/acme/SomeClass.java") << """
package org.acme;
public class SomeClass {}
"""
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ class JdkDownloadPluginFuncTest extends AbstractGradleFuncTest {

then:
result.tasks.size() == 3
result.output.count("Unpacking linux-12.0.2-x64.tar.gz using ${SymbolicLinkPreservingUntarTransform.simpleName}.") == 1
result.output.count("Unpacking linux-12.0.2-x64.tar.gz using ${SymbolicLinkPreservingUntarTransform.simpleName}") == 1

where:
platform | jdkVendor | jdkVersion | expectedJavaBin
Expand Down Expand Up @@ -177,18 +177,20 @@ class JdkDownloadPluginFuncTest extends AbstractGradleFuncTest {

def commonGradleUserHome = testProjectDir.newFolder().toString()
// initial run
gradleRunner('clean', 'getJdk', '-g', commonGradleUserHome).build()
def firstResult = gradleRunner('clean', 'getJdk', '-i', '--warning-mode', 'all', '-g', commonGradleUserHome).build()
// assert the output of an executed transform is shown
assertOutputContains(firstResult.output, "Unpacking $expectedArchiveName using $transformType")
// run against up-to-date transformations
gradleRunner('clean', 'getJdk', '-i', '-g', commonGradleUserHome).build()
gradleRunner('clean', 'getJdk', '-i', '--warning-mode', 'all', '-g', commonGradleUserHome).build()
}

then:
assertOutputContains(result.output, "Skipping $transformType")
normalized(result.output).contains("Unpacking $expectedArchiveName using $transformType") == false

where:
platform | transformType
"linux" | SymbolicLinkPreservingUntarTransform.class.simpleName
"windows" | UnzipTransform.class.simpleName
platform | expectedArchiveName | transformType
"linux" | "linux-12.0.2-x64.tar.gz" | SymbolicLinkPreservingUntarTransform.class.simpleName
"windows" | "windows-12.0.2-x64.zip" | UnzipTransform.class.simpleName
}

static boolean assertExtraction(String output, String javaBin) {
Expand Down Expand Up @@ -237,6 +239,7 @@ class JdkDownloadPluginFuncTest extends AbstractGradleFuncTest {
if(repo.name == "jdk_repo_${jdkVendor}_${jdkVersion}") {
repo.setUrl('${server.baseUrl()}')
}
allowInsecureProtocol = true
}
}"""
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@ abstract class AbstractGradleFuncTest extends Specification {

File settingsFile
File buildFile
File propertiesFile

def setup() {
settingsFile = testProjectDir.newFile('settings.gradle')
settingsFile << "rootProject.name = 'hello-world'\n"
buildFile = testProjectDir.newFile('build.gradle')
propertiesFile = testProjectDir.newFile('gradle.properties')
propertiesFile << "org.gradle.java.installations.fromEnv=JAVA_HOME,RUNTIME_JAVA_HOME,JAVA15_HOME,JAVA14_HOME,JAVA13_HOME,JAVA12_HOME,JAVA11_HOME,JAVA8_HOME"
}

GradleRunner gradleRunner(String... arguments) {
Expand All @@ -60,6 +63,10 @@ abstract class AbstractGradleFuncTest extends Specification {
true
}

def assertOutputMissing(String givenOutput, String expected) {
assert normalized(givenOutput).contains(normalized(expected)) == false
true
}
String normalized(String input) {
String normalizedPathPrefix = testProjectDir.root.canonicalPath.replace('\\', '/')
return input.readLines()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ private void runInsecureArtifactRepositoryTest(final String name, final String u
.withProjectDir(tmpDir.getRoot())
.withArguments("clean", "hello", "-s", "-i", "--warning-mode=all", "--scan")
.withPluginClasspath()
.forwardOutput()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep this here or was this only for testing? I'm ok with keeping this around across the board BTW.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep it.

.buildAndFail();

assertOutputContains(
result.getOutput(),
"repository [" + name + "] on project with path [:] is not using a secure protocol for artifacts on [" + url + "]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void testNamingConvention() {
" * org.elasticsearch.gradle.testkit.LooksLikeATestWithoutNamingConvention2",
" * org.elasticsearch.gradle.testkit.LooksLikeATestWithoutNamingConvention3"
);
assertOutputDoesNotContain(result.getOutput(), "LooksLikeTestsButAbstract");
assertOutputMissing(result.getOutput(), "LooksLikeTestsButAbstract");
}

public void testNoEmptyTasks() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ public void testViolationFoundAndCompileOnlyIgnored() {

assertTaskFailed(result, ":absurd");
assertOutputContains(result.getOutput(), "Classes with violations:", " * TestingIO", "> Audit of third party dependencies failed");
assertOutputDoesNotContain(result.getOutput(), "Missing classes:");
assertOutputMissing(result.getOutput(), "Missing classes:");
assertNoDeprecationWarning(result);
}

Expand All @@ -96,7 +96,7 @@ public void testClassNotFoundAndCompileOnlyIgnored() {
" * org.apache.logging.log4j.LogManager",
"> Audit of third party dependencies failed"
);
assertOutputDoesNotContain(result.getOutput(), "Classes with violations:");
assertOutputMissing(result.getOutput(), "Classes with violations:");
assertNoDeprecationWarning(result);
}

Expand All @@ -118,7 +118,7 @@ public void testJarHellWithJDK() {
" Jar Hell with the JDK:",
" * java.lang.String"
);
assertOutputDoesNotContain(result.getOutput(), "Classes with violations:");
assertOutputMissing(result.getOutput(), "Classes with violations:");
assertNoDeprecationWarning(result);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,7 @@

# forcing to use TLS1.2 to avoid failure in vault
# see https://github.com/hashicorp/vault/issues/8750#issuecomment-631236121
systemProp.jdk.tls.client.protocols=TLSv1.2
systemProp.jdk.tls.client.protocols=TLSv1.2

# java homes resolved by environment variables
org.gradle.java.installations.fromEnv=JAVA_HOME,RUNTIME_JAVA_HOME,JAVA15_HOME,JAVA14_HOME,JAVA13_HOME,JAVA12_HOME,JAVA11_HOME,JAVA8_HOME
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@
import org.gradle.api.Project;
import org.gradle.api.logging.Logger;
import org.gradle.api.logging.Logging;
import org.gradle.api.model.ObjectFactory;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.ProviderFactory;
import org.gradle.internal.jvm.Jvm;
import org.gradle.jvm.toolchain.JavaInstallation;
import org.gradle.jvm.toolchain.JavaInstallationRegistry;
import org.gradle.internal.jvm.inspection.JvmInstallationMetadata;
import org.gradle.internal.jvm.inspection.JvmMetadataDetector;
import org.gradle.jvm.toolchain.internal.InstallationLocation;
import org.gradle.jvm.toolchain.internal.SharedJavaInstallationRegistry;
import org.gradle.util.GradleVersion;

import javax.inject.Inject;
Expand All @@ -50,7 +50,6 @@
import java.nio.file.Paths;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Iterator;
Expand All @@ -68,14 +67,18 @@ public class GlobalBuildInfoPlugin implements Plugin<Project> {
private static final String DEFAULT_VERSION_JAVA_FILE_PATH = "server/src/main/java/org/elasticsearch/Version.java";
private static Integer _defaultParallel = null;

private final JavaInstallationRegistry javaInstallationRegistry;
private final ObjectFactory objects;
private final SharedJavaInstallationRegistry javaInstallationRegistry;
private final JvmMetadataDetector metadataDetector;
private final ProviderFactory providers;

@Inject
public GlobalBuildInfoPlugin(JavaInstallationRegistry javaInstallationRegistry, ObjectFactory objects, ProviderFactory providers) {
public GlobalBuildInfoPlugin(
SharedJavaInstallationRegistry javaInstallationRegistry,
JvmMetadataDetector metadataDetector,
ProviderFactory providers
) {
this.javaInstallationRegistry = javaInstallationRegistry;
this.objects = objects;
this.metadataDetector = metadataDetector;
this.providers = providers;
}

Expand Down Expand Up @@ -105,8 +108,8 @@ public void apply(Project project) {
params.setRuntimeJavaHome(runtimeJavaHome);
params.setRuntimeJavaVersion(determineJavaVersion("runtime java.home", runtimeJavaHome, minimumRuntimeVersion));
params.setIsRutimeJavaHomeSet(Jvm.current().getJavaHome().equals(runtimeJavaHome) == false);
params.setRuntimeJavaDetails(getJavaInstallation(runtimeJavaHome).getImplementationName());
params.setJavaVersions(getAvailableJavaVersions(minimumCompilerVersion));
params.setRuntimeJavaDetails(getJavaInstallation(runtimeJavaHome).getDisplayName());
params.setJavaVersions(getAvailableJavaVersions());
params.setMinimumCompilerVersion(minimumCompilerVersion);
params.setMinimumRuntimeVersion(minimumRuntimeVersion);
params.setGradleJavaVersion(Jvm.current().getJavaVersion());
Expand Down Expand Up @@ -146,20 +149,22 @@ private void logGlobalBuildInfo() {
final String osVersion = System.getProperty("os.version");
final String osArch = System.getProperty("os.arch");
final Jvm gradleJvm = Jvm.current();
final String gradleJvmDetails = getJavaInstallation(gradleJvm.getJavaHome()).getImplementationName();

JvmInstallationMetadata gradleJvmMetadata = metadataDetector.getMetadata(gradleJvm.getJavaHome());
final String gradleJvmVendorDetails = gradleJvmMetadata.getVendor().getDisplayName();
LOGGER.quiet("=======================================");
LOGGER.quiet("Elasticsearch Build Hamster says Hello!");
LOGGER.quiet(" Gradle Version : " + GradleVersion.current().getVersion());
LOGGER.quiet(" OS Info : " + osName + " " + osVersion + " (" + osArch + ")");
if (BuildParams.getIsRuntimeJavaHomeSet()) {
String runtimeJvmDetails = getJavaInstallation(BuildParams.getRuntimeJavaHome()).getImplementationName();
LOGGER.quiet(" Runtime JDK Version : " + BuildParams.getRuntimeJavaVersion() + " (" + runtimeJvmDetails + ")");
final String runtimeJvmVendorDetails = metadataDetector.getMetadata(BuildParams.getRuntimeJavaHome())
.getVendor()
.getDisplayName();
LOGGER.quiet(" Runtime JDK Version : " + BuildParams.getRuntimeJavaVersion() + " (" + runtimeJvmVendorDetails + ")");
LOGGER.quiet(" Runtime java.home : " + BuildParams.getRuntimeJavaHome());
LOGGER.quiet(" Gradle JDK Version : " + gradleJvm.getJavaVersion() + " (" + gradleJvmDetails + ")");
LOGGER.quiet(" Gradle JDK Version : " + gradleJvm.getJavaVersion() + " (" + gradleJvmVendorDetails + ")");
LOGGER.quiet(" Gradle java.home : " + gradleJvm.getJavaHome());
} else {
LOGGER.quiet(" JDK Version : " + gradleJvm.getJavaVersion() + " (" + gradleJvmDetails + ")");
LOGGER.quiet(" JDK Version : " + gradleJvm.getJavaVersion() + " (" + gradleJvmVendorDetails + ")");
LOGGER.quiet(" JAVA_HOME : " + gradleJvm.getJavaHome());
}
LOGGER.quiet(" Random Testing Seed : " + BuildParams.getTestSeed());
Expand All @@ -168,8 +173,8 @@ private void logGlobalBuildInfo() {
}

private JavaVersion determineJavaVersion(String description, File javaHome, JavaVersion requiredVersion) {
JavaInstallation installation = getJavaInstallation(javaHome);
JavaVersion actualVersion = installation.getJavaVersion();
InstallationLocation installation = getJavaInstallation(javaHome);
JavaVersion actualVersion = metadataDetector.getMetadata(installation.getLocation()).getLanguageVersion();
if (actualVersion.isCompatibleWith(requiredVersion) == false) {
throwInvalidJavaHomeException(
description,
Expand All @@ -182,46 +187,33 @@ private JavaVersion determineJavaVersion(String description, File javaHome, Java
return actualVersion;
}

private JavaInstallation getJavaInstallation(File javaHome) {
JavaInstallation installation;
if (isCurrentJavaHome(javaHome)) {
installation = javaInstallationRegistry.getInstallationForCurrentVirtualMachine().get();
} else {
installation = javaInstallationRegistry.installationForDirectory(objects.directoryProperty().fileValue(javaHome)).get();
}

return installation;
private InstallationLocation getJavaInstallation(File javaHome) {
return javaInstallationRegistry.listInstallations()
.stream()
.filter(installationLocation -> isSameFile(javaHome, installationLocation))
.findFirst()
.get();
}

private List<JavaHome> getAvailableJavaVersions(JavaVersion minimumCompilerVersion) {
final List<JavaHome> javaVersions = new ArrayList<>();
for (int v = 8; v <= Integer.parseInt(minimumCompilerVersion.getMajorVersion()); v++) {
int version = v;
String javaHomeEnvVarName = getJavaHomeEnvVarName(Integer.toString(version));
if (System.getenv(javaHomeEnvVarName) != null) {
File javaHomeDirectory = new File(findJavaHome(Integer.toString(version)));
Provider<JavaInstallation> javaInstallationProvider = javaInstallationRegistry.installationForDirectory(
objects.directoryProperty().fileValue(javaHomeDirectory)
);
JavaHome javaHome = JavaHome.of(version, providers.provider(() -> {
int actualVersion = Integer.parseInt(javaInstallationProvider.get().getJavaVersion().getMajorVersion());
if (actualVersion != version) {
throwInvalidJavaHomeException("env variable " + javaHomeEnvVarName, javaHomeDirectory, version, actualVersion);
}
return javaHomeDirectory;
}));
javaVersions.add(javaHome);
}
private boolean isSameFile(File javaHome, InstallationLocation installationLocation) {
try {
return Files.isSameFile(installationLocation.getLocation().toPath(), javaHome.toPath());
} catch (IOException ioException) {
throw new UncheckedIOException(ioException);
}
return javaVersions;
}

private static boolean isCurrentJavaHome(File javaHome) {
try {
return Files.isSameFile(javaHome.toPath(), Jvm.current().getJavaHome().toPath());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
/**
* We resolve all available java versions using auto detected by gradles tool chain
* To make transition more reliable we only take env var provided installations into account for now
*/
private List<JavaHome> getAvailableJavaVersions() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bit we lose here is that we no longer enorce that the environment variable name matches the JDK version it points at. I guess this would later fail w/ us not being able to resovle a compatible JDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. is that something you think is worth to invest in? I guess those issues are rare, but when they occur they suck to be debugged I can imagine. Maybe we want to look into this safety net in a follow up?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think realistically what we want to do is remove the reliance on these environment variables and use Gradle's toolchain capabilities to just download a compatible JDK when necessary. These variables are only used for build time purposes, where we intend to run tests with a particular JDK implementation, that's what RUNTIME_JAVA_HOME is for.

So I think we can punt here and instead focus on ditching all usages of these things vs resolving JDKs dynamically.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will look into this in a separate PR. I didn't want to make too many changes in this PR. So RUNTIME_JAVA_HOME is basically used to configure the jdk on CI to run all tests against?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem I see at the moment is that the compatibility choices (which vendor is available etc.) is limited in gradle at the moment and might not be enough. If you say that's less of an issue for build time then the toolchain capabilities in gradle might be enough at the moment to pick a proper one

return javaInstallationRegistry.listInstallations().stream().map(installationLocation -> {
File installationDir = installationLocation.getLocation();
JvmInstallationMetadata metadata = metadataDetector.getMetadata(installationDir);
int actualVersion = Integer.parseInt(metadata.getLanguageVersion().getMajorVersion());
return JavaHome.of(actualVersion, providers.provider(() -> installationDir));
}).collect(Collectors.toList());
}

private static String getTestSeed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ public class JarHellPrecommitPlugin extends PrecommitPlugin {
public TaskProvider<? extends Task> createTask(Project project) {
Configuration jarHellConfig = project.getConfigurations().create("jarHell");
if (BuildParams.isInternal() && project.getPath().equals(":libs:elasticsearch-core") == false) {
// ideally we would configure this as a default dependency. But Default dependencies do not work correctly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine anyway as we don't expect this to be replaced.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other question though. How does this work for external builds? Where does the jarhell implementation come from?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You would need to add a dependency like `jarHell 'org.elasticsearch:elasticsearch-core:7.10' at the moment. In general I think we need to put some more dedicated effort into making these external available plugins work nicely for contributors. By that I mean having proper test coverage of third party plugin author use cases etc. I remember we talked about that very early on when having our first talks about the elastic build already.
We're moving in that direction with having internal plugins nowadays but there's more work required here

// with gradle project dependencies as they're resolved to late in the build and don't setup according task
// dependencies properly
project.getDependencies().add("jarHell", project.project(":libs:elasticsearch-core"));
}
TaskProvider<JarHellTask> jarHell = project.getTasks().register("jarHell", JarHellTask.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
import org.apache.commons.compress.compressors.gzip.GzipCompressorInputStream;
import org.gradle.api.logging.Logging;

import java.io.File;
import java.io.FileInputStream;
Expand All @@ -40,8 +39,6 @@ public abstract class SymbolicLinkPreservingUntarTransform implements UnpackTran
private static final Path CURRENT_DIR_PATH = Paths.get(".");

public void unpack(File tarFile, File targetDir) throws IOException {
Logging.getLogger(SymbolicLinkPreservingUntarTransform.class)
.info("Unpacking " + tarFile.getName() + " using " + SymbolicLinkPreservingUntarTransform.class.getSimpleName() + ".");
Function<String, Path> pathModifier = pathResolver();

TarArchiveInputStream tar = new TarArchiveInputStream(new GzipCompressorInputStream(new FileInputStream(tarFile)));
Expand Down
Loading