fix: handling of --all when local dep name and dir name differ#3664
fix: handling of --all when local dep name and dir name differ#3664topecongiro merged 1 commit intorust-lang:masterfrom
Conversation
| .find(|p| p.name == dependency.name); | ||
| let manifest_path = if dependency_package.is_some() { | ||
| PathBuf::from(&dependency_package.unwrap().manifest_path) | ||
| } else { |
There was a problem hiding this comment.
I'm not sure if there's a circumstance where the dependency name wouldn't be found/when the if block woudln't be executed, but have this else block to fallback to the previous behavior just in case.
| ) -> Result<(), io::Error> { | ||
| let metadata = get_cargo_metadata(manifest_path)?; | ||
| let metadata = get_cargo_metadata(manifest_path, false)?; | ||
| let metadata_with_deps = get_cargo_metadata(manifest_path, true)?; |
There was a problem hiding this comment.
The dependency struct returned from the cargo_metadata crate unsurprisingly does not include info about the manifest location of the dependency.
As such, I figured I could make another call to cargo_metadata to get the metadata including dependencies since that cargo metadata will include the correct manifest path for the dependency, regardless of what package name/directory name are used. That "correct" manifest path is then used below.
There was a problem hiding this comment.
Calling cargo_metadata twice is a bit unfortunate, but I think this is the best we can do right now. A possible approach in the future would be to refactor cargo_metadata so that the cargo_metadata::Dependency contains the manifest location if the dependency is local
There was a problem hiding this comment.
Completely agree 👍
calebcartwright
changed the title
topecongiro
left a comment
topecongiro
left a comment
There was a problem hiding this comment.
Thank you for investigating the issue and creating the PR! LGTM.
| ) -> Result<(), io::Error> { | ||
| let metadata = get_cargo_metadata(manifest_path)?; | ||
| let metadata = get_cargo_metadata(manifest_path, false)?; | ||
| let metadata_with_deps = get_cargo_metadata(manifest_path, true)?; |
There was a problem hiding this comment.
Calling cargo_metadata twice is a bit unfortunate, but I think this is the best we can do right now. A possible approach in the future would be to refactor cargo_metadata so that the cargo_metadata::Dependency contains the manifest location if the dependency is local
2 participants
Refs #3643 (the
cargo fmt --allportion)Fixes issue running
cargo fmt --allwhen local dependencies reside in a directory name that differs from the package name.I created a sample repo here with local dependencies that reside in directories named differently than their package names that can be used to test against. Run
cargo fmt --all -- --checkin the root of the project, and note the formatting errors coming from the local dependency directories.