Mstoker/windows #29
Open
Mstoker/windows #29
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… true for testWindowsTrue()
…tform. false/true tests work now
…rn the string instead of the empty string. Refined windows tests (some still fail)
…o process struct. Removed some debug printing
…t to test outside a test suite
…tion deep in swift libs and then bailed.
…ods from libuv for quoting command arguments and searching the execution path. Used the quoting method to quote arguments.
… same time possible
…ynchronous write within an async context
… changing the environment. Failed tests early that were blocking on Windows. Test suite now runs to completion
…to restrict handles inherited by the child process
…causing a hang on Windows).
…ends FileManager as that type already posesses a 'nullDevice' property, but lacks its path.
… variable directly on Windows
cobbal
requested changes
Jan 16, 2024
| let pipe = FDPipe() | ||
| _ = async(fdMap: [ .stdout: pipe.fileHandleForWriting.fileDescriptor ]) | ||
| pipe.fileHandleForWriting.close() | ||
| // TODO: Previously, this returned a FileHandle, but the test using it fails without returning an FDFileHandle, because otherwise, on return, the FDFileHandle is deallocate and calls close(). Can maintain liveness of the FDFileHandle in some way without changing the interface of this call? |
There was a problem hiding this comment.
To do that, just duplicate the handle
| throw InvalidString(data: data, encoding: encoding) | ||
| } | ||
| guard let trimStop = string.lastIndex(where: { $0 != "\n" }) else { | ||
| // TODO: Shoudn't this return `string`? |
There was a problem hiding this comment.
No, the string is all newlines and should be completely trimmed.
| self.spawner = PosixSpawn() | ||
| #elseif canImport(Glibc) | ||
| self.spawner = LinuxSpawn() | ||
| #elseif canImport(ucrt) |
There was a problem hiding this comment.
This is mismatched with the import statement at the top of the file. They should both have the same #condition.
| } | ||
|
|
||
| static let NtSuspendProcess: (ProcessInformation) -> Int32 = { | ||
| // TODO: Tie into (undocumented) NtSuspendProcess() syscall |
| } else if signal == SIGSTOP { | ||
| // Method borrowed from https://github.com/giampaolo/psutil/blob/a7e70bb66d5823f2cdcdf0f950bdbf26875058b4/psutil/arch/windows/proc.c#L539 | ||
| // See also https://ntopcode.wordpress.com/2018/01/16/anatomy-of-the-thread-suspension-mechanism-in-windows-windows-internals/ | ||
| _ = Self.NtSuspendProcess(process) |
There was a problem hiding this comment.
These errors shouldn't be ignored
| let parentFDMax = fdMap.keys.max() ?? -1 | ||
| var childHandleArray = Array<HANDLE?>(repeating: nil, count: Int(parentFDMax + 1)) | ||
| for (parentFD, childFD) in fdMap { | ||
| childHandleArray[Int(parentFD)] = Self.osHandle(for: childFD) |
There was a problem hiding this comment.
Suggested change
| childHandleArray[Int(parentFD)] = Self.osHandle(for: childFD) | |
| childHandleArray[Int(parentFD)] = osHandle(for: childFD) |
| } | ||
|
|
||
| private init?(_ handles: [HANDLE?]) { | ||
| let handles = !shouldDuplicate ? handles : handles.map { Self.duplicate($0) } |
There was a problem hiding this comment.
Self shouldn't be used in classes unless you're absolutely sure you want to dispatch based on the dynamic type.
|
|
||
| func testAbsPath() { | ||
| #if os(Windows) | ||
| XCTAssertEqual(try? cmd("C:\\Windows\\System32\\cmd.exe", "/C", "ECHO 1").runString(), "1\r\n") |
There was a problem hiding this comment.
This is the wrong answer. runString is supposed to remove the newline
| let cProgramExecutable = "writer-\(id)" | ||
| #endif | ||
|
|
||
| try cmd("cat").input(cProgram).output(overwritingFile: cProgramFile).run() |
There was a problem hiding this comment.
This should be put in the temp directory, not the current folder
There was a problem hiding this comment.
This script has been made non-executable
try to fix open call on windows
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds Windows support to swsh, while trying to avoid the most grievous of sins.