cargo fails if it can't find optional dependencies, even if corresponding feature not enabled #3369
Conversation
|
I have a patch written for this. I'll upload it as soon as I can test it more thoroughly, which will happen after the fix for #3368. |
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
|
The attached pull request implements this. |
|
Hm this doesn't quite feel like the right fix to me, but I'll have to think about this. Basically all the code around resolve is extremely carefully sequenced and removing assertions like In the meantime can you be sure to add a test case as well for this? |
|
Ok so thinking through this a bit. This'll definitely cause problems with path overrides in play currently, unfortunately. The assumption behind path overrides is that there's always an existing locked dependency graph, and if not weird errors will start getting emitted. I believe we can fix this though by just not adding overrides during a Other than that though I think this should work, and I definitely think it's worthwhile to fix! |
|
☔ The latest upstream changes (presumably #3221) made this pull request unmergeable. Please resolve the merge conflicts. |
joshtriplett
force-pushed
the
cargo-install-only-required-dependencies
branch
from
d393977
to
208f6b4
Compare
|
@alexcrichton Just updated this to latest Cargo, and expanded it to avoid adding overrides as you suggested. Much easier in current Cargo with the changes to the resolve API; I kept it self-contained in the resolver without leaking out into I also added three new tests: one for the simple case of |
joshtriplett
force-pushed
the
cargo-install-only-required-dependencies
branch
from
208f6b4
to
7016b84
Compare
|
Fixed matches of paths in output to pass on Windows. |
|
I believe that the |
|
@alexcrichton Even if you had the optional dependencies, AFAICT However, if you consider that a problem, I can probably find a way to distinguish these cases. (It might involve passing down some new parameter in the workspace.) |
joshtriplett
changed the title
|
@alexcrichton The additional patch I just pushed distinguishes between "cargo install" and "cargo package", and only allows missing optional dependencies for "cargo install". If that looks reasonable to you, I can squash it in. |
|
@joshtriplett yeah that looks great to me, thanks! Want to squash it up? |
…endencies When building with a directory registry that contains only the subset of crates required to build an application crate, cargo fails if that subset doesn't include optional dependencies pulled in for every possible feature of the root crate, even when the install doesn't enable those features. This prevents Linux distributions from building with a minimal set of dependencies (omitting, for instance, packages for unstable/nightly features). Introduce a new workspace flag "require_optional_deps", disabled for install and enabled for everything else. Skip the initial Method::Everything resolve in this case, and modify resolve_with_previous to support running a Method::Required resolve without a previous resolution. This also skips adding path overrides, as those won't make sense (and won't work) for an install directly from a registry. Introduce a set of tests for "cargo install" directly from a directory registry.
joshtriplett
force-pushed
the
cargo-install-only-required-dependencies
branch
from
9bb8f84
to
db71d87
Compare
|
@alexcrichton Done. |
|
@bors: r+ |
|
📌 Commit db71d87 has been approved by |
…encies, r=alexcrichton cargo fails if it can't find optional dependencies, even if corresponding feature not enabled I have a directory registry containing all the crate sources needed to build an application crate (for instance, ripgrep), and a `$CARGO_HOME/config` file that looks like this: ```toml [source.crates-io] replace-with = "dh-cargo-registry" [source.dh-cargo-registry] directory = "/usr/share/cargo/registry/" ``` When I attempt to build ripgrep via "cargo install ripgrep" from that directory registry, I get this error: ``` error: failed to compile `ripgrep v0.3.1`, intermediate artifacts can be found at `/tmp/cargo-install.rmKApOw9BwAL` Caused by: no matching package named `simd` found (required by `bytecount`) location searched: registry https://github.com/rust-lang/crates.io-index version required: ^0.1.1 ``` The directory registry indeed does not contain "simd"; however, bytecount doesn't require simd. It has an optional dependency on simd, and nothing enables the feature that requires that dependency. Placing the simd crate sources into the directory registry allows ripgrep to build; the resulting build does not actually build the simd crate. I can reproduce this by just trying to build the "bytecount" crate directly, using the same `$CARGO_HOME`: ``` error: no matching package named `simd` found (required by `bytecount`) location searched: registry https://github.com/rust-lang/crates.io-index version required: = 0.1.1 ``` (Incidentally, that "version required" seems wrong: bytecount has an optional dependency on simd `^0.1.1`, not `=0.1.1`.) However, this doesn't seem consistent with other crates in the same dependency tree. For instance, ripgrep also depends on clap, and clap has an optional dependency on yaml-rust, yet cargo does not complain about the missing yaml-rust. I'd *guess* that the difference occurs because ripgrep has an optional feature `simd-accel` that depends on `bytecount/simd-accel`, so cargo wants to compute what packages it needs for that case too, even when building without that feature. (Similar to #3233.) However, this makes it impossible to build a package while installing only the packaged dependencies for the enabled features. Could `cargo install` ignore any dependencies not actually required by the enabled feature? (That behavior would make no sense for "cargo build", which builds a Cargo.lock file that should remain consistent regardless of enabled features, but it makes sense for "cargo install cratename", which doesn't build a Cargo.lock file.)
|
☀️ Test successful - status-appveyor, status-travis |
|
Thanks! |
I have a directory registry containing all the crate sources needed to build an application crate (for instance, ripgrep), and a
$CARGO_HOME/configfile that looks like this:When I attempt to build ripgrep via "cargo install ripgrep" from that directory registry, I get this error:
The directory registry indeed does not contain "simd"; however, bytecount doesn't require simd. It has an optional dependency on simd, and nothing enables the feature that requires that dependency.
Placing the simd crate sources into the directory registry allows ripgrep to build; the resulting build does not actually build the simd crate.
I can reproduce this by just trying to build the "bytecount" crate directly, using the same
$CARGO_HOME:(Incidentally, that "version required" seems wrong: bytecount has an optional dependency on simd
^0.1.1, not=0.1.1.)However, this doesn't seem consistent with other crates in the same dependency tree. For instance, ripgrep also depends on clap, and clap has an optional dependency on yaml-rust, yet cargo does not complain about the missing yaml-rust.
I'd guess that the difference occurs because ripgrep has an optional feature
simd-accelthat depends onbytecount/simd-accel, so cargo wants to compute what packages it needs for that case too, even when building without that feature. (Similar to #3233.)However, this makes it impossible to build a package while installing only the packaged dependencies for the enabled features. Could
cargo installignore any dependencies not actually required by the enabled feature? (That behavior would make no sense for "cargo build", which builds a Cargo.lock file that should remain consistent regardless of enabled features, but it makes sense for "cargo install cratename", which doesn't build a Cargo.lock file.)