-
Notifications
You must be signed in to change notification settings - Fork 15.6k
[libcxx] Do not use libcxx-builder-base image for Android image #168756
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
[libcxx] Do not use libcxx-builder-base image for Android image #168756
Conversation
|
@llvm/pr-subscribers-libcxx Author: Aiden Grossman (boomanaiden154) ChangesThe Android image did not use this prior to Full diff: https://github.com/llvm/llvm-project/pull/168756.diff 1 Files Affected:
diff --git a/libcxx/utils/ci/docker/android-builder.dockerfile b/libcxx/utils/ci/docker/android-builder.dockerfile
index 9c5d5047dbb86..e0eef49a8987d 100644
--- a/libcxx/utils/ci/docker/android-builder.dockerfile
+++ b/libcxx/utils/ci/docker/android-builder.dockerfile
@@ -27,8 +27,7 @@
#
# BUILDKITE_AGENT_TOKEN=<token>
-ARG BASE_IMAGE_VERSION
-FROM ghcr.io/llvm/libcxx-linux-builder-base:${BASE_IMAGE_VERSION}
+FROM docker.io/library/ubuntu:jammy AS android-builder-base
ARG ANDROID_CLANG_VERSION
ARG ANDROID_CLANG_PREBUILTS_COMMIT
|
The Android image did not use this prior to bf07226 and this will significantly increase the size of the container image for dependencies we do not need.
cbad68a to
75cbb94
Compare
ldionne
left a comment
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.
For some reason I thought we needed it, but examination seems to confirm what you say.
This seems fine to me. However, in that case, do we even need a base image at all? Can we fold the base image into the normal linux image to simplify things further?
We use the base image currently to contain everything but the Github runner binary so that we can push the base image and the github runner image separately. This allows pulling in an old base image and updating the Github runner. We can fold them once we have enabled setting the container in the Github workflow working (still on the backlog). |
ldionne
left a comment
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.
LGTM but the Docker image build was failing when I checked. Also, were you able to test this (i.e. run a build within this image)?
Yeah. Looks like we're missing a couple system dependencies that were coming in from the base image after the refactoring. I'll reintroduce that step. I haven't tested it yet, but can make sure I do so before landing. |
|
Looks like we were using this before bf07226, so this is not a regression. I'm going to close this for now. |
The Android image did not use this prior to
bf07226 and this will significantly increase the size of the container image for dependencies we do not need.