[libcxx-commits] [PATCH] D155271: [libc++][Android] Add Android support to CI Docker image

Sun Jul 23 21:39:46 PDT 2023

rprichard added inline comments.

================
Comment at: libcxx/utils/ci/run-buildbot-container:24

+SKIP_PULL=0
+while [[ ${#} -gt 0 ]]; do
----------------
Mordante wrote:
> rprichard wrote:
> > Mordante wrote:
> > > Why do we need this change?
> > I needed it for testing the updated Docker image. i.e. I would build ldionne/libcxx-builder locally with the Android stuff added to it, and then I'd enter the buildbot container to run `run-buildbot android-ndk`. If I were to run `docker pull ldionne/libcxx-builder`, though, my locally-modified image would be replaced with the published one.
> > 
> > This isn't a new problem, though, so I'm wondering how the Docker image is ordinarily updated. e.g. When updating ldionne/libcxx-builder, is someone (e.g. ldionne or a bot) running tests with the new image before pushing it? The script was simpler before, so maybe the old process was to run `docker run ...` by hand, and not use this `run-buildbot-container` script.
> > 
> I just comment out that line when testing with a locally built version. I typically test a few build locally before pushing an update to Dockerhub.
I kinda dislike the practice of keeping local edits around because it makes other version control work harder. e.g. I've been rebasing/merging/splitting/amending the Android commits for a while. I think I can put a docker wrapper script in my PATH that disables `docker pull ldionne/libcxx-builder`. I might do that instead of a local edit.

I'm thinking `--no-pull` or `--local-image` might be better names for the option if it were kept.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155271/new/

https://reviews.llvm.org/D155271