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

apply substitution only to configurations on java SourceSets #10

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

apply substitution only to configurations on java SourceSets #10

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6a846ec
apply substitution only to configurations on java SourceSets
Dec 9, 2022
b9d61b2
only apply if SourceSetContainer extension is present
Dec 9, 2022
4ada190
only apply when java-base plugin is present
Dec 9, 2022
9a28a8c
explicitly include unifiedClasspath configuration
Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions .../main/java/com/palantir/gradle/jakartapackagealignment/JakartaPackageAlignmentPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,55 @@
*/
package com.palantir.gradle.jakartapackagealignment;

import com.google.common.collect.ImmutableSet;
import java.util.Set;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
import org.gradle.api.artifacts.Configuration;
import org.gradle.api.artifacts.component.ModuleComponentSelector;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.SourceSetContainer;

public class JakartaPackageAlignmentPlugin implements Plugin<Project> {
@Override
public final void apply(Project project) {
project.getConfigurations().configureEach(configuration -> {
configuration.getResolutionStrategy().getDependencySubstitution().all(dep -> {
if (dep.getRequested() instanceof ModuleComponentSelector) {
ModuleComponentSelector selector = (ModuleComponentSelector) dep.getRequested();
VersionMappings.getReplacement(selector)
.ifPresent(replacement -> dep.useTarget(
replacement,
"forced to Java EE 8 dependency because the requested Jakarta "
+ "dependency is < Jakarta EE 9"));
}
project.getPluginManager().withPlugin("java-base", _plugin -> {
SourceSetContainer sourceSets = project.getExtensions().getByType(SourceSetContainer.class);
sourceSets.configureEach(sourceSet -> {
configureAllConfigurationsForSourceSet(project, sourceSet);
});
});

project.getPluginManager().withPlugin("com.palantir.consistent-versions", _plugin -> {
// necessary to get g-c-v to write substituted dependencies to the versions.lock file
project.getConfigurations().getByName("unifiedClasspath", this::configureConfigurationForSubstitution);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we need something along these lines depending on the order things are applied?

(I defer to @CRogers on this sort of thing, though)

project.getConfigurations().configureEach(new Action<Configuration>() {
    @Override
    public void execute(Configuration config) {
        if ("unifiedClasspath".equals(config.getName())) {
            configureConfigurationForSubstitution(config);
        }
    }
});

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, i guess? withPlugin guarantees that the action won't run until after the plugin is applied, and this configuration is created when the plugin is applied: https://github.com/palantir/gradle-consistent-versions/blob/develop/src/main/java/com/palantir/gradle/versions/VersionsLockPlugin.java#L220-L226

});
}
Comment on lines +37 to +41
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awareness of specific dependency plugins seems like a bug that may come back to haunt us. I don't know of a better option, but we should make sure we understand why this is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, this feels hacky.

I just found this (see: https://docs.gradle.org/current/userguide/resolution_rules.html#sec:dependency_substitution_rules):

Adding a dependency substitution rule to a configuration changes the timing of when that configuration is resolved. Instead of being resolved on first use, the configuration is instead resolved when the task graph is being constructed.

So I wonder if the dependency substitution rules on e.g. runtimeClasspath isn't getting picked up when unifiedClasspath is resolved because the dependencies/constraints are copied to that configuration before the task graph is constructed?


private void configureAllConfigurationsForSourceSet(Project project, SourceSet sourceSet) {
// see: https://docs.gradle.org/current/userguide/java_plugin.html#java_source_set_configurations
Set<String> configurationsToConfigure = ImmutableSet.of(
sourceSet.getCompileClasspathConfigurationName(),
sourceSet.getRuntimeClasspathConfigurationName(),
sourceSet.getAnnotationProcessorConfigurationName());

project.getConfigurations().configureEach(configuration -> {
if (configurationsToConfigure.contains(configuration.getName())) {
configureConfigurationForSubstitution(configuration);
}
});
}

private void configureConfigurationForSubstitution(Configuration configuration) {
configuration.getResolutionStrategy().getDependencySubstitution().all(dep -> {
if (dep.getRequested() instanceof ModuleComponentSelector) {
ModuleComponentSelector selector = (ModuleComponentSelector) dep.getRequested();
VersionMappings.getReplacement(selector)
.ifPresent(replacement -> dep.useTarget(
replacement,
"forced to Java EE 8 dependency because the requested Jakarta "
+ "dependency is < Jakarta EE 9"));
}
});
}
}