-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add Ubuntu22.04 build, simplify workflow, and cleanup a couple things in the script #4
Conversation
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.
Thanks for the PR! I've left a few comments, some stylistic nits but others are substantial, so it'd be great if you could address those.
create-toolchain
Outdated
if [[ "${is_mac}" -eq 1 ]]; then | ||
cp -a "${mac_toolchain}"/usr/lib/{swift-5.0,arc} linux/iphone/lib/ | ||
fi |
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.
What's the rationale behind this change? arclite is arguably less relevant now, but including swift-5.0 is still necessary if back-deploying to iOS < 12.2.
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.
They're not included in the OSX toolchain provided by Apple -- https://github.com/L1ghtmann/swift-toolchain-linux/actions/runs/3506320032/jobs/5873283058#step:3:5360
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.
hmm interesting, thanks for flagging — in that case we should either emit a warning that the toolchain won't include pre-12.2 compat, or not support building on Linux. There's also a third option which is getting the 5.0 libs from somewhere else (since they're effectively frozen in time now) but I'm not sure what a good official source would be.
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.
re the third option, we might be able to extract them from https://download.swift.org/swift-5.0-release/xcode/swift-5.0-RELEASE/swift-5.0-RELEASE-osx.pkg
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.
Unsure which 5.0 directories need to be included, so just went with a warning for the time being.
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.
Love the release.yml cleanup 🙌
Tested successfully on both osx and linux