don't show the full linker args unless --verbose is passed

[jyn514]

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 #119286. fixes #109979.

r? @bjorn3

try-build: i686-mingw

[rustbot]

The run-make-support library was changed

cc @jieyouxu

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

[jyn514]

hmm. i have just remembered response files exist (#109979 (comment)). maybe that would be better, so that the linker options are always still shown? and —verbose would print the contents of the response file but otherwise have no effect?

[bjorn3]

Aren't the response files deleted even after a failed link?

[jyn514]

possibly! also using a response file for some arguments but not others seems complicated lol

[jyn514]

Aren't the response files deleted even after a failed link?

yes, you are correct. they use a tempdir that's autoremoved unless you use -C save-temps.

rust/compiler/rustc_codegen_ssa/src/back/link.rs

Line 105 in fe9f272

let path = MaybeTempDir::new(tmpdir, sess.opts.cg.save_temps);

we're the compiler though, so we have access to that file before it's deleted. so --verbose could still work.

[jyn514]

using a response file for some arguments but not others seems complicated lol

oh god ok so i looked into how much work this would be and the answer is. a lot. we do not store the linker arguments in a Vec, we put them directly into a Command, and the order of the arguments is extremely important. so either we would need a separate fake Command that we pass into all the Linker implementations, or we'd need to refactor the trait so it only gives a Vec instead of a command. both of those sound annoying and i would rather not do that here, i do not think in practice it will be common to need to see the linker arguments and be unable to pass --verbose.

[bjorn3]

Looks like the following is enough to avoid object files from showing up while printing the rest. It is easy to extend this to rlibs too. I'm going to try to see if I can adapt it to get a good balance between compactness and showing useful information.

The diff

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 487ef3f88a8..45fafd2321a 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -1001,7 +1001,7 @@ fn is_illegal_instruction(_status: &ExitStatus) -> bool {
                 let err = errors::LinkingFailed {
                     linker_path: &linker_path,
                     exit_status: prog.status,
-                    command: &cmd,
+                    command: cmd,
                     escaped_output,
                     verbose: sess.opts.verbose,
                 };
diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs
index 2dfebf3e8f0..1469e0413e7 100644
--- a/compiler/rustc_codegen_ssa/src/errors.rs
+++ b/compiler/rustc_codegen_ssa/src/errors.rs
@@ -347,13 +347,13 @@ fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
 pub(crate) struct LinkingFailed<'a> {
     pub linker_path: &'a PathBuf,
     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));
@@ -363,7 +363,15 @@ fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
         if self.verbose {
             diag.note(format!("{:?}", self.command));
         } else {
-            diag.note("use `--verbose` to show all linker arguments");
+            // Omit rust object files in the error by default to make linker errors a bit less
+            // verbose.
+            let mut args = self.command.take_args();
+            args.retain(|arg| !arg.as_encoded_bytes().ends_with(b".rcgu.o"));
+            self.command.args(args);
+            self.command.arg("<some object files omitted>");
+
+            diag.note(format!("{:?}", self.command));
+            diag.note("some arguments are omitted. use `--verbose` to show all linker arguments");
         }
 
         diag.note(self.escaped_output);

[bjorn3]

The following also folds rlib arguments to omit the common paths:

