Skip to content

Commit

Permalink
fix: emit warnings as warnings when learning rust target info (#15036)
Browse files Browse the repository at this point in the history
### What does this PR try to resolve?

This is a horrible hack,
which lets the rustc invocation for learning target info always emit
warnings as warnings.
But at least it unblocks people who pass `-Awarnings` via RUSTFLAGS.

A long-term solution is a better interface
between build systems and the compiler.
See the discussion on Zulip:

https://rust-lang.zulipchat.com/#narrow/channel/131828-t-compiler/topic/Improving.20communication.20interface.20between.20cargo.2Frustc

Fixes #8010

### How should we test and review this PR?

Ensure `CFG_DISABLE_CROSS_TESTS` is not set,
and run `cargo t --test testsuite
always_emit_warnings_as_warnings_when_learning_target_info`

This also additionally adds `wasm32-unknown-unknown` target to Cargo's
CI.

### Additional information
  • Loading branch information
weihanglo authored Jan 8, 2025
2 parents c3a2b1a + 5e3558c commit a4c0d39
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 23 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ jobs:
- run: rustup update --no-self-update stable
- run: rustup update --no-self-update ${{ matrix.rust }} && rustup default ${{ matrix.rust }}
- run: rustup target add ${{ matrix.other }}
- run: rustup target add wasm32-unknown-unknown
- run: rustup target add aarch64-unknown-none # need this for build-std mock tests
if: startsWith(matrix.rust, 'nightly')
- run: rustup component add rustc-dev llvm-tools-preview rust-docs
Expand Down Expand Up @@ -225,6 +226,7 @@ jobs:
- uses: actions/checkout@v4
- run: rustup update --no-self-update stable && rustup default stable
- run: rustup target add i686-unknown-linux-gnu
- run: rustup target add wasm32-unknown-unknown
- run: sudo apt update -y && sudo apt install gcc-multilib libsecret-1-0 libsecret-1-dev -y
- run: rustup component add rustfmt || echo "rustfmt not available"
- run: cargo test -p cargo
Expand Down
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ cargo-credential-macos-keychain = { version = "0.4.7", path = "credential/cargo-
cargo-credential-wincred = { version = "0.4.7", path = "credential/cargo-credential-wincred" }
cargo-platform = { path = "crates/cargo-platform", version = "0.2.0" }
cargo-test-macro = { version = "0.4.0", path = "crates/cargo-test-macro" }
cargo-test-support = { version = "0.7.0", path = "crates/cargo-test-support" }
cargo-test-support = { version = "0.7.1", path = "crates/cargo-test-support" }
cargo-util = { version = "0.2.14", path = "crates/cargo-util" }
cargo-util-schemas = { version = "0.7.3", path = "crates/cargo-util-schemas" }
cargo_metadata = "0.19.0"
Expand Down
2 changes: 1 addition & 1 deletion crates/cargo-test-support/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "cargo-test-support"
version = "0.7.0"
version = "0.7.1"
edition.workspace = true
rust-version = "1.83" # MSRV:1
license.workspace = true
Expand Down
30 changes: 30 additions & 0 deletions crates/cargo-test-support/src/cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,33 @@ pub fn can_run_on_host() -> bool {
return true;
}
}

/// Check if the given target has been installed.
///
/// Generally [`disabled`] should be used to check if cross-compilation is allowed.
/// And [`alternate`] to get the cross target.
///
/// You should only use this as a last resort to skip tests,
/// because it doesn't report skipped tests as ignored.
pub fn requires_target_installed(target: &str) -> bool {
let has_target = std::process::Command::new("rustup")
.args(["target", "list", "--installed"])
.output()
.ok()
.map(|output| {
String::from_utf8(output.stdout)
.map(|stdout| stdout.contains(target))
.unwrap_or_default()
})
.unwrap_or_default();
if !has_target {
let msg =
format!("to run this test, run `rustup target add {target} --toolchain <toolchain>`",);
if cargo_util::is_ci() {
panic!("{msg}");
} else {
eprintln!("{msg}");
}
}
has_target
}
4 changes: 4 additions & 0 deletions src/cargo/core/compiler/build_context/target_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,10 @@ impl TargetInfo {
process.arg("--print=crate-name"); // `___` as a delimiter.
process.arg("--print=cfg");

// parse_crate_type() relies on "unsupported/unknown crate type" error message,
// so make warnings always emitted as warnings.
process.arg("-Wwarnings");

let (output, error) = rustc
.cached_output(&process, extra_fingerprint)
.with_context(|| {
Expand Down
35 changes: 35 additions & 0 deletions tests/testsuite/cross_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1260,3 +1260,38 @@ fn doctest_xcompile_linker() {
"#]])
.run();
}

#[cargo_test]
fn always_emit_warnings_as_warnings_when_learning_target_info() {
if cross_compile::disabled() {
return;
}

let target = "wasm32-unknown-unknown";
if !cross_compile::requires_target_installed(target) {
return;
}

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
edition = "2015"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("build -v --target")
.env("RUSTFLAGS", "-Awarnings")
.arg(target)
.with_stderr_data(str![[r#"
[COMPILING] foo v0.0.0 ([ROOT]/foo)
[RUNNING] `rustc --crate-name foo [..]-Awarnings[..]`
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.run();
}
27 changes: 7 additions & 20 deletions tests/testsuite/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use std::path::{Path, PathBuf};

use cargo_test_support::cross_compile;
use cargo_test_support::prelude::*;
use cargo_test_support::registry::{Dependency, Package};
use cargo_test_support::ProjectBuilder;
Expand Down Expand Up @@ -391,24 +392,8 @@ fn check_core() {

#[cargo_test(build_std_mock)]
fn build_std_with_no_arg_for_core_only_target() {
let has_rustup_aarch64_unknown_none = std::process::Command::new("rustup")
.args(["target", "list", "--installed"])
.output()
.ok()
.map(|output| {
String::from_utf8(output.stdout)
.map(|stdout| stdout.contains("aarch64-unknown-none"))
.unwrap_or_default()
})
.unwrap_or_default();
if !has_rustup_aarch64_unknown_none {
let msg =
"to run this test, run `rustup target add aarch64-unknown-none --toolchain nightly`";
if cargo_util::is_ci() {
panic!("{msg}");
} else {
eprintln!("{msg}");
}
let target = "aarch64-unknown-none";
if !cross_compile::requires_target_installed(target) {
return;
}

Expand All @@ -427,7 +412,8 @@ fn build_std_with_no_arg_for_core_only_target() {
.build();

p.cargo("build -v")
.arg("--target=aarch64-unknown-none")
.arg("--target")
.arg(target)
.build_std(&setup)
.with_stderr_data(
str![[r#"
Expand Down Expand Up @@ -457,7 +443,8 @@ fn build_std_with_no_arg_for_core_only_target() {
// Note that we don't download std dependencies for the second call
// because `-Zbuild-std` downloads them all also when building for core only.
p.cargo("build -v")
.arg("--target=aarch64-unknown-none")
.arg("--target")
.arg(target)
.target_host()
.build_std(&setup)
.with_stderr_data(
Expand Down

0 comments on commit a4c0d39

Please sign in to comment.