From a95fe11e7ee67fe0cc03f671c89d0a0a3460f5bd Mon Sep 17 00:00:00 2001 From: Christoph Rueger Date: Sat, 14 Sep 2024 13:14:32 +0200 Subject: [PATCH] Remove imports of exports without a version range imports (coming from exports) should not be added if the export does not specify a version, because it would lead to imports without a version. this could cause resolver problems because the resolver then has no version constrains and too many wiring options. Signed-off-by: Christoph Rueger --- .../test/test/BuilderTest.java | 37 ++++++++++++++++++- .../test/test/component/DSAnnotationTest.java | 23 +++++++++++- .../src/aQute/bnd/osgi/Analyzer.java | 16 ++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) diff --git a/biz.aQute.bndlib.tests/test/test/BuilderTest.java b/biz.aQute.bndlib.tests/test/test/BuilderTest.java index 14b86a7867..dd36dbff46 100644 --- a/biz.aQute.bndlib.tests/test/test/BuilderTest.java +++ b/biz.aQute.bndlib.tests/test/test/BuilderTest.java @@ -452,9 +452,11 @@ public void test1017UsingPrivatePackagesVersion() throws Exception { @Test public void testNoImportForUsedExport_971() throws Exception { + // exports with version should be added to imports Builder b = new Builder(); b.addClasspath(new File("bin_test")); - b.setExportPackage("test.missingimports_971.p1,test.missingimports_971.p2,test.missingimports_971.p4"); + b.setExportPackage( + "test.missingimports_971.p1;version=1.1.0,test.missingimports_971.p2;version=1.1.0,test.missingimports_971.p4;version=1.1.0"); b.setPrivatePackage("test.missingimports_971.p3"); b.build(); assertTrue(b.check()); @@ -473,6 +475,39 @@ public void testNoImportForUsedExport_971() throws Exception { .getManifest() .write(System.out); } + + + /** + * Counterpart of {@link #testNoImportForUsedExport_971()} + * + * @throws Exception + */ + @Test + public void testEnsureNoImportForUsedExport_971_WithMissingExportVersion() throws Exception { + // exports without version should not be added to imports + Builder b = new Builder(); + b.addClasspath(new File("bin_test")); + b.setExportPackage( + "test.missingimports_971.p1,test.missingimports_971.p2,test.missingimports_971.p4"); + b.setPrivatePackage("test.missingimports_971.p3"); + b.build(); + assertTrue(b.check()); + + assertTrue(b.getExports() + .containsFQN("test.missingimports_971.p1")); + assertTrue(b.getExports() + .containsFQN("test.missingimports_971.p2")); + assertTrue(b.getExports() + .containsFQN("test.missingimports_971.p4")); + assertFalse(b.getImports() + .containsFQN("test.missingimports_971.p1")); + assertFalse(b.getImports() + .containsFQN("test.missingimports_971.p2")); + b.getJar() + .getManifest() + .write(System.out); + } + /* * Private package header doesn't allow the use of negation (!) #840 */ diff --git a/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java b/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java index 4d4cef223f..6a6e05c7b1 100644 --- a/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java +++ b/biz.aQute.bndlib.tests/test/test/component/DSAnnotationTest.java @@ -198,9 +198,10 @@ public void testDuplicateExtender() throws Exception { // #2876 @Test public void testExportComponentImplPackage() throws Exception { + // exports with version should be added to imports try (Builder b = new Builder()) { b.setProperty(Constants.DSANNOTATIONS, "test.component.ds14.*"); - b.setProperty("Export-Package", "test.component.ds14"); + b.setProperty("Export-Package", "test.component.ds14;version=1.1.0"); b.addClasspath(new File("bin_test")); Jar jar = b.build(); @@ -214,6 +215,26 @@ public void testExportComponentImplPackage() throws Exception { } } + // #2876 + @Test + public void testExportComponentImplPackage2() throws Exception { + // exports without version should not be added to imports + try (Builder b = new Builder()) { + b.setProperty(Constants.DSANNOTATIONS, "test.component.ds14.*"); + b.setProperty("Export-Package", "test.component.ds14"); + b.addClasspath(new File("bin_test")); + Jar jar = b.build(); + + if (!b.check()) + fail(); + Domain domain = Domain.domain(jar.getManifest()); + Parameters exportPackages = domain.getExportPackage(); + assertThat(exportPackages).containsOnlyKeys("test.component.ds14"); + Parameters importPackages = domain.getImportPackage(); + assertThat(importPackages).doesNotContainKeys("test.component.ds14"); + } + } + /** * Property test */ diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java index 513cfa78be..5002d8ebb6 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Analyzer.java @@ -1922,6 +1922,22 @@ Packages doExportsToImports(Packages exports) { String noimport = parameters.get(NO_IMPORT_DIRECTIVE); return !Boolean.parseBoolean(noimport); }) + // Remove packages without a version range + // because this would lead imports without a version. + // and this can cause to surprising resolver problems + // because the resolver has too many options in case multiple + // provideers of that package + .filter(p -> { + Attrs parameters = exports.get(p); + if (parameters == null) { + return true; + } + // check only for presence of version (not validity, because + // this done by #check()) + // (see also test.VerifierTest.testStrict()) + return parameters.getVersion() != null; + }) + // Clean up attributes and generate result map .collect(toMap(p -> p, p -> new Attrs(), (a1, a2) -> a1, Packages::new)); return result;