[libcxx][ci][NFC] Remove commented install line and disutils reference #158015
Conversation
76667c7 added distutils because "spawn" was used, which I then removed in 268a4b0. I removed it as part of removing all uses of distutils in llvm-project, tracked in llvm#54337. Python has removed distutils in its latest versions. Distutils was not being installed in the docker image but just mentioned in a commented out line. I think this line was leftover from when it was reformated into the multi-line command above. So I'm removing the whole line and relocating the comments a bit.
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: David Spickett (DavidSpickett) Changes76667c7 added distutils because "spawn" was used, which I then removed in 268a4b0. I removed it as part of removing all uses of distutils in llvm-project, tracked in #54337. Python has removed distutils in its latest versions. Distutils was not being installed in the docker image but just mentioned in a commented out line. I think this line was leftover from when it was reformated into the multi-line command above. So I'm removing the whole line and relocating the comments a bit. Full diff: https://github.com/llvm/llvm-project/pull/158015.diff 1 Files Affected:
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
index 79e11569c0d08..8e1c341c10b92 100644
--- a/libcxx/utils/ci/Dockerfile
+++ b/libcxx/utils/ci/Dockerfile
@@ -76,6 +76,9 @@ RUN sudo apt-get update \
&& sudo apt-get install -y \
tzdata
+# Install various tools used by the build or the test suite
+# TODO add ninja-build once 1.11 is available in Ubuntu, also remove the manual
+# installation below.
RUN sudo apt-get update \
&& sudo apt-get install -y \
bash \
@@ -108,9 +111,6 @@ RUN sudo apt-get update \
xz-utils \
&& sudo rm -rf /var/lib/apt/lists/*
-# Install various tools used by the build or the test suite
-#RUN apt-get update && apt-get install -y ninja-build python3 python3-distutils python3-psutil git gdb ccache
-# TODO add ninja-build once 1.11 is available in Ubuntu, also remove the manual installation.
RUN <<EOF
set -e
wget -qO /tmp/ninja.gz https://github.com/ninja-build/ninja/releases/latest/download/ninja-linux.zip
|
|
This is purely cosmetic so if landing this would trigger a bunch of container rebuilds and you'd like to avoid that, this change can be delayed as long as you like. |
| tzdata | ||
|
|
||
| # Install various tools used by the build or the test suite | ||
| # TODO add ninja-build once 1.11 is available in Ubuntu, also remove the manual |
There was a problem hiding this comment.
And off topic for this PR, but I checked this and Oracular is the first with a new enough package, but you're probably using Noble at the moment as that's the LTS.
https://packages.ubuntu.com/search?suite=all&searchon=names&keywords=ninja-build
There was a problem hiding this comment.
The comment says we need 1.11, which seems to be available with noble? I think we're still on jammy though, so an upgrade may be a good idea.
There was a problem hiding this comment.
Oh sorry, yes, I read it as wanting > 1.11.
So yeah if you are / will be on Noble, something to keep in mind.
| tzdata | ||
|
|
||
| # Install various tools used by the build or the test suite | ||
| # TODO add ninja-build once 1.11 is available in Ubuntu, also remove the manual |
There was a problem hiding this comment.
The comment says we need 1.11, which seems to be available with noble? I think we're still on jammy though, so an upgrade may be a good idea.
76667c7 added distutils because "spawn" was used, which I then removed in 268a4b0.
I removed it as part of removing all uses of distutils in llvm-project, tracked in #54337. Python has removed distutils in its latest versions.
Distutils was not being installed in the docker image but just mentioned in a commented out line. I think this line was leftover from when it was reformated into the multi-line command above.
So I'm removing the whole line and relocating the comments a bit.