Skip to content

Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHandler #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 12, 2018
Merged

Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHandler #5

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4584433
Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHa…
spevans Aug 31, 2018
78a2370
Use makeReadSource on macOS to use a consistent method across all pla…
spevans Sep 7, 2018
e944cd7
Linux: Add LinuxMain and make 'swift test' run on Linux
spevans Sep 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions Sources/SwiftSyntax/SwiftcInvocation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,36 @@ private func runCore(_ executable: URL, _ arguments: [String] = [])
-> ProcessResult {
let stdoutPipe = Pipe()
var stdoutData = Data()

#if os(macOS)
stdoutPipe.fileHandleForReading.readabilityHandler = { file in
stdoutData.append(file.availableData)
}
#else
// Temporary fix until swift-corelibs-foundation supports .readabilityHandler
let stdoutSource = DispatchSource.makeReadSource(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to also use DispatchSource on macOS? That way we wouldn't need to differentiate between macOS and Linux which would feel desirable to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer to keep it like this for now just incase the solution for Linux is missing something as I don't want to regress the macOS part, but I will get the fix into corelibs-foundation asap so that this temp fix can be removed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having it consistent between the two platforms would mean that the two hit the same issues. That way we would catch any bugs earlier when developing on Mac because it also fails on that platform.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahoppen I've updated it to use makeReadSource on both platforms.

fileDescriptor: stdoutPipe.fileHandleForReading.fileDescriptor)
stdoutSource.setEventHandler {
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
}
stdoutSource.resume()
#endif

let stderrPipe = Pipe()
var stderrData = Data()

#if os(macOS)
stderrPipe.fileHandleForReading.readabilityHandler = { file in
stderrData.append(file.availableData)
}
#else
let stderrSource = DispatchSource.makeReadSource(
fileDescriptor: stderrPipe.fileHandleForReading.fileDescriptor)
stderrSource.setEventHandler {
stderrData.append(stderrPipe.fileHandleForReading.availableData)
}
stderrSource.resume()
#endif

let process = Process()
process.launchPath = executable.path
Expand Down