Merge pull request #6270 from chrisrueger/6267-Prevent-generation-of-…

…Substitution-Packages-(Import-Package)-without-version

Remove imports of exports without a version range

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
 	 */

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
 	 */

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;