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

spevans · 2018-08-31T13:43:52Z

swift-corelibs-foundation does not currently support FileHandle handlers
so use DispatchSource.makeReadSource instead of
FileHandle.readabilityHandlerForReading if not on macOS.

This is only a temporary fix and there may be other Linux issues, but it was enough to get the example in the readme working (although I had to rebase this fix on top of tag swift-DEVELOPMENT-SNAPSHOT-2018-08-25-a for testing).

…ndler - swift-corelibs-foundation does not currently support FileHandle handlers so use DispatchSource.makeReadSource instead of FileHandle.readabilityHandlerForReading if not on macOS.

SDGGiesbrecht · 2018-08-31T19:52:17Z

@spevans, @akyrtzi, @ahoppen, @aciidb0mb3r

This is only a temporary fix and there may be other Linux issues

I have been using a SwiftSyntax on Linux for a while now (for documentation generation and proofreading), though I have been doing it with the source from the 4.1.2 release. There were only three things I needed to adjust:

(I was lazy and hoisted SwiftSyntax up above several other modules to make it easier, so you won’t want to copy what I did, but the links will still help you find the issues.)

The compiler invocation. This you already found and fixed.
The search for the compiler. This is more critical on Linux because Swift tends to get installed all over the place instead of just as part of Xcode. It might be a good idea to provide an API to allow clients a means of specifying the location—and thereby the right Swift version (just like they can when linking against the Package Manager).

Looking at the current source, changing this...

private static let _swiftcURL: URL? = SwiftcRunner.locateSwiftc()

...to...

public static var swiftcURL: URL? = SwiftcRunner.locateSwiftc()

...would probably suffice. (private → public, let → var, drop underscore?)

An annoying fatal error. Not really Linux specific, but SwiftSyntax was pretty much useless out here in the wild as long as it chose to terminate instead of just report an unknown node. The “get” and “set” keywords were its most notorious triggers, but maybe all that is already fixed in the 4.2 branch. My own test files are here, if someone wants to check against them with 4.2 (or add pieces of them to this package’s test suite).

spevans · 2018-08-31T21:00:06Z

For the compiler search, it looks like the full path can be passed to the SwiftcRunner(sourceFile: URL, swiftcURL: URL? = nil) constructor otherwise it will search the PATH to find it.

see https://github.com/apple/swift-syntax/blob/master/Sources/SwiftSyntax/SwiftcInvocation.swift#L175

SDGGiesbrecht · 2018-08-31T22:08:36Z

@spevans,

For the compiler search, it looks like the full path can be passed to the SwiftcRunner(sourceFile: URL, swiftcURL: URL? = nil) constructor otherwise it will search the PATH to find it.

Hmm... You’re right. But the structure itself is internal. Does it actually surface in the API somewhere?...

...never mind. I found it. It does. I guess that’s been dealt with since 4.1.2.

ahoppen · 2018-08-31T22:29:42Z

Sources/SwiftSyntax/SwiftcInvocation.swift

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


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.

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.

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.

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

ahoppen · 2018-08-31T22:43:42Z

Thanks for the great work here.

The ability to specify the swiftc executable to SyntaxTreeParser was added here: fbd1628. We should maybe cherry-pick it over to swift-4.2-branch so that we can get Linux support for that branch as well.

The fatalError has also been resolved on master (b3171b7). If useful for Linux development we can also cherry-pick that over to swift-4.2-branch.

…tforms.

ahoppen · 2018-09-08T06:40:24Z

Looks good! Thanks a lot for tackling this.

I would like for PR testing to be set up before merging this but that shouldn't take much longer.

Also, as I understand it, SwiftSyntax will completely work on Linux with this PR, right? If so, you can also close SR-8670 once this is merged.

spevans · 2018-09-08T14:26:03Z

@ahoppen Ive just updated the tests so the swift test will work correctly on Linux. It needs the tests to be explicitly listed on Linux and a LinuxMain.swift added as well. All of the tests pass on Linux.

ahoppen · 2018-09-08T15:08:45Z

That's amazing! Thanks for all your work on this.

ahoppen · 2018-09-12T06:18:02Z

https://ci.swift.org/view/Pull%20Request/job/swift-PR-osx/7361/
https://ci.swift.org/view/Pull%20Request/job/swift-PR-Linux/7402/

CI for linux is not activated yet, but Mac is passing so let's 🛳 it.

rintaro

This LGTM.

Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHandler

[4.2] Merge pull request #5 from spevans/pr_linux_fix

# This is the 1st commit message: fixed testAvailabilityQuery34 and testAvailabilityQueryUnavailability28 # This is the commit message swiftlang#2: Update Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift Co-authored-by: Kim de Vos <[email protected]> # This is the commit message swiftlang#3: added fixedSource in test case # This is the commit message swiftlang#4: minor changes # This is the commit message swiftlang#5: implemented recovery inside the parser # This is the commit message swiftlang#6: runned format.py # This is the commit message swiftlang#7: minor changes # This is the commit message swiftlang#8: minor changes

Ensure a space after `@unknown` in a default case.

Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHa…

4584433

…ndler - swift-corelibs-foundation does not currently support FileHandle handlers so use DispatchSource.makeReadSource instead of FileHandle.readabilityHandlerForReading if not on macOS.

ahoppen reviewed Aug 31, 2018

View reviewed changes

Use makeReadSource on macOS to use a consistent method across all pla…

78a2370

…tforms.

ahoppen approved these changes Sep 8, 2018

View reviewed changes

Linux: Add LinuxMain and make 'swift test' run on Linux

e944cd7

rintaro mentioned this pull request Sep 11, 2018

[DO NOT MERGE] dummy PR for swift-syntax PR testing (master) swiftlang/swift#19240

Closed

ahoppen merged commit ab1f0f5 into swiftlang:master Sep 12, 2018

rintaro reviewed Sep 12, 2018

View reviewed changes

ahoppen mentioned this pull request Sep 12, 2018

Update the README to include macOS trigger for Swift CI #9

Merged

allevato pushed a commit to allevato/swift-syntax that referenced this pull request Sep 17, 2018

Merge pull request swiftlang#5 from spevans/pr_linux_fix

785a2e3

Linux: Use a DispatchSourceRead instead of a FileHandle.readabilityHandler

nkcsgexi added a commit that referenced this pull request Sep 17, 2018

Merge pull request #14 from allevato/swift-4.2-DispatchSourceRead

9a3a3cb

[4.2] Merge pull request #5 from spevans/pr_linux_fix

spevans mentioned this pull request Sep 12, 2018

[SR-8670] Make SwiftSyntax available on Linux #431

Closed

adevress pushed a commit to adevress/swift-syntax that referenced this pull request Jan 14, 2024

Merge pull request swiftlang#5 from allevato/sr-11113

8032cf2

Ensure a space after `@unknown` in a default case.

gfusee mentioned this pull request Sep 17, 2024

The package doesn't compile using the Swift 6.0 open source toolchain swiftlang/swift#76534

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

spevans commented Aug 31, 2018

Uh oh!

SDGGiesbrecht commented Aug 31, 2018

Uh oh!

spevans commented Aug 31, 2018

Uh oh!

SDGGiesbrecht commented Aug 31, 2018

Uh oh!

ahoppen Aug 31, 2018

Uh oh!

spevans Sep 3, 2018

Uh oh!

ahoppen Sep 7, 2018

Uh oh!

spevans Sep 7, 2018

Uh oh!

ahoppen commented Aug 31, 2018

Uh oh!

ahoppen commented Sep 8, 2018

Uh oh!

spevans commented Sep 8, 2018

Uh oh!

ahoppen commented Sep 8, 2018

Uh oh!

ahoppen commented Sep 12, 2018

Uh oh!

rintaro left a comment

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

spevans commented Aug 31, 2018

Uh oh!

SDGGiesbrecht commented Aug 31, 2018

Uh oh!

spevans commented Aug 31, 2018

Uh oh!

SDGGiesbrecht commented Aug 31, 2018

Uh oh!

ahoppen Aug 31, 2018

Choose a reason for hiding this comment

Uh oh!

spevans Sep 3, 2018

Choose a reason for hiding this comment

Uh oh!

ahoppen Sep 7, 2018

Choose a reason for hiding this comment

Uh oh!

spevans Sep 7, 2018

Choose a reason for hiding this comment

Uh oh!

ahoppen commented Aug 31, 2018

Uh oh!

ahoppen commented Sep 8, 2018

Uh oh!

spevans commented Sep 8, 2018

Uh oh!

ahoppen commented Sep 8, 2018

Uh oh!

ahoppen commented Sep 12, 2018

Uh oh!

rintaro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!