cargo: Fix duplicate Cargo.lock entries for feature-gated git dependencies#14725
cargo: Fix duplicate Cargo.lock entries for feature-gated git dependencies#14725
Conversation
c6fec0e to
ca6b480
Compare
There was a problem hiding this comment.
Pull request overview
This PR aims to prevent Cargo from producing invalid Cargo.lock output when updating a project that includes SSH-based git dependencies behind non-default features, by ensuring Cargo sees consistent git source URLs during the update.
Changes:
- Write the temporary
Cargo.lockwith SSH git URLs rewritten to HTTPS before runningcargo update. - Add a new lockfile updater spec context for an SSH URL + feature-gated git dependency scenario.
- Add new manifest/lockfile fixtures for the above spec.
Show a summary per file
| File | Description |
|---|---|
cargo/lib/dependabot/cargo/file_updater/lockfile_updater.rb |
Writes the temporary lockfile with SSH→HTTPS URL rewriting to avoid source mismatches during cargo update. |
cargo/spec/dependabot/cargo/file_updater/lockfile_updater_spec.rb |
Adds a regression spec around feature-gated SSH git dependencies and URL swapping behavior. |
cargo/spec/fixtures/manifests/feature_gated_git_dep_ssh |
New Cargo.toml fixture modeling a feature-gated optional SSH git dependency. |
cargo/spec/fixtures/lockfiles/feature_gated_git_dep_ssh |
New Cargo.lock fixture used by the regression spec. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
| before do | ||
| captured = lockfile_on_disk | ||
| allow(updater).to receive(:run_cargo_command) do |_command, **_kwargs| | ||
| lockfile_content = File.read("Cargo.lock") | ||
| captured << lockfile_content.dup | ||
|
|
||
| old_time = "version = \"0.1.38\"\n" \ | ||
| "source = \"registry+https://github.com/rust-lang/crates.io-index\"\n" \ | ||
| "dependencies = [\n" \ | ||
| " \"kernel32-sys" | ||
| new_time = "version = \"0.1.40\"\n" \ | ||
| "source = \"registry+https://github.com/rust-lang/crates.io-index\"\n" \ | ||
| "dependencies = [\n" \ | ||
| " \"kernel32-sys" | ||
| lockfile_content = lockfile_content.gsub(old_time, new_time) | ||
| lockfile_content = lockfile_content.gsub( | ||
| "d5d788d3aa77bc0ef3e9621256885555368b47bd495c13dd2e7413c89f845520", | ||
| "d825be0eb33fda1a7e68012d51e9c7f451dc1a69391e7fdc197060bb8c56667b" | ||
| ) | ||
|
|
||
| File.write("Cargo.lock", lockfile_content) | ||
| end |
There was a problem hiding this comment.
This new spec context doesn’t actually reproduce the reported failure mode (two [[package]] stanzas with the same (name, version, source) but different dependency lists). The stubbed run_cargo_command only updates the time stanza, so the spec won’t fail if the duplicate-entry bug reappears. Consider adjusting the stubbed Cargo.lock output to include both the SSH- and HTTPS-sourced entries (with different dependency lists) and assert the final lockfile contains only the correct single entry.
…ncies When a Cargo project has an optional SSH git dependency behind a non-default feature, the lockfile updater swaps SSH→HTTPS in manifests but writes the lockfile with original SSH URLs. Cargo sees mismatched sources and creates a second [[package]] entry for the git dep via HTTPS. After post-processing swaps HTTPS→SSH, both entries share the same identity but have different dependency lists, producing an invalid lockfile. Fix: also apply the SSH→HTTPS swap to the lockfile before writing it to disk. This way cargo sees consistent HTTPS URLs in both manifest and lockfile, doesn't re-resolve the git dep, and produces no duplicates. post_process_lockfile already handles swapping URLs back to SSH afterward. Reproduction verified end-to-end using dependabot/cli against real projects (jurre/repro-crate + jurre/helper-crate) mirroring the customer's structure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ca6b480 to
cf31827
Compare
jurre
left a comment
There was a problem hiding this comment.
Addressed: the test stub now conditionally injects the duplicate entry based on whether the lockfile still has SSH URLs — simulating cargo's actual behavior. If the fix is reverted, the stub produces the duplicate and both tests fail (verified).
Summary
Fixes invalid
Cargo.lockfiles produced when a project has an optional SSH git dependency behind a non-default feature.Problem
When Dependabot updates a dependency (e.g.,
regex) in a Cargo project that has a feature-gated optional SSH git dependency, the lockfile updater produces two[[package]]entries with the same(name, version, source)triple but different dependency lists. Cargo rejects this at parse time:Root cause
prepared_manifest_contentswaps SSH→HTTPS URLs in manifests so cargo can fetch through the proxy without SSH keys. But the lockfile was written with the original SSH URLs. Cargo sees the HTTPS URL in the manifest and the SSH URL in the lockfile as different sources, so it re-resolves the git dependency via HTTPS with only default features — creating a second[[package]]entry with fewer dependencies.post_process_lockfilethen swaps HTTPS→SSH in the output, making both entries share the same identity but with different dep lists.Fix
Apply the same SSH→HTTPS swap to the lockfile before writing it to disk, so cargo sees consistent URLs in both manifest and lockfile. Cargo then recognizes the existing lockfile entry, doesn't re-resolve the git dep, and produces no duplicates.
post_process_lockfilealready handles swapping URLs back to SSH afterward.The production change is a single line:
Verification
dependabot updatevia CLI produces duplicate entries before the fix and a single correct entry after.