-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
std.process.Child
is not useable for Linux libraries not linking libc even when an env_map
is provided.
#23372
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
Comments
Related: #4524 |
I'm going to take a whack at the Lines 482 to 483 in 263ba34
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.). |
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
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
Did you happen to try to bisect this? I could look deeper to validate, but I think this actually broke more recently in 0.14 with how optionals are evaluated. |
@GrayHatter I did not. I did come to the conclusion this was probably not a compiler bug but an API decision - snipped from above:
So the conditional, in my case, boils down to the following at comptime: if (self.env_map) |env_map| {
break :m (try process.createEnvironFromMap(arena, env_map, .{
.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");
} Say the compiler did a pass on the control flow where it assumed the optional was present at runtime, then the Say the compiler assumes the optional is absent (or could be) at runtime like it currently appears to do. Then the The only other possibility I can see is the compiler recognizes the optional could be either present or absent at runtime and so coerces the |
Zig Version
0.14.0
Steps to Reproduce and Observed Behavior
Given
example.zig
:Try:
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:
zig/lib/std/process/Child.zig
Lines 591 to 611 in b4b1daf
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 usestd.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.The text was updated successfully, but these errors were encountered: