Skip to content

Facilitate implicit import of Dispatch in Foundation #201

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 2 commits into from
Sep 14, 2017
Merged

Facilitate implicit import of Dispatch in Foundation #201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5746785
For XCTest to link against libdispatch
johnno1962 Sep 12, 2017
deb34d1
To build for Android
johnno1962 Sep 12, 2017
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
2 changes: 2 additions & 0 deletions Sources/XCTest/Private/PrintObserver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ internal class PrintObserver: XCTestObservation {

fileprivate func printAndFlush(_ message: String) {
print(message)
#if !os(Android)
fflush(stdout)
#endif
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

}

private func formatTimeInterval(_ timeInterval: TimeInterval) -> String {
Expand Down
3 changes: 2 additions & 1 deletion build_script.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,14 @@ def build(args):
libdispatch_args=libdispatch_args,
source_paths=" ".join(sourcePaths)))
run("{swiftc} -emit-library {build_dir}/XCTest.o "
"-L {foundation_build_dir} -lswiftGlibc -lswiftCore -lFoundation -lm "
"-L {dispatch_build_dir} -L {foundation_build_dir} -lswiftGlibc -lswiftCore -lFoundation -lm "
# 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'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Totally in favor of this change, which makes this a little more explicit. Thanks!

foundation_build_dir=foundation_build_dir))

# Build the static library.
Expand Down