From 097ccf994a9f4afc0f30e6cb752c5c57f79332f6 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 10:02:16 -0700 Subject: [PATCH 1/6] Added tests for minimize with project exclusions --- .../plugins/shadow/ShadowPluginSpec.groovy | 106 ++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy index ea18ebc83..bc4619789 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy @@ -318,6 +318,112 @@ class ShadowPluginSpec extends PluginSpecification { doesNotContain(serverOutput, ['client/Client.class']) } + /** + * 'Client', 'Server' and 'junit' are independent. + * Unused classes of 'client' and theirs dependencies shouldn't be removed. + */ + def 'exclude a project from minimize '() { + given: + file('settings.gradle') << """ + include 'client', 'server' + """.stripIndent() + + file('client/src/main/java/client/Client.java') << """ + package client; + public class Client {} + """.stripIndent() + + file('client/build.gradle') << """ + apply plugin: 'java' + repositories { maven { url "${repo.uri}" } } + """.stripIndent() + + file('server/src/main/java/server/Server.java') << """ + package server; + public class Server {} + """.stripIndent() + + file('server/build.gradle') << """ + apply plugin: 'java' + apply plugin: 'com.github.johnrengelman.shadow' + + shadowJar { + minimize { + exclude(project(':client')) + } + } + + repositories { maven { url "${repo.uri}" } } + dependencies { compile project(':client') } + """.stripIndent() + + File serverOutput = file('server/build/libs/server-all.jar') + + when: + runner.withArguments(':server:shadowJar', '--stacktrace').withDebug(true).build() + + then: + contains(serverOutput, [ + 'client/Client.class', + 'server/Server.class' + ]) + } + + /** + * 'Client', 'Server' and 'junit' are independent. + * Unused classes of 'client' and theirs dependencies shouldn't be removed. + */ + def 'exclude a project from minimize - excludes transitive dependencies that are not used in the project dependency'() { + given: + file('settings.gradle') << """ + include 'client', 'server' + """.stripIndent() + + file('client/src/main/java/client/Client.java') << """ + package client; + public class Client {} + """.stripIndent() + + file('client/build.gradle') << """ + apply plugin: 'java' + repositories { maven { url "${repo.uri}" } } + dependencies { compile 'junit:junit:3.8.2' } + """.stripIndent() + + file('server/src/main/java/server/Server.java') << """ + package server; + public class Server {} + """.stripIndent() + + file('server/build.gradle') << """ + apply plugin: 'java' + apply plugin: 'com.github.johnrengelman.shadow' + + shadowJar { + minimize { + exclude(project(':client')) + } + } + + repositories { maven { url "${repo.uri}" } } + dependencies { compile project(':client') } + """.stripIndent() + + File serverOutput = file('server/build/libs/server-all.jar') + + when: + runner.withArguments(':server:shadowJar', '--stacktrace').withDebug(true).build() + + then: + contains(serverOutput, [ + 'client/Client.class', + 'server/Server.class' + ]) + doesNotContain(serverOutput, [ 'junit/framework/Test.class']) + + } + + /** * 'api' used as api for 'impl', and depended on 'lib'. 'junit' is independent. * The minimize shall remove 'junit', but not 'api'. From be98227c9769c5e137f2c2562465dd0a84c944fd Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 10:15:24 -0700 Subject: [PATCH 2/6] write test case where project is excluded but its dependencies are not and should be --- .../plugins/shadow/ShadowPluginSpec.groovy | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy index bc4619789..b77c6fc91 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy @@ -373,6 +373,62 @@ class ShadowPluginSpec extends PluginSpecification { * 'Client', 'Server' and 'junit' are independent. * Unused classes of 'client' and theirs dependencies shouldn't be removed. */ + def 'exclude a project from minimize - shall not excludes transitive dependencies that are used in the project'() { + given: + file('settings.gradle') << """ + include 'client', 'server' + """.stripIndent() + + file('client/src/main/java/client/Client.java') << """ + package client; + import junit.framework.TestCase; + public class Client extends TestCase { + public static void main(String[] args) {} + } + """.stripIndent() + + file('client/build.gradle') << """ + apply plugin: 'java' + repositories { maven { url "${repo.uri}" } } + dependencies { compile 'junit:junit:3.8.2' } + """.stripIndent() + + file('server/src/main/java/server/Server.java') << """ + package server; + public class Server {} + """.stripIndent() + + file('server/build.gradle') << """ + apply plugin: 'java' + apply plugin: 'com.github.johnrengelman.shadow' + + shadowJar { + minimize { + exclude(project(':client')) + } + } + + repositories { maven { url "${repo.uri}" } } + dependencies { compile project(':client') } + """.stripIndent() + + File serverOutput = file('server/build/libs/server-all.jar') + + when: + runner.withArguments(':server:shadowJar', '--stacktrace').withDebug(true).build() + + then: + contains(serverOutput, [ + 'client/Client.class', + 'server/Server.class', + 'junit/framework/TestCase.class' + ]) + } + + /** + * 'Client', 'Server' and 'junit' are independent. + * Unused classes of 'client' should not be removed + */ def 'exclude a project from minimize - excludes transitive dependencies that are not used in the project dependency'() { given: file('settings.gradle') << """ @@ -420,7 +476,6 @@ class ShadowPluginSpec extends PluginSpecification { 'server/Server.class' ]) doesNotContain(serverOutput, [ 'junit/framework/Test.class']) - } From 9fa51e14b44aada647b34899dd76032c1287bc7c Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 10:55:19 -0700 Subject: [PATCH 3/6] make sure dependencies from excluded project are not removed --- .../internal/AbstractDependencyFilter.groovy | 120 ++++++++++++++++++ .../internal/DefaultDependencyFilter.groovy | 109 +--------------- .../internal/MinimizeDependencyFilter.groovy | 23 ++++ .../plugins/shadow/tasks/ShadowJar.java | 4 +- .../plugins/shadow/ShadowPluginSpec.groovy | 12 +- 5 files changed, 153 insertions(+), 115 deletions(-) create mode 100644 src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/AbstractDependencyFilter.groovy create mode 100644 src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/AbstractDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/AbstractDependencyFilter.groovy new file mode 100644 index 000000000..ca497e559 --- /dev/null +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/AbstractDependencyFilter.groovy @@ -0,0 +1,120 @@ +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 +import org.gradle.api.specs.Spec +import org.gradle.api.specs.Specs + +@Slf4j +abstract class AbstractDependencyFilter implements DependencyFilter { + private final Project project + + protected final List> includeSpecs = [] + protected final List> excludeSpecs = [] + + AbstractDependencyFilter(Project project) { + assert project + this.project = project + } + + abstract protected void resolve(Set dependencies, + Set includedDependencies, + Set excludedDependencies) + + FileCollection resolve(Configuration configuration) { + Set includedDeps = [] + Set excludedDeps = [] + resolve(configuration.resolvedConfiguration.firstLevelModuleDependencies, includedDeps, excludedDeps) + return project.files(configuration.files) - project.files(excludedDeps.collect { + it.moduleArtifacts*.file + }.flatten()) + } + + FileCollection resolve(Collection configurations) { + configurations.collect { + resolve(it) + }.sum() as FileCollection ?: project.files() + } + + /** + * Exclude dependencies that match the provided spec. + * + * @param spec + * @return + */ + DependencyFilter exclude(Spec spec) { + excludeSpecs << spec + return this + } + + /** + * Include dependencies that match the provided spec. + * + * @param spec + * @return + */ + DependencyFilter include(Spec spec) { + includeSpecs << spec + return this + } + + /** + * Create a spec that matches the provided project notation on group, name, and version + * @param notation + * @return + */ + Spec project(Map notation) { + dependency(project.dependencies.project(notation)) + } + + /** + * Create a spec that matches the default configuration for the provided project path on group, name, and version + * + * @param notation + * @return + */ + Spec project(String notation) { + dependency(project.dependencies.project(path: notation, configuration: 'default')) + } + + /** + * Create a spec that matches dependencies using the provided notation on group, name, and version + * @param notation + * @return + */ + Spec dependency(Object notation) { + dependency(project.dependencies.create(notation)) + } + + /** + * Create a spec that matches the provided dependency on group, name, and version + * @param dependency + * @return + */ + Spec dependency(Dependency dependency) { + this.dependency({ ResolvedDependency it -> + (!dependency.group || it.moduleGroup.matches(dependency.group)) && + (!dependency.name || it.moduleName.matches(dependency.name)) && + (!dependency.version || it.moduleVersion.matches(dependency.version)) + }) + } + + /** + * Create a spec that matches the provided closure + * @param spec + * @return + */ + Spec dependency(Closure spec) { + return Specs.convertClosureToSpec(spec) + } + + protected boolean isIncluded(ResolvedDependency dependency) { + boolean include = includeSpecs.empty || includeSpecs.any { it.isSatisfiedBy(dependency) } + boolean exclude = !excludeSpecs.empty && excludeSpecs.any { it.isSatisfiedBy(dependency) } + return include && !exclude + } +} diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/DefaultDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/DefaultDependencyFilter.groovy index 3cb7dee7b..4734fedd1 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/DefaultDependencyFilter.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/DefaultDependencyFilter.groovy @@ -2,114 +2,16 @@ 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 -import org.gradle.api.specs.Spec -import org.gradle.api.specs.Specs @Slf4j -class DefaultDependencyFilter implements DependencyFilter { - - private final Project project - - private final List> includeSpecs = [] - private final List> excludeSpecs = [] +class DefaultDependencyFilter extends AbstractDependencyFilter { DefaultDependencyFilter(Project project) { - assert project - this.project = project - } - - FileCollection resolve(Configuration configuration) { - Set includedDeps = [] - Set excludedDeps = [] - resolve(configuration.resolvedConfiguration.firstLevelModuleDependencies, includedDeps, excludedDeps) - return project.files(configuration.files) - project.files(excludedDeps.collect { - it.moduleArtifacts*.file - }.flatten()) - } - - FileCollection resolve(Collection configurations) { - configurations.collect { - resolve(it) - }.sum() as FileCollection ?: project.files() - } - - /** - * Exclude dependencies that match the provided spec. - * - * @param spec - * @return - */ - DependencyFilter exclude(Spec spec) { - excludeSpecs << spec - return this - } - - /** - * Include dependencies that match the provided spec. - * - * @param spec - * @return - */ - DependencyFilter include(Spec spec) { - includeSpecs << spec - return this - } - - /** - * Create a spec that matches the provided project notation on group, name, and version - * @param notation - * @return - */ - Spec project(Map notation) { - dependency(project.dependencies.project(notation)) - } - - /** - * Create a spec that matches the default configuration for the provided project path on group, name, and version - * - * @param notation - * @return - */ - Spec project(String notation) { - dependency(project.dependencies.project(path: notation, configuration: 'default')) + super(project) } - /** - * Create a spec that matches dependencies using the provided notation on group, name, and version - * @param notation - * @return - */ - Spec dependency(Object notation) { - dependency(project.dependencies.create(notation)) - } - - /** - * Create a spec that matches the provided dependency on group, name, and version - * @param dependency - * @return - */ - Spec dependency(Dependency dependency) { - this.dependency({ ResolvedDependency it -> - (!dependency.group || it.moduleGroup.matches(dependency.group)) && - (!dependency.name || it.moduleName.matches(dependency.name)) && - (!dependency.version || it.moduleVersion.matches(dependency.version)) - }) - } - - /** - * Create a spec that matches the provided closure - * @param spec - * @return - */ - Spec dependency(Closure spec) { - return Specs.convertClosureToSpec(spec) - } - - + @Override protected void resolve(Set dependencies, Set includedDependencies, Set excludedDependencies) { @@ -120,9 +22,4 @@ class DefaultDependencyFilter implements DependencyFilter { } } - protected boolean isIncluded(ResolvedDependency dependency) { - boolean include = includeSpecs.empty || includeSpecs.any { it.isSatisfiedBy(dependency) } - boolean exclude = !excludeSpecs.empty && excludeSpecs.any { it.isSatisfiedBy(dependency) } - return include && !exclude - } } diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy new file mode 100644 index 000000000..907212420 --- /dev/null +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy @@ -0,0 +1,23 @@ +package com.github.jengelman.gradle.plugins.shadow.internal + +import groovy.util.logging.Slf4j +import org.gradle.api.Project +import org.gradle.api.artifacts.ResolvedDependency + +@Slf4j +class MinimizeDependencyFilter extends AbstractDependencyFilter { + MinimizeDependencyFilter(Project project) { + super(project) + } + + @Override + protected void resolve(Set dependencies, + Set includedDependencies, + Set excludedDependencies) { + dependencies.each { dependency -> + if (isIncluded(dependency) && includedDependencies.any { it.parents.contains(dependency) } ? includedDependencies.add(dependency) : excludedDependencies.add(dependency)) { + resolve(dependency.children, includedDependencies, excludedDependencies) + } + } + } +} \ No newline at end of file diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.java b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.java index 159d90f6e..6130a3cdc 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.java +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/tasks/ShadowJar.java @@ -9,8 +9,6 @@ import com.github.jengelman.gradle.plugins.shadow.transformers.ServiceFileTransformer; import com.github.jengelman.gradle.plugins.shadow.transformers.Transformer; import groovy.lang.MetaClass; -import java.io.File; -import java.util.Set; import org.codehaus.groovy.runtime.InvokerHelper; import org.gradle.api.Action; import org.gradle.api.artifacts.Configuration; @@ -45,7 +43,7 @@ public ShadowJar() { super(); versionUtil = new GradleVersionUtil(getProject().getGradle().getGradleVersion()); dependencyFilter = new DefaultDependencyFilter(getProject()); - dependencyFilterForMinimize = new DefaultDependencyFilter(getProject()); + dependencyFilterForMinimize = new MinimizeDependencyFilter(getProject()); setManifest(new DefaultInheritManifest(getServices().get(FileResolver.class))); transformers = new ArrayList<>(); relocators = new ArrayList<>(); diff --git a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy index b77c6fc91..d2e9765b6 100644 --- a/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy +++ b/src/test/groovy/com/github/jengelman/gradle/plugins/shadow/ShadowPluginSpec.groovy @@ -373,7 +373,7 @@ class ShadowPluginSpec extends PluginSpecification { * 'Client', 'Server' and 'junit' are independent. * Unused classes of 'client' and theirs dependencies shouldn't be removed. */ - def 'exclude a project from minimize - shall not excludes transitive dependencies that are used in the project'() { + def 'exclude a project from minimize - shall not exclude transitive dependencies that are used in subproject'() { given: file('settings.gradle') << """ include 'client', 'server' @@ -427,9 +427,9 @@ class ShadowPluginSpec extends PluginSpecification { /** * 'Client', 'Server' and 'junit' are independent. - * Unused classes of 'client' should not be removed + * Unused classes of 'client' and theirs dependencies shouldn't be removed. */ - def 'exclude a project from minimize - excludes transitive dependencies that are not used in the project dependency'() { + def 'exclude a project from minimize - shall not exclude transitive dependencies from subproject that are not used'() { given: file('settings.gradle') << """ include 'client', 'server' @@ -437,7 +437,7 @@ class ShadowPluginSpec extends PluginSpecification { file('client/src/main/java/client/Client.java') << """ package client; - public class Client {} + public class Client { } """.stripIndent() file('client/build.gradle') << """ @@ -473,9 +473,9 @@ class ShadowPluginSpec extends PluginSpecification { then: contains(serverOutput, [ 'client/Client.class', - 'server/Server.class' + 'server/Server.class', + 'junit/framework/TestCase.class' ]) - doesNotContain(serverOutput, [ 'junit/framework/Test.class']) } From edc204cf8ef777d88daa0fa88800abc211ecd9d7 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 12:13:11 -0700 Subject: [PATCH 4/6] fix logic for exclusion in projects --- .../shadow/internal/MinimizeDependencyFilter.groovy | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy index 907212420..2b2000028 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy @@ -15,9 +15,16 @@ class MinimizeDependencyFilter extends AbstractDependencyFilter { Set includedDependencies, Set excludedDependencies) { dependencies.each { dependency -> - if (isIncluded(dependency) && includedDependencies.any { it.parents.contains(dependency) } ? includedDependencies.add(dependency) : excludedDependencies.add(dependency)) { - resolve(dependency.children, includedDependencies, excludedDependencies) + if(isIncluded(dependency) && !isParentExcluded(excludedDependencies, dependency)) { + includedDependencies.add(dependency) + } else { + excludedDependencies.add(dependency) } + resolve(dependency.children, includedDependencies, excludedDependencies) } } + + private boolean isParentExcluded(Set excludedDependencies, ResolvedDependency dependency) { + excludedDependencies.any { dependency.parents.contains(it) } + } } \ No newline at end of file From e195c1e5eca6635cdf2d55babb9478d801ac7549 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 12:19:57 -0700 Subject: [PATCH 5/6] fix logic for exclusion in projects --- .../plugins/shadow/internal/MinimizeDependencyFilter.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy index 2b2000028..66cfeadd8 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy @@ -6,6 +6,7 @@ import org.gradle.api.artifacts.ResolvedDependency @Slf4j class MinimizeDependencyFilter extends AbstractDependencyFilter { + MinimizeDependencyFilter(Project project) { super(project) } From 992add8312ee78c0c56a2ba41e5e6483a81cbf64 Mon Sep 17 00:00:00 2001 From: Roberto Perez Alcolea Date: Thu, 1 Nov 2018 12:31:43 -0700 Subject: [PATCH 6/6] Fix issue with infinite dependency resolve --- .../shadow/internal/MinimizeDependencyFilter.groovy | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy index 66cfeadd8..762005ffc 100644 --- a/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy +++ b/src/main/groovy/com/github/jengelman/gradle/plugins/shadow/internal/MinimizeDependencyFilter.groovy @@ -15,13 +15,11 @@ class MinimizeDependencyFilter extends AbstractDependencyFilter { protected void resolve(Set dependencies, Set includedDependencies, Set excludedDependencies) { - dependencies.each { dependency -> - if(isIncluded(dependency) && !isParentExcluded(excludedDependencies, dependency)) { - includedDependencies.add(dependency) - } else { - excludedDependencies.add(dependency) + + dependencies.each { + if (isIncluded(it) && !isParentExcluded(excludedDependencies, it) ? includedDependencies.add(it) : excludedDependencies.add(it)) { + resolve(it.children, includedDependencies, excludedDependencies) } - resolve(dependency.children, includedDependencies, excludedDependencies) } }