-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Maven31DependencyResolverException for aggregate dependency check #3730
Comments
@viktor-thell-seal Thanks for the PoC project. Will help in easier reproduction of the issue |
Slowness I would expect to more likely be linked to other changes in 6.4.0/1: #3690 / #3722 / #3725 - the throttled retries for NIST NVD dataset downloads. In your gist (https://gist.github.com/viktor-thell-seal/c5c91f6b0ab12fb97f074ec442861ed6#file-maven-log-L744-L857) update/recreate of your CVE database took 5m41 and was even incomplete, as the 2014 and 2017 failed to download. The exception is related to the workaround of #3679. For the classifier-dependencies we need to resort to full aggregate project resolution as applying a filter to just resolve the artifact we need the file for results in a NullPointerException due to a bug in the maven shared libraries for artifact resolution filtering (the MSHARED-998 issue referenced in there). Missing in the testing of the work-around was the case that you now run into - unresolvable reactor-project versions, which means that for any resolution of a classifier-dependency there is an exception. You can see that effect as well in the report, as the |
@aikebah Thanks for looking at this! I don't think those changes are responsible for the slowness I'm seeing. The times a mentioned are when running with an up-to-date CVE database. Is it possible to adapt the work-around to accommodate this case as well? |
Hope to extend the work around to make this case work as well, needs some experimentation |
@aikebah I tested locally with your branch
These dependencies exists in my local Maven repo, example:
Common thing seems to be that they all have a classifier and they are not reactor-projects. I'll see if I can extend the PoC to cover this as well. |
@viktor-thell-seal gut feel is that for the project handled the given dependency is a transitive dependency via another reactor project. I still wanted to check such a scenario as I can imagine that removing the reactor projects might result in issues with transitive dependencies of those in some way. That it's only jars with classifiers that give trouble is logical, as the given code-branch deals with handling the dependencies with classifiers only (the 'regular' non-classifier artifacts are succesfully handled by the other codebranch that feeds a filter to the maven shared routine in order to only resolve that specific artifact. I hope to add some extra logging to the branch late tonight (CEST) in order to show detail about the (child)module in processing when running into these errors. |
@aikebah It's indeed the transitive dependencies with classifiers that causes this. I have extended the PoC to cover this as well. When running with your branch on the
|
@viktor-thell-seal Thanks for the update project. Running the final local tests on a modified strategy to resolve the dependencies to see I didn't break any other integration-tested case, but for your project
for my local snapshot. If all tests pass I'll update the branch in github and finalize the PR |
Makes other cases fail, so have to investigate further |
…itive dependencies via reactor projects as per #3730 (comment)
@viktor-thell-seal Can you check whether my branch now successfully covers all cases in your project? |
@aikebah Great work, it now successfully passes the projects we have! 👍🏻 The only "issue" is that performance is really bad. A project we have has gone from < 1 min (on 6.3.1 which is the latest working release) to ~4:30 with your branch, but I guess this is due to the "full aggregate project resolution" that is considerably slower. |
Yes, likely you have multiple projects with the classifier-artifacts in your analysis tree. Which, for every submodule triggers a full round of 'all dependencies' dependency-resolving. Hope to spend some time in the coming weeks if other activities allow to take a look at the overall logic and see if we can restructure it to do a single resolution round now that I've found (in debugging for this issue) that on a resolution exception we have all the successfully resolved artifacts hidden inside the exception. But such a case would be almost a full rewrite of the class, so for now I focussed on getting it back to working. |
@aikebah Sounds good. Thanks for resolving this! |
Describe the bug
When running an aggregate check for a project we have we get a
Maven31DependencyResolverException
. This project has worked fine to check with previous versions of the plugin.One strange this is that the dependency check actually seems to run correctly as it finds dependency issues, but it is very slow. On one project it now runs for ~4:30 min where it on 6.2.2 only takes ~0:30 min to complete.
Version of dependency-check used
The problem occurs using version 6.4.0 and 6.4.1 of the maven plugin.
Log file
https://gist.github.com/viktor-thell-seal/c5c91f6b0ab12fb97f074ec442861ed6
To Reproduce
I have created a minimal project where this can be seen:https://github.com/viktor-thell-seal/odc-example
Run
mvn dependency-check:aggregate
on the project to reproduce.Expected behavior
The aggregate dependency check should succeed.
Additional context
I have tested this with a couple versions of ODC:
6.2.2 - Works!
6.3.1 - NPE (#3679)
6.4.0 - Maven31DependencyResolverException
6.4.1 - Maven31DependencyResolverException
My guess is that this is related to #3679 and #3627.
The text was updated successfully, but these errors were encountered: