Facilitate implicit import of Dispatch in Foundation #201
Conversation
| # We embed an rpath of `$ORIGIN` to ensure other referenced | ||
| # libraries (like `Foundation`) can be found solely via XCTest. | ||
| "-Xlinker -rpath=\\$ORIGIN " | ||
| "-o {build_dir}/libXCTest.so".format( | ||
| swiftc=swiftc, | ||
| build_dir=build_dir, | ||
| dispatch_build_dir=os.path.join(args.libdispatch_build_dir, 'src', '.libs'), |
There was a problem hiding this comment.
Totally in favor of this change, which makes this a little more explicit. Thanks!
| fflush(stdout) | ||
| #endif |
There was a problem hiding this comment.
I'll defer to @briancroom on this one. Does Android really not provide a stdout file handle? Is this a shortcoming in the Swift Glibc overlay when built for Android? If so, I guess I'm fine with this change.
There was a problem hiding this comment.
Yeah, this is interesting! I dug a little bit and found that stdout isn't being build for Android currently: https://github.com/apple/swift/blob/48308411393e730cf3cb21d353a9be31045c47e4/stdlib/public/Platform/Platform.swift#L98
And here's a bug about a Swift test failure caused by the same issue (guess who filed it? 🙃): https://bugs.swift.org/browse/SR-1835
It looks like these could be brought in only by building against Android API level 23, which may be an acceptable change at some point in the future.
johnno1962
changed the title
|
Hi @briancroom, I’ve changed the title of the this PR as while it does include a change for Android it is primarily about facilitating an implicit import of Dispatch when developers import Foundation to mimic the behaviour of Darwin for which this small change is a blocker. Otherwise the Linux Swift build breaks see: swiftlang/swift-corelibs-foundation#1206 (comment) |
|
@swift-ci please test |
|
Thanks @johnno1962, let's get this merged, pending swift-ci going smoothly. |
|
Thanks @briancroom, I’ve corrected the nits and indenting on the other PR and squashed so it shouldn’t conflict once this PR is merged if you're also happy with the Package.swift changes. |
Hi Apple,
Would you consder this minor change that achieves two things: It allows XCTest to build for Android by removing a reference to stdout and it paves the way for Foundation to implicitly import Dispatch for compatibility with Darwin. As this would create a implicit link against libdispatch.so, this requires this small change so building XCTest.so knows where to find the library when building on all Linuxies.
This PR is essentially the previous PR but not modifiying Package.swift and adding the path to the lib dispatch.so library during a toolchain build to resolve: https://ci.swift.org/job/swift-corelibs-foundation-PR-Linux/119/console when we tried to have Foundation implicitly import dispatch: swiftlang/swift-corelibs-foundation#1206 (comment)
Thanks,
John