[libcxx-commits] [PATCH] D144575: [libc++][CI] Improves Dockerfile

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added a comment.
This revision now requires changes to proceed.

I like that we are removing the LLVM 16 todo, but I am not sold on the other changes to the Dockerfile. I'd like to understand your rationale for them.



================
Comment at: libcxx/utils/ci/Dockerfile:62
 ENV LLVM_HEAD_VERSION=17
-RUN apt-get update && apt-get install -y lsb-release wget software-properties-common
-RUN wget https://apt.llvm.org/llvm.sh -O /tmp/llvm.sh
-RUN bash /tmp/llvm.sh $(($LLVM_HEAD_VERSION - 3)) # for CI transitions
-RUN bash /tmp/llvm.sh $(($LLVM_HEAD_VERSION - 2)) # previous release
-RUN bash /tmp/llvm.sh $(($LLVM_HEAD_VERSION - 1)) # latest release
-RUN bash /tmp/llvm.sh $LLVM_HEAD_VERSION          # current ToT
-
-# Install clang-format; always use the lastest stable branch.
-RUN apt-get update && apt-get install -y clang-format-$(($LLVM_HEAD_VERSION - 2)) clang-format-$(($LLVM_HEAD_VERSION - 1))
-
-# Install clang-tidy
-RUN apt-get update && apt-get install -y clang-tidy-$(($LLVM_HEAD_VERSION - 2)) clang-tidy-$((LLVM_HEAD_VERSION - 1))
-
-# Install llvm-dev and libclang-dev to compile custom clang-tidy checks
-RUN apt-get update && apt-get install -y llvm-$(($LLVM_HEAD_VERSION - 1))-dev     llvm-$(($LLVM_HEAD_VERSION - 2))-dev \
-                                     libclang-$(($LLVM_HEAD_VERSION - 1))-dev libclang-$(($LLVM_HEAD_VERSION - 2))-dev
-
-# TODO LLVM16 Don't install llvm-16-dev explicitly
-RUN apt-get update && apt-get install -y llvm-16-dev libclang-16-dev
+RUN apt-get update &&                                                          \
+    apt-get install -y lsb-release wget software-properties-common &&          \
----------------
Isn't it better to have more layers? That way you get more layers of caching and can potentially re-download less when you update the image, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144575