refactor(zigcli): upgrade dependencies to Zig 0.16#81
refactor(zigcli): upgrade dependencies to Zig 0.16#81jiacai2050 wants to merge 7 commits intomainfrom
Conversation
Migrate all binaries to std.process.Init API. Replace deprecated std.process.Child.run with direct std.Io handling. Refactor zfetch to use libc file wrappers due to syscall removals. Update structargs, progress, and term modules for std.Io. Remove curl dependency and zigfetch binary.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the project to Zig 0.16.0, which involves a significant refactor of I/O operations across all binaries and modules to use the new std.Io and std.process.Init APIs. Key changes include the removal of the zigfetch utility and its curl dependency, a complete rewrite of the tcp-proxy to utilize std.Io.Group for asynchronous networking, and the introduction of libc-based file helpers in zfetch to simplify absolute path access. Review feedback identifies a critical compilation error in tree.zig regarding ArrayList usage, a potential runtime failure in loc.zig when encountering lines exceeding the fixed 4KB buffer, and opportunities to optimize memory usage by removing redundant string duplications and improving buffer management in tcp-proxy.zig.
| } | ||
|
|
||
| var files: std.ArrayList(fs.Dir.Entry) = .empty; | ||
| var files: std.ArrayList(OwnedEntry) = .empty; |
There was a problem hiding this comment.
The variable files is declared as std.ArrayList(OwnedEntry), but it is initialized with .empty and used with append(local, ...) at line 216. In Zig, std.ArrayList is a managed container that does not have an .empty field and its append method does not take an allocator. This pattern is specific to std.ArrayListUnmanaged. This will cause a compilation error.
var files: std.ArrayListUnmanaged(OwnedEntry) = .empty;
| var rdr = file.reader(io, &buf); | ||
| while (true) { | ||
| const line = rdr.interface.takeDelimiterExclusive('\n') catch |e| { | ||
| switch (e) { | ||
| error.EndOfStream => return, | ||
| else => { | ||
| std.log.err("Error when seek line delimiter, name:{s}, err:{any}", .{ basename, e }); | ||
| return e; | ||
| }, | ||
| } | ||
| }, | ||
| else => { | ||
| var ptr = try std.posix.mmap( | ||
| null, | ||
| file_size, | ||
| std.posix.PROT.READ, | ||
| .{ .TYPE = .PRIVATE }, | ||
|
|
||
| file.handle, | ||
| 0, | ||
| ); | ||
| defer std.posix.munmap(ptr); | ||
|
|
||
| var offset_so_far: usize = 0; | ||
| while (offset_so_far < ptr.len) { | ||
| var line_end = offset_so_far; | ||
| while (line_end < ptr.len and ptr[line_end] != '\n') { | ||
| line_end += 1; | ||
| } | ||
| const line = ptr[offset_so_far..line_end]; | ||
| offset_so_far = line_end + 1; | ||
| }; | ||
|
|
||
| state = updateLineType(state, line, lang, loc_entry); | ||
| } | ||
| }, | ||
| state = updateLineType(state, line, lang, loc_entry); | ||
| } |
There was a problem hiding this comment.
The use of std.Io.Reader.takeDelimiterExclusive with a fixed 4KB buffer (buf) is not robust for a line-counting tool. If any line in the source file exceeds 4096 bytes, the reader will return error.StreamTooLong, causing the program to fail. The previous implementation using mmap was more robust and efficient. Consider using a larger/dynamic buffer or reverting to mmap (available in std.posix) to handle files with long lines correctly.
| const rel_path_owned = try allocator.dupe(u8, rel_path); | ||
| defer allocator.free(rel_path_owned); |
There was a problem hiding this comment.
Duplicating rel_path into rel_path_owned is redundant. The local_arena used to allocate rel_path is deinitialized via defer at line 319, which occurs only after the function returns. Therefore, rel_path remains valid during the recursive walk call at line 350. Furthermore, gi_stack.tryPushDir (line 345) performs its own internal duplication of the path string, so it does not require the caller to provide an owned copy.
| fn deinit(self: Proxy, io: std.Io) void { | ||
| self.source.close(io); | ||
| self.remote.close(io); | ||
| self.allocator.free(self.src_to_remote.ptr[0 .. self.src_to_remote.len + self.remote_to_src.len]); |
There was a problem hiding this comment.
The deallocation logic in deinit is fragile as it assumes src_to_remote and remote_to_src are contiguous slices from a single allocation performed in init. While this is currently true, it creates a hidden dependency between the two methods. A safer and more idiomatic approach would be to store the original buffer slice in the Proxy struct and free it directly.
There was a problem hiding this comment.
Pull request overview
This PR migrates the zigcli codebase to Zig 0.16 by updating argument parsing, I/O usage, and build wiring, while removing deprecated process helpers and the curl/zigfetch dependency path.
Changes:
- Updated core modules (
structargs,term,progress,pretty-table,gitignore) to usestd.Io-based APIs and Zig 0.16 process init/args patterns. - Migrated multiple CLI binaries and examples to
main(init: std.process.Init)and replaced removed stdlib APIs (Child.run,std.Thread.sleep,std.fs-centric I/O). - Refactored zfetch platform code to avoid
@cImportissues and removed the curl dependency +zigfetchbuild step.
Reviewed changes
Copilot reviewed 11 out of 26 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/term.zig | Switches TTY detection/terminal width logic to std.Io.File + ioctl-based approach. |
| src/structargs.zig | Updates parser to accept std.Io + std.process.Args, and adapts output to new writer APIs. |
| src/progress.zig | Refactors progress rendering to std.Io writers and replaces timer with an std.Io.Clock-backed helper. |
| src/pretty-table.zig | Migrates writer types and test output capture to std.Io.Writer helpers. |
| src/gitignore.zig | Updates directory/file reading to std.Io.Dir APIs and threads Io through tryPushDir. |
| src/bin/zfetch/macos.zig | Replaces @cImport with manual extern declarations and libc dir iteration for Zig 0.16 compatibility. |
| src/bin/zfetch/common.zig | Introduces libc-based file reading helpers and removes Child.run usage. |
| src/bin/zfetch.zig | Converts to main(init) style, threads io, and updates env access + stdout writer creation. |
| src/bin/util.zig | Replaces std.once verbose enable hook with a simple callable struct. |
| src/bin/tree.zig | Migrates filesystem walking to std.Io.Dir iteration and main(init) entrypoint. |
| src/bin/timeout.zig | Updates to main(init), switches to new spawn/wait APIs, and updates signal handler wiring. |
| src/bin/tcp-proxy.zig | Refactors networking to std.Io.net and structured concurrency primitives. |
| src/bin/repeat.zig | Updates process execution + sleeping to std.Io APIs and main(init). |
| src/bin/progress-it.zig | Threads io through output and replaces std.Thread.sleep with std.Io.sleep. |
| src/bin/pretty-csv.zig | Updates file/stdin reading to std.Io APIs and adjusts stdout writer creation. |
| src/bin/pidof.zig | Updates parsing/output to main(init) and std.Io.File writers; uses std.process.exit. |
| src/bin/night-shift.zig | Updates to main(init) and std.Io.File writer usage. |
| src/bin/loc.zig | Migrates dir walking and file reading to std.Io APIs; removes mmap-based path. |
| src/bin/dark-mode.zig | Updates to main(init) and new structargs parse signature. |
| src/bin/cowsay.zig | Migrates parsing/output to main(init) and uses std.Io.Writer.fixed for message assembly. |
| examples/structargs-demo.zig | Updates example to main(init) and new structargs + writer APIs. |
| examples/progress-demo.zig | Updates example to pass io into progress options and uses std.Io.sleep. |
| examples/pretty-table-demo.zig | Migrates stdout writer creation to std.Io.File and main(init). |
| build.zig.zon | Bumps minimum Zig version to 0.16.0 and removes curl dependency. |
| build.zig | Updates module registration/linking for Zig 0.16 build APIs and removes zigfetch wiring. |
| .tool-versions | Pins toolchain to Zig 0.16.0. |
| /// Implemented directly via `TIOCGWINSZ` to avoid requiring an `Io` instance. | ||
| pub fn isTty(file: std.Io.File) bool { | ||
| if (builtin.os.tag == .windows) { | ||
| return false; |
There was a problem hiding this comment.
isTty now unconditionally returns false on Windows, which will disable ANSI/progress output and any other TTY-dependent behavior on that platform (previously std.fs.File.isTty() handled Windows). Consider adding a Windows implementation (e.g., using console handle checks / GetConsoleMode, or calling an std.Io-based isTty helper when available) instead of hardcoding false.
| /// Implemented directly via `TIOCGWINSZ` to avoid requiring an `Io` instance. | |
| pub fn isTty(file: std.Io.File) bool { | |
| if (builtin.os.tag == .windows) { | |
| return false; | |
| /// Implemented directly via platform APIs to avoid requiring an `Io` instance. | |
| pub fn isTty(file: std.Io.File) bool { | |
| if (builtin.os.tag == .windows) { | |
| var mode: std.os.windows.DWORD = undefined; | |
| return std.os.windows.kernel32.GetConsoleMode(file.handle, &mode) != 0; |
Refactor process and file scanning to consistently use the std.Io interface instead of std.fs and direct file APIs. This prepares for enhanced testability and future portability, following Zig's latest async I/O designs. Adds curl dependency for future network functionality. Minor FreeBSD uptime and sysctl buffer trimming fixes for better accuracy.
Disallow compiling mach source when host_os differs from target_os. This avoids build failures caused by unsupported opaque types on Ubuntu targeting macOS. The change ensures compatibility checks are stricter, preventing known build issues for developers.
| .dependencies = .{ | ||
| .curl = .{ | ||
| .url = "https://github.com/jiacai2050/zig-curl/archive/refs/tags/v0.3.2.tar.gz", | ||
| .hash = "curl-0.3.2-P4tT4SXPAACuV6f5eyh4jG_1SspjWwMm_vRJfoKrQep5", | ||
| .url = "https://github.com/jiacai2050/zig-curl/archive/refs/tags/v0.5.0.tar.gz", | ||
| .hash = "curl-0.5.0-P4tT4e_-AABZBa1zBmiSKFQo7xV0DJ2cUxFfH4KiQKj2", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
PR description mentions removing the curl dependency, but build.zig.zon still declares curl (and now upgrades it). Since build.zig no longer consumes this dependency, this is now unused extra fetch/build work; either remove it from build.zig.zon or update the PR description to reflect the new plan.
Switch tests to use std.testing.io instead of global_single_threaded.io to improve reliability and consistency in test environments. Also, optimize file reading for non-Windows platforms by using mmap, which reduces allocations and increases performance when processing large files. This change simplifies future test maintenance and speeds up code analysis in non-Windows OS.
Update zigfetch to use the new std.Io filesystem and process APIs, replacing usages of fs and related legacy APIs. This includes changes for directory/file operations, random number generation, and environment variable access. "zigfetch" is now supported in the build system, and the Zigfetch binary links against libcurl. This change improves compatibility with upcoming Zig stdlib design, better aligns with Zig's evolving ecosystem, and makes the codebase more portable across platforms.
Ensure test steps receive the optimization mode parameter. This change improves consistency between build and test steps, allowing tests to run under the same optimization settings as builds. It prepares the codebase for more flexible test configurations.
Migrate all binaries to std.process.Init API.
Replace deprecated std.process.Child.run with direct std.Io handling. Refactor zfetch to use libc file wrappers due to syscall removals. Update structargs, progress, and term modules for std.Io. Remove curl dependency and zigfetch binary.