Fix the issue of provisioning the same toolchain multiple times

subprojects/jvm-services/src/main/java/org/gradle/jvm/toolchain/internal/JavaInstallationRegistry.java

@@ -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;
@@ -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;
 
@@ -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;
     }
 
@@ -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)
@@ -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();
+            }
+        }
+
+    }
+
 }

subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java

@@ -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;
@@ -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
@@ -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) {

subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadComplexProjectSoakTest.groovy

@@ -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!");
+                }
+            }
+        """
+    }
+}

subprojects/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy

@@ -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:
@@ -72,7 +71,7 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec {
 
         when:
         result = executer
-               .withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=false")
+               .withTasks("compileJava")
                .run()
 
         then: