Skip to content

Upgrade runner and package architectures #314

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

Merged
merged 14 commits into from
May 12, 2025

Conversation

ChadKillingsworth
Copy link
Collaborator

@ChadKillingsworth ChadKillingsworth commented Apr 15, 2025

  • Supports arm64 architectures on both MacOS and Linux. Only arm64 MacOS builds are now supported.
  • Renames the google-closure-compiler-osx package to google-closure-compiler-macos
  • Updates NodeJS to match the current supported version set
  • Updates UPX version

@lauraharker lauraharker self-assigned this Apr 24, 2025
@lauraharker lauraharker self-requested a review April 24, 2025 22:49
@ChadKillingsworth ChadKillingsworth merged commit 1ff4e61 into master May 12, 2025
9 checks passed
@ChadKillingsworth ChadKillingsworth deleted the update-build-architectures branch May 12, 2025 20:39
@sbc100
Copy link

sbc100 commented May 14, 2025

Is there some way to avoid dropping support for x86_64 MacOS machines? I think we are going to have to continue to support running emscirpten on x86_64 mac for a while now. See emscripten-core/emsdk#1548.

@ChadKillingsworth
Copy link
Collaborator Author

I don't think we've had x86 support for quite some time. The build runner was set to runs-on: macos-latest and according to the docs not since MacOS 13 has intel chipsets been supported. This change was basically just referencing reality.

@sbc100
Copy link

sbc100 commented May 14, 2025

Github actions might not support x86 macOS by default, but it seems there are enough x86 MacOS devices in the wild that we should continue to support them. Especially since there is no reverse-Rosetta as a fallback, so instead or running binary in emulation mode such users will just see a hard crash.

Also, I'm not sure what you mean by "I don't think we've had x86 support for quite some time", that current npm release of closure compiler (the one that folks are using today) includes an x86 macOS binary.

@ChadKillingsworth
Copy link
Collaborator Author

that current npm release of closure compiler (the one that folks are using today) includes an x86 macOS binary.

But are you sure it actually works on an x86 intel Mac? I know the package.json says it does, but I believe the binary was built on an arm processor and will only execute on an ARM Mac.

@ChadKillingsworth
Copy link
Collaborator Author

With this change, Intel Mac users will fall back to utilizing the Java version. That seems very appropriate given that this is now a minority of Macs.

@sbc100
Copy link

sbc100 commented May 14, 2025

The current release (which was built 1 year, and which we ship in emscripten) does have an x86 binary in it :

$ file node_modules/google-closure-compiler-osx/compiler 
node_modules/google-closure-compiler-osx/compiler: Mach-O 64-bit executable x86_64

I guess it just happens to run on arm machines due to Rosetta.

@sbc100
Copy link

sbc100 commented May 14, 2025

With this change, Intel Mac users will fall back to utilizing the Java version. That seems very appropriate given that this is now a minority of Macs.

True, that seems like one option. However it does require those users to install the JVM, which I'm not sure ships enabled by default with macOS. Including/bundling the JVM as part of the emscripten SDK is possible, might be possible but seems like lot extra compexity/size just to run this one binary.

@ChadKillingsworth
Copy link
Collaborator Author

There's definitely an effort/reward piece to this too. This is an open-source project and keeping up with multiple npm packages and the build jobs takes effort.

@sbc100
Copy link

sbc100 commented May 15, 2025

Would you be interesting in contributions in this area? I will look into how smooth/feasible the Java fallback is for x86 users, but it may be worth our time (emscripten) to help out here instead of relying on Java.

@ChadKillingsworth
Copy link
Collaborator Author

ChadKillingsworth commented May 16, 2025

Would you be interesting in contributions in this area?

Absolutely - as long as it also comes with helping keeping up with general runner maintenance (actions need version bumps, base runner images need updated, node versions have to be bumped, etc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants