-
Notifications
You must be signed in to change notification settings - Fork 33
Add Embedded Swift for Wasm support to install-and-build-with-sdk.sh
#145
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
* Add workflows for Linux static SDK and Wasm SDK builds * Update URLs and remove extra PR testing to prepare for merge
jrflat
left a comment
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 fixing the SDK terminology and adding embedded support! Left some comments on logs that could also be updated from just "static SDK" but looks good.
Just to clarify, the _wasm-embedded SDK is included in the _wasm.artifactbundle.tar.gz that's installed, right? Also, do we want to add a job that tests the embedded build in swift_package_test.yml and pull_request.yml?
Thanks!
Yes.
Those will be changed in follow up PRs, as I don't see a way for a workflow change PR to refer to a script using absolute URL changed in the same PR. Before merging we need it to refer to |
jrflat
left a comment
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.
Looks great, thanks!
Also cleaned up incorrect "SDK" naming: we're using Swift SDKs and not plain Clang or Xcode SDKs in these workflows.
Additionally "static SDK" is very non-specific, Swift SDKs for Wasm are also static, as we currently don't support dynamic linking on that platform. We should fully specify it as "Static Linux", same as in https://www.swift.org/documentation/articles/static-linux-getting-started.html