zig icon indicating copy to clipboard operation
zig copied to clipboard

fs.Dir: Give the option to stat a symbolic link

Open samy-00007 opened this issue 1 year ago • 11 comments

This pull request adds Dir.statLink (with Z and W variants) to stat symlink. It also improves the windows version of Dir.statFile.

samy-00007 avatar Jul 28 '24 16:07 samy-00007

Note that this will need Linux-specific statx()-based code once #20389 is merged.

alexrp avatar Jul 28 '24 16:07 alexrp

Note that this will need Linux-specific statx()-based code once #20389 is merged.

Just dropping in again to say that it has been merged.

alexrp avatar Jul 30 '24 00:07 alexrp

I will make the necessary changes.

samy-00007 avatar Jul 30 '24 13:07 samy-00007

It should be good.

samy-00007 avatar Jul 30 '24 20:07 samy-00007

From what i am seeing, the test fails with wasm because in stage1/wasi.c, DirEntry_lookup which is used to stat the file ignores the flags, and so follows the symlink. Should i skip the test for wasi ? (Unless this is not the where the wasm functions are defined. I don't know enough about that)

samy-00007 avatar Jul 31 '24 16:07 samy-00007

Unless this is not the where the wasm functions are defined. I don't know enough about that

That's not the relevant code for the test failure (that's only used for bootstrapping a Zig compiler).

My guess is that it's wasi-libc which is ignoring the AT.SYMLINK_NOFOLLOW flag of fstat called from here, but will need to confirm (see https://github.com/ziglang/zig/issues/20747 for a similar wasi-libc bug). My second guess is that this test failure will only be present when linking wasi-libc, but also need to confirm that.


EDIT: Confirmed both guesses:

strace of the Dir.readLink function when linking wasi-libc (lack of O_NOFOLLOW in the openat2 flags):

openat2(13, "symlink", {flags=O_RDONLY|O_CLOEXEC|O_PATH, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
statx(11, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFREG|0664, stx_size=17, ...}) = 0
close(11)                               = 0

and without (inclusion of O_NOFOLLOW in the openat2 flags):

openat2(13, "symlink", {flags=O_RDONLY|O_NOFOLLOW|O_CLOEXEC|O_PATH, resolve=RESOLVE_NO_MAGICLINKS|RESOLVE_BENEATH}, 24) = 11
statx(11, "", AT_STATX_SYNC_AS_STAT|AT_EMPTY_PATH, STATX_ALL, {stx_mask=STATX_ALL|STATX_MNT_ID, stx_attributes=0, stx_mode=S_IFLNK|0777, stx_size=9, ...}) = 0
close(11)                               = 0

This is very likely to be either a wasmtime or a wasi-libc bug. Will try to put together a reproduction with wasi-sdk and report it downstream.


EDIT#2: Unable to find a reproduction, filed a follow-up issue: https://github.com/ziglang/zig/issues/20890

squeek502 avatar Jul 31 '24 21:07 squeek502

Is it possible to redo the CI tests ? They did not fail because of my code

samy-00007 avatar Aug 02 '24 08:08 samy-00007

Rebase the branch onto the latest master branch and it should get fixed.

squeek502 avatar Aug 02 '24 08:08 squeek502

Is it good ?

samy-00007 avatar Aug 05 '24 09:08 samy-00007

Looks good to me.

One possible variation on this would be to make statFile take an options struct parameter with a follow_symlinks field (instead of having separate pub statLink functions). Unsure which would be better, will leave that up to the core team member(s) that review this.

squeek502 avatar Aug 06 '24 23:08 squeek502