Skip to content
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

Reintroduce workaround for arm64 to use rosetta #278

Closed
wants to merge 1 commit into from

Conversation

JonoPrest
Copy link

HI @jfrolich and graphql-ppx team!

I would like to open a pull request to reintroduce this work around for arm64 macs to be able to use rosetta.

I'm not entirely sure what the reasoning for removing this workaround was on the latest update but I would be a very appreciative developer if it could be reintroduced.

I have also modified the if statement to only check whether process.arch is "arm64" and not to specifically require process.platform to equal "darwin". This is because using the new optimized docker for apple silicon one could be running a container with linux in an arm64 environment for example and it would still work perfectly using the graphql-ppx-linux-x64.exe.

This has been the case for me on a current project and I've had code in a virtual environment in order to work with graphql-ppx.

Thanks very much.

@@ -13,6 +13,11 @@ if (platform === "win32") {
platform = "win";
}

// Workaround for arm64 to use Rosetta for now
if (arch === "arm64") {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed this code from:

if (platform === "darwin" && arch === "arm64") {
  arch = "x64";
}

So that it will assign the x64 file no matter what platform. The platform could vary based on docker containers so darwin is not the only possible platform.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't e.g. a raspberryPi also be process.arch == "arm64"?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah - yes of course. Its not so simple :(

I will close the PR for now and just use my own workaround script. Thanks!

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.

2 participants