.cargo/config path override & crates with optional deps broken #3313
Comments
alexcrichton
added a commit
to alexcrichton/cargo
that referenced
this issue
Nov 28, 2016
Bad path overrides are currently detected to issue warnings in cases where path overrides are not suitable and have exhibited buggy behavior in the past. Unfortunately though it looks like some false positives are being issued, causing unnecessary confusion about `paths` overrides. This commit fixes the detection of these "bad path overrides" by comparing `Summary` dependencies (what's written down in `Cargo.toml`) rather than comparing the `Cargo.toml` of the override with `Cargo.lock`. We're guaranteed that the package we're overridding has already been resolved into `Cargo.lock`, so we know that if the two `Cargo.toml` files are equivalent we'll continue with the same crate graph. I'm not actually entirely sure why I originally thought it'd be better to go through the `Cargo.lock` comparison route. Unfortunately that doesn't take into account optional deps which aren't in `Cargo.lock` but are in `Cargo.toml` of the override, causing the false positive. This method, however, simply ensures that the two dependency lists are the same. Closes rust-lang#3313
|
Fixed in #3336 |
bors
added a commit
that referenced
this issue
Dec 2, 2016
Test for bad path overrides with summaries Bad path overrides are currently detected to issue warnings in cases where path overrides are not suitable and have exhibited buggy behavior in the past. Unfortunately though it looks like some false positives are being issued, causing unnecessary confusion about `paths` overrides. This commit fixes the detection of these "bad path overrides" by comparing `Summary` dependencies (what's written down in `Cargo.toml`) rather than comparing the `Cargo.toml` of the override with `Cargo.lock`. We're guaranteed that the package we're overridding has already been resolved into `Cargo.lock`, so we know that if the two `Cargo.toml` files are equivalent we'll continue with the same crate graph. I'm not actually entirely sure why I originally thought it'd be better to go through the `Cargo.lock` comparison route. Unfortunately that doesn't take into account optional deps which aren't in `Cargo.lock` but are in `Cargo.toml` of the override, causing the false positive. This method, however, simply ensures that the two dependency lists are the same. Closes #3313
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
If a crate has an optional dependency, and it's ever used a .cargo/config, the optional dependency is always reported as a changed dep. STR:
Results in:
(Note -- dwrite-rs is only used on Windows; for purposes of this, it should be possible to modify the Cargo.toml's in webrender to require it on all platforms... but a similar issue can likely be seen in many other crates.)
The text was updated successfully, but these errors were encountered: