This is an automated email from the ASF dual-hosted git repository. ppkarwasz pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit d50128c15e0fdb08c2a86f212f8ec6935ca05cca Author: Piotr P. Karwasz <[email protected]> AuthorDate: Thu Jun 11 09:12:10 2026 +0200 Fix JPMS and OSGi resolution of optional dependencies (#23) The module and bundle descriptors generated for 0.1.0 declared the runtime-optional Saxon HE and Xerces dependencies as mandatory, leaving the artifact unresolvable in any JPMS or OSGi deployment that does not also provide them. This change relaxes both descriptors. The asymmetry between them is worth noting: bnd detects the reflective Class.forName("org.apache.xerces...") reference and emits a mandatory OSGi import, so both Saxon and Xerces had to be marked optional there, whereas jdeps never sees that reflective reference and so only Saxon ever entered the module descriptor. A descriptor integration test now reads the built jar and asserts that no mandatory dependency escapes the platform, so a future bnd or jdeps run that re-promotes an optional dependency back to mandatory fails the build. Assisted-By: Claude Opus 4.8 (1M context) <[email protected]> --- pom.xml | 83 ++++++++++++ .../eu/copernik/xml/factory/DescriptorIT.java | 147 +++++++++++++++++++++ 2 files changed, 230 insertions(+) diff --git a/pom.xml b/pom.xml index 9dbd01c..f09f841 100644 --- a/pom.xml +++ b/pom.xml @@ -87,6 +87,11 @@ <!-- OSGi bundle metadata: override commons-parent's org.apache.commons.* defaults. --> <commons.osgi.symbolicName>eu.copernik.xml.factory</commons.osgi.symbolicName> <commons.osgi.export>eu.copernik.xml.factory.*;version=${project.version};-noimport:=true</commons.osgi.export> + <commons.osgi.import> + net.sf.saxon.*;resolution:=optional, + org.apache.xerces.*;resolution:=optional, + * + </commons.osgi.import> <project.build.outputTimestamp>2026-04-22T00:00:00Z</project.build.outputTimestamp> <!-- No JIRA: tracked in GitHub Issues. --> @@ -341,6 +346,27 @@ </executions> </plugin> + <!-- jdeps makes Saxon.HE a hard requires; flip it to static (optional). The trailing * keeps the rest. --> + <plugin> + <groupId>org.moditect</groupId> + <artifactId>moditect-maven-plugin</artifactId> + <executions> + <execution> + <id>add-module-infos</id> + <configuration> + <module> + <moduleInfo> + <requires> + static Saxon.HE; + *; + </requires> + </moduleInfo> + </module> + </configuration> + </execution> + </executions> + </plugin> + <!-- Publish to the new Sonatype Central API. --> <plugin> <groupId>org.sonatype.central</groupId> @@ -392,4 +418,61 @@ </plugins> </build> + + <profiles> + <!-- + Regression guard for the generated OSGi and JPMS descriptors. + The JPMS test requires JDK 9+ + --> + <profile> + <id>java11-tests</id> + <activation> + <jdk>[11,)</jdk> + </activation> + <build> + <plugins> + <!-- Compile the src/test/java11 source root at release 11; the default test sources stay at release 8. --> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-compiler-plugin</artifactId> + <executions> + <execution> + <id>testCompile-java11</id> + <phase>test-compile</phase> + <goals> + <goal>testCompile</goal> + </goals> + <configuration> + <release>11</release> + <compileSourceRoots> + <compileSourceRoot>${project.basedir}/src/test/java11</compileSourceRoot> + </compileSourceRoots> + </configuration> + </execution> + </executions> + </plugin> + + <!-- Run DescriptorIT against the packaged jar; no OSGi framework or separate JPMS module needed. --> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-failsafe-plugin</artifactId> + <executions> + <execution> + <id>verify-descriptors</id> + <goals> + <goal>integration-test</goal> + <goal>verify</goal> + </goals> + <configuration> + <systemPropertyVariables> + <buildJar>${project.build.directory}/${project.build.finalName}.jar</buildJar> + </systemPropertyVariables> + </configuration> + </execution> + </executions> + </plugin> + </plugins> + </build> + </profile> + </profiles> </project> diff --git a/src/test/java11/eu/copernik/xml/factory/DescriptorIT.java b/src/test/java11/eu/copernik/xml/factory/DescriptorIT.java new file mode 100644 index 0000000..e242c18 --- /dev/null +++ b/src/test/java11/eu/copernik/xml/factory/DescriptorIT.java @@ -0,0 +1,147 @@ +/* + * SPDX-FileCopyrightText: 2026 Piotr P. Karwasz <[email protected]> + * SPDX-License-Identifier: Apache-2.0 + */ + +package eu.copernik.xml.factory; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.io.InputStream; +import java.lang.module.ModuleDescriptor; +import java.lang.module.ModuleDescriptor.Requires; +import java.util.ArrayList; +import java.util.Enumeration; +import java.util.List; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import java.util.jar.Manifest; + +import org.junit.jupiter.api.Test; + +/** + * Guards the generated OSGi and JPMS descriptors against a regression that makes any of the library's optional dependencies mandatory. + * + * <p>Both descriptors are generated from bytecode by tools whose output can shift across versions: bnd derives the {@code Import-Package} header, and jdeps + * (via moditect) derives {@code module-info}. Rather than launch an OSGi framework or a separate module layer, this test reads the two descriptors straight + * out of the built jar and asserts the same invariant against each: the only mandatory dependency may be on the platform itself. Every other dependency, such + * as Saxon HE or Xerces, must be optional, so a deployment that does not provide it still resolves.</p> + * + * <p>"Platform" means the packages an OSGi system bundle always exports, or the {@code java.*} modules the JDK always supplies. Those are the one set of + * dependencies that does not need to be optional.</p> + * + * <p>This test reads {@code module-info} through {@link ModuleDescriptor}, a Java 9 API the project's release-8 test sources cannot reference. It therefore + * lives in {@code src/test/java11} and is compiled and run only under the {@code java11-tests} profile, which activates on JDK 11 or later.</p> + */ +class DescriptorIT { + + /** + * Package prefixes an OSGi system bundle always exports, so an import of them need not be optional. {@code java} and {@code javax} cover the JRE; the two + * {@code org} entries cover the DOM and SAX APIs the JRE also ships. + */ + private static final List<String> PLATFORM_PACKAGE_PREFIXES = List.of("java", "javax", "org.w3c", "org.xml.sax"); + + /** + * Opens the built artifact named by the failsafe {@code buildJar} system property. + */ + private static JarFile openBuildJar() throws IOException { + final String jar = System.getProperty("buildJar"); + assertNotNull(jar, "System property 'buildJar' must point at the built artifact"); + return new JarFile(jar); + } + + @Test + void everyModuleRequiresIsPlatformOrOptional() throws IOException { + try (JarFile jar = openBuildJar()) { + final JarEntry entry = findModuleInfo(jar); + assertNotNull(entry, "Built jar must contain a module descriptor (root or under META-INF/versions/)"); + final ModuleDescriptor descriptor; + try (InputStream in = jar.getInputStream(entry)) { + descriptor = ModuleDescriptor.read(in); + } + + for (final Requires requires : descriptor.requires()) { + final boolean platform = requires.name().startsWith("java."); + final boolean optional = requires.modifiers().contains(Requires.Modifier.STATIC); + assertTrue(platform || optional, + "Non-platform module '" + requires.name() + "' must be an optional (static) requires, was: " + requires); + } + } + } + + @Test + void everyBundleImportIsPlatformOrOptional() throws IOException { + try (JarFile jar = openBuildJar()) { + final Manifest manifest = jar.getManifest(); + assertNotNull(manifest, "Built jar must contain a manifest"); + final String importPackage = manifest.getMainAttributes().getValue("Import-Package"); + assertNotNull(importPackage, "Bundle must declare an Import-Package header"); + + for (final String clause : splitClauses(importPackage)) { + final String pkg = packageName(clause); + final boolean platform = isPlatformPackage(pkg); + final boolean optional = clause.replace(" ", "").contains("resolution:=optional"); + assertTrue(platform || optional, + "Import of non-platform package '" + pkg + "' must be optional, was: " + clause.trim()); + } + } + } + + /** + * Locates the module descriptor in the jar. A Java 8 library carries it as a Multi-Release entry under {@code META-INF/versions/<N>/module-info.class} + * rather than at the root, so this accepts either location. + */ + private static JarEntry findModuleInfo(final JarFile jar) { + final Enumeration<JarEntry> entries = jar.entries(); + while (entries.hasMoreElements()) { + final JarEntry entry = entries.nextElement(); + final String name = entry.getName(); + if (name.equals("module-info.class") || (name.startsWith("META-INF/versions/") && name.endsWith("/module-info.class"))) { + return entry; + } + } + return null; + } + + /** + * Returns whether {@code pkg} is exported by an OSGi system bundle, i.e. it sits under one of {@link #PLATFORM_PACKAGE_PREFIXES}. + */ + private static boolean isPlatformPackage(final String pkg) { + for (final String prefix : PLATFORM_PACKAGE_PREFIXES) { + if (pkg.equals(prefix) || pkg.startsWith(prefix + ".")) { + return true; + } + } + return false; + } + + /** + * Splits an OSGi header into its clauses on commas, ignoring commas inside double-quoted attribute values such as {@code version="[1.0,2.0)"}. + */ + private static List<String> splitClauses(final String header) { + final List<String> clauses = new ArrayList<>(); + boolean quoted = false; + int start = 0; + for (int i = 0; i < header.length(); i++) { + final char c = header.charAt(i); + if (c == '"') { + quoted = !quoted; + } else if (c == ',' && !quoted) { + clauses.add(header.substring(start, i)); + start = i + 1; + } + } + clauses.add(header.substring(start)); + return clauses; + } + + /** + * Returns the package name of a clause, i.e. everything before its first {@code ;} attribute separator. + */ + private static String packageName(final String clause) { + final int semicolon = clause.indexOf(';'); + return (semicolon < 0 ? clause : clause.substring(0, semicolon)).trim(); + } +}
