rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
96.99k stars 12.54k forks source link

rustc --test should not check linking when using --emit=metadata or -Z no-trans #39948

Open Wilfred opened 7 years ago

Wilfred commented 7 years ago

Suppose I have a library configured with panic = "abort":

$ cargo init --lib foo
$ cd foo
$ nano Cargo.toml
$ more Cargo.toml
[package]
name = "foo"
version = "0.1.0"
authors = ["Wilfred Hughes <me@wilfred.me.uk>"]

[dependencies]

[profile.dev]
panic = "abort"

I'm unable to build with with --test:

$ cargo rustc -- --test
   Compiling foo v0.1.0 (file:///home/wilfred/tmp/foo)
error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort`

error: aborting due to previous error

error: Could not compile `foo`.

To learn more, run the command again with --verbose.

As discussed in https://github.com/rust-lang/cargo/issues/3166#issuecomment-252120541 this is because the test harness needs unwinding on panic.

However, I get the same error with -Z no-trans:

$ cargo rustc -- -Z no-trans --test 
   Compiling foo v0.1.0 (file:///home/wilfred/tmp/foo)
error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort`

error: aborting due to previous error

error: Could not compile `foo`.

To learn more, run the command again with --verbose.

This causes problems in editor integration like https://github.com/flycheck/flycheck-rust/issues/47 . We want to pass --test as otherwise we don't get syntax checking on test files. However, we get the above error for projects with abort-on-panic.

Could we teach --test to ignore the runtime when we pass -Z no-trans?

steveklabnik commented 7 years ago

-Z no-trans is on its way out, now that we have the meatdata-only crate type, right?

nagisa commented 7 years ago

Right; -Z no-trans will likely become an unusable actually unstable flag even before that.

Wilfred commented 7 years ago

Aha, great to see this stabilising. My understanding from #38666 is that --crate-type=metadata has had a short life (and had issues distinguishing between libraries and applications) and the correct version is now --emit=metadata.

We'll certainly move to --emit=metadata once it lands on stable Rust.

Note that the same problem exists with --emit=metadata though:

$ rustc --version
rustc 1.17.0-nightly (306035c21 2017-02-18)
$ cargo rustc -- --emit=metadata --test
   Compiling abortexample v0.1.0 (file:///home/wilfred/tmp/abortexample)
error: the linked panic runtime `panic_unwind` is not compiled with this crate's panic strategy `abort`

error: aborting due to previous error

error: Could not compile `abortexample`.

To learn more, run the command again with --verbose.
Mark-Simulacrum commented 7 years ago

As far as I can tell this works today, so I'm going to close. rustc --emit=metadata -Cpanic=abort --test test.rs works on:

#[test]
fn t() {
    panic!("oh no!");
}
Wilfred commented 7 years ago

@Mark-Simulacrum I don't think your test is equivalent. The example from the first comment still fails on current Rust. Could you please reopen?

ehuss commented 4 years ago

The intended way to do this with cargo is to use cargo check --profile=test. That forces panic to be unwind in order to be compatible with libtest.

Additionally, once #65710 lands, there is a new flag cargo check --profile=test -Zpanic-abort-tests where it will successfully build with abort.