From 5c172933eed519b7173bd1a15d4e544ea093b675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3zsef=20Bart=C3=B3k?= Date: Fri, 9 Dec 2022 11:21:33 +0200 Subject: [PATCH] Fix the issue of provisioning the same toolchain multiple times (backport #23024) --- .../internal/JavaInstallationRegistry.java | 39 +++++++- .../internal/JavaToolchainQueryService.java | 37 +++++--- ...chainDownloadComplexProjectSoakTest.groovy | 90 +++++++++++++++++++ .../JavaToolchainDownloadSoakTest.groovy | 5 +- 4 files changed, 153 insertions(+), 18 deletions(-) create mode 100644 subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadComplexProjectSoakTest.groovy diff --git a/subprojects/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/JavaInstallationRegistry.java b/subprojects/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/JavaInstallationRegistry.java index e4b9d4660b97..a541a2e1ea85 100644 --- a/subprojects/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/JavaInstallationRegistry.java +++ b/subprojects/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/JavaInstallationRegistry.java @@ -17,8 +17,6 @@ package org.gradle.jvm.toolchain.internal; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import org.gradle.api.GradleException; import org.gradle.api.logging.Logger; import org.gradle.api.logging.Logging; @@ -37,12 +35,13 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.stream.Collectors; public class JavaInstallationRegistry { private final BuildOperationExecutor executor; - private final Supplier> installations; + private final Installations installations; private final Logger logger; private final OperatingSystem os; @@ -54,7 +53,7 @@ public JavaInstallationRegistry(List suppliers, BuildOpera private JavaInstallationRegistry(List suppliers, Logger logger, BuildOperationExecutor executor, OperatingSystem os) { this.logger = logger; this.executor = executor; - this.installations = Suppliers.memoize(() -> collectInBuildOperation(suppliers)); + this.installations = new Installations(() -> collectInBuildOperation(suppliers)); this.os = os; } @@ -71,6 +70,10 @@ public Set listInstallations() { return installations.get(); } + public void addInstallation(InstallationLocation installation) { + installations.add(installation); + } + private Set collectInstallations(List suppliers) { return suppliers.parallelStream() .map(InstallationSupplier::get) @@ -146,4 +149,32 @@ public BuildOperationDescriptor.Builder description() { } } + private static class Installations { + + private final Supplier> initializer; + + private Set locations = null; + + Installations(Supplier> initializer) { + this.initializer = initializer; + } + + synchronized Set get() { + initIfNeeded(); + return locations; + } + + synchronized void add(InstallationLocation location) { + initIfNeeded(); + locations.add(location); + } + + private void initIfNeeded() { + if (locations == null) { + locations = initializer.get(); + } + } + + } + } diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java index f5ec0db7bcb0..3e26e40c8e76 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java @@ -25,6 +25,8 @@ import org.gradle.api.provider.ProviderFactory; import org.gradle.internal.deprecation.DeprecationLogger; import org.gradle.internal.jvm.Jvm; +import org.gradle.internal.service.scopes.Scopes; +import org.gradle.internal.service.scopes.ServiceScope; import org.gradle.jvm.toolchain.JavaToolchainSpec; import org.gradle.jvm.toolchain.internal.install.DefaultJavaToolchainProvisioningService; import org.gradle.jvm.toolchain.internal.install.JavaToolchainProvisioningService; @@ -36,6 +38,7 @@ import java.util.Objects; import java.util.Optional; +@ServiceScope(Scopes.Project.class) public class JavaToolchainQueryService { private final JavaInstallationRegistry registry; @@ -121,30 +124,42 @@ private JavaToolchain query(JavaToolchainSpec spec) { if (spec instanceof CurrentJvmToolchainSpec) { return asToolchain(new InstallationLocation(Jvm.current().getJavaHome(), "current JVM"), spec).get(); } + if (spec instanceof SpecificInstallationToolchainSpec) { return asToolchain(new InstallationLocation(((SpecificInstallationToolchainSpec) spec).getJavaHome(), "specific installation"), spec).get(); } - return registry.listInstallations().stream() - .map(javaHome -> asToolchain(javaHome, spec)) - .filter(Optional::isPresent) - .map(Optional::get) - .filter(new JavaToolchainMatcher(spec)) - .min(new JavaToolchainComparator()) - .orElseGet(() -> downloadToolchain(spec)); + Optional detectedToolchain = registry.listInstallations().stream() + .map(javaHome -> asToolchain(javaHome, spec)) + .filter(Optional::isPresent) + .map(Optional::get) + .filter(new JavaToolchainMatcher(spec)) + .min(new JavaToolchainComparator()); + + if (detectedToolchain.isPresent()) { + return detectedToolchain.get(); + } + + InstallationLocation downloadedInstallation = downloadToolchain(spec); + JavaToolchain downloadedToolchain = asToolchainOrThrow(downloadedInstallation, spec); + registry.addInstallation(downloadedInstallation); + return downloadedToolchain; } - private JavaToolchain downloadToolchain(JavaToolchainSpec spec) { + private InstallationLocation downloadToolchain(JavaToolchainSpec spec) { final Optional installation = installService.tryInstall(spec); if (!installation.isPresent()) { throw new NoToolchainAvailableException(spec, detectEnabled.getOrElse(true), downloadEnabled.getOrElse(true)); } - Optional toolchain = asToolchain(new InstallationLocation(installation.get(), "provisioned toolchain"), spec); + return new InstallationLocation(installation.get(), "provisioned toolchain"); + } + + private JavaToolchain asToolchainOrThrow(InstallationLocation javaHome, JavaToolchainSpec spec) { + Optional toolchain = asToolchain(javaHome, spec); if (!toolchain.isPresent()) { - throw new GradleException("Provisioned toolchain '" + installation.get() + "' could not be probed."); + throw new GradleException("Toolchain installation '" + javaHome.getLocation() + "' could not be probed."); } - return toolchain.get(); } diff --git a/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadComplexProjectSoakTest.groovy b/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadComplexProjectSoakTest.groovy new file mode 100644 index 000000000000..6b238975f95d --- /dev/null +++ b/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadComplexProjectSoakTest.groovy @@ -0,0 +1,90 @@ +/* + * Copyright 2022 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.gradle.jvm.toolchain + +import org.gradle.integtests.fixtures.AbstractIntegrationSpec + +class JavaToolchainDownloadComplexProjectSoakTest extends AbstractIntegrationSpec { + + def setup() { + executer.requireOwnGradleUserHomeDir() + executer.withToolchainDownloadEnabled() + } + + def "multiple subprojects with identical toolchain definitions"() { + given: + settingsFile << settingsForBuildWithSubprojects() + + setupSubproject("subproject1", "Foo", "ADOPTIUM") + setupSubproject("subproject2", "Bar", "ADOPTIUM") + + when: + result = executer + .withTasks("compileJava") + .run() + + then: + !result.plainTextOutput.matches("(?s).*The existing installation will be replaced by the new download.*") + } + + def "multiple subprojects with different toolchain definitions"() { + given: + settingsFile << settingsForBuildWithSubprojects() + + setupSubproject("subproject1", "Foo", "ADOPTIUM") + setupSubproject("subproject2", "Bar", "ORACLE") + + when: + result = executer + .withTasks("compileJava") + .withArgument("--info") + .run() + + then: + result.plainTextOutput.matches("(?s).*Compiling with toolchain.*adoptium.*") + result.plainTextOutput.matches("(?s).*Compiling with toolchain.*oracle.*") + } + + private String settingsForBuildWithSubprojects() { + return """ + plugins { + id 'org.gradle.toolchains.foojay-resolver-convention' version '0.2' + } + rootProject.name = 'main' + + include('subproject1') + include('subproject2') + """ + } + + private void setupSubproject(String subprojectName, String className, String vendorName) { + file("${subprojectName}/build.gradle") << """ + plugins { + id 'java' + } + + java { + toolchain { + languageVersion = JavaLanguageVersion.of(11) + vendor = JvmVendorSpec.${vendorName} + } + } + """ + file("${subprojectName}/src/main/java/${className}.java") << "public class ${className} {}" + } + +} diff --git a/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy b/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy index 444468b5b1a2..c8bc3cf7c82b 100644 --- a/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy +++ b/subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy @@ -37,14 +37,13 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec { executer.requireOwnGradleUserHomeDir() executer - .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() } def "can download missing jdk automatically"() { when: result = executer - .withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=false") + .withTasks("compileJava") .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + "This behaviour has been deprecated and is scheduled to be removed in Gradle 8.0. " + "In order to declare a repository for java toolchains, you must edit your settings script and add one via the toolchainManagement block. " + @@ -67,7 +66,7 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec { when: result = executer - .withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=false") + .withTasks("compileJava") .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + "This behaviour has been deprecated and is scheduled to be removed in Gradle 8.0. " + "In order to declare a repository for java toolchains, you must edit your settings script and add one via the toolchainManagement block. " +