build.zig: Fix various issues around `-Dconfig`
Here is a PR with issues 1, 2, and 3 from https://github.com/raysan5/raylib/pull/4393#issuecomment-2423777267 addressed.
I didn't bother implementing 4 because there seemed to be multiple possible solutions to it, the way I originally attempted it was quite hacky and caused breaking changes.
Breaking changes:
Now, valid flags like "-x c++" will be treated as "-x" "c++" and thus behave incorrectly. I found this to be a non-issue as Zig currently doesn't accept those flags, and I believe "-xc++" is a valid alternative. I don't know enough C to know if there are other possibly useful flags that often come with spaces between.
~~> Here is a PR with issues 1, 2, and 3 from #4393 (comment) addressed.~~
~~It's worth noting that issues 1 and 2 (the dependency problem) still occur when using @embedFile() over pathFromRootDir(). But since the goal of this commit is purely to resolve the -Dconfig problems, not the dependency problem, I think it's perfectly fine as-is. Not to mention, one can just uncomment those two lines should the need arise.~~
My bad was looking at the wrong comment, gotta reread before I actually respond to this
Also, if you don't mind explaining, could I ask how @embedFile() "bloats the size of the binary"? I'm reading the Zig documentation and it sounds like the bloat comes from creating a very long string, is that right?
Wait, are you still experiencing breakage for your use-case?
Also, if you don't mind explaining, could I ask how @embedFile() "bloats the size of the binary"?
Yes, if you have any values defined at comptime and is used at runtime, it will be added to the binary. Since src/config.h is about 16 KiB, it means any program embedding it AND using the value at runtime should become 16 KiB larger. Note, the "program" here is the build system and it will have no result in the final binary people should care about.
Anyhow, I thought of something that seems okay to me. Should be far smaller than embedding the entire file; also probably easier to debug. I need to check if this actually does reduce the binary size compared to just using @embedFile though. Maybe debug mode might not be smart enough to optimise it out.
/// A list of all flags from `src/config.h` that one may override
const config_h_flags = outer: {
// Set this value higher if compile errors happen as `src/config.h` gets larger
@setEvalBranchQuota(1 << 20);
const config_h = @embedFile("config.h");
var flags: []const []const u8 = &.{};
var lines = std.mem.tokenizeScalar(u8, config_h, '\n');
while (lines.next()) |line| {
if (!std.mem.containsAtLeast(u8, line, 1, "SUPPORT")) continue;
if (std.mem.startsWith(u8, line, "//")) continue;
if (std.mem.startsWith(u8, line, "#if")) continue;
var flag = std.mem.trimLeft(u8, line, " \t"); // Trim whitespace
flag = flag["#define ".len - 1 ..]; // Remove #define
flag = std.mem.trimLeft(u8, flag, " \t"); // Trim whitespace
flag = flag[0 .. std.mem.indexOf(u8, flag, " ") orelse continue]; // Flag is only one word, so capture till space
flag = "-D" ++ flag; // Prepend with -D
flags = @as([]const []const u8, flags ++ [1][]const u8{flag});
}
@compileLog(flags);
break :outer flags;
};
$ zig build
/home/plumeus/repos/raylib/zig_patch_2/src/build.zig:98:5: error: found compile log statement
@compileLog(flags);
^~~~~~~~~~~~~~~~~~
Compile Log Output:
@as([]const []const u8, &.{ "-DSUPPORT_MODULE_RSHAPES"[0..24], "-DSUPPORT_MODULE_RTEXTURES"[0..26], "-DSUPPORT_MODULE_RTEXT"[0..22], "-DSUPPORT_MODULE_RMODELS"[0..24], "-DSUPPORT_MODULE_RAUDIO"[0..23], "-DSUPPORT_CAMERA_SYSTEM"[0..23], "-DSUPPORT_GESTURES_SYSTEM"[0..25], "-DSUPPORT_RPRAND_GENERATOR"[0..26], "-DSUPPORT_MOUSE_GESTURES"[0..24], "-DSUPPORT_SSH_KEYBOARD_RPI"[0..26], "-DSUPPORT_WINMM_HIGHRES_TIMER"[0..29], "-DSUPPORT_PARTIALBUSY_WAIT_LOOP"[0..31], "-DSUPPORT_SCREEN_CAPTURE"[0..24], "-DSUPPORT_GIF_RECORDING"[0..23], "-DSUPPORT_COMPRESSION_API"[0..25], "-DSUPPORT_AUTOMATION_EVENTS"[0..27], "-DRL_SUPPORT_MESH_GPU_SKINNING"[0..30], "-DSUPPORT_QUADS_DRAW_MODE"[0..25], "-DSUPPORT_FILEFORMAT_PNG"[0..24], "-DSUPPORT_FILEFORMAT_GIF"[0..24], "-DSUPPORT_FILEFORMAT_QOI"[0..24], "-DSUPPORT_FILEFORMAT_DDS"[0..24], "-DSUPPORT_IMAGE_EXPORT"[0..22], "-DSUPPORT_IMAGE_GENERATION"[0..26], "-DSUPPORT_IMAGE_MANIPULATION"[0..28], "-DSUPPORT_DEFAULT_FONT"[0..22], "-DSUPPORT_FILEFORMAT_TTF"[0..24], "-DSUPPORT_FILEFORMAT_FNT"[0..24], "-DSUPPORT_TEXT_MANIPULATION"[0..27], "-DSUPPORT_FONT_ATLAS_WHITE_REC"[0..30], "-DSUPPORT_FILEFORMAT_OBJ"[0..24], "-DSUPPORT_FILEFORMAT_MTL"[0..24], "-DSUPPORT_FILEFORMAT_IQM"[0..24], "-DSUPPORT_FILEFORMAT_GLTF"[0..25], "-DSUPPORT_FILEFORMAT_VOX"[0..24], "-DSUPPORT_FILEFORMAT_M3D"[0..24], "-DSUPPORT_MESH_GENERATION"[0..25], "-DSUPPORT_FILEFORMAT_WAV"[0..24], "-DSUPPORT_FILEFORMAT_OGG"[0..24], "-DSUPPORT_FILEFORMAT_MP3"[0..24], "-DSUPPORT_FILEFORMAT_QOA"[0..24], "-DSUPPORT_FILEFORMAT_XM"[0..23], "-DSUPPORT_FILEFORMAT_MOD"[0..24], "-DSUPPORT_STANDARD_FILEIO"[0..25], "-DSUPPORT_TRACELOG"[0..18] }[0..45])
Wait, are you still experiencing breakage for your use-case?
When using @embedFile() instead of pathFromRoot(), yes. It still duplicates the submodule's file path:
/tmp/raylib-zig-dep-bug % zig build
install
└─ install raylib-zig-dep-bug
└─ zig build-exe raylib-zig-dep-bug Debug native
└─ WriteFile raylib.h failure
error: FileNotFound: /tmp/raylib-zig-dep-bug/raylib/raylib/src/raylib.h
But it's easily solvable by uncommenting the two pathFromRoot() lines, so I think it's perfectly fine. Not to mention, should src/build.zig ever be moved to the root directory in the future, that would also solve the problem.
Note, the "program" here is the build system and it will have no result in the final binary people should care about.
Oh, I thought it would affect the actual library binary that was outputted. That would be perfectly fine, then.
Don't know enough to comment on your other proposal regarding reading config.h, but if it makes the build script easier to debug, that sounds good to me.
When using
@embedFile()instead ofpathFromRoot(), yes. It still duplicates the submodule's file path:
Sorry to hear that. I'll try your repo with this again. This is why merging src/build.zig to build.zig simplifies a lot of things and makes it easier to debug too.
File size differences with usage of @embedFile:
test1.zig:
const std = @import("std");
pub fn main() void {
var x = @embedFile("config.h");
_ = &x;
}
test2.zig:
const std = @import("std");
/// A list of all flags from `src/config.h` that one may override
const config_h_flags = outer: {
// Set this value higher if compile errors happen as `src/config.h` gets larger
@setEvalBranchQuota(1 << 20);
const config_h = @embedFile("config.h");
var flags: []const []const u8 = &.{};
var lines = std.mem.tokenizeScalar(u8, config_h, '\n');
while (lines.next()) |line| {
if (!std.mem.containsAtLeast(u8, line, 1, "SUPPORT")) continue;
if (std.mem.startsWith(u8, line, "//")) continue;
if (std.mem.startsWith(u8, line, "#if")) continue;
var flag = std.mem.trimLeft(u8, line, " \t"); // Trim whitespace
flag = flag["#define ".len - 1 ..]; // Remove #define
flag = std.mem.trimLeft(u8, flag, " \t"); // Trim whitespace
flag = flag[0 .. std.mem.indexOf(u8, flag, " ") orelse continue]; // Flag is only one word, so capture till space
flag = "-D" ++ flag; // Prepend with -D
flags = @as([]const []const u8, flags ++ [1][]const u8{flag});
}
break :outer flags;
};
pub fn main() void {
var x = config_h_flags;
_ = &x;
}
$ zig build-exe test1.zig && zig build-exe test2.zig && du -b test1 test2
2454400 test1
2444984 test2
$ zig build-exe test1.zig -fstrip && zig build-exe test2.zig -fstrip && du -b test1 test2
201056 test1
186992 test2
$ strings test1 | rg RTEXT
#define SUPPORT_MODULE_RTEXTURES 1
#define SUPPORT_MODULE_RTEXT 1 // WARNING: It requires SUPPORT_MODULE_RTEXTURES to load sprite font textures
$ strings test2 | rg RTEXT
-DSUPPORT_MODULE_RTEXTURES
-DSUPPORT_MODULE_RTEXT
Okay, the difference is negligible here. But I still think the latter is easier to debug so I would prefer it that way.
Sorry to hear that. I'll try your repo with this again.
Oh, really, don't worry about it. Your code already has an opt-in fix: like I mentioned earlier, it works perfectly fine when uncommenting the pathFromRoot() lines.
This is why merging
src/build.zigtobuild.zigsimplifies a lot of things and makes it easier to debug too.
Yup. So I wouldn't worry about introducing temporary fixes to the dependency problem right now, since just moving src/build.zig to root is a far more graceful way of handling it.
Weird, when trying to use your repo on Zig 0.12.1 and 0.13.0, I get the following:
$ zig build
install
└─ install raylib
└─ zig build-lib raylib Debug native
└─ run wayland-scanner (xdg-shell-client-protocol-code.h) failure
error: FileNotFound: /tmp/raylib-zig-dep-bug/raylib/raylib/src/external/glfw/deps/wayland/xdg-shell.xml
<omitted>
I didn't notice this until now as I'm used to using the master branch of Zig, where everything compiles without any problems (including the @embedFile portion). And it's a different and more serious compile error that should be affecting more users than those passing -Dconfig... I wonder why you aren't encountering this... ugh, I really want to merge src/build.zig to build.zig. I'm getting headaches.
Okay, fine, I'll just give up and call it a day and be satisfied that this repo successfully builds from the root and src dir for all versions of Zig from 0.12.0 and onwards. I also added the commit on parsing src/config.h at comptime.
I plan to use #4393 to actually bring the logic over from src/build.zig to build.zig as I'm not sure how to get this working for all intended use-cases otherwise.
I wonder why you aren't encountering this...
Oh, I am actually. That's the same problem I mentioned earlier, where using pathFromRoot instead of @embedFile (or moving src/build.zig to build.zig`) makes it work.
I'm used to using the master branch of Zig, where everything compiles without any problems
Huh, didn't know that. I probably should have mentioned I was using 0.12, my bad.
And it's a different and more serious compile error that should be affecting more users than those passing
-Dconfigugh, I really want to mergesrc/build.zigtobuild.zig. I'm getting headaches.
Oh, I was thinking to just not worry about it until we try proposing the move to a single build.zig. As I was writing this, I just saw that you made a new commit. Sorry about that, didn't mean to make you work harder.
I'll go and test it with my repo in a bit.
As I was writing this, I just saw that you made a new commit. Sorry about that, didn't mean to make you work harder.
Oh, that commit is totally unrelated so it won't fix your immediate problems.
Anyhow, @raysan5, I think this PR is ready to be merged and I'll fix the other existing problems with #4393.
Oh, that commit is totally unrelated so it won't fix your immediate problems.
Ah, gotcha. No worries, you've accomplished your goals for this commit already. And the fix on my end is very easy, anyway.
Thanks for everything you've done.
@sagehane Thanks for the review! Merged!