From d87c742c40cdf2f6d242ad61cae189be63e720db Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 17 Jul 2019 15:40:15 +0200 Subject: Log embedded packages that are presumably unnecessary (included in both compile scoped jars and provided jars) * Add methods and refactor to facilitate this feature. --- .../container/plugin/bundle/AnalyzeBundle.java | 11 +++++++- .../plugin/mojo/GenerateOsgiManifestMojo.java | 32 +++++++++++++++------- .../container/plugin/bundle/AnalyzeBundleTest.java | 9 ++++++ 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java index ecd95dbfdba..0626c786822 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/bundle/AnalyzeBundle.java @@ -11,6 +11,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.jar.Manifest; import java.util.stream.Collectors; @@ -26,10 +27,18 @@ public class AnalyzeBundle { public final List exports; public final List globals; - public PublicPackages(List exports, List globals) { + PublicPackages(List exports, List globals) { this.exports = exports; this.globals = globals; } + + public Set exportedPackageNames() { + return exports.stream() + .map(Export::getPackageNames) + .flatMap(Collection::stream) + .collect(Collectors.toSet()); + } + } public static PublicPackages publicPackagesAggregated(Collection jarFiles) { diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java index 2389849ad02..685cda66a16 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/mojo/GenerateOsgiManifestMojo.java @@ -100,6 +100,9 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = publicPackagesAggregated( artifactSet.getJarArtifactsProvided().stream().map(Artifact::getFile).collect(Collectors.toList())); + // Packages from Export-Package headers in provided scoped jars + Set exportedPackagesFromProvidedDeps = publicPackagesFromProvidedJars.exportedPackageNames(); + // Packaged defined in this project's code PackageTally projectPackages = getProjectClassesTally(); @@ -110,16 +113,16 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { PackageTally includedPackages = projectPackages.combine(compileJarsPackages); warnIfPackagesDefinedOverlapsGlobalPackages(includedPackages.definedPackages(), publicPackagesFromProvidedJars.globals); - logDebugPackageSets(publicPackagesFromProvidedJars, includedPackages); if (hasJdiscCoreProvided(artifactSet.getJarArtifactsProvided())) { - // jdisc_core being provided, guarantees that log output does not contain its exported packages - logMissingPackages(publicPackagesFromProvidedJars.exports, projectPackages, compileJarsPackages, includedPackages); + // jdisc_core being provided guarantees that log output does not contain its exported packages + logMissingPackages(exportedPackagesFromProvidedDeps, projectPackages, compileJarsPackages, includedPackages); } else { getLog().warn("This project does not have jdisc_core as provided dependency, so the " + "generated 'Import-Package' OSGi header may be missing important packages."); } + logUnnecessaryPackages(compileJarsPackages, exportedPackagesFromProvidedDeps); Map calculatedImports = calculateImports(includedPackages.referencedPackages(), includedPackages.definedPackages(), @@ -138,6 +141,20 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { } } + /** + * This mostly detects packages re-exported via composite bundles like jdisc_core and container-disc. + * An artifact can only be represented once, either in compile or provided scope. So if the project + * adds an artifact in compile scope that we deploy as a pre-installed bundle, we won't see the same + * artifact as provided via container-dev and hence can't detect the duplicate packages. + */ + private void logUnnecessaryPackages(PackageTally compileJarsPackages, Set exportedPackagesFromProvidedDeps) { + Set unnecessaryPackages = Sets.intersection(compileJarsPackages.definedPackages(), exportedPackagesFromProvidedDeps); + if (! unnecessaryPackages.isEmpty()) { + getLog().info("Compile scoped jars contain the following packages that are most likely " + + "available from jdisc runtime: " + unnecessaryPackages); + } + } + private void logDebugPackageSets(AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars, PackageTally includedPackages) { if (getLog().isDebugEnabled()) { getLog().debug("Referenced packages = " + includedPackages.referencedPackages()); @@ -151,17 +168,12 @@ public class GenerateOsgiManifestMojo extends AbstractMojo { return providedArtifacts.stream().anyMatch(artifact -> artifact.getArtifactId().equals("jdisc_core")); } - private void logMissingPackages(List exportedPackagesFromProvidedJars, + private void logMissingPackages(Set exportedPackagesFromProvidedJars, PackageTally projectPackages, PackageTally compileJarPackages, PackageTally includedPackages) { - Set exportedPackagesFromProvidedDeps = exportedPackagesFromProvidedJars - .stream() - .map(Export::getPackageNames) - .flatMap(Collection::stream) - .collect(Collectors.toSet()); - Set definedAndExportedPackages = Sets.union(includedPackages.definedPackages(), exportedPackagesFromProvidedDeps); + Set definedAndExportedPackages = Sets.union(includedPackages.definedPackages(), exportedPackagesFromProvidedJars); Set missingProjectPackages = projectPackages.referencedPackagesMissingFrom(definedAndExportedPackages); if (! missingProjectPackages.isEmpty()) { diff --git a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java index 8cccf0598ab..a09604b842b 100644 --- a/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java +++ b/bundle-plugin/src/test/java/com/yahoo/container/plugin/bundle/AnalyzeBundleTest.java @@ -11,8 +11,10 @@ import org.junit.rules.ExpectedException; import java.io.File; import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import static com.yahoo.container.plugin.classanalysis.TestUtilities.throwableMessage; import static org.hamcrest.CoreMatchers.containsString; @@ -28,6 +30,7 @@ import static org.junit.Assert.assertThat; */ public class AnalyzeBundleTest { private final List exports; + private final Set exportedPackageNames; private final Map exportsByPackageName; File jarDir = new File("src/test/resources/jar"); @@ -37,6 +40,7 @@ public class AnalyzeBundleTest { File simple = new File(jarDir, "simple1.jar"); PublicPackages pp = AnalyzeBundle.publicPackagesAggregated(Arrays.asList(notOsgi, simple)); this.exports = pp.exports; + this.exportedPackageNames = pp.exportedPackageNames(); this.exportsByPackageName = ExportPackages.exportsByPackageName(exports); } @@ -55,6 +59,11 @@ public class AnalyzeBundleTest { assertThat(exportsByPackageName.keySet(), hasItem("com.yahoo.sample.exported.package")); } + @Test + public void exported_class_names_can_be_retrieved() { + assertThat(exportedPackageNames, is(new HashSet<>(exports.get(0).getPackageNames()))); + } + @Rule public ExpectedException exception = ExpectedException.none(); -- cgit v1.2.3