Add macOS toolchain build and test #162
Conversation
|
Thanks, have you tried building random Swift packages, say any of those I build and test on the linux CI, using this macOS setup? I wonder if we can start building and testing full packages on macOS using this toolchain setup. Also, I have had issues cross-compiling packages that use macros using this Android SDK on linux: have you seen the same on macOS? Finally, you mention porting the Foundation rewrite to Android on the forum: have you started working on this? Other than some patches for nullability annotations in NDK 26, I have not done much with the rewrite yet. |
I have manually built and tested a few packages, but not the full gamut that you are running in the
I haven't experimented with macros yet. I know there have been some hairy issues with macros and cross-compilation in general.
I haven't started anything other than thinking about it. Perhaps a first step would be to see if we can get |
Great, good to hear it works.
Do you mind if I just take the macOS toolchain download portion of this pull, commands that I was not familar with because I don't use macOS, and try to build all these packages in macOS runners too, in a separate pull?
Yep, I wonder if that works with new versions of the destination schema, that are used in the new SDK bundles.
It would be easier to do so locally, which I plan to try natively on Android in the coming week. |
|
What I had in mind is to simply add macOS runners to the build matrix, then make sure I can cross-compile all these packages on macOS too. If you would like to change this pull to do that, go for it, but you don't have to either. |
| @@ -164,7 +166,7 @@ for termuxPackage in termuxPackages { | |||
| print("Checking for \(packageName)") | |||
| if !fmd.fileExists(atPath: termuxArchive.appendingPathComponent(String(packageName))) { | |||
| print("Downloading \(packageName)") | |||
| _ = runCommand("curl", with: ["-o", "termux/\(packageName)", | |||
| _ = runCommand("curl", with: ["-f", "-o", "termux/\(packageName)", | |||
There was a problem hiding this comment.
What does adding this flag help with?
There was a problem hiding this comment.
If there is an HTTP error (e.g., 404), then the curl command will fail when the "-f" flag is set. Otherwise it will output a blank file, and the subsequent tar x command will fail will an unhelpful error. I added it because I had previously made a typo in a package name, which was causing the download to fail.
Do you mean something like this? I think that would be ideal, but the I've gone ahead and copied over your package builds from Ubuntu to macOS in this PR. There's now a lot of duplicated logic between the two jobs, but we can tackle ways to de-dupe in the future. Other than changing the build arguments for the different toolchain setups, the only other change is a tweak to the Most packages build fine, with the following exceptions (which I've disabled so the build will pass): swift-argument-parserswift-nio-http2 and swift-nio-sslLet me know if this looks OK to you, or if you'd like to see some cleanup before merging. And feel free to make any changes, either in this PR or in a new one. I'm here to help with any follow-up work (especially figuring our ways to de-duplicate the build logic). |
Yep, exactly.
Since we are currently not updating the SDK snapshot tags or versions, cross-compiling packages should work fine in parallel, as I think you can use the previously cached copy of the SDK on macOS. Once that is working, I'd look into cross-compiling the SDK for Android in the macOS runner too. Feel free to work on any part of that you'd like, or none of it, up to you. I will come back to this in a couple days and either merge your work, or use it as a basis for attempting the above. |
Good idea to re-use the cached copy!
I've merged all the logic into a single job that runs on both macOS and Ubuntu, using the cached SDK from the Ubuntu build, which gets around the build-script error on macOS (which is something I have gotten around before in my local builds, I just don't remember exactly how). Hopefully it is good enough to merge, but I'm happy to do any further cleanup or refactoring that you think might be needed. |
.github/workflows/sdks.yml
Outdated
| @@ -3,6 +3,9 @@ on: | |||
| pull_request: | |||
| schedule: | |||
| - cron: '0 9 * * *' | |||
| push: | |||
| branches: '*' | |||
There was a problem hiding this comment.
I prefer to have the CI run once daily instead, please remove.
There was a problem hiding this comment.
Mind if I add workflow_dispatch? That way we can manually trigger the action to run.
There was a problem hiding this comment.
What is the use case for that? I submit a pull if I want to make a change, and all direct pushes are tested by the daily CI run.
When would you want to randomly trigger a CI run with no changes to this repo?
There was a problem hiding this comment.
I was thinking for cases where there is a new devel or trunk release and we want to immediately test it (once the toolchain check goes back to tracking the latest changes).
There was a problem hiding this comment.
I just wait for the next daily CI run, but you can add it if you like.
Thanks for all the work you put into this, macOS support is long overdue. 😄
I'm going to go through this long pull and list some nits. Once you've fixed all that and squashed into a single commit, I will experiment a bit with the failing packages and merge. |
Can't you just do "Squash and merge" when merging the PR?
I just figured out the NIO problems are likely due to a macOS-incompatible regex where the local dependency replacement in the Still not sure about the swift-argument-parser linker error, though. |
Yes, that was the problem. The differences in I've merged in the fix, so the NIO packages are now being built and tested on macOS. |
I think that would also squash my commit into yours, whereas I'd like this to consist finally of two commits: your changes and mine.
Yes, especially since it's built first because it's one of the simpler packages to build. You mentioned that you have an Android SDK bundle working locally: does cross-compiling code with Macros work with the bundle? You can try cross-compiling the swift-syntax examples, as shown here but using the bundle command instead (make sure you build the tests too, as that's where that failed), and see if that works. |
OK. I fear I may need to make a new branch and open a new PR, since I'm not sure how to rebase to selectively squash my own commits while retaining yours.
Not with 5.10 (as expected), where it incorrectly cross-compiles the macro (it incorrectly builds an ELF Thanks for swiftlang/swift-package-manager#7118 @MaxDesiatov! I can add a CI test for macros in another PR, either by running against |
Right now, I have no commits in this pull, so if there's a git command to just squash the last 145 commits, you can use that.
This is using your SDK bundle or just my CI-generated 6.0 SDK? That SwiftPM pull you linked didn't fix the latter for me, but maybe subsequent work has. |
I just reset my branch and made a new one with a single commit. It seemed to automatically close the PR, which I then re-opened. Let me know if it looks OK to you.
Using your CI-generated 6.0 SDK and the |
Huh, I just tried it with that 6.0 SDK from my CI on linux, still getting the same error: That would explain why my bug report was ignored, if it worked on macOS but only failed on linux. Looks like this is some incompatibility between a linux and mac host toolchain, same error with the trunk July 15 SDK too. Your single commit looks fine, I will go through it today and list some nits. |
Oops, it turns out I'm also seeing the same error (or nearly: TBH, I'm not quite sure why I was thinking that it was working. I had definitely seen it build when I reported success, but I may have accidentally just been using the host toolchain w/ the host destination or something like that. I can dig further, but this might be worthy of a separate issue thread. |
|
OK, good to hear it's not just linux, as that would have been very unusual. Can you try with the SDK bundle you created, as I asked initially? I wonder if that makes a difference. |
Same result with the bundle I created. 😞 |
I was planning on taking a run at swift-foundation in the coming week. Have you started looking at it? Do you have a sense of what hurdles there might be to overcome? |
Sigh, I was hoping that might fix it. What destination config file schema version are you using in your bundle?
Funny you ask now, 😄 as I just got all the |
v1. Do you think that the more recent version might work differently?
Fabulous! I can't wait to see it. |
I was hoping it might fix this issue, else these SDK bundles are completely busted if you still can't cross-compile packages with Macros.
Surprisingly, most of the tests passed on Android already, I just fixed and enabled a few more, swiftlang/swift-foundation#871. |
OK, thanks for letting me know.
Nice!
I did take another run at it, and I've gotten the build a bit further along, but it is still failing. FTR, the main issue seems to be the cmake scripts that use checks like I plan to put together a branch with CI that tries to build against the macOS target, and then we can see if we can inch forward with failures there. But for the record, here's what I've needed to do so far just to get the basics building: This gets the build as far as llbuild, where it is failing with: |
.github/workflows/sdks.yml
Outdated
|
|
||
| ./swift/utils/build-script -RA --skip-build-cmark --build-llvm=0 --android --android-ndk $ANDROID_NDK --android-arch ${{ matrix.arch }} --android-api-level $ANDROID_API_LEVEL --build-swift-tools=0 --native-swift-tools-path=`pwd`/$SWIFT_TAG-ubuntu22.04/usr/bin --native-clang-tools-path=`pwd`/$SWIFT_TAG-ubuntu22.04/usr/bin --cross-compile-hosts=android-${{ matrix.arch }} --cross-compile-deps-path=$SDK --skip-local-build --build-swift-static-stdlib --xctest --skip-early-swift-driver --install-swift --install-libdispatch --install-foundation --install-xctest --install-destdir=$SDK --swift-install-components='clang-resource-dir-symlink;license;stdlib;sdk-overlay' --cross-compile-append-host-target-to-destdir=False -b -p --install-llbuild --sourcekit-lsp --skip-early-swiftsyntax | ||
| ./swift/utils/build-script -RA --skip-build-cmark --build-llvm=0 --android --android-ndk $NDK --android-arch ${{ matrix.arch }} --android-api-level $ANDROID_API_LEVEL --build-swift-tools=0 --native-swift-tools-path=${TOOLCHAIN}/bin --native-clang-tools-path=${TOOLCHAIN}/bin --cross-compile-hosts=android-${{ matrix.arch }} --cross-compile-deps-path=$SDK --skip-local-build --build-swift-static-stdlib --xctest --skip-early-swift-driver --install-swift --install-libdispatch --install-foundation --install-xctest --install-destdir=$SDK --swift-install-components='clang-resource-dir-symlink;license;stdlib;sdk-overlay' --cross-compile-append-host-target-to-destdir=False -b -p --install-llbuild --sourcekit-lsp --skip-early-swiftsyntax |
There was a problem hiding this comment.
You reverted $ANDROID_NDK to $NDK by mistake here.
There was a problem hiding this comment.
Oops! Fixed in f35d681
Sounds good, can you turn this into a patch instead and add it to this pull? I only use Then, don't set I only set that variable and those build flags on the CI as a test if the freshly-built SDK is working well: their build output is not actually used. As you said, we can try to get those working in a separate pull later. Finally, use your freshly-built SDK to build the packages on macOS on this CI and we can remove the dependency on the linux-built SDK. |
|
Alright, got all the Foundation tests running natively on Android: pretty good shape with 12 tests that had been failing before the Swift rewrite now fixed by upstream, 5 tests still failing, and 3 tests newly failing, mostly small stuff that doesn't matter. I will upstream a few more local tweaks and need to look into an issue with the latest trunk toolchain requiring more In between, I will review this pull; would be good to get the SDK building on macOS too, as I described in my last comment. |
I'm still banging my head against the wall with this. There are endless errors on the macOS build – I've figured out and fixed a bunch of them, but they keep coming. I'd recommend against gating this PR on getting the macOS build working, and instead we can work through that in #164. |
|
I don't understand: the last error you listed was when building llbuild, Is one of those SDK libraries failing to build again? |
.github/workflows/sdks.yml
Outdated
| ${TOOLCHAIN}/bin/swift --version | ||
| BUILD_SWIFT_PM=1 ${TOOLCHAIN}/bin/swift get-packages-and-swift-source.swift | ||
|
|
||
| SDK_NAME=$(ls | grep swift-$(echo ${{ matrix.version }} | sed "s/-2[5-9][a-e]*//")-android-${{ matrix.arch }}) |
There was a problem hiding this comment.
You need to rebase, this sed is no longer required.
.github/workflows/sdks.yml
Outdated
| "-resource-dir", "${SDK_PATH}/usr/lib/swift", | ||
| "-tools-directory", "${NDK_PREBUILT}/bin", | ||
| "-L", "${SDK_PATH}/usr/lib", | ||
| "-L", "${SDK_PATH}/usr/lib/swift/android" |
There was a problem hiding this comment.
I don't think this flag is necessary, as the compiler should automatically add it.
.github/workflows/sdks.yml
Outdated
|
|
||
| # need to free up some space or else the emulator fails to launch: | ||
| # ERROR | Not enough space to create userdata partition. Available: 6086.191406 MB at /home/runner/.android/avd/../avd/test.avd, need 7372.800000 MB. | ||
| rm -rf */.build |
There was a problem hiding this comment.
Move this up and combine it with my last rm -rf line above, and move this comment above that too.
|
Thanks for all your work on this pull, really appreciate it and will be sure to attribute you when I announce the next SDK 6 release on the forum. I think if you rebase and address this last handful of nits, we can get this in. |
|
OK, I've rebased and addressed the recent batch of issues. Let me know if there is anything else I can do for this. |
|
Rebase and fix the one-line merge conflict, then go ahead and squash all your commits. Once it passes CI, I will look this over one last time tomorrow and merge. |
marcprux
force-pushed
the
main
branch
2 times, most recently
from
00489dd to
e7784c7
Compare
I rebased and fixed, but now I'm having a hard time squashing everything, because there are merge commits interleaved in the commit history, and it is leading to a lot of conflicts. If you want just one single commit, I could simply make a new PR, since there are only a couple of files that were changed. |
|
Hmm, you could try something like |
|
That seemed to work! Brilliant solution! |
|
Thanks, that should do it. I plan to try building the latest devel/trunk snapshots on this CI next, ie with the Foundation rewrite, so we'll have to get the SDK building on macOS too after that, or stagger the macOS package-building runs to keep using the linux-built SDKs initially. Anyway, that depends on my getting the Foundation rewrite cross-compiling on linux for Android first, so I will let you know. |
|
Heh, github is still complaining, I think because you missed a single commit of mine when you recreated this branch initially. Try repeating the above commands, but run |
You're right. I've run through it again, and now we're back to 1 commit with no conflicts. |
Yeah, I plan on working on that next. In the meantime, it probably just means running the CI a second time after the Linux builds pass, so that the macOS builds can grab the cached SDK. |
|
Thanks for all your work on this! 🥳 I had a vague goal of doing this someday, but given that I don't use macOS, would probably have given up considering how much work it took you. |
This PR adds a
macos-toolchain-testsjob to the CI workflow that builds a simple test executable on a macOS runner and runs it in an Android x86_64 emulator. The macOS job uses the SDK artifacts that are built and uploaded by the earlierbuild-sdk-and-testsjob.The PR also fixes an issue with the recent NDK 27 update where a sed command in the script was expecting a version of the form "release-2.." (e.g., "release-25c", "release-26a"), which wasn't matching "release-27". Additionally, some debugging affordances are added to the workflow as well as the
get-packages-and-swift-source.swiftscript.An example workflow result can be seen at https://github.com/marcprux/swift-android-sdk/actions/runs/10231920008