From 226d669371f05f06208eff5d96242c98ccb23e5d Mon Sep 17 00:00:00 2001 From: Christoph Rueger Date: Mon, 14 Oct 2024 21:17:54 +0200 Subject: [PATCH] add MergeFiles & new globing dup_* directives to handle -includeresource expressions like this, as suggested by @pkriens : -includeresource @jar1.jar, @jar2.jar;dup_overwrite:=*, @jar3.jar;dup_skip:="META-INF/services/*,META-INF/MANIFEST.MF" The value of these directives is a list of globs on the paths in the resource. Priority is probably merge (if plugin exists), overwrite, skip. Error/Warning should always be executed even if it matches the other ones. Signed-off-by: Christoph Rueger rework to MergeFiles plugin --- .../test/test/IncludeResourceTest.java | 50 ++++++- .../src/aQute/bnd/osgi/Builder.java | 130 ++++++++++++++++-- biz.aQute.bndlib/src/aQute/bnd/osgi/Jar.java | 42 +----- .../osgi/metainf/MetaInfServiceMerger.java | 39 ++++++ .../aQute/bnd/service/merge/MergeFiles.java | 9 ++ .../aQute/bnd/service/merge/package-info.java | 2 + 6 files changed, 214 insertions(+), 58 deletions(-) create mode 100644 biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java create mode 100644 biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java create mode 100644 biz.aQute.bndlib/src/aQute/bnd/service/merge/package-info.java diff --git a/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java b/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java index 071f08e4fad..ff7a1749321 100644 --- a/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java +++ b/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.File; @@ -185,13 +186,13 @@ private String testPreprocessing(String ir, String resource, String... checks) t } @Test - public void testIncludeResourceAppendDuplicates() throws Exception { + public void testIncludeResourceDuplicatesMerge() throws Exception { try (Builder a = new Builder();) { a.addClasspath(new File("jar/jarA.jar")); a.addClasspath(a.getFile("jar/jarB.jar")); a.setIncludeResource( - "@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;:duplicates:=APPEND"); + "@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;dup_merge:=*"); Jar jar = a.build(); assertTrue(a.check()); @@ -199,17 +200,38 @@ public void testIncludeResourceAppendDuplicates() throws Exception { .containsKey("META-INF/services")); Resource resource = jar.getResource("META-INF/services/foo"); - assertEquals("ab", IO.collect(resource.openInputStream())); + assertEquals("a\nb", IO.collect(resource.openInputStream())); } } @Test - public void testIncludeResourceAppendDuplicatesLiteral(@InjectTemporaryDirectory + public void testIncludeResourceDuplicatesError() throws Exception { + + try (Builder a = new Builder();) { + a.addClasspath(new File("jar/jarA.jar")); + a.addClasspath(a.getFile("jar/jarB.jar")); + a.setIncludeResource("@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;dup_error:=*"); + Jar jar = a.build(); + assertFalse(a.check()); + assertEquals("Duplicate file overwritten: META-INF/services/foo", a.getErrors() + .get(0)); + + assertTrue(jar.getDirectories() + .containsKey("META-INF/services")); + + Resource resource = jar.getResource("META-INF/services/foo"); + assertEquals("b", IO.collect(resource.openInputStream())); + + } + } + + @Test + public void testIncludeResourceLiteralDuplicatesMerge(@InjectTemporaryDirectory File tmp) throws Exception { try (Builder b = new Builder()) { - b.setIncludeResource("/a/a.txt;literal='a', /a/a.txt;literal='b';:duplicates:=APPEND"); + b.setIncludeResource("/a/a.txt;literal='a', /a/a.txt;literal='b';dup_merge:=*"); b.build(); assertTrue(b.check()); @@ -219,4 +241,22 @@ public void testIncludeResourceAppendDuplicatesLiteral(@InjectTemporaryDirectory assertEquals("ab", IO.collect(IO.getFile(tmp, "a/a.txt"))); } } + + @Test + public void testIncludeResourceLiteralDuplicatesError(@InjectTemporaryDirectory + File tmp) throws Exception { + + try (Builder b = new Builder()) { + b.setIncludeResource("/a/a.txt;literal='a', /a/a.txt;literal='b';dup_error:=*"); + b.build(); + assertFalse(b.check()); + assertEquals("Duplicate file overwritten: /a/a.txt", b.getErrors() + .get(0)); + + b.getJar() + .writeFolder(tmp); + + assertEquals("b", IO.collect(IO.getFile(tmp, "a/a.txt"))); + } + } } diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java index e42c838cf8a..3ec7cc2c6e4 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java @@ -14,7 +14,9 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.io.SequenceInputStream; import java.net.URI; +import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -26,6 +28,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; @@ -42,6 +45,7 @@ import aQute.bnd.cdi.CDIAnnotations; import aQute.bnd.component.DSAnnotations; import aQute.bnd.differ.DiffPluginImpl; +import aQute.bnd.exceptions.Exceptions; import aQute.bnd.header.Attrs; import aQute.bnd.header.OSGiHeader; import aQute.bnd.header.Parameters; @@ -55,7 +59,7 @@ import aQute.bnd.metatype.MetatypeAnnotations; import aQute.bnd.osgi.Descriptors.PackageRef; import aQute.bnd.osgi.Descriptors.TypeRef; -import aQute.bnd.osgi.Jar.DupStrategy; +import aQute.bnd.osgi.metainf.MetaInfServiceMerger; import aQute.bnd.osgi.metainf.MetaInfServiceParser; import aQute.bnd.plugin.jpms.JPMSAnnotations; import aQute.bnd.plugin.jpms.JPMSModuleInfoPlugin; @@ -66,6 +70,7 @@ import aQute.bnd.service.diff.Diff; import aQute.bnd.service.diff.Tree; import aQute.bnd.service.diff.Type; +import aQute.bnd.service.merge.MergeFiles; import aQute.bnd.service.specifications.BuilderSpecification; import aQute.bnd.stream.MapStream; import aQute.bnd.unmodifiable.Maps; @@ -78,6 +83,7 @@ import aQute.lib.strings.Strings; import aQute.lib.zip.ZipUtil; import aQute.libg.generics.Create; +import aQute.libg.glob.Glob; import aQute.libg.re.RE; /** @@ -1377,8 +1383,6 @@ public boolean addAll(Jar to, Jar sub, Instruction filter, String destination) { private boolean addAll(Jar to, Jar sub, Instruction filter, String destination, Function modifier, Map extra) { - DupStrategy dupStrategy = dupStrategy(extra); - boolean dupl = false; for (String name : sub.getResources() .keySet()) { @@ -1390,8 +1394,19 @@ private boolean addAll(Jar to, Jar sub, Instruction filter, String destination, if (filter == null || filter.matches(name) ^ filter.isNegated()) { - dupl |= to.putResource(Processor.appendPath(destination, modifier.apply(name)), sub.getResource(name), - dupStrategy); + String path = Processor.appendPath(destination, modifier.apply(name)); + Resource resource = sub.getResource(name); + Resource existing = to.getResource(path); + boolean duplicate = existing != null; + + if (!duplicate) { + dupl |= to.putResource(path, resource, true); + } else { + Optional maybeMerged = DupStrategy.onDuplicate(path, existing, resource, extra, this); + if (maybeMerged.isPresent()) { + dupl |= to.putResource(path, maybeMerged.get()); + } + } } } @@ -1433,9 +1448,18 @@ private void copy(Jar jar, String path, File from, Instructions preprocess, Map< } private void copy(Jar jar, String path, Resource resource, Map extra) { - DupStrategy dupStrategy = dupStrategy(extra); + Resource existing = jar.getResource(path); + + boolean duplicate = existing != null; + if (!duplicate) { + jar.putResource(path, resource, true); + } else { + Optional maybeMerged = DupStrategy.onDuplicate(path, existing, resource, extra, this); + if (maybeMerged.isPresent()) { + jar.putResource(path, maybeMerged.get(), true); + } + } - jar.putResource(path, resource, dupStrategy); if (isTrue(extra.get(LIB_DIRECTIVE))) { setProperty(BUNDLE_CLASSPATH, append(getProperty(BUNDLE_CLASSPATH, "."), path)); } @@ -1775,6 +1799,28 @@ public Pattern getDoNotCopy() { static SPIDescriptorGenerator spiDescriptorGenerator = new SPIDescriptorGenerator(); static JPMSMultiReleasePlugin jpmsReleasePlugin = new JPMSMultiReleasePlugin(); static MetaInfServiceParser metaInfoServiceParser = new MetaInfServiceParser(); + static MetaInfServiceMerger metaInfoServiceMerger = new MetaInfServiceMerger(); + static MergeFiles defaultResourceMerger = new MergeFiles() { + + @Override + public Optional merge(String path, Resource a, + Resource b) { + + try ( + SequenceInputStream in = new SequenceInputStream( + a.openInputStream(), b.openInputStream())) { + + long lastModified = Math.max(a.lastModified(), + b.lastModified()); + return Optional.of(new EmbeddedResource( + ByteBuffer.wrap(in.readAllBytes()), + lastModified)); + } catch (Exception e) { + throw Exceptions.duck(e); + } + + } + }; @Override protected void setTypeSpecificPlugins(PluginsContainer pluginsContainer) { @@ -1789,6 +1835,8 @@ protected void setTypeSpecificPlugins(PluginsContainer pluginsContainer) { pluginsContainer.add(spiDescriptorGenerator); pluginsContainer.add(jpmsReleasePlugin); pluginsContainer.add(metaInfoServiceParser); + pluginsContainer.add(metaInfoServiceMerger); + pluginsContainer.add(defaultResourceMerger); super.setTypeSpecificPlugins(pluginsContainer); } @@ -2095,11 +2143,67 @@ public String system(boolean allowFail, String command, String input) throws IOE return cachedSystemCalls.computeIfAbsent(key, asFunction(k -> super.system(allowFail, command, input))); } - private DupStrategy dupStrategy(Map extra) { - String val = extra.get(":duplicates:"); - DupStrategy dupStrategy = val != null ? DupStrategy.valueOf(val.trim() - .toUpperCase()) - : DupStrategy.OVERWRITE; - return dupStrategy; + + + + + /** + * Helper for handling inluderesource duplicates. + */ + private final class DupStrategy { + + private static final String DUP_MSG = "Duplicate file overwritten: %s"; + + private static Optional onDuplicate(String path, Resource existing, Resource resource, + Map extra, + Processor proc) { + // The value of these directives is a list of globs on the paths in + // the resource. + String dup_overwrite = extra.get("dup_overwrite:"); + String dup_merge = extra.get("dup_merge:"); + String dup_error = extra.get("dup_error:"); + String dup_warning = extra.get("dup_warning:"); + String dup_skip = extra.get("dup_skip:"); + + + if (matches(path, dup_error)) { + proc.error(DUP_MSG, path); + } + if (matches(path, dup_warning)) { + proc.warning(DUP_MSG, path); + } + + if (matches(path, dup_merge)) { + + return proc.getPlugins(MergeFiles.class) + .stream() + .map(mf -> mf.merge(path, existing, resource)) + .filter(Optional::isPresent) + .findFirst() + .orElse(Optional.of(resource)); + + } else if (matches(path, dup_overwrite)) { + return Optional.ofNullable(resource); + } else if (matches(path, dup_skip)) { + return Optional.ofNullable(null); + } + + + return Optional.ofNullable(resource); + } + + private static boolean matches(String path, String globs) { + if(globs == null) { + return false; + } + + // default is '*' if blank + if (globs.isBlank() || globs.trim() + .equals("*")) { + return Glob.ALL.matches(path); + } + + return Stream.of(globs.split(",")).anyMatch(glob -> new Glob(glob).matches(path)); + } } } diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Jar.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Jar.java index 109496c3951..9547ac69982 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Jar.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Jar.java @@ -11,7 +11,6 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.SequenceInputStream; import java.net.URI; import java.nio.ByteBuffer; import java.nio.file.FileVisitOption; @@ -73,25 +72,6 @@ public class Jar implements Closeable { - /** - * Controls how duplicate resources are handled which are put into a jar - * file. - */ - public enum DupStrategy { - /** - * default: overwrite existing - */ - OVERWRITE, - /** - * append (concatenate) the new to the existing - */ - APPEND, - /** - * skip if already exist - */ - NOTHING - } - private static final int BUFFER_SIZE = IOConstants.PAGE_SIZE * 16; /** * Note that setting the January 1st 1980 (or even worse, "0", as time) @@ -379,10 +359,6 @@ public boolean putResource(String path, Resource resource) { } public boolean putResource(String path, Resource resource, boolean overwrite) { - return putResource(path, resource, overwrite ? DupStrategy.OVERWRITE : DupStrategy.NOTHING); - } - - public boolean putResource(String path, Resource resource, DupStrategy strategy) { check(); path = ZipUtil.cleanPath(path); @@ -407,29 +383,15 @@ public boolean putResource(String path, Resource resource, DupStrategy strategy) Resource existing = s.get(path); boolean duplicate = existing != null; - if (!duplicate || DupStrategy.OVERWRITE == strategy) { + if (!duplicate || overwrite) { resources.put(path, resource); s.put(path, resource); updateModified(resource.lastModified(), path); } - else if (duplicate && DupStrategy.APPEND == strategy) { - try (SequenceInputStream in = new SequenceInputStream(existing.openInputStream(), - resource.openInputStream());) { - - long lastModified = Math.max(existing.lastModified(), resource.lastModified()); - Resource r = new EmbeddedResource(ByteBuffer.wrap(in.readAllBytes()), - lastModified); - // addExtra(r, extra.get("extra")); - resources.put(path, r); - s.put(path, r); - updateModified(resource.lastModified(), path); - } catch (Exception e) { - throw Exceptions.duck(e); - } - } return duplicate; } + public Resource getResource(String path) { check(); path = ZipUtil.cleanPath(path); diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java new file mode 100644 index 00000000000..0830f226df6 --- /dev/null +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java @@ -0,0 +1,39 @@ +package aQute.bnd.osgi.metainf; + +import java.io.ByteArrayInputStream; +import java.io.SequenceInputStream; +import java.nio.ByteBuffer; +import java.util.Arrays; +import java.util.Collections; +import java.util.Optional; + +import aQute.bnd.exceptions.Exceptions; +import aQute.bnd.osgi.EmbeddedResource; +import aQute.bnd.osgi.Resource; +import aQute.bnd.service.merge.MergeFiles; + +public class MetaInfServiceMerger implements MergeFiles { + + @Override + public Optional merge(String path, Resource a, Resource b) { + + if (!path.startsWith("META-INF/services/")) { + return Optional.empty(); + } + + // do something with a and b + try (SequenceInputStream in = new SequenceInputStream(Collections.enumeration( + Arrays.asList(a.openInputStream(), new ByteArrayInputStream("\n".getBytes()), b.openInputStream())));) { + + long lastModified = Math.max(a.lastModified(), b.lastModified()); + Resource r = new EmbeddedResource(ByteBuffer.wrap(in.readAllBytes()), lastModified); + + return Optional.of(r); + } catch (Exception e) { + throw Exceptions.duck(e); + } + + + } + +} diff --git a/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java b/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java new file mode 100644 index 00000000000..897373a47cf --- /dev/null +++ b/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java @@ -0,0 +1,9 @@ +package aQute.bnd.service.merge; + +import java.util.Optional; + +import aQute.bnd.osgi.Resource; + +public interface MergeFiles { + Optional merge(String path, Resource a, Resource b); +} diff --git a/biz.aQute.bndlib/src/aQute/bnd/service/merge/package-info.java b/biz.aQute.bndlib/src/aQute/bnd/service/merge/package-info.java new file mode 100644 index 00000000000..b4399c1b879 --- /dev/null +++ b/biz.aQute.bndlib/src/aQute/bnd/service/merge/package-info.java @@ -0,0 +1,2 @@ +@org.osgi.annotation.versioning.Version("1.0.0") +package aQute.bnd.service.merge;