Skip to content
This repository has been archived by the owner on Dec 5, 2023. It is now read-only.

Commit

Permalink
Merge pull request #27251 Deprecate registerFeature when using the ma…
Browse files Browse the repository at this point in the history 
…in source set.

Calling registerFeature with the main source set does some weird magic where the actual main source set will extend from the configurations created by the feature. This is confusing and different than the behavior for all other source sets. This deprecates that behavior, so that in 9.0 we can make the main source set behave the same as all other source sets when registering features.

For more information see, the code that we are trying to remove, and the associated comments: https://github.com/gradle/gradle/blob/f59fdfd691627dd0ed37b4c09f43873937c41a29/platforms/jvm/plugins-java-base/src/main/java/org/gradle/api/plugins/jvm/internal/DefaultJvmFeature.java#L168-L200

This deprecation has already been reflected in documentation by removing documentation telling users to use the main source set: gradle/gradle#27190

Co-authored-by: Justin Van Dort <[email protected]>
  • Loading branch information
@bot-gradle @jvandort
bot-gradle and jvandort committed Dec 4, 2023
2 parents c6e8a33 + ad1bbca commit b78379e
Show file tree
Hide file tree
Showing 10 changed files with 217 additions and 64 deletions.
93 changes: 93 additions & 0 deletions ...cumentation/docs/src/docs/userguide/releases/upgrading/upgrading_version_8.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,99 @@ JaCoCo has been updated to https://www.jacoco.org/jacoco/trunk/doc/changes.html[

=== Deprecations

[[register_feature_main_source_set]]
==== Deprecated calling `registerFeature` using the `main` source set

Calling `link:{javadocPath}/org/gradle/api/plugins/JavaPluginExtension.html#registerFeature-java.lang.String-org.gradle.api.Action-[registerFeature]` on the `link:{javadocPath}/org/gradle/api/plugins/JavaPluginExtension.html[java]` extension using the `main` source set is deprecated and will change behavior in Gradle 9.0.

Currently, features created while calling `link:{javadocPath}/org/gradle/api/plugins/FeatureSpec.html#usingSourceSet-org.gradle.api.tasks.SourceSet-[usingSourceSet]` with the `main` source set are initialized differently than features created while calling `usingSourceSet` with any other source set.
Previously, when using the `main` source set, new `implementation`, `compileOnly`, `runtimeOnly`, `api`, and `compileOnlyApi` configurations were created, and the compile and runtime classpaths of the `main` source set were configured to extend these configurations.

Starting in Gradle 9.0, the `main` source set will be treated like any other source set.
With the `java-library` plugin applied (or any other plugin that applies the `java` plugin), calling `usingSourceSet` with the `main` source set will throw an exception.
This is because the `java` plugin already configures a main feature.
Only if the `java` plugin is not applied will the `main` source set be permitted when calling `usingSourceSet`.

Code that currently registers features with the main source set, like so:
=====
[.multi-language-sample]
======
.build.gradle.kts
[source,kotlin]
----
plugins {
id("java-library")
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets["main"])
}
}
----
======
[.multi-language-sample]
======
.build.gradle
[source,groovy]
----
plugins {
id("java-library")
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets.main)
}
}
----
======
=====

Should instead create a separate source set for the feature, and register the feature with that source set:
=====
[.multi-language-sample]
======
.build.gradle.kts
[source,kotlin]
----
plugins {
id("java-library")
}
sourceSets {
create("feature")
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets["feature"])
}
}
----
======
[.multi-language-sample]
======
.build.gradle
[source,groovy]
----
plugins {
id("java-library")
}
sourceSets {
feature
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets.feature)
}
}
----
======
=====

