Skip to content

Commit

Permalink
Merge pull request #23024 Fix the issue of provisioning the same tool…
Browse files Browse the repository at this point in the history
…chain multiple times

Fixes #22796

Co-authored-by: József Bartók <jbartok@gradle.com>
  • Loading branch information
bot-gradle and jbartok committed Dec 8, 2022
2 parents efd2931 + 05ef23c commit 3fb0faa
Show file tree
Hide file tree
Showing 4 changed files with 279 additions and 15 deletions.
Expand Up @@ -18,7 +18,6 @@

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;
Expand All @@ -42,7 +41,7 @@

public class JavaInstallationRegistry {
private final BuildOperationExecutor executor;
private final Supplier<Set<InstallationLocation>> installations;
private final Installations installations;
private final Logger logger;
private final OperatingSystem os;

Expand All @@ -54,7 +53,7 @@ public JavaInstallationRegistry(List<InstallationSupplier> suppliers, BuildOpera
private JavaInstallationRegistry(List<InstallationSupplier> 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;
}

Expand All @@ -71,6 +70,10 @@ public Set<InstallationLocation> listInstallations() {
return installations.get();
}

public void addInstallation(InstallationLocation installation) {
installations.add(installation);
}

private Set<InstallationLocation> collectInstallations(List<InstallationSupplier> suppliers) {
return suppliers.parallelStream()
.map(InstallationSupplier::get)
Expand Down Expand Up @@ -146,4 +149,32 @@ public BuildOperationDescriptor.Builder description() {
}
}

private static class Installations {

private final Supplier<Set<InstallationLocation>> initialiser;

private Set<InstallationLocation> locations = null;

Installations(Supplier<Set<InstallationLocation>> initialiser) {
this.initialiser = initialiser;
}

synchronized Set<InstallationLocation> get() {
initIfNeeded();
return locations;
}

synchronized void add(InstallationLocation location) {
initIfNeeded();
locations.add(location);
}

private void initIfNeeded() {
if (locations == null) {
locations = initialiser.get();
}
}

}

}
Expand Up @@ -27,6 +27,8 @@
import org.gradle.internal.deprecation.DocumentedFailure;
import org.gradle.internal.jvm.Jvm;
import org.gradle.internal.jvm.inspection.JvmInstallationMetadata;
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;
Expand All @@ -38,6 +40,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

@ServiceScope(Scopes.Project.class) //TODO: should be much higher scoped, as many other toolchain related services, but is bogged down by the scope of services it depends on
public class JavaToolchainQueryService {

// A key that matches only the fallback toolchain
Expand Down Expand Up @@ -126,27 +129,36 @@ private JavaToolchain query(JavaToolchainSpec spec, boolean isFallback) {
// TODO (#22023) Look into checking if this optional is present and throwing an exception
return asToolchain(new InstallationLocation(Jvm.current().getJavaHome(), "current JVM"), spec, isFallback).toolchain().get();
}

if (spec instanceof SpecificInstallationToolchainSpec) {
final InstallationLocation installation = new InstallationLocation(((SpecificInstallationToolchainSpec) spec).getJavaHome(), "specific installation");
return asToolchainOrThrow(installation, spec);
}

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<JavaToolchain> 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<File> installation = installService.tryInstall(spec);
if (!installation.isPresent()) {
throw new NoToolchainAvailableException(spec, detectEnabled.getOrElse(true), downloadEnabled.getOrElse(true));
}

return asToolchainOrThrow(new InstallationLocation(installation.get(), "provisioned toolchain"), spec);
return new InstallationLocation(installation.get(), "provisioned toolchain");
}

private Optional<JavaToolchain> asToolchain(InstallationLocation javaHome, JavaToolchainSpec spec) {
Expand Down
@@ -0,0 +1,222 @@
/*
* 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
import spock.lang.Ignore

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} {}"
}

@Ignore("this test doesn't really test anything as is; see TODO in setupIncludedBuild()")
def "included build with different toolchain repository definition"() {
given:
settingsFile << settingsForBuildWithIncludedBuilds()
buildFile << buildConfigForBuildWithIncludedBuilds()

setupIncludedBuild()

when:
result = executer
.withTasks("compileJava")
.withArgument("--info")
.run()

then:
result.plainTextOutput.matches("(?s).*Compiling with toolchain.*")
}

private String settingsForBuildWithIncludedBuilds() {
return """
pluginManagement {
includeBuild 'plugin1'
}
plugins {
id 'org.gradle.toolchains.foojay-resolver-convention' version '0.2'
}
rootProject.name = 'main'
"""
}

private String buildConfigForBuildWithIncludedBuilds() {
return """
plugins {
id 'java'
id 'org.example.plugin1'
}
java {
toolchain {
languageVersion = JavaLanguageVersion.of(11)
}
}
"""
}

private void setupIncludedBuild() {
/*file("plugin1/settings.gradle") << """
abstract class CustomToolchainResolverPlugin implements Plugin<Settings> {
@Inject
protected abstract JavaToolchainResolverRegistry getToolchainResolverRegistry();
void apply(Settings settings) {
settings.getPlugins().apply("jvm-toolchain-management");
JavaToolchainResolverRegistry registry = getToolchainResolverRegistry();
registry.register(CustomToolchainResolver.class);
}
}
import javax.inject.Inject
import java.util.Optional;
abstract class CustomToolchainResolver implements JavaToolchainResolver {
@Override
Optional<JavaToolchainDownload> resolve(JavaToolchainRequest request) {
URI uri = URI.create("https://good_for_nothing.com/");
return Optional.of(JavaToolchainDownload.fromUri(uri));
}
}
apply plugin: CustomToolchainResolverPlugin
toolchainManagement {
jvm {
javaRepositories {
repository('custom') {
resolverClass = CustomToolchainResolver
}
}
}
}
rootProject.name = 'plugin1'
"""*/ //TODO: atm the included build will use the definition from its own settings file, so if this is the settings we use it won't be able to download toolchains; need to clarify if this ok in the long term
file("plugin1/settings.gradle") << """
plugins {
id 'org.gradle.toolchains.foojay-resolver-convention' version '0.2'
}
rootProject.name = 'plugin1'
"""
file("plugin1/build.gradle") << """
plugins {
id 'java-gradle-plugin'
}
java {
toolchain {
languageVersion = JavaLanguageVersion.of(11)
}
}
gradlePlugin {
plugins {
simplePlugin {
id = 'org.example.plugin1'
implementationClass = 'org.example.Plugin1'
}
}
}
"""
file("plugin1/src/main/java/Plugin1.java") << """
package org.example;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
public class Plugin1 implements Plugin<Project> {
@Override
public void apply(Project project) {
System.out.println("Plugin1 applied!");
}
}
"""
}
}
Expand Up @@ -46,14 +46,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")
.run()

then:
Expand All @@ -72,7 +71,7 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec {

when:
result = executer
.withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=false")
.withTasks("compileJava")
.run()

then:
Expand Down

0 comments on commit 3fb0faa

Please sign in to comment.