From f5df6e463a7e66de3a5c82c68d915dadebf3b7b0 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Tue, 29 Sep 2020 17:48:39 +0100 Subject: [PATCH] Reduce configuration resolution when building a layered jar Previously, BootJar would resolves all of a project's configurations when building a layered jar. This was unnecessarily broad as it was likely to include configurations that had contributed nothing to the jar's classpath. This commit replaces the configuration resolution with an afterResolve action that populates the ResolvedDependencies in response to a configuration being resolved. This allows the resolved dependencies to be populated from all of the configurations that were resolved as part of determining the jars classpath and no more. Closes gh-23528 --- .../boot/gradle/tasks/bundling/BootJar.java | 22 +-- .../gradle/tasks/bundling/LayerResolver.java | 126 +----------------- .../tasks/bundling/ResolvedDependencies.java | 114 ++++++++++++++++ .../gradle/tasks/bundling/BootJarTests.java | 38 +++--- 4 files changed, 146 insertions(+), 154 deletions(-) create mode 100644 spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/ResolvedDependencies.java diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootJar.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootJar.java index 6ea136cadb..a7e9f2fe96 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootJar.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/BootJar.java @@ -22,16 +22,15 @@ import java.util.concurrent.Callable; import org.gradle.api.Action; import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ResolvableDependencies; import org.gradle.api.file.CopySpec; import org.gradle.api.file.FileCollection; import org.gradle.api.file.FileCopyDetails; import org.gradle.api.file.FileTreeElement; import org.gradle.api.internal.file.copy.CopyAction; -import org.gradle.api.plugins.JavaPluginConvention; import org.gradle.api.specs.Spec; import org.gradle.api.tasks.Internal; import org.gradle.api.tasks.Nested; -import org.gradle.api.tasks.SourceSet; import org.gradle.api.tasks.bundling.Jar; /** @@ -55,6 +54,8 @@ public class BootJar extends Jar implements BootArchive { private static final String CLASSPATH_INDEX = "BOOT-INF/classpath.idx"; + private final ResolvedDependencies resolvedDependencies = new ResolvedDependencies(); + private final BootArchiveSupport support; private final CopySpec bootInfSpec; @@ -73,6 +74,11 @@ public class BootJar extends Jar implements BootArchive { this.bootInfSpec = getProject().copySpec().into("BOOT-INF"); configureBootInfSpec(this.bootInfSpec); getMainSpec().with(this.bootInfSpec); + getProject().getConfigurations().all((configuration) -> { + ResolvableDependencies incoming = configuration.getIncoming(); + incoming.afterResolve( + (resolvableDependencies) -> this.resolvedDependencies.processConfiguration(configuration)); + }); } private void configureBootInfSpec(CopySpec bootInfSpec) { @@ -108,12 +114,7 @@ public class BootJar extends Jar implements BootArchive { @Override protected CopyAction createCopyAction() { if (!isLayeredDisabled()) { - JavaPluginConvention javaPluginConvention = getProject().getConvention() - .findPlugin(JavaPluginConvention.class); - Iterable sourceSets = (javaPluginConvention != null) ? javaPluginConvention.getSourceSets() - : Collections.emptySet(); - LayerResolver layerResolver = new LayerResolver(sourceSets, getConfigurations(), this.layered, - this::isLibrary); + LayerResolver layerResolver = new LayerResolver(this.resolvedDependencies, this.layered, this::isLibrary); String layerToolsLocation = this.layered.isIncludeLayerTools() ? LIB_DIRECTORY : null; return this.support.createCopyAction(this, layerResolver, layerToolsLocation); } @@ -308,4 +309,9 @@ public class BootJar extends Jar implements BootArchive { return callable; } + @Internal + ResolvedDependencies getResolvedDependencies() { + return this.resolvedDependencies; + } + } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/LayerResolver.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/LayerResolver.java index bbeee6d6e4..0d414582da 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/LayerResolver.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/LayerResolver.java @@ -17,18 +17,9 @@ package org.springframework.boot.gradle.tasks.bundling; import java.io.File; -import java.util.HashSet; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Set; -import org.gradle.api.artifacts.Configuration; -import org.gradle.api.artifacts.ModuleVersionIdentifier; -import org.gradle.api.artifacts.ResolvedArtifact; -import org.gradle.api.artifacts.ResolvedConfiguration; import org.gradle.api.file.FileCopyDetails; import org.gradle.api.specs.Spec; -import org.gradle.api.tasks.SourceSet; import org.springframework.boot.loader.tools.Layer; import org.springframework.boot.loader.tools.Library; @@ -52,9 +43,9 @@ class LayerResolver { private final Spec librarySpec; - LayerResolver(Iterable sourceSets, Iterable configurations, - LayeredSpec layeredConfiguration, Spec librarySpec) { - this.resolvedDependencies = new ResolvedDependencies(sourceSets, configurations); + LayerResolver(ResolvedDependencies resolvedDependencies, LayeredSpec layeredConfiguration, + Spec librarySpec) { + this.resolvedDependencies = resolvedDependencies; this.layeredConfiguration = layeredConfiguration; this.librarySpec = librarySpec; } @@ -89,115 +80,4 @@ class LayerResolver { return new Library(null, file, null, coordinates, false); } - /** - * Tracks and provides details of resolved dependencies in the project so we can find - * {@link LibraryCoordinates}. - */ - private static class ResolvedDependencies { - - private final Set deprecatedForResolutionConfigurationNames; - - private final Map configurationDependencies = new LinkedHashMap<>(); - - ResolvedDependencies(Iterable sourceSets, Iterable configurations) { - this.deprecatedForResolutionConfigurationNames = deprecatedForResolutionConfigurationNames(sourceSets); - configurations.forEach(this::processConfiguration); - } - - @SuppressWarnings("deprecation") - private Set deprecatedForResolutionConfigurationNames(Iterable sourceSets) { - Set configurationNames = new HashSet<>(); - configurationNames.add("archives"); - configurationNames.add("default"); - for (SourceSet sourceSet : sourceSets) { - try { - configurationNames.add(sourceSet.getCompileConfigurationName()); - } - catch (NoSuchMethodError ex) { - // Continue - } - configurationNames.add(sourceSet.getCompileOnlyConfigurationName()); - try { - configurationNames.add(sourceSet.getRuntimeConfigurationName()); - } - catch (NoSuchMethodError ex) { - // Continue - } - } - return configurationNames; - } - - private void processConfiguration(Configuration configuration) { - if (configuration.isCanBeResolved() - && !this.deprecatedForResolutionConfigurationNames.contains(configuration.getName())) { - this.configurationDependencies.put(configuration, - new ResolvedConfigurationDependencies(configuration.getResolvedConfiguration())); - } - } - - LibraryCoordinates find(File file) { - for (ResolvedConfigurationDependencies dependencies : this.configurationDependencies.values()) { - LibraryCoordinates coordinates = dependencies.find(file); - if (coordinates != null) { - return coordinates; - } - } - return null; - } - - } - - /** - * Stores details of resolved configuration dependencies. - */ - private static class ResolvedConfigurationDependencies { - - private final Map artifactCoordinates = new LinkedHashMap<>(); - - ResolvedConfigurationDependencies(ResolvedConfiguration resolvedConfiguration) { - for (ResolvedArtifact resolvedArtifact : resolvedConfiguration.getResolvedArtifacts()) { - this.artifactCoordinates.put(resolvedArtifact.getFile(), - new ModuleVersionIdentifierLibraryCoordinates(resolvedArtifact.getModuleVersion().getId())); - } - } - - LibraryCoordinates find(File file) { - return this.artifactCoordinates.get(file); - } - - } - - /** - * Adapts a {@link ModuleVersionIdentifier} to {@link LibraryCoordinates}. - */ - private static class ModuleVersionIdentifierLibraryCoordinates implements LibraryCoordinates { - - private final ModuleVersionIdentifier identifier; - - ModuleVersionIdentifierLibraryCoordinates(ModuleVersionIdentifier identifier) { - this.identifier = identifier; - } - - @Override - public String getGroupId() { - return this.identifier.getGroup(); - } - - @Override - public String getArtifactId() { - return this.identifier.getName(); - } - - @Override - public String getVersion() { - return this.identifier.getVersion(); - } - - @Override - public String toString() { - return this.identifier.toString(); - } - - } - } diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/ResolvedDependencies.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/ResolvedDependencies.java new file mode 100644 index 0000000000..45ee221d2f --- /dev/null +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/main/java/org/springframework/boot/gradle/tasks/bundling/ResolvedDependencies.java @@ -0,0 +1,114 @@ +/* + * Copyright 2012-2020 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.gradle.tasks.bundling; + +import java.io.File; +import java.util.LinkedHashMap; +import java.util.Map; + +import org.gradle.api.artifacts.Configuration; +import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.gradle.api.artifacts.ResolvedArtifact; +import org.gradle.api.artifacts.ResolvedConfiguration; + +import org.springframework.boot.loader.tools.LibraryCoordinates; + +/** + * Tracks and provides details of resolved dependencies in the project so we can find + * {@link LibraryCoordinates}. + * + * @author Madhura Bhave + * @author Scott Frederick + * @author Phillip Webb + * @author Paddy Drury + * @author Andy Wilkinson + */ +class ResolvedDependencies { + + private final Map configurationDependencies = new LinkedHashMap<>(); + + void processConfiguration(Configuration configuration) { + this.configurationDependencies.put(configuration, + new ResolvedConfigurationDependencies(configuration.getResolvedConfiguration())); + } + + LibraryCoordinates find(File file) { + for (ResolvedConfigurationDependencies dependencies : this.configurationDependencies.values()) { + LibraryCoordinates coordinates = dependencies.find(file); + if (coordinates != null) { + return coordinates; + } + } + return null; + } + + /** + * Stores details of resolved configuration dependencies. + */ + private static class ResolvedConfigurationDependencies { + + private final Map artifactCoordinates = new LinkedHashMap<>(); + + ResolvedConfigurationDependencies(ResolvedConfiguration resolvedConfiguration) { + if (!resolvedConfiguration.hasError()) { + for (ResolvedArtifact resolvedArtifact : resolvedConfiguration.getResolvedArtifacts()) { + this.artifactCoordinates.put(resolvedArtifact.getFile(), + new ModuleVersionIdentifierLibraryCoordinates(resolvedArtifact.getModuleVersion().getId())); + } + } + } + + LibraryCoordinates find(File file) { + return this.artifactCoordinates.get(file); + } + + } + + /** + * Adapts a {@link ModuleVersionIdentifier} to {@link LibraryCoordinates}. + */ + private static class ModuleVersionIdentifierLibraryCoordinates implements LibraryCoordinates { + + private final ModuleVersionIdentifier identifier; + + ModuleVersionIdentifierLibraryCoordinates(ModuleVersionIdentifier identifier) { + this.identifier = identifier; + } + + @Override + public String getGroupId() { + return this.identifier.getGroup(); + } + + @Override + public String getArtifactId() { + return this.identifier.getName(); + } + + @Override + public String getVersion() { + return this.identifier.getVersion(); + } + + @Override + public String toString() { + return this.identifier.toString(); + } + + } + +} diff --git a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootJarTests.java b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootJarTests.java index f06c46c02d..5abc46f0ae 100644 --- a/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootJarTests.java +++ b/spring-boot-project/spring-boot-tools/spring-boot-gradle-plugin/src/test/java/org/springframework/boot/gradle/tasks/bundling/BootJarTests.java @@ -21,7 +21,6 @@ import java.io.File; import java.io.IOException; import java.io.InputStreamReader; import java.util.ArrayList; -import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -32,6 +31,7 @@ import java.util.zip.ZipEntry; import org.gradle.api.Action; import org.gradle.api.artifacts.Configuration; import org.gradle.api.artifacts.ModuleVersionIdentifier; +import org.gradle.api.artifacts.ResolvableDependencies; import org.gradle.api.artifacts.ResolvedArtifact; import org.gradle.api.artifacts.ResolvedConfiguration; import org.gradle.api.artifacts.ResolvedModuleVersion; @@ -40,11 +40,12 @@ import org.gradle.api.artifacts.component.ModuleComponentIdentifier; import org.gradle.api.artifacts.component.ProjectComponentIdentifier; import org.junit.jupiter.api.Test; -import org.springframework.boot.gradle.tasks.bundling.BootJarTests.TestBootJar; import org.springframework.boot.loader.tools.JarModeLibrary; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; +import static org.mockito.BDDMockito.willAnswer; import static org.mockito.Mockito.mock; /** @@ -55,10 +56,10 @@ import static org.mockito.Mockito.mock; * @author Scott Frederick * @author Paddy Drury */ -class BootJarTests extends AbstractBootArchiveTests { +class BootJarTests extends AbstractBootArchiveTests { BootJarTests() { - super(TestBootJar.class, "org.springframework.boot.loader.JarLauncher", "BOOT-INF/lib/", "BOOT-INF/classes/"); + super(BootJar.class, "org.springframework.boot.loader.JarLauncher", "BOOT-INF/lib/", "BOOT-INF/classes/"); } @Test @@ -272,8 +273,9 @@ class BootJarTests extends AbstractBootArchiveTests { return getTask().getArchiveFile().get().getAsFile(); } + @SuppressWarnings("unchecked") private void addContent() throws IOException { - TestBootJar bootJar = getTask(); + BootJar bootJar = getTask(); bootJar.setMainClassName("com.example.Main"); File classesJavaMain = new File(this.temp, "classes/java/main"); File applicationClass = new File(classesJavaMain, "com/example/Application.class"); @@ -302,9 +304,15 @@ class BootJarTests extends AbstractBootArchiveTests { ResolvedConfiguration resolvedConfiguration = mock(ResolvedConfiguration.class); given(resolvedConfiguration.getResolvedArtifacts()).willReturn(artifacts); Configuration configuration = mock(Configuration.class); - given(configuration.isCanBeResolved()).willReturn(true); given(configuration.getResolvedConfiguration()).willReturn(resolvedConfiguration); - bootJar.setConfiguration(Collections.singleton(configuration)); + ResolvableDependencies resolvableDependencies = mock(ResolvableDependencies.class); + given(configuration.getIncoming()).willReturn(resolvableDependencies); + willAnswer((invocation) -> { + invocation.getArgument(0, Action.class).execute(resolvableDependencies); + return null; + }).given(resolvableDependencies).afterResolve(any(Action.class)); + given(configuration.getIncoming()).willReturn(resolvableDependencies); + bootJar.getResolvedDependencies().processConfiguration(configuration); } private ResolvedArtifact mockLibraryArtifact(String fileName, String group, String module, String version) { @@ -334,7 +342,6 @@ class BootJarTests extends AbstractBootArchiveTests { given(moduleVersion.getId()).willReturn(moduleVersionIdentifier); ResolvedArtifact libraryArtifact = mock(ResolvedArtifact.class); File file = new File(this.temp, fileName).getAbsoluteFile(); - System.out.println(file); given(libraryArtifact.getFile()).willReturn(file); given(libraryArtifact.getModuleVersion()).willReturn(moduleVersion); return libraryArtifact; @@ -362,19 +369,4 @@ class BootJarTests extends AbstractBootArchiveTests { getTask().copy(); } - public static class TestBootJar extends BootJar { - - private Iterable configurations = Collections.emptySet(); - - @Override - protected Iterable getConfigurations() { - return this.configurations; - } - - void setConfiguration(Iterable configurations) { - this.configurations = configurations; - } - - } - }