Skip to content

Fix unused dependencies breaking with gradle 6.4 #1338

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

Merged
merged 7 commits into from
May 14, 2020
Merged

Fix unused dependencies breaking with gradle 6.4 #1338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
52b3822
Copy all compileClasspath attributes
dansanduleac May 13, 2020
072dd92
make sure dependenciesConfigurations are resolved as task inputs
dansanduleac May 13, 2020
c12a231
manually set up LIBRARY_ELEMENTS_ATTRIBUTE when allowed
dansanduleac May 13, 2020
62d722e
good catch!
dansanduleac May 14, 2020
9bf59fd
Add generated changelog entries
dansanduleac May 14, 2020
30a33e8
fix test, and test checkImplicitDependencies too
dansanduleac May 14, 2020
01cab05
oops make the right things @Classpath
dansanduleac May 14, 2020
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
7 changes: 7 additions & 0 deletions changelog/@unreleased/pr-1338.v2.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
type: fix
fix:
description: Fix task dependencies onto other subprojects for `com.palantir.baseline-exact-dependencies`
tasks (`checkUnusedDependencies`, `checkImplicitDependencies`), so they work with
gradle 6.4.
links:
- https://github.com/palantir/gradle-baseline/pull/1338
21 changes: 16 additions & 5 deletions ...aseline-java/src/main/groovy/com/palantir/baseline/plugins/BaselineExactDependencies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,13 @@
import org.gradle.api.artifacts.component.ComponentIdentifier;
import org.gradle.api.artifacts.component.ProjectComponentIdentifier;
import org.gradle.api.attributes.Attribute;
import org.gradle.api.attributes.LibraryElements;
import org.gradle.api.attributes.Usage;
import org.gradle.api.plugins.JavaPluginConvention;
import org.gradle.api.tasks.SourceSet;
import org.gradle.api.tasks.TaskProvider;
import org.gradle.util.GUtil;
import org.gradle.util.GradleVersion;

/** Validates that java projects declare exactly the dependencies they rely on, no more and no less. */
public final class BaselineExactDependencies implements Plugin<Project> {
Expand Down Expand Up @@ -99,11 +101,20 @@ private static void configureSourceSet(
compile.toString(), implementation.toString()));
conf.setVisible(false);
conf.setCanBeConsumed(false);
// Important! this ensures we resolve 'compile' variants rather than 'runtime'
// This is the same attribute that's being set on compileClasspath
conf.getAttributes()
.attribute(
Usage.USAGE_ATTRIBUTE, project.getObjects().named(Usage.class, Usage.JAVA_API));

conf.attributes(attributes -> {
// This ensures we resolve 'compile' variants rather than 'runtime'
// This is the same attribute that's being set on compileClasspath
attributes.attribute(
Usage.USAGE_ATTRIBUTE, project.getObjects().named(Usage.class, Usage.JAVA_API));
// Ensure we resolve the classes directory for local projects where possible, rather than the
// 'jar' file. We can only do this on Gradle 5.6+, otherwise do nothing.
if (GradleVersion.current().compareTo(GradleVersion.version("5.6")) >= 0) {
attributes.attribute(
LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE,
project.getObjects().named(LibraryElements.class, LibraryElements.CLASSES));
}
});
Comment on lines +112 to +117
Copy link
Contributor Author

@dansanduleac dansanduleac May 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is mostly an optimisation so we don't force the production of jars since we can use the classes dirs.
Gradle 6.4 has made it so that it sometimes forces jars when resolving compileClasspath in order to be able to confidently extract the Automatic-Module-Name if there is one set in the manifest.
However I think we can always just use the classes dir for this case, as it won't change the behaviour.


// Without this, the 'checkUnusedDependencies correctly picks up project dependency on java-library'
// test fails, by not causing gradle run the jar task, but resolving the path to the jar (rather
Expand Down
6 changes: 3 additions & 3 deletions ...eline-java/src/main/groovy/com/palantir/baseline/tasks/CheckImplicitDependenciesTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.SetProperty;
import org.gradle.api.tasks.Classpath;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.TaskAction;

public class CheckImplicitDependenciesTask extends DefaultTask {
Expand Down Expand Up @@ -142,7 +142,7 @@ private boolean shouldIgnore(ResolvedArtifact artifact) {
return ignore.get().contains(BaselineExactDependencies.asString(artifact));
}

@Input
@Classpath
public final Provider<List<Configuration>> getDependenciesConfigurations() {
return dependenciesConfigurations;
}
Expand All @@ -151,7 +151,7 @@ public final void dependenciesConfiguration(Configuration dependenciesConfigurat
this.dependenciesConfigurations.add(Objects.requireNonNull(dependenciesConfiguration));
}

@InputFiles
@Classpath
public final Provider<FileCollection> getSourceClasses() {
return sourceClasses;
}
Expand Down
6 changes: 3 additions & 3 deletions ...aseline-java/src/main/groovy/com/palantir/baseline/tasks/CheckUnusedDependenciesTask.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
import org.gradle.api.provider.Property;
import org.gradle.api.provider.Provider;
import org.gradle.api.provider.SetProperty;
import org.gradle.api.tasks.Classpath;
import org.gradle.api.tasks.Input;
import org.gradle.api.tasks.InputFiles;
import org.gradle.api.tasks.TaskAction;

public class CheckUnusedDependenciesTask extends DefaultTask {
Expand Down Expand Up @@ -176,7 +176,7 @@ private boolean shouldIgnore(ResolvedArtifact artifact) {
return ignore.get().contains(BaselineExactDependencies.asString(artifact));
}

@Input
@Classpath
public final Provider<List<Configuration>> getDependenciesConfigurations() {
return dependenciesConfigurations;
}
Expand Down Expand Up @@ -206,7 +206,7 @@ public final void sourceOnlyConfiguration(Configuration configuration) {
this.sourceOnlyConfigurations.add(Objects.requireNonNull(configuration));
}

@InputFiles
@Classpath
public final Provider<FileCollection> getSourceClasses() {
return sourceClasses;
}
Expand Down
9 changes: 7 additions & 2 deletions ...-baseline-java/src/test/groovy/com/palantir/baseline/BaselineExactDependenciesTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.palantir.baseline
import java.nio.file.Files
import org.gradle.testkit.runner.BuildResult
import org.gradle.testkit.runner.TaskOutcome
import spock.lang.Unroll

class BaselineExactDependenciesTest extends AbstractPluginTest {

Expand Down Expand Up @@ -113,7 +114,8 @@ class BaselineExactDependenciesTest extends AbstractPluginTest {
result.task(':checkUnusedDependenciesMain').getOutcome() == TaskOutcome.SUCCESS
}

def 'checkUnusedDependencies correctly picks up project dependency on java-library'() {
@Unroll
def '#task correctly picks up project dependency on java-library'() {
when:
buildFile << standardBuildFile
buildFile << """
Expand All @@ -139,8 +141,11 @@ class BaselineExactDependenciesTest extends AbstractPluginTest {
""".stripIndent()

then:
def result = with('checkUnusedDependencies', '--stacktrace').build()
def result = with(":${task}", '--stacktrace').build()
assert result.task(':needs-building-first:compileJava').getOutcome() != null

where:
task << ['checkUnusedDependencies', 'checkImplicitDependencies']
}

def 'checkUnusedDependenciesTest passes if dependency from main source set is not referenced in test'() {
Expand Down