Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple path dependencies with the same name confuses the resolver #13405

Open
emilio opened this issue Feb 6, 2024 · 4 comments · May be fixed by #13572
Open

multiple path dependencies with the same name confuses the resolver #13405

emilio opened this issue Feb 6, 2024 · 4 comments · May be fixed by #13572
Assignees
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@emilio
Copy link

emilio commented Feb 6, 2024

I recently published a new rust-bindgen version (0.69.4), and that caused some Firefox CI jobs to fail (https://bugzilla.mozilla.org/show_bug.cgi?id=1878575), because some CI checks that run cargo metadata ended up touching the lockfile and updating bindgen.

The relevant bits that seem to be needed to reproduce this is a local crate for an older version of the crate.

To reproduce:

  • git clone https://github.com/mozilla/gecko-dev
  • cd gecko-dev
  • git reset --hard 9d8d867dac5d35156a51ef21fe7b46f573aaabef
  • cargo metadata

Somehow, the lockfile changes to bindgen 0.69.4 even though the lockfile specifies 0.69.2

@emilio
Copy link
Author

emilio commented Feb 6, 2024

(Apologies for not having something more reduced, can try to do so if needed, though might take a bit)

@ehuss
Copy link
Contributor

ehuss commented Feb 6, 2024

Thanks for the report! I believe I have a partial understanding of the problem. The following is a test for cargo's testsuite that demonstrates the issue:

#[cargo_test]
fn patched_reexport_stays_locked() {
    // Patch example where you emulate a semver-incompatible patch via a re-export.
    // Testing an issue where the lockfile does not stay locked after a new version is published.
    Package::new("bar", "1.0.0").publish();
    Package::new("bar", "2.0.0").publish();
    Package::new("bar", "3.0.0").publish();

    let p = project()
        .file(
            "Cargo.toml",
            r#"
                [workspace]


                [package]
                name = "foo"

                [dependencies]
                bar1 = {package="bar", version="1.0.0"}
                bar2 = {package="bar", version="2.0.0"}

                [patch.crates-io]
                bar1 = { package = "bar", path = "bar-1-as-3" }
                bar2 = { package = "bar", path = "bar-2-as-3" }
            "#,
        )
        .file("src/lib.rs", "")
        .file(
            "bar-1-as-3/Cargo.toml",
            r#"
                [package]
                name = "bar"
                version = "1.0.999"

                [dependencies]
                bar = "3.0.0"
            "#,
        )
        .file("bar-1-as-3/src/lib.rs", "")
        .file(
            "bar-2-as-3/Cargo.toml",
            r#"
                [package]
                name = "bar"
                version = "2.0.999"

                [dependencies]
                bar = "3.0.0"
            "#,
        )
        .file("bar-2-as-3/src/lib.rs", "")
        .build();

    p.cargo("tree")
        .with_stdout(
            "\
foo v0.0.0 ([ROOT]/foo)
├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3)
│   └── bar v3.0.0
└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3)
    └── bar v3.0.0
",
        )
        .run();

    fs::copy(
        p.root().join("Cargo.lock"),
        p.root().join("Cargo.lock.orig"),
    )
    .unwrap();

    Package::new("bar", "3.0.1").publish();
    p.cargo("tree")
        .with_stdout(
            "\
foo v0.0.0 ([ROOT]/foo)
├── bar v1.0.999 ([ROOT]/foo/bar-1-as-3)
│   └── bar v3.0.0
└── bar v2.0.999 ([ROOT]/foo/bar-2-as-3)
    └── bar v3.0.0
",
        )
        .run();

    assert_eq!(p.read_file("Cargo.lock"), p.read_file("Cargo.lock.orig"));
}

The problem I believe is here. When loading the original Cargo.lock, that bit of code expects package names to be unique in the set of path dependencies. However, in this scenario there are two packages both with the same name. I think the solution is to also check the version, though there might be some subtle issues there.

@ehuss ehuss added C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review A-lockfile Area: Cargo.lock issues labels Feb 6, 2024
@ehuss ehuss changed the title cargo metadata can somehow update dependencies in some edge cases multiple path dependencies with the same name confuses the resolver Feb 6, 2024
@linyihai
Copy link
Contributor

linyihai commented Mar 4, 2024

@rustbot claim

@linyihai
Copy link
Contributor

Hi, I copy the test into the cargo repo and get this output

   Compiling cargo v0.79.0 (/root/workspace/cargo)
    Finished test [unoptimized + debuginfo] target(s) in 40.17s
     Running tests/testsuite/main.rs (target/debug/deps/testsuite-a446100106b28075)

running 1 test
running `/root/workspace/cargo/target/debug/cargo tree`
running `/root/workspace/cargo/target/debug/cargo tree`
thread 'tree::patched_reexport_stays_locked' panicked at tests/testsuite/tree.rs:2287:10:

test failed running `/root/workspace/cargo/target/debug/cargo tree`
error: stdout did not match:
1   1     foo v0.0.0 (/root/workspace/cargo/target/tmp/cit/t0/foo)
2   2     ├── bar v1.0.999 (/root/workspace/cargo/target/tmp/cit/t0/foo/bar-1-as-3)
3        -│   └── bar v3.0.0
    3    +│   └── bar v3.0.1
4   4     └── bar v2.0.999 (/root/workspace/cargo/target/tmp/cit/t0/foo/bar-2-as-3)
5        -    └── bar v3.0.0
    5    +    └── bar v3.0.1

Hmm, the test case failed to run, and I think the final code fix is to get the use case to pass

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue May 5, 2024
…es. r=supply-chain-reviewers

Because fallible_collections pulls hashbrown 0.13, we also upgrade
hashlink to 0.8.2, which updates to that version as well. Those were the
last two uses of hashbrown 0.12, so we can update the fake hashbrown
0.12 to 0.13.

We could skip the upgrade of hashlink, but that would leave us with two
fake hashbrowns, and we'd hit rust-lang/cargo#13405

Differential Revision: https://phabricator.services.mozilla.com/D209317
ErichDonGubler pushed a commit to ErichDonGubler/firefox that referenced this issue May 6, 2024
…es. r=supply-chain-reviewers

Because fallible_collections pulls hashbrown 0.13, we also upgrade
hashlink to 0.8.2, which updates to that version as well. Those were the
last two uses of hashbrown 0.12, so we can update the fake hashbrown
0.12 to 0.13.

We could skip the upgrade of hashlink, but that would leave us with two
fake hashbrowns, and we'd hit rust-lang/cargo#13405

Differential Revision: https://phabricator.services.mozilla.com/D209317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lockfile Area: Cargo.lock issues C-bug Category: bug S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants