Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Improve Toolchain related deprecation nagging in 7.6 #23560

Merged
merged 1 commit into from Jan 18, 2023
Merged

Improve Toolchain related deprecation nagging in 7.6 #23560

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 14 additions & 14 deletions ...src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadIntegrationTest.groovy
View file
Open in desktop
Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand Down Expand Up @@ -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.")
Expand All @@ -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"
}
}

Expand Down
2 changes: 1 addition & 1 deletion .../integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSpiIntegrationTest.groovy
View file
Open in desktop
Expand Up @@ -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.")
Expand Down
22 changes: 17 additions & 5 deletions ...atform-jvm/src/main/java/org/gradle/jvm/toolchain/internal/JavaToolchainQueryService.java
View file
Open in desktop
Expand Up @@ -129,12 +129,14 @@ private JavaToolchain query(JavaToolchainSpec spec) {
return asToolchain(new InstallationLocation(((SpecificInstallationToolchainSpec) spec).getJavaHome(), "specific installation"), spec).get();
}

warnIfAutoProvisioningOnWithoutRepositoryDefinitions();

Optional<JavaToolchain> 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();
Expand All @@ -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<File> installation = installService.tryInstall(spec);
if (!installation.isPresent()) {
Expand Down
20 changes: 10 additions & 10 deletions ...va/org/gradle/jvm/toolchain/internal/install/DefaultJavaToolchainProvisioningService.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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<File> tryInstall(JavaToolchainSpec spec) {
if (!isAutoDownloadEnabled()) {
return Optional.empty();
Expand All @@ -99,11 +108,6 @@ public Optional<File> 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<JavaToolchainDownload> download = openJdkBinary.resolve(toolchainRequest);
if (download.isPresent()) {
return Optional.of(provisionInstallation(spec, download.get().getUri(), Collections.emptyList()));
Expand Down Expand Up @@ -157,10 +161,6 @@ private String getFileName(URI uri, ExternalResource resource) {
return fileName;
}

private boolean isAutoDownloadEnabled() {
return downloadEnabled.getOrElse(true);
}

private <T> T wrapInOperation(String displayName, Callable<T> provisioningStep) {
return buildOperationExecutor.call(new ToolchainProvisioningBuildOperation<>(displayName, provisioningStep));
}
Expand Down
3 changes: 3 additions & 0 deletions ...main/java/org/gradle/jvm/toolchain/internal/install/JavaToolchainProvisioningService.java
View file
Open in desktop
Expand Up @@ -25,4 +25,7 @@ public interface JavaToolchainProvisioningService {

Optional<File> tryInstall(JavaToolchainSpec spec);

boolean isAutoDownloadEnabled();

boolean hasConfiguredToolchainRepositories();
}
33 changes: 33 additions & 0 deletions ...vm/src/test/groovy/org/gradle/jvm/toolchain/internal/JavaToolchainQueryServiceTest.groovy
View file
Open in desktop
Expand Up @@ -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<File> tryInstall(JavaToolchainSpec spec) {
installed = true
Optional.of(new File("/path/12"))
Expand All @@ -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<File> tryInstall(JavaToolchainSpec spec) {
installed = true
Optional.of(new File("/path/12.broken"))
Expand All @@ -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<File> tryInstall(JavaToolchainSpec spec) {
installed++
Optional.of(new File("/path/12"))
Expand Down
32 changes: 30 additions & 2 deletions ...s/soak/src/integTest/groovy/org/gradle/jvm/toolchain/JavaToolchainDownloadSoakTest.groovy
View file
Open in desktop
Expand Up @@ -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.")
Expand All @@ -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.")
Expand All @@ -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)
Expand Down