Move to Zig 0.14 #40
Move to Zig 0.14 #40
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRepository updated for Zig v0.14.x: CI and docs bumped; build system refactored to module+executable wrappers and adapted to updated std.Build APIs; manifest Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main / tests
participant Child as std.process.Child
participant POSIX as posix
Note over Main,Child: Child process lifecycle (ngui/tests)
Main->>Child: std.process.Child.init(args, gpa)
Child-->>Main: child handle
Main->>Child: Child.run(...) / spawn / wait / kill
Note over Main,POSIX: Signal registration (now non-throwing)
Main->>POSIX: posix.sigaction(SIGINT, &sa, null)
Main->>POSIX: posix.sigaction(SIGTERM, &sa, null)
Note over Main,Main: Module-wrapped build/test creation (build.zig changes)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/sys/Service.zig (1)
92-99: Adddefer proc.deinit()immediately after initializingproc. spawnAndWait only cleans up the OS handles; in Zig 0.14 you must callChild.deinit()yourself to free the allocated Child object and avoid leaks.
🧹 Nitpick comments (6)
lib/ini (1)
1-1: CI/submodule hygiene for Zig 0.14 migration
- Ensure CI runs with submodules initialised:
git submodule update --init --recursivebefore the Zig build.- If reproducibility matters, keep the submodule pinned to a commit (as done) and record notable upstream changes in the PR description or CHANGELOG.
README.md (1)
15-15: Align README versioning with CI (pin or clearly state range).CI pins Zig to 0.14.1, while README states 0.14.x. Either pin the README or explicitly note that any 0.14.x works, with CI using 0.14.1, to avoid confusion.
Proposed tweak:
-you'll need [zig v0.14.x](https://ziglang.org/download/). +you'll need [Zig v0.14.1](https://ziglang.org/download/) (any 0.14.x should work; CI currently pins 0.14.1).src/sys/Service.zig (1)
40-43: Prefer using the project abstraction for Term.Use
types.ChildProcess.Terminstead ofstd.process.Child.Termto keep the indirection in one place (types.zig), making future upgrades (e.g., API churn) cheaper.- started: std.process.Child.Term, + started: types.ChildProcess.Term, @@ - stopped: std.process.Child.Term, + stopped: types.ChildProcess.Term,src/test.zig (3)
156-163: Confirm parity with std.process.Child.kill() return typeIn Zig 0.14, std.process.Child.kill typically returns !void (kill signal) and does not yield a Term; the Term comes from wait(). Your stub returns !Child.Term, which may diverge from production APIs and can leak into call sites (e.g., assigning the result of kill()).
If parity is desired, adapt the stub and call sites:
- pub fn kill(self: *TestChildProcess) !std.process.Child.Term { + pub fn kill(self: *TestChildProcess) !void { defer self.killed = true; if (self.kill_callback) |cb| { - return cb(self); + return cb(self); } - return .{ .Exited = 0 }; + return; }Outside this hunk (caller example):
try ngui_proc.kill(); const term = try ngui_proc.wait();
239-275: TypeInfo tag casing updates are correct; unify error tag for consistencyThe lower-case tags (.pointer/.one/.slice) and @"struct"/@"union"/.optional match Zig 0.14. Minor consistency nit: you return error.ExpectDeepEqual in the slice-length guard but error.TestExpectDeepEqual elsewhere; consider a single error tag.
- break :blk error.ExpectDeepEqual; + break :blk error.TestExpectDeepEqual;
284-298: Optional handling aligns with stdlib expectationsBranching prints are helpful; consistent error tag with the slice case would reduce noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(3 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(2 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (36)
lib/ini (2)
1-1: Submodule pointer bump looks fineThis is a straightforward update to the
lib/inisubmodule SHA; no code or API changes are present in this repo.
1-1: Upstream SHA exists; new SHA untagged
Both the old (19e1210…) and new (da0af3a… da0af3a32e3403e3113e103767065cbe9584f505) SHAs are present in https://github.com/ziglibs/ini.git, but no tags point at the new commit. Confirm whether pinning to this untagged commit is acceptable or if you’d rather update to a tagged release..github/workflows/ci.yml (1)
18-18: Keep the Zig version pinned at ‘0.14.1’.goto-bus-stop/setup-zig@v2requires an exact version and does not accept semver ranges like “0.14.x”..gitignore (1)
3-3: LGTM: ignore both .zig-cache and zig-cache.Covers hidden and legacy cache dirs; consistent with Zig’s recent cache layouts.
src/types.zig (1)
25-25: Ensure no leftover std.ChildProcess references after aliasing to std.process.Child.
Run:rg -nP '\bstd\.ChildProcess\b' -C2(per Zig 0.14 deprecations: std.ChildProcess → std.process.Child) (ziglang.org)
src/ui/lightning.zig (1)
17-38: StaticStringMap migration looks goodinitComptime usage and value type ([:0]const u8) align with Zig 0.14; existing get(appname) call remains valid. No action needed.
src/lightning/LndConf.zig (1)
232-243: Test harness writeFile API update LGTMSwitch to tmp.dir.writeFile with structured args matches Zig 0.14. The test content and assertions remain equivalent.
build.zig.zon (1)
2-3: Verify manifest name symbol and fingerprint stability.name = .ndg is correct for 0.14, but ensure the fingerprint matches the resolver output for your dependency state; mismatches can trigger rebuilds or warnings.
src/nd.zig (1)
173-173: std.process.Child migration is correctinit, stdio behaviours, spawn/kill usage remain valid under Zig 0.14.
src/sys/sysimpl.zig (2)
50-50: LGTM! Explicit type annotation improves clarity.The explicit type annotation for
AtomicFileOptionsaligns with Zig 0.14's preference for explicit typing and makes the code more maintainable.
75-75: LGTM! Structured argument pattern forwriteFile.The change to use a structured argument with named fields (
sub_pathanddata) follows Zig 0.14's API patterns and improves code readability.src/nd/network.zig (2)
107-107: LGTM! Consistent migration totokenizeScalar.The migration from
tokenizetotokenizeScalarwith character literals (e.g.,'\n'instead of"\n") is correct and aligns with Zig 0.14's API changes. This provides better type safety and potentially improved performance.Also applies to: 127-127, 160-160, 167-167
700-702: LGTM! Proper use ofsplitScalarfor key-value parsing.The change from
tokenizetotokenizeScalarwith a space delimiter andsplitScalarwith'='for parsing key-value pairs is appropriate for the CTRL-EVENT-SSID-TEMP-DISABLED message format.src/nd/Daemon.zig (2)
865-865: LGTM! Type-info path update for Zig 0.14.The change from
Pointertopointerin the type-info path correctly reflects Zig 0.14's naming convention changes for reflection APIs.
700-702: LGTM! Tokenisation API updates are consistent.The changes to use
tokenizeScalarandsplitScalarin the WPA control message parsing are appropriate and consistent with the broader migration pattern.lib/nif/build.zig (2)
8-18: Module-based library creation looks goodThe migration to the module-based build pattern correctly follows Zig 0.14's new build system architecture. The separation of module creation (
createModule) from library creation (addLibrary) with proper linkage is well-implemented.
19-20: Correct migration of macro definitions to root_moduleThe macro definitions have been properly moved from the library level to
root_module, aligning with Zig 0.14's new module ownership pattern.build.zig (4)
34-44: Well-structured ngui module wrapper implementationThe ngui executable creation using the module-based pattern is correctly implemented. The module owns the compilation settings whilst the executable wrapper handles the build artifacts properly.
135-144: Test module creation follows best practicesThe test setup correctly uses
createModulefor test configuration and passes it toaddTestviaroot_module. The filter migration from singular to array format is properly handled.
396-396: LVGLLogLevel.default signature update is correctThe parameter type change from
std.builtin.Modetostd.builtin.OptimizeModealigns with Zig 0.14's API changes.
447-447: VersionStep.make signature properly updatedThe addition of the
std.Build.Step.MakeOptionsparameter correctly follows Zig 0.14's new build step API.src/nd/Config.zig (7)
203-203: Type info access correctly updatedThe change from
Fn.return_typeto@"fn".return_typeproperly reflects Zig 0.14's type info API changes wherefnis now a reserved keyword requiring escaping.
217-217: bcrypt verification options properly structuredThe explicit
.silently_truncate_password = falseoption enhances security by preventing silent password truncation during verification.
236-237: Hash options restructuring follows Zig 0.14 patternsMoving
silently_truncate_passwordinto the.paramsstruct correctly aligns with the bcrypt API changes in Zig 0.14.
309-309: Explicit type annotations improve code clarityThe addition of explicit
std.fs.Dir.AtomicFileOptionstype annotations removes ambiguity and improves maintainability.Also applies to: 353-353, 433-433
373-373: Child process API migration is correctThe update from
std.ChildProcess.runtostd.process.Child.runproperly follows Zig 0.14's process API restructuring.
534-541: Test file writing API correctly updatedAll test file write operations have been properly migrated to use the new structured argument format with
.sub_pathand.datafields, following Zig 0.14's file API changes.Also applies to: 604-604, 637-640, 747-751, 788-795, 806-814
668-668: JSON parse options type annotation enhances clarityThe explicit type annotation for
std.json.ParseOptionsimproves code readability and type safety.src/test.zig (6)
136-141: spawn() error set updated appropriatelyReturning std.process.Child.SpawnError matches stdlib. The fall-through “success” return is fine for !void.
143-149: wait() signature update is consistentanyerror!std.process.Child.Term mirrors stdlib behaviour; default .Exited = 0 sentinel is OK for tests.
151-154: spawnAndWait() combines the two steps cleanlyPattern is sound; no concerns.
276-283: Struct field-wise deep-compare path looks goodInline iteration and targeted diagnostics are appropriate.
299-311: Tagged union path is correctGuarding untagged unions and comparing tags before payload recursion is the right approach.
109-111: std.process.Child callback migration approved
Callback signatures align with Zig 0.14’s std.process.Child and no stray std.ChildProcess references were found.src/test/guiplay.zig (2)
11-11: Child type migration is correctstd.process.Child is the right replacement for std.ChildProcess in Zig 0.14.
361-361: Initialiser updated correctlyUsing std.process.Child.init(argv, allocator) is correct for 0.14.
Double-check downstream calls (kill/wait): if kill() is !void, prefer:
try ngui_proc.kill(); const term = try ngui_proc.wait(); logger.info("ngui term: {any}", .{term});
| posix.sigaction(posix.SIG.INT, &sa, null); | ||
| posix.sigaction(posix.SIG.TERM, &sa, null); | ||
| sigquit.wait(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle sigaction errors instead of ignoring them
Same concern as ngui: if handler install fails, graceful shutdown may be skipped. Log failures.
- posix.sigaction(posix.SIG.INT, &sa, null);
- posix.sigaction(posix.SIG.TERM, &sa, null);
+ posix.sigaction(posix.SIG.INT, &sa, null) catch |err| {
+ logger.warn("sigaction INT failed: {any}", .{err});
+ };
+ posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| {
+ logger.warn("sigaction TERM failed: {any}", .{err});
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| posix.sigaction(posix.SIG.INT, &sa, null); | |
| posix.sigaction(posix.SIG.TERM, &sa, null); | |
| sigquit.wait(); | |
| // Install SIGINT handler, logging if it fails | |
| posix.sigaction(posix.SIG.INT, &sa, null) catch |err| { | |
| logger.warn("sigaction INT failed: {any}", .{err}); | |
| }; | |
| // Install SIGTERM handler, logging if it fails | |
| posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| { | |
| logger.warn("sigaction TERM failed: {any}", .{err}); | |
| }; | |
| sigquit.wait(); |
🤖 Prompt for AI Agents
In src/nd.zig around lines 227-229, the two posix.sigaction calls are used but
their return values are ignored; if installing a signal handler fails the
program may skip graceful shutdown. Change each posix.sigaction call to capture
its return/error, and when it indicates failure log the error (include
errno/strerror or Zig's error detail) via the existing logger or std.debug.warn
with clear context (e.g., "failed to install SIGINT handler: <err>"), then
decide a safe fallback (continue running with logging or initiate shutdown)
rather than silently ignoring the failure.
| posix.sigaction(posix.SIG.INT, &sa, null); | ||
| posix.sigaction(posix.SIG.TERM, &sa, null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don't swallow sigaction errors; at least log them
Silently ignoring failures to install SIGINT/SIGTERM handlers can skip clean shutdown paths. Log-and-continue instead of bare calls.
- posix.sigaction(posix.SIG.INT, &sa, null);
- posix.sigaction(posix.SIG.TERM, &sa, null);
+ posix.sigaction(posix.SIG.INT, &sa, null) catch |err| {
+ logger.warn("sigaction INT failed: {any}", .{err});
+ };
+ posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| {
+ logger.warn("sigaction TERM failed: {any}", .{err});
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| posix.sigaction(posix.SIG.INT, &sa, null); | |
| posix.sigaction(posix.SIG.TERM, &sa, null); | |
| posix.sigaction(posix.SIG.INT, &sa, null) catch |err| { | |
| logger.warn("sigaction INT failed: {any}", .{err}); | |
| }; | |
| posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| { | |
| logger.warn("sigaction TERM failed: {any}", .{err}); | |
| }; |
🤖 Prompt for AI Agents
In src/ngui.zig around lines 448-449, the calls to posix.sigaction for SIGINT
and SIGTERM currently ignore possible errors; change them to check the
return/error result and log any failure instead of swallowing it. After each
posix.sigaction call, capture its error/return value, and if it indicates
failure, call the existing logger or std.debug.warn/std.log with a clear message
including the errno or error detail and context (e.g., "failed to install SIGINT
handler: {err}"), then continue execution so the program still runs even if
handler install fails.
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the codebase from Zig 0.12 to Zig 0.14, updating APIs and build system to match the latest Zig changes while maintaining the existing usingnamespace feature (which will be removed in Zig 0.15).
- Updates process APIs from
std.ChildProcesstostd.process.Child - Modernizes build system to use module-based structure for apps, libraries and tests
- Updates tokenization, signal handling, and file system APIs to match Zig 0.14 changes
Reviewed Changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates documentation to require Zig v0.14.x |
| .github/workflows/ci.yml | Updates CI to use Zig 0.14.1 |
| build.zig | Modernizes build system to module-based structure |
| build.zig.zon | Updates package name format and adds fingerprint |
| lib/nif/build.zig | Updates library build to new module system |
| src/ui/lightning.zig | Updates ComptimeStringMap to StaticStringMap API |
| src/types.zig | Updates ChildProcess type reference |
| src/test/guiplay.zig | Updates process and signal handling APIs |
| src/test.zig | Updates process types and type info enum names |
| src/sys/sysimpl.zig | Updates file system API calls |
| src/sys/Service.zig | Updates process type references |
| src/ngui.zig | Updates signal handling API |
| src/nd/network.zig | Updates tokenization API calls |
| src/nd/Daemon.zig | Updates tokenization, type info, and error handling |
| src/nd/Config.zig | Updates bcrypt, file system, and type info APIs |
| src/nd.zig | Updates process and signal handling APIs |
| src/lightning/LndConf.zig | Updates file system API call |
| lib/ini | Updates submodule to compatible version |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lightning/LndConf.zig (1)
39-41: Use the managed map’s built-in key ownership: passkeydirectly togetOrPut
std.StringArrayHashMapduplicates the key internally; pre-duplicating causes a double allocation on insert and leaves the extra copy unfreed on lookups. Replace:- const res = try self.props.getOrPut(try self.alloc.dupe(u8, key)); + const res = try self.props.getOrPut(key);and likewise for the
var rescase.
♻️ Duplicate comments (1)
src/nd/Daemon.zig (1)
135-138: Erroneous lifetime management: use two-phase errdefer; current defer risks double-free/UAF or leaks on later field failures.
deferat initialisation freessvlistunconditionally; after ownership transfer viatoOwnedSlice, this can invalidate the returnedservices.list(or, iftoOwnedSlicedetaches the buffer, you still risk leaking services when latertryfields fail). Useerrdeferbefore the move, and introduce a seconderrdeferafter the move to clean up the owned slice if later steps error out.Apply:
var svlist = std.ArrayList(sys.Service).init(opt.allocator); - defer { + errdefer { for (svlist.items) |*sv| sv.deinit(); svlist.deinit(); } @@ - logger.debug("conf = {any}", .{opt.conf}); + logger.debug("conf = {any}", .{opt.conf}); + + // Transfer ownership to a slice; ensure cleanup if subsequent initialisation fails. + const services_list = try svlist.toOwnedSlice(); + errdefer { + for (services_list) |*sv| sv.deinit(); + opt.allocator.free(services_list); + } @@ - return .{ + return .{ .allocator = opt.allocator, .conf = opt.conf, .uireader = opt.uir, .uiwriter = opt.uiw, .wpa_ctrl = try types.WpaControl.open(opt.wpa), .state = .stopped, .screenstate = if (opt.conf.data.slock != null) .locked else .unlocked, - .services = .{ .list = try svlist.toOwnedSlice() }, + .services = .{ .list = services_list }, // send persisted settings immediately on start .want_settings = true, // send a network report right at start without wifi scan to make it faster. .want_network_report = true, .want_wifi_scan = false, .network_report_ready = true, // report bitcoind status immediately on start .want_onchain_report = true, .bitcoin_timer = try time.Timer.start(), // report lightning status immediately on start .want_lnd_report = true, .lnd_timer = try time.Timer.start(), };Also applies to: 146-168
🧹 Nitpick comments (5)
src/ngui.zig (1)
448-449: Non-throwing sigaction in Zig 0.14: fine; consider clarifying commentZig 0.14’s
std.posix.sigactionis non-throwing, so droppingtryis correct. As a tiny polish, add a note so future readers don’t assume error handling was forgotten.const sa = posix.Sigaction{ .handler = .{ .handler = sighandler }, .mask = posix.empty_sigset, .flags = 0, }; - posix.sigaction(posix.SIG.INT, &sa, null); - posix.sigaction(posix.SIG.TERM, &sa, null); + // Zig 0.14: posix.sigaction is non-throwing. + posix.sigaction(posix.SIG.INT, &sa, null); + posix.sigaction(posix.SIG.TERM, &sa, null);src/test/guiplay.zig (1)
382-383: Non-throwing sigaction: optional comment for claritySame note as in ngui: adding a brief comment can prevent confusion about missing
try/catch.const sa = posix.Sigaction{ .handler = .{ .handler = sighandler }, .mask = posix.empty_sigset, .flags = 0, }; - posix.sigaction(posix.SIG.INT, &sa, null); - posix.sigaction(posix.SIG.TERM, &sa, null); + // Zig 0.14: posix.sigaction is non-throwing. + posix.sigaction(posix.SIG.INT, &sa, null); + posix.sigaction(posix.SIG.TERM, &sa, null);src/test.zig (1)
239-275: TypeInfo tag updates align with Zig 0.14; unify custom error names (nit)Lower-case tags and quoted variants look correct. Minor consistency nit: use a single custom error name.
- std.debug.print("expected {any}, found null\n", .{x}); - return error.TestExpectDeepEqual; + std.debug.print("expected {any}, found null\n", .{x}); + return error.ExpectDeepEqual; ... - std.debug.print("expected null, found {any}\n", .{v}); - return error.TestExpectDeepEqual; + std.debug.print("expected null, found {any}\n", .{v}); + return error.ExpectDeepEqual;Also applies to: 276-283, 284-298, 299-312
src/lightning/LndConf.zig (1)
101-104: Fix minor typos in doc commentsSmall grammar/typo nits: “a empty” → “an empty”, “no long” → “no longer”, “confg” → “config”.
-/// creates a empty config, ready to be populated start with `appendSection`. -/// the object is serialized with `dumpWriter`. `deinit` when no long used. +/// creates an empty config, ready to be populated starting with `appendSection`. +/// the object is serialized with `dumpWriter`. `deinit` when no longer used. ... -/// creates a new confg section, owning a name copy dup'ed using the `arena` allocator. +/// creates a new config section, owning a name copy dup'ed using the `arena` allocator.Also applies to: 179-181
src/nd/Daemon.zig (1)
865-866: Avoid brittle @typeinfo path; use std.meta.Child for channel element type.Relying on
@typeInfo(...).pointer.childis version-fragile.std.meta.Child(@TypeOf(...))is clearer and stable across Zig releases.Apply:
- var channels = std.ArrayList(@typeInfo(@TypeOf(lndrep.channels)).pointer.child).init(self.allocator); + const Channel = std.meta.Child(@TypeOf(lndrep.channels)); + var channels = std.ArrayList(Channel).init(self.allocator);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(3 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(3 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (14)
- .gitignore
- src/types.zig
- .github/workflows/ci.yml
- src/sys/Service.zig
- src/nd.zig
- src/nd/network.zig
- src/sys/sysimpl.zig
- build.zig.zon
- lib/ini
- src/ui/lightning.zig
- build.zig
- src/nd/Config.zig
- lib/nif/build.zig
- README.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
src/test/guiplay.zig (2)
11-11: Migration to std.process.Child looks correctType swap from
std.ChildProcesstostd.process.Childaligns with Zig 0.14. No issues spotted.
361-361: Init call updated appropriately
std.process.Child.init(a.items, gpa)matches the new API. Good.src/test.zig (2)
109-112: Callback type migration to std.process.Child: OKThe callback signatures now reference
std.process.Child.*types. Looks consistent.
136-141: No references tostd.ChildProcessremain—changes verified.src/lightning/LndConf.zig (1)
232-242: No legacywriteFile(path, data)calls remain
Alltmp.dir.writeFileinvocations now use the{ .sub_path, .data }options struct.src/nd/Daemon.zig (1)
701-710: Parsing change LGTM; Zig 0.14 tokenisers used correctly.
mem.tokenizeScalar+mem.splitScalaris appropriate here and preserves prior behaviour forauth_failures>0.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nd/Daemon.zig (1)
1178-1185: Bug: unlocking a mutex you didn’t lock (double-unlock risk).
resetLndTlsUnguardedstates callers must holdself.mu, but it performsdefer self.mu.unlock()in an early-return branch. This will unlock the caller-held lock and the caller’s owndefer self.mu.unlock()will then unlock again, leading to undefined behaviour/panic.Apply:
fn resetLndTlsUnguarded(self: *Daemon) !void { if (self.lnd_tls_reset_count > 0) { return error.LndTlsResetCount; } - switch (self.state) { - .stopped, .poweroff, .wallet_reset => { - defer self.mu.unlock(); - switch (self.state) { - .poweroff => return Error.PoweroffActive, - .wallet_reset => return Error.WalletResetActive, - else => return Error.InvalidState, - } - }, - // proceed only when in one of the following states - .running, .standby => {}, - } + switch (self.state) { + .poweroff => return Error.PoweroffActive, + .wallet_reset => return Error.WalletResetActive, + .stopped => return Error.InvalidState, + // proceed only when in one of the following states + .running, .standby => {}, + }
♻️ Duplicate comments (1)
src/nd/Daemon.zig (1)
146-152: Service ownership transfer and cleanup are now correct.Switching to
toOwnedSlice()with anerrdeferonservices_listfixes the earlier unconditional cleanup problem and prevents double-free/use-after-free when ownership moves into the returnedDaemon. Looks good.Also applies to: 161-161
🧹 Nitpick comments (2)
.gitignore (1)
3-5: Tiny consistency nit: use trailing slashes for directoriesMakes intent explicit and avoids accidentally ignoring files named exactly “zig-cache” or “zig-out”.
Apply:
-.zig-cache -zig-cache -zig-out +.zig-cache/ +zig-cache/ +zig-out/src/nd/Daemon.zig (1)
872-873: Avoid brittle type-info spelunking; usestd.meta.Childfor slice element type.This reduces dependence on TypeInfo field names that changed across Zig versions and reads clearer.
Apply:
- var channels = std.ArrayList(@typeInfo(@TypeOf(lndrep.channels)).pointer.child).init(self.allocator); + const Channel = std.meta.Child(@TypeOf(lndrep.channels)); + var channels = std.ArrayList(Channel).init(self.allocator);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(4 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(3 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (16)
- src/ui/lightning.zig
- lib/ini
- .github/workflows/ci.yml
- src/nd.zig
- src/sys/Service.zig
- src/nd/network.zig
- lib/nif/build.zig
- src/lightning/LndConf.zig
- src/test/guiplay.zig
- src/types.zig
- src/nd/Config.zig
- src/ngui.zig
- build.zig.zon
- src/test.zig
- build.zig
- src/sys/sysimpl.zig
🔇 Additional comments (2)
.gitignore (1)
3-3: LGTM: add .zig-cache to ignoresGood addition for Zig 0.14+ workflows.
src/nd/Daemon.zig (1)
707-718: Tokenisation migration totokenizeScalar/splitScalarlooks correct.The parsing still reliably catches non-zero
auth_failuresand mirrors the old behaviour. No issues spotted.
kristapsk
force-pushed
the
zig-0.14
branch
2 times, most recently
from
65540b8 to
4be24ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/nd.zig (1)
227-228: Don’t ignore sigaction errors (dup of prior feedback)Log failures so shutdown semantics aren’t silently lost.
- posix.sigaction(posix.SIG.INT, &sa, null); - posix.sigaction(posix.SIG.TERM, &sa, null); + posix.sigaction(posix.SIG.INT, &sa, null) catch |err| { + logger.warn("sigaction INT failed: {any}", .{err}); + }; + posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| { + logger.warn("sigaction TERM failed: {any}", .{err}); + };
🧹 Nitpick comments (5)
src/sys/sysimpl.zig (3)
51-55: Make atomic write robust with absolute paths and Windows portabilityRelying on std.fs.cwd() with a possibly absolute sub_path works on POSIX via openat semantics but is brittle and less portable. Split the path, open the parent dir explicitly (absolute vs relative), and pass only the basename to BufferedAtomicFile.create.
- const file = try std.io.BufferedAtomicFile.create(allocator, std.fs.cwd(), hostname_filepath, opt); + const parent = std.fs.path.dirname(hostname_filepath) orelse "."; + const base = std.fs.path.basename(hostname_filepath); + const dir = if (std.fs.path.isAbsolute(hostname_filepath)) + try std.fs.openDirAbsolute(parent, .{}) + else + try std.fs.cwd().openDir(parent, .{}); + defer dir.close(); + const file = try std.io.BufferedAtomicFile.create(allocator, dir, base, opt);
52-52: Nit: fix comment grammarChange “does NOT deletes” → “does NOT delete”.
- defer file.destroy(); // releases resources; does NOT deletes the file + defer file.destroy(); // releases resources; does NOT delete the file
74-81: Dir.writeFile prefers a relative sub_path; avoid passing an absolute tmp pathIf tmp.join returns an absolute path (likely), passing it as Dir.writeFile.sub_path may be rejected on some platforms/build modes. Keep the absolute path for setHostname, but use a relative basename for Dir operations in the test.
- hostname_filepath = try tmp.join(&.{"hostname"}); - try tmp.dir.writeFile(.{ .sub_path = hostname_filepath, .data = "dummy" }); + const hostname_filename = "hostname"; + hostname_filepath = try tmp.join(&.{ hostname_filename }); + try tmp.dir.writeFile(.{ .sub_path = hostname_filename, .data = "dummy" }); @@ - const cont = try tmp.dir.readFile(hostname_filepath, &buf); + const cont = try tmp.dir.readFile(hostname_filename, &buf);Please confirm whether tmp.join returns an absolute path in tt.TempDir; if it already returns a relative path, this change is optional.
src/nd/Daemon.zig (2)
707-717: Harden parsing: also detect reason=WRONG_KEY/WRONG_PSKSome wpa_supplicant builds omit auth_failures; “reason” still signals bad creds.
- var it = mem.tokenizeScalar(u8, m, ' '); + var it = mem.tokenizeScalar(u8, m, ' '); while (it.next()) |kv_str| { - var kv = mem.splitScalar(u8, kv_str, '='); + var kv = mem.splitScalar(u8, kv_str, '='); if (mem.eql(u8, kv.first(), "auth_failures")) { const v = kv.next(); if (v != null and !mem.eql(u8, v.?, "0")) { self.wifiInvalidKey(); break; } } + if (mem.eql(u8, kv.first(), "reason")) { + const rv = kv.next(); + if (rv != null and (mem.eql(u8, rv.?, "WRONG_KEY") or mem.eql(u8, rv.?, "WRONG_PSK"))) { + self.wifiInvalidKey(); + break; + } + } }
872-872: Prefer std.meta.Child over @typeinfo(...).pointer.childKeeps this resilient to type-info shape changes.
- var channels = std.ArrayList(@typeInfo(@TypeOf(lndrep.channels)).pointer.child).init(self.allocator); + var channels = std.ArrayList(std.meta.Child(@TypeOf(lndrep.channels))).init(self.allocator);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(4 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(3 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- src/ngui.zig
- lib/ini
- .github/workflows/ci.yml
- src/sys/Service.zig
- build.zig.zon
- src/lightning/LndConf.zig
- README.md
- src/ui/lightning.zig
- src/nd/network.zig
- src/test/guiplay.zig
- lib/nif/build.zig
- src/test.zig
- src/nd/Config.zig
🔇 Additional comments (15)
src/sys/sysimpl.zig (1)
50-50: Explicitly typing AtomicFileOptions for Zig 0.14 — LGTMGood upgrade to an explicit type; this matches 0.14 API expectations.
.gitignore (1)
3-3: Add .zig-cache ignore — LGTMCovers both zig-cache variants; no further action.
src/types.zig (1)
25-25: Alias ChildProcess to std.process.Child — OK
No references tostd.ChildProcessremain in the codebase; changes are safe.src/nd.zig (1)
173-173: Switch to std.process.Child.init — LGTMConsistent with the types alias and wider migration.
src/nd/Daemon.zig (1)
146-152: Ownership transfer via toOwnedSlice + scoped cleanup — LGTMSafe move from ArrayList; upper errdefer remains harmless as svlist is emptied.
Also applies to: 161-161
build.zig (10)
34-44: Module-based build pattern looks goodThe refactoring to use
b.createModulefor creating the ngui module and then passing it tob.addExecutableviaroot_modulealigns well with Zig 0.14's build system changes.
46-81: Correct usage of root_module for imports and macrosThe migration from direct executable methods to
root_modulefor adding imports, include paths, and C macros is properly implemented according to Zig 0.14 conventions.
113-128: Consistent module pattern for nd daemonThe nd daemon follows the same module-based pattern as ngui, maintaining consistency across the build configuration.
135-144: Test configuration properly updated for Zig 0.14The test setup correctly uses:
b.createModulefor the test moduleroot_modulefield instead of deprecatedroot_source_filefiltersarray instead of singularfilter
158-171: GUI playground module configuration is correctThe guiplay executable properly uses the module pattern with inline module creation for the comm import.
176-190: Bitcoin RPC playground follows consistent patternThe btcrpc executable correctly implements the module-based build approach.
194-205: LND HTTP client playground properly configuredThe lndhc executable follows the established module pattern consistently.
396-396: LVGLLogLevel.default signature updated correctlyThe function signature has been properly updated to accept
std.builtin.OptimizeModeinstead of the deprecatedstd.builtin.Mode, aligning with Zig 0.14's API changes.
447-447: VersionStep.make signature updated for Zig 0.14The make function signature correctly accepts the new
std.Build.Step.MakeOptionsparameter instead of*std.Progress.Node, following Zig 0.14's build system changes.
117-117: The scripts will list everyaddExecutableblock and all.link_libcsettings inbuild.zig, so we can confirm whether any playground targets (e.g. guiplay, btcrpc, lndhc) omitlink_libcdespite needing it.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/nd/Config.zig (1)
217-225: Normalise bcrypt.strVerify error handling and count all user-caused failures
Zig 0.14’sbcrypt.strVerifyreturns onlyInvalidEncodingandPasswordVerificationFailedfrompwhash.HasherError; treat both as user-caused failures, incrementincorrect_attempts, and returnIncorrectSlockPin, while logging and propagating all other errors.- std.crypto.pwhash.bcrypt.strVerify(slock.bcrypt_hash, input, .{ .silently_truncate_password = false }) catch |err| { - if (err == error.PasswordVerificationFailed) { - self.data.slock.?.incorrect_attempts += 1; - return error.IncorrectSlockPin; - } - logger.err("bcrypt.strVerify: {!}", .{err}); - return err; - }; + std.crypto.pwhash.bcrypt.strVerify(slock.bcrypt_hash, input, .{ .silently_truncate_password = false }) catch |err| { + switch (err) { + // Consolidate all user-caused failures + error.PasswordVerificationFailed, + error.InvalidEncoding, + => { + self.data.slock.?.incorrect_attempts += 1; + return error.IncorrectSlockPin; + }, + else => { + logger.err("bcrypt.strVerify: {!}", .{err}); + return err; + }, + } + };
♻️ Duplicate comments (2)
src/nd/Config.zig (2)
353-366: Ditto: reuse AtomicFileOptionsSame note as above; a tiny helper/constant would keep these modes consistent.
433-446: Ditto: AtomicFileOptions reuseSame DRY suggestion as earlier.
🧹 Nitpick comments (3)
src/nd/Config.zig (3)
237-239: HashOptions update is correct; consider hoisting to a shared constantThe move of silently_truncate_password under .params matches 0.14. To keep config in one place and avoid drift, consider a shared constant for SLOCK_BCRYPT_OPTS.
Example outside this hunk:
const bcrypt = std.crypto.pwhash.bcrypt; const SLOCK_BCRYPT_OPTS: bcrypt.HashOptions = .{ .params = .{ .rounds_log = 12, .silently_truncate_password = false }, .encoding = .phc, };Then:
const hash = try bcrypt.strHash(s, SLOCK_BCRYPT_OPTS, &buf);
309-314: Repeated AtomicFileOptions: consider a helper/constantGood explicit typing for Zig 0.14. This same .mode appears in multiple places; consider a helper (e.g., createAtomicFile(mode)) or constants to reduce duplication.
373-388: Bound Child.run output to avoid unbounded allocationsZig 0.14’s std.process.Child.run supports
max_output_bytes(default 50 KiB) to limit captured stdout/stderr. Without it, long outputs can balloon memory.Apply:
- const res = try std.process.Child.run(.{ .allocator = allocator, .argv = &.{scriptpath} }); + const res = try std.process.Child.run(.{ + .allocator = allocator, + .argv = &.{ scriptpath }, + .max_output_bytes = 64 * 1024, // adjust as needed + });No other std.process.Child.run usages found in the codebase.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(4 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(3 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lib/ini
🚧 Files skipped from review as they are similar to previous changes (16)
- .gitignore
- .github/workflows/ci.yml
- src/ui/lightning.zig
- src/lightning/LndConf.zig
- src/types.zig
- src/sys/Service.zig
- build.zig.zon
- src/sys/sysimpl.zig
- README.md
- src/test/guiplay.zig
- src/ngui.zig
- src/nd/Daemon.zig
- src/nd.zig
- src/nd/network.zig
- lib/nif/build.zig
- src/test.zig
🔇 Additional comments (18)
src/nd/Config.zig (7)
203-207: Zig 0.14 type-info migration looks correctSwitching to @typeinfo(@typeof(F)).@"fn".return_type.? is the right update for 0.14. No further action from me here.
534-542: writeFile migration LGTMThe new Dir.writeFile({ .sub_path, .data }) usage looks correct.
604-604: LGTMZero-length file creation via writeFile with the new API looks fine.
636-641: LGTMTest script write and chmod look correct with the updated API.
788-795: LGTMConf fixture writes with the new API look correct.
806-815: LGTMNull slock conf fixture writes with the new API look correct.
669-670: Explicit ParseOptions type annotation LGTMGood defensive typing for Zig 0.14’s std.json API.
build.zig (11)
10-10: LGTM: LVGL log level parameter updated correctly.The parameter change from
std.builtin.Modetostd.builtin.OptimizeModealigns with Zig 0.14's API updates.
34-44: LGTM: Module-based wrapper pattern implemented correctly.The refactoring to use
b.createModule()with a separate executable wrapper follows Zig 0.14 best practices. The module configuration properly inherits target, optimization, and linking settings.
46-48: LGTM: Module imports and include paths properly configured.The transition from direct executable configuration to
root_moduleproperties is correctly implemented for Zig 0.14 compatibility.
75-81: LGTM: C macros properly migrated to root_module.All macro definitions have been correctly moved from the executable to the
root_module, maintaining the same functionality whilst adhering to the new API structure.
86-88: LGTM: Driver-specific configurations updated consistently.All driver configurations (SDL2, X11, fbev) have been properly updated to use the
root_moduleAPI whilst maintaining the same conditional logic and system library linking.Also applies to: 98-100, 104-106
135-144: LGTM: Test module configuration updated for Zig 0.14.The test setup correctly uses
b.createModule()and the newaddTest()API withroot_moduleandfiltersparameters instead of the deprecatedroot_source_fileandfilter.
158-171: LGTM: GUI playground refactored consistently.The guiplay executable follows the same module-based pattern with proper module creation and import configuration.
396-396: LGTM: Parameter type updated for Zig 0.14 compatibility.The function signature correctly changes from
std.builtin.Modetostd.builtin.OptimizeMode, which aligns with the build system API updates.
447-447: LGTM: VersionStep.make signature updated correctly.The
makefunction now properly acceptsstd.Build.Step.MakeOptionsinstead of*std.Progress.Node, conforming to Zig 0.14's build system API changes.
176-189: LGTM: Playground executables refactored consistently.Both
btcrpcandlndhcexecutables have been properly updated to use the module-based approach with correct import configurations.Also applies to: 194-205
113-128: Verifylink_libcon modules
Ensure that movinglink_libc = trueinto thecreateModulecalls (e.g. lines 38, 117, 139 in build.zig) still links libc for the generated executables exactly as before on all targets.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build.zig (1)
481-486: Two issues in git tag handling: ensure parseable tag and fix catch typing
- Using git describe without --abbrev=0 often yields “vX.Y.Z-N-gHASH”, which std.SemanticVersion.parse rejects.
- The catch block currently tries to break with an error into a non-error value, which won’t type-check.
Apply:
- const cmd = [_][]const u8{ git, "-C", self.b.pathFromRoot("."), "describe", "--match", matchTag, "--tags", "--abbrev=8" }; + const cmd = [_][]const u8{ git, "-C", self.b.pathFromRoot("."), "describe", "--match", matchTag, "--tags", "--abbrev=0" }; @@ - return std.SemanticVersion.parse(repotag) catch |err| ret: { - std.log.err("unparsable git tag semver '{s}': {any}", .{ repotag, err }); - break :ret err; - }; + return std.SemanticVersion.parse(repotag) catch |parse_err| { + std.log.err("unparsable git tag semver '{s}': {any}", .{ repotag, parse_err }); + return null; + };Optional: only enforce the presence of a semver when optimise is a Release* mode to keep local Debug builds frictionless.
Also applies to: 487-491
🧹 Nitpick comments (2)
build.zig (2)
135-145: Avoid passing an empty test filterPassing filters = &.{""} may behave differently than “no filter”. Prefer setting filters only when a value is provided.
Apply:
- const tests = b.addTest(.{ - .root_module = mod_tests, - .filters = &.{b.option([]const u8, "test-filter", "run tests matching the filter") orelse ""}, - }); + const tf = b.option([]const u8, "test-filter", "run tests matching the filter"); + const tests = b.addTest(.{ + .root_module = mod_tests, + .filters = if (tf) |f| &.{f} else null, + });If Build.TestOptions.filters is not nullable in your Zig 0.14, ping me and I’ll adjust accordingly.
158-168: Playground targets: minor consistency nitsConsider applying the same strip/pie choices across guiplay/btcrpc/lndhc for consistency (guiplay currently lacks strip and pie toggles).
Also applies to: 176-187, 194-202
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)build.zig(5 hunks)build.zig.zon(1 hunks)lib/ini(1 hunks)lib/nif/build.zig(1 hunks)src/lightning/LndConf.zig(2 hunks)src/nd.zig(2 hunks)src/nd/Config.zig(14 hunks)src/nd/Daemon.zig(4 hunks)src/nd/network.zig(3 hunks)src/ngui.zig(1 hunks)src/sys/Service.zig(1 hunks)src/sys/sysimpl.zig(2 hunks)src/test.zig(5 hunks)src/test/guiplay.zig(3 hunks)src/types.zig(1 hunks)src/ui/lightning.zig(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (17)
- .gitignore
- lib/ini
- src/lightning/LndConf.zig
- build.zig.zon
- lib/nif/build.zig
- src/nd/Config.zig
- src/types.zig
- src/sys/sysimpl.zig
- src/nd.zig
- .github/workflows/ci.yml
- src/sys/Service.zig
- src/ui/lightning.zig
- src/ngui.zig
- src/nd/Daemon.zig
- src/nd/network.zig
- src/test/guiplay.zig
- src/test.zig
🔇 Additional comments (5)
build.zig (5)
34-47: Module+executable wrapper for ngui looks correct for Zig 0.14Using createModule + root_module and wiring options/imports is idiomatic; pie and C integration are fine.
113-129: Daemon target migration is cleanModule-first approach, imports (nif/ini), and linking the static nif artefact all look correct for 0.14.
395-402: Correct API: OptimizeMode in LVGLLogLevel.defaultUpdate to std.builtin.OptimizeMode is spot-on for 0.14.
447-452: Correct Step.make signature for Zig 0.14Switch to fn make(_: *std.Build.Step, _: std.Build.Step.MakeOptions) is accurate.
56-56: No action needed: SDL draw sources are protected by#if LV_USE_GPU_SDLso they don’t pull in SDL headers or code when SDL is disabled.Likely an incorrect or invalid review comment.
1 participant
#24. Currently target is Zig 0.15, but there
usingnamespaceis removed, which it looks to me will require more complex changes, so prefer to do that in a separate PR later.Summary by CodeRabbit
New Requirements
Refactor
Tests
Documentation
Chores