[[deprecated_artifact_identifier]]
==== Deprecated `ArtifactIdentifier`
The `ArtifactIdentifier` class has been deprecated for removal in Gradle 9.0.
Expand Down
14 changes: 11 additions & 3 deletions ...ugins-java-base/src/main/java/org/gradle/api/plugins/internal/DefaultJavaFeatureSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,18 @@
import org.gradle.api.InvalidUserCodeException;
import org.gradle.api.capabilities.Capability;
import org.gradle.api.internal.project.ProjectInternal;
import org.gradle.api.plugins.FeatureSpec;
import org.gradle.api.plugins.jvm.internal.DefaultJvmFeature;
import org.gradle.api.plugins.jvm.internal.JvmFeatureInternal;
import org.gradle.api.tasks.SourceSet;
import org.gradle.internal.component.external.model.DefaultImmutableCapability;
import org.gradle.internal.component.external.model.ProjectDerivedCapability;
import org.gradle.internal.deprecation.DeprecationLogger;

import java.util.LinkedHashSet;
import java.util.Set;

public class DefaultJavaFeatureSpec implements FeatureSpecInternal {
public class DefaultJavaFeatureSpec implements FeatureSpec {
private final String name;
private final Set<Capability> capabilities = new LinkedHashSet<>(1);
private final ProjectInternal project;
Expand Down Expand Up @@ -67,12 +69,10 @@ public void disablePublication() {
allowPublication = false;
}

@Override
public boolean isPublished() {
return allowPublication;
}

@Override
public JvmFeatureInternal create() {
if (sourceSet == null) {
throw new InvalidUserCodeException("You must specify which source set to use for feature '" + name + "'");
Expand All @@ -82,6 +82,14 @@ public JvmFeatureInternal create() {
capabilities.add(new ProjectDerivedCapability(project, name));
}

if (SourceSet.isMain(sourceSet)) {
DeprecationLogger.deprecateBehaviour(String.format("The '%s' feature was created using the main source set.", name))
.withAdvice("The main source set is reserved for production code and should not be used for features. Use another source set instead.")
.willBecomeAnErrorInGradle9()
.withUpgradeGuideSection(8, "deprecate_register_feature_main_source_set")
.nagUser();
}

JvmFeatureInternal feature = new DefaultJvmFeature(name, sourceSet, capabilities, project, true, SourceSet.isMain(sourceSet));
feature.withApi();

Expand Down
2 changes: 1 addition & 1 deletion ...s-java-base/src/main/java/org/gradle/api/plugins/internal/DefaultJavaPluginExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ public boolean getAutoTargetJvmDisabled() {
*/
@Override
public void registerFeature(String name, Action<? super FeatureSpec> configureAction) {
FeatureSpecInternal spec = new DefaultJavaFeatureSpec(validateFeatureName(name), project);
DefaultJavaFeatureSpec spec = new DefaultJavaFeatureSpec(validateFeatureName(name), project);
configureAction.execute(spec);
JvmFeatureInternal feature = spec.create();

Expand Down
24 changes: 0 additions & 24 deletions .../plugins-java-base/src/main/java/org/gradle/api/plugins/internal/FeatureSpecInternal.java
View file

This file was deleted.

21 changes: 12 additions & 9 deletions platforms/jvm/plugins-java/src/integTest/groovy/org/gradle/api/plugins/JavaPluginTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,23 +474,26 @@ class JavaPluginTest extends AbstractProjectBuilderSpec {
project.pluginManager.apply(JavaPlugin)
commonProject.pluginManager.apply(JavaPlugin)
commonProject.group = "group"
commonProject.extensions.configure(JavaPluginExtension) {
it.registerFeature("other") {
it.usingSourceSet(commonProject.sourceSets.main)
commonProject.extensions.configure(JavaPluginExtension) { java ->
java.sourceSets.create("other")
java.registerFeature("other") {
it.usingSourceSet(java.sourceSets.other)
}
}
middleProject.pluginManager.apply(JavaPlugin)
middleProject.group = "group"
middleProject.extensions.configure(JavaPluginExtension) {
it.registerFeature("other") {
it.usingSourceSet(middleProject.sourceSets.main)
middleProject.extensions.configure(JavaPluginExtension) { java ->
java.sourceSets.create("other")
java.registerFeature("other") {
it.usingSourceSet(java.sourceSets.other)
}
}
appProject.pluginManager.apply(JavaPlugin)
appProject.group = "group"
appProject.extensions.configure(JavaPluginExtension) {
it.registerFeature("other") {
it.usingSourceSet(appProject.sourceSets.main)
appProject.extensions.configure(JavaPluginExtension) { java ->
java.sourceSets.create("other")
java.registerFeature("other") {
it.usingSourceSet(java.sourceSets.other)
}
}

Expand Down
5 changes: 4 additions & 1 deletion ...org/gradle/integtests/resolve/GradleMetadataJavaLibraryCrossVersionIntegrationTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,11 @@ class GradleMetadataJavaLibraryCrossVersionIntegrationTest extends CrossVersionI
java {
if (JavaPluginExtension.metaClass.respondsTo(delegate, 'registerFeature')) {
sourceSets {
hibernateSupport
}
registerFeature("hibernateSupport") {
usingSourceSet(sourceSets.main)
usingSourceSet(sourceSets.hibernateSupport)
capability("com.acme", "producer-hibernate-support", "1.0")
}
}
Expand Down
20 changes: 14 additions & 6 deletions .../integTest/groovy/org/gradle/api/publish/ivy/IvyPublishFeaturesJavaPluginIntegTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
given:
buildFile << """
sourceSets {
feature
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets.main)
usingSourceSet(sourceSets.feature)
}
}
Expand All @@ -50,7 +54,7 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
noMoreDependencies()
}
javaLibrary.parsedModuleMetadata.variant("featureRuntimeElements") {
assert files*.name == ['publishTest-1.9.jar']
assert files*.name == ['publishTest-1.9-feature.jar']
dependency('org', 'optionaldep', '1.0')
noMoreDependencies()
}
Expand All @@ -65,7 +69,7 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
resolveRuntimeArtifacts(javaLibrary) {
optionalFeatureCapabilities << "org.gradle.test:publishTest-feature:1.0"
withModuleMetadata {
expectFiles "publishTest-1.9.jar", "optionaldep-1.0.jar"
expectFiles "publishTest-1.9.jar", "publishTest-1.9-feature.jar", "optionaldep-1.0.jar"
}
withoutModuleMetadata {
// documents the current behavior - ivy does not use variant matching and hence the requested capability is ignored and the default configuration is selected
Expand All @@ -80,9 +84,13 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
given:
buildFile << """
sourceSets {
feature
}
java {
registerFeature("feature") {
usingSourceSet(sourceSets.main)
usingSourceSet(sourceSets.feature)
}
}
Expand Down Expand Up @@ -112,7 +120,7 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
noMoreDependencies()
}
javaLibrary.parsedModuleMetadata.variant("featureRuntimeElements") {
assert files*.name == ["${name}-${version}.jar"]
assert files*.name == ["${name}-${version}-feature.jar"]
dependency('org', 'optionaldep', '1.0')
noMoreDependencies()
}
Expand All @@ -127,7 +135,7 @@ class IvyPublishFeaturesJavaPluginIntegTest extends AbstractIvyPublishFeaturesJa
resolveRuntimeArtifacts(javaLibrary) {
optionalFeatureCapabilities << "$group:${name}-feature:1.0"
withModuleMetadata {
expectFiles "${name}-${version}.jar", "optionaldep-1.0.jar"
expectFiles "${name}-${version}.jar", "${name}-${version}-feature.jar", "optionaldep-1.0.jar"
}
withoutModuleMetadata {
// documents the current behavior - ivy does not use variant matching and hence the requested capability is ignored and the default configuration is selected
Expand Down
24 changes: 18 additions & 6 deletions ...egTest/groovy/org/gradle/api/publish/maven/MavenPublishFeaturesJavaPluginIntegTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,15 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
given:
buildFile << """
sourceSets {
feature
}
repositories { maven { url "${mavenRepo.uri}" } }
java {
registerFeature("feature") {
usingSourceSet(sourceSets.main)
usingSourceSet(sourceSets.feature)
}
}
Expand Down Expand Up @@ -53,7 +59,7 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
noMoreDependencies()
}
javaLibrary.parsedModuleMetadata.variant("featureRuntimeElements") {
assert files*.name == ['publishTest-1.9.jar']
assert files*.name == ['publishTest-1.9-feature.jar']
dependency('org', 'optionaldep', '1.0')
noMoreDependencies()
}
Expand All @@ -70,7 +76,7 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
resolveRuntimeArtifacts(javaLibrary) {
optionalFeatureCapabilities << "org.gradle.test:publishTest-feature:1.0"
withModuleMetadata {
expectFiles "publishTest-1.9.jar", "optionaldep-1.0.jar"
expectFiles "publishTest-1.9.jar", "publishTest-1.9-feature.jar", "optionaldep-1.0.jar"
}
withoutModuleMetadata {
shouldFail {
Expand All @@ -87,9 +93,15 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
given:
buildFile << """
sourceSets {
feature
}
repositories { maven { url "${mavenRepo.uri}" } }
java {
registerFeature("feature") {
usingSourceSet(sourceSets.main)
usingSourceSet(sourceSets.feature)
}
}
Expand Down Expand Up @@ -119,7 +131,7 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
noMoreDependencies()
}
javaLibrary.parsedModuleMetadata.variant("featureApiElements") {
assert files*.name == ["${name}-${version}.jar"]
assert files*.name == ["${name}-${version}-feature.jar"]
noMoreDependencies()
}
javaLibrary.parsedModuleMetadata.variant("featureRuntimeElements") {
Expand All @@ -139,7 +151,7 @@ class MavenPublishFeaturesJavaPluginIntegTest extends AbstractMavenPublishFeatur
resolveRuntimeArtifacts(javaLibrary) {
optionalFeatureCapabilities << "$group:${name}-feature:1.0"
withModuleMetadata {
expectFiles "${name}-${version}.jar", "optionaldep-1.0.jar"
expectFiles "${name}-${version}.jar", "${name}-${version}-feature.jar", "optionaldep-1.0.jar"
}
withoutModuleMetadata {
shouldFail {
Expand Down
1 change: 0 additions & 1 deletion subprojects/architecture-test/src/changes/archunit_store/internal-api-nullability.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,6 @@ Class <org.gradle.api.plugins.internal.DefaultJavaPluginConvention> is not annot
Class <org.gradle.api.plugins.internal.DefaultJavaPluginExtension$DefaultJavaResolutionConsistency> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (DefaultJavaPluginExtension.java:0)
Class <org.gradle.api.plugins.internal.DefaultJavaPluginExtension> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (DefaultJavaPluginExtension.java:0)
Class <org.gradle.api.plugins.internal.DefaultWarPluginConvention> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (DefaultWarPluginConvention.java:0)
Class <org.gradle.api.plugins.internal.FeatureSpecInternal> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (FeatureSpecInternal.java:0)
Class <org.gradle.api.plugins.internal.HelpBuiltInCommand> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (HelpBuiltInCommand.java:0)
Class <org.gradle.api.plugins.internal.HelpTasksAutoApplyAction> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (HelpTasksAutoApplyAction.java:0)
Class <org.gradle.api.plugins.internal.JavaConfigurationVariantMapping$UnpublishableArtifactTypeSpec> is not annotated (directly or via its package) with @org.gradle.api.NonNullApi in (JavaConfigurationVariantMapping.java:0)
Expand Down
Loading

0 comments on commit b78379e

Please sign in to comment.