[Graph] Fix handling of multiple version resolution#15099
Merged
Conversation
5 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes npm/yarn/pnpm dependency graph resolution when the same package is present at multiple resolved versions, ensuring each distinct version is captured as its own node and that subdependency edges point to the correct versioned node.
Changes:
- Override
NpmAndYarn::DependencyGrapher#resolved_dependenciesto expand:all_versionsinto distinct resolved dependency entries. - Switch relationship mapping to use
name@versionkeys and emit subdependency edges as versioned purls. - Add fixtures + spec coverage for multi-version scenarios across npm, Yarn v1, pnpm (v6 and v9), including scoped packages.
Show a summary per file
| File | Description |
|---|---|
| npm_and_yarn/lib/dependabot/npm_and_yarn/dependency_grapher.rb | Implements multi-version expansion and version-aware relationship resolution across npm/yarn/pnpm. |
| npm_and_yarn/spec/dependabot/npm_and_yarn/dependency_grapher_spec.rb | Adds test coverage for multi-version dependency graph scenarios (including nested/scoped cases). |
| npm_and_yarn/spec/fixtures/projects/grapher/yarn_with_multiversion_subdeps/yarn.lock | Yarn v1 lockfile fixture with two resolved versions of the same package. |
| npm_and_yarn/spec/fixtures/projects/grapher/yarn_with_multiversion_subdeps/package.json | Manifest fixture corresponding to the Yarn multiversion case. |
| npm_and_yarn/spec/fixtures/projects/grapher/yarn_with_multiversion_subdeps_nested/yarn.lock | Yarn v1 lockfile fixture for nested multi-version-with-subdeps scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/yarn_with_multiversion_subdeps_nested/package.json | Manifest fixture for nested Yarn multiversion scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps/pnpm-lock.yaml | pnpm v6 lockfile fixture containing multiple versions of a transitive dependency. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps/package.json | Manifest fixture for pnpm v6 multiversion scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps_nested/pnpm-lock.yaml | pnpm v9-style lockfile fixture with nested multi-version relationships. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps_nested/package.json | Manifest fixture for pnpm nested multiversion scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_v9_with_multiversion_subdeps/pnpm-lock.yaml | pnpm v9 lockfile fixture for multiversion transitive dependencies. |
| npm_and_yarn/spec/fixtures/projects/grapher/pnpm_v9_with_multiversion_subdeps/package.json | Manifest fixture for pnpm v9 multiversion scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps/package.json | npm manifest fixture for a multiversion transitive dependency scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps/package-lock.json | npm lockfile fixture exhibiting two installed versions of the same dependency. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps_nested/package.json | npm manifest fixture for nested multiversion-with-subdeps scenario. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps_nested/package-lock.json | npm lockfile fixture for nested multiversion relationships. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_scoped/package.json | npm manifest fixture for scoped packages with multiple resolved versions. |
| npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_scoped/package-lock.json | npm lockfile fixture demonstrating scoped multi-version dependency chains. |
Copilot's findings
Files not reviewed (6)
- npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_scoped/package-lock.json: Language not supported
- npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps/package-lock.json: Language not supported
- npm_and_yarn/spec/fixtures/projects/grapher/npm_with_multiversion_subdeps_nested/package-lock.json: Language not supported
- npm_and_yarn/spec/fixtures/projects/grapher/pnpm_v9_with_multiversion_subdeps/pnpm-lock.yaml: Language not supported
- npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps/pnpm-lock.yaml: Language not supported
- npm_and_yarn/spec/fixtures/projects/grapher/pnpm_with_multiversion_subdeps_nested/pnpm-lock.yaml: Language not supported
- Files reviewed: 10/18 changed files
- Comments generated: 5
… pnpm peer suffixes - Return nil for blank versions in split_name_version to prevent malformed purls - Skip npm lockfile entries (v2/v3 and v1) with nil/blank versions (workspace links) - Match Yarn grouped requirement keys (e.g. 'foo@^1, foo@^2') properly - Strip pnpm peer metadata suffixes from child versions before building purls - Add test fixtures: npm workspace link, yarn grouped keys, pnpm peer metadata - Extract resolve_npm_v3_children and resolve_npm_v1_children to satisfy complexity cops Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
brrygrdn
force-pushed
the
brrygrdn/dg-11931-capture-multi-version-graphs
branch
from
29b3b7e to
957fc1b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish?
When testing with a variety of projects, I noticed a deviation from our existing parsers when the package manager resolved subdependencies to different versions of the same package - we would only collect the version that occurred first in as we navigated the graph downwards from direct dependencies.
This PR fixes this behaviour by ensuring that we use the
all_versionsproperty of each dependency to add all of the versions we've encountered to the snapshot.The deeper change set is starting to collect child dependencies using
name@versioninstead ofnameto ensure the relationships are correctly mapped as well.Anything you want to highlight for special attention from reviewers?
This approach is a fairly natural progression on the existing implementation, the biggest consequence is we now need to override more of the base class for npm-specific resolution paths.
How will you know you've accomplished your goal?
I've primarily used realistic fixtures to validate this change set, rather than drop in a complex real-world example I used copilot to find real-world dependency relationships that I could use to illustrate minimal examples of both:
Checklist