-
Notifications
You must be signed in to change notification settings - Fork 548
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: darwin-arm64 #1116
feat: darwin-arm64 #1116
Conversation
faddat
commented
May 8, 2021
•
edited
Loading
edited
- Uses pkg from the master branch of https://github.com/vercel/pkg
- Removes compiled binaries for nodejs from the repository
can build on apple silicon if you add macos-amd64 to this. Also need to tweak the script some, but this version bump is just a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @faddat and @mangas (#1118)!
We recently decided to temporarily rm ARM support by another PR (not merged to develop yet) because we don't have a solid way of producing ARM binaries in a regular developer machine (amd64).
I think updating AMD binaries and leaving ARM ones out-of-date (because we can't produce them) seems unconsistent and will likely lead to bugs because they'll be pointing to different versions of nodetime.
I suggest to discover another way to produce these binaries for both architectures that can be done at all of our machines. We can also utilize Docker for this.
For anyone who's willing to investigate, there is this buildx
feature of Docker, maybe it can be a solution. Please see:
What do you think?
What about pre compiling the binaries and making them available to the cache this way? There are couple of managed services that would also enable building on m1. I think it would be a shame to not support this just because it can't be built on the developer machine, which btw could be a security flaw since the binaries can be built locally and are not checked against any known checksums as far as I can tell. Could be interesting to explore moving this generation to the CICD side and that would also resolve the problem? |
Uh, I just noticed that I recevived a bunch of Do you guys receive any errors or warnings with/without @mangas we are not accepting pre-compiled nodetime binaries from outside collobrators for security reasons but I strongly agree that moving this part to CI for complete transparency is the way to go. We can eighter have a CI task that only compares checksums and does not change the source code of Starport or changes Starport's source code by generating these binaries. I personally prefer the first option because it's simpler and eliminates the need of creating an automated PR from the GH workflow and the review process involved by that. But if we cannot solve the errors I added above -which seems not happening on macOS according to the GH issue on |
I think that removing arm support, even temporarily, would be a pretty bad idea. EG: I would absolutely be forced to maintain a version that includes ARM support. I prefer to contribute to this Starport, AND if we are doing things right, this really isn't a question we should have (the users system architecture) One option is indeed docker buildx (I use it extensively), but those builds are going to take an incredibly long time, each time, because you would be compiling node.js using QEMU. Pain and wasted time would surely occur using buildx. I guess if I were going to question something and yeah, welcome to hell, the thing that I might question is actually the practice of embedding node.js altogether and instead what we might want to do is just support the very latest LTS, so for example, currently that would be node.js 16... The same way that we currently support only the latest release of golang, we could say that the standard is "the latest LTS release of node.js" All of this gets into what's actually the most common issue that I have seen users have with this product, and probably the most common issue in the world of the Go programming language: Development environment. Go needs a https://rustup.sh like yesterday. Obviously this is out of scope but I figure it's worth mentioning. The number of times that I have had the conversation about oh by the way you need to export such and such and you should make a folder called go under your home folder and you should add those to your bash profile oh and by the way a bash profile is blah blah blah blah blah blah blah blah blah blah..... Is really going up dramatically, to the point that I can say with confidence that it's a significant user experience issue for Starport itself. Options to retain arm support for Linux and mac
reasons to drop arm support
At one point I dug into the arm support issues surrounding cosmwasm and it's quite fair to say that I quickly became overwhelmed-- I figure that it's possible but I also figure that it's a solid month's work.... Additionally, no guarantees on that month's work. So, in regard to all my big plans to run validators on Raspberry Pi's, well those plans still work but they now must exclude every single chain that supports CosmWasm. |
Hey @faddat! Did you have chance to read my comment above? In the After seen this and now knowing that The only thing we need (a) to figure out is how to solve the error lines I mentioned in my comment regarding to Ubuntu machines or (b) we can create a special Dockerfile for creating nodetime binaries that has all the bits required for Any of a, b, c would work but it would be nicer if we can try to solve this first with option b so, this could work in all of our machines. And then we can create a CI workflow to compare checksums as @mangas suggested. I think we're pretty close to finding a solution and also we're not tight on the time for an implementation because the next release of Starport will still keep the ARM support. |
@ilgooz I think the errors you got were cause by the fact that the binary version is not available pre-compiled. See the link I provided above. The solution to this would be that you could rent an m1 and add the pre-built version to your local cache. This could be added as part of the process I think. You can use the one I shared on my PR as an example and see if it passes your build. It should not trigger the rebuild. For me it only triggered everything the first time and then just used the local artifacts so I think the options are either provide these from outside the local build and download them. The arm64 builds are now supported but probably for the same reason, pkg doesn't have a prebuilt package for that arch :( |
@mangas are you sure that pre compiled binaries aren't provided by |
The pre-compiled binaries are working for everything except m1 |
Yep, I'm sure, it's completely different when you build it, see here |
Hey, I am not following this repo as closely as I'd like but I want to suggest a way to put this issue: to bed. Probably the approach that I used here, isn't the best one. Also I just realized that coding it will be faster than describing. One caveat: Merging this would allow us to be 100% latest-node-LTS. @ilgooz Yep, I am familiar with buildx and multiplatform manifests (intimately so) but not certain how it applies here. @mangas probably has the right solution. I'll shortly submit a PR that removes the binaries from the repository altogether, and expects that they will be fetched at build time. Might affect the Makefile. wdyt? |
Completed:
|
Thanks @faddat, do you see the same logs on your machine when you run the gen-nodetime script?
|
Let me double check, but certainly don't recall seeing that. PS: recommended invocation of gen-nodetime is now: make install |
Yes, yes I do, when building in gitpods. It looks like it is printing a binary to the terminal. |
Do you have an m1 machine, how about there? |
I have an m1. Currently on an arch linux machine. Pulling out the m1 now; issue was not present when I pushed the latest commits. Found it. Will always refer to the amd64's as x64 and that should resolve it. |
@barriebyron this works on my M1 machine now. How about you? |
@ilgooz hey, my apologies. I have to brush up a thing here and will convert back from draft. |
Closing this and starting again from the develop branch. Had it going but then merged uncleanly. |
Continued in #1187 |