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

Add macOS arm64 support to 0.5.x #390

Merged
merged 4 commits into from Oct 28, 2020

Conversation

alloy
Copy link
Contributor

@alloy alloy commented Oct 11, 2020

Summary

Build macOS artefacts for macOS 10.13 and with arm64 slices.

Test Plan

Screenshot 2020-10-22 at 19 27 07

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Oct 11, 2020
hermes.podspec Outdated
@@ -22,14 +24,14 @@ end

Pod::Spec.new do |spec|
spec.name = "hermes"
spec.version = "0.5.1"
spec.version = "0.5.2-rc.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed this to be inline with the current version in this branch, but this should likely change before the actual release?

@alloy
Copy link
Contributor Author

alloy commented Oct 12, 2020

Ok, build is green. Will test on a DTK later today.

@alloy
Copy link
Contributor Author

alloy commented Oct 22, 2020

@Huxpro This is tested and good to go. The one thing that would need a change is the version, I guess it can just be v0.5.2 or v0.5.3?

@Huxpro
Copy link
Contributor

Huxpro commented Oct 22, 2020

@alloy hey i'm coming back to this!

The one thing that would need a change is the version, I guess it can just be v0.5.2 or v0.5.3?

Hm i'm not sure how does the arm64 support work to tell this. What does adding arm64 to the CMAKE_OSX_ARCHITECTURES do? It seems like there is still only one destroot/Library/Frameworks/hermes.framework/Versions/0/hermes in the hermes-runtime-darwin tarball built from the CircleCI of this PR, and it is still an x86_64 executable? (Well actually, I realized Idk anything about the darwin thing here...apparently it's different with the libhermes.so used by Android).

Basically, we used rc1 in the 0.5.2-rc1 for Proxy because it's an opt-in npm package instead of being a default. npm install hermes-engine@~0.5.0 can pick 0.5.2 but not 0.5.2-rc1.

So if the npm package after this PR can work for both x86 and ARM out of the box I will say 0.5.3. If it's not going to work with x86 I will say 0.5.3-blah (rc1, or mac-arm, idk) and let RN macOS ARM somehow opt in the hermes-engine@0.5.3-blah.

@Huxpro Huxpro self-assigned this Oct 22, 2020
@alloy
Copy link
Contributor Author

alloy commented Oct 23, 2020

What does adding arm64 to the CMAKE_OSX_ARCHITECTURES do? It seems like there is still only one destroot/Library/Frameworks/hermes.framework/Versions/0/hermes in the hermes-runtime-darwin tarball built from the CircleCI of this PR, and it is still an x86_64 executable?

Mach-O binaries can be “fat” binaries, which mean they can contain multiple slices to support different architectures.

In the case of these artefacts:

$ file hermes-runtime-darwin-v0.5.2-rc1/destroot/bin/hermesc 
hermes-runtime-darwin-v0.5.2-rc1/destroot/bin/hermesc: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit executable x86_64] [arm64]
hermes-runtime-darwin-v0.5.2-rc1/destroot/bin/hermesc (for architecture x86_64):	Mach-O 64-bit executable x86_64
hermes-runtime-darwin-v0.5.2-rc1/destroot/bin/hermesc (for architecture arm64):	Mach-O 64-bit executable arm64

$ file hermes-runtime-darwin-v0.5.2-rc1/destroot/Library/Frameworks/hermes.framework/Versions/0/hermes 
hermes-runtime-darwin-v0.5.2-rc1/destroot/Library/Frameworks/hermes.framework/Versions/0/hermes: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit dynamically linked shared library x86_64] [arm64]
hermes-runtime-darwin-v0.5.2-rc1/destroot/Library/Frameworks/hermes.framework/Versions/0/hermes (for architecture x86_64):	Mach-O 64-bit dynamically linked shared library x86_64
hermes-runtime-darwin-v0.5.2-rc1/destroot/Library/Frameworks/hermes.framework/Versions/0/hermes (for architecture arm64):	Mach-O 64-bit dynamically linked shared library arm64

So if the npm package after this PR can work for both x86 and ARM out of the box I will say 0.5.3.

Yup, it will work for both archs 👍

Basically, we used rc1 in the 0.5.2-rc1 for Proxy because it's an opt-in npm package instead of being a default. npm install hermes-engine@~0.5.0 can pick 0.5.2 but not 0.5.2-rc1.

Ah gotcha, good for me to at least understand the reason for that being a prerelease 👍

Copy link
Contributor

@Huxpro Huxpro left a comment

Choose a reason for hiding this comment

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

@alloy thanks for the elaboration! TIL the fat binaries!

Yeah then let's finalize this as 0.5.3 so we can have artifacts built with the correct version number ;)

hermes.podspec Outdated Show resolved Hide resolved
@alloy
Copy link
Contributor Author

alloy commented Oct 27, 2020

Updated the versions to 0.5.3.

CMakeLists.txt Outdated Show resolved Hide resolved
@Huxpro Huxpro merged commit 564c64b into facebook:release-v0.5 Oct 28, 2020
@alloy alloy deleted the add-macOS-arm64-to-0.5 branch October 28, 2020 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants