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.

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

Fix for shadowJar out-of-date with configuration caching #708

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package com.github.jengelman.gradle.plugins.shadow.internal

import groovy.util.logging.Slf4j
import org.gradle.api.Project
import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.Dependency
import org.gradle.api.artifacts.ResolvedDependency
import org.gradle.api.file.FileCollection
Expand All @@ -25,7 +24,7 @@ abstract class AbstractDependencyFilter implements DependencyFilter {
Set<ResolvedDependency> includedDependencies,
Set<ResolvedDependency> excludedDependencies)

FileCollection resolve(Configuration configuration) {
FileCollection resolve(FileCollection configuration) {
Set<ResolvedDependency> includedDeps = []
Set<ResolvedDependency> excludedDeps = []
resolve(configuration.resolvedConfiguration.firstLevelModuleDependencies, includedDeps, excludedDeps)
Expand All @@ -34,7 +33,7 @@ abstract class AbstractDependencyFilter implements DependencyFilter {
}.flatten())
}

FileCollection resolve(Collection<Configuration> configurations) {
FileCollection resolve(Collection<FileCollection> configurations) {
configurations.collect {
resolve(it)
}.sum() as FileCollection ?: project.files()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package com.github.jengelman.gradle.plugins.shadow.internal

import org.gradle.api.artifacts.Configuration
import org.gradle.api.artifacts.Dependency
import org.gradle.api.artifacts.ResolvedDependency
import org.gradle.api.file.FileCollection
Expand All @@ -9,18 +8,18 @@ import org.gradle.api.specs.Spec
interface DependencyFilter {

/**
* Resolve a Configuration against the include/exclude rules in the filter
* Resolve a FileCollection against the include/exclude rules in the filter
* @param configuration
* @return
*/
FileCollection resolve(Configuration configuration)
FileCollection resolve(FileCollection configuration)

/**
* Resolve all Configurations against the include/exclude ruels in the filter and combine the results
* Resolve all FileCollections against the include/exclude ruels in the filter and combine the results
* @param configurations
* @return
*/
FileCollection resolve(Collection<Configuration> configurations)
FileCollection resolve(Collection<FileCollection> configurations)

/**
* Exclude dependencies that match the provided spec.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public class ShadowJar extends Jar implements ShadowSpec {

private List<Transformer> transformers;
private List<Relocator> relocators;
private transient List<Configuration> configurations;
private List<FileCollection> configurations;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure whether the removal of transient can affect correctness of outputs; I was surprised that I didn't notice any differences in output between my first and second builds even though the second build was not UP-TO-DATE.

However, removing transient does seem to allow the task to be up-to-date in the second build.

cc @eskatos since it seems to have been added here: c9e30f1#diff-d5abeaae583093a8a92ef53ffe7f5a21cf185cf0d5e3a76edd236fa26dbf7e9fR34

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call! Looks good to me

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

private transient DependencyFilter dependencyFilter;

private boolean minimizeJar;
Expand Down Expand Up @@ -429,11 +429,11 @@ public void setRelocators(List<Relocator> relocators) {
}

@Classpath @Optional
public List<Configuration> getConfigurations() {
public List<FileCollection> getConfigurations() {
return this.configurations;
}

public void setConfigurations(List<Configuration> configurations) {
public void setConfigurations(List<FileCollection> configurations) {
this.configurations = configurations;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,38 @@ class ConfigurationCacheSpec extends PluginSpecification {
and:
result.output.contains("Reusing configuration cache.")
}

def "configuration caching of configurations is up-to-date"() {
given:
file('settings.gradle') << """
include 'lib'
""".stripIndent()

and:
file('lib/src/main/java/lib/Lib.java') << """
package lib;
public class Lib {}
""".stripIndent()
file('lib/build.gradle') << """
apply plugin: 'java'
apply plugin: 'com.github.johnrengelman.shadow'

repositories { maven { url "${repo.uri}" } }
dependencies {
implementation "junit:junit:3.8.2"
}

shadowJar {
configurations = [project.configurations.compileClasspath]
}

""".stripIndent()

when:
run('--configuration-cache', 'shadowJar', '-s')
def result = run('--configuration-cache', 'shadowJar', '-s')

then:
result.output.contains(":lib:shadowJar UP-TO-DATE")
}
}