Fix some Maven manifest & resolver issues #1008
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1008 +/- ##
==========================================
+ Coverage 65.04% 65.16% +0.12%
==========================================
Files 149 149
Lines 12252 12338 +86
==========================================
+ Hits 7969 8040 +71
- Misses 3835 3847 +12
- Partials 448 451 +3 ☔ View full report in Codecov by Sentry. |
|
|
||
| newKeys := allKeys() | ||
| if len(prevKeys) != len(newKeys) { | ||
| return errors.New("number of dependencies changed after interpolation") |
There was a problem hiding this comment.
the number of dependencies may get changed after interpolation if we are not able to resolve any placeholder, but this should be very rare...
There was a problem hiding this comment.
It's kind of hard to know which dependencies are which in those cases, so I'll leave it as an error for now.
Added a comment here.
| @@ -238,6 +309,12 @@ func (m MavenManifestIO) MergeParents(ctx context.Context, result *maven.Project | |||
| // - identifier to locate the profile/plugin which is profile ID or plugin name | |||
| // - (optional) suffix indicates if this is a dependency management | |||
| func makeRequirementVersion(dep maven.Dependency, origin string) resolve.RequirementVersion { | |||
| // Treat test & optional dependencies as regular dependencies to force the resolver to resolve them. | |||
There was a problem hiding this comment.
I think this should be fine for now, but we may "fix" the resolver to handle test and optional dependencies if it is not that much work, and this would benefit transitive support when scanning pom.xml.
Some fixes for a few Maven resolver issues I've come across: 1. Requirement origins weren't being tracked correctly if a package key was set by a property. Fixed by checking dependency keys before and after interpolation, and updating the map if they changed. (I've modified one of the tests to check for this case) 2. To work around the resolver not resolving test or optional dependencies, made it so the pom.xml parser removes the test scope and optional flags. 3. `resolve.PackageKey` was not sufficient to uniquely key the requirements for the `Groups` map in the `Manifest`. Made a new `RequirementKey` type with ecosystem-specific information for both npm and maven to solve this.
Some fixes for a few Maven resolver issues I've come across:
resolve.PackageKeywas not sufficient to uniquely key the requirements for theGroupsmap in theManifest. Made a newRequirementKeytype with ecosystem-specific information for both npm and maven to solve this.