-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Android: add bootstrap build support and fix tests #2417
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.
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
Conversation
@@ -681,7 +685,7 @@ def process_runtime_libraries(build, args, lib_path): | |||
cmd = [args.swiftc_path, "-emit-library", "-o", runtime_lib_path, | |||
"-Xlinker", "--whole-archive", | |||
"-Xlinker", input_lib_path, | |||
"-Xlinker", "--no-whole-archive", "-lswiftGlibc", | |||
"-Xlinker", "--no-whole-archive", "-lswiftGlibc", "-lFoundation", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only part of this pull that affects non-Android platforms. I had issues for a long time with getting the bootstrapped SPM to interpret its own manifest and dump it to JSON (as mentioned in #2396) and it turned out this was why.
The problem is that libPackageDescription.so depends on some functions from Foundation, but doesn't list it as a required dependency:
> readelf -sW 512-release/usr/lib/swift/pm/4_2/libPackageDescription.so | ag Foundation
160: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $s10Foundation11JSONEncoderCACycfc
161: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $s10Foundation11JSONEncoderCMa
162: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $sSS10FoundationE4data8encodingSSSgAA4DataVh_SSAAE8EncodingVtcfC
163: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $sSS10FoundationE8EncodingV4utf8ACvau
1616: 000000000001d130 78 FUNC LOCAL HIDDEN 11 $s10Foundation4DataV15_RepresentationOWOe
2341: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $s10Foundation11JSONEncoderCACycfc
2342: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $s10Foundation11JSONEncoderCMa
2445: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $sSS10FoundationE4data8encodingSSSgAA4DataVh_SSAAE8EncodingVtcfC
2446: 0000000000000000 0 NOTYPE GLOBAL DEFAULT UND $sSS10FoundationE8EncodingV4utf8ACvau
> readelf -d 512-release/usr/lib/swift/pm/4_2/libPackageDescription.so| ag library
0x0000000000000001 (NEEDED) Shared library: [libswiftSwiftOnoneSupport.so]
0x0000000000000001 (NEEDED) Shared library: [libswiftCore.so]
0x0000000000000001 (NEEDED) Shared library: [libswiftGlibc.so]
0x0000000000000001 (NEEDED) Shared library: [libstdc++.so.6]
0x0000000000000001 (NEEDED) Shared library: [libm.so.6]
0x0000000000000001 (NEEDED) Shared library: [libgcc_s.so.1]
0x0000000000000001 (NEEDED) Shared library: [libc.so.6]
0x000000000000001d (RUNPATH) Library runpath: [/home/buildnode/jenkins/workspace/oss-swift-5.1-package-linux-ubuntu-18_04/build/buildbot_linux/swift-linux-x86_64/lib/swift/linux:$ORIGIN/../../linux]
These results were from the official 5.1.2 release for linux x86_64, I get the same results with the latest snapshots from master.
The Android dynamic linker enforces that even if a executable or library links against libPackageDescription.so and lists libFoundation.so as a dependency there, it isn't transitively applied to libPackageDescription.so, unlike the GNU dynamic linker historically, though that may be changing. It appears that's why this hasn't hit on linux, as the Swift compiler seems to automatically add the needed Foundation dependency to a linux executable I tested linking against libPackageDescription.so, but that doesn't help on Android since it enforces that each library list its requirements itself.
@@ -270,6 +270,10 @@ class Target(object): | |||
if args.llbuild_link_framework: | |||
other_args.extend(["-F", args.llbuild_build_dir]) | |||
|
|||
# Don't use GNU strerror_r on Android. | |||
if 'ANDROID_DATA' in os.environ: | |||
other_args.extend(["-Xcc", "-U_GNU_SOURCE"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am trying to move SwiftPM away from this bootstrap script and rely the CMake 3.15.1 which has builtin Swift support. I don't know when that'll happen but I expect to it to happen within next couple of months (ofc not a guarantee). So this might mean you have to do some of these changes again. If you want to try the new WIP CMake bootstrapping, run Utilities/new-bootstrap
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'm going to try cross-compiling SPM itself from linux to Android next, so I was going to try the CMake support for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome, thanks!
@swift-ci smoke test |
@@ -1175,6 +1186,10 @@ def main(): | |||
# Append the versioning build flags. | |||
build_flags.extend(create_versoning_args(args)) | |||
|
|||
# Don't use GNU strerror_r on Android. | |||
if 'ANDROID_DATA' in os.environ: | |||
build_flags.extend(["-Xswiftc", "-Xcc", "-Xswiftc", "-U_GNU_SOURCE"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine for now but this should go to BuildPlan.swift so swiftpm can correct compile itself and other Swift targets for Android.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, _GNU_SOURCE
was set for all clang imports for Android in the Swift compiler a year ago, swiftlang/swift@60e2205, so changing that for all packages built for Android could break stuff. This tweak simply unsets it so the GNU strerror_r
isn't used when building SPM itself, and avoids those larger issues.
I've been backporting this pull and the previous #2396 to the latest Swift 5.1.2 release, only had to make some small tweaks to account for the older toolchain symlinks and cherry-picked two commits from master, which got all the SPM 5.1.2 tests passing natively on Android. You may want to consider cherry-picking those two commits back to the swift-5.1-branch, to help those porting SPM from the latest source releases, say 5.1.3 next. One commit was caea53a, which I only needed because I enable and run those filesystem tests on Android in my previous pull. Since that changes the implementation of The other commit, b876852, would be useful for anyone running the tests and hitting those hardcoded triples for linux x86_64, say if someone was porting SPM to linux ARM. Since it only fixes tests, no risk for the 5.1 branch. |
5.1 branch is generally not open to take commits. You could nominate changes for the monthly Linux releases but I am not sure what's the process for that. cc @weissi |
OK, I'm not sure what the process is either, just a suggestion based on what commits helped me when porting SPM 5.1.2 to run natively on Android. Anyway, thanks for merging. |
With this pull, I got SPM built natively on Android and all the tests passing with the Nov. 8 source snapshot from the master branch.