Skip to content

Conversation

@gemaxim
Copy link
Contributor

@gemaxim gemaxim commented May 14, 2025

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

Bumps snyk-mvn-plugin to include: snyk/snyk-mvn-plugin#189.
Includes:

  1. dfs for graph creation - for cycles and dense graphs.
  2. package version in the key for the visited/ancestry.

As mentioned on the PR, dependening on the decision to only keep the scope/versions that maven resolves dependencies to, instead of the Dverbose output, a change will come on top of this one.

Where should the reviewer start?

snyk/snyk-mvn-plugin#189

How should this be manually tested?

Test with the example here.

What's the product update that needs to be communicated to CLI users?

[test, monitor, sbom] Maven Dverbose improvement for long running scans resulting from dense dependency graphs creation.

What are the relevant tickets?

https://snyksec.atlassian.net/browse/OSM-2668

@gemaxim gemaxim requested a review from a team as a code owner May 14, 2025 09:47
@gemaxim gemaxim self-assigned this May 14, 2025
@gemaxim gemaxim force-pushed the feat/OSM-2668/maven-dverbose-dfs branch from 03714ca to 76f3f7e Compare May 14, 2025 13:45
@gemaxim gemaxim force-pushed the feat/OSM-2668/maven-dverbose-dfs branch from 76f3f7e to 5abe28b Compare May 14, 2025 14:21
Comment on lines -64 to -66
expect(sbom.dependencies[6].dependsOn.length).toEqual(1);
expect(sbom.dependencies[6].dependsOn[0]).toEqual(
'commons-logging:commons-logging@1.0.4',
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious about this change, can you explain? Length of dependencies because you go depth first I presume? But why does the version change?

Copy link
Contributor Author

@gemaxim gemaxim May 14, 2025

Choose a reason for hiding this comment

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

package version in the key for the visited/ancestry. this means that we'll include all the versions that dverbose outputs. An example, with scope too is included is here in this output: https://github.com/snyk/snyk-mvn-plugin/pull/188/files#diff-2ce7e54ca0da5e7003be9ff7c3606b9a54fbcddf4b01c062b202b57786475b94.
That version is not actually resolved by maven, but maven Dverbose does mention it as omitted.
We are waiting on a product decision here for our Dverbose functionality, I personally think that only the version that maven resolves should be included.
But, in the meantime, to unblock, we are adding exactly what Dverbose outputs.

@gemaxim gemaxim merged commit b7a84f5 into main May 15, 2025
15 checks passed
@gemaxim gemaxim deleted the feat/OSM-2668/maven-dverbose-dfs branch May 15, 2025 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants