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

Proper Mac M1 support, updated node, npm and yarn #204

Closed
wants to merge 1 commit into from
Closed

Proper Mac M1 support, updated node, npm and yarn #204

wants to merge 1 commit into from

Conversation

iamironz
Copy link

As "uname" syscall in M1 Mac always returns "x86_64" on some Java builds, this value it's not possible to determine in a general way (as previously was described in osArch field lazy block) correct architecture. The good way is first to check for osName and then decide the arch.

  • Fixed checking for darwin aarch64
  • Updated node, npm, and yarn to be able to download darwin aarch64 versions

@deepy
Copy link
Member

deepy commented Dec 15, 2021

Hi @iamironz, thank you for the PR, but I'd just like to confirm something first, the osArch being used is the os.arch property from the JVM being run, uname is only used on specific platforms and when we can't rely on os.arch
This takes that into account right?

On M1 macs (with the correct JVM) os.arch should be aarch64 according to this comment: #154 (comment)

We also have a task in this project that can be used to test these:

tasks.register("runParameterTest", JavaExec::class.java) {
classpath = sourceSets["main"].runtimeClasspath
main = "com.github.gradle.node.util.PlatformHelperKt"
}

I'm hoping to be able to take a good look at it tomorrow, the logic here is a bit hairy to read through :P

@deepy
Copy link
Member

deepy commented Dec 15, 2021

oh and there'll probably be test failures from updating the node/npm/yarn versions in the tests

@iamironz
Copy link
Author

iamironz commented Dec 15, 2021

Hey @deepy !

This takes that into account right?

Exactly, I've debugged your plugin implementation a few times and can confirm that I've receiving uname = "x86_64" on my Mac M1 machine with such jdk 11 build like:

openjdk version "11.0.11" 2021-04-20 LTS
OpenJDK Runtime Environment Zulu11.48+21-CA (build 11.0.11+9-LTS)
OpenJDK 64-Bit Server VM Zulu11.48+21-CA (build 11.0.11+9-LTS, mixed mode)

It is correct jvm version as the java -XshowSettings:properties -version returns aarch64 as value for os.arch field, also the process itself is Apple kind. Means the build of jdk is valid and system call still returns invalid arch name. Just to add the fallback would be better and safe at my opinion.

@deepy
Copy link
Member

deepy commented Dec 15, 2021

Could you test it with the runParameterTest task just to be certain? We're executing uname -m directly from Java so if Java isn't running under Rosetta then uname should say arm64 and if Java is running under Rosetta then os.arch shouldn't say aarch64.

But I also found that a lot of people have Gradle running under a different JDK without knowing about it and if you are running under Rosetta (or the JDK used by Gradle is) then using the arm node is going to fail.

Looking at PlatformHelperTester I do notice that while we don't have a specific test for M1 Macs, we do have one that does cover os.arch = aarch64 + uname = arm64 which is the expected outcome on a M1 Mac

@iamironz
Copy link
Author

iamironz commented Dec 16, 2021

Yeah, the output is:

Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: arm64
You're not on windows (isWindows == false)

It is interesting why conditions sometimes returns then x86_64.

As I said I'm using arm64 compatible build of Azul jdk, my java processes is not running under Rosetta 2.

@iamironz
Copy link
Author

iamironz commented Dec 16, 2021

Stranger things happened now: uname sometimes returns aarch64 but sometimes x86_64

@iamironz
Copy link
Author

Yes, I can confirm that the problem with uname call in some JDK: in some Java JDK runtimes suddenly started returning x86_64 (Azul JDK 8 for example, Azul JDK 11 has no problem).The build is for Apple Silicon, definitely, I've double-checked twice.

@deepy
Copy link
Member

deepy commented Dec 16, 2021

That's where runParameterTest comes in handy :-)

Since it shows all parameters and their result.

But I suspect that if you were to try and use an arm64 binary when uname returns x86_64 it would fail as its running under Rosetta.

I wonder where something goes wrong, maybe they've made a mistake with their jdk8?
It'd be interesting to be see the runParameterTest result from one of those where it fails, because it might show something funny in os.arch

@iamironz
Copy link
Author

iamironz commented Dec 20, 2021

Hey!

I wonder where something goes wrong, maybe they've made a mistake with their jdk8?

No, definitely, all my JDK's are all running In Apple environment. I may admit that JDK 11 is also affected by this issue. Proof:

As you can see only one instance of JRE are running and the kind are Apple instead of Intel:

Screen Shot 2021-12-20 at 1 40 29 PM

Screen Shot 2021-12-20 at 1 40 52 PM

In one of my projects with Gradle gradle-7.2 it is after the few iteration of compilation start reporting for me x86_64 :)

Screen Shot 2021-12-20 at 1 41 14 PM

It was totally broken :D

@erdi
Copy link

erdi commented Jan 4, 2022

A person on my team who is using an M1 Mac is reporting the same issue where things worked for them yesterday when it comes to downloading node but today they are getting:

* What went wrong:
Could not determine the dependencies of task ':frontend:nodeSetup'.
> Failed to query the value of task ':frontend:nodeSetup' property 'nodeArchiveFile'.
  > Could not resolve all files for configuration ':frontend:detachedConfiguration1'.
    > Could not find node-16.12.0-darwin-x86_64.tar.gz (org.nodejs:node:16.12.0).
     Searched in the following locations:
       https://nodejs.org/dist/v16.12.0/node-v16.12.0-darwin-x86_64.tar.gz

So this also looks like an intermittent issue on our end. Please let me know if there is anything I can do to help with getting a fix for this merged and released.

@deepy
Copy link
Member

deepy commented Jan 4, 2022

I've been asking around a bit to try and figure out how this could be happening and I can't really find a good answer, but if uname and os.arch are providing a mismatch we probably need to listen to uname

i.e. if uname says we're x64 then we need to assume we're running under Rosetta and get the x64 binary since the arm ones won't work.

I don't have a M1 Mac to test things out on so I can't confirm these but @erdi to get more output you could add something like this:

def parameterTest = tasks.register("runParameterTest"){
   doLast {
       def instance = com.github.gradle.node.util.PlatformHelper.INSTANCE
       project.logger.lifecycle("Your os.name is: '${System.getProperty("os.name")}' and is parsed as: ${instance.osName}")
       project.logger.lifecycle("Your os.arch is: '${System.getProperty("os.arch")}' and is parsed as: ${instance.osArch}")
   }
}

tasks.named("nodeSetup").configure {
    finalizedBy(parameterTest)
}

If that ends up saying Your os.arch is: 'aarch64' and is parsed as: x86_64 then I have a hat to eat and some tests to write

@CodeClark
Copy link

Hey @deepy
I am the person getting the issue that @erdi mentioned, and I can provide the result:

./gradlew runParameterTest

> Task :frontend:runParameterTest
Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: x86_64

I hope this helps.

@deepy
Copy link
Member

deepy commented Jan 4, 2022

That is so bizarre :D

One easy way to test this would be to clone this repository (and use the branch master) and add it to settings.gradle under pluginManagement as an includedBuild (the plugin kind)

And then edit PlatformHelper's open val osArch to only return x64 if that works it'd be great if you could also try arm64
Both times making certain that the runParameterTest continues to say aarch64

@CodeClark
Copy link

Of course, when I try to run a test with the includeBuild method, it magically reverts to working again.

This must have been what happened to me yesterday, as I was initially running our build fine, and it only occurred later in the day after I ran the build a few times.

I checked the output of the runParameterTest task when my build works, it looks like this:

> Task :frontend:runParameterTest
Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: arm64

I have walked through everything I can think of that I did yesterday when it happened and have not yet been able to trigger it again so far.

I still tried the tests as you suggested, even though my build is magically working again:

-when open val osArch returns x64, our build is able to download node frontend/.gradle/nodejs/node-v16.12.0-darwin-x64
The result of the runParameterTest task

Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: x64

-when open val osArch returns arm64 our build is able to download node frontend/.gradle/nodejs/node-v16.12.0-darwin-arm64
The result of the runParameterTest task

Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: arm64

If the issue comes back, I will try to dig deeper by using the includeBuild method you suggested.

If there is anything else you want me to try let me know 👍🏼

@iamironz
Copy link
Author

@deepy Hey! What's the status of this PR? Users still facing issues on M1

@deepy
Copy link
Member

deepy commented Jan 13, 2022

@iamironz this specific one has failing tests, but I'll push a branch tomorrow and hopefully create an init-script so it could be tested without modifying the build.

afaik, if uname reports x86_64 we're running under Rosetta and using an arm64 binary is not going to work so it's going to resolve to x64
(although I suppose the opposite would work and if you have a good way of reproducing the issue then testing it would be great!)

deepy added a commit that referenced this pull request Jan 15, 2022
Even if we were launched by aarch64 Java, see #204 for discussion.
@iamironz
Copy link
Author

iamironz commented Jan 20, 2022

@deepy Again: I'm not running any Rosetta (means x86) builds of JDK. It is a bug of JDK or the libc (I don't know actually but the bug definitely exists) in the current days which indicates invalid uname call output and why we need then download the Rosetta compatible version (which artificially degrades the performance) of node instead of running in the normal mode? Please, implement a solution which based on this bug regarding the system problems instead of fixing this by degrading the performance significantly.

@deepy
Copy link
Member

deepy commented Jan 20, 2022

I get that you're not running under Rosetta intentionally but I'm not finding any references to people having this issue without running under Rosetta.

I guess the proper solution is checking the output of sysctl sysctl.proc_translated and then pick accordingly

@deepy deepy mentioned this pull request Feb 10, 2022
deepy added a commit that referenced this pull request Feb 10, 2022
Even if we were launched by aarch64 Java, see #204 for discussion.
@deepy
Copy link
Member

deepy commented Feb 10, 2022

I'm currently recovering from a fever but will be releasing a minor with some fixes, currently I've added the x86_64 since I know for certain it would work in all cases.

I want to replace it with checking against sysctl sysctl.proc_translated but don't trust myself to implement that in my current state (and with no access to a m1 mac), the check against proc_translated should be guaranteed to come out right though so it would solve this satisfactorily I think

@deepy
Copy link
Member

deepy commented Mar 3, 2022

My next work computer seems like it's going to be a M1 Mac, so I'll be able to have a better look then

@gtebrean
Copy link

gtebrean commented Mar 23, 2022

Hi, is there any update on this one?

I ran runParameterTest from master version on a M1 pro and I got the following:

Task :runParameterTest
Your os.name is: 'Mac OS X' and is parsed as: darwin
Your os.arch is: 'aarch64' and is parsed as: arm64
You're not on windows (isWindows == false)

but with all these it still fails with:

  • What went wrong:
    Could not determine the dependencies of task ':nodeSetup'.

    Failed to query the value of task ':nodeSetup' property 'nodeArchiveFile'.
    > Could not resolve all files for configuration ':detachedConfiguration1'.
    > Could not find org.nodejs:node:14.15.4.
    Searched in the following locations:
    - https://nodejs.org/dist/v14.15.4/node-v14.15.4-darwin-arm64.tar.gz

UPDATE

Tried to do some debug and I think I spot the cause. In my opinion for M1 final name of the node artefact should be like node--darwin-x64.tar.gz, at least for my case.
You can see in the picture that I've hardcoded the path and I could successfully install it.

Screenshot 2022-03-23 at 20 34 57

The thing is that I'm not that familiar with kotlin so I'm not currently able to make all the tests run and build properly. If any of you could help me I will really appreciate it!

@deepy
Copy link
Member

deepy commented Mar 23, 2022

@gtebrean if you're on 3.2.1 there's a fix available.
But the issue you're seeing is that node 14 doesn't have M1 (official) builds, if you upgrade to 16+ it will work fine

@gtebrean
Copy link

@gtebrean if you're on 3.2.1 there's a fix available. But the issue you're seeing is that node 14 doesn't have M1 (official) builds, if you upgrade to 16+ it will work fine

Thank you so much for the hint, now it finally worked! Maybe it will help if this project readme will be updated with this reference.

This pull request was closed.
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.

5 participants