Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MNG-7758] Report dependency problems for all repository #1563

Merged
merged 2 commits into from
Jun 12, 2024
Merged

[MNG-7758] Report dependency problems for all repository #1563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8483603
[MNG-7758] Report dependency problems for all repository
slawekjaranowski Jun 5, 2024
ca4c13a
[MNG-7758] Report dependency problems for all repository - plugins
slawekjaranowski Jun 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions maven-core/src/main/java/org/apache/maven/plugin/PluginResolutionException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
*/
package org.apache.maven.plugin;

import java.util.List;
import java.util.stream.Collectors;

import org.apache.maven.model.Plugin;

/**
Expand All @@ -35,6 +38,18 @@ public PluginResolutionException(Plugin plugin, Throwable cause) {
this.plugin = plugin;
}

public PluginResolutionException(Plugin plugin, List<Exception> exceptions, Throwable cause) {
super(
"Plugin " + plugin.getId() + " or one of its dependencies could not be resolved:"
+ System.lineSeparator() + "\t"
+ exceptions.stream()
.map(Throwable::getMessage)
.collect(Collectors.joining(System.lineSeparator() + "\t"))
+ System.lineSeparator(),
cause);
this.plugin = plugin;
}

public Plugin getPlugin() {
return plugin;
}
Expand Down
12 changes: 8 additions & 4 deletions ...ore/src/main/java/org/apache/maven/plugin/internal/DefaultPluginDependenciesResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,15 +132,17 @@ public Artifact resolve(Plugin plugin, List<RemoteRepository> repositories, Repo
pluginArtifact = pluginArtifact.setProperties(props);
}
} catch (ArtifactDescriptorException e) {
throw new PluginResolutionException(plugin, e);
throw new PluginResolutionException(
plugin, e.getResult().getExceptions(), logger.isDebugEnabled() ? e : null);
}

try {
ArtifactRequest request = new ArtifactRequest(pluginArtifact, repositories, REPOSITORY_CONTEXT);
request.setTrace(trace);
pluginArtifact = repoSystem.resolveArtifact(session, request).getArtifact();
} catch (ArtifactResolutionException e) {
throw new PluginResolutionException(plugin, e);
throw new PluginResolutionException(
plugin, e.getResult().getExceptions(), logger.isDebugEnabled() ? e : null);
}

return pluginArtifact;
Expand Down Expand Up @@ -229,9 +231,11 @@ private DependencyResult resolveInternal(
depRequest.setRoot(node);
return repoSystem.resolveDependencies(session, depRequest);
} catch (DependencyCollectionException e) {
throw new PluginResolutionException(plugin, e);
throw new PluginResolutionException(
plugin, e.getResult().getExceptions(), logger.isDebugEnabled() ? e : null);
} catch (DependencyResolutionException e) {
throw new PluginResolutionException(plugin, e.getCause());
throw new PluginResolutionException(
plugin, e.getResult().getCollectExceptions(), logger.isDebugEnabled() ? e : null);
}
}
}
8 changes: 6 additions & 2 deletions maven-core/src/main/java/org/apache/maven/project/DefaultProjectDependenciesResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,9 @@ public DependencyResolutionResult resolve(DependencyResolutionRequest request)
result.setCollectionErrors(e.getResult().getExceptions());

throw new DependencyResolutionException(
result, "Could not resolve dependencies for project " + project.getId() + ": " + e.getMessage(), e);
result,
"Could not collect dependencies for project " + project.getId(),
logger.isDebugEnabled() ? e : null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why omitting the exception?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we have exception in stack all message from all exception will be added to output in normal run we will have some information duplicate now

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But isn't that a problem of the catcher, not the thrower?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we have more detailed info at debug level, especially in case of an error, it may make sense to indicate so in the non detailed error message ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

detailed contains more info which is not available in o.e.aether.resolution.ArtifactResolutionException
ArtifactResolutionException.getMessage returns error for last repository

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without it some of message will be duplicated

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without passing exception (current proposition) we will have:

[ERROR] Failed to execute goal on project test: Could not resolve dependencies for project org.apache.maven.its.mng3477:test:jar:1
[ERROR] dependency: org.apache.maven.its.mng3477:dep:jar:1.0 (compile)
[ERROR] 	Could not find artifact org.apache.maven.its.mng3477:dep:jar:1.0 in maven-core-it (http://localhost:64468/repo)
[ERROR] 	Could not find artifact org.apache.maven.its.mng3477:dep:jar:1.0 in central (http://localhost:64468/repo)
[ERROR] -> [Help 1]

with exception will be as:

[ERROR] Failed to execute goal on project test: Could not resolve dependencies for project org.apache.maven.its.mng3477:test:jar:1
[ERROR] dependency: org.apache.maven.its.mng3477:dep:jar:1.0 (compile)
[ERROR] 	Could not find artifact org.apache.maven.its.mng3477:dep:jar:1.0 in central (http://localhost:64398/repo)
[ERROR] 	Could not find artifact org.apache.maven.its.mng3477:dep:jar:1.0 in maven-core-it (http://localhost:64398/repo): The following artifacts could not be resolved: org.apache.maven.its.mng3477:dep:jar:1.0 (absent): Could not find artifact org.apache.maven.its.mng3477:dep:jar:1.0 in central (http://localhost:64398/repo)
[ERROR] -> [Help 1]

so it is a reason

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope that simply message will be more readable for user, if user need more details for debug or investigation can execute in verbose/debug mode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERY UGLY. I guess this needs a long term solution.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can think about it ... how to reporting a problems to user but it is another issue for me, now I did as simple as possible
we also have many other UGLY code ... 😸

}

depRequest.setRoot(node);
Expand Down Expand Up @@ -190,7 +192,9 @@ public DependencyResolutionResult resolve(DependencyResolutionRequest request)
process(result, e.getResult().getArtifactResults());

throw new DependencyResolutionException(
result, "Could not resolve dependencies for project " + project.getId() + ": " + e.getMessage(), e);
result,
"Could not resolve dependencies for project " + project.getId(),
logger.isDebugEnabled() ? e : null);
}

return result;
Expand Down
34 changes: 34 additions & 0 deletions maven-core/src/main/java/org/apache/maven/project/DependencyResolutionException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,52 @@
*/
package org.apache.maven.project;

