-
Notifications
You must be signed in to change notification settings - Fork 6.1k
CI: Add ARM64 builds #13305
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
CI: Add ARM64 builds #13305
Conversation
29df2b2
to
9f62758
Compare
@cameel following your instructions in #11351 (comment), note that at the moment CircleCI doesn't support docker in ARM, so you'll see the workflow is slightly different. Is this approach reasonable (if I finish the PR will be merged?). |
@cameel question for you /\ |
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.
following your instructions in #11351 (comment), note that at the moment CircleCI doesn't support docker in ARM, so you'll see the workflow is slightly different. Is this approach reasonable (if I finish the PR will be merged?).
Normally we'd add a new dockerfile to buildpack-deps/
and plug it into the buildpack-deps.yml
workflow but if CircleCI does not allow custom images then I guess what you did here is the only thing we can really do. We'd ultimately do the same thing.
So yeah, we will not reject the PR just because of that :) But there are quite a few things missing and we'll only be willing to merge it if it's complete. Check my comments below, especially #13305 (comment).
In any case thanks for the work you already put into it. If we get this to completion this will be really appreciated by users.
Closes #11351
I don't think the PR fixes the whole issue because we also need to provide builds of the older compiler versions and that's going to be a lot of separate work. So for now I'd just change it to something like Part of #11351
.
.circleci/config.yml
Outdated
environment: | ||
TERM: xterm | ||
MAKEFLAGS: -j 5 | ||
CMAKE_OPTIONS: -DCMAKE_BUILD_TYPE=Release -DUSE_Z3_DLOPEN=ON -DSTRICT_Z3_VERSION=OFF -DUSE_CVC4=OFF -DSOLC_STATIC_STDLIBS=ON |
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.
In CI we always want to run with -DSTRICT_Z3_VERSION=ON
CMAKE_OPTIONS: -DCMAKE_BUILD_TYPE=Release -DUSE_Z3_DLOPEN=ON -DSTRICT_Z3_VERSION=OFF -DUSE_CVC4=OFF -DSOLC_STATIC_STDLIBS=ON | |
CMAKE_OPTIONS: -DCMAKE_BUILD_TYPE=Release -DUSE_Z3_DLOPEN=ON -DUSE_CVC4=OFF -DSOLC_STATIC_STDLIBS=ON |
.circleci/config.yml
Outdated
@@ -1315,6 +1344,7 @@ workflows: | |||
|
|||
# Static build | |||
- b_ubu_static: *workflow_trigger_on_tags | |||
- b_ubu_static_arm: *workflow_trigger_on_tags |
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.
I think this should have some test runs. We don't have one for b_ubu_static
only because we already run a few different variations of soltest on standard Ubuntu. For ARM we should have at least one.
Also, I think we might want to have a debug build as well. We used to have them for macOS and Windows but they were slow and expensive. For ARM I see that pricing is the same as for standard linux so it's not an obstacle. As long as they are not too slow, we should have them.
So overall, please add:
b_ubu_arm
t_ubu_arm_soltest_all
t_ubu_arm_cli
t_ubu_arm_static_soltest
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.
A few more missing bits:
- We absolutely need
b_bytecode_ubu_arm
and that should be plugged intot_bytecode_compare
. - (optional) Maybe it would be worth having
t_ubu_arm_ext_hardhat
similar to the currentt_ems_ext_hardhat
? - The new binary should be included in the archive created by
c_release_binaries
. - The new platform must be mentioned in the
ReleaseChecklist
. - The new platform has to be added to solc-bin. This involves:
- Creating a directory for the new platform with list stubs.
- Adjusting the
update
script to process the binaries from it. - Adding the new platform to
t-bytecode-compare.yml
and creating a test PR to make sure it does work (this would be best done in a fork at first not to spam the repo with too many test PRs).
- This PR should have a changelog entry.
@@ -711,6 +721,25 @@ jobs: | |||
- persist_to_workspace: *artifacts_executables | |||
- gitter_notify_failure_unless_pr | |||
|
|||
b_ubu_static_arm: | |||
<<: *base_ubuntu2004_arm_large |
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.
What's the running time for each size? We'd want the one that has the best time/cost ratio with a preference toward the faster one if ratios are close.
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.
I do not know the average time as I have not access to CircleCI and have done no benchmark. That said, the large instance is the fastest one in the free tier. https://circleci.com/docs/using-arm
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.
The CI is public so you should be able to view the results (you may have to login with a github account though).
Anyway, yeah, if the CI checks in this PR are not running under the solidity org then the instance sizes may be limited. In that case we'll have to adjust it ourselves. I'm going to keep this comment open so that we don't forget.
- run: | ||
name: "install dependencies" | ||
command: sudo apt-get --quiet --assume-yes --no-install-recommends install libboost-all-dev z3 libz3-dev |
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.
I think we'll need more stuff from Dockerfile.ubuntu2004
here.
For example Z3. We have our own PPA because Z3 in Ubuntu is usually outdated. That's why you needed -DSTRICT_Z3_VERSION=OFF
but that's not something we want to keep in this PR. I guess this means we'll have to extend deps-ppa/static_z3.sh
to cover ARM too.
You'll also need evmone to run soltest.
Other stuff like hera (needed for wasm), sphinx (docs) or other deps might actually be rendundant since the default Ubuntu image is used for a lot of different jobs and this one not necessarily.
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.
@cameel can you elaborate how these packages are built and pushed to the PPA?
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.
The script I linked builds a .deb
source package and then uploads to ~ethereum/cpp-build-deps
PPA on Launchpad. We run it locally whenever a new version of Z3 is released.
To be honest I'm only vaguely familiar with it. I know we run the script when we update Z3 but so far I never had to do it myself and I don't know much about how it's set up. In particular not sure if you could fit an ARM version into the same PPA or if we'd need a separate one.
Actually, I see that the source package has Architecture: any
so it's possible that it might be as simple as selecting ARM from some list when the source package is uploaded. I remember that when @Marenz last ran the other PPA script (for solc) he had to choose an Ubuntu version. @leo Do you know how it works?
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.
I was initially compelled to simply change Architecture: any
but in this case the source code must be compiled in the target architecture, then packed and uploaded. Meaning if I change and someone runs the script for ARM from an X86 the library files won't work.
As for the PPA, like most modern artifact storage, it can receive multiple packages from multiple architectures. Launchpad won't have any problem receiving packages for ARM.
9f62758
to
0d125d2
Compare
0d125d2
to
23ec08e
Compare
@willianpaixao Do you want to continue working on this? Need any help? |
I'm going to close this but feel free to reopen if you'd like to resume working on this (or anyone else for that matter). If not, we'll eventually do it ourselves, though at the moment there are just too many topics and this one is not the top priority so it has to wait for its turn. |
Closes #11351