-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a .rc
-> .res
compiler to the Zig compiler
#17069
Conversation
c82d8c8
to
4e02f4a
Compare
All 4 failing tests on aarch64-windows have
|
Yeah, not sure what's up. From the
|
Only populating the rc include dirs when there are Something unanticipated that I'm running into that I'm not sure how to handle:
The problem with this:
So, I'm not sure what the best thing to do here is. Allowing the user to separately choose the "ABI" (really, the include paths) for
but I'm unsure about that. Separately, there's also a question of whether or not |
97dad71
to
728f2d4
Compare
One thing to consider which may help us resolve these design questions is that the idea of including a res compiler tool within zig itself is for these reasons:
Otherwise, there would be no benefit to using zig's res compiler over the system-installed one that ships with MSVC right? Given this, I think we should try to figure out how to satisfy these dependencies without relying on an MSVC installation path. Maybe we could consider a real world use case, of compiling a game or a GUI application from source. Without the MSVC include paths available, what goes wrong? |
Agreed, those are precisely the use-cases I'm trying to enable. The goal here is for things to 'just work' for basically everyone by default.
These both work, and is what the added standalone test ensures. MSVC is not required (but its includes will be used if it's available), and cross-compiling from any target works fine (it'll use the MinGW includes when cross-compiling) as long as the
For most It will only go wrong if the To be clear, MSVC not being found is currently never an error condition on its own unless In terms of a real example, here's a Note: I wasn't able to get the By default it compiles fine (MSVC includes are used since it's installed on my system): > zig build-lib -dynamic empty.zig COMRTS.rc
> echo %errorlevel%
0 When using > zig build-lib -dynamic empty.zig COMRTS.rc -rcincludes=gnu
error(compilation): clang preprocessor failed with stderr:
COMRTS.rc:210:10: fatal error: 'afxres.rc' file not found
COMRTS.rc:1:1: error: clang preprocessor exited with code 1 Note that which include paths get used when compiling |
It all sounds pretty reasonable to me. One possible idea- what if it just never tried to do the MSVC include paths ever, so that the behavior was always the same no matter the host. Then there would never be any surprises when doing a compilation on one host or another. And for people who want the MSVC includes, they could use |
To me that would depend on what we expect the primary/supported use case of this to be. If the primary/supported use case is cross-compiling Windows programs, then consistently using MinGW includes would make some sense (to essentially force The current approach of "use MSVC, fall back to MinGW" is geared towards being maximally useful-by-default when compiling on Windows, at the expense of a little bit of potentially surprising behavior when cross-compiling that same program from other platforms (e.g. being able to reference The other possibility (and this is how it worked at the start of this PR) would be to have the include directories match the target, so EDIT: Also just to note, any usage of |
Ultimately, I think it's your call! You did the hard work of implementing it, and you've consistently been the champion of the zig toolchain user experience on Windows. I'm happy to brainstorm with you and discuss pros and cons, but, in this matter, you da boss. |
Sounds good. In that case, I'm going to stick with the MSVC + fallback approach and potentially re-evaluate depending on any issues that come up afterwards. Note that I'm nearly ready to remove the draft status of this PR. Let me know if there's anything I can do to make this easier to review for you. |
…age when source_line is 0 The zero value needs special handling since it means 'no source line' and should be preserved through the copy.
728f2d4
to
a299a02
Compare
https://learn.microsoft.com/en-us/cpp/mfc/mfc-and-atl Note that this include directory gets added to %INCLUDE% by vcvarsall.bat, and is especially crucial when working with resource files (many .rc files within the https://github.com/microsoft/Windows-classic-samples/ set reference files from the ATLMFC include directory).
…ormat is not coff
The include directories used when preprocessing .rc files are now separate from the target, and by default will use the system MSVC include paths if the MSVC + Windows SDK are present, otherwise it will fall back to the MinGW includes distributed with Zig. This default behavior can be overridden by the `-rcincludes` option (possible values: any (the default), msvc, gnu, or none). This behavior is useful because Windows resource files may `#include` files that only exist with in the MSVC include dirs (e.g. in `<MSVC install directory>/atlmfc/include` which can contain other .rc files, images, icons, cursors, etc). So, by defaulting to the `any` behavior (MSVC if present, MinGW fallback), users will by default get behavior that is most-likely-to-work. It also should be okay that the include directories used when compiling .rc files differ from the include directories used when compiling the main binary, since the .res format is not dependent on anything ABI-related. The only relevant differences would be things like `#define` constants being different values in the MinGW headers vs the MSVC headers, but any such differences would likely be a MinGW bug.
412ca22
to
0168ed7
Compare
…GNU abi Also update the standalone test so that this failure would have been detected on any host system.
Okay, I'm marking this as ready for review. Some stuff that might be relevant:
Also, I made a modified version of the scripts I use that test all the
|
Here are some of my thoughts as a real-world user of resinator. First, I don't mind having to add an entry to my Next, all this messy business with include directories around RC files is a pain just like it is in C. For new projects, I'd like to be able to generate I'm certain Squeek is much more expert than me on the topic of win32 RC/RES files, but I imagine a const res = b.addResources();
res.addIcon("icon16", .{ .path = "myicon16.ico" });
res.addIcon("icon32", .{ .path = "myicon32.ico" });
res.addRcData("stuff", .{ .path = "generic-data-to-hide-in-my-exe"});
exe.addResources(res); Then maybe some way to access those names: const std = @import("std");
const res_names = @import("resource_names");
fn main() {
std.log.info("icon16 id is {}", .{res_names.icon16});
} |
This may look appealing on first glance, but there are some issues with it:
icon16 ICON "myicon16.ico"
icon32 ICON "myicon32.ico"
stuff RCDATA "generic-data-to-hide-in-my-exe"
EDIT: Thinking about it more, the two approaches could probably coexist. Either
It seems like this would mostly be reimplementing |
To clarify, I was suggesting another "front end" on top of what resinator is providing:
Suggestion:
The idea being that maybe the RC frontend could remain a third party package if Zig has a way of adding resources to windows binaries in its build system. However, this is only advantageous if separating the RC frontend into another package is possible and would save a noticeable amount of code/space in the Zig toolchain. For example, if you need to parse an icon file to create a res file, then that's a piece that Zig would need either way and I would categorize that in the "Resinator" piece in the diagrams above rather than the "RC Frontend" piece. It's unclear to me how much of Resinator is responsible for parsing/processing RC files and how much is responsible for piecing that information together to generate the RES files? To add even more clarification, this means there is some theoretical "simpler interface" to resinator other than RC. Maybe this interface already exists in the form of some sort of AST within resinator already, or maybe this is a pipe dream that doesn't exist because RC is already a "simple enough" interface? These are questions I'm hoping you can answer Squeek. P.S. I just thought of another important advantage to this. This theoretical simpler interface wouldn't have any dependence on the Clang preprocessor which prevents us from becoming more dependent on Clang when Zig's goal is to make this an optional dependency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work. Thanks for taking the time to grok Compilation.zig and fully integrate this into the frontend of the compiler, including with error message reporting and parallelism. Love it!
One follow-up change that would be interesting to explore would be using Aro for the preprocessor rather than clang.
Thanks! Will definitely have a follow-up soon--that
Shouldn't be too hard to swap between them, since the preprocessing is a very distinct step and I've also considered writing a custom preprocessor (by forking aro, probably): squeek502/resinator#5 since in theory an @marler8997 here's a very rough estimate of the size of the different parts if that's helpful:
Again, though, dropping the lexing/parsing part would not be compatible with #9564 I think it would also be possible to implement roughly what you want as userland code that you call in |
My main point is that I think depending on a C preprocessor and the esoteric RC language just to add resources to our windows executables seems like a "local maximum". I think it would be better for Zig to have native support for resource configuration without a dependence on these old clunky technologies. Adding a new frontend that runs before the RC frontend doesn't remove these dependencies, rather, the pipeline I'm suggesting would look more like this:
I expect "ZigResourceConfig" could just be an internal data structure within |
My position is that the clunky RC language is necessary for use cases that I think Zig cares about (compiling existing As a thought experiment, how would your same argument apply to C files? Is |
Completely agree
I'm actually less concerned about how easy it is to use and more concerned with the dependency on the C preprocessor. For example, one way to implement this idea is to make the "ZigResourceConfig" interface the RC language but without the C preprocessor. Instead of C preprocessor macros, format a string in your build.zig file. Instead of shared header files, use build options to pass names to your program. I'm not saying this is what we should or shouldn't do (not familiar enough with RC syntax) but since your RC code was only 5000 lines, it doesn't seem totally unreasonable. But before this I would first want to explore a "higher maximum" which would be a syntax that's more familiar to Zig developers and checked by the Zig compiler.
The C language is the local maximum in this case :) Ziglang is the higher plateau in which we strive for. That being said, I believe the same idea about making the RC frontend an optional package have also been considered for general C interoperability. I can imagine one day Zig making translate-c and compiling C an optional dependency that's not included out-of-the box. Note that this is all predicated on it being very easy to add these dependencies. A couple lines in a build.zig.zon file for example. |
resinator will be able to use
Basically, I think I'm mostly going to take a "we'll cross that bridge when we come to it" stance on this for now. If there ends up being a need for a preprocessor-less resource compiler, then something can be figured out. |
In any case thanks so much for doing this @squeek502! Every part of what you've done here is valuable, I'm just wondering if we should add an additional piece to this puzzle and get your thoughts/guidance on it since you've become very familiar with this topic now. |
Follow up to ziglang#17069. This TODO being left in was a complete oversight. Before, any 'retryable' error would hit: error: thread 2920 panic: access of union field 'success' while field 'failure_retryable' is active Now, it will be reported/handled properly: C:\Users\Ryan\Programming\Zig\zig\test\standalone\windows_resources\res\zig.rc:1:1: error: FileNotFound
Follow up to #17069. This TODO being left in was a complete oversight. Before, any 'retryable' error would hit: error: thread 2920 panic: access of union field 'success' while field 'failure_retryable' is active Now, it will be reported/handled properly: C:\Users\Ryan\Programming\Zig\zig\test\standalone\windows_resources\res\zig.rc:1:1: error: FileNotFound
Uses resinator under-the-hood (see ziglang#17069) Closes ziglang#9564
This integrates resinator into the Zig compiler and closes #3702. This will provide Zig with the ability to compile (and cross-compile) Windows resource files into byte-for-byte identical
.res
files when compared to the canonicalrc.exe
implementation.I gave a talk about
resinator
a little bit ago if you want more detail: https://www.youtube.com/watch?v=RZczLb_uI9EStill some stuff left to do before this should be merged, but it's far enough along that it's ready for feedback.How it's used
In the simplest form, you can just give the path to the
.rc
file via the command line like any other source file:the equivalent in
build.zig
would be:If you need to pass
rc.exe
-like flags,-rcflags <flags> --
can be used before the.rc
file like so:the equivalent in
build.zig
would be:By default,
zig
will try to use the most appropriate system headers available (independent of the target ABI). On Windows, it will always try to use MSVC/Windows SDK include paths if they exist, and fall back to the MinGW headers bundled with Zig if not. On non-Windows, it will always use the MinGW header include paths. The intention with this is to make most.rc
files work by default whenever possible, since the MSVC includes have some.rc
-related include files that MinGW does not.If the default header include behavior is unwanted, the
-rcincludes
option can be used:the equivalent in
build.zig
would be:The possible values are
any
(this is the default),msvc
(always use MSVC, no fall back),gnu
(always use MinGW), ornone
(no system include paths provided automatically).Note: If the target object file is not
coff
, then specifying a.rc
or.res
file on the command line is an error:But
std.Build.Compile.Step.addWin32ResourceFile
can be used regardless of the target, and if the target object format is not coff, then the resource file will just be ignored.How it works
There is a four step process for each
.rc
file:flags
are parsed by resinator, if there are any invalid flags it'll error and fail the compilation.rc
file is run through the clang preprocessor and turned into an intermediate.rcpp
file.rcpp
file is compiled by resinator and turned into a.res
file.res
file is added to the list of link objects and linked into the final binary by the linkerNote that this is all integrated with the cache system (or should be):
.d
and all dependencies are added to the cache manifestSo, for example, if
a.rc
includesb.rc
andb.rc
has a resource that referencesc.ico
, then modifying any of the three will cause the.res
to be recompiled.Example
Here's a zip of an example project: zig-ico.zip
Building on Windows:
Cross-compiling:
Result (both the icon and the version information comes from the
.rc
file):Stuff left to do
utils.zig
as is feasible/wanted