diff --git a/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java b/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java index 3180ccf5a6..e00b3f165f 100644 --- a/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java +++ b/biz.aQute.bndlib.tests/test/test/IncludeResourceTest.java @@ -195,7 +195,7 @@ public void testIncludeResourceDuplicatesDefaultOverwrite() throws Exception { Jar jar = a.build(); assertFalse(a.check()); assertEquals( - "Duplicate file overwritten: META-INF/services/foo (Consider using the onduplicate: directive to handle duplicates.)", + "includeresource.duplicates: Duplicate overwritten: META-INF/services/foo (Consider using the onduplicate: directive to handle duplicates.)", a.getWarnings() .get(0)); @@ -266,7 +266,7 @@ public void testIncludeResourceMixedMetaInfDuplicatesMerge() throws Exception { assertEquals("a\nb", IO.collect(resourceFoo.openInputStream())); Resource resourceManifest = jar.getResource("META-INF/bar.txt"); - assertEquals("b", IO.collect(resourceManifest.openInputStream())); + assertEquals("a", IO.collect(resourceManifest.openInputStream())); } } @@ -300,7 +300,7 @@ public void testIncludeResourceDuplicatesError() throws Exception { "@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;onduplicate:=ERROR"); Jar jar = a.build(); assertFalse(a.check()); - assertEquals("Duplicate file: META-INF/services/foo", a.getErrors() + assertEquals("includeresource.duplicates: duplicate found for path META-INF/services/foo", a.getErrors() .get(0)); assertTrue(jar.getDirectories() @@ -322,7 +322,7 @@ public void testIncludeResourceDuplicatesWarning() throws Exception { "@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;onduplicate:=WARN"); Jar jar = a.build(); assertFalse(a.check()); - assertEquals("Duplicate file: META-INF/services/foo", a.getWarnings() + assertEquals("includeresource.duplicates: duplicate found for path META-INF/services/foo", a.getWarnings() .get(0)); assertTrue(jar.getDirectories() @@ -387,7 +387,7 @@ public void testIncludeResourceLiteralDuplicatesMerge(@InjectTemporaryDirectory b.getJar() .writeFolder(tmp); - assertEquals("b", IO.collect(IO.getFile(tmp, "a/a.txt"))); + assertEquals("a", IO.collect(IO.getFile(tmp, "a/a.txt"))); } } @@ -416,7 +416,7 @@ public void testIncludeResourceLiteralDuplicatesError(@InjectTemporaryDirectory b.setIncludeResource("/a/a.txt;literal='a', /a/a.txt;literal='b';onduplicate:=ERROR"); b.build(); assertFalse(b.check()); - assertEquals("Duplicate file: /a/a.txt", b.getErrors() + assertEquals("includeresource.duplicates: duplicate found for path /a/a.txt", b.getErrors() .get(0)); b.getJar() @@ -461,12 +461,12 @@ public void testIncludeResourceDuplicatesMergeWithoutPlugin() throws Exception { .containsKey("META-INF/services")); Resource resource = jar.getResource("META-INF/services/foo"); - // we expect OVERWRITE because there is no plugin with the tag + // we expect nothing because there is no plugin with the tag // 'nonexistingtag' // which means we have nothing which can merge META-INF/services // files. - // so the default OVERWRITE behavior applies - assertEquals("b", IO.collect(resource.openInputStream())); + // so we keep the existing file + assertEquals("a", IO.collect(resource.openInputStream())); } @@ -507,12 +507,12 @@ public void testIncludeResourceDuplicatesTagWithoutStrategyEnumButNonExistingTag .containsKey("META-INF/services")); Resource resource = jar.getResource("META-INF/services/foo"); - // we expect OVERWRITE because there is no plugin with the tag + // we expect nothing because there is no plugin with the tag // 'nonexistingtag' // which means we have nothing which can merge META-INF/services // files. - // so the default OVERWRITE behavior applies - assertEquals("b", IO.collect(resource.openInputStream())); + // so we keep the existing file + assertEquals("a", IO.collect(resource.openInputStream())); } } @@ -527,7 +527,7 @@ public void testIncludeResourceDuplicatesMergeWithTagAndWarn() throws Exception "@jar/jarA.jar!/META-INF/services/*, @jar/jarB.jar!/META-INF/services/*;onduplicate:='MERGE,metainfservices,WARN'"); Jar jar = a.build(); assertFalse(a.check()); - assertEquals("Duplicate file: META-INF/services/foo", a.getWarnings() + assertEquals("includeresource.duplicates: duplicate found for path META-INF/services/foo", a.getWarnings() .get(0)); assertTrue(jar.getDirectories() diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java index 87bf510bd6..ecff4d28c2 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/Builder.java @@ -31,6 +31,7 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Consumer; import java.util.function.Function; import java.util.jar.JarFile; import java.util.jar.Manifest; @@ -69,7 +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.merge.MergeResources; import aQute.bnd.service.specifications.BuilderSpecification; import aQute.bnd.stream.MapStream; import aQute.bnd.unmodifiable.Maps; @@ -1381,7 +1382,7 @@ 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 = parseDupStrategy(extra); + Function> dupStrategy = parseDupStrategy(extra); boolean dupl = false; for (String name : sub.getResources() @@ -1402,9 +1403,11 @@ private boolean addAll(Jar to, Jar sub, Instruction filter, String destination, if (!duplicate) { dupl |= to.putResource(path, resource, true); } else { - Resource maybeMerged = dupStrategy.onDuplicate(path, existing, resource, this); - if (maybeMerged != null) { - dupl |= to.putResource(path, maybeMerged); + Optional maybeMerged = dupStrategy.apply(new Duplication(path, existing, resource)); + // Resource maybeMerged = dupStrategy.onDuplicate(path, + // existing, resource, this); + if (maybeMerged.isPresent()) { + dupl |= to.putResource(path, maybeMerged.get()); } } @@ -1454,10 +1457,10 @@ private void copy(Jar jar, String path, Resource resource, Map e if (!duplicate) { jar.putResource(path, resource, true); } else { - DupStrategy dupStrategy = parseDupStrategy(extra); - Resource maybeMerged = dupStrategy.onDuplicate(path, existing, resource, this); - if (maybeMerged != null) { - jar.putResource(path, maybeMerged, true); + Function> dupStrategy = parseDupStrategy(extra); + Optional maybeMerged = dupStrategy.apply(new Duplication(path, existing, resource)); + if (maybeMerged.isPresent()) { + jar.putResource(path, maybeMerged.get(), true); } } @@ -2122,147 +2125,121 @@ public String system(boolean allowFail, String command, String input) throws IOE return cachedSystemCalls.computeIfAbsent(key, asFunction(k -> super.system(allowFail, command, input))); } - private DupStrategy parseDupStrategy(Map extra) { + private Function> parseDupStrategy(Map extra) { String onduplicate = extra.get(DUP_STRATEGY); - return DupStrategy.parse(onduplicate); - } - - private enum DupStrategyEnum { - OVERWRITE, - MERGE, - WARN, - ERROR, - SKIP, - /** - * same as OVERWRITE, for backwards compatibility, - * and to distinguish, because here we warn too. - */ - DEFAULT_OVERWRITE; - - /** - * like valueOf() returns null instead of exception. - */ - public static DupStrategyEnum fromString(String s) { - try { - return valueOf(s); - } catch (Exception e) { - return null; - } - } - - + return Duplication.doDuplicate(onduplicate, this); } + /** - * Helper for handling inluderesource duplicates. + * Handles how duplicate resources are handled in -includeresource + * instruction. */ - private final static class DupStrategy { - - private static final DupStrategy DEFAULT = new DupStrategy( - Set.of(DupStrategyEnum.DEFAULT_OVERWRITE), Set.of()); + public record Duplication(String path, Resource existing, Resource candidate) { - private static final String DUP_MSG_DEFAULT_OVERWRITE = "Duplicate file overwritten: %s (Consider using the %s directive to handle duplicates.)"; - private static final String DUP_MSG = "Duplicate file: %s"; - private Set strategies; - private Set tags; + private static final String DUP_MSG_DEFAULT_OVERWRITE = "includeresource.duplicates: Duplicate overwritten: %s (Consider using the %s directive to handle duplicates.)"; - private DupStrategy(Set strategies, Set tags) { - Objects.requireNonNull(strategies); - Objects.requireNonNull(tags); - this.strategies = strategies; - this.tags = tags; + enum OnDuplicateCommand { + OVERWRITE, + SKIP, + MERGE, + WARN, + ERROR; } - public static DupStrategy parse(String directive) { + public static Function> doDuplicate(String onduplicate, Processor processor) { - // enum ::= WARN, ERROR, MERGE, SKIP, OVERWRITE - // onduplicate: ::= (tagGlob | enum)* + if (onduplicate == null || onduplicate.isBlank()) { + Consumer warn = dupl -> { + // but only warn if resources are not identical + if (!isIdentical(dupl.existing, dupl.candidate)) { + processor.warning(DUP_MSG_DEFAULT_OVERWRITE, dupl.path, Constants.DUP_STRATEGY); + } + }; + return dupl -> { - if (directive == null || directive.isBlank()) { - return DEFAULT; + warn.accept(dupl); + return Optional.of(dupl.candidate); + }; } - String[] enumOrTags = directive.split(","); - Set dupEnums = new LinkedHashSet<>(); - Set tags = new LinkedHashSet<>(); - - for (String enumOrTag : enumOrTags) { + Set commands = new LinkedHashSet<>(); + List tags = new ArrayList(); - DupStrategyEnum e = DupStrategyEnum.fromString(enumOrTag.trim()); - if (e == null) { - // not an enum it must be a tag - tags.add(enumOrTag); - } else { - dupEnums.add(e); + Strings.split(onduplicate) + .forEach(string -> { + try { + commands.add(OnDuplicateCommand.valueOf(string)); + } catch (Exception e) { + tags.add(string); + } + }); + + Consumer error = commands.remove(OnDuplicateCommand.ERROR) + ? dupl -> processor.error("includeresource.duplicates: duplicate found for path %s", dupl.path) + : d -> {}; + Consumer warn = commands.remove(OnDuplicateCommand.WARN) + ? dupl -> processor.warning("includeresource.duplicates: duplicate found for path %s", dupl.path) + : d -> {}; + + String[] tags2 = tags.toArray(new String[0]); + Function> result; + + int what = tags.isEmpty() ? 0 : 1; + if (!commands.isEmpty()) + what += 2; + + result = switch (what) { + case 0 -> dupl -> Optional.of(dupl.candidate); + case 1 -> { + List mergers = processor.getPlugins(MergeResources.class, tags2); + yield dupl -> merge(dupl, mergers); } + case 2, 3 -> getCommand(commands, tags2, processor); + default -> throw new UnsupportedOperationException(); + }; - } - - if (dupEnums.isEmpty() && tags.isEmpty()) { - return DEFAULT; - } - - return new DupStrategy(dupEnums, tags); - + return dupl -> { + error.accept(dupl); + warn.accept(dupl); + return result.apply(dupl); + }; } - public Resource onDuplicate(String path, Resource existing, Resource resource, - Processor proc) { + private static Function> getCommand(Set commands, + String[] tags, Processor processor) { - if (strategies.contains(DupStrategyEnum.DEFAULT_OVERWRITE)) { - - // default if not specified is 'overwrite' as it always was - // and show a warning which mentions the DUP_STRATEGY - // directive - - // but only warn if resources are not identical - if (!isIdentical(existing, resource)) { - proc.warning(DUP_MSG_DEFAULT_OVERWRITE, path, DUP_STRATEGY); - } - return resource; - } - - /** - * MERGE, SKIP, OVERWRITE -> exclusive // WARN, ERROR -> can be - * combined - */ - - if (strategies.isEmpty() && !tags.isEmpty()) { - // special case: no strategy but tags - // is considered same as MERGE - - // apply the first plugin that can handle the path - return applyMergePlugins(path, existing, resource, proc); - } - - if (strategies.contains(DupStrategyEnum.WARN)) { - proc.warning(DUP_MSG, path); + if (commands.size() != 1) { + processor.error("includeresource.duplicates: specifies multiple strategies to handle duplicates: %s", + commands); + return dupl -> Optional.empty(); } - if (strategies.contains(DupStrategyEnum.ERROR)) { - proc.error(DUP_MSG, path); + if (commands.contains(OnDuplicateCommand.OVERWRITE)) { + return dupl -> Optional.of(dupl.candidate); + } else if (commands.contains(OnDuplicateCommand.SKIP)) { + return dupl -> Optional.empty(); + } else if (commands.contains(OnDuplicateCommand.MERGE)) { + return dupl -> merge(dupl, processor.getPlugins(MergeResources.class, tags)); + } else { + throw new UnsupportedOperationException("missed an enum value? " + commands); } + } - if (strategies.contains(DupStrategyEnum.SKIP)) { - return null; - } else if (strategies.contains(DupStrategyEnum.MERGE)) { - - // apply the first plugin that can handle the path - return applyMergePlugins(path, existing, resource, proc); + private static Optional merge(Duplication dupl, List list) { + for (MergeResources mr : list) { + Optional merged = mr.tryMerge(dupl.path, dupl.existing, dupl.candidate); + if (merged.isPresent()) + return merged; - } else if (strategies.contains(DupStrategyEnum.OVERWRITE)) { - return resource; - } else { - // OVERWRITE - return resource; } - + return Optional.empty(); } - private boolean isIdentical(Resource existing, Resource resource) { + private static boolean isIdentical(Resource a, Resource b) { try { - ByteBuffer buffer1 = existing.buffer(); - ByteBuffer buffer2 = resource.buffer(); + ByteBuffer buffer1 = a.buffer(); + ByteBuffer buffer2 = b.buffer(); if (buffer1.remaining() != buffer2.remaining()) { return false; @@ -2274,18 +2251,8 @@ private boolean isIdentical(Resource existing, Resource resource) { return false; } } - - private Resource applyMergePlugins(String path, Resource existing, Resource resource, Processor proc) { - return proc.getPlugins(MergeFiles.class, tags.toArray(new String[0])) - .stream() - .map(mf -> mf.merge(path, existing, resource)) - .filter(Optional::isPresent) - .map(Optional::get) - .findFirst() - .orElse(resource); - } - } + } diff --git a/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java b/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java index 14d1a1d9ad..2383bcddbb 100644 --- a/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java +++ b/biz.aQute.bndlib/src/aQute/bnd/osgi/metainf/MetaInfServiceMerger.java @@ -10,7 +10,7 @@ import aQute.bnd.exceptions.Exceptions; import aQute.bnd.osgi.EmbeddedResource; import aQute.bnd.osgi.Resource; -import aQute.bnd.service.merge.MergeFiles; +import aQute.bnd.service.merge.MergeResources; import aQute.bnd.service.tags.Tagged; import aQute.bnd.service.tags.Tags; @@ -18,13 +18,13 @@ * Knows how to "merge" duplicate files in META-INF/services, by concatenating * them with a linebreak in between. */ -public class MetaInfServiceMerger implements MergeFiles, Tagged { +public class MetaInfServiceMerger implements MergeResources, Tagged { private final static Tags META_INF_SERVICES = Tags.of("metainfservices"); @Override - public Optional merge(String path, Resource a, Resource b) { + public Optional tryMerge(String path, Resource a, Resource b) { if (!path.startsWith("META-INF/services/")) { return Optional.empty(); diff --git a/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java b/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java deleted file mode 100644 index 897373a47c..0000000000 --- a/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeFiles.java +++ /dev/null @@ -1,9 +0,0 @@ -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/MergeResources.java b/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeResources.java new file mode 100644 index 0000000000..1dad3995cf --- /dev/null +++ b/biz.aQute.bndlib/src/aQute/bnd/service/merge/MergeResources.java @@ -0,0 +1,20 @@ +package aQute.bnd.service.merge; + +import java.util.Optional; + +import aQute.bnd.osgi.Resource; + +/** + * For plugins knowing how to merge two resources. + */ +public interface MergeResources { + /** + * @param path a path (used for validation if the path segment is supported + * for merge) + * @param a first resource + * @param b second resource to be merged with a + * @return the merged resource if possible or an empty optional if merging + * was not possible. + */ + Optional tryMerge(String path, Resource a, Resource b); +}