From 2fb3e08dda61fa3ab39fb353f4b063e66e7c26fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B3zsef=20Bart=C3=B3k?= Date: Tue, 17 Jan 2023 14:47:45 +0200 Subject: [PATCH] Change how deprecation message is triggered --- ...avaToolchainDownloadIntegrationTest.groovy | 28 ++++++++-------- ...ToolchainDownloadSpiIntegrationTest.groovy | 2 +- .../internal/JavaToolchainQueryService.java | 22 ++++++++++--- ...faultJavaToolchainProvisioningService.java | 20 +++++------ .../JavaToolchainProvisioningService.java | 3 ++ .../JavaToolchainQueryServiceTest.groovy | 33 +++++++++++++++++++ .../JavaToolchainDownloadSoakTest.groovy | 32 ++++++++++++++++-- 7 files changed, 108 insertions(+), 32 deletions(-) diff --git a/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadIntegrationTest.groovy b/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadIntegrationTest.groovy index dc8ac783a409..440fbc00b0a0 100644 --- a/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadIntegrationTest.groovy +++ b/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadIntegrationTest.groovy @@ -44,7 +44,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .withTasks("compileJava") .requireOwnGradleUserHomeDir() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -84,7 +84,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .requireOwnGradleUserHomeDir() .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -152,7 +152,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .requireOwnGradleUserHomeDir() .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -191,7 +191,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() .expectDeprecationWarning('Due to changes in AdoptOpenJDK download endpoint, downloading a JDK with an explicit vendor of AdoptOpenJDK should be replaced with a spec without a vendor or using Eclipse Temurin / IBM Semeru.') - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -228,7 +228,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .requireOwnGradleUserHomeDir() .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -265,7 +265,7 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { .requireOwnGradleUserHomeDir() .withToolchainDetectionEnabled() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -281,26 +281,26 @@ class JavaToolchainDownloadIntegrationTest extends AbstractIntegrationSpec { private static String os() { OperatingSystem os = OperatingSystem.current() if (os.isWindows()) { - return "windows"; + return "windows" } else if (os.isMacOsX()) { - return "mac"; + return "mac" } else if (os.isLinux()) { - return "linux"; + return "linux" } - return os.getFamilyName(); + return os.getFamilyName() } private static String architecture() { SystemInfo systemInfo = NativeServices.getInstance().get(SystemInfo.class) switch (systemInfo.architecture) { case SystemInfo.Architecture.i386: - return "x32"; + return "x32" case SystemInfo.Architecture.amd64: - return "x64"; + return "x64" case SystemInfo.Architecture.aarch64: - return "aarch64"; + return "aarch64" default: - return "unknown"; + return "unknown" } } diff --git a/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSpiIntegrationTest.groovy b/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSpiIntegrationTest.groovy index 945978777cd6..2c8015d4b574 100644 --- a/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSpiIntegrationTest.groovy +++ b/subprojects/platform-jvm/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSpiIntegrationTest.groovy @@ -172,7 +172,7 @@ class JavaToolchainDownloadSpiIntegrationTest extends AbstractJavaToolchainDownl .withTasks("compileJava") .requireOwnGradleUserHomeDir() .withToolchainDownloadEnabled() - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") 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 3e26e40c8e76..52b2005c0cef 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 @@ -129,12 +129,14 @@ private JavaToolchain query(JavaToolchainSpec spec) { return asToolchain(new InstallationLocation(((SpecificInstallationToolchainSpec) spec).getJavaHome(), "specific installation"), spec).get(); } + warnIfAutoProvisioningOnWithoutRepositoryDefinitions(); + Optional detectedToolchain = registry.listInstallations().stream() - .map(javaHome -> asToolchain(javaHome, spec)) - .filter(Optional::isPresent) - .map(Optional::get) - .filter(new JavaToolchainMatcher(spec)) - .min(new JavaToolchainComparator()); + .map(javaHome -> asToolchain(javaHome, spec)) + .filter(Optional::isPresent) + .map(Optional::get) + .filter(new JavaToolchainMatcher(spec)) + .min(new JavaToolchainComparator()); if (detectedToolchain.isPresent()) { return detectedToolchain.get(); @@ -146,6 +148,16 @@ private JavaToolchain query(JavaToolchainSpec spec) { return downloadedToolchain; } + private void warnIfAutoProvisioningOnWithoutRepositoryDefinitions() { + if (installService.isAutoDownloadEnabled() && !installService.hasConfiguredToolchainRepositories()) { + DeprecationLogger.deprecateBehaviour("Java toolchain auto-provisioning enabled, but no java toolchain repositories declared by the build. Will rely on the built-in repository.") + .withAdvice("In order to declare a repository for java toolchains, you must edit your settings script and add one via the toolchainManagement block.") + .willBeRemovedInGradle8() + .withUserManual("toolchains", "sec:provisioning") + .nagUser(); + } + } + private InstallationLocation downloadToolchain(JavaToolchainSpec spec) { final Optional installation = installService.tryInstall(spec); if (!installation.isPresent()) { diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/DefaultJavaToolchainProvisioningService.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/DefaultJavaToolchainProvisioningService.java index 9d104a633ab3..50a7b9b12ba5 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/DefaultJavaToolchainProvisioningService.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/DefaultJavaToolchainProvisioningService.java @@ -23,7 +23,6 @@ import org.gradle.cache.FileLock; import org.gradle.jvm.toolchain.JavaToolchainDownload; import org.gradle.platform.BuildPlatform; -import org.gradle.internal.deprecation.DeprecationLogger; import org.gradle.internal.exceptions.Contextual; import org.gradle.internal.operations.BuildOperationContext; import org.gradle.internal.operations.BuildOperationDescriptor; @@ -90,6 +89,16 @@ public DefaultJavaToolchainProvisioningService( this.buildPlatform = buildPlatform; } + @Override + public boolean isAutoDownloadEnabled() { + return downloadEnabled.getOrElse(true); + } + + @Override + public boolean hasConfiguredToolchainRepositories() { + return !toolchainResolverRegistry.requestedRepositories().isEmpty(); + } + public Optional tryInstall(JavaToolchainSpec spec) { if (!isAutoDownloadEnabled()) { return Optional.empty(); @@ -99,11 +108,6 @@ public Optional tryInstall(JavaToolchainSpec spec) { DefaultJavaToolchainRequest toolchainRequest = new DefaultJavaToolchainRequest(spec, buildPlatform); if (repositories.isEmpty()) { - DeprecationLogger.deprecateBehaviour("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository.") - .withAdvice("In order to declare a repository for java toolchains, you must edit your settings script and add one via the toolchainManagement block.") - .willBeRemovedInGradle8() - .withUserManual("toolchains", "sec:provisioning") - .nagUser(); Optional download = openJdkBinary.resolve(toolchainRequest); if (download.isPresent()) { return Optional.of(provisionInstallation(spec, download.get().getUri(), Collections.emptyList())); @@ -157,10 +161,6 @@ private String getFileName(URI uri, ExternalResource resource) { return fileName; } - private boolean isAutoDownloadEnabled() { - return downloadEnabled.getOrElse(true); - } - private T wrapInOperation(String displayName, Callable provisioningStep) { return buildOperationExecutor.call(new ToolchainProvisioningBuildOperation<>(displayName, provisioningStep)); } diff --git a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/JavaToolchainProvisioningService.java b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/JavaToolchainProvisioningService.java index eba85a54c87c..32e4e9e35c57 100644 --- a/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/JavaToolchainProvisioningService.java +++ b/subprojects/platform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/install/JavaToolchainProvisioningService.java @@ -25,4 +25,7 @@ public interface JavaToolchainProvisioningService { Optional tryInstall(JavaToolchainSpec spec); + boolean isAutoDownloadEnabled(); + + boolean hasConfiguredToolchainRepositories(); } diff --git a/subprojects/platform-jvm/src/test/groovy/org/gradle/jvm/toolchain/internal/JavaToolchainQueryServiceTest.groovy b/subprojects/platform-jvm/src/test/groovy/org/gradle/jvm/toolchain/internal/JavaToolchainQueryServiceTest.groovy index 2f4561152ca7..ececcde638c5 100644 --- a/subprojects/platform-jvm/src/test/groovy/org/gradle/jvm/toolchain/internal/JavaToolchainQueryServiceTest.groovy +++ b/subprojects/platform-jvm/src/test/groovy/org/gradle/jvm/toolchain/internal/JavaToolchainQueryServiceTest.groovy @@ -249,6 +249,17 @@ class JavaToolchainQueryServiceTest extends Specification { def toolchainFactory = newToolchainFactory() def installed = false def provisionService = new JavaToolchainProvisioningService() { + @Override + boolean isAutoDownloadEnabled() { + return true + } + + @Override + boolean hasConfiguredToolchainRepositories() { + return true + } + + @Override Optional tryInstall(JavaToolchainSpec spec) { installed = true Optional.of(new File("/path/12")) @@ -272,6 +283,17 @@ class JavaToolchainQueryServiceTest extends Specification { def toolchainFactory = newToolchainFactory() def installed = false def provisionService = new JavaToolchainProvisioningService() { + @Override + boolean isAutoDownloadEnabled() { + return true + } + + @Override + boolean hasConfiguredToolchainRepositories() { + return true + } + + @Override Optional tryInstall(JavaToolchainSpec spec) { installed = true Optional.of(new File("/path/12.broken")) @@ -296,6 +318,17 @@ class JavaToolchainQueryServiceTest extends Specification { def toolchainFactory = newToolchainFactory() int installed = 0 def provisionService = new JavaToolchainProvisioningService() { + @Override + boolean isAutoDownloadEnabled() { + return true + } + + @Override + boolean hasConfiguredToolchainRepositories() { + return true + } + + @Override Optional tryInstall(JavaToolchainSpec spec) { installed++ Optional.of(new File("/path/12")) 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 c8bc3cf7c82b..aaca49d7373e 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 @@ -44,7 +44,7 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec { when: result = executer .withTasks("compileJava") - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -67,7 +67,7 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec { when: result = executer .withTasks("compileJava") - .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning needed, but no java toolchain repositories declared by the build. Will rely on the built-in repository. " + + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") @@ -78,6 +78,34 @@ class JavaToolchainDownloadSoakTest extends AbstractIntegrationSpec { assertJdkWasDownloaded("openj9") } + def "will get deprecation message for no configured repositories even when not downloading"() { + when: "build runs and doesn't have a local JDK to use for compilation" + result = executer + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") + .withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=false") + .run() + + then: "suitable JDK gets auto-provisioned" + javaClassFile("Foo.class").assertExists() + assertJdkWasDownloaded("adoptopenjdk") + + when: "build has no toolchain repositories configured" + settingsFile.text = '' + + then: "build runs again, uses previously auto-provisioned toolchain and warns about toolchain repositories not being configured" + executer + .expectDocumentedDeprecationWarning("Java toolchain auto-provisioning enabled, 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. " + + "See https://docs.gradle.org/current/userguide/toolchains.html#sec:provisioning for more details.") + .withTasks("compileJava", "-Porg.gradle.java.installations.auto-detect=true", "-Porg.gradle.java.installations.auto-download=true") + .run() + } + + private void assertJdkWasDownloaded(String implementation) { assert executer.gradleUserHomeDir.file("jdks").listFiles({ file -> file.name.contains("-14-") && file.name.contains(implementation)