From c4163ad9c9df275dba79f86b49d30f71061cba12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christoph=20L=C3=A4ubrich?= Date: Tue, 4 Jul 2023 17:43:19 +0200 Subject: [PATCH] Do not fail if wrapping dependencies fails Fix https://github.com/eclipse-m2e/m2e-core/issues/1402 --- .../tests/OSGiMetadataGenerationTest.java | 53 ++++++++++++++++++- .../m2e/pde/target/MavenTargetBundle.java | 29 +++++++--- .../pde/target/shared/MavenBundleWrapper.java | 47 ++++++++++++++-- .../m2e/pde/target/shared/WrappedBundle.java | 17 ++++-- 4 files changed, 132 insertions(+), 14 deletions(-) diff --git a/org.eclipse.m2e.pde.target.tests/src/org/eclipse/m2e/pde/target/tests/OSGiMetadataGenerationTest.java b/org.eclipse.m2e.pde.target.tests/src/org/eclipse/m2e/pde/target/tests/OSGiMetadataGenerationTest.java index fadba4f496..f40884d1a7 100644 --- a/org.eclipse.m2e.pde.target.tests/src/org/eclipse/m2e/pde/target/tests/OSGiMetadataGenerationTest.java +++ b/org.eclipse.m2e.pde.target.tests/src/org/eclipse/m2e/pde/target/tests/OSGiMetadataGenerationTest.java @@ -39,7 +39,58 @@ public class OSGiMetadataGenerationTest extends AbstractMavenTargetTest { @Test -// @Ignore("currently fails on CI") + public void testBadDependencyInChain() throws Exception { + ITargetLocation target = resolveMavenTarget(""" + + + + edu.ucar + cdm + 4.5.5 + jar + + + + """); + assertStatusOk(getTargetStatus(target)); + } + + @Test + public void testBadDependencyDirect() throws Exception { + ITargetLocation target = resolveMavenTarget(""" + + + + com.ibm.icu + icu4j + 2.6.1 + jar + + + + """); + IStatus targetStatus = getTargetStatus(target); + assertEquals(String.valueOf(targetStatus), IStatus.ERROR, targetStatus.getSeverity()); + } + + @Test + public void testMissingOptionalDependency() throws Exception { + ITargetLocation target = resolveMavenTarget(""" + + + + net.sf.saxon + Saxon-HE + 10.9 + jar + + + + """); + assertStatusOk(getTargetStatus(target)); + } + + @Test public void testNonOSGiArtifact_missingArtifactError() throws Exception { ITargetLocation target = resolveMavenTarget(""" diff --git a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/MavenTargetBundle.java b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/MavenTargetBundle.java index f8af883443..dbf36b97cf 100644 --- a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/MavenTargetBundle.java +++ b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/MavenTargetBundle.java @@ -132,24 +132,29 @@ private static TargetBundle getWrappedArtifact(Artifact artifact, MavenTargetLoc }; IMavenExecutionContext exeContext = IMavenExecutionContext.getThreadContext() .orElseGet(maven::createExecutionContext); - + MultiStatus bundleStatus = new MultiStatus(MavenTargetBundle.class, 0, + "Some problems where detected while wrapping " + artifact); Path wrappedBundle = exeContext.execute((context, monitor1) -> { RepositorySystem repoSystem = MavenPluginActivator.getDefault().getRepositorySystem(); RepositorySystemSession repositorySession = context.getRepositorySession(); try { WrappedBundle wrap = MavenBundleWrapper.getWrappedArtifact(artifact, instructionsLookup, repositories, repoSystem, repositorySession, context.getComponentLookup().lookup(SyncContextFactory.class)); - List errors = wrap.messages() + List directErrors = wrap.messages(false) .filter(msg -> msg.type() == ProcessingMessage.Type.ERROR).toList(); - if (errors.isEmpty()) { + if (directErrors.isEmpty()) { + // treat all items as warnings.... + wrap.messages(true).map(ProcessingMessage::message).distinct().forEach(msg -> { + bundleStatus.add(Status.warning(msg)); + }); return wrap.getFile(); } - if (errors.size() == 1) { - throw new CoreException(Status.error(errors.get(0).message())); + if (directErrors.size() == 1) { + throw new CoreException(Status.error(directErrors.get(0).message())); } MultiStatus multiStatus = new MultiStatus(MavenTargetBundle.class, IStatus.ERROR, "wrapping artifact " + artifact.getArtifactId() + " failed!"); - for (ProcessingMessage message : errors) { + for (ProcessingMessage message : directErrors) { multiStatus.add(Status.error(message.message())); } throw new CoreException(multiStatus); @@ -159,7 +164,17 @@ private static TargetBundle getWrappedArtifact(Artifact artifact, MavenTargetLoc throw new CoreException(Status.error("Can't collect dependencies!", e)); } }, monitor); - return new TargetBundle(wrappedBundle.toFile()); + TargetBundle bundle = new TargetBundle(wrappedBundle.toFile()) { + @Override + public IStatus getStatus() { + if (!bundleStatus.isOK()) { + // TODO see https://github.com/eclipse-pde/eclipse.pde/issues/656 + // return bundleStatus; + } + return super.getStatus(); + } + }; + return bundle; } diff --git a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/MavenBundleWrapper.java b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/MavenBundleWrapper.java index 88ebdcdfa7..1b872c4516 100644 --- a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/MavenBundleWrapper.java +++ b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/MavenBundleWrapper.java @@ -39,11 +39,15 @@ import org.eclipse.aether.artifact.Artifact; import org.eclipse.aether.collection.CollectRequest; import org.eclipse.aether.graph.Dependency; +import org.eclipse.aether.graph.DependencyFilter; import org.eclipse.aether.graph.DependencyNode; import org.eclipse.aether.graph.DependencyVisitor; import org.eclipse.aether.impl.SyncContextFactory; import org.eclipse.aether.repository.RemoteRepository; +import org.eclipse.aether.resolution.ArtifactRequest; +import org.eclipse.aether.resolution.ArtifactResolutionException; import org.eclipse.aether.resolution.DependencyRequest; +import org.eclipse.aether.resolution.DependencyResolutionException; import org.eclipse.core.runtime.Platform; import org.eclipse.m2e.pde.target.shared.ProcessingMessage.Type; import org.osgi.framework.Constants; @@ -96,6 +100,22 @@ public static WrappedBundle getWrappedArtifact(Artifact artifact, DependencyRequest dependencyRequest = new DependencyRequest(); dependencyRequest.setRoot(node); + dependencyRequest.setFilter(new DependencyFilter() { + + @Override + public boolean accept(DependencyNode node, List parents) { + ArtifactRequest request = new ArtifactRequest(); + request.setRepositories(repositories); + Artifact nodeArtifact = node.getArtifact(); + request.setArtifact(nodeArtifact); + try { + repoSystem.resolveArtifact(repositorySession, request); + } catch (ArtifactResolutionException e) { + return false; + } + return true; + } + }); repoSystem.resolveDependencies(repositorySession, dependencyRequest); try (SyncContext syncContext = syncContextFactory.newInstance(repositorySession, false)) { @@ -117,7 +137,10 @@ public boolean visitEnter(DependencyNode n) { Map visited = new HashMap<>(); WrappedBundle wrappedNode = getWrappedNode(node, instructionsLookup, visited); for (WrappedBundle wrap : visited.values()) { - wrap.getJar().close(); + Jar jar = wrap.getJar(); + if (jar != null) { + jar.close(); + } } return wrappedNode; } @@ -132,6 +155,18 @@ private static WrappedBundle getWrappedNode(DependencyNode node, } Artifact artifact = node.getArtifact(); File originalFile = artifact.getFile(); + if (originalFile == null) { + if (node.getDependency().isOptional()) { + visited.put(node, + wrappedNode = new WrappedBundle(node, List.of(), null, null, null, + List.of(new ProcessingMessage(artifact, Type.WARN, + "Optional artifact " + node.getArtifact() + " was not found")))); + } else { + visited.put(node, wrappedNode = new WrappedBundle(node, List.of(), null, null, null, List.of( + new ProcessingMessage(artifact, Type.ERROR, "Artifact " + node.getArtifact() + " not found")))); + } + return wrappedNode; + } Jar jar = new Jar(originalFile); Manifest originalManifest = jar.getManifest(); if (originalManifest != null @@ -174,8 +209,14 @@ private static WrappedBundle getWrappedNode(DependencyNode node, analyzer.setProperty(property, trimValue); } for (WrappedBundle dep : depends) { - analyzer.addClasspath(dep.getJar()); - analyzer.removeClose(dep.getJar()); + Jar depJar = dep.getJar(); + if (depJar == null) { + messages.add(new ProcessingMessage(artifact, Type.WARN, + "Dependency " + dep.getNode().getDependency() + " was ignored!")); + continue; + } + analyzer.addClasspath(depJar); + analyzer.removeClose(depJar); } analyzerJar.setManifest(analyzer.calcManifest()); analyzerJar.write(wrapArtifactFile); diff --git a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/WrappedBundle.java b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/WrappedBundle.java index 05137989c0..9f8824a968 100644 --- a/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/WrappedBundle.java +++ b/org.eclipse.m2e.pde.target/src/org/eclipse/m2e/pde/target/shared/WrappedBundle.java @@ -48,14 +48,25 @@ Jar getJar() { return jar; } + DependencyNode getNode() { + return node; + } + /** @return the location of the wrapped bundle's files */ public Path getFile() { return file; } - /** @return the messages that where produced */ - public Stream messages() { - return Stream.concat(messages.stream(), depends.stream().flatMap(dep -> dep.messages())); + /** + * @param includeDependent if true includes messages from dependent + * items. + * @return the messages that where produced + */ + public Stream messages(boolean includeDependent) { + if (includeDependent) { + return Stream.concat(messages.stream(), depends.stream().flatMap(dep -> dep.messages(true))); + } + return messages.stream(); } @Override