POSIX is not portable, remove it #2045
Conversation
compnerd
force-pushed
the
posix-is-not-portable
branch
from
d8069c3
to
0f2179b
Compare
|
Thanks. These were written before Foundation on linux was complete. I am a little afraid of the fallout here so maybe start doing this one at a time so it can be reviewed and merged independently? |
|
@aciidb0mb3r - sure, would you mind looking through the changes individually? I can split it up into a single PR for each of the changes, but, if you can take a preliminary look through this, I can change them before splitting them up into individual PRs. |
Add support for the Windows targets by importing libc with the correct name rather than `Darwin.C`.
compnerd
force-pushed
the
posix-is-not-portable
branch
from
0f2179b
to
995d941
Compare
|
I took a preliminary look, they look great in general. I probably will have minor comments on some individual changes but go ahead and break them up. It probably makes the most sense to break them according to the API (symlink, createdir, contents of dir, getenv, etc). For getting the environment, use |
Remove some of the custom error messages that were in use previously. We can forward along the NSError from Foundation to relay the necessary diagnostic information.
| var process: Process = Process() | ||
| process.executableURL = path | ||
| process.arguments = args | ||
| try process.run() |
There was a problem hiding this comment.
Two notes:
- This API is for replacing the current process with a new one so we shouldn't just run it as a subprocess.
- Don't use Foundation's Process. We have our own implementation for various reasons. It is one of the thing that we'll want to implement for all platforms instead of using Foundation's.
There was a problem hiding this comment.
That isn't a great idea ... there is a bunch of complexity that is involved for process execution, particularly on Windows. On Windows, CreateProcess is the closest equivalent to exec.
There was a problem hiding this comment.
We should conditionalize it then..
| @@ -21,64 +21,10 @@ import SPMLibc | |||
| import POSIX | |||
There was a problem hiding this comment.
Probably, but, can't as of yet, I'm just mechanically removing the items so that we can split this up and merge it piecemeal. It's easier to review this an API at a time rather than everything at once.
| // Otherwise, we failed due to some other error. Report it. | ||
| do { | ||
| try FileManager.default.createDirectory(atPath: path.pathString, withIntermediateDirectories: recursive) | ||
| } catch { | ||
| throw FileSystemError(errno: errno) | ||
| } |
There was a problem hiding this comment.
You should be able to catch an _NSErrorWithErrno and extract the errno from that as it may be overwritten in between.
There was a problem hiding this comment.
Yeah, however, it seems that there is no real reason to keep the custom error wrappers around, so, Im going to change towards letting the NSError propagate through which makes things simpler and less likely to introduce errors like that.
|
I've removed the POSIX module. There are now less things requires conversion (in Basic) but that can happen overtime: #2055 |
This prepares swift-package-manager to adopt more functionality in Foundation. The implementation in swift-package-manager is EXTREMELY unfriendly to porting due to the highly POSIX/UNIX specific implementation. Rather than trying to implement a POSIX/UNIX layer for Windows, it is preferable to use Foundation where ever possible and generalizing the implementation otherwise to permit the use of Windows semantics.