Support recent versions of Bazel in build-tools containers.#531
Support recent versions of Bazel in build-tools containers.#531PiotrSikora wants to merge 4 commits intoproxy-wasm:mainfrom
build-tools containers.#531Conversation
While there, increase the jobs count and memory limit. Tested with Bazel v7.7.1, v8.5.1, v8.6.0 and v9.0.1. Signed-off-by: Piotr Sikora <code@piotrsikora.dev>
PiotrSikora
requested review from
leonm1,
martijneken and
mpwarres
as code owners
|
cc @mmorel-35 |
| --jobs=1 \ | ||
| --local_ram_resources=2048 \ | ||
| --jobs=HOST_CPUS*.5 \ | ||
| --local_resources=memory=8192 \ |
There was a problem hiding this comment.
To be honest, I don't think there is a good reason to keep memory usage limited at all, since the crashes / hangs were all happening because of bugs in QEMU CPU emulation and some race conditions there... On the other hand, it doesn't feel like Bazel should require more than 8 GiB RAM to build itself.
There was a problem hiding this comment.
What do we want to do here? Using HOST_RAM*.5 might unnecessary limit available resources when building using a small container.
| # For Bazel v7.7.1 - patch MODULE.bazel and tools/cpp/unix_cc_configure.bzl to: | ||
| # - Force bazel_features v1.11.0 (avoids bzlmod macro requirement during bootstrap) | ||
| # - Configure C++ toolchain compilation flags to avoid GCC segmentation faults on s390x under QEMU emulation | ||
| COPY bazel/external/bazel-v7.7.1.patch /tmp/bazel-v7.7.1.patch |
There was a problem hiding this comment.
Can you use ARG BAZEL_VERSION to do COPY "bazel/external/bazel-v${BAZEL_VERSION}.patch" /tmp/bazel.patch this will make Dockerfile version agnostic
And use only the patch you need
There was a problem hiding this comment.
Does COPY work with non-existing files (we only have patches for v7.7.1 and v9.0.1, but not for 8.x)?
Alternatively, we could just COPY bazel/external/bazel-*.patch /tmp/ and be done with it...
There was a problem hiding this comment.
That might work.
I'm also wondering if it would be better to have a dedicated project to build this image. Then have branches per bazel major version
There was a problem hiding this comment.
Alternatively, we could just
COPY bazel/external/bazel-*.patch /tmp/and be done with it...
Actually, I think we could do:
COPY bazel/external/bazel-*.patch /tmp/
RUN if [ -f "/tmp/bazel-v${BAZEL_VERSION}.patch" ]; then
patch -p1 </tmp/bazel-v${BAZEL_VERSION}.patch;
fi
and include the patch description in the .patch files themselves to keep the Dockerfile version-agnostic.
@mmorel-35 @leonm1 any preference vs the current state? We'd be losing descriptions in the Dockerfile, but that shouldn't matter too much.
I'm also wondering if it would be better to have a dedicated project to build this image. Then have branches per bazel major version
That's probably over-engineering. This file doesn't change much, and keeping many branches would mean that even a simple change (e.g. adding cmake in #524) would require multiple PRs.
Having said that, someone should ask Bazel team to build & publish releases for s390x...
|
I'm happy to merge this as-is and make iterative improvements, since this already brings a benefit over what exists in |
3 participants
While there, increase the jobs count and memory limit.
Tested with Bazel v7.7.1, v8.5.1, v8.6.0 and v9.0.1.