Port directory enumeration related code to WASI #836
Conversation
|
@swift-ci test |
| @@ -193,6 +193,9 @@ extension _FileManagerImpl { | |||
| } | |||
| } | |||
| return results | |||
| #elseif os(WASI) | |||
| // wasi-libc does not support FTS for now | |||
| throw CocoaError.errorWithFilePath(path, errno: ENOTSUP, reading: true) | |||
There was a problem hiding this comment.
This will just end up mapping to .fileReadUnknown, instead could we throw CocoaError.errorWithFilePath(.featureUnsupported, path) to get the feature unsupported error code?
| #if os(WASI) | ||
|
|
||
| // wasi-libc does not support FTS, so we don't support removing non-empty directories on WASI for now. | ||
| throw CocoaError(.featureUnsupported) |
There was a problem hiding this comment.
Should we include the pathStr here via CocoaError.errorWithFilePath(.featureUnsupported, pathStr)?
There was a problem hiding this comment.
Very good catch, +1 here
| guard lstat(srcPtr, &stat) == 0, !stat.isDirectory else { | ||
| // wasi-libc does not support FTS for now, so we don't support copying/linking | ||
| // directories on WASI for now. | ||
| throw CocoaError(.featureUnsupported) |
There was a problem hiding this comment.
Similar comment as above - should we include the path here via String(cString: srcPtr)?
| #if os(WASI) | ||
| private static func _linkOrCopyFile(_ srcPtr: UnsafePointer<CChar>, _ dstPtr: UnsafePointer<CChar>, with fileManager: FileManager, delegate: some LinkOrCopyDelegate) throws { | ||
| var stat = stat() | ||
| guard lstat(srcPtr, &stat) == 0, !stat.isDirectory else { |
There was a problem hiding this comment.
We might want to consult the delegate here - calling shouldPerformOnItemAtPath first, and if failing try throwIfNecessary and just return otherwise if the delegate wants to continue past this directory
There was a problem hiding this comment.
The first piece looks good, but I think we still need to check with the delegate before throwing below via delegate.throwIfNecessary - that way if the delegate wants to continue, we correctly continue on past this item
There was a problem hiding this comment.
Oops, I missed the second part.
| } | ||
|
|
||
| // Define getter shims for constants because wasi-libc defines them as function-like macros | ||
| // which are not supported by ClangImporter yet. |
There was a problem hiding this comment.
For things like this, is there any hope for us resolving this somehow beyond updating the ClangImporter (some sort of API notes or something at the toolchain layer? Not sure if that's possible today, but I'm curious what our options are since it seems like WASI requires a large amount of shims
There was a problem hiding this comment.
Yes, we have three directions, 1. provide those definitions in WASILibc overlay module, 2. change the upstream wasi-libc headers to be compatible with ClangImporter, 3. "fix" ClangImporter to import those constants from wasi-libc headers directly.
1 is what we are doing for some frequently used constants now. Still, it's quite ad-hoc and not a scalable solution. 2 is an option worth considering to align with other platform's headers, but not sure if it's acceptable for the upstream and if they can maintain the compatibility with ClangImporter.
I think 3 is the way we should go, and there are several open tickets for this kind of ClangImporter enhancement around macros (swiftlang/swift#46679, swiftlang/swift#45008, swiftlang/swift#43102, swiftlang/swift#45009). We need to carefully do it since it can break source compatibilities, but I have a plan to work on this.
For now wasi-libc does not include fts(3) implementation, so mark features depending on it as unsupported on WASI. Once wasi-libc includes fts or we decide to implement and maintain our own fts-like API, we can remove these `#if os(WASI)` guards. wasi-libc issue tracking fts support: WebAssembly/wasi-libc#520 Also, wasi-libc defines some constants in a way that ClangImporter can't understand, so we need to grab them manually through _FoundationCShims in function call form.
kateinoigakukun
force-pushed
the
pr-6852de63bfe058b155044011077d5671301acefc
branch
from
3fe5e1b to
08af69e
Compare
|
@swift-ci test |
|
@swift-ci test |
kateinoigakukun
deleted the
pr-6852de63bfe058b155044011077d5671301acefc
branch
* Port directory enumeration related code to WASI For now wasi-libc does not include fts(3) implementation, so mark features depending on it as unsupported on WASI. Once wasi-libc includes fts or we decide to implement and maintain our own fts-like API, we can remove these `#if os(WASI)` guards. wasi-libc issue tracking fts support: WebAssembly/wasi-libc#520 Also, wasi-libc defines some constants in a way that ClangImporter can't understand, so we need to grab them manually through _FoundationCShims in function call form. * Delegate error throwing decisions to the delegate
For now wasi-libc does not include fts(3) implementation, so mark features depending on it as unsupported on WASI. Once wasi-libc includes fts or we decide to implement and maintain our own fts-like API, we can remove these
#if os(WASI)guards.I'm working on porting FTS to wasi-libc (WebAssembly/wasi-libc#522), and we can share most of the enumerator code with other posix platforms after that.
Also, wasi-libc defines some constants in a way that ClangImporter can't understand, so we need to grab them manually through _FoundationCShims in function call form.