-
Notifications
You must be signed in to change notification settings - Fork 52
Add docker buildx bake and support for cmake #692
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 docker buildx bake and support for cmake #692
Conversation
This is much cleaner than maintaining a list of complet shell commands.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
Nice work @danvergara 👏 💪 There were a few suggestions from myself, Onyekachukwu-Nweke and macgyver13 (can't tag them) on the original PR that you might want to consider incorporating. #669 These suggestions reduce target duplication and make the |
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.
Looks good so far, one comment below. I'm going to run a bunch of builds on a remote server today in the background and will report back how it all goes.
target "cmake-base" { | ||
inherits = ["maintained-base"] | ||
dockerfile = "./Dockerfile.dev" | ||
args = { | ||
BUILD_ARGS = "-DBUILD_TESTS=OFF -DBUILD_GUI=OFF -DBUILD_BENCH=OFF -DBUILD_FUZZ_BINARY=OFF -DWITH_ZMQ=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.
does this need amd and arm platforms?
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.
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.
Yeah, it only rewrites what's needed for the cmake
build, other than that, it inherits from the maintained-base
target.
Thanks @deadmanoz. Let me address those suggestions later today after work. |
# build the dummy image that will crash on 5k invs | ||
docker buildx bake bitcoin-5k-inv | ||
|
||
# build the same image, but set platform to only linux/amd64 | ||
docker buildx bake bitcoin-5k-inv --set bitcoin-5k-inv.platform=linux/amd64 |
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.
should also mention --load or --push ?
Or actually, see src/warnet/image_build.py
I'm not sure if these explicit docker buildx commands are necessary because of that abstract command...?
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.
Added some a few paragraphs on how to use those options.
From this branch on Ubuntu/x86 I was able to build two autotools images (v0-16-1 and inv-5k) but failed on a cmake build (master) with an OOM, which is really wild since I have 24 GB RAM on this server... but I also have 32 CPUs, so I actually wonder if
Also weird because I have built bitcoin with cmake outside of docker on this server just fine with Here's the build error:
|
@pinheadmz have you built multiple targets at the same time for arm64? I think build in series is the way to go. I have pipelines that cross-build images at work (from Ubuntu/x86 to linux/arm64) that take up to 60 minutes. Btw, do you know why the test suite failed? I did not touch other that the documentation in the latest commit |
Ill restart CI, looks like a flaky test. I haven't tried building multiple targets on this server outside of guix builds which are run in parallel and take hours. |
@pinheadmz any luck in series builds? |
Tonight, I built the |
machine is busy on a guix build for right now but I'll try again later. Do you know what the dockerx flags are to force serial builds? |
My bad, seems like there's no forced serial builds, only parallelism management and it is done via Buildkit configuration. https://docs.docker.com/build/buildkit/configure/#max-parallelism Another way might using be multiple docker enabled remote builder nodes: https://www.ivonet.nl/2022/04/10/docker-multi-platform-build-with-buildx/ |
Ok i set parallelism to |
ACK -- OK great thank you! |
Replaces #669 and rewrites #673
Changes
<=28.1
images as it is today, due to the removal of theautosgen.sh
file in favor ofcmake
to build Bitcoin.Dockerfile
to support the new way to compile Bitcoin and a couple of new “base” targets to thedocker-bake.hcl
file that both inherit from the maintained-base target>28.1
and uses the newDockerfile.dev
file. The name can be changed to anything else, just the first option that came to mindBUILD_ARGS
field was modified to support new parameters to build Bitcoin usingcmake