linker: fail the compilation if there were linker errors

[kcbanner]

This fixes #13432 and #13235

There was no check for linker errors after flushing, which meant that if the link failed the build would continue, trying to copy the non-existent artifact, and also write the manifest as if it had succeeded.

I added Compilation.lockAndParseLldStderr to consume the stderr result of the lld call, and accumulate it in lld_errors. These errors are then surfaced via the same mechanism the other compilation errors are.

[kcbanner]

CI failure looks like #13549

[kubkon]

Can you try rebasing now that your fix has landed in master?

[kcbanner]

Rebased, CI is passing :D

[Vexu]

I don't know if it is intentional but it would make sense to me to render the error notes of plain errors in the same way as other errors:

diff --git a/src/Compilation.zig b/src/Compilation.zig
index fac2ed688..af323e7fa 100644
--- a/src/Compilation.zig
+++ b/src/Compilation.zig
@@ -514,7 +514,7 @@ pub const AllErrors = struct {
                     }
                     ttyconf.setColor(stderr, .Reset);
                     for (plain.notes) |note| {
-                        try note.renderToWriter(ttyconf, stderr, "error", .Red, indent + 4);
+                        try note.renderToWriter(ttyconf, stderr, "note", .Cyan, indent + 4);
                     }
                 },
             }

[Vexu]

Suggested change

	fn parseLddStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) Allocator.Error!void {
	fn parseLldStderr(comp: *Compilation, comptime prefix: []const u8, stderr: []const u8) Allocator.Error!void {

[Vexu]

Suggested change

	const duped_msg = try comp.gpa.dupe(u8, line);
	var it = std.mem.split(u8, line, "error: ");
	_ = it.first();
	const trimmed = it.rest();
	const duped_msg = try std.fmt.allocPrint(comp.gpa, "{s}: {s}", .{ prefix, trimmed });

This turns error: ld.lld: error: ... into just error: ld.lld: ... which is nicer IMO.

[kcbanner]

Verified in the lld source all the errors come through the same function so should have the error: prefix. Updated.

[Vexu]

I think this could be trimmed from both sides since the error notes are already indented but if you want to preserve the errors in their original form I'm OK with that too.

Suggested change

	var trimmed = mem.trimRight(u8, line, &std.ascii.whitespace);
	if (mem.startsWith(u8, trimmed, context_prefix)) {
	trimmed = trimmed[context_prefix.len..];
	}
	var trimmed = line;
	if (mem.startsWith(u8, line, context_prefix)) {
	trimmed = mem.trim(u8, trimmed[context_prefix.len..], &std.ascii.whitespace);
	}

[kcbanner]

I think preserving the beginning whitespace is nicer for this case, to maintain alignment:

error: lld-link: undefined symbol: definitely_not_existing
    note: referenced by C:\cygwin64\home\kcbanner\temp\zig_link_problem\src\main.zig:6
    note:               C:\cygwin64\home\kcbanner\temp\zig_link_problem\zig-cache\o\90ba81227403799d17d7062446163b66\zig_link_problem.exe.obj:(main.main)

[kcbanner]

Thanks for the review! Agreed on the formatting change, updated.