std: add compile error when using std.os.getenv on the wasi target

[ereslibre]

std.process.getEnvMap or std.process.getEnvVarOwned should be used instead, requiring allocation.

Fixes: #17117

Thanks to @squeek502 for the pointer!

[ereslibre]

I do wonder what happens in this block when targeting WASI:

zig/lib/std/os.zig

Lines 1896 to 1908 in 047efec

	if (builtin.link_libc) {
	var ptr = std.c.environ;
	while (ptr[0]) |line| : (ptr += 1) {
	var line_i: usize = 0;
	while (line[line_i] != 0 and line[line_i] != '=') : (line_i += 1) {}
	const this_key = line[0..line_i];
	if (!mem.eql(u8, this_key, key)) continue;
	return mem.sliceTo(line + line_i + 1, 0);
	}
	return null;
	}

Given nothing prepopulates std.c.environ, is there a possibility of a user requesting link_libc, targeting WASI? In that case I understand this fix is not enough and I should adapt the PR.

[squeek502]

When targeting WASI and libc is linked, wasi-libc is built and linked (the wasi-libc sources are distributed with Zig), which populates environ:

> zig build-exe getenv.zig -target wasm32-wasi  -lc

> wasmtime --env NAME=noone getenv.wasm
NAME is noone
NAME is noone

[ereslibre]

When targeting WASI and libc is linked, wasi-libc is built and linked

Didn't know that was the case. It's great then 👍

[ereslibre]

I'm not sure why CI is failing on linux. I'm going to check how to reproduce this locally so I can iterate faster.

[squeek502]

It's failing when running zig build update-zig1. Here's the trace with referenced by:

lib/std/os.zig:1912:9: error: std.os.getenv is unavailable for WASI. See std.process.getEnvMap or std.process.getEnvVarOwned for a cross-platform API.
        @compileError("std.os.getenv is unavailable for WASI. See std.process.getEnvMap or std.process.getEnvVarOwned for a cross-platform API.");
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
referenced by:
    detect: lib/std/zig/system/NativePaths.zig:149:19
    buildOutputType: src/main.zig:2695:49

So it's failing because of the os.getenv calls in this branch:

zig/lib/std/zig/system/NativePaths.zig

Lines 103 to 155 in 8fb4a4e

	if (builtin.os.tag != .windows) {
	const triple = try native_target.linuxTriple(arena);
	const qual = native_target.ptrBitWidth();
	// TODO: $ ld --verbose | grep SEARCH_DIR
	// the output contains some paths that end with lib64, maybe include them too?
	// TODO: what is the best possible order of things?
	// TODO: some of these are suspect and should only be added on some systems. audit needed.
	try self.addIncludeDir("/usr/local/include");
	try self.addLibDirFmt("/usr/local/lib{d}", .{qual});
	try self.addLibDir("/usr/local/lib");
	try self.addIncludeDirFmt("/usr/include/{s}", .{triple});
	try self.addLibDirFmt("/usr/lib/{s}", .{triple});
	try self.addIncludeDir("/usr/include");
	try self.addLibDirFmt("/lib{d}", .{qual});
	try self.addLibDir("/lib");
	try self.addLibDirFmt("/usr/lib{d}", .{qual});
	try self.addLibDir("/usr/lib");
	// example: on a 64-bit debian-based linux distro, with zlib installed from apt:
	// zlib.h is in /usr/include (added above)
	// libz.so.1 is in /lib/x86_64-linux-gnu (added here)
	try self.addLibDirFmt("/lib/{s}", .{triple});
	// Distros like guix don't use FHS, so they rely on environment
	// variables to search for headers and libraries.
	// We use os.getenv here since this part won't be executed on
	// windows, to get rid of unnecessary error handling.
	if (std.os.getenv("C_INCLUDE_PATH")) |c_include_path| {
	var it = mem.tokenizeScalar(u8, c_include_path, ':');
	while (it.next()) |dir| {
	try self.addIncludeDir(dir);
	}
	}
	if (std.os.getenv("CPLUS_INCLUDE_PATH")) |cplus_include_path| {
	var it = mem.tokenizeScalar(u8, cplus_include_path, ':');
	while (it.next()) |dir| {
	try self.addIncludeDir(dir);
	}
	}
	if (std.os.getenv("LIBRARY_PATH")) |library_path| {
	var it = mem.tokenizeScalar(u8, library_path, ':');
	while (it.next()) |dir| {
	try self.addLibDir(dir);
	}
	}
	}

My initial thought is that the condition should just be changed to

if (builtin.os.tag != .windows and builtin.os.tag != .wasi) {

but I'm not familiar enough with this code.

Either way, this is finding a real bug since those getenv calls would have been silently broken without this compile error.

cc @kubkon @andrewrk: Is this branch in NativePaths.detect meant to be run when targeting wasi?

[ereslibre]

I have updated the PR, thanks again @squeek502!

I am not familiar with Zig and its build process, however in terms of WASI, unless the build machinery is doing something I am missing, adding /lib/, and /usr/lib and the likes is not going to fly because they will likely contain system-specific libs (e.g. x86_64, ...) -- if preopened at all with the wasm runtime --.

Not so sure about the include paths right now, that depends. I have updated the PR to skip the whole block though and check on CI.

[ereslibre]

Shoutout to @voigt for having stumbled upon the original issue!