Skip to content

update zig env to respect ZIG_LIB_DIR and support wasi #24012

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Techatrix
Copy link
Contributor

The zig env command does not respect the ZIG_LIB_DIR environment variable which seems to be unintentional as the similar ZIG_GLOBAL_CACHE_DIR variable is being handled as expected.

With the recent addition of Compilation.Directories, I also went ahead and made zig env runnable on wasi to make some progress on #20665. Here is how the output wasmtime run --dir=lib::/lib --dir=cache::/cache build/stage4-wasi/bin/zig.wasm env looks like:

{
 "zig_exe": "zig.wasm",
 "lib_dir": "/lib",
 "std_dir": "/lib/std",
 "global_cache_dir": "/cache",
 "version": "0.15.0-dev.650+4f3b59f70",
 "target": "wasm32-wasi.0.1...0.2.2-musl",
 "env": {
  "ZIG_GLOBAL_CACHE_DIR": null,
  "ZIG_LOCAL_CACHE_DIR": null,
  "ZIG_LIB_DIR": null,
  "ZIG_LIBC": null,
  "ZIG_BUILD_RUNNER": null,
  "ZIG_VERBOSE_LINK": null,
  "ZIG_VERBOSE_CC": null,
  "ZIG_BTRFS_WORKAROUND": null,
  "ZIG_DEBUG_CMD": null,
  "CC": null,
  "NO_COLOR": null,
  "CLICOLOR_FORCE": null,
  "XDG_CACHE_HOME": null,
  "HOME": null
 }
}

To test these changes on wasm32-wasi, I had to edit src/dev.zig to only enable the .env_command feature.

@alexrp alexrp requested a review from mlugg May 29, 2025 12:18
@mlugg
Copy link
Member

mlugg commented May 30, 2025

Hmm... this is definitely good overall, but it raises the question (which I haven't previously considered) of what kind of paths zig env should emit. In particular, on targets with absolute paths (basically everything except WASI), should it be able to give you relative paths for zig_exe, lib_dir, etc, or should they be made absolute even if under the cwd?

In the compiler internally it's fine to use cwd-relative paths where possible, but absolute paths may make more sense when we're printing them out as JSON, because the job of tooling becomes more difficult if it has to worry about the cwd of the zig process in order to use these paths.

The reason I'm bringing this up is just because this PR made me realise this problem, and it does probably make zig env more likely to print cwd-relative paths (previously I think that would have happened sometimes but not always). I think this is worth deciding before we go ahead and merge this.

FWIW, making the paths absolute would just be a matter of doing std.fs.path.resolve(arena, &.{ dirs.cwd, my_path }) to convert a maybe-relative my_path to an absolute path (except on WASI where this is a nop). I'm not saying we should make them absolute, but if we do think that's the right thing to do, that should work.

@Techatrix
Copy link
Contributor Author

I always wondered why zig env would try to give relative paths to the zig library. I just assumed that it had to have some justification for this but I never looked much into it. From the outside this always appeared like an edge case that tooling has to be aware of rather than something helpful. But the good thing is that the more zig env emits relative paths, the more likely it is that users are aware of this.

and it does probably make zig env more likely to print cwd-relative paths (previously I think that would have happened sometimes but not always).

That appears to be true. lib_dir and std_dir would become relative under cwd but not global_cache_dir. With this change all three of them can become relative but zig_exe remains absolute.

@mlugg
Copy link
Member

mlugg commented May 30, 2025

I do agree that zig env printing relative paths whenever possible is preferable to only doing it sometimes. However, I also agree that this might be an annoyance for tooling. I think we need to pick between two options:

  • zig env outputs cwd-relative paths whenever possible (without using .., i.e. whenever the path is below the cwd). This matches, for instance, the file paths we print in error bundles.
  • zig env always outputs absolute paths (except for paths relative to the cwd preopen on WASI, which are still printed as cwd-relative, because absolute paths don't really exist on WASI).

The first option is arguably the most "correct" thing to do, since absolute paths have the flaw that they can cause ENAMETOOLONG in cases which cwd-relative paths could avoid. However, since zig env is for external consumption, cwd-relative paths could be more bug-prone, since people are, frankly, pretty bad at handling relative paths. Plus, cwd-relative paths from zig env would still be rare in practice (most users might never see them because their Zig installation prefix differs from the directory of whatever they're working on), so I think it's still very likely that tooling will handle them incorrectly without anyone realising.

For that reason, my vote would probably be that zig env print absolute paths. However, I don't really mind either way. I suspect Andrew will have an opinion here (probably the opposite of mine), so I'll defer to @andrewrk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants