Skip to content

[6.2] Update wasmkit toolchain build to emit universal on macOS to fix #82390 #82406

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

Jump to bottom
Merged
merged 1 commit into from
Jun 26, 2025
Merged

[6.2] Update wasmkit toolchain build to emit universal on macOS to fix #82390 #82406

merged 1 commit into from
Jun 26, 2025

Conversation

wti
Copy link
Contributor

@wti wti commented Jun 23, 2025

  • Explanation: Build wasmkit, a new toolchain executable, as universal binary on macOS using SwiftPM --arch flags. (Cherry picked from commit 6530236)
  • Scope: wasmkit is new, and users targeting wasm (and preferring wasmkit over other wasm runtimes) might be few. wasmkit can be invoked directly by users and indirectly via swift run when using the current wasm Swift-SDK's.
  • Issues: Resolves wasmkit in 6.2 toolchain is not universal on macOS #82390
  • Original PRs: Update wasmkit toolchain build to emit universal binary on macOS to fix #82390 #82393
  • Risk: The new configuration only affects macOS targets. The binary is output to a new location, but the code already introspects the location correctly. The bug and fix impacts users of wasmkit binary newly added to toolchain, directly and via swift run.
    Risk of no-fix: The Swift.org wasm "Getting Started" [1] refers explicitly to this (unreleased) 6.2 toolchain wasmkit binary. Without this fix, users on Apple silicon following those instructions get an obscure "bad CPU" error because it is built only for the host x86_64 arch.
  • Testing: @kateinoigakukun verified wasmkit is universal after change in main.
    To verify, run shell file usr/bin/wasmkit and verify outputs include x86_64 and arm64 arch.
    An outstanding question is whether to add a check that toolchain executables are universal binaries on macOS. Currently they are, except for docc.
  • Reviewers: @kateinoigakukun

[1] https://www.swift.org/documentation/articles/wasm-getting-started.html

@wti
Update wasmkit toolchain build to emit universal on macOS to fix swif…
b902bf4 
…tlang#82390

[platform_args] has platform variants of SwiftPM wasmkit build command:
- unchanged: non-macOS + set_installation_rpath -> rpath config
- added: macOS -> universal binaries using --arch

side effect ok?: binary path changes are captured by '--show-bin-path'
@wti wti requested a review from a team as a code owner June 23, 2025 06:32
@kateinoigakukun
Copy link
Member

@swift-ci test

@MaxDesiatov
Copy link
Contributor

@swift-ci test macos

@MaxDesiatov MaxDesiatov requested a review from bnbarham June 23, 2025 08:22
@wti
Copy link
Contributor Author

wti commented Jun 23, 2025

For test, please see swiftlang/swift-integration-tests#163

As a newbie, I'd like to verify logistics of getting test + fix into main + release(s):

  1. fix to swift main (done: issue, PR both completed)
  2. test to swift-integration-test main (PR-163 above)
  3. test to swift-integration-test 6.2 (new PR when 163 merged)
  4. fix to swift 6.2 (this PR)

I assume wasmkit does not belong in release/6.1 b/c it wasn't in the original.

@MaxDesiatov
Copy link
Contributor

For test, please see swiftlang/swift-integration-tests#163

Thank you so much for the fix and the test! 🙇

As a newbie, I'd like to verify logistics of getting test + fix into main + release(s):

  1. fix to swift main (done: issue, PR both completed)
  2. test to swift-integration-test main (PR-163 above)
  3. test to swift-integration-test 6.2 (new PR when 163 merged)
  4. fix to swift 6.2 (this PR)

We're done with 1 and 2 now, 3 and 4 are tested together, and 4 would have to be merged before 3.

I assume wasmkit does not belong in release/6.1 b/c it wasn't in the original.

You're correct, 6.1 didn't have it, and even if it were to see patch releases in the feature, the bar for cherry-picking to that branch is very high.

@MaxDesiatov MaxDesiatov mentioned this pull request Jun 24, 2025
Merged
@MaxDesiatov
Copy link
Contributor

swiftlang/swift-integration-tests#164
@swift-ci build toolchain

@MaxDesiatov
Copy link
Contributor

@swift-ci build toolchain macos

@MaxDesiatov
Copy link
Contributor

swiftlang/swift-integration-tests#164
@swift-ci build toolchain macos

@MaxDesiatov MaxDesiatov merged commit ea734e2 into swiftlang:release/6.2 Jun 26, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnbarham bnbarham bnbarham approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wti @kateinoigakukun @MaxDesiatov @bnbarham