Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

std.process.Child is not useable for Linux libraries not linking libc even when an env_map is provided. #23372

Open
jsirois opened this issue Mar 26, 2025 · 2 comments · May be fixed by #23380 or #23402
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@jsirois
Copy link

jsirois commented Mar 26, 2025

Zig Version

0.14.0

Steps to Reproduce and Observed Behavior

Given example.zig:

const std = @import("std");

export fn check_call() void {
    const allocator = std.heap.page_allocator;
    const output = collect_output(
        allocator,
        &.{"bash", "-c", "echo $Slartibartfast"},
    ) catch "Failure!";
    defer allocator.free(output);
    std.debug.print("{s}", .{output});
}

pub fn collect_output(allocator: std.mem.Allocator, argv: []const []const u8) ![]const u8 {
    var env_map = std.process.EnvMap.init(allocator);
    defer env_map.deinit();
    try env_map.put("Slartibartfast", "fjords");

    const result = try std.process.Child.run(
        .{ .allocator = allocator, .argv = argv, .env_map = &env_map },
    );
    errdefer allocator.free(result.stdout);
    allocator.free(result.stderr);

    switch (result.term) {
        .Exited => |code| {
            if (code == 0) {
                return result.stdout;
            } else {
                return error.NonZeroExit;
            }
        },
        else => return error.Failure,
    }
}

Try:

# Ok:
:; zig build-lib -dynamic -target x86_64-windows example.zig

# Ok:
:; zig build-lib -dynamic -target x86_64-macos example.zig

# Not ok:
:; zig build-lib -dynamic -target x86_64-linux example.zig
/home/jsirois/.local/opt/zig-linux-x86_64-0.14.0/lib/std/process/Child.zig:605:13: error: missing std lib enhancement: ChildProcess implementation has no way to collect the environment variables to forward to the child process
            @compileError("missing std lib enhancement: ChildProcess implementation has no way to collect the environment variables to forward to the child process");
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
referenced by:
    spawn: /home/jsirois/.local/opt/zig-linux-x86_64-0.14.0/lib/std/process/Child.zig:247:31
    run: /home/jsirois/.local/opt/zig-linux-x86_64-0.14.0/lib/std/process/Child.zig:425:20
    7 reference(s) hidden; use '-freference-trace=9' to see all references

Expected Behavior

See https://github.com/ziglang/zig/pull/7553/files#r2014522673 where the originating issue is identified in a 2020 change. The same code exists today though:

const envp: [*:null]const ?[*:0]const u8 = m: {
const prog_fd: i32 = if (prog_pipe[1] == -1) -1 else prog_fileno;
if (self.env_map) |env_map| {
break :m (try process.createEnvironFromMap(arena, env_map, .{
.zig_progress_fd = prog_fd,
})).ptr;
} else if (builtin.link_libc) {
break :m (try process.createEnvironFromExisting(arena, std.c.environ, .{
.zig_progress_fd = prog_fd,
})).ptr;
} else if (builtin.output_mode == .Exe) {
// Then we have Zig start code and this works.
// TODO type-safety for null-termination of `os.environ`.
break :m (try process.createEnvironFromExisting(arena, @ptrCast(std.os.environ.ptr), .{
.zig_progress_fd = prog_fd,
})).ptr;
} else {
// TODO come up with a solution for this.
@compileError("missing std lib enhancement: ChildProcess implementation has no way to collect the environment variables to forward to the child process");
}
};

It seems like the compile error is too aggressive, disallowing legitimate use of the Child API by passing in a hand crafted env_map. Presumably the else branch could even optimistically use std.os.environ since it can be set by things other than the zig start code used for exes. In my case, I actually do this! I naively tried passing in env_map though to work around, and then realized what I thought was a compiler bug handling mixed comptime-known / runtime-known if / else branches was actually correct compiler behavior - this just appeared to be an API decision. I'm hoping that decision can be changed to make this API more widely usable.

I'm happy to take this issue up if the idea of populating env from std.os.environ sounds reasonable. Perhaps paired with stdlib docs updates to point out the caveats. I guess this would require an API for setting std.os.environ since its status is currently inherently ambiguous via undefined instead of optional. Maybe a paired bool flag to indicate it was populated by startup code? Or, if breaking existing users is OK, maybe just switch to an optional since std.os.environ truly is optional.

@jsirois jsirois added the bug Observed behavior contradicts documented or intended behavior label Mar 26, 2025
@alexrp alexrp added the standard library This issue involves writing Zig code for the standard library. label Mar 26, 2025
@alexrp alexrp added this to the 0.15.0 milestone Mar 26, 2025
@alexrp
Copy link
Member

alexrp commented Mar 26, 2025

Related: #4524

@jsirois
Copy link
Author

jsirois commented Mar 26, 2025

I'm going to take a whack at the pub var environ: ?[][*:0]u8 = null; idea since it does seem to ~match the referenced:

zig/lib/std/os/linux.zig

Lines 482 to 483 in 263ba34

/// Set by startup code, used by `getauxval`.
pub var elf_aux_maybe: ?[*]std.elf.Auxv = null;

It does break ~800 known uses though: https://github.com/search?q=os.environ++path%3A*.zig&type=code&ref=advsearch

I realize I may be wasting my time here in terms of this getting accepted, but I could use the practice trying to make a zig contribution (zig-bootstrap, etc.).

jsirois added a commit to jsirois/zig that referenced this issue Mar 27, 2025
This makes their status with respect to start code clear and also allows
alternative start code to set them up.

Fixes ziglang#23372
Affects ziglang#4524
@jsirois jsirois linked a pull request Mar 27, 2025 that will close this issue
jsirois added a commit to jsirois/zig that referenced this issue Mar 29, 2025
Move processing of std.os.environ centrally to std.os and provide a
hook for discovering the raw environ block for libraries that don't
link libc.

Fixes ziglang#23372
Affects ziglang#4524
@jsirois jsirois linked a pull request Mar 29, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants