-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 panic in ResolvedFeatures::activated_features_int
with unstable bindeps
feature
#12358
Comments
Thanks for the report! Does it share the same root cause with #10593? Looks a bit similar. |
Definitely looks similar... One weird thing I'm also noticing: If we make [package]
name = "build-only-dep"
version = "0.1.0"
edition = "2021"
# [target.'cfg(unix)'.dependencies] # << turn this off
[dependencies] # << make this a normal dependency
some-leaf-dep = { path = "../some-leaf-dep" } Then something like $ cargo check
Compiling some-leaf-dep v0.1.0
Checking build-only-dep v0.1.0
Compiling dep-that-depends-on-build-only-dep v0.1.0
Checking will-get-bindeped v0.1.0
Compiling top-level-that-depends-on-bindep v0.1.0
Finished dev [unoptimized + debuginfo] target(s) in 0.48s $ cargo tree
thread 'main' panicked at 'activated_features for invalid package: features did not find PackageId { name: "will-get-bindeped", version: "0.1.0", source: "/Users/phlip9/dev/cargo-bindep-panic/will-get-bindeped" } HostDep', src/cargo/core/resolver/features.rs:322:14 |
The feature resolution path is slightly different between |
In case its helpful for anyone, I turned this excellent reproduction into two integration tests: Expand...The first test reproduces the #[cargo_test]
fn resolver2_bindep_crosscompile() {
if cross_compile::disabled() {
return;
}
// At least on linux the i686-unknown-linux-gnu target triggered a panic.
let target = cross_compile::alternate();
let p = project()
.file(
"Cargo.toml",
&r#"
[package]
name = "foo"
version = "0.0.0"
authors = []
resolver = "2"
[dependencies]
bindep = { path = "bindep", artifact = "bin", target = "$TARGET" }
"#
.replace("$TARGET", target),
)
.file("src/lib.rs", "")
.file("bindep/Cargo.toml", &basic_manifest("bindep", "0.0.0"))
.file("bindep/src/main.rs", "fn main() {}")
.build();
p.cargo("check -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_contains(
r#"[COMPILING] bindep v0.0.0 ([CWD]/bindep)
[CHECKING] foo v0.0.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]"#,
)
.with_status(0)
.run();
p.cargo("tree -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stdout(
"\
foo v0.0.0 ([CWD])
└── bindep v0.0.0 ([CWD]/bindep)",
)
.with_status(0)
.run();
}
#[cargo_test]
fn check_bindep_depending_on_build_only_dep_with_optional_dep() {
if cross_compile::disabled() {
return;
}
// i686-unknown-linux-gnu did not trigger a panic so we need to manually specify wasm32-unknown-unknown instead of relying on cross_compile::alternate()
// This is possibly because the `cfg(unix)` later on needs to evaluate to false on the cross compile but true on the host.
let target = "wasm32-unknown-unknown";
let p = project()
.file(
"Cargo.toml",
&r#"
[package]
name = "foo"
version = "0.0.0"
authors = []
resolver = "2"
[dependencies]
bindep = { path = "bindep", artifact = "bin", target = "$TARGET" }
"#
.replace("$TARGET", target),
)
.file("src/lib.rs", "")
.file(
"bindep/Cargo.toml",
&r#"
[package]
name = "bindep"
version = "0.0.0"
authors = []
[dependencies]
depends-on-build-only-dep = { path = "../depends-on-build-only-dep" }
"#,
)
.file("bindep/src/main.rs", "fn main() {}")
.file(
"depends-on-build-only-dep/Cargo.toml",
&r#"
[package]
name = "depends-on-build-only-dep"
version = "0.0.0"
authors = []
[build-dependencies]
build-only-dep = { path = "../build-only-dep" }
"#,
)
.file("depends-on-build-only-dep/src/lib.rs", "")
.file("depends-on-build-only-dep/build.rs", "fn main() {}")
.file(
"build-only-dep/Cargo.toml",
&r#"
[package]
name = "build-only-dep"
version = "0.0.0"
authors = []
[target.'cfg(unix)'.dependencies]
some-leaf-dep = { path = "../some-leaf-dep" }
"#,
)
.file("build-only-dep/src/lib.rs", "")
.file(
"some-leaf-dep/Cargo.toml",
&basic_manifest("some-leaf-dep", "0.0.1"),
)
.file("some-leaf-dep/src/lib.rs", "")
.build();
p.cargo("check -Z bindeps")
.masquerade_as_nightly_cargo(&["bindeps"])
.with_stderr_contains(
r#"[COMPILING] bindep v0.0.0 ([CWD]/bindep)
[CHECKING] foo v0.0.0 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]"#,
)
.with_status(0)
.run();
} |
Managed to trigger this in rust-analyzer by accident as well (without any special targets set afaik), so if it helps, just putting a reproducer here as well: https://github.com/Veykril/rust-analyzer/tree/cargo-panic
|
I have bumped into this problem too, trying to compile a
|
The `mnemos-x86_64-bootloader` image is built by a separate crate `mnemos-x86_64-bootloader`, which includes a build script that runs the post-build workflow to take the `bootloader_api`-compatible kernel binary and turn it into a bootable disk image that links with the actual bootloader. At present, we use a Cargo artifact dependency to build the actual kernel and expose it to the bootloader image build script. This is the ideal solution for this, since it allows the dependency between the build script and the kernel binary to be expressed as a normal Cargo dependency. However, using an artifact dep on a binary that's built for a different target than the crate with the artifact dep is sadly broken: see rust-lang/cargo#12358. We've somehow managed to find a way to avoid this issue in the past, but updating to a recent nightly seems to have brought it back (see #322). Therefore, this PR changes the build script to instead shell out to a new `cargo` invocation that builds the kernel. This is a bit unfortunate, since it means that the build output from building the actual kernel isn't exposed to the user as nicely, and there's probably less build caching. However, it works for now. I've modified the `just build-x86` Just recipe to invoke `cargo check` for the actual x86_64 kernel binary before building the bootable image, to help ensure that the cargo build output is made visible to the user. A nicer long-term solution would be to switch from a build script to using a cargo `xtask`-like builder, that's invoked as a cargo `runner` for that target, with the path to the builder binary passed in on the CLI. This is what [mycelium does][1]. That approach is nicer because it allows the kernel to be built using a normal cargo invocation that's actually visible to the user, instead of secretly in a build script. [1]: https://github.com/hawkw/mycelium/blob/e51eb8aa98e7609490fa674f408db32fd51caa70/.cargo/config.toml#L1-L2
The `mnemos-x86_64-bootloader` image is built by a separate crate `mnemos-x86_64-bootloader`, which includes a build script that runs the post-build workflow to take the `bootloader_api`-compatible kernel binary and turn it into a bootable disk image that links with the actual bootloader. At present, we use a Cargo artifact dependency to build the actual kernel and expose it to the bootloader image build script. This is the ideal solution for this, since it allows the dependency between the build script and the kernel binary to be expressed as a normal Cargo dependency. However, using an artifact dep on a binary that's built for a different target than the crate with the artifact dep is sadly broken: see rust-lang/cargo#12358. We've somehow managed to find a way to avoid this issue in the past, but updating to a recent nightly seems to have brought it back (see #322). Therefore, this PR changes the build script to instead shell out to a new `cargo` invocation that builds the kernel. This is a bit unfortunate, since it means that the build output from building the actual kernel isn't exposed to the user as nicely, and there's probably less build caching. However, it works for now. I've modified the `just build-x86` Just recipe to invoke `cargo check` for the actual x86_64 kernel binary before building the bootable image, to help ensure that the cargo build output is made visible to the user. A nicer long-term solution would be to switch from a build script to using a cargo `xtask`-like builder, that's invoked as a cargo `runner` for that target, with the path to the builder binary passed in on the CLI. This is what [mycelium does][1]. That approach is nicer because it allows the kernel to be built using a normal cargo invocation that's actually visible to the user, instead of secretly in a build script. What I've written here is just a minimum viable solution, and I'd like to change it to use the cargo `runner` approach later. [1]: https://github.com/hawkw/mycelium/blob/e51eb8aa98e7609490fa674f408db32fd51caa70/.cargo/config.toml#L1-L2
…weihanglo Fix panic when running cargo tree on a package with a cross compiled bindep ### What does this PR try to resolve? This is an attempt to close out `@rukai's` [PR](#13207 (comment)) for #12358 and #10593 by adjusting the new integration test and resolving merge conflicts. I have also separated the changes into atomic commits as per [previous review](#13207 (comment)). ### How should we test and review this PR? The integration test that has been edited here is sufficient, plus the new integration test that confirms a more specific case where `cargo tree` throws an error. ### Additional information I have confirmed the test `artifact_dep_target_specified` fails on master branch and succeeds on this branch. The first commit fixes the panic and the integration test. Commits 2 and 3 add other tests that confirm behaviour mentioned in related issues. Commits: 1. [Fix panic when running cargo tree on a package with a cross compiled bindep](5c5ea78) - fixes some panics and changes the integration test to succeed 2. [test: cargo tree panic on artifact dep target deactivated](ed294ab) - adds test to confirm the behaviour for the specific panic from [#10539 (comment)](#10593 (comment))
Update on this: Since rustup nightly release cargo tree output:
And if |
Problem
While experimenting with using the unstable
bindeps
feature, I stumbled upon a panic. Here's a repo with a minimized failing case: https://github.com/phlip9/cargo-bindep-panic. Simply clone it and run a cargo command likecargo build
orcargo tree
to see the panic.In plain english, we have a crate,
will-get-bindeped
, that's targetting some other arch likewasm
orx86_64-fortanix-unknown-sgx
in our case. At the root is a crate,top-level-that-depends-on-bindep
, that wants to depend on the built binary ofwill-get-bindeped
(hence thebindeps
feature).This binary,
will-get-bindeped
, depends ondep-that-depends-on-build-only-dep
that has a build-time dependency. Thisbuild-only-dep
represents something likeprotoc-rust
that needs to run on the host and compile some .proto's or w/e.build-only-dep
has acfg(unix)
(this seems important!) onsome-leaf-dep
. Ifsome-leaf-dep
is just an unconditional dependency or even acfg(windows)
, the panic no longer triggers.Visually, the crate graph looks like:
Steps
Other cargo commands also fail immediately:
Possible Solution(s)
No response
Notes
No response
Version
The text was updated successfully, but these errors were encountered: