Lazily compile the zig rc subcommand and use it during zig build-exe, etc

[squeek502]

This moves .rc/.manifest compilation out of the main Zig binary, contributing towards #19063

Also:

• Makes resinator use Aro as its preprocessor instead of clang (closes make resinator use aro's preprocessor instead of clang #17752, contributes towards make the main zig executable no longer depend on LLVM, LLD, and Clang libraries #16270)
• Syncs resinator with upstream

---

TODO

• Better error message integration
• Better progress bar support when zig rc has to be lazily built during zig build-exe, etc. Right now it produces misleading output where it looks like some other operation is taking a long time (compiler_rt..., LLVM emit, etc)
• More testing and general cleanup
• Deal with this error when using the MinGW includes (might need to define __GNUC__ or _MSC_VER during preprocessing or something lame like that):

D:\a\zig\zig\test\standalone\windows_resources\res\zig.rc:1:1: error: zig rc exited with code 1
error: error(compilation): zig rc failed with stderr:
error: failed during preprocessing:

D:/a/zig/zig/lib/libc/include/any-windows-any/vadefs.h:35:2: error: VARARGS not implemented for this compiler
#error VARARGS not implemented for this compiler
 ^
1 error generated.

[andrewrk]

For error messages, one option is std.zig.ErrorBundle. If you use this API to build your errors then it offers both a way to render to stderr as well as a trivially serializable representation which could be used to communicate with a parent process. Perhaps the parent process would set a flag and then receive that format on stdout. Or, for more flexibility, use std.zig.Server over stdout/stdin just like the test runner and some other components do.
As for the progress bar... Resinator is pretty fast isn't it? Perhaps progress isn't necessary?

[squeek502]

Resinator is pretty fast isn't it? Perhaps progress isn't necessary?

If the lazily compiled resinator binary isn't already in the cache, spawning zig rc will have to lazily build it first, which is not fast. The actual .rc -> .res compilation is indeed fast.

It's the (potential) lazy compiliation of the resinator binary that isn't represented nicely by the progress indicator currently.

[ehaas]

Deal with this error when using the MinGW includes (might need to define GNUC or _MSC_VER during preprocessing or something lame like that):

Commit eb837b092c9c6c9f80e80fcbf54e35f57d2dbd59 of aro added the __GNUC__ macros. Just set comp.langopts.gnuc_version to 40201 to match clang's behavior, which is to pretend to be GCC 4.2.1

[andrewrk]

It's the (potential) lazy compiliation of the resinator binary that isn't represented nicely by the progress indicator currently.

That affects all the lazily built commands right? I can add a commit to address that for you.

I'm generally displeased with std.Progress currently. I think it leaves a lot of room for improvement.

[squeek502]

That affects all the lazily built commands right? I can add a commit to address that for you.

Yes, that'd be great.

Commit eb837b092c9c6c9f80e80fcbf54e35f57d2dbd59 of aro added the GNUC macros. Just set comp.langopts.gnuc_version to 40201 to match clang's behavior, which is to pretend to be GCC 4.2.1

Thanks for the heads up. I should have seen this coming, really--this is what I did when using clang as the preprocessor:

zig/src/resinator/preprocess.zig

Lines 56 to 68 in 0b744da

	// Using -fms-compatibility and targeting the GNU abi interact in a strange way:
	// - Targeting the GNU abi stops _MSC_VER from being defined
	// - Passing -fms-compatibility stops __GNUC__ from being defined
	// Neither being defined is a problem for things like MinGW's vadefs.h,
	// which will fail during preprocessing if neither are defined.
	// So, when targeting the GNU abi, we need to force __GNUC__ to be defined.
	//
	// TODO: This is a workaround that should be removed if possible.
	if (include_args.needs_gnu_workaround) {
	// This is the same default gnuc version that Clang uses:
	// https://github.com/llvm/llvm-project/blob/4b5366c9512aa273a5272af1d833961e1ed156e7/clang/lib/Driver/ToolChains/Clang.cpp#L6738
	try argv.append("-fgnuc-version=4.2.1");
	}

[squeek502]

Got error messages working ~nicely using std.zig.Server as suggested (but one-way-communication only, Compilation.zig just reads the stdout of the resinator binary; no stdin involved).

Some fairly nice examples:

(the last one is returning a raw error from resinator/main.zig which in theory should only happen for OutOfMemory, but I haven't made that strictly true yet. Note that if ZIG_DEBUG_CMD is set it'd show the error return trace)

And some that might still need some work since they don't give enough context:

---

@Vexu you might want to take a look at aroDiagnosticsToErrorBundle on the off-chance that's something that Aro might want to provide itself--it's entirely possible there'd be little use-case for it outside of resinator, though.

[Vexu]

@Vexu you might want to take a look at aroDiagnosticsToErrorBundle on the off-chance that's something that Aro might want to provide itself--it's entirely possible there'd be little use-case for it outside of resinator, though.

Maybe not Aro itself should provide it but the Aro based translate-c is going to need it.

[squeek502]

Got progress working when the resinator binary is not in the cache:

zig-rc-progress2-crop.mp4

but there are some potential issues that I'm not sure about:

• If multiple .rc files are compiled in one Compilation (e.g. zig build-exe main.zig foo.manifest foo.rc) and the resinator binary is not cached, then the progress can freak out a bit. Might be a bug in my implementation, but in general this is kicking off competing zig rc commands that will each try building the same resinator binary which may make sense to try to avoid?
• Once a compiled .res is cached, zig build-exe will skip rebuilding resinator even if the resinator source code has changed. This is because the manifest of the .res only cares about the direct inputs (the .rc file, any included files, any resource files) and not about the resinator binary/source.

One thing I can think of that might fix both of these would be to:

• Check whether or not zig rc is going to be needed (rc_source_files.len > 0) and, if so, kick off one zig rc call to (1) build/cache the resinator binary and (2) get the path to the cached binary.
• Use the path to the cached binary as part of the manifest of the .res file, and possibly just call the cached binary directly in updateWin32Resource instead of going through zig rc again.

I'm not sure if that's a good idea, though, or where exactly the initial check would go.

[andrewrk]

kicking off competing zig rc commands that will each try building the same resinator binary which may make sense to try to avoid?

It's fine actually - Zig's cache system ensures that two processes executed at the same time coordinate when they would produce the same cache artifacts. One of them will win the race and start working on the compilation while the other one waits patiently to reuse the results.

As for the other issue, I think it would be OK to accept that tradeoff, but also your solution seems reasonable too.

[andrewrk]

Some data points on this branch:

A ReleaseSmall build of zig goes from 9.9M to 9.7M (-3%).

Similar reduction in time to build the compiler:

Benchmark 1 (8 runs): before/zig build-exe ...
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          85.3s  ± 1.63s     81.3s  … 86.2s           1 (13%)        0%
  peak_rss           4.56GB ± 21.3MB    4.55GB … 4.61GB          1 (13%)        0%
  cpu_cycles          346G  ± 1.76G      345G  …  351G           1 (13%)        0%
  instructions        505G  ± 1.26G      504G  …  508G           1 (13%)        0%
  cache_references   21.6G  ±  110M     21.3G  … 21.7G           1 (13%)        0%
  cache_misses       1.71G  ± 8.50M     1.70G  … 1.73G           1 (13%)        0%
  branch_misses      2.43G  ± 7.86M     2.42G  … 2.45G           2 (25%)        0%
Benchmark 2 (8 runs): after/zig build-exe ...
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          83.7s  ± 1.41s     82.9s  … 87.1s           1 (13%)          -  1.9% ±  1.9%
  peak_rss           4.45GB ±  287KB    4.45GB … 4.45GB          0 ( 0%)        ⚡-  2.3% ±  0.4%
  cpu_cycles          337G  ±  485M      337G  …  338G           0 ( 0%)        ⚡-  2.7% ±  0.4%
  instructions        492G  ±  231M      492G  …  492G           0 ( 0%)        ⚡-  2.6% ±  0.2%
  cache_references   20.6G  ± 70.4M     20.6G  … 20.8G           1 (13%)        ⚡-  4.4% ±  0.5%
  cache_misses       1.65G  ± 6.55M     1.65G  … 1.67G           0 ( 0%)        ⚡-  3.1% ±  0.5%
  branch_misses      2.37G  ± 2.76M     2.36G  … 2.37G           0 ( 0%)        ⚡-  2.5% ±  0.3%

[squeek502]

Thanks for the benchmark, I was planning on doing that as well. Note that this PR also switches resinator to use Aro instead of clang, meaning the benchmark is not necessarily a complete comparison. In other words, instead of going from "built-in resinator with clang" -> "built-in resinator with aro" -> "lazily compiled resinator with aro", we're going from (1) to (3), whereas the transition from (2) to (3) might have shown larger savings.

[squeek502]

EDIT: You can ignore the text below if you'd like, fixed the progress bar in Compilation.zig instead by ignoring 0-length strings in progress messages.

---

@andrewrk could you briefly explain how the two zig rc processes coordinate when they're both trying to build the resinator binary? The progress bar freaking out is due to one of the processes sending "" as the progress string within the progressThread fn here while the other sends the "real" progress strings.

I'm wondering why that is the case, and whether or not it would make sense to do something like:

        if (progress_string.len > 0) {
            server.serveMessage(.{
                .tag = .progress,
                .bytes_len = @as(u32, @intCast(progress_string.len)),
            }, &.{
                progress_string,
            }) catch |err| {
                fatal("unable to write to client: {s}", .{@errorName(err)});
            };
        }

That'd solve my particular problem but unsure if it'd cause other problems elsewhere.

[squeek502]

Marking this as ready for review. Some testing done:

• zig rc in win32-samples-rc-tests with MSVC includes
• zig build-lib in a branch of win32-samples-rc-tests with MSVC includes

506 .rc files processed without discrepancies
identical .res outputs:     481
expected compile errors:    25

• zig rc in win32-samples-rc-tests with MinGW includes
• zig build-lib in a branch of win32-samples-rc-tests with MinGW includes

506 .rc files processed without discrepancies
identical .res outputs:     436
expected compile errors:    70

I also tried confirming that rufus still builds from the example in this article but hit this regression from the switch to MinGW ucrt: #18621

[andrewrk]

Thanks for putting this together!