Make Chilli more portable

Author: habedi

README.md

@@ -59,9 +59,9 @@ This command will download Chilli and add it to Zig's global cache and update yo
 Zig version supported by the main releases of Chilli:
 
 | Zig      | Chilli Tags |
-|----------|-------------|
-| `0.16.0` | `v0.3.x`    |
-| `0.15.x` | `v0.2.x`    |
+|----------|----------|
+| `0.16.0` | `v0.4.x` |
+| `0.15.x` | `v0.2.x` |
 
 The `main` branch normally tracks the latest (non-developmental) Zig release.
 

build.zig.zon

@@ -1,6 +1,6 @@
 .{
     .name = .chilli,
-    .version = "0.3.1",
+    .version = "0.4.0",
     .fingerprint = 0x6c259741ae4f5f73, // Changing this has security and trust implications.
     .minimum_zig_version = "0.16.0",
     .paths = .{

src/chilli/context.zig

@@ -1,18 +1,9 @@
 //! Provides the execution context for a command, giving access to parsed arguments and flags.
 const std = @import("std");
-const builtin = @import("builtin");
 const types = @import("types.zig");
 const command = @import("command.zig");
 const errors = @import("errors.zig");
 
-/// Returns true on targets where POSIX environment variable lookup is available.
-fn canLookupEnv() bool {
-    return switch (builtin.os.tag) {
-        .windows, .wasi, .emscripten, .freestanding, .other => false,
-        else => true,
-    };
-}
-
 /// (Private) Describes the source of a value for error reporting.
 const ValueSource = enum {
     parsed,
@@ -76,10 +67,14 @@ pub const CommandContext = struct {
     /// Retrieves the value for a flag, searching parsed values, then environment
     /// variables, and finally falling back to the default value.
     ///
-    /// NOTE: If the value comes from an environment variable, the returned slice
-    /// borrows directly from the process environment block (valid for the
-    /// lifetime of the process). If you need to hold it past an environment
-    /// change, copy it using `app_allocator`.
+    /// Memory model for env-var-sourced values: the raw string read from the
+    /// environment is allocated on `tmp_allocator`. For flags of type
+    /// `.String`, that allocation IS the returned value and stays alive until
+    /// `tmp_allocator`'s arena is released (in the normal `execute` flow,
+    /// this is when the `exec` function returns). For non-`.String` flags
+    /// the raw string is freed immediately after it has been parsed into a
+    /// primitive `FlagValue`. Tests that call `getFlag` directly should use
+    /// an arena-backed `tmp_allocator` to avoid manual cleanup.
     pub fn getFlag(self: *const CommandContext, name: []const u8, comptime T: type) errors.Error!T {
         // 1. Check for a parsed value from the command line.
         if (self.command.getFlagValue(name)) |parsed_value| {
@@ -91,14 +86,24 @@ pub const CommandContext = struct {
             std.debug.panic("Attempted to access an undefined flag: '{s}'", .{name});
 
         // 3. Check for a value from an environment variable.
-        // getPosix is only available on POSIX targets (not Windows/WASI/freestanding).
-        if (comptime canLookupEnv()) {
-            if (flag_def.env_var) |env_name| {
-                const environ = std.Options.debug_threaded_io.?.environ.process_environ;
-                if (std.process.Environ.getPosix(environ, env_name)) |env_val_str| {
-                    const env_value = try types.parseValue(flag_def.type, env_val_str);
-                    return castFlagValueTo(env_value, T, "flag", name, .environment);
-                }
+        // `Environ.getAlloc` is cross-platform (POSIX and Windows): it
+        // returns an owned WTF-8 copy of the value or `EnvironmentVariableMissing`.
+        if (flag_def.env_var) |env_name| {
+            const environ = std.Options.debug_threaded_io.?.environ.process_environ;
+            if (environ.getAlloc(self.tmp_allocator, env_name)) |env_val_str| {
+                // For `.String` flags, env_val_str becomes the returned value
+                // and must stay allocated for the caller. For other types,
+                // `parseValue` copies into a primitive `FlagValue`, so we
+                // release the source string eagerly — this keeps lookups
+                // clean even when `tmp_allocator` is not an arena.
+                const keep_alloc = flag_def.type == .String;
+                defer if (!keep_alloc) self.tmp_allocator.free(env_val_str);
+                const env_value = try types.parseValue(flag_def.type, env_val_str);
+                return castFlagValueTo(env_value, T, "flag", name, .environment);
+            } else |err| switch (err) {
+                error.EnvironmentVariableMissing => {}, // fall through to default
+                error.OutOfMemory => return error.OutOfMemory,
+                error.InvalidWtf8 => return error.InvalidWtf8,
             }
         }
 
@@ -340,17 +345,11 @@ test "context: getArgsAs for typed variadic" {
     try testing.expectError(error.InvalidCharacter, ctx_int.getArgsAs(i64, "numbers", allocator));
 }
 
-test "context: getFlag reads env var via debug_threaded_io environ" {
-    // Verify that the Environ.getPosix lookup through debug_threaded_io
-    // can find a real environment variable (PATH is always set on POSIX).
-    //
-    // `canLookupEnv()` in context.zig returns false on Windows/WASI/etc.,
-    // so the lookup is skipped and getFlag falls back to the default on
-    // those targets. This test only exercises the POSIX lookup path; the
-    // sibling tests above and below cover the default-fallback path that
-    // is what Windows runs through unconditionally.
-    if (comptime !canLookupEnv()) return;
-
+test "context: getFlag reads env var on all platforms via Environ.getAlloc" {
+    // Regression: env-var lookup previously used the POSIX-only
+    // `Environ.getPosix`, so Windows silently fell through to the default.
+    // `Environ.getAlloc` works on both POSIX and Windows, and this test
+    // covers both (PATH is set on both).
     const allocator = testing.allocator;
     var cmd = try command.Command.init(allocator, .{ .name = "test", .description = "", .exec = dummyExec });
     defer cmd.deinit();
@@ -363,14 +362,55 @@ test "context: getFlag reads env var via debug_threaded_io environ" {
         .description = "",
     });
 
-    const ctx = CommandContext{ .app_allocator = allocator, .tmp_allocator = allocator, .command = cmd, .data = null };
+    // For `.String` flags sourced from an env var, the returned slice lives
+    // on `tmp_allocator`. Back it with an arena here so the test does not
+    // have to guess the origin of the slice it owns. This mirrors how the
+    // normal `execute` flow wires up `tmp_allocator`.
+    var arena = std.heap.ArenaAllocator.init(allocator);
+    defer arena.deinit();
+    const ctx = CommandContext{
+        .app_allocator = allocator,
+        .tmp_allocator = arena.allocator(),
+        .command = cmd,
+        .data = null,
+    };
 
-    // PATH is always set; getFlag should return its value, not the default
+    // PATH is set on both POSIX and Windows; getFlag should return its
+    // value, not the default.
     const path_value = try ctx.getFlag("search-path", []const u8);
     try testing.expect(path_value.len > 0);
     try testing.expect(!std.mem.eql(u8, path_value, "fallback"));
 }
 
+test "regression: env-var Int parse failure does not leak source string" {
+    // For non-`.String` env-var flags, `getFlag` parses the raw string into
+    // a primitive `FlagValue` and must then free the source. This test
+    // forces a parse failure (PATH is not a valid integer) and relies on
+    // `testing.allocator` to flag any leak.
+    const allocator = testing.allocator;
+    var cmd = try command.Command.init(allocator, .{ .name = "test", .description = "", .exec = dummyExec });
+    defer cmd.deinit();
+
+    try cmd.addFlag(.{
+        .name = "bad-number",
+        .type = .Int,
+        .default_value = .{ .Int = 0 },
+        .env_var = "PATH", // always set, never a valid integer
+        .description = "",
+    });
+
+    const ctx = CommandContext{
+        .app_allocator = allocator,
+        .tmp_allocator = allocator,
+        .command = cmd,
+        .data = null,
+    };
+
+    // Parse should fail with InvalidCharacter; the source string allocated
+    // on `tmp_allocator` must be freed before the error propagates.
+    try testing.expectError(error.InvalidCharacter, ctx.getFlag("bad-number", i64));
+}
+
 test "context: getFlag env var lookup returns default for unset var" {
     const allocator = testing.allocator;
     var cmd = try command.Command.init(allocator, .{ .name = "test", .description = "", .exec = dummyExec });

src/chilli/errors.zig

@@ -32,4 +32,8 @@ pub const Error = error{
     RequiredArgumentAfterOptional,
     /// A command was defined with an empty string as an alias.
     EmptyAlias,
+    /// An environment variable name or value was not valid WTF-8 on Windows.
+    /// Unreachable on POSIX targets; included so Windows callers of
+    /// `getFlag` with an `env_var`-backed flag can handle it uniformly.
+    InvalidWtf8,
 } || std.fmt.ParseIntError || std.fmt.ParseFloatError || std.mem.Allocator.Error;