error: linking with /bin/false failed: exit status: 1
|
= note: LC_ALL="C" PATH="/home/gh-bjorn3/rust/build/aarch64-unknown-linux-gnu/stage1/lib/rustlib/aarch64-unknown-linux-gnu/bin:/home/gh-bjorn3/.local/bin:/home/gh-bjorn3/.cargo/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin" VSLANG="1033" "/bin/false" "/tmp/rustcqUWvwy/symbols.o" "<2 object files omitted>" "-Wl,--as-needed" "-Wl,-Bstatic" "/home/gh-bjorn3/rust/build/aarch64-unknown-linux-gnu/stage1/lib/rustlib/aarch64-unknown-linux-gnu/lib/{libstd-c298d88188ee915f.rlib,libpanic_unwind-a10c1edfcf7c40ad.rlib,libobject-670602f4a4a0700b.rlib,libmemchr-002d5b70217fdab9.rlib,libaddr2line-b10ed1ee0775d1a7.rlib,libgimli-8ec0a4584c0db3e8.rlib,librustc_demangle-fdbaf6b5c23ed63d.rlib,libstd_detect-c886b7c11c6f80e0.rlib,libhashbrown-d682722822e656f5.rlib,librustc_std_workspace_alloc-ed5742d84e315598.rlib,libminiz_oxide-7d81ee9b8a4ad220.rlib,libadler-8dd7f69390baefd4.rlib,libunwind-90a9a4da96723235.rlib,libcfg_if-3fa0aef5e1654966.rlib,liblibc-644549e6e4fc9b45.rlib,liballoc-6c3106e0de272f45.rlib,librustc_std_workspace_core-c603386b60e50955.rlib,libcore-f28b39bac25306f7.rlib,libcompiler_builtins-2b4a04f0fe654523.rlib}" "-Wl,-Bdynamic" "-lgcc_s" "-lutil" "-lrt" "-lpthread" "-lm" "-ldl" "-lc" "-Wl,--eh-frame-hdr" "-Wl,-z,noexecstack" "-L" "/home/gh-bjorn3/rust/build/aarch64-unknown-linux-gnu/stage1/lib/rustlib/aarch64-unknown-linux-gnu/lib" "-o" "rust_out" "-Wl,--gc-sections" "-pie" "-Wl,-z,relro,-z,now" "-nodefaultlibs"
= note: some arguments are omitted. use --verbose to show all linker arguments
= note:

The diff

diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs
index 487ef3f88a8..45fafd2321a 100644
--- a/compiler/rustc_codegen_ssa/src/back/link.rs
+++ b/compiler/rustc_codegen_ssa/src/back/link.rs
@@ -1001,7 +1001,7 @@ fn is_illegal_instruction(_status: &ExitStatus) -> bool {
                 let err = errors::LinkingFailed {
                     linker_path: &linker_path,
                     exit_status: prog.status,
-                    command: &cmd,
+                    command: cmd,
                     escaped_output,
                     verbose: sess.opts.verbose,
                 };
diff --git a/compiler/rustc_codegen_ssa/src/errors.rs b/compiler/rustc_codegen_ssa/src/errors.rs
index 2dfebf3e8f0..c1f104e1010 100644
--- a/compiler/rustc_codegen_ssa/src/errors.rs
+++ b/compiler/rustc_codegen_ssa/src/errors.rs
@@ -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};
@@ -345,15 +346,15 @@ fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
 }
 
 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));
@@ -363,7 +364,61 @@ fn into_diag(self, dcx: DiagCtxtHandle<'_>, level: Level) -> Diag<'_, G> {
         if self.verbose {
             diag.note(format!("{:?}", self.command));
         } else {
-            diag.note("use `--verbose` to show all linker arguments");
+            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);

[bjorn3]

@bors r=bjorn3,jyn514

[bors]

📌 Commit 2a6a7be has been approved by bjorn3,jyn514

It is now in the queue for this repository.

[matthiaskrgr]

@bors r- #134282 (comment)
🫠

[jyn514]

oh lol it has two backslashes, not one

[jyn514]

@bors try

[bors]

@jyn514: 🔑 Insufficient privileges: not in try users

[jyn514]

:(

[Zalathar]

@bors try

[bors]

⌛ Trying commit a4ef751 with merge 6a52c82f56fb1eed4cd5cd5b008e469075f6d030...

[jieyouxu]

@bors delegate+ (try jobs)

[bors]

✌️ @jyn514, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

[bors]

☀️ Try build successful - checks-actions
Build commit: 6a52c82 (6a52c82f56fb1eed4cd5cd5b008e469075f6d030)

[jyn514]

@bors r=bjorn3,jyn514

(i only made changes to the tests, not the compiler.)

[bors]

📌 Commit a4ef751 has been approved by bjorn3,jyn514

It is now in the queue for this repository.