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

feat: macOS arm64 support #1263

Closed
wants to merge 2 commits into from
Closed

feat: macOS arm64 support #1263

wants to merge 2 commits into from

Conversation

Pantani
Copy link
Collaborator

@Pantani Pantani commented Jun 19, 2021

What does this PR does?

  • Add mac M1 support (darwin arm64);

Changes

  • Bump nodejs pkg version;
  • Add darwin arm64 nodetime binary;
  • Update linux-x64 and macos-x64 binaries;

How to test?

$ make install

@Pantani Pantani requested review from fadeev, ilgooz and lumtis as code owners June 19, 2021 04:03
@Pantani Pantani closed this Jun 19, 2021
@faddat
Copy link
Contributor

faddat commented Jun 19, 2021

Why'd you close the PR?

it is of course super interesting :)

@Pantani
Copy link
Collaborator Author

Pantani commented Jun 19, 2021

@faddat the nodetime for ARM64 is returning some errors, I'll try to fix it today and open the PR again

@faddat
Copy link
Contributor

faddat commented Jun 20, 2021

@Pantani OK-- same here, you can see #1201 .

I think that we may be missing some build information, because it ought to be the same across amd64 and arm64.

Thanks for working on this though, I think it's important.

@Pantani
Copy link
Collaborator Author

Pantani commented Jun 20, 2021

@faddat great!!
For arm64 support, we can use the Nodetime binary for amd64, the macOS can run it using rosetta, but it's not the proper way. We need to build the Nodetime compiled for macOS arm64 in the future

@ilgooz
Copy link
Member

ilgooz commented Jun 21, 2021

@faddat the nodetime for ARM64 is returning some errors, I'll try to fix it today and open the PR again

@Pantani Can you please provide more info about these errors? Are you seeing random bytes printed to the stdout?

Please check this branch, it should be able to produce binaries for all platforms without any error.

@Pantani
Copy link
Collaborator Author

Pantani commented Jun 21, 2021

@ilgooz I bumped the pkg version into the package.json for I can get the Node build for ARM64 versions:
https://github.com/Pantani/starport/blob/feat/macos-arm/scripts/data/gen-nodetime/package.json#L22

And, also, I added the ARM binary:
https://github.com/Pantani/starport/blob/feat/macos-arm/scripts/data/gen-nodetime/package.json#L10

It's working, but when I try to scaffold a new project (starport app github.com/tendermint/poll), I receive some errors related to the Typescript:

Screen Shot 2021-06-21 at 17 49 21

My current implementation

@faddat
Copy link
Contributor

faddat commented Jun 22, 2021

Ok, @Pantani are on exactly the same page, it is the exact issue that I am having.

I think that #1201 might help us get to the bottom of this. The way it feels to me is that the build machine does something we're not currently doing.

@ilgooz
Copy link
Member

ilgooz commented Jun 22, 2021

@Pantani we still need to disable byte code generation as in the branch I shared in my previous comment, otherwise build process won't be successful on linux machines even if it seems to.

For the error you mention, I beleive it's a bug caused by pkg, probably in the assets part. We're still investigating it. I don't think #1201 is relavent with this.

@faddat
Copy link
Contributor

faddat commented Jun 22, 2021

That was the right issue. I do think it's relevant.

@Pantani
Copy link
Collaborator Author

Pantani commented Jun 22, 2021

@faddat, @ilgooz I was trying to figure out how is the reason, but we need to go more deeply into the binaries built by pkg. If we compile with version 5 or greater is returning this error, IHMO, I think some configuration is changed. Do you need some help to solve it? Can we continue this thread at issue #1201 or it's better we create an issue for nodetime compilation for darwin arm64?

@faddat
Copy link
Contributor

faddat commented Jun 22, 2021

@Pantani let me check out @ilgooz's arm64 branch. He made some changes that I didn't.

@faddat
Copy link
Contributor

faddat commented Jun 22, 2021

image

@Pantani @ilgooz -- same result, compiled on an aws graviton machine.

@faddat
Copy link
Contributor

faddat commented Jun 22, 2021

And personally, I think the issue is #1201.

Everything I am seeing is pointing to "something" in the build env that we aren't aware of or doing.

Here's where I grabbed @ilgooz's attempt and merged in develop:

https://github.com/faddat/starport/tree/starport-arm-ilker

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