Fix swift-system to work on Windows. #158
Conversation
|
@swift-ci Please test |
|
@swift-ci Please test Windows platform |
|
I guess we don't test Windows for swift-system. We should. |
al45tair
force-pushed
the
eng/PR-125087707
branch
from
a740351
to
be06941
Compare
|
@swift-ci Please test |
There was a problem hiding this comment.
Yay for the FilePath fixes! 🎉 Thanks 👍
I have lots of grumpy comments about the additions. swift-system was never intended to be a POSIX emulation package, and I really dislike that it's being pushed to become that. (I don't think we have the means to do a good job of that, even if it was a good idea, which I don't think it is.) This package isn't for building cross-platform abstraction layers -- that's Foundation's job.
al45tair
force-pushed
the
eng/PR-125087707
branch
2 times, most recently
from
5818960
to
b45ea64
Compare
|
@swift-ci Please test |
1 similar comment
|
@swift-ci Please test |
|
@swift-ci Please test |
The swift-system tests were not passing, for various reasons, mostly to do with path related confusion. As part of this, add a function to create a temporary directory, as that was one of the problems. Also add Windows support for pipe() and ftruncate(), which was missing. rdar://125087707
It seems we can get the temporary directory from confstr(). rdar://125087707
Use idiomatic Swift notation, not `Optional<Bool>`. Prefer Win32 APIs to UCRT APIs. rdar://125087707
This adds support for mapping POSIX-style mode bits to Windows ACLs when calling `open()` or `mkdir()`. rdar://125087707
Co-authored-by: Karoy Lorentey <[email protected]>
Broke `FilePathTemp.swift` into separate files for POSIX and Windows. Added tests for permissions code on Windows. Added leading underscores to some function names. Made some other functions `fileprivate`. rdar://125087707
Swift ends up with OpaquePointer instead of a typed pointer. rdar://125087707
|
Rebased as some WASI stuff landed that conflicted. |
|
@swift-ci Please test Linux platform |
|
@swift-ci Please test macOS platform |
| /// Creates a temporary directory with a name based on the given `basename`, | ||
| /// executes `body`, passing in the path of the created directory, then | ||
| /// deletes the directory and all of its contents before returning. | ||
| public func withTemporaryFilePath<R>( |
There was a problem hiding this comment.
Make sure that any API additions go through some kind of review, ideally on the forums. At the very least, a dedicated PR that highlights it's an API addition PR as opposed to just a bug-fix PR for Windows.
For this function, would a static method on FilePath work instead of a global? E.g. FilePath.withTemporaryPath { ... }.
There was a problem hiding this comment.
Yes, it would. My only reservation is that (to me) things like FilePath.withTemporaryPath read like a function that constructs and returns a FilePath from some kind of TemporaryPath thing. This isn't the same as e.g. Array's withUnsafeBytes, because that's called on an instance rather than on the type itself. Maybe that's just because I'm used to ObjC. It's obvious from the arguments that it takes a closure, of course.
I only needed this for the tests, so for now I think the thing to do is to make this non-public (I'll raise a PR to fix that). I'll check through to make sure I didn't leak any other new API and do the same there.
The swift-system tests were not passing, for various reasons, mostly to do with path related confusion.
As part of this, add a function to create a temporary directory, as that was one of the problems.
Also add Windows support for pipe() and ftruncate(), which was missing.
rdar://125087707