Skip to content

Commit

Permalink
Do not fail if wrapping dependencies fails
Browse files Browse the repository at this point in the history
Fix #1402
  • Loading branch information
laeubi committed Jul 4, 2023
1 parent 3ed03c8 commit c4163ad
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,58 @@
public class OSGiMetadataGenerationTest extends AbstractMavenTargetTest {

@Test
// @Ignore("currently fails on CI")
public void testBadDependencyInChain() throws Exception {
ITargetLocation target = resolveMavenTarget("""
<location includeDependencyScope="compile" missingManifest="generate" type="Maven">
<dependencies>
<dependency>
<groupId>edu.ucar</groupId>
<artifactId>cdm</artifactId>
<version>4.5.5</version>
<type>jar</type>
</dependency>
</dependencies>
</location>
""");
assertStatusOk(getTargetStatus(target));
}

@Test
public void testBadDependencyDirect() throws Exception {
ITargetLocation target = resolveMavenTarget("""
<location missingManifest="generate" type="Maven">
<dependencies>
<dependency>
<groupId>com.ibm.icu</groupId>
<artifactId>icu4j</artifactId>
<version>2.6.1</version>
<type>jar</type>
</dependency>
</dependencies>
</location>
""");
IStatus targetStatus = getTargetStatus(target);
assertEquals(String.valueOf(targetStatus), IStatus.ERROR, targetStatus.getSeverity());
}

@Test
public void testMissingOptionalDependency() throws Exception {
ITargetLocation target = resolveMavenTarget("""
<location includeDependencyDepth="none" includeDependencyScopes="compile" missingManifest="generate" type="Maven">
<dependencies>
<dependency>
<groupId>net.sf.saxon</groupId>
<artifactId>Saxon-HE</artifactId>
<version>10.9</version>
<type>jar</type>
</dependency>
</dependencies>
</location>
""");
assertStatusOk(getTargetStatus(target));
}

@Test
public void testNonOSGiArtifact_missingArtifactError() throws Exception {
ITargetLocation target = resolveMavenTarget("""
<location includeDependencyDepth="none" includeSource="true" missingManifest="error" type="Maven">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProcessingMessage> errors = wrap.messages()
List<ProcessingMessage> 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);
Expand All @@ -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;

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<DependencyNode> 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)) {
Expand All @@ -117,7 +137,10 @@ public boolean visitEnter(DependencyNode n) {
Map<DependencyNode, WrappedBundle> 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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ProcessingMessage> messages() {
return Stream.concat(messages.stream(), depends.stream().flatMap(dep -> dep.messages()));
/**
* @param includeDependent if <code>true</code> includes messages from dependent
* items.
* @return the messages that where produced
*/
public Stream<ProcessingMessage> messages(boolean includeDependent) {
if (includeDependent) {
return Stream.concat(messages.stream(), depends.stream().flatMap(dep -> dep.messages(true)));
}
return messages.stream();
}

@Override
Expand Down

0 comments on commit c4163ad

Please sign in to comment.