mojohaus / extra-enforcer-rules

Extra Enforcer Rules
https://www.mojohaus.org/extra-enforcer-rules/
Apache License 2.0
73 stars 47 forks source link

Rewrite dependencies resolution in AbstractResolveDependencies. #303

Open wilx opened 2 months ago

wilx commented 2 months ago

Use repositorySystem.resolveDependencies() to fold collection and resolution steps into one.

Use filters for resolveDependencies() call. Do not pre-filter dependencies.

Fixes #302.

slawekjaranowski commented 2 months ago

Thanks for PR. I will check it, but afraid that we should filter artifacts by scope before resolving/collecting it.

dependency:tree has a bug for such cases https://issues.apache.org/jira/browse/MDEP-909

@cstamas please also look

wilx commented 2 months ago

I will check it, but afraid that we should filter artifacts by scope before resolving/collecting it.

Why? The filter does it for us during the collection. For my test case, the provided dependency gets filtered out correctly.

wilx commented 2 months ago

When I was looking into this, I looked around the other sources. I saw org.apache.maven.project.DefaultProjectDependenciesResolver#resolve. It looks like the original code was trying to do what the DefaultProjectDependenciesResolver#resolve does, but the original code lacks few of the hacks that the DefaultProjectDependenciesResolver#resolve has. I was wondering, if that is why it was returning different results.

cstamas commented 2 months ago

This looks correct in Maven3 universe. Due being in "maven3 universe" it suffers from MNG-8041, but is acceptable.

slawekjaranowski commented 2 months ago

@cstamas we should be consequently - we did filtering before resolving in m-assembly-p https://github.com/apache/maven-assembly-plugin/pull/166

So how plugins should resolve dependencies tree with given scope ...?

another issue with the similar problem https://issues.apache.org/jira/browse/MINVOKER-368

cstamas commented 2 months ago

Right. This all evolves around MNG-8041 where maven3 provides "wrong runtime" classpath/tree. Reason is that at the "start" in includes unwanted scopes like test scope, that "eclipses" possible runtime dependencies lower in tree, and finally dep filter removes "test" scope as well (and the losers are removed by conflict resolution before that) and classpath ends up being incomplete, as explained here: https://maven.apache.org/resolver-archives/resolver-2.0.0-alpha-11/common-misconceptions.html

So, I do agree with @slawekjaranowski we should align these plugin scattered "fixes", until Maven4 API generally provides fixed and one-shop solution for all.