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.

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

[MENFORCER-332] Dependency graph performance fix #53

Closed

Conversation

rjatkins
Copy link

Fixes main performance issue in bannedDependencies, and other rules that use DependencyGraphBuilder or the legacy DependencyTreeBuilder.

 * Also upgrade to maven-dependency-tree 3.0.1, maven-plugin-testing-harness 3.3.0, commons-lang3 3.8.1, junit 4.12
 * Upgraded maven-dependency-tree is a breaking change for uses of AbstractBanDependencies.getDependenciesToCheck, requiring a signature change from Project to ProjectBuildingRequest
 * Also removed all the default scoped fields from EnforcerRuleUtils that are not in use, and cleaned up the corresponding constructor
 * Also removed maven-compat, and updated affected code to use RepositorySystem and RepositoryRequest instead of ArtifactResolver, ArtifactFactory and manually tracking the local and remote ArtifactRepository entries.
     * Make all rules that use DependencyGraphBuilder use a common component
     * Use sisu-maven-plugin to allow the new common component to use annotations. Keeping the rules using explicit component lookups for now
     * Make the common DependencyGraphLookup component cache lookups for a dependency tree per MavenProject, so that even highly parallel builds can also avoid retrieving the dependency tree more than once per project
@rjatkins
Copy link
Author

rjatkins commented Apr 3, 2019

The upgrade of maven-dependency-tree has changed the semantics of what it means to build a dependency graph, since it removed the DependencyTreeBuilder and replaced it with DependencyGraphBuilder, which has broken the integration tests for DependencyConvergence and RequireUpperBoundDeps. I'm digging into what needs to be done to make the graph compatible with both of these, which requires debugging through mrm-maven-plugin to find out why it's not serving all the required mock poms and jars. I hope to have a fix for this tomorrow.

 * Add support for building dependency graphs with and without the default ConflictResolver transforming the graph, since some rules require the full conflicting tree to be able to see artifacts with conflicting versions
 * Upgrade to latest maven-invoker-plugin and mrm-maven-plugin
 * Add override for MAVEN_DEBUG_OPTS with maven-invoker-plugin, so mvnDebug can work with the outer build, without also triggering on the invoker build too
 * Also update DependencyGraphLookup to use @Inject for its dependencies, so it no longer needs a separate init phase
@rjatkins
Copy link
Author

The remaining test failures in RequireUpperBoundDeps needs a fix in mrm-maven-plugin to add support for mocking timestamped snapshots (fixed in mojohaus/mrm#22 ), and another fix in maven-dependency-tree to restore support for annotating DependencyNodes with managed versions and scopes (tracked in https://issues.apache.org/jira/browse/MSHARED-816 with a fix in apache/maven-dependency-tree#2 ).

@elharo
Copy link
Contributor

elharo commented May 25, 2020

Since this is over a year old, I'm going to close it. Feel free to open a new PR that resolved the merge conflicts.

@elharo elharo closed this May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants