search

dtolnay / trybuild

Test harness for ui tests of compiler diagnostics
Apache License 2.0
809 stars 68 forks source link

Regression: Trybuild doesn't pass on `--target`, even when needed #122

Closed hannobraun closed 3 years ago

hannobraun commented 3 years ago

We're using Trybuild in LPC8xx HAL. We have a .cargo/config.toml there that sets the target to thumbv6m-none-eabi.

We override the target to run the Trybuild tests using --target in our build script (specifically these lines). This works perfectly with Trybuild 1.0.43 (and earlier), but fails with Trybuild 1.0.44:

     Running `/home/hanno/Projects/lpc-rs/lpc8xx-hal/target/x86_64-unknown-linux-gnu/debug/deps/compiletest-a70653e23af66043`

running 1 test
    Checking lpc8xx-hal-tests v0.0.0 (/home/hanno/Projects/lpc-rs/lpc8xx-hal/target/tests/lpc8xx-hal)
error[E0463]: can't find crate for `std`
  |
  = note: the `thumbv6m-none-eabi` target may not support the standard library
  = note: `std` is required by `lpc8xx_hal_tests` because it does not declare `#![no_std]`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: could not compile `lpc8xx-hal-tests`

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

failures:

---- compile_test stdout ----
thread 'compile_test' panicked at 'tests failed', /home/hanno/.cargo/registry/src/github.com-1ecc6299db9ec823/trybuild-1.0.44/src/run.rs:51:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    compile_test

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.40s

This error indicates that --target didn't take, and that the target from .cargo/config.toml is used.

121 seems to be the culprit here. I think the code it added wrongly assumes that Trybuild doesn't need to pass --target, if the --target it receives is the same as the host. This is not true, if --target is used to override a target set in .cargo/config.toml/.cargo/config.

cc @taiki-e, @dtolnay

dtolnay commented 3 years ago

Thanks -- sorry about the breakage. I've yanked 1.0.44 until someone can send a fix.

hannobraun commented 3 years ago

No worries! Thank you for addressing this so quickly!

taiki-e commented 3 years ago

Sorry for the breakage! It seems the heuristic using RUSTFLAGS was not enough.

On lpc-rs/lpc8xx-hal, If rustflags is not set, it will succeed.

cargo +stable test +stable --features 82x,no-target-warning,trybuild --target x86_64-apple-darwin

If rustflags is set, it will fail.

RUSTFLAGS="-D warnings" cargo +stable test --features 82x,no-target-warning,trybuild --target x86_64-apple-darwin

Therefore, it seems that my understanding here was incorrect (not only CARGO_ENCODED_RUSTFLAGS, but also RUSTFLAGS seems to have the same nature):

https://github.com/dtolnay/trybuild/blob/0b33c0b7209bb8791a3923097cc6b230de4e02d3/build.rs#L28-L31

taiki-e commented 3 years ago

The best way for this seems to be https://github.com/rust-lang/cargo/pull/9532, but it may take quite a while before it is merged.

taiki-e commented 3 years ago

https://github.com/dtolnay/trybuild/pull/123 is not a perfect approach for this because it cannot detect automatically, but it is enough for #115 and #121's use cases.