Skip to content

Commit

Permalink
Auto merge of rust-lang#133633 - jyn514:hide-linker-args, r=<try>
Browse files Browse the repository at this point in the history
don't show the full linker args unless `--verbose` is passed

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.

split out from rust-lang#119286. fixes rust-lang#109979.

r? `@bjorn3`

try-build: i686-mingw
  • Loading branch information
bors committed Dec 15, 2024
2 parents b57d93d + a4ef751 commit 6a52c82
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 14 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -986,12 +986,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,
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
70 changes: 66 additions & 4 deletions compiler/rustc_codegen_ssa/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Errors emitted by codegen_ssa
use std::borrow::Cow;
use std::ffi::OsString;
use std::io::Error;
use std::num::ParseIntError;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -345,21 +346,82 @@ impl<G: EmissionGuarantee> Diagnostic<'_, G> for ThorinErrorWrapper {
}

pub(crate) struct LinkingFailed<'a> {
pub linker_path: &'a PathBuf,
pub linker_path: &'a Path,
pub exit_status: ExitStatus,
pub command: &'a Command,
pub command: Command,
pub escaped_output: String,
pub verbose: bool,
}

impl<G: EmissionGuarantee> Diagnostic<'_, G> for LinkingFailed<'_> {
fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
fn into_diag(mut self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
let mut diag = Diag::new(dcx, level, fluent::codegen_ssa_linking_failed);
diag.arg("linker_path", format!("{}", self.linker_path.display()));
diag.arg("exit_status", format!("{}", self.exit_status));

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 {
enum ArgGroup {
Regular(OsString),
Objects(usize),
Rlibs(PathBuf, Vec<OsString>),
}

// Omit rust object files and fold rlibs in the error by default to make linker errors a
// bit less verbose.
let orig_args = self.command.take_args();
let mut args: Vec<ArgGroup> = vec![];
for arg in orig_args {
if arg.as_encoded_bytes().ends_with(b".rcgu.o") {
if let Some(ArgGroup::Objects(n)) = args.last_mut() {
*n += 1;
} else {
args.push(ArgGroup::Objects(1));
}
} else if arg.as_encoded_bytes().ends_with(b".rlib") {
let rlib_path = Path::new(&arg);
let dir = rlib_path.parent().unwrap();
let filename = rlib_path.file_name().unwrap().to_owned();
if let Some(ArgGroup::Rlibs(parent, rlibs)) = args.last_mut() {
if parent == dir {
rlibs.push(filename);
} else {
args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename]));
}
} else {
args.push(ArgGroup::Rlibs(dir.to_owned(), vec![filename]));
}
} else {
args.push(ArgGroup::Regular(arg));
}
}
self.command.args(args.into_iter().map(|arg_group| match arg_group {
ArgGroup::Regular(arg) => arg,
ArgGroup::Objects(n) => OsString::from(format!("<{n} object files omitted>")),
ArgGroup::Rlibs(dir, rlibs) => {
let mut arg = dir.into_os_string();
arg.push("/{");
let mut first = true;
for rlib in rlibs {
if !first {
arg.push(",");
}
first = false;
arg.push(rlib);
}
arg.push("}");
arg
}
}));

diag.note(format!("{:?}", self.command));
diag.note("some arguments are omitted. 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 @@ -325,6 +325,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
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);
}
_ => (),
}
}
}
1 change: 1 addition & 0 deletions tests/run-make/linker-warning/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}
28 changes: 28 additions & 0 deletions tests/run-make/linker-warning/rmake.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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();

// 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")
.assert_stderr_not_contains("object files omitted")
.assert_stderr_contains_regex(r"lib(/|\\\\)libstd");
run_rustc()
.link_arg("run_make_error")
.run_fail()
.assert_stderr_contains("fake-linker")
.assert_stderr_contains("object files omitted")
.assert_stderr_contains_regex(r"\{")
.assert_stderr_not_contains_regex(r"lib(/|\\\\)libstd");
}

0 comments on commit 6a52c82

Please sign in to comment.