Skip to content

Commit

Permalink
don't show the full linker args unless --verbose is passed
Browse files Browse the repository at this point in the history
the linker arguments can be *very* long, especially for crates with many dependencies. often they are not useful. omit them unless the user specifically requests them.
  • Loading branch information
jyn514 committed Sep 9, 2024
1 parent b58c2fb commit 6b77d59
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 31 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use rustc_data_structures::temp_dir::MaybeTempDir;
use rustc_errors::{DiagCtxtHandle, ErrorGuaranteed, FatalError};
use rustc_fs_util::{fix_windows_verbatim_for_gcc, try_canonicalize};
use rustc_hir::def_id::{CrateNum, LOCAL_CRATE};
use rustc_macros::Diagnostic;
use rustc_metadata::fs::{copy_to_stdout, emit_wrapper_file, METADATA_FILENAME};
use rustc_metadata::{find_native_static_library, walk_native_lib_search_dirs};
use rustc_middle::bug;
Expand Down Expand Up @@ -984,12 +985,12 @@ fn link_natively(
let mut output = prog.stderr.clone();
output.extend_from_slice(&prog.stdout);
let escaped_output = escape_linker_output(&output, flavor);
// FIXME: Add UI tests for this error.
let err = errors::LinkingFailed {
linker_path: &linker_path,
exit_status: prog.status,
command: &cmd,
escaped_output,
verbose: sess.opts.verbose,
};
sess.dcx().emit_err(err);
// If MSVC's `link.exe` was expected but the return code
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ pub struct LinkingFailed<'a> {
pub exit_status: ExitStatus,
pub command: &'a Command,
pub escaped_output: String,
pub verbose: bool,
}

impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
Expand All @@ -358,7 +359,13 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {

let contains_undefined_ref = self.escaped_output.contains("undefined reference to");

diag.note(format!("{:?}", self.command)).note(self.escaped_output);
if self.verbose {
diag.note(format!("{:?}", self.command));
} else {
diag.note("use `--verbose` to show all linker arguments");
}

diag.note(self.escaped_output);

// Trying to match an error from OS linkers
// which by now we have no way to translate.
Expand Down
6 changes: 6 additions & 0 deletions src/tools/run-make-support/src/external_deps/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,12 @@ impl Rustc {
self
}

/// Pass the `--verbose` flag.
pub fn verbose(&mut self) -> &mut Self {
self.cmd.arg("--verbose");
self
}

/// `EXTRARSCXXFLAGS`
pub fn extra_rs_cxx_flags(&mut self) -> &mut Self {
// Adapted from tools.mk (trimmed):
Expand Down
6 changes: 4 additions & 2 deletions tests/run-make/link-args-order/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,17 @@ fn main() {
.link_args("b c")
.link_args("d e")
.link_arg("f")
.arg("--print=link-args")
.run_fail()
.assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
.assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#);
rustc()
.input("empty.rs")
.linker_flavor(linker)
.arg("-Zpre-link-arg=a")
.arg("-Zpre-link-args=b c")
.arg("-Zpre-link-args=d e")
.arg("-Zpre-link-arg=f")
.arg("--print=link-args")
.run_fail()
.assert_stderr_contains(r#""a" "b" "c" "d" "e" "f""#);
.assert_stdout_contains(r#""a" "b" "c" "d" "e" "f""#);
}
12 changes: 6 additions & 6 deletions tests/run-make/link-dedup/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@ fn main() {
rustc().input("depb.rs").run();
rustc().input("depc.rs").run();

let output = rustc().input("empty.rs").cfg("bar").run_fail();
output.assert_stderr_contains(needle_from_libs(&["testa", "testb", "testa"]));
let output = rustc().input("empty.rs").cfg("bar").arg("--print=link-args").run_fail();
output.assert_stdout_contains(needle_from_libs(&["testa", "testb", "testa"]));

let output = rustc().input("empty.rs").run_fail();
output.assert_stderr_contains(needle_from_libs(&["testa"]));
output.assert_stderr_not_contains(needle_from_libs(&["testb"]));
output.assert_stderr_not_contains(needle_from_libs(&["testa", "testa", "testa"]));
let output = rustc().input("empty.rs").arg("--print=link-args").run_fail();
output.assert_stdout_contains(needle_from_libs(&["testa"]));
output.assert_stdout_not_contains(needle_from_libs(&["testb"]));
output.assert_stdout_not_contains(needle_from_libs(&["testa", "testa", "testa"]));
// Adjacent identical native libraries are no longer deduplicated if
// they come from different crates (https://github.com/rust-lang/rust/pull/103311)
// so the following will fail:
Expand Down
17 changes: 0 additions & 17 deletions tests/run-make/linker-warning/Makefile

This file was deleted.

13 changes: 13 additions & 0 deletions tests/run-make/linker-warning/fake-linker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn main() {
for arg in std::env::args() {
match &*arg {
"run_make_info" => println!("foo"),
"run_make_warn" => eprintln!("warning: bar"),
"run_make_error" => {
eprintln!("error: baz");
std::process::exit(1);
}
_ => (),
}
}
}
45 changes: 45 additions & 0 deletions tests/run-make/linker-warning/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
use std::path::Path;

use run_make_support::rfs::remove_file;
use run_make_support::{rustc, Rustc};

fn run_rustc() -> Rustc {
let mut rustc = rustc();
rustc.arg("main.rs").output("main").linker("./fake-linker");
rustc
}

fn main() {
// first, compile our linker
rustc().arg("fake-linker.rs").output("fake-linker").run();

// Run rustc with our fake linker, and make sure it shows warnings
let warnings = run_rustc().link_arg("run_make_warn").run();
warnings.assert_stderr_contains("warning: linker stderr: bar");

// Make sure it shows stdout, but only when --verbose is passed
run_rustc()
.link_arg("run_make_info")
.verbose()
.run()
.assert_stderr_contains("warning: linker stdout: foo");
run_rustc()
.link_arg("run_make_info")
.run()
.assert_stderr_not_contains("warning: linker stdout: foo");

// Make sure we short-circuit this new path if the linker exits with an error
// (so the diagnostic is less verbose)
run_rustc().link_arg("run_make_error").run_fail().assert_stderr_contains("note: error: baz");

// Make sure we don't show the linker args unless `--verbose` is passed
run_rustc()
.link_arg("run_make_error")
.verbose()
.run_fail()
.assert_stderr_contains_regex("fake-linker.*run_make_error");
run_rustc()
.link_arg("run_make_error")
.run_fail()
.assert_stderr_not_contains_regex("fake-linker.*run_make_error");
}
9 changes: 5 additions & 4 deletions tests/run-make/rust-lld/rmake.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ fn main() {
// Opt-in to lld and the self-contained linker, to link with rust-lld. We'll check that by
// asking the linker to display its version number with a link-arg.
let output = rustc()
.env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info")
.arg("-Zlinker-features=+lld")
.arg("-Clink-self-contained=+linker")
.arg("-Zunstable-options")
.arg("--verbose")
.link_arg(linker_version_flag)
.input("main.rs")
.run();
Expand All @@ -29,8 +29,8 @@ fn main() {

// It should not be used when we explicitly opt-out of lld.
let output = rustc()
.env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info")
.link_arg(linker_version_flag)
.arg("--verbose")
.arg("-Zlinker-features=-lld")
.input("main.rs")
.run();
Expand All @@ -43,8 +43,8 @@ fn main() {
// While we're here, also check that the last linker feature flag "wins" when passed multiple
// times to rustc.
let output = rustc()
.env("RUSTC_LOG", "rustc_codegen_ssa::back::link=info")
.link_arg(linker_version_flag)
.arg("--verbose")
.arg("-Clink-self-contained=+linker")
.arg("-Zunstable-options")
.arg("-Zlinker-features=-lld")
Expand All @@ -60,6 +60,7 @@ fn main() {
}

fn find_lld_version_in_logs(stderr: String) -> bool {
let lld_version_re = Regex::new(r"^LLD [0-9]+\.[0-9]+\.[0-9]+").unwrap();
let lld_version_re =
Regex::new(r"^warning: linker stdout: LLD [0-9]+\.[0-9]+\.[0-9]+").unwrap();
stderr.lines().any(|line| lld_version_re.is_match(line.trim()))
}

0 comments on commit 6b77d59

Please sign in to comment.