From 1a3c34e68a1135ce236a5b4b000c965988397611 Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Fri, 13 May 2022 09:16:12 -0400 Subject: [PATCH 1/3] bndlib.tests: Fixes to use temp dir for temp files Signed-off-by: BJ Hargrave --- .../test/test/BuilderTest.java | 169 +++++++----------- 1 file changed, 67 insertions(+), 102 deletions(-) diff --git a/biz.aQute.bndlib.tests/test/test/BuilderTest.java b/biz.aQute.bndlib.tests/test/test/BuilderTest.java index 669eec3148..6861b3ee78 100644 --- a/biz.aQute.bndlib.tests/test/test/BuilderTest.java +++ b/biz.aQute.bndlib.tests/test/test/BuilderTest.java @@ -541,39 +541,29 @@ public void testOverlappingPackageMissesImportVersions() throws Exception { */ @Test - public void testLastModifiedForManifest() throws Exception { - File file = new File("tmp.jar"); - try { - long time = System.currentTimeMillis(); + public void testLastModifiedForManifest(@InjectTemporaryDirectory + File tmp) throws Exception { + long time = System.currentTimeMillis(); - Builder b = new Builder(); + try (Builder b = new Builder()) { b.addClasspath(IO.getFile("jar/osgi.jar")); b.setExportPackage("org.osgi.framework"); Jar build = b.build(); - try { - assertTrue(b.check()); + assertTrue(b.check()); - build.write("tmp.jar"); - Jar ajr = new Jar(file); - try { - Resource r = ajr.getResource("META-INF/MANIFEST.MF"); - assertNotNull(r); - long t = r.lastModified(); - Date date = new Date(t); - System.out.println(date + " " + t); - // TODO we need to adapt the timestamp handling - assertThat(t).as("%s %s", date, t) - .isEqualTo(1142555622000L); - } finally { - ajr.close(); - } - } finally { - build.close(); + File file = new File(tmp, "tmp.jar"); + build.write(file); + try (Jar ajr = new Jar(file)) { + Resource r = ajr.getResource("META-INF/MANIFEST.MF"); + assertNotNull(r); + long t = r.lastModified(); + Date date = new Date(t); + System.out.println(date + " " + t); + // TODO we need to adapt the timestamp handling + assertThat(t).as("%s %s", date, t) + .isEqualTo(1142555622000L); } - } finally { - file.delete(); } - } /** @@ -806,69 +796,49 @@ public void testBadPackageInfo() throws Exception { * removes A from 'a', and checks again that the last modified data changed. */ @Test - public void testRemoveClassFromPackage() throws Exception { - try { - Builder b = new Builder(); - try { - IO.getFile("bin_test/a1/a") - .mkdirs(); - IO.copy(IO.getFile("bin_test/a/A.class"), IO.getFile("bin_test/a1/a/A.class")); - IO.copy(IO.getFile("bin_test/a/B.class"), IO.getFile("bin_test/a1/a/B.class")); - Jar classpath = new Jar(IO.getFile("bin_test/a1")); - b.addClasspath(classpath); - b.setPrivatePackage("a"); - Jar result = b.build(); - Resource ra = result.getResource("a/A.class"); - Resource rb = result.getResource("a/B.class"); - long lm1 = result.lastModified(); - assertTrue(lm1 > 0, "Last modified date of bundle > 0"); - - // windows has a very low resolution sometimes - Thread.sleep(IO.isWindows() ? 1000 : 100); - - IO.getFile("bin_test/a1/a/B.class") - .delete(); - classpath.remove("a/B.class"); - classpath.updateModified(System.currentTimeMillis(), "Removed file B"); - result = b.build(); - long lm2 = result.lastModified(); - assertTrue(lm2 > lm1, "Last modified date of bundle has increased after deleting class from package"); - - // windows has a very low resolution sometimes - Thread.sleep(IO.isWindows() ? 1000 : 100); - - IO.getFile("bin_test/a1/a/A.class") - .delete(); - classpath.remove("a/A.class"); - classpath.updateModified(System.currentTimeMillis(), "Removed file A"); - - // windows has a very low resolution sometimes - Thread.sleep(IO.isWindows() ? 1000 : 100); - - result = b.build(); - long lm3 = result.lastModified(); - assertTrue(lm3 > lm2, - "Last modified date of bundle has increased after deleting last class from package"); - } finally { - b.close(); - } - } finally { - try { - IO.getFile("bin_test/a1/a/A.class") - .delete(); - } catch (Exception e) {} - try { - IO.getFile("bin_test/a1/a/B.class") - .delete(); - } catch (Exception e) {} - try { - IO.getFile("bin_test/a1/a") - .delete(); - } catch (Exception e) {} - try { - IO.getFile("bin_test/a1") - .delete(); - } catch (Exception e) {} + public void testRemoveClassFromPackage(@InjectTemporaryDirectory + File tmp) throws Exception { + try (Builder b = new Builder()) { + IO.mkdirs(IO.getFile(tmp, "a")); + IO.copy(IO.getFile("bin_test/a/A.class"), IO.getFile(tmp, "a/A.class")); + IO.copy(IO.getFile("bin_test/a/B.class"), IO.getFile(tmp, "a/B.class")); + Jar classpath = new Jar(tmp); + b.addClasspath(classpath); + b.setPrivatePackage("a"); + Jar result = b.build(); + Resource ra = result.getResource("a/A.class"); + assertThat(ra).isNotNull(); + Resource rb = result.getResource("a/B.class"); + assertThat(rb).isNotNull(); + long lm1 = result.lastModified(); + assertThat(lm1).as("Last modified date of bundle > 0") + .isGreaterThan(0L); + + // windows has a very low resolution sometimes + Thread.sleep(IO.isWindows() ? 1000 : 100); + + IO.delete(IO.getFile(tmp, "a/B.class")); + classpath.remove("a/B.class"); + classpath.updateModified(System.currentTimeMillis(), "Removed file B"); + result = b.build(); + long lm2 = result.lastModified(); + assertThat(lm2).as("Last modified date of bundle has increased after deleting class from package") + .isGreaterThan(lm1); + + // windows has a very low resolution sometimes + Thread.sleep(IO.isWindows() ? 1000 : 100); + + IO.delete(IO.getFile(tmp, "a/A.class")); + classpath.remove("a/A.class"); + classpath.updateModified(System.currentTimeMillis(), "Removed file A"); + + // windows has a very low resolution sometimes + Thread.sleep(IO.isWindows() ? 1000 : 100); + + result = b.build(); + long lm3 = result.lastModified(); + assertThat(lm3).as("Last modified date of bundle has increased after deleting last class from package") + .isGreaterThan(lm2); } } @@ -943,7 +913,7 @@ public void testDigests(@InjectTemporaryDirectory File tmp) throws Exception { Builder b = new Builder(); try { - b.addClasspath(IO.getFile(new File(""), "jar/osgi.jar")); + b.addClasspath(IO.getFile("jar/osgi.jar")); b.setExportPackage("org.osgi.framework"); b.setProperty(Constants.DIGESTS, "MD5, SHA1"); Jar jar = b.build(); @@ -1860,7 +1830,7 @@ public void testNoManifest(@InjectTemporaryDirectory Jar jar = b.build(); assertTrue(b.check()); - File f = new File(tmp, "tmp.jar"); + File f = IO.getFile(tmp, "tmp.jar"); jar.write(f); JarInputStream jin = new JarInputStream(new FileInputStream(f)); @@ -2402,11 +2372,11 @@ public void testConduit() throws Exception { } @Test - public void testSignedJarConduit() throws Exception { + public void testSignedJarConduit(@InjectTemporaryDirectory + File tmp) throws Exception { Properties p = new Properties(); p.setProperty("-conduit", "jar/osgi-3.0.0.jar"); - Builder b = new Builder(); - try { + try (Builder b = new Builder()) { b.setProperties(p); Jar jars[] = b.builds(); assertTrue(b.check()); @@ -2417,8 +2387,7 @@ public void testSignedJarConduit() throws Exception { Resource r = jar.getResource("META-INF/OSGI.RSA"); assertNotNull(r); - File f = new File("tmp.jar"); - f.deleteOnExit(); + File f = IO.getFile(tmp, "tmp.jar"); jar.write(f); try (Jar wj = new Jar(f)) { @@ -2426,8 +2395,6 @@ public void testSignedJarConduit() throws Exception { assertNotNull(wr); assertEquals(wj.getSHA256(), jar.getSHA256()); } - } finally { - b.close(); } } @@ -2911,8 +2878,7 @@ public void testImportRangeCalculatedFromClasspath_2() throws Exception { Properties base = new Properties(); base.put(Constants.IMPORT_PACKAGE, "javax.servlet,javax.servlet.http"); - String pwd = System.getProperty("user.dir"); - base.put("pwd", new File(pwd).toURI() + base.put("pwd", IO.work.toURI() .toString()); base.put("-classpath", "${pwd}/jar/jsp-api.jar,${pwd}/jar/servlet-api.jar"); @@ -2977,8 +2943,7 @@ public void testImportRangeCalculatedFromClasspath_4() throws Exception { Properties base = new Properties(); base.put(Constants.IMPORT_PACKAGE, "javax.servlet,javax.servlet.http"); - String pwd = System.getProperty("user.dir"); - base.put("pwd", new File(pwd).toURI() + base.put("pwd", IO.work.toURI() .toString()); base.put("-classpath", "${pwd}/jar/jsp-api.jar,${pwd}/jar/servlet-api.jar"); From b91428644ecc729b22276fdaf75556ffa9f90c17 Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Wed, 18 May 2022 17:17:54 -0400 Subject: [PATCH 2/3] utf8properties: Fix store(File) which did not remove leading comment Also other fixes to clean up store implementation. Signed-off-by: BJ Hargrave --- .../lib/utf8properties/UTF8Properties.java | 40 ++++++++----------- .../utf8properties/UTF8PropertiesTest.java | 20 +++++++++- 2 files changed, 36 insertions(+), 24 deletions(-) diff --git a/aQute.libg/src/aQute/lib/utf8properties/UTF8Properties.java b/aQute.libg/src/aQute/lib/utf8properties/UTF8Properties.java index f8e06e38a7..61298f986d 100644 --- a/aQute.libg/src/aQute/lib/utf8properties/UTF8Properties.java +++ b/aQute.libg/src/aQute/lib/utf8properties/UTF8Properties.java @@ -8,6 +8,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.OutputStreamWriter; import java.io.Reader; import java.io.Writer; import java.nio.ByteBuffer; @@ -144,45 +145,38 @@ private String decode(byte[] buffer) { return new String(buffer); // default decoding } + private static final Pattern LINE_SPLITTER = Pattern.compile("\n\r?"); + @Override - public void store(OutputStream out, String msg) throws IOException { + public void store(Writer writer, String msg) throws IOException { CharArrayWriter sw = new CharArrayWriter(); super.store(sw, null); - String[] lines = sw.toString() - .split("\n\r?"); + String[] lines = LINE_SPLITTER.split(sw.toString()); - byte[] newline = "\n".getBytes(UTF_8); for (String line : lines) { if (line.startsWith("#")) continue; - out.write(line.getBytes(UTF_8)); - out.write(newline); + writer.write(line); + writer.write('\n'); } } @Override - public void store(Writer out, String msg) throws IOException { - CharArrayWriter sw = new CharArrayWriter(); - super.store(sw, null); - - String[] lines = sw.toString() - .split("\n\r?"); - - for (String line : lines) { - if (line.startsWith("#")) - continue; - - out.write(line); - out.write('\n'); + public void store(OutputStream out, String msg) throws IOException { + Writer writer = new OutputStreamWriter(out, UTF_8); + try { + store(writer, msg); + } finally { + writer.flush(); } } - public void store(File out) throws IOException { - CharArrayWriter sw = new CharArrayWriter(); - super.store(sw, null); - IO.store(sw.toString(), out); + public void store(File file) throws IOException { + try (OutputStream out = IO.outputStream(file)) { + store(out, null); + } } public void store(OutputStream out) throws IOException { diff --git a/aQute.libg/test/aQute/lib/utf8properties/UTF8PropertiesTest.java b/aQute.libg/test/aQute/lib/utf8properties/UTF8PropertiesTest.java index fdfd763988..2adab324c6 100644 --- a/aQute.libg/test/aQute/lib/utf8properties/UTF8PropertiesTest.java +++ b/aQute.libg/test/aQute/lib/utf8properties/UTF8PropertiesTest.java @@ -4,6 +4,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.File; import java.io.IOException; import java.io.StringReader; import java.io.StringWriter; @@ -12,6 +13,7 @@ import org.junit.jupiter.api.Test; +import aQute.bnd.test.jupiter.InjectTemporaryDirectory; import aQute.lib.io.IO; import aQute.libg.reporter.ReporterAdapter; import aQute.service.reporter.Report.Location; @@ -381,12 +383,28 @@ public void testWrite() throws IOException { StringWriter sw = new StringWriter(); p.store(sw, null); String s = sw.toString(); - assertThat(s).contains("#comment"); + assertThat(s).doesNotStartWith("#") + .contains("#comment"); UTF8Properties p1 = new UTF8Properties(); p1.load(new StringReader(s)); assertThat(p1).containsExactlyInAnyOrderEntriesOf(p); } + @Test + public void testWriteFile(@InjectTemporaryDirectory + File tmp) throws Exception { + UTF8Properties p = new UTF8Properties(); + p.put("Foo", "Foo"); + p.put("Bar", "Bar"); + File props = new File(tmp, "props.properties"); + p.store(props); + assertThat(props).isFile(); + assertThat(IO.collect(props)).doesNotStartWith("#"); + UTF8Properties p1 = new UTF8Properties(); + p1.load(props, null); + assertThat(p1).containsExactlyInAnyOrderEntriesOf(p); + } + private void testProperty(String content, String key, String value) throws IOException { testProperty(content, key, value, null); } From 9911193c71ee1e72605c47687349237021f5a9b9 Mon Sep 17 00:00:00 2001 From: BJ Hargrave Date: Wed, 18 May 2022 17:30:44 -0400 Subject: [PATCH 3/3] bndtools container: Address regression in Open Type Hierarch\y The JDT type hierarchy search does not work properly when a source folder is not-accessible following by a binary jar which is. This caused types in the jar to be missing from the hierarchy result. So we add back the `source=project` -buildpath clause option. The value `project` is the default. So saying `source=none` on a -buildpath entry, will cause Bndtools to mark the source project as discouraged to force the compiler to use the classes in the generated jar. Fixes https://github.com/bndtools/bnd/issues/5250 Signed-off-by: BJ Hargrave --- .../builder/classpath/BndContainer.java | 4 +- .../classpath/BndContainerInitializer.java | 115 ++++++++++++------ 2 files changed, 78 insertions(+), 41 deletions(-) diff --git a/bndtools.builder/src/org/bndtools/builder/classpath/BndContainer.java b/bndtools.builder/src/org/bndtools/builder/classpath/BndContainer.java index a2fee640a0..e389796967 100644 --- a/bndtools.builder/src/org/bndtools/builder/classpath/BndContainer.java +++ b/bndtools.builder/src/org/bndtools/builder/classpath/BndContainer.java @@ -88,7 +88,7 @@ void refresh() throws CoreException { resources = null; } - private static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test", + static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test", Boolean.TRUE.toString()); private static final IClasspathAttribute PROJECT = JavaCore.newClasspathAttribute(Constants.VERSION_ATTRIBUTE, Constants.VERSION_ATTR_PROJECT); @@ -146,7 +146,7 @@ IRuntimeClasspathEntry[] getRuntimeClasspathEntries() throws JavaModelException return runtime.toArray(EMPTY_RUNTIMEENTRIES); } - private static boolean hasAttribute(IClasspathEntry cpe, IClasspathAttribute attr) { + static boolean hasAttribute(IClasspathEntry cpe, IClasspathAttribute attr) { return Arrays.stream(cpe.getExtraAttributes()) .anyMatch(attr::equals); } diff --git a/bndtools.builder/src/org/bndtools/builder/classpath/BndContainerInitializer.java b/bndtools.builder/src/org/bndtools/builder/classpath/BndContainerInitializer.java index 0ebc8d7a69..69bd750f4d 100644 --- a/bndtools.builder/src/org/bndtools/builder/classpath/BndContainerInitializer.java +++ b/bndtools.builder/src/org/bndtools/builder/classpath/BndContainerInitializer.java @@ -1,6 +1,8 @@ package org.bndtools.builder.classpath; import static java.util.stream.Collectors.toList; +import static org.bndtools.builder.classpath.BndContainer.TEST; +import static org.bndtools.builder.classpath.BndContainer.hasAttribute; import java.io.File; import java.io.IOException; @@ -10,6 +12,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.WeakHashMap; import java.util.jar.Manifest; @@ -200,13 +203,9 @@ private static File getContainerFile(IProject p) { private static class Updater { private static final IAccessRule DISCOURAGED = JavaCore.newAccessRule(new Path("**"), IAccessRule.K_DISCOURAGED | IAccessRule.IGNORE_IF_BETTER); - private static final IAccessRule NON_ACCESSIBLE = JavaCore.newAccessRule(new Path("**"), - IAccessRule.K_NON_ACCESSIBLE | IAccessRule.IGNORE_IF_BETTER); private static final IClasspathAttribute EMPTY_INDEX = JavaCore.newClasspathAttribute( IClasspathAttribute.INDEX_LOCATION_ATTRIBUTE_NAME, "platform:/plugin/" + BndtoolsBuilder.PLUGIN_ID + "/org/bndtools/builder/classpath/empty.index"); - private static final IClasspathAttribute TEST = JavaCore.newClasspathAttribute("test", - Boolean.TRUE.toString()); private static final IClasspathAttribute WITHOUT_TEST_CODE = JavaCore .newClasspathAttribute("without_test_code", Boolean.TRUE.toString()); private static final Pattern packagePattern = Pattern.compile("(?<=^|\\.)\\*(?=\\.|$)|\\."); @@ -302,7 +301,7 @@ static void setClasspathContainer(IJavaProject javaProject, BndContainer contain }, null); } - private Void calculateProjectClasspath() { + private Void calculateProjectClasspath() throws CoreException { if (!project.isOpen()) { return null; } @@ -326,17 +325,11 @@ private Void calculateProjectClasspath() { return null; } - private void calculateContainersClasspath(String instruction, Collection containers) { - boolean testattr = false; - if (instruction.equals(Constants.TESTPATH)) { - try { - testattr = Arrays.stream(javaProject.getRawClasspath()) - .filter(cpe -> cpe.getEntryKind() == IClasspathEntry.CPE_SOURCE) - .map(IClasspathEntry::getExtraAttributes) - .flatMap(Arrays::stream) - .anyMatch(TEST::equals); - } catch (JavaModelException e) {} - } + private void calculateContainersClasspath(String instruction, Collection containers) + throws CoreException { + boolean testattr = instruction.equals(Constants.TESTPATH) && Arrays.stream(javaProject.getRawClasspath()) + .filter(cpe -> cpe.getEntryKind() == IClasspathEntry.CPE_SOURCE) + .anyMatch(cpe -> hasAttribute(cpe, TEST)); for (Container c : containers) { File file = c.getFile(); assert file.isAbsolute(); @@ -383,21 +376,36 @@ private void calculateContainersClasspath(String instruction, Collection projectAccessRules = Objects.equals(source, "project") ? accessRules + : Collections.emptyList(); /* - * If not version=project, add project entry for - * source code before library entry for generated - * jar. We use NON_ACCESSIBLE access to make eclipse - * use the classes in the library entry and not the - * project entry. We use the project entry for - * source code only. + * Add project entry for source code before library + * entry for generated jar. */ - addProjectEntry(projectPath, Collections.singletonList(NON_ACCESSIBLE), extraAttrs); + addProjectEntry(otherProject.getFullPath(), projectAccessRules, extraAttrs); /* * Supply an empty index for the generated JAR of a * workspace project dependency. This prevents the @@ -405,11 +413,37 @@ private void calculateContainersClasspath(String instruction, Collection accessRules, List accessRules, - List extraAttrs) { - IPath sourceAttachmentPath = calculateSourceAttachmentPath(path, file); - builder.entry(JavaCore.newLibraryEntry(path, sourceAttachmentPath, null, toAccessRulesArray(accessRules), - toClasspathAttributesArray(extraAttrs), false)); + List extraAttrs, IPath sourceAttachmentPath, IPath sourceAttachmentRootPath) { + IClasspathEntry libraryEntry = JavaCore.newLibraryEntry(path, sourceAttachmentPath, + sourceAttachmentRootPath, toAccessRulesArray(accessRules), toClasspathAttributesArray(extraAttrs), + false); + builder.entry(libraryEntry); builder.updateLastModified(file.lastModified()); }