Skip to content

Commit 6221f98

Browse files
update translate-c to work recursively and fix bug where calling getGraph would cache too early (outside of "make" step) (#94)
**Changelog** - update translate-c to work recursively - fix bug where calling getGraph would cache too early (outside of "make" step) The bug with `getGraph` could cause the following crash: ```sh thread 68890 panic: attempt to use null value /home/jae/Tools/zig/current/lib/std/Build/Step/Compile.zig:1309:88: 0x142802c in getZigArgs (std.zig) const import_index = cli_named_modules.modules.getIndex(import).?; ^ /home/jae/Tools/zig/current/lib/std/Build/Step/Compile.zig:1787:36: 0x15e259f in make (std.zig) const zig_args = try getZigArgs(compile, false); ^ /home/jae/Tools/zig/current/lib/std/Build/Step.zig:278:33: 0x148cdbf in make (std.zig) const make_result = s.makeFn(s, options); ^ /home/jae/Tools/zig/current/lib/compiler/build_runner.zig:1345:26: 0x148adfe in makeStep (build_runner.zig) } else if (s.make(.{ ^ /home/jae/Tools/zig/current/lib/std/Io.zig:1245:17: 0x148aa25 in start (std.zig) _ = @as(Cancelable!void, @call(.auto, function, args_casted.*)) catch {}; ^ /home/jae/Tools/zig/current/lib/std/Io/Threaded.zig:552:22: 0x124b165 in start (std.zig) task.func(task.contextPointer()); ^ /home/jae/Tools/zig/current/lib/std/Io/Threaded.zig:1797:29: 0x1249364 in worker (std.zig) runnable.startFn(runnable, &thread, t); ^ /home/jae/Tools/zig/current/lib/std/Thread.zig:422:13: 0x1249045 in callFn__anon_27841 (std.zig) @call(.auto, f, args); ^ /home/jae/Tools/zig/current/lib/std/Thread.zig:1431:30: 0x1248e00 in entryFn (std.zig) return callFn(f, self.fn_args); ^ /home/jae/Tools/zig/current/lib/std/os/linux/x86_64.zig:105:5 ``` Fixes #91
1 parent 9f00193 commit 6221f98

3 files changed

Lines changed: 162 additions & 72 deletions

File tree

build.zig

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,13 @@ pub fn build(b: *std.Build) void {
2828

2929
// Create stub of builtin options.
3030
// This is discovered and then replaced by "Apk" in the build process
31-
const android_builtin_options = std.Build.addOptions(b);
32-
android_builtin_options.addOption([:0]const u8, "package_name", "");
33-
const android_builtin_module = android_builtin_options.createModule();
31+
// const android_builtin_options = std.Build.addOptions(b);
32+
// android_builtin_options.addOption([:0]const u8, "package_name", "");
33+
const android_builtin_module = b.createModule(.{
34+
.root_source_file = b.path("src/android/android_builtin.zig"),
35+
.target = target,
36+
.optimize = optimize,
37+
});
3438

3539
// Create android module
3640
const android_module = b.addModule("android", .{

src/android/android_builtin.zig

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
//! Stub that is replaced by build system
2+
pub const package_name: [:0]const u8 = "";

src/androidbuild/Apk.zig

Lines changed: 153 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -442,10 +442,8 @@ fn doInstallApk(apk: *Apk) Allocator.Error!*Step.InstallFile {
442442
break :blk aapt2_package_name_file;
443443
};
444444

445-
const android_builtin = blk: {
446-
const android_builtin_options = BuiltinOptionsUpdate.create(b, package_name_file);
447-
break :blk android_builtin_options.createModule();
448-
};
445+
const android_builtin_options = BuiltinOptionsUpdate.create(b, package_name_file);
446+
const android_builtin_mod = android_builtin_options.createModule();
449447

450448
// We could also use that information to create easy to use Zig step like
451449
// - zig build adb-uninstall (adb uninstall "com.zig.sdl2")
@@ -492,71 +490,87 @@ fn doInstallApk(apk: *Apk) Allocator.Error!*Step.InstallFile {
492490
@panic(b.fmt("artifact[{d}] has no 'target' set", .{artifact_index}));
493491
}
494492

493+
// Add libraries *and* this artifact (exe) to collected libraries
494+
//
495+
// As of Zig 0.15.2 the order looks like
496+
// - your_app_name
497+
// - SDL3
498+
// - freetype
499+
// - imgui
500+
const compile_dep_list = apk.getCompileDependencies(artifact, true);
501+
502+
for (compile_dep_list) |compile_dep| {
503+
const graph = apk.getGraph(compile_dep.root_module);
504+
505+
// Update android_builtin
506+
for (graph.modules) |module| {
507+
if (module.import_table.get("android_builtin")) |prev_module| {
508+
if (prev_module != android_builtin_mod) {
509+
module.addImport("android_builtin", android_builtin_mod);
510+
compile_dep.step.dependOn(&android_builtin_options.options.step);
511+
}
512+
}
513+
}
514+
// Update translate-c module
515+
for (graph.modules) |module| {
516+
const root_source_file = module.root_source_file orelse continue;
517+
const c_translate_target = module.resolved_target orelse continue;
518+
if (!c_translate_target.result.abi.isAndroid()) continue;
519+
switch (root_source_file) {
520+
.generated => |gen| {
521+
const step = gen.file.step;
522+
switch (step.id) {
523+
.translate_c => {
524+
// Detect if using Translate-C vendored version
525+
//
526+
// NOTE(jae): 2026-04-29
527+
// Longterm this will deprecated from Zig
528+
529+
const translate_c: *std.Build.Step.TranslateC = @fieldParentPtr("step", step);
530+
translate_c.addIncludePath(.{ .cwd_relative = apk.ndk.include_path });
531+
translate_c.addSystemIncludePath(.{ .cwd_relative = apk.getSystemIncludePath(c_translate_target) });
532+
},
533+
.run => {
534+
// Detect if using Translate-C external dependency and make assumptions about the flags
535+
// we can pass into it such as "isystem" and "-I"
536+
//
537+
// Name: https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L89
538+
// Imports: https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L103-L104
539+
if (std.mem.startsWith(u8, step.name, "translate-c ") and
540+
(module.import_table.contains("c_builtins") and module.import_table.contains("helpers")))
541+
{
542+
const run: *std.Build.Step.Run = @fieldParentPtr("step", step);
543+
544+
const ndk_include_path: LazyPath = .{ .cwd_relative = apk.ndk.include_path };
545+
const system_include_path: LazyPath = .{ .cwd_relative = apk.getSystemIncludePath(c_translate_target) };
546+
547+
// Exposes the system include path `path` to both translate-c and to `t.mod`.
548+
// https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L207-L211
549+
module.addSystemIncludePath(system_include_path);
550+
run.addPrefixedDirectoryArg("-isystem", system_include_path);
551+
552+
// Exposes the include path `path` to both translate-c and to `t.mod`.
553+
// https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L203-L206
554+
module.addIncludePath(ndk_include_path);
555+
run.addPrefixedDirectoryArg("-I", ndk_include_path);
556+
}
557+
},
558+
else => continue,
559+
}
560+
},
561+
else => continue,
562+
}
563+
}
564+
}
495565
// Add module
496566
// - If a module has no `root_source_file` (e.g you're only compiling C files using `addCSourceFiles`)
497567
// then adding an import module will cause a build error (as of Zig 0.15.1).
498568
if (artifact.root_module.root_source_file != null) {
499-
artifact.root_module.addImport("android_builtin", android_builtin);
500-
}
501-
502-
var modules_it = artifact.root_module.import_table.iterator();
503-
while (modules_it.next()) |entry| {
504-
const module = entry.value_ptr.*;
505-
if (module.import_table.get("android_builtin")) |_| {
506-
module.addImport("android_builtin", android_builtin);
507-
}
508-
}
509-
510-
// Find TranslateC dependencies and add system path
511-
var iter = artifact.root_module.import_table.iterator();
512-
while (iter.next()) |it| {
513-
const module = it.value_ptr.*;
514-
const root_source_file = module.root_source_file orelse continue;
515-
const c_translate_target = module.resolved_target orelse continue;
516-
if (!c_translate_target.result.abi.isAndroid()) continue;
517-
switch (root_source_file) {
518-
.generated => |gen| {
519-
const step = gen.file.step;
520-
switch (step.id) {
521-
.translate_c => {
522-
// Detect if using Translate-C vendored version
523-
//
524-
// NOTE(jae): 2026-04-29
525-
// Longterm this will deprecated from Zig
526-
527-
const translate_c: *std.Build.Step.TranslateC = @fieldParentPtr("step", step);
528-
translate_c.addIncludePath(.{ .cwd_relative = apk.ndk.include_path });
529-
translate_c.addSystemIncludePath(.{ .cwd_relative = apk.getSystemIncludePath(c_translate_target) });
530-
},
531-
.run => {
532-
// Detect if using Translate-C external dependency and make assumptions about the flags
533-
// we can pass into it such as "isystem" and "-I"
534-
//
535-
// Name: https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L89
536-
// Imports: https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L103-L104
537-
if (std.mem.startsWith(u8, step.name, "translate-c ") and
538-
(module.import_table.contains("c_builtins") and module.import_table.contains("helpers")))
539-
{
540-
const run: *std.Build.Step.Run = @fieldParentPtr("step", step);
541-
542-
const ndk_include_path: LazyPath = .{ .cwd_relative = apk.ndk.include_path };
543-
const system_include_path: LazyPath = .{ .cwd_relative = apk.getSystemIncludePath(c_translate_target) };
544-
545-
// Exposes the system include path `path` to both translate-c and to `t.mod`.
546-
// https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L207-L211
547-
module.addSystemIncludePath(system_include_path);
548-
run.addPrefixedDirectoryArg("-isystem", system_include_path);
549-
550-
// Exposes the include path `path` to both translate-c and to `t.mod`.
551-
// https://codeberg.org/ziglang/translate-c/src/commit/71642ad0084d433f14b091a7b2b109f0be915dbb/build/Translator.zig#L203-L206
552-
module.addIncludePath(ndk_include_path);
553-
run.addPrefixedDirectoryArg("-I", ndk_include_path);
554-
}
555-
},
556-
else => continue,
557-
}
558-
},
559-
else => continue,
569+
const module = artifact.root_module;
570+
if (module.import_table.get("android_builtin")) |prev_module| {
571+
if (prev_module != android_builtin_mod) {
572+
artifact.root_module.addImport("android_builtin", android_builtin_mod);
573+
}
560574
}
561575
}
562576

@@ -823,6 +837,76 @@ fn setLibCFile(apk: *Apk, compile: *Step.Compile) void {
823837
compile.setLibCFile(android_libc_path);
824838
}
825839

840+
/// Copy-paste of "lib/std/Build/Module.zig" but it doesn't cache via GetGraph and won't potentially break the
841+
/// Zig build system.
842+
///
843+
/// Return the full set of `Step.Compile` which `start` depends on, recursively. `start` itself is
844+
/// always returned as the first element. If `chase_dynamic` is `false`, then dynamic libraries are
845+
/// not included, and their dependencies are not considered; if `chase_dynamic` is `true`, dynamic
846+
/// libraries are treated the same as other linked `Compile`s.
847+
fn getCompileDependencies(apk: *Apk, start: *Step.Compile, chase_dynamic: bool) []const *Step.Compile {
848+
const arena = start.step.owner.graph.arena;
849+
850+
var compiles: std.AutoArrayHashMapUnmanaged(*Step.Compile, void) = .empty;
851+
var next_idx: usize = 0;
852+
853+
compiles.putNoClobber(arena, start, {}) catch @panic("OOM");
854+
855+
while (next_idx < compiles.count()) {
856+
const compile = compiles.keys()[next_idx];
857+
next_idx += 1;
858+
859+
for (apk.getGraph(compile.root_module).modules) |mod| {
860+
for (mod.link_objects.items) |lo| {
861+
switch (lo) {
862+
.other_step => |other_compile| {
863+
if (!chase_dynamic and other_compile.isDynamicLibrary()) continue;
864+
compiles.put(arena, other_compile, {}) catch @panic("OOM");
865+
},
866+
else => {},
867+
}
868+
}
869+
}
870+
}
871+
872+
return compiles.keys();
873+
}
874+
875+
const Graph = struct {
876+
modules: []const *std.Build.Module,
877+
names: []const []const u8,
878+
};
879+
880+
/// Copy-paste of "lib/std/Build/Module.zig" but it doesn't cache and won't potentially break the
881+
/// Zig build system.
882+
///
883+
/// Given that `root` is the root `Module` of a compilation, return all `Module`s
884+
/// in the module graph, including `root` itself. `root` is guaranteed to be the
885+
/// first module in the returned slice.
886+
fn getGraph(apk: *Apk, root: *std.Build.Module) Graph {
887+
const arena = apk.b.graph.arena;
888+
889+
var modules: std.AutoArrayHashMapUnmanaged(*std.Build.Module, []const u8) = .empty;
890+
var next_idx: usize = 0;
891+
892+
modules.putNoClobber(arena, root, "root") catch @panic("OOM");
893+
894+
while (next_idx < modules.count()) {
895+
const mod = modules.keys()[next_idx];
896+
next_idx += 1;
897+
modules.ensureUnusedCapacity(arena, mod.import_table.count()) catch @panic("OOM");
898+
for (mod.import_table.keys(), mod.import_table.values()) |import_name, other_mod| {
899+
modules.putAssumeCapacity(other_mod, import_name);
900+
}
901+
}
902+
903+
const result: Graph = .{
904+
.modules = modules.keys(),
905+
.names = modules.values(),
906+
};
907+
return result;
908+
}
909+
826910
fn updateArtifact(apk: *Apk, artifact: *Step.Compile, raw_top_level_apk_files: *Step.WriteFile) void {
827911
const b = apk.b;
828912

@@ -911,7 +995,7 @@ fn applyLibLinkCppWorkaroundIssue19(apk: *Apk, artifact: *Step.Compile) void {
911995
const b = apk.b;
912996

913997
const should_apply_fix = (artifact.root_module.link_libcpp == true or
914-
dependsOnSystemLibrary(artifact, "c++abi_zig_workaround"));
998+
apk.dependsOnSystemLibrary(artifact, "c++abi_zig_workaround"));
915999
if (!should_apply_fix) {
9161000
return;
9171001
}
@@ -953,9 +1037,9 @@ fn applyLibLinkCppWorkaroundIssue19(apk: *Apk, artifact: *Step.Compile) void {
9531037

9541038
/// Copy-paste of "dependsOnSystemLibrary" that only checks if that system library is included to
9551039
/// workaround a bug with in Zig 0.15.0-dev.1092+d772c0627
956-
fn dependsOnSystemLibrary(compile: *Step.Compile, name: []const u8) bool {
957-
for (compile.getCompileDependencies(true)) |some_compile| {
958-
for (some_compile.root_module.getGraph().modules) |mod| {
1040+
fn dependsOnSystemLibrary(apk: *Apk, compile: *Step.Compile, name: []const u8) bool {
1041+
for (apk.getCompileDependencies(compile, true)) |dep_compile| {
1042+
for (apk.getGraph(dep_compile.root_module).modules) |mod| {
9591043
for (mod.link_objects.items) |lo| {
9601044
switch (lo) {
9611045
.system_lib => |lib| if (std.mem.eql(u8, lib.name, name)) return true,

0 commit comments

Comments
 (0)