zig

zig copied to clipboard

Second attempt at #14661 :)

in Zig's WASI support:

cwd() always points at fd=3, i.e. the first given preopen. However, if multiple preopens are available, there is currently no predefined library function to look through all of them.

The patch adds a new std lib function Preopens.openDir(path) that looks through all preopens and returns an open dir for the given path.

• If no preopens are available or there is no match, then an error is returned
• if there is at least one preopen p, and it is a prefix of path, then the suffix path[p.len..] is looked under dir p
• if there are multiple preopens p that are prefix of path then the longest prefix is used

The current patch is configuring the test mode so that the following preopens are pre-configured:

  .  => /
/tmp => /tmp

I am opening this PR to rebase on master and discuss a different approach for testing (if applicable)

Signed-off-by: Edoardo Vacchi [email protected]

Thank you. The findDir function looks very nice. However, why is the /tmp preopen included as part of this PR? What use case are you solving with it?

My understanding is that -fwasmtime currently only preopens ..

findDir() navigates all the preopens and returns the longest prefix match to a given path. If we only preopen . then we cannot test that the prefix search is correctly picking the right preopen in a list, because there is basically no list to pick from.

/tmp is a somewhat arbitrary choice to add 2 preopens instead of just one. It may still be useful in other test cases that need to write to a designated temporary location.

In any case, I am open to suggestions on the best approach :)

ping @andrewrk @Luukdegram 🙏

ping @andrewrk @Luukdegram 🙏

I'm currently on holiday and won't be able to take a look at this for the next 2 weeks.

Ah sorry for bothering then, enjoy your holiday! :)

I just found that the wasmtime developers behind wasi decided recently to enforce a bunch of rules. Most notably, runtimes must return EPERM if the path to a pre-open is absolute (leading slash). This might affect this PR

See https://github.com/tetratelabs/wazero/pull/1266 for investigation around https://github.com/WebAssembly/wasi-testsuite/pull/67 merged yesterday

hey @Luukdegram / @andrewrk any further changes you would like to see?

hey @Luukdegram / @andrewrk any further changes you would like to see?

Not necessarily from my side. I'll leave the final judgment to Andrew as he mentioned he had some reservations to the path he'd like to take when it comes to Zig's WASI std support.

Thanks for your patience, @evacchi. I have rebased this on your behalf and force-pushed to this PR. I'll let the CI run and come back for the final review & merge in the coming days.

Sweet, I really appreciate this!

force pushed so that also aarch64-macos-{release,debug} are executed

I think this is basically application code that does not belong in the standard library. The idea of mapping preopens to the same names as file system paths is opinionated. The idea of doing linear search through those names is opinionated. Doing a heap allocated hash map instead would be also opinionated. The logic of finding a parent directory via a match and then a subdirectory is also opinionated.

All these opinions belong in the application, not the Zig standard library.