From 28e10125cd97ee7846f72bce4a42b1ea1ccca027 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 1 Jun 2023 13:12:58 +0200 Subject: Warn when a USER bundle imports non-PublicApi packages. --- .../container/plugin/bundle/AnalyzeBundle.java | 18 ++++++++++++++++++ .../container/plugin/classanalysis/Packages.java | 21 +++++++++++++++++++++ .../plugin/mojo/GenerateOsgiManifestMojo.java | 16 +++++++++++++++- 3 files changed, 54 insertions(+), 1 deletion(-) (limited to 'bundle-plugin') 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 fe3d149a308..af6c82023ab 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 @@ -7,12 +7,14 @@ import com.yahoo.container.plugin.util.JarFiles; import java.io.File; import java.util.ArrayList; +import java.util.Arrays; 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; /** * Static utilities for analyzing jar files. @@ -43,6 +45,22 @@ public class AnalyzeBundle { } } + public static List publicApiPackagesAggregated(Collection jarFiles) { + return jarFiles.stream() + .map(AnalyzeBundle::publicApiPackages) + .flatMap(List::stream) + .distinct() + .toList(); + } + + static List publicApiPackages(File jarFile) { + var manifest = getOsgiManifest(jarFile); + if (manifest == null) return Collections.emptyList(); + return getMainAttributeValue(manifest, "X-JDisc-PublicApi-Package") + .map(s -> Arrays.asList(s.split(","))) + .orElseGet(ArrayList::new); + } + private static Manifest getOsgiManifest(File jarFile) { Optional jarManifest = JarFiles.getManifest(jarFile); if (jarManifest.isPresent()) { diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/Packages.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/Packages.java index 9eef8a55c01..48a128c2f0d 100644 --- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/Packages.java +++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/classanalysis/Packages.java @@ -1,8 +1,13 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.plugin.classanalysis; +import com.yahoo.container.plugin.osgi.ImportPackages; + import java.util.HashSet; +import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; /** * Utility methods related to packages. @@ -31,6 +36,22 @@ public class Packages { } } + /** + * Returns the imported Vespa packages that don't exist in the given set of allowed packages. + */ + public static List disallowedVespaImports(Map imports, List allowed) { + if (imports == null || imports.isEmpty()) return List.of(); + + var publicApi = allowed == null ? Set.of() : new HashSet<>(allowed); + + Set yahooImports = imports.keySet().stream() + .filter(pkg -> pkg.startsWith("com.yahoo") || pkg.startsWith("ai.vespa.")) + .collect(Collectors.toSet()); + + List disallowedImports = yahooImports.stream().collect(Collectors.groupingBy(publicApi::contains)).get(false); + return disallowedImports == null ? List.of() : disallowedImports; + } + public static PackageMetaData analyzePackages(Set allClasses) { Set definedPackages = new HashSet<>(); Set referencedPackages = new HashSet<>(); 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 d1c6c5d20c3..d5c9d905a16 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 @@ -26,6 +26,8 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.container.plugin.bundle.AnalyzeBundle.exportedPackagesAggregated; +import static com.yahoo.container.plugin.bundle.AnalyzeBundle.publicApiPackagesAggregated; +import static com.yahoo.container.plugin.classanalysis.Packages.disallowedVespaImports; import static com.yahoo.container.plugin.osgi.ExportPackages.exportsByPackageName; import static com.yahoo.container.plugin.osgi.ImportPackages.calculateImports; import static com.yahoo.container.plugin.util.Files.allDescendantFiles; @@ -79,7 +81,9 @@ public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { throwIfInternalContainerArtifactsAreIncluded(artifactSet.getJarArtifactsToInclude()); List providedJarArtifacts = artifactSet.getJarArtifactsProvided(); - List exportedPackagesFromProvidedJars = exportedPackagesAggregated(providedJarArtifacts.stream().map(Artifact::getFile).toList()); + List providedJarFiles = providedJarArtifacts.stream().map(Artifact::getFile).toList(); + List exportedPackagesFromProvidedJars = exportedPackagesAggregated(providedJarFiles); + List publicApiPackagesFromProvidedJars = publicApiPackagesAggregated(providedJarFiles); // Packages from Export-Package/PublicApi headers in provided scoped jars Set exportedPackagesFromProvidedDeps = ExportPackages.packageNames(exportedPackagesFromProvidedJars); @@ -109,6 +113,7 @@ public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { includedPackages.definedPackages(), exportsByPackageName(exportedPackagesFromProvidedJars)); + logNonPublicApiUsage(calculatedImports, publicApiPackagesFromProvidedJars); Map manifestContent = generateManifestContent(artifactSet.getJarArtifactsToInclude(), calculatedImports, includedPackages); addAdditionalManifestProperties(manifestContent, includedPackages); @@ -119,6 +124,15 @@ public class GenerateOsgiManifestMojo extends AbstractGenerateOsgiManifestMojo { } } + private void logNonPublicApiUsage(Map calculatedImports, List publicApiPackagesFromProvidedJars) { + if (bundleType != BundleType.USER) return; + + List nonPublicApiUsed = disallowedVespaImports(calculatedImports, publicApiPackagesFromProvidedJars); + if (! nonPublicApiUsed.isEmpty()) { + getLog().warn("This project uses packages that are not part of Vespa's public api: %s".formatted(nonPublicApiUsed)); + } + } + private String wantedProvidedDependency() { return switch (effectiveBundleType()) { case CORE -> "jdisc_core"; -- cgit v1.2.3