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

Cargo incorrectly detects a package name conflict when a workspace package has the same name as a (transitive) dependency #12891

Open
avsaase opened this issue Oct 28, 2023 · 7 comments
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@avsaase
Copy link

avsaase commented Oct 28, 2023

Problem

Take a project directory structure like this:

.
├── Cargo.lock
├── Cargo.toml
├── examples
│   └── grid
│       ├── Cargo.toml
│       └── src
│           └── main.rs
└── src
    └── lib.rs

Cargo.toml:

[package]
name = "library"
version = "0.1.0"
edition = "2021"

[dependencies]
taffy = { version = "0.3.16", features = ["grid"] }

[workspace]
members = ["examples/grid"]

examples/grid/Cargo.toml:

[package]
name = "grid"
version = "0.1.0"

[dependencies]
library = { path = "../../" }

Dependency tree:

library v0.1.0 ([omitted])
└── taffy v0.3.16
    ├── arrayvec v0.7.4
    ├── grid v0.10.0
    ├── num-traits v0.2.17
    │   [build-dependencies]
    │   └── autocfg v1.1.0
    └── slotmap v1.0.6
        [build-dependencies]
        └── version_check v0.9.4

When you try to run the grid example with cargo run --package grid you get an error:

error: There are multiple `grid` packages in your project, and the specification `grid` is ambiguous.
Please re-run this command with `-p <spec>` where `<spec>` is one of the following:
  [email protected]
  [email protected]

Steps

  1. Checkout https://github.com/avsaase/cargo-ambiguous-package
  2. Run cargo run --package grid
  3. Run cargo run --package [email protected]

Possible Solution(s)

The problem seems to be that cargo sees the transitive grid dependency of taffy and sees that as a conflict with the package of the same name in the local workspace. This must be a bug because there is no reason to run a binary target from a (transitive) dependency with cargo run. And even if there was, the grid crate doesn't have a binary target so there's nothing to run.

The solution suggested by cargo is also not correct because running cargo run -p [email protected] gives the error that the package is not found in the workspace.

A workaround is to use globally unique package names in workspaces but this is very restrictive because adding a dependency can easily create a new conflict.

Notes

No response

Version

cargo 1.73.0 (9c4383fb5 2023-08-26)
release: 1.73.0
commit-hash: 9c4383fb55986096b414d98125421ab87b5fd642
commit-date: 2023-08-26
host: x86_64-unknown-linux-gnu
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 8.2.1-DEV (sys:0.4.65+curl-8.2.1 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Linux 16 (focal) [64-bit]
@avsaase avsaase added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Oct 28, 2023
@epage
Copy link
Contributor

epage commented Oct 28, 2023

The challenge is you're dealing with layers of calculations. We first look up a package and then the relevant command does what it wants with it. We also don't know what bins are in package unless we parse the manifest. So for this to fully work as requested, we'd need to cross layers and then load the manifests to resolve the ambiguity. We'd need the ambiguity resolution to be unambiguous to the user since (1) they might not be aware of details and (2) they can make mistakes.

Something I've considered is

  • Most commands are not destructive
  • Users generally mean to specify workspace members

If we could prioritize workspace members with a warning saying other options are available and still error when all else is equal, I think that might be good enough.

@avsaase
Copy link
Author

avsaase commented Oct 28, 2023

But why would cargo run ever run a package from a dependency? I fail to see a use case for that.

@epage
Copy link
Contributor

epage commented Oct 30, 2023

I didn't say that part was ideal. I was more explaining why it is the way it is and the challenge with changing it to something else.

@epage
Copy link
Contributor

epage commented Nov 1, 2023

#8157 also touches on the problems with us evaluating the available packages before the available binaries.

@IceTDrinker
Copy link

Hello, this is of interest also in the semver trick approach https://github.com/dtolnay/semver-trick to add compatibility code (e.g. for serialization) to an old version of a crate by depending on a more recent version of the crate and adding bridge/glue code in the old crate allowing users of the old crate to use a minor release to proceed with serialization format update for example.

IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 8, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 8, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
@IceTDrinker
Copy link

IceTDrinker commented Nov 8, 2023

a workaround seems to not specify the package at all, this is an issue if you have several crates with colliding binary/example names I guess

i.e. if you have unique names for binaries/examples you can

cargo run --release --bin my_bin

as long as my_bin has a unique name in the workspace it should be run ok, adding -p my_package when my_package is also a transitive dependency brings you back to the original issue where the name is ambiguous, but running with the spec is not accepted

IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 8, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 9, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 9, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
IceTDrinker added a commit to zama-ai/tfhe-rs that referenced this issue Nov 10, 2023
- adding the tfhe package as a dependency is currently causing issues with
Cargo because of unified feature resolution it seems, it needs an
additional version specifier to disambiguate which package we are referring
to, an issue exists on their end but I don't think a fix is to be expected
soon rust-lang/cargo#12891
- commiting this to main and then backporting the relevant pieces to 0.4.x
@elagergren-spideroak
Copy link

This also occurs with cargo test -p foo where your workspace has a crate called foo and you also have a dependency called foo (renamed to something else, of course).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

4 participants