fix(yarn2): Avoid failing on ambiguously resolved locators#12145
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #12145 +/- ##
============================================
+ Coverage 58.60% 58.64% +0.03%
- Complexity 1825 1840 +15
============================================
Files 361 361
Lines 13578 13632 +54
Branches 1404 1411 +7
============================================
+ Hits 7958 7994 +36
- Misses 5114 5129 +15
- Partials 506 509 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
oheger-bosch
left a comment
There was a problem hiding this comment.
Sophisticated stuff! I have some questions.
There was a problem hiding this comment.
Pull request overview
This PR updates ORT’s Yarn 2+ analyzer to tolerate valid Yarn Berry installs where multiple installed locators exist for the same module (e.g., npm vs. @yarnpkg/plugin-compat patched variants), avoiding hard failures by selecting a best candidate and surfacing a warning instead.
Changes:
- Add locator “patch vs. non-patch” awareness and improved semver-range extraction for patch descriptors during fallback resolution.
- When ambiguity remains, pick the highest semantic version and record/report a WARNING issue instead of throwing.
- Extend test coverage to validate patch detection, ambiguity handling, warning draining, and semantic version ordering.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| plugins/package-managers/node/src/test/kotlin/yarn2/Yarn2DependencyHandlerTest.kt | Adds tests for patch detection and for ambiguity resolution now producing WARNINGs while selecting the highest version. |
| plugins/package-managers/node/src/main/kotlin/yarn2/Yarn2DependencyHandler.kt | Implements ambiguity-tolerant resolution with patch-type preference, semver-range parsing for patch descriptors, and warning recording/draining. |
| plugins/package-managers/node/src/main/kotlin/yarn2/Yarn2.kt | Includes drained ambiguity warnings in the per-project analyzer result issues. |
Previously, ORT aborted the analysis of a definition file with an `IllegalStateException` when a dependency could not be resolved to a single installed package. This happens for valid Yarn Berry trees that install several versions of the same module, for example a package that exists both as a plain npm package and as a compat-patched variant added by `@yarnpkg/plugin-compat`. The fallback that looks up installed packages by module name now prefers candidates of the same patch or non-patch type as the locator, and reads the npm version range embedded in patch descriptors. If more than one candidate remains, the highest version is picked and a warning is recorded on the project instead of failing the analysis. Fixes oss-review-toolkit#12008. Signed-off-by: Julian Olderdissen <julian.olderdissen@bosch.com>
5c37856 to
84ac07b
Compare
Previously, ORT aborted the analysis of a definition file with an
IllegalStateExceptionwhen a dependency could not be resolved to a single installed package. This happens for valid Yarn Berry trees that install several versions of the same module, for example a package that exists both as a plain npm package and as a compat-patched variant added by@yarnpkg/plugin-compat.The fallback that looks up installed packages by module name now prefers candidates of the same patch or non-patch type as the locator, and reads the npm version range embedded in patch descriptors. If more than one candidate remains, the highest version is picked and a warning is recorded on the project instead of failing the analysis.
Fixes #12008.