[1/3] Proper path resolution: add openat functionality #2254
Conversation
yagehu
force-pushed
the
path-res/1/openat
branch
6 times, most recently
from
c4eb9c5 to
63aac67
Compare
yagehu
force-pushed
the
path-res/1/openat
branch
2 times, most recently
from
26f2ef6 to
6debef3
Compare
This commit adds the `openat` functionality to `sys.FS`. On UNIX-like platforms, this is done with the `openat(2)` syscall. On Windows, this is done with [`NtCreateFile`][1]. This is the first in a series of commit that overhauls how paths are resolved. The wasi-filesystem spec now [describes][2] a path resolution implementation that uses `openat`. [1]: https://learn.microsoft.com/en-us/windows/win32/api/winternl/nf-winternl-ntcreatefile [2]: https://github.com/WebAssembly/wasi-filesystem/blob/main/path-resolution.md Signed-off-by: Yage Hu <[email protected]>
yagehu
force-pushed
the
path-res/1/openat
branch
from
6debef3 to
ea28b4d
Compare
Signed-off-by: Yage Hu <[email protected]>
Signed-off-by: Yage Hu <[email protected]>
yagehu
force-pushed
the
path-res/1/openat
branch
from
1c64d11 to
107be03
Compare
Signed-off-by: Yage Hu <[email protected]>
Signed-off-by: Yage Hu <[email protected]>
yagehu
force-pushed
the
path-res/1/openat
branch
3 times, most recently
from
a960173 to
7f90143
Compare
Signed-off-by: Yage Hu <[email protected]>
yagehu
force-pushed
the
path-res/1/openat
branch
from
7f90143 to
2dd5cda
Compare
yagehu
force-pushed
the
path-res/1/openat
branch
2 times, most recently
from
796b45e to
8b8f220
Compare
Signed-off-by: Yage Hu <[email protected]>
yagehu
force-pushed
the
path-res/1/openat
branch
from
8b8f220 to
00f22bb
Compare
yagehu
changed the title
yagehu
force-pushed
the
path-res/1/openat
branch
from
4071412 to
00f22bb
Compare
|
FYI I can't stack PRs on top of this one because I can't push branches to this repo. |
| @@ -71,7 +72,7 @@ func (r *ReadFS) Utimens(path string, atim, mtim int64) experimentalsys.Errno { | |||
| } | |||
|
|
|||
| // compile-time check to ensure readFile implements api.File. | |||
| var _ experimentalsys.File = (*readFile)(nil) | |||
| var _ fsapi.File = (*readFile)(nil) | |||
There was a problem hiding this comment.
this was intentionally implementing only the experimental public VFS API. In fact I think this experimental VFS should not expose OpenAt
There was a problem hiding this comment.
Thanks for the context! I had thought it was an oversight.
| OpenAt( | ||
| fs experimentalsys.FS, | ||
| path string, | ||
| flag experimentalsys.Oflag, | ||
| mode fs.FileMode, | ||
| ) (experimentalsys.File, experimentalsys.Errno) |
There was a problem hiding this comment.
do we really need to expose this at this level?
the reason I am asking is that this obviously only applies to hierarchical FSs, but it won't work (as you can see later) on other types of file descriptors such as sockets or stdin/err/out. Could it be possible to keep this internal and limited to tree-like file systems?
Essentially, the final goal of this (series of) PR is to introduce an API that will reproduce the correct behavior when resolving paths, so it would great if we can scope it only to files for which "paths" make sense.
There was a problem hiding this comment.
It definitely doesn't need to be exposed at this level. I misunderstood your last comment and thought you wanted the method moved from the public interface to a private one.
There was a problem hiding this comment.
well, that was a fair assumption, but I was wondering if we could keep it as an implementation detail of Open for those FSs that need hierarchical file access 🤔
|
Quick update: I'm on a cross-country move at the moment. Will get around to this again after mid-August. Thanks for the speedy reviews @evacchi |
|
feel free to reopen when ready to continue the work |
This PR is the first in a series that will implement the path resolution algorithm described by wasi-filesystem. Since this new path resolution implementation requires the use of
openat, this PR first introducesopenat(2)and its Windows equivalentNtCreateFileto Wazero.openat(2)openat(2)NtCreateFileinternal/sysfs/dirfs_test.goThe next PR will stack on this one and use the
openatfunctionality to implement path resolution.