Revert "Move to Zig 0.14" #44
Conversation
This reverts commit 83cb38d. Upgrade to Zig 0.14 has caused random GUI crashes. This is not a proper solution to a problem, but let's revert for now and then work on a proper fix.
WalkthroughThe PR updates Zig tooling and migrates code to Zig 0.12 APIs. It revises build scripts to use new executable-centric APIs, adjusts std library paths (std.ChildProcess, tokenisation, type info), updates CI and README Zig versions, modifies tests and signal handling to propagate errors, and refreshes a submodule pointer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant nd as nd (daemon)
participant OS as OS
participant ngui as ngui (GUI)
User->>nd: start
nd->>nd: std.ChildProcess.init([...])
nd->>ngui: spawn()
activate ngui
ngui->>OS: register SIGINT/SIGTERM handlers (try)
OS-->>nd: SIGINT/SIGTERM
nd->>nd: sigaction handlers (try)
nd-->>ngui: signal/terminate child
ngui-->>nd: exit (Term)
deactivate ngui
nd-->>User: exit/cleanup
note over nd,ngui: Errors during sigaction/ spawn now propagate
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
It is causing errors ("sudo apt-get install -y libsdl2-dev" is failing).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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: Explicitly disable silent password truncation in bcrypt verificationUse the
.silently_truncate_password = falseflag to prevent truncated inputs from being accepted:- std.crypto.pwhash.bcrypt.strVerify(slock.bcrypt_hash, input, .{}) catch |err| { + std.crypto.pwhash.bcrypt.strVerify( + slock.bcrypt_hash, + input, + .{ .silently_truncate_password = false }, + ) catch |err| {
🧹 Nitpick comments (4)
src/ngui.zig (1)
448-450: Failing hard on sigaction errors – confirm this intenttry will abort on handler registration failure. If you prefer to continue without handlers, catch and log instead.
Example:
- try posix.sigaction(posix.SIG.INT, &sa, null); - try posix.sigaction(posix.SIG.TERM, &sa, null); + posix.sigaction(posix.SIG.INT, &sa, null) catch |err| logger.warn("sigaction INT: {any}", .{err}); + posix.sigaction(posix.SIG.TERM, &sa, null) catch |err| logger.warn("sigaction TERM: {any}", .{err});src/nd.zig (1)
227-229: Sigaction now aborts on errorSame note as in ngui: confirm you want to fail early; otherwise catch and log to continue.
See suggested diff in src/ngui.zig comment.
src/test/guiplay.zig (1)
382-384: Sigaction: consider non‑fatal handlingIf handler install fails, try will abort the test harness. Consider logging and continuing if that’s acceptable.
build.zig (1)
430-435: Approve signature change & recommend pinning Zig version
- VersionStep.make signature change is correct: Progress node parameter matches older Build API.
- No minimum_zig_version found in build.zig.zon or CI; add a minimum_zig_version and pin the Zig toolchain to 0.12.x in CI to avoid accidental upgrades.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.github/workflows/ci.yml(1 hunks).gitignore(0 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 with no reviewable changes (1)
- .gitignore
⏰ 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 (53)
lib/ini (1)
1-1: Submodule pointer revert looks sound.The lib/ini pointer now matches the pre-Zig-0.14 state, aligning with the revert objective. No issues spotted.
README.md (1)
15-15: LGTM: Documentation correctly updated for Zig 0.12.xThe toolchain version requirement is properly downgraded to match the revert, aligning with the CI workflow change to Zig 0.12.1.
lib/nif/build.zig (1)
8-16: LGTM: Build script correctly adapted for Zig 0.12.x APIThe changes properly migrate from module-based macro definitions to library-level macro definitions using
defineCMacro, maintaining functional equivalence.build.zig.zon (1)
2-2: LGTM: Zone file format updated for Zig 0.12.xThe name field correctly changes from a symbol literal to a string, and the fingerprint field removal aligns with Zig 0.12 zone file format.
.github/workflows/ci.yml (2)
18-18: LGTM: Zig version correctly downgraded to 0.12.1The CI workflow properly reflects the toolchain revert.
23-27: LGTM: SDL2 build step added for comprehensive testingEnabling the SDL2 build path alongside X11 provides better test coverage across different driver implementations, which is beneficial given the GUI crash issues mentioned in the PR description.
src/nd/network.zig (1)
127-127: LGTM: Tokenization calls correctly updated for Zig 0.12The migration from
tokenizeScalartotokenizewith string delimiters ("\n", "\t") maintains functional equivalence whilst conforming to Zig 0.12 API.Also applies to: 160-160, 167-167
src/nd/Daemon.zig (3)
154-154: LGTM: Options field access updated for Zig 0.12The change to use
.listdirectly relies on type inference, which is consistent with Zig 0.12 patterns.
700-702: LGTM: Tokenization API correctly updatedThe migration from
tokenizeScalartotokenizeand fromsplitScalartosplitwith string delimiters is correct for Zig 0.12.
865-865: LGTM: Type introspection updated for Zig 0.12 APIThe use of
@typeInfo(@TypeOf(lndrep.channels)).Pointer.childprovides explicit type information access, which is the correct pattern for Zig 0.12.src/sys/sysimpl.zig (2)
50-50: LGTM: Options type inference updated for Zig 0.12Removing the explicit type annotation and relying on inference is consistent with Zig 0.12 API patterns.
75-75: LGTM: Test updated for Zig 0.12 writeFile APIThe simplified
writeFilecall with direct positional arguments (path, data) correctly reflects the Zig 0.12 API.src/types.zig (1)
25-25: ChildProcess API updated consistently. No remainingstd.process.Childreferences; allChildProcess.initcalls now target the Zig 0.12std.ChildProcessAPI.src/ui/lightning.zig (1)
17-38: ComptimeStringMap usage looks goodInitialisation and later get() usage are correct; value type matches setTextStatic.
src/lightning/LndConf.zig (1)
232-242: Two‑argument Dir.writeFile looks correctMatches the pre‑0.14 API and keeps the test concise.
src/nd.zig (1)
173-173: std.ChildProcess init aligns with the rest of the codebaseInitialisation and behaviour fields are consistent. No issues spotted.
src/test/guiplay.zig (2)
11-11: Switch to std.ChildProcess is consistentType change matches production code; good consistency.
361-368: Child process init and spawn flow looks correctPipes set, spawn error handled via fatal. LGTM.
src/nd/Config.zig (12)
203-207: TypeInfo path revert for safeReadOnly is fineUsing .Fn.return_type.? matches pre-0.14 layout; function semantics unchanged.
237-241: Good: HashOptions explicitly disables silent truncationTop-level .silently_truncate_password = false is clear and correct for hashing.
311-315: Type inference for BufferedAtomicFile options LGTMUsing const opt = .{ .mode = 0o600 } keeps it concise with no behaviour change.
355-367: Cron script file mode initialisation looks goodMode 0o755 via inferred opt is fine.
374-389: std.ChildProcess.run usage is appropriateAllocator ownership and freeing of stdout/stderr handled correctly; exit/term checks preserved.
535-543: Test setup change to writeFile with literal path looks fineSimplifies the test; semantics unchanged.
605-606: Creating empty conf.json via writeFile is fineMatches the updated test helper usage.
639-642: Embedded shell script fixture in tests LGTMExecutable bit set just below; end-to-end run asserts remain valid.
670-671: json.parseFromSlice options literal is correctStays equivalent to the earlier named type; clearer and concise.
749-753: Minimal INI content for mutate test is fineAdequate fixture for exercising persist/finish.
790-797: Config file without slock field test fixture LGTMCovers the null/absent slock cases well.
808-816: Config file with null slock test fixture LGTMGood coverage for the disabled state.
build.zig (13)
34-46: Switch to addExecutable with explicit .name is correctMatches the older Build API; flags and properties preserved.
42-46: PIE and module/import wiring LGTMSetting pie and build_options import through root_module is correct for 0.12-era Build.
54-54: C sources: LVGL generic list wiring is OKFlags array usage is correct; no functional changes.
61-70: UI C sources added via root + files looks goodConsistent with Build API expectations.
72-79: C macro definitions consistent and clearMix of root_module.addCMacro/defineCMacro works; values correct.
81-85: Driver-specific wiring (SDL2/X11/FBDEV) looks correctSource lists and system libraries are correctly gated by the selected driver.
Confirm LVGL SDL draw sources in lvgl_generic_src don’t require SDL headers when SDL2 driver isn’t selected (to avoid build failures without SDL dev headers). If needed, move those files under the SDL2 case.
Also applies to: 87-97, 99-103
108-108: Install artefact dependency is correctDepend on addInstallArtifact step is the right pattern here.
110-122: nd executable and install step wiring LGTMImports/libraries and PIE setting preserved; install step dependency set.
Also applies to: 125-125
128-134: addTest usage with filter option is correctMatches the reverted API; linking and imports are consistent.
147-154: guiplay executable wiring LGTMImporting comm via createModule is fine; install dependency set.
Also applies to: 156-157
162-170: btcrpc executable wiring LGTMModule import and install step look correct.
Also applies to: 172-172
177-185: lndhc executable wiring LGTMModule import and install dependency are correct.
Also applies to: 187-187
379-385: LVGLLogLevel.default signature updated correctlyUsing std.builtin.Mode aligns with reverted toolchain.
src/test.zig (9)
109-112: TestChildProcess callback signatures updated correctlyCallbacks now use std.ChildProcess types; aligns with the reverted API.
136-142: spawn signature and behaviour LGTMReturns std.ChildProcess.SpawnError!void and preserves hook semantics.
143-150: wait return type and default term LGTManyerror!std.ChildProcess.Term is appropriate; default Exited=0 retained.
151-155: spawnAndWait updated correctlyComposition over updated spawn/wait signatures works.
156-163: kill return type updated correctlyConsistent with ChildProcess.Term usage.
239-276: TypeInfo case names reverted correctly (.Pointer/.One/.Slice)Slice and pointer handling matches 0.12-era TypeInfo names.
276-284: Struct case handling is correctField-wise deep equality remains intact.
284-298: Optional case handling is correctClear mismatch diagnostics; semantics preserved.
299-311: Union case handling matches reverted TypeInfoTag extraction and payload comparison intact.
src/sys/Service.zig (1)
40-43: Reverting to std.ChildProcess.Term is correct
Aliasing verified: in src/types.zig,ChildProcessresolves tostd.ChildProcessin production, matching the union payload in Service.zig. No behavioural changes implied.
| const resp = try wpa_ctrl.request("STATUS", &buf, null); | ||
| const ssid = "ssid="; | ||
| var it = mem.tokenizeScalar(u8, resp, '\n'); | ||
| var it = mem.tokenize(u8, resp, "\n"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify tokenization API migration across the codebase
The change from tokenizeScalar to tokenize with string delimiters is correct for Zig 0.12. However, ensure all tokenization call sites have been updated consistently throughout the codebase.
Run the following script to verify all tokenization calls have been migrated:
🏁 Script executed:
#!/bin/bash
# Description: Check for any remaining tokenizeScalar usage
# Search for tokenizeScalar usage
echo "Checking for remaining tokenizeScalar calls:"
rg -nP '\btokenizeScalar\b' --type=zig
# Search for tokenize with character literals (should be strings now)
echo "Checking for tokenize with character literals:"
rg -nP "tokenize\([^,]+,\s*'[^']*'\)" --type=zigLength of output: 326
Update remaining tokenizeScalar calls
Replace the following in src/nd/Config.zig with std.mem.tokenize(u8, …, "\n") or "\ " as appropriate:
- Line 125:
std.mem.tokenizeScalar(u8, bytes, '\n') - Line 137:
std.mem.tokenizeScalar(u8, s, ' ') - Line 185:
std.mem.tokenizeScalar(u8, conf, '\n')
🤖 Prompt for AI Agents
In src/nd/network.zig around line 107 and in src/nd/Config.zig at lines 125,
137, and 185, replace usages of std.mem.tokenizeScalar(u8, ..., '\n' or ' ')
with the non-scalar tokenize API: use std.mem.tokenize(u8, bytes, "\n") for
newline splits and std.mem.tokenize(u8, s, " ") for space splits (or "\" \"" if
your codebase prefers explicit escaped space), ensuring the third argument is a
string slice not a character literal and update any variable names/types
accordingly to match the iterator returned by std.mem.tokenize.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
18-18: Rollback to Zig 0.12.1 in CI looks correct; add an explicit version check step.This aligns with the revert objective. To make failures clearer and aid debugging, print the Zig version right after setup.
- name: Set up Zig uses: goto-bus-stop/setup-zig@v2 with: version: '0.12.1' + - name: Verify Zig version + run: zig version
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/ci.yml(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
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 (3)
src/nd/Daemon.zig (2)
134-168: Fix error-path leak after toOwnedSlice() in init()If a later fallible call (e.g., Timer.start) fails after Line 154, the owned slice is leaked and services aren’t deinit’ed. Guard the owned slice with an errdefer that deinits services and frees the slice.
@@ try svlist.append(sys.Service.init(opt.allocator, sys.Service.BITCOIND, .{ .stop_wait_sec = 600 })); logger.debug("conf = {any}", .{opt.conf}); - return .{ + // take ownership of the services list with cleanup on later failure + var services_list = try svlist.toOwnedSlice(); + errdefer { + for (services_list) |*sv| sv.deinit(); + opt.allocator.free(services_list); + } + + return .{ .allocator = opt.allocator, @@ - .screenstate = if (opt.conf.data.slock != null) .locked else .unlocked, - .services = .{ .list = try svlist.toOwnedSlice() }, + .screenstate = if (opt.conf.data.slock != null) .locked else .unlocked, + .services = .{ .list = services_list }, // send persisted settings immediately on start
700-711: Use kv.next() with mem.split and avoid kv.first()mem.split iterators expose next(), not first(). Also avoids subtle iterator misuse.
- var it = mem.tokenize(u8, m, " "); - while (it.next()) |kv_str| { - var kv = mem.split(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; - } - } - } + var it = mem.tokenize(u8, m, " "); + while (it.next()) |kv_str| { + var kv = mem.split(u8, kv_str, "="); + const key = kv.next() orelse continue; + if (mem.eql(u8, key, "auth_failures")) { + const v = kv.next(); + if (v != null and !mem.eql(u8, v.?, "0")) { + self.wifiInvalidKey(); + break; + } + } + }src/test.zig (1)
299-311: Fix union tag comparison in expectDeepEqualCasting the union value to Tag is invalid; use std.meta.activeTag.
- const Tag = std.meta.Tag(@TypeOf(expected)); - const atag = @as(Tag, actual); - try std.testing.expectEqual(@as(Tag, expected), atag); + const Tag = std.meta.Tag(@TypeOf(expected)); + const atag = std.meta.activeTag(actual); + try std.testing.expectEqual(std.meta.activeTag(expected), atag);
🧹 Nitpick comments (6)
.github/workflows/ci.yml (2)
18-18: Pin to 0.12.1 is fine; ensure version guidance matches docsCI uses Zig 0.12.1 while README mentions 0.12.x. Consider aligning both (either pin README to 0.12.1 or keep CI at 0.12.x) to avoid confusion.
28-28: X11 build may require system packages on ubuntu-latestzig build -Ddriver=x11 can fail without X11 headers/libs. If CI flakes, add:
- sudo apt-get update && sudo apt-get install -y libx11-dev
README.md (1)
15-15: Match Zig version with CIDocs say v0.12.x while CI pins 0.12.1. Recommend aligning (e.g., “v0.12.1”) for clarity.
src/nd/Config.zig (2)
217-226: Avoid u8 wrap on incorrect_attemptsIf brute-forced long enough, u8 will wrap to 0. Saturate instead to preserve audit semantics.
Apply:
- self.data.slock.?.incorrect_attempts += 1; + if (self.data.slock.?.incorrect_attempts < std.math.maxInt(u8)) { + self.data.slock.?.incorrect_attempts += 1; + }
374-389: Child process run compatible with 0.12; include stdout in error logs?This is fine for 0.12. For diagnostics, consider logging stdout on failures too.
Apply:
- logger.err("runSysupdates: {s} exit code = {d}; stderr: {s}", .{ scriptpath, code, res.stderr }); + logger.err("runSysupdates: {s} exit code = {d}; stderr: {s}; stdout: {s}", .{ scriptpath, code, res.stderr, res.stdout });src/nd/Daemon.zig (1)
865-866: Prefer std.meta.Child for slice element typeSimpler and clearer than @typeinfo(...).Pointer.child.
- 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
📒 Files selected for processing (19)
.github/workflows/ci.yml(2 hunks).gitignore(0 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 with no reviewable changes (1)
- .gitignore
⏰ 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 (48)
lib/ini (1)
1-1: Submodule pointer rollback looks correctThe submodule now points to the earlier commit, matching the revert goal. No further action needed.
build.zig.zon (1)
2-2: LGTM: name literal compatible with 0.12Using a string literal for .name is correct for Zig 0.12; removing fingerprint is appropriate.
src/nd/Config.zig (12)
237-241: Bcrypt options look sensibleRounds 12, PHC encoding, and no silent truncation are good choices for 0.12.
310-315: Correct permissions for config file0o600 with BufferedAtomicFile is appropriate.
355-367: Cron script generation and perms look correct0o755 and atomic write are appropriate.
435-446: Wallet unlock file perms and ownership are correct0o400 plus chown to LND user is appropriate.
535-543: Tests: writeFile two-arg form is correct for 0.12Matches current Dir.writeFile signature.
605-606: Initialise empty config file in testGood setup for subsequent writes.
638-642: Executable test script content OKShebang and write to success file look fine.
669-671: JSON parse options align with 0.12 APISwitch to parseFromSlice with options is correct.
748-753: INI test: writeFile signature update is correctLGTM.
789-797: Config without slock: writeFile update is correctLGTM.
808-816: Config with slock=null: writeFile update is correctLGTM.
203-207: Approve .Fn.return_type change; no lingering Zig 0.14 patterns
No matches for @“fn” orstd.process.Child–.Fn.return_typeis correctly used.src/lightning/LndConf.zig (1)
232-242: Tests: Dir.writeFile two-arg usage matches 0.12Signature update is correct; test content unchanged.
src/sys/sysimpl.zig (2)
50-50: LGTM: Type inference simplification.The explicit type annotation has been removed, allowing the compiler to infer the type from the struct literal. This is cleaner and aligns with Zig 0.12 conventions.
75-75: LGTM: Simplified test helper API.The
writeFilecall has been updated to use the simpler two-argument form, which is consistent with Zig 0.12's API.lib/nif/build.zig (1)
8-16: LGTM: Build script simplification.The build configuration has been streamlined by defining C macros directly on the static library rather than through an intermediate module. This aligns with Zig 0.12's build API patterns.
src/types.zig (1)
25-25: LGTM: Core API migration.The public alias has been updated from
std.process.Childtostd.ChildProcess, which is the correct path for Zig 0.12. This change propagates consistently throughout the codebase.src/ngui.zig (1)
448-449: LGTM: Proper error handling for signal registration.Signal action registration now properly propagates errors, allowing
main()to return an error if signal setup fails. This is more robust than silently ignoring potential failures.src/ui/lightning.zig (1)
17-38: LGTM: Compile-time string map migration.The string map has been updated from
StaticStringMap(...).initComptime(...)toComptimeStringMap(...), which is the Zig 0.12 API. The mapping entries and behaviour remain unchanged.src/nd.zig (2)
173-173: LGTM: Process API migration.The child process initialisation has been updated to use
std.ChildProcess.init, consistent with the Zig 0.12 API and the type alias change insrc/types.zig.
227-228: LGTM: Proper error handling for signal registration.Signal action registration now properly propagates errors, consistent with the same change in
src/ngui.zig. This improves robustness by surfacing signal setup failures.src/sys/Service.zig (1)
40-42: LGTM: Process termination type migration.The
Stateunion fields have been updated to usestd.ChildProcess.Terminstead ofstd.process.Child.Term, consistent with the API migration throughout the codebase.src/test/guiplay.zig (3)
11-11: LGTM: Process type migration.The global
ngui_proctype has been updated tostd.ChildProcess, consistent with the API migration throughout the codebase.
361-367: LGTM: Process initialisation and error handling.The child process initialisation has been updated to use
std.ChildProcess.init, and spawn errors are now properly handled with a fatal error message.
382-383: LGTM: Proper error handling for signal registration.Signal action registration now properly propagates errors, consistent with the changes in production code (
src/ngui.zigandsrc/nd.zig).src/nd/network.zig (3)
107-116: LGTM: newline tokenisation updateString-based mem.tokenize improves API stability; logic unchanged.
127-131: LGTM: scan results header tokenisationHeader-skip logic preserved; string delimiter is correct.
160-176: LGTM: LIST_NETWORKS parsingUsing mem.tokenize with tab delimiter reads columns cleanly; behaviour matches prior code.
src/test.zig (5)
109-112: LGTM: callback type updates to std.ChildProcessSignatures align with the new process API.
136-142: LGTM: spawn signatureMatches std.ChildProcess.SpawnError.
143-149: LGTM: wait signature and default termReturns Term with Exited=0; OK.
151-155: LGTM: spawnAndWait signatureOK with updated types.
156-163: LGTM: kill signatureOK with updated types.
build.zig (13)
34-46: LGTM: ngui executable set-upExecutable-centric API usage is correct; includes and imports wired properly.
53-71: LGTM: C sources and flags for nguiFlags are sensible; source lists look coherent.
72-79: LGTM: LVGL and display macrosMacro definitions and display sizing are correctly parameterised.
80-105: LGTM: driver-specific wiringConditional sources, defines, and system libs look correct for SDL2/X11/FBDEV.
106-108: LGTM: ngui install stepInstall step correctly depends on artifact.
110-122: LGTM: nd executable set-upImports and linking for nif/ini look correct; PIE enabled.
123-125: LGTM: nd install stepWiring is correct.
128-134: LGTM: tests configurationaddTest with optional filter and libc link is set correctly.
147-158: LGTM: guiplay executable and installImport of comm via createModule is fine; install wiring OK.
162-173: LGTM: btcrpc playgroundModule import and install step are correct.
177-188: LGTM: lndhc playgroundModule import and install step are correct.
379-386: LGTM: LVGLLogLevel.default signatureUpdated to std.builtin.Mode correctly; defaults are sensible.
430-435: LGTM: VersionStep.make signatureMatches newer build API (Progress.Node). Behaviour unchanged.
|
Did some testing, even subset of #40 to migrate to Zig 0.13 already causes GUI crashes. With only changes being std.ChildProcess -> std.process.Child, std.ComptimeStringMap -> std.StaticStringMap, newer ini lib version, changed fn make(). So, nothing directly related to GUI. |
1 participant
This reverts commit 83cb38d (#40).
Upgrade to Zig 0.14 has caused random GUI crashes. This is not a proper solution to a problem, but let's revert for now and then work on a proper fix.
Summary by CodeRabbit