From 3cc6b59c8c0870aef132093e031c566cb6919ee7 Mon Sep 17 00:00:00 2001 From: Peter Kriens Date: Tue, 31 Oct 2023 17:35:25 +0100 Subject: [PATCH 1/2] Added bracketing function to Processor --- Signed-off-by: Peter Kriens Signed-off-by: Peter Kriens --- .../src/aQute/bnd/build/package-info.java | 2 +- .../src/aQute/bnd/maven/package-info.java | 2 +- .../src/aQute/bnd/osgi/Processor.java | 143 +++++++++++++++--- .../bnd/osgi/repository/package-info.java | 2 +- .../src/aQute/bnd/print/package-info.java | 2 +- .../bnd/service/generate/package-info.java | 2 +- 6 files changed, 130 insertions(+), 23 deletions(-) diff --git a/biz.aQute.bndlib/src/aQute/bnd/build/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/build/package-info.java index a7c7d47d73..b1e27609c8 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/build/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/build/package-info.java @@ -1,6 +1,6 @@ /** */ -@Version("4.5.0") +@Version("4.6.0") package aQute.bnd.build; import org.osgi.annotation.versioning.Version; diff --git a/biz.aQute.bndlib/src/aQute/bnd/maven/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/maven/package-info.java index e2c2921adb..5890fdc768 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/maven/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/maven/package-info.java @@ -1,6 +1,6 @@ /** */ -@Version("3.4.0") +@Version("3.5.0") package aQute.bnd.maven; import org.osgi.annotation.versioning.Version; diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java index b02f9ab145..6177465093 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Processor.java @@ -26,6 +26,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -38,13 +39,16 @@ import java.util.Set; import java.util.Spliterator; import java.util.TreeSet; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executor; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Consumer; import java.util.function.Predicate; +import java.util.function.Supplier; import java.util.jar.Attributes; import java.util.jar.Manifest; import java.util.regex.Matcher; @@ -57,6 +61,7 @@ import org.slf4j.LoggerFactory; import aQute.bnd.exceptions.Exceptions; +import aQute.bnd.exceptions.RunnableWithException; import aQute.bnd.header.Attrs; import aQute.bnd.header.OSGiHeader; import aQute.bnd.header.Parameters; @@ -97,16 +102,16 @@ public class Processor extends Domain implements Reporter, Registry, Constants, log = reporterAdapter; } - static final int BUFFER_SIZE = IOConstants.PAGE_SIZE * 1; - - final static ThreadLocal current = new ThreadLocal<>(); + static final int BUFFER_SIZE = IOConstants.PAGE_SIZE * 1; + final static ThreadLocal current = new ThreadLocal<>(); private static final Memoize executors = Memoize.supplier(ExecutorGroup::new); - private static final Memoize random = Memoize.supplier(Random::new); - public final static String LIST_SPLITTER = "\\s*,\\s*"; - final List errors = new ArrayList<>(); - final List warnings = new ArrayList<>(); + public final static String LIST_SPLITTER = "\\s*,\\s*"; + + final ThreadLocal bracket = new ThreadLocal<>(); + final List errors = new ArrayList<>(); + final List warnings = new ArrayList<>(); private final Set basicPlugins = Collections .newSetFromMap(new ConcurrentHashMap<>()); private final Set toBeClosed = Collections @@ -118,19 +123,19 @@ public class Processor extends Domain implements Reporter, Registry, Constants, boolean pedantic; boolean trace; boolean exceptions; - boolean fileMustExist = true; + boolean fileMustExist = true; - private File base = new File("").getAbsoluteFile(); - private URI baseURI = base.toURI(); + private File base = new File("").getAbsoluteFile(); + private URI baseURI = base.toURI(); Properties properties; String profile; private Macro replacer; private long lastModified; private File propertiesFile; - private boolean fixup = true; + private boolean fixup = true; private Processor parent; - private final CopyOnWriteArrayList included = new CopyOnWriteArrayList<>(); + private final CopyOnWriteArrayList included = new CopyOnWriteArrayList<>(); Collection filter; Boolean strict; @@ -397,7 +402,6 @@ public void addClose(AutoCloseable closeable) { toBeClosed.add(closeable); } - public void removeClose(AutoCloseable closeable) { assert closeable != null; toBeClosed.remove(closeable); @@ -631,7 +635,8 @@ private void clearPlugins() { IO.close(outgoingPluginLoader); } - public String _basedir(@SuppressWarnings("unused") String args[]) { + public String _basedir(@SuppressWarnings("unused") + String args[]) { if (base == null) throw new IllegalArgumentException("No base dir set"); @@ -1161,8 +1166,8 @@ public static String printClauses(Map> exports) throws IO return printClauses(exports, false); } - public static String printClauses(Map> exports, - @SuppressWarnings("unused") boolean checkMultipleVersions) throws IOException { + public static String printClauses(Map> exports, @SuppressWarnings("unused") + boolean checkMultipleVersions) throws IOException { StringBuilder sb = new StringBuilder(); String del = ""; for (Entry> entry : exports.entrySet()) { @@ -1212,7 +1217,6 @@ public static void printClause(Map map, StringBuilder sb) throws IOExcepti } } - /** * @param sb * @param value @@ -1474,7 +1478,6 @@ protected CL getLoader() { return pluginLoader.get(); } - private CloseableMemoize newPluginLoader() { return CloseableMemoize.closeableSupplier(() -> { CL pluginLoader = new CL(this); @@ -1484,6 +1487,7 @@ private CloseableMemoize newPluginLoader() { return pluginLoader; }); } + /* * Check if this is a valid project. */ @@ -2648,4 +2652,107 @@ public boolean isPropertySet(Set keys) { return false; } + class Bracket { + final List atEnds = new ArrayList<>(); + final Map, Object> data = new HashMap<>(); + + } + + /** + * Can be called by Processors to bracket an operation. A bracketed + * operation allows the called methods to register a Runnable for execution + * at the end of the bracket. Brackets can be nested to any depth. + * + * @param call the Callable to execute inside the bracket + * @throws Exception thrown by the callable + */ + protected T bracketed(Callable call) throws Exception { + Bracket old = bracket.get(); + bracket.set(new Bracket()); + try { + return call.call(); + } finally { + bracket.get().atEnds.forEach(this::runit); + bracket.set(old); + } + } + + /** + * Can be called by Processors to bracket an operation. A bracketed + * operation allows the called methods to register a Runnable for execution + * at the end of the bracket. Brackets can be nested to any depth. + * + * @param runnable the runnable to execute inside the bracket + * @throws Exception thrown by the runnable + */ + protected void bracketed(RunnableWithException runnable) throws Exception { + bracketed(() -> { + runnable.run(); + return null; + }); + } + + /** + * This method is intended to coalesce multiple values. Typical use case is + * if you have an error that can happen multiple times over a bracket but + * you want to report it once. To keep plugins stateless, they should not + * store data in a bracket nor do they have a callback mechanism at the end + * of a bracket. + *

+ * This method provides a unique type for the coalescing, this is best a + * class inside a method for uniqueness. The work method, takes an instance + * of the type and can do some work, for example, a name that should be + * reported at the end as a list instead for each occurrence. The factory is + * used to create the instance when the type is used for the first time in + * the bracket. + * + * 

+	 * void dosomething(Processor p, String name) {
+	 * 	class Foo extends AutoCloseable {
+	 * 		final Set names = new TreeSet<>();
+	 *
+	 * 		public void close() {
+	 * 			p.error("names too long: %s", names);
+	 * 		}
+	 * 	}
+	 * 	if (name.size() > 10) {
+	 * 		p.atEnd(Foo.class, foo -> foo.names.add(name), Foo::new);
+	 * 	}
+	 * }
+	 *
+ * + * @param the type of the worker + * @param type the worker type + * @param work the work to do + * @param factory the factory + */ + @SuppressWarnings({ + "rawtypes", "unchecked" + }) + public void atEndOfBracket(Class type, Consumer work, Supplier factory) { + Bracket b = bracket.get(); + + if (b == null) { + runit(() -> { + X x = factory.get(); + work.accept(x); + x.close(); + }); + } else { + X data = (X) b.data.computeIfAbsent(type, t -> { + X newData = factory.get(); + b.atEnds.add(newData::close); + return newData; + }); + work.accept(data); + } + } + + private void runit(RunnableWithException r) { + try { + r.run(); + } catch (Exception e) { + exception(e, "failed to run a runnable at the end of a bracket: %s", e.getMessage()); + } + } } diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/repository/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/repository/package-info.java index 5d49631c5e..6970bc3c87 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/repository/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/repository/package-info.java @@ -1,6 +1,6 @@ /** */ -@Version("3.1.0") +@Version("3.2.0") package aQute.bnd.osgi.repository; import org.osgi.annotation.versioning.Version; diff --git a/biz.aQute.bndlib/src/aQute/bnd/print/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/print/package-info.java index 9fa77110fa..f3d2ff8f22 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/print/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/print/package-info.java @@ -1,4 +1,4 @@ -@Version("2.1.0") +@Version("2.2.0") package aQute.bnd.print; import org.osgi.annotation.versioning.Version; diff --git a/biz.aQute.bndlib/src/aQute/bnd/service/generate/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/service/generate/package-info.java index 0a009bec34..8c6d59e10d 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/service/generate/package-info.java +++ b/biz.aQute.bndlib/src/aQute/bnd/service/generate/package-info.java @@ -1,2 +1,2 @@ -@org.osgi.annotation.versioning.Version("2.1.0") +@org.osgi.annotation.versioning.Version("2.2.0") package aQute.bnd.service.generate; From cf88a63f625bb37214789c63867c29e3cd1a8afb Mon Sep 17 00:00:00 2001 From: Peter Kriens Date: Tue, 31 Oct 2023 17:36:34 +0100 Subject: [PATCH 2/2] Added warning when automatic module names were calculated from JAR names. Fixes #5792 --- Signed-off-by: Peter Kriens Signed-off-by: Peter Kriens --- .../test/test/MultiReleaseTest.java | 2 +- .../test/jpms/JPMSModuleInfoPluginTest.java | 26 ++++++------- .../src/aQute/bnd/osgi/Analyzer.java | 8 ++++ .../bnd/plugin/jpms/JPMSModuleInfoPlugin.java | 37 +++++++++++-------- .../_instructions/jpms-module-info-options.md | 2 +- 5 files changed, 45 insertions(+), 30 deletions(-) diff --git a/biz.aQute.bndlib.tests/test/test/MultiReleaseTest.java b/biz.aQute.bndlib.tests/test/test/MultiReleaseTest.java index 7e66615dee..e6435ac290 100644 --- a/biz.aQute.bndlib.tests/test/test/MultiReleaseTest.java +++ b/biz.aQute.bndlib.tests/test/test/MultiReleaseTest.java @@ -71,7 +71,7 @@ public void testBuild() throws Exception { assertThat(v17.check()).isTrue(); assertThat(v9.check()).isTrue(); - assertThat(main.check()).isTrue(); + assertThat(main.check("jpms.jarname")).isTrue(); assertThat(main.getFile("generated/multirelease.main.jar")).isNotNull(); diff --git a/biz.aQute.bndlib.tests/test/test/jpms/JPMSModuleInfoPluginTest.java b/biz.aQute.bndlib.tests/test/test/jpms/JPMSModuleInfoPluginTest.java index 7e9ac2184a..9e8f31bba1 100644 --- a/biz.aQute.bndlib.tests/test/test/jpms/JPMSModuleInfoPluginTest.java +++ b/biz.aQute.bndlib.tests/test/test/jpms/JPMSModuleInfoPluginTest.java @@ -131,7 +131,7 @@ public void moduleWithOptionsIgnore() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -186,7 +186,7 @@ public void moduleWithOptionsStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -247,7 +247,7 @@ public void moduleWithOptionsMapModuleNameStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -308,7 +308,7 @@ public void moduleWithOptionsMapModuleName() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -369,7 +369,7 @@ public void moduleWithAllDynamicImportsOrOptionalIsStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -433,7 +433,7 @@ public void moduleWithAllDynamicImportsIsStatic_2() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -498,7 +498,7 @@ public void moduleWithAllDynamicImportsIsStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -562,7 +562,7 @@ public void moduleWithAllResolutionOptionalImportsIsStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -625,7 +625,7 @@ public void moduleWithNoImportsIsStatic() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -689,7 +689,7 @@ public void moduleManualConfiguration() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -767,7 +767,7 @@ public void moduleRequiresModuleB() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json.bind-api-1.0.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -843,7 +843,7 @@ public void moduleRequiresModuleA() throws Exception { b.addClasspath(IO.getFile("testresources/javax.json-api-1.1.3.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); @@ -905,7 +905,7 @@ public void moduleRequiresA() throws Exception { b.addClasspath(IO.getFile("testresources/geronimo-jcdi_2.0_spec-1.1.jar")); Jar jar = b.build(); - if (!b.check()) + if (!b.check("jpms.jarname")) fail(); Resource moduleInfo = jar.getResource(Constants.MODULE_INFO_CLASS); diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java index 18ad3c01f3..a8828827c9 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java @@ -202,6 +202,10 @@ public static Properties getManifest(File dirOrJar) throws Exception { * @throws IOException */ public void analyze() throws Exception { + bracketed(this::analyze0); + } + + private void analyze0() throws Exception { if (!analyzed) { analyzed = true; @@ -1034,6 +1038,10 @@ boolean isResourceOnly() { * @throws IOException */ public Manifest calcManifest() throws Exception { + return bracketed(this::calcManifest0); + } + + private Manifest calcManifest0() throws Exception { try { analyze(); Manifest manifest = new Manifest(); diff --git a/biz.aQute.bndlib/src/aQute/bnd/plugin/jpms/JPMSModuleInfoPlugin.java b/biz.aQute.bndlib/src/aQute/bnd/plugin/jpms/JPMSModuleInfoPlugin.java index adf4413abd..462116e1a9 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/plugin/jpms/JPMSModuleInfoPlugin.java +++ b/biz.aQute.bndlib/src/aQute/bnd/plugin/jpms/JPMSModuleInfoPlugin.java @@ -49,9 +49,9 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.TreeSet; import java.util.function.Supplier; import java.util.jar.Manifest; -import java.util.regex.Pattern; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -92,9 +92,9 @@ enum Access { public static Set parse(String input) { return switch (input.toLowerCase(Locale.ROOT)) { - case "open" ,"0x0020" , "32" -> EnumSet.of(OPEN); - case "synthetic" , "0x1000" , "4096" -> EnumSet.of(SYNTHETIC); - case "mandated" , "0x8000" ,"32768" -> EnumSet.of(MANDATED); + case "open", "0x0020", "32" -> EnumSet.of(OPEN); + case "synthetic", "0x1000", "4096" -> EnumSet.of(SYNTHETIC); + case "mandated", "0x8000", "32768" -> EnumSet.of(MANDATED); default -> { if (input.indexOf(',') > -1) { yield Strings.splitAsStream(input) @@ -125,8 +125,6 @@ public int getValue() { private static final Logger logger = LoggerFactory.getLogger(JPMSModuleInfoPlugin.class); - private static final Pattern mangledModuleName = Pattern.compile("(.*)-\\d.*"); - private static final EE DEFAULT_MODULE_EE = EE.JavaSE_11; private static final String INTERNAL_MODULE_DIRECTIVE = "-internal-module:"; @@ -168,7 +166,6 @@ public void mainSet(Analyzer analyzer, Manifest manifest) throws Exception { Packages exports = analyzer.getExports(); Packages index = new Packages(); - // Index the whole class path for (Jar jar : analyzer.getClasspath()) { JPMSModule m = new JPMSModule(jar); @@ -226,7 +223,19 @@ private String getModuleName(Analyzer analyzer, JPMSModule m, Parameters moduleI return null; } - moduleName = JPMSModule.cleanupName(jar.getName()); + String jarName = jar.getName(); + + class CMN implements AutoCloseable { + final Set names = new TreeSet<>(); + + @Override + public void close() throws Exception { + analyzer.warning("jpms.jarname calculated module names from file name: %s", names); + } + } + analyzer.atEndOfBracket(CMN.class, cmn -> cmn.names.add(jarName), CMN::new); + + moduleName = JPMSModule.cleanupName(jarName); final String name = moduleName; moduleName = moduleInfoOptions.stream() .mapValue(attrs -> attrs.get(SUBSTITUTE_ATTRIBUTE)) @@ -236,8 +245,7 @@ private String getModuleName(Analyzer analyzer, JPMSModule m, Parameters moduleI .findFirst() .orElse(moduleName); - if (logger.isDebugEnabled()) - logger.debug("Using module name '{}' for: {}", moduleName, jar); + logger.debug("Using module name '{}' for: {}", moduleName, jar); } return JPMSModule.cleanupName(moduleName); } @@ -246,7 +254,6 @@ private String name(Analyzer analyzer) { return analyzer.getProperty(AUTOMATIC_MODULE_NAME, JPMSModule.cleanupName(analyzer.getBsn())); } - private void packages(ModuleInfoBuilder builder, Analyzer analyzer) { MapStream.ofNullable(analyzer.getJar() .getDirectories()) @@ -373,8 +380,8 @@ private void requires(ModuleInfoBuilder builder, Analyzer analyzer, Entry k.equals(RESOLUTION_DIRECTIVE) && v.equals(RESOLUTION_OPTIONAL))) { + MapStream.ofNullable(importAttrs) + .noneMatch((k, v) -> k.equals(RESOLUTION_DIRECTIVE) && v.equals(RESOLUTION_OPTIONAL))) { logger.debug("Can't find a module name for imported package: {}", packageRef.getFQN()); } @@ -478,8 +485,8 @@ private void serviceLoaderProviders(ModuleInfoBuilder builder, Analyzer analyzer // We need the `register:` directive to be present for this to work. .filterValue(attrs -> attrs.containsKey(SERVICELOADER_REGISTER_DIRECTIVE)) .values() - .collect(groupingBy(attrs -> analyzer.getTypeRefFromFQN(attrs.get(SERVICELOADER_NAMESPACE)), - mapFactory(), toList())) + .collect(groupingBy(attrs -> analyzer.getTypeRefFromFQN(attrs.get(SERVICELOADER_NAMESPACE)), mapFactory(), + toList())) .forEach((typeRef, attrsList) -> { // We can't provide JPMS services from packages on the diff --git a/docs/_instructions/jpms-module-info-options.md b/docs/_instructions/jpms-module-info-options.md index a94092c689..d34de35a86 100644 --- a/docs/_instructions/jpms-module-info-options.md +++ b/docs/_instructions/jpms-module-info-options.md @@ -19,7 +19,7 @@ The `-jpms-module-info-options` instruction provides some capabilities to help t They attributes are: -- **`substitute`** - If bnd generates a module name matching the value of this attribute it should be substituted with the key of the instruction. +- **`substitute`** - If bnd generates a module name based on the file name and it matches the value of this attribute it should be substituted with the key of the instruction. e.g. -jpms-module-info-options: java.enterprise;substitute="geronimo-jcdi_2.0_spec"