-
Notifications
You must be signed in to change notification settings - Fork 10
Platform constraints #208
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.
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
base: main
Are you sure you want to change the base?
Platform constraints #208
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -259,6 +259,15 @@ configurations.runtimeClasspath { | |||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| In addition to creating a common configuration for resolution, a platform project can also be enabled to enforce | ||||||
| versions. | ||||||
|
|
||||||
| ```kotlin | ||||||
| extraJavaModuleInfo { | ||||||
| platformDependency = project.provider { project.dependencies.platform(project.dependencies.create("org.example:bom:1.0.0")) } | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
💄 We are always in the Would be great if there would be ca notation without Alternatively, we could think about providing one (or two) String-based method as for the dependency: Then, you won't need the validation and instead could always create a platform dependency from the coordinates. I like this a bit better, as it is easier to read for users that skim the build scripts and do not know all the details. It looks more like a DSL then. You would need to check if the string is coordinates or project name (starts with |
||||||
| } | ||||||
| ``` | ||||||
|
|
||||||
| ## I have many automatic modules in my project. How can I convert them into proper modules and control what they export or require? | ||||||
|
|
||||||
| The plugin provides a set of `<sourceSet>moduleDescriptorRecommendations` tasks that generate the real module declarations utilizing [jdeps](https://docs.oracle.com/en/java/javase/11/tools/jdeps.html) and dependency metadata. | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,9 @@ | |||||||
| import static java.util.Collections.emptyList; | ||||||||
| import static java.util.Objects.requireNonNull; | ||||||||
| import static org.gradle.api.attributes.Category.CATEGORY_ATTRIBUTE; | ||||||||
| import static org.gradle.api.attributes.Category.ENFORCED_PLATFORM; | ||||||||
| import static org.gradle.api.attributes.Category.LIBRARY; | ||||||||
| import static org.gradle.api.attributes.Category.REGULAR_PLATFORM; | ||||||||
| import static org.gradle.api.attributes.Usage.USAGE_ATTRIBUTE; | ||||||||
|
|
||||||||
| import java.io.Serializable; | ||||||||
|
|
@@ -15,6 +17,8 @@ | |||||||
| import org.gradle.api.Project; | ||||||||
| import org.gradle.api.artifacts.Configuration; | ||||||||
| import org.gradle.api.artifacts.ConfigurationContainer; | ||||||||
| import org.gradle.api.artifacts.Dependency; | ||||||||
| import org.gradle.api.artifacts.ModuleDependency; | ||||||||
| import org.gradle.api.artifacts.result.DependencyResult; | ||||||||
| import org.gradle.api.artifacts.result.ResolvedDependencyResult; | ||||||||
| import org.gradle.api.artifacts.result.UnresolvedDependencyResult; | ||||||||
|
|
@@ -34,6 +38,8 @@ | |||||||
| public class PublishedMetadata implements Serializable { | ||||||||
| private static final Attribute<String> CATEGORY_ATTRIBUTE_UNTYPED = | ||||||||
| Attribute.of(CATEGORY_ATTRIBUTE.getName(), String.class); | ||||||||
| private static final Attribute<Category> CATEGORY_ATTRIBUTE_TYPED = | ||||||||
| Attribute.of(CATEGORY_ATTRIBUTE.getName(), Category.class); | ||||||||
| private static final String DEFAULT_VERSION_SOURCE_CONFIGURATION = "definedDependenciesVersions"; | ||||||||
|
|
||||||||
| private final String gav; | ||||||||
|
|
@@ -47,10 +53,16 @@ public class PublishedMetadata implements Serializable { | |||||||
| PublishedMetadata(String gav, Project project, ExtraJavaModuleInfoPluginExtension extension) { | ||||||||
| this.gav = gav; | ||||||||
|
|
||||||||
| List<String> compileDependencies = | ||||||||
| componentVariant(extension.getVersionsProvidingConfiguration(), project, Usage.JAVA_API); | ||||||||
| List<String> runtimeDependencies = | ||||||||
| componentVariant(extension.getVersionsProvidingConfiguration(), project, Usage.JAVA_RUNTIME); | ||||||||
| List<String> compileDependencies = componentVariant( | ||||||||
| extension.getVersionsProvidingConfiguration(), | ||||||||
| extension.getPlatformDependency(), | ||||||||
| project, | ||||||||
| Usage.JAVA_API); | ||||||||
| List<String> runtimeDependencies = componentVariant( | ||||||||
| extension.getVersionsProvidingConfiguration(), | ||||||||
| extension.getPlatformDependency(), | ||||||||
| project, | ||||||||
| Usage.JAVA_RUNTIME); | ||||||||
|
|
||||||||
| Stream.concat(compileDependencies.stream(), runtimeDependencies.stream()) | ||||||||
| .distinct() | ||||||||
|
|
@@ -67,7 +79,10 @@ public class PublishedMetadata implements Serializable { | |||||||
|
|
||||||||
| @SuppressWarnings({"UnstableApiUsage", "unchecked"}) | ||||||||
| private List<String> componentVariant( | ||||||||
| Provider<String> versionsProvidingConfiguration, Project project, String usage) { | ||||||||
| Provider<String> versionsProvidingConfiguration, | ||||||||
| Provider<Dependency> platformDependencyProvider, | ||||||||
| Project project, | ||||||||
| String usage) { | ||||||||
| Configuration versionsSource; | ||||||||
| if (versionsProvidingConfiguration.isPresent()) { | ||||||||
| versionsSource = project.getConfigurations() | ||||||||
|
|
@@ -81,8 +96,29 @@ private List<String> componentVariant( | |||||||
| project.getExtensions().findByType(SourceSetContainer.class)); | ||||||||
| } | ||||||||
|
|
||||||||
| Configuration singleComponentVariantResolver = project.getConfigurations() | ||||||||
| .detachedConfiguration(project.getDependencies().create(gav)); | ||||||||
| List<Dependency> dependencies = new ArrayList<>(); | ||||||||
| dependencies.add(project.getDependencies().create(gav)); | ||||||||
| if (platformDependencyProvider.isPresent()) { | ||||||||
| if (!ModuleDependency.class.isAssignableFrom( | ||||||||
| platformDependencyProvider.get().getClass())) { | ||||||||
|
Comment on lines
+102
to
+103
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
💄 For consistency: do it similar as other |
||||||||
| throw new IllegalArgumentException("Unable to determine dependency '" | ||||||||
| + platformDependencyProvider.get().getName() + "' type"); | ||||||||
| } | ||||||||
|
|
||||||||
| ModuleDependency platformDependency = (ModuleDependency) platformDependencyProvider.get(); | ||||||||
| // A platform dependency must have the platform attribute specified. | ||||||||
| Category category = platformDependency.getAttributes().getAttribute(CATEGORY_ATTRIBUTE_TYPED); | ||||||||
| if (category == null | ||||||||
| || (!category.getName().equals(REGULAR_PLATFORM) | ||||||||
| && !category.getName().equals(ENFORCED_PLATFORM))) { | ||||||||
| throw new IllegalArgumentException( | ||||||||
| "Dependency '" + platformDependency.getName() + "' is not a platform"); | ||||||||
| } | ||||||||
| dependencies.add(platformDependency); | ||||||||
| } | ||||||||
|
|
||||||||
| Configuration singleComponentVariantResolver = | ||||||||
| project.getConfigurations().detachedConfiguration(dependencies.toArray(new Dependency[0])); | ||||||||
| singleComponentVariantResolver.setCanBeConsumed(false); | ||||||||
| singleComponentVariantResolver.shouldResolveConsistentlyWith(versionsSource); | ||||||||
| versionsSource.getAttributes().keySet().forEach(a -> { | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -266,4 +266,25 @@ class EdgeCasesFunctionalTest extends Specification { | |||||
| expect: | ||||||
| build() | ||||||
| } | ||||||
|
|
||||||
| def "resolve against a platform project if specified"() { | ||||||
| given: | ||||||
| buildFile << """ | ||||||
| val springBom = "org.springframework:spring-framework-bom:6.2.9" | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The test fails on Gradle We may drop support for |
||||||
| dependencies { | ||||||
| implementation(platform(springBom)) | ||||||
| implementation("org.springframework:spring-jcl") | ||||||
| } | ||||||
|
|
||||||
| extraJavaModuleInfo { | ||||||
| failOnAutomaticModules.set(true) | ||||||
| platformDependency.set(project.provider { project.dependencies.platform(project.dependencies.create(springBom)) }) | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
💄 consistent with doc change |
||||||
| module("org.springframework:spring-jcl", "spring.jcl") | ||||||
| } | ||||||
| """ | ||||||
|
|
||||||
| expect: | ||||||
| build() | ||||||
| } | ||||||
|
|
||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I have one more question for clarification and to understand what setups we can recommend to users. If you set
versionsProvidingConfigurationto a configuration that resolves all dependencies that you compute from your platform (see discussion in #207), then you don't need the addition of this PR, right?So this is an alternative to
versionsProvidingConfiguration, correct? Which then indeed makes the setup more concise as you do not have to deal with all the ceremony aroundversionsProvidingConfiguration(see my comment here).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To partly answer my own question:
versionsProvidingConfigurationapproach has the advantage (may be considered a disadvantage) that it also provides the versions of all (well, almost all) transitive dependencies. In your own platform project, you may only define versions of you direct dependencies. Then this approach still works.platformDependencyapproach (introduced in this PR) requires you to define all versions in the platform, but has the advantage of a simpler overall setup as I wrote above. Maybe you want to define all versions anyway to have more control and overview over all transitives. Or you mainly rely on an existing platform, likespring-framework-bom, that defines all the versions for you already.I think then, for the moment, both are valid approaches and adding this PR, while also keeping
versionsProvidingConfiguration, is a good addition.