[R2-12][P4] Decompose build.zig:build (668-line function → focused helpers)

[MichaelFisher1997]

Part of #839 — [Audit][Round 2] A++ umbrella.
Phase: P4 · Finding: R2-12

Problem

The top-level pub fn build in build.zig is 668 lines — the single largest function in the codebase. It bundles: option declarations (~80 lines), module creation, dependency wiring, shader validation (~40 validate_vulkan_* steps at the tail), and install steps into one linear procedural body. Hard to navigate, hard to extend, hard to review.

Evidence

build.zig:3-671 — pub fn build(b: *std.Build) void spans 668 lines.

Some decomposition already exists:

fn addCimgui(...) void { ... }
fn addSharedImports(...) void { ... }
fn addSharedImportsNoOptions(...) void { ... }
fn addProjectModuleImports(module, *22 module params) void { ... }  // ← 22-param smell

But the bulk — options, module creation, shader validation — is one linear function.

For scale: the second-largest function in the codebase is lod_renderer.zig:LODRenderer at 552 lines (a struct factory, inherent to Vulkan-style init). The third is vulkan/bloom_system.zig:init at 347 lines. build.zig:build exceeds both.

Fix

Extract focused helpers, each verifiably correct in isolation:

Helper 1 — defineBuildOptions(b) BuildOptions

Move the ~80 lines of b.option(...) declarations into one helper that returns a struct of all resolved options. Eliminates ~80 lines from build and gives options a single named handle.

Helper 2 — defineModules(b, options) Modules

Move the ~30 b.addModule(...) calls into one helper. Returns a struct of all module references.

Helper 3 — defineShaderValidation(b, modules, options) []Step

Move the ~40 validate_vulkan_* steps (currently at the tail of build) into one helper. Returns the validation steps so they can be wired into test dependency.

Helper 4 — defineInstallSteps(b, ...) void

Move install artifacts, prefixes, and post-install hooks into one helper.

Helper 5 — collapse addProjectModuleImports's 22 params

The 22-parameter helper is itself a smell. Replace with:

const ProjectModules = struct {
    engine_core: *std.Build.Module,
    engine_math: *std.Build.Module,
    // ... all 22
};
fn addProjectModuleImports(mod: *std.Build.Module, deps: ProjectModules) void

Single struct param, named access at call sites.

Target

pub fn build shrinks from 668 lines to ~50 lines of orchestration: const opts = defineBuildOptions(b); const mods = defineModules(b, opts); const validation = defineShaderValidation(b, mods, opts); const install = defineInstallSteps(b, mods); b.getInstallStep().dependOn(&validation.step);

Verification

• nix develop --command zig build (full build must succeed unchanged)
• nix develop --command zig build test (includes shader validation)
• nix develop --command zig build -Doptimize=ReleaseFast (release build)
• nix develop --command zig build run (smoke run)
• All build options still respected: try -Dskip-present, -Dsmoke-test, -Ddebug_shadows, -Dbenchmark-preset=low, -Dauto-world=flat etc. — confirm they still take effect.
• Single PR — behavior-preserving mechanical refactor.

Constraints

• Behavior-preserving. Build output and option semantics unchanged.
• Conventional commit: refactor(build): decompose pub fn build into focused helpers.
• Do not change the public build option names — downstream tooling (Nix, CI, AI workflows) depends on them.
• The ProjectModules struct change is the riskiest (touches all 22 callers) — consider landing as a separate PR.

Notes

• This is the lowest-urgency architectural issue in Round 2 but it pays dividends every time someone touches the build (new module, new option, new shader).
• A future stretch: split build.zig itself by area (build_options.zig, build_modules.zig, etc.) — out of scope here.

Tracking: #839

[github-actions]

Validated against build.zig HEAD:

Audit findings confirmed

• pub fn build currently spans lines 3-671 (669 lines) — matches the reported 668.
• 23 b.option(...) calls at lines 8-83 (matches "~80 lines of option declarations" including the per-option options.addOption registrations).
• 24 validate_vulkan_* addSystemCommand steps + 25 matching test_step.dependOn calls at lines 620-670 (matches "~40").
• addProjectModuleImports does take 22 module parameters (verified lines 691-712).

One scope note worth flagging in the PR description

Helper 2 (defineModules) as proposed ("~30 b.addModule(...) calls") understates the actual wiring block. The module + imports plumbing occupies roughly lines 119-330+: 36 createModule calls (119-153), then addSharedImports/addImport pairs for every module. The helper should bundle both creation and the import-wiring for each module, otherwise build won't shrink much beyond the options extraction. Same caveat for addSharedImports/addSharedImportsNoOptions — they should be folded into the modules helper rather than left as separate top-level fns.

Pre-existing structure to preserve

build.zig:7 creates const options = b.addOptions(); once, then each b.option(...) is paired with an options.addOption(...) registration. The BuildOptions struct returned by defineBuildOptions needs to carry both the raw b.option(...) return values and the *std.Build.Step.Options handle (since downstream code passes options into addSharedImports). Cleanest shape is probably:

const BuildOptions = struct {
    raw: struct { debug_shadows: bool, ... },
    step: *std.Build.Step.Options,
};

Scope split — agree with the author's own caveat

The issue already correctly flags Helper 5 (22-param → ProjectModules struct) as the riskiest and suggests landing it separately. I'd take that one step further and recommend two PRs:

1. PR A (this issue, shrink build): Helpers 1-4 only. Pure mechanical extraction of options / modules / shader validation / install steps. build becomes ~50 lines of orchestration. addProjectModuleImports stays 22-param for this PR. Behavior-preserving, low review burden, unblocks every subsequent build.zig change.

2. PR B (follow-up, separate): Convert addProjectModuleImports (and ideally addSharedImports/addSharedImportsNoOptions at the same time) to take struct params. Same idea the issue already endorses; keeping it isolated makes the diff greppable and reviewable.

Trying to land both in a single PR risks merging 668→50 lines plus a 22-callsite signature change in one go, which is exactly the kind of mega-diff that hides subtle regressions in option-name / module-name typos.

Verification checklist is solid. The six build commands listed (debug, test, ReleaseFast, run, all option variants) cover the matrix. Worth adding zig fmt build.zig to the checklist since refactors tend to touch alignment, and nix develop --command zig build install if any install-artefact paths are touched in Helper 4.

No duplicate or conflicting PRs found via gh pr list --search build.zig. No related issues (the other R2 findings target engine-rhi, world-runtime, LODManager, updateGlobalUniforms, etc. — all orthogonal).

[github-actions]

Comment posted on issue #851: validation of audit findings, scope-refinement note about Helper 2 needing to bundle imports wiring too, recommendation for two-PR split (Helpers 1-4 in PR A, Helper 5 in follow-up PR B), and confirmation that no duplicate PRs exist.

opencode session  |  github run