import java.util.List;

import org.eclipse.aether.graph.Dependency;

/**
*/
public class DependencyResolutionException extends Exception {

private final transient DependencyResolutionResult result;
private final transient String detailMessage;

public DependencyResolutionException(DependencyResolutionResult result, String message, Throwable cause) {
super(message, cause);
this.result = result;
this.detailMessage = prepareDetailMessage(message, result);
}

private static String prepareDetailMessage(String message, DependencyResolutionResult result) {
StringBuilder msg = new StringBuilder(message);
msg.append(System.lineSeparator());
for (Dependency dependency : result.getUnresolvedDependencies()) {
msg.append("dependency: ").append(dependency).append(System.lineSeparator());
List<Exception> exceptions = result.getResolutionErrors(dependency);
for (Exception e : exceptions) {
msg.append("\t").append(e.getMessage()).append(System.lineSeparator());
}
}

for (Exception exception : result.getCollectionErrors()) {
msg.append(exception.getMessage()).append(System.lineSeparator());
if (exception.getCause() != null) {
msg.append("\tCaused by: ")
.append(exception.getCause().getMessage())
slawekjaranowski marked this conversation as resolved.
Show resolved Hide resolved
.append(System.lineSeparator());
}
}

return msg.toString();
}

public DependencyResolutionResult getResult() {
return result;
}

@Override
public String getMessage() {
return detailMessage;
}
}
4 changes: 1 addition & 3 deletions ...n-embedder/src/main/java/org/apache/maven/cli/internal/BootstrapCoreExtensionManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,9 +226,7 @@ private List<Artifact> resolveExtension(
.filter(ArtifactResult::isResolved)
.map(ArtifactResult::getArtifact)
.collect(Collectors.toList());
} catch (PluginResolutionException e) {
throw new ExtensionResolutionException(extension, e.getCause());
} catch (InterpolationException e) {
} catch (PluginResolutionException | InterpolationException e) {
throw new ExtensionResolutionException(extension, e);
}
}
Expand Down