Add crate versions when running cargo -p commands. #3781
Conversation
| @@ -77,7 +77,11 @@ impl CargoTargetSpec { | |||
| ProjectWorkspace::Cargo { cargo, .. } => { | |||
| let tgt = cargo.target_by_root(&path)?; | |||
| Some(CargoTargetSpec { | |||
| package: cargo[cargo[tgt].package].name.clone(), | |||
There was a problem hiding this comment.
Would cargo[cargo[tgt].package].id.clone() work here?
There was a problem hiding this comment.
Hmm I think it might, for now: https://docs.rs/cargo_metadata/0.9.1/cargo_metadata/struct.PackageId.html
It is possible to inspect the repr field, if the need arises, but its precise format is an implementation detail and is subject to change.
Would you like me to give it a try?
There was a problem hiding this comment.
Yes, and if it works, I think we need to use this field, as it is the package id spec.
OTOH, it's also useful to keep this command human editable, so I think ideally the logic should be:
- if there's a single package with this name, use the name
- if there can be ambiguity, use the fully unabmigious id.
There was a problem hiding this comment.
That's a great idea, I'm going to give it a go!
o0Ignition0o
force-pushed
the
crate_version
branch
from
ff75713
to
3e937ee
Compare
|
First tryouts implementing this:
Need to figure out a way to test the "conflicting names". |
Until now cargo commands with the -p flag would pass the package name only. It doesn't play super well with the toml Renaming dependencies feature. This commit specifies the package name and version when a cargo command is run with the -p flag, to avoid ambiguities.
o0Ignition0o
force-pushed
the
crate_version
branch
from
05d42a4
to
331d1db
Compare
|
bors r+ We generally don't test the integration bit between cargo/vscode/rust-analyzer too thoroughly, as these things usually break in a ways which are more interesting than a typical unit-test can catch. |
Build succeeded |
3819: Unique package by name and version. r=matklad a=o0Ignition0o This commit is a fixup of #3781 I introduced a bug by using a PackageId to refer to a crate when its name conflicts with a dependency. It turns out the package id currently is `name version path` while cargo expects `name:version` as argument eg: Cargo command with a `PackageId`: ``` > Executing task: cargo test --package 'config 0.1.0 (path+file:///Users/ignition/Projects/oss/config)' --test default -- test_with_name --exact --nocapture < ``` Cargo command with `name:version`: ``` > Executing task: cargo test --package 'config:0.1.0' --test default -- test_with_name --exact --nocapture < ``` Co-authored-by: o0Ignition0o <jeremy.lempereur@gmail.com>
If someone (unfortunately) creates a project that happens to have the same name as one of its (future) dependencies, there is a way for them to change the dependency's alias in the Cargo.toml file, to mitigate the name conflict. Unfortunately cargo -p commands don't seem to pick it up, which seems to put rust-analyzer run commands in a tough situation:
cargo suggests us to be more specific and refer to a package by its name and version, which this PR achieves.
I passed the version as a String because I don't really understand how the ra_db types work, but I would love to switch it to a fully fledged Version type if you guide me towards that :)