Merge pull request #13661 from vespa-engine/remove-global-package_part2

Remove all traces of global packages and rename public -> exported

4 files changed, 36 insertions, 79 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 35db7b5fef3..8920acd431b 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
@@ -16,56 +16,34 @@ import java.util.jar.Manifest;
 import java.util.stream.Collectors;
 
 /**
- * "Public package" in this context means a package that is either exported or declared as "Global-Package"
- * in a bundle's manifest.
+ * Static utilities for analyzing jar files.
  *
  * @author Tony Vaagenes
  * @author ollivir
+ * @author gjoranv
  */
 public class AnalyzeBundle {
-    public static class PublicPackages {
-        public final List<Export> exports;
-        public final List<String> globals;
 
-        PublicPackages(List<Export> exports, List<String> globals) {
-            this.exports = exports;
-            this.globals = globals;
-        }
-
-        public Set<String> exportedPackageNames() {
-            return exports.stream()
-                    .map(Export::getPackageNames)
-                    .flatMap(Collection::stream)
-                    .collect(Collectors.toSet());
-        }
-
-    }
-
-    public static PublicPackages publicPackagesAggregated(Collection<File> jarFiles) {
+    public static List<Export> exportedPackagesAggregated(Collection<File> jarFiles) {
         List<Export> exports = new ArrayList<>();
-        List<String> globals = new ArrayList<>();
 
         for (File jarFile : jarFiles) {
-            PublicPackages pp = publicPackages(jarFile);
-            exports.addAll(pp.exports);
-            globals.addAll(pp.globals);
-
-            // TODO: remove and clean up everything related to global packages.
-            if (! pp.globals.isEmpty()) throw new RuntimeException("Found global packages in bundle " + jarFile.getAbsolutePath());
+            var exported = exportedPackages(jarFile);
+            exports.addAll(exported);
         }
-        return new PublicPackages(exports, globals);
+        return exports;
     }
 
-    static PublicPackages publicPackages(File jarFile) {
+    static List<Export> exportedPackages(File jarFile) {
         try {
             Optional<Manifest> jarManifest = JarFiles.getManifest(jarFile);
             if (jarManifest.isPresent()) {
                 Manifest manifest = jarManifest.get();
                 if (isOsgiManifest(manifest)) {
-                    return new PublicPackages(parseExports(manifest), parseGlobals(manifest));
+                    return parseExports(manifest);
                 }
             }
-            return new PublicPackages(Collections.emptyList(), Collections.emptyList());
+            return Collections.emptyList();
         } catch (Exception e) {
             throw new RuntimeException(String.format("Invalid manifest in bundle '%s'", jarFile.getPath()), e);
         }
@@ -75,24 +53,10 @@ public class AnalyzeBundle {
         return JarFiles.getManifest(jarFile).flatMap(AnalyzeBundle::getBundleSymbolicName);
     }
 
-    private static List<Export> parseExportsFromAttribute(Manifest manifest, String attributeName) {
-        return getMainAttributeValue(manifest, attributeName).map(ExportPackageParser::parseExports).orElseGet(() -> new ArrayList<>());
-    }
-
     private static List<Export> parseExports(Manifest jarManifest) {
-        return parseExportsFromAttribute(jarManifest, "Export-Package");
-    }
-
-    private static List<String> parseGlobals(Manifest manifest) {
-        List<Export> globals = parseExportsFromAttribute(manifest, "Global-Package");
-
-        for (Export export : globals) {
-            if (export.getParameters().isEmpty() == false) {
-                throw new RuntimeException("Parameters not valid for Global-Package.");
-            }
-        }
-
-        return globals.stream().flatMap(g -> g.getPackageNames().stream()).collect(Collectors.toList());
+        return getMainAttributeValue(jarManifest, "Export-Package")
+                .map(ExportPackageParser::parseExports)
+                .orElseGet(ArrayList::new);
     }
 
     private static Optional<String> getMainAttributeValue(Manifest manifest, String attributeName) {
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 73fdcf1c471..41420b38360 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
@@ -2,7 +2,6 @@
 package com.yahoo.container.plugin.mojo;
 
 import com.google.common.collect.Sets;
-import com.yahoo.container.plugin.bundle.AnalyzeBundle;
 import com.yahoo.container.plugin.classanalysis.Analyze;
 import com.yahoo.container.plugin.classanalysis.ClassFileMetaData;
 import com.yahoo.container.plugin.classanalysis.ExportPackageAnnotation;
@@ -27,7 +26,6 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
@@ -39,7 +37,7 @@ import java.util.jar.Manifest;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static com.yahoo.container.plugin.bundle.AnalyzeBundle.publicPackagesAggregated;
+import static com.yahoo.container.plugin.bundle.AnalyzeBundle.exportedPackagesAggregated;
 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;
@@ -98,12 +96,11 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
             Artifacts.ArtifactSet artifactSet = Artifacts.getArtifacts(project);
             warnOnUnsupportedArtifacts(artifactSet.getNonJarArtifacts());
 
-            // Packages from Export-Package and Global-Package headers in provided scoped jars
-            AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars = publicPackagesAggregated(
+            List<Export> exportedPackagesFromProvidedJars = exportedPackagesAggregated(
                     artifactSet.getJarArtifactsProvided().stream().map(Artifact::getFile).collect(Collectors.toList()));
 
             // Packages from Export-Package headers in provided scoped jars
-            Set<String> exportedPackagesFromProvidedDeps = publicPackagesFromProvidedJars.exportedPackageNames();
+            Set<String> exportedPackagesFromProvidedDeps = ExportPackages.packageNames(exportedPackagesFromProvidedJars);
 
             // Packaged defined in this project's code
             PackageTally projectPackages = getProjectClassesTally();
@@ -114,8 +111,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
             // The union of packages in the project and compile scoped jars
             PackageTally includedPackages = projectPackages.combine(compileJarsPackages);
 
-            warnIfPackagesDefinedOverlapsGlobalPackages(includedPackages.definedPackages(), publicPackagesFromProvidedJars.globals);
-            logDebugPackageSets(publicPackagesFromProvidedJars, includedPackages);
+            logDebugPackageSets(exportedPackagesFromProvidedJars, includedPackages);
 
             if (hasJdiscCoreProvided(artifactSet.getJarArtifactsProvided())) {
                 // jdisc_core being provided guarantees that log output does not contain its exported packages
@@ -129,7 +125,7 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
 
             Map<String, Import> calculatedImports = calculateImports(includedPackages.referencedPackages(),
                                                                      includedPackages.definedPackages(),
-                                                                     exportsByPackageName(publicPackagesFromProvidedJars.exports));
+                                                                     exportsByPackageName(exportedPackagesFromProvidedJars));
 
             Map<String, Optional<String>> manualImports = emptyToNone(importPackage).map(GenerateOsgiManifestMojo::getManualImports)
                     .orElseGet(HashMap::new);
@@ -144,11 +140,11 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
         }
     }
 
-    private void logDebugPackageSets(AnalyzeBundle.PublicPackages publicPackagesFromProvidedJars, PackageTally includedPackages) {
+    private void logDebugPackageSets(List<Export> exportedPackagesFromProvidedJars, PackageTally includedPackages) {
         if (getLog().isDebugEnabled()) {
             getLog().debug("Referenced packages = " + includedPackages.referencedPackages());
             getLog().debug("Defined packages = " + includedPackages.definedPackages());
-            getLog().debug("Exported packages of dependencies = " + publicPackagesFromProvidedJars.exports.stream()
+            getLog().debug("Exported packages of dependencies = " + exportedPackagesFromProvidedJars.stream()
                     .map(e -> "(" + e.getPackageNames().toString() + ", " + e.version().orElse("")).collect(Collectors.joining(", ")));
         }
     }
@@ -201,15 +197,6 @@ public class GenerateOsgiManifestMojo extends AbstractMojo {
         }
     }
 
-    private static void warnIfPackagesDefinedOverlapsGlobalPackages(Set<String> internalPackages, List<String> globalPackages)
-            throws MojoExecutionException {
-        Set<String> overlap = Sets.intersection(internalPackages, new HashSet<>(globalPackages));
-        if (! overlap.isEmpty()) {
-            throw new MojoExecutionException(
-                    "The following packages are both global and included in the bundle:\n   " + String.join("\n   ", overlap));
-        }
-    }
-
     private Collection<String> osgiExportPackages(Map<String, ExportPackageAnnotation> exportedPackages) {
         return exportedPackages.entrySet().stream().map(entry -> entry.getKey() + ";version=" + entry.getValue().osgiVersion())
                 .collect(Collectors.toList());
diff --git a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java
index 253e0727050..fd2d098d74a 100644
--- a/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java
+++ b/bundle-plugin/src/main/java/com/yahoo/container/plugin/osgi/ExportPackages.java
@@ -6,6 +6,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  * @author Tony Vaagenes
@@ -37,6 +39,7 @@ public class ExportPackages {
         public List<Parameter> getParameters() {
             return parameters;
         }
+
     }
 
     public static class Parameter {
@@ -57,6 +60,13 @@ public class ExportPackages {
         }
     }
 
+    public static Set<String> packageNames(Collection<Export> exports) {
+        return exports.stream()
+                .map(Export::getPackageNames)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+    }
+
     public static Map<String, Export> exportsByPackageName(Collection<Export> exports) {
         Map<String, Export> ret = new HashMap<>();
         for (Export export : exports) {
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 a09604b842b..133cca164f8 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
@@ -1,8 +1,8 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.plugin.bundle;
 
-import com.yahoo.container.plugin.bundle.AnalyzeBundle.PublicPackages;
 import com.yahoo.container.plugin.osgi.ExportPackages;
+import com.yahoo.container.plugin.osgi.ExportPackages.Export;
 import org.hamcrest.Description;
 import org.hamcrest.TypeSafeMatcher;
 import org.junit.Rule;
@@ -14,7 +14,6 @@ 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;
@@ -29,19 +28,16 @@ import static org.junit.Assert.assertThat;
  * @author ollivir
  */
 public class AnalyzeBundleTest {
-    private final List<ExportPackages.Export> exports;
-    private final Set<String> exportedPackageNames;
-    private final Map<String, ExportPackages.Export> exportsByPackageName;
+    private final List<Export> exports;
+    private final Map<String, Export> exportsByPackageName;
 
     File jarDir = new File("src/test/resources/jar");
 
     public AnalyzeBundleTest() {
         File notOsgi = new File(jarDir, "notAOsgiBundle.jar");
         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);
+        exports = AnalyzeBundle.exportedPackagesAggregated(Arrays.asList(notOsgi, simple));
+        exportsByPackageName = ExportPackages.exportsByPackageName(this.exports);
     }
 
     private File jarFile(String name) {
@@ -61,7 +57,7 @@ public class AnalyzeBundleTest {
 
     @Test
     public void exported_class_names_can_be_retrieved() {
-        assertThat(exportedPackageNames, is(new HashSet<>(exports.get(0).getPackageNames())));
+        assertThat(ExportPackages.packageNames(exports), is(new HashSet<>(exports.get(0).getPackageNames())));
     }
 
     @Rule
@@ -75,7 +71,7 @@ public class AnalyzeBundleTest {
         exception.expectMessage(matchesPattern("Invalid manifest in bundle '.*errorExport.jar'"));
         exception.expectCause(throwableMessage(startsWith("Failed parsing Export-Package")));
 
-        AnalyzeBundle.publicPackages(jarFile("errorExport.jar"));
+        AnalyzeBundle.exportedPackages(jarFile("errorExport.jar"));
     }
 
     private TypeSafeMatcher<String> matchesPattern(String pattern) {