-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[GitHub][CI] Add a new container to use for the abi tests #166886
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
Conversation
This way it can be shared with another container.
|
@llvm/pr-subscribers-github-workflow Author: Tom Stellard (tstellar) ChangesThis way we don't need to install dependencies every time we run the job and it allows us to use a PGO'd compiler which performs much better than the system compiler. Full diff: https://github.com/llvm/llvm-project/pull/166886.diff 3 Files Affected:
diff --git a/.github/workflows/build-ci-container-tooling.yml b/.github/workflows/build-ci-container-tooling.yml
index 0bb8242eb35a9..da525335d2824 100644
--- a/.github/workflows/build-ci-container-tooling.yml
+++ b/.github/workflows/build-ci-container-tooling.yml
@@ -36,6 +36,9 @@ jobs:
test-command: 'cd $HOME && clang-format --version | grep version && git-clang-format -h | grep usage && black --version | grep black'
- container-name: code-lint
test-command: 'cd $HOME && clang-tidy --version | grep version && clang-tidy-diff.py -h | grep usage'
+
+ - container-name: abi-tests
+ test-command: 'cd $HOME && abi-compliance-checker --help'
steps:
- name: Checkout LLVM
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
diff --git a/.github/workflows/containers/github-action-ci-tooling/Dockerfile b/.github/workflows/containers/github-action-ci-tooling/Dockerfile
index 707bdb309b789..7e02d2309e832 100644
--- a/.github/workflows/containers/github-action-ci-tooling/Dockerfile
+++ b/.github/workflows/containers/github-action-ci-tooling/Dockerfile
@@ -47,6 +47,28 @@ RUN echo '%sudo ALL=(ALL) NOPASSWD:ALL' >> /etc/sudoers
# as root in 'ci-container-code-format' and 'ci-container-code-lint' containers
+FROM base AS ci-container-build-tools
+ARG LLVM_VERSION
+ARG LLVM_VERSION_MAJOR
+
+COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-${LLVM_VERSION_MAJOR} \
+ ${LLVM_SYSROOT}/bin/
+COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/lib/clang/${LLVM_VERSION_MAJOR}/include \
+ ${LLVM_SYSROOT}/lib/clang/${LLVM_VERSION_MAJOR}/include
+RUN ln -s ${LLVM_SYSROOT}/bin/clang-${LLVM_VERSION_MAJOR} ${LLVM_SYSROOT}/bin/clang && \
+ ln -s ${LLVM_SYSROOT}/bin/clang ${LLVM_SYSROOT}/bin/clang++
+
+RUN apt-get update && \
+ DEBIAN_FRONTEND=noninteractive apt-get install -y \
+ cmake \
+ ninja-build && \
+ apt-get clean && \
+ rm -rf /var/lib/apt/lists/*
+
+ENV CC=${LLVM_SYSROOT}/bin/clang
+ENV CXX=${LLVM_SYSROOT}/bin/clang++
+
+
FROM base AS ci-container-code-format
ARG LLVM_VERSION
@@ -63,31 +85,38 @@ USER gha
WORKDIR /home/gha
-FROM base AS ci-container-code-lint
+FROM ci-container-build-tools AS ci-container-code-lint
ARG LLVM_VERSION
ARG LLVM_VERSION_MAJOR
COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-tidy \
- /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/bin/clang-${LLVM_VERSION_MAJOR} \
${LLVM_SYSROOT}/bin/
-COPY --from=llvm-downloader /llvm-extract/LLVM-${LLVM_VERSION}-Linux-X64/lib/clang/${LLVM_VERSION_MAJOR}/include \
- ${LLVM_SYSROOT}/lib/clang/${LLVM_VERSION_MAJOR}/include
COPY clang-tools-extra/clang-tidy/tool/clang-tidy-diff.py ${LLVM_SYSROOT}/bin/clang-tidy-diff.py
-RUN ln -s ${LLVM_SYSROOT}/bin/clang-${LLVM_VERSION_MAJOR} ${LLVM_SYSROOT}/bin/clang && \
- ln -s ${LLVM_SYSROOT}/bin/clang ${LLVM_SYSROOT}/bin/clang++
+# Install dependencies for 'pr-code-lint.yml' job
+COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
+RUN pip install -r requirements_linting.txt --break-system-packages && \
+ rm requirements_linting.txt
+USER gha
+WORKDIR /home/gha
+FROM ci-container-build-tools as ci-container-abi-tests
+
RUN apt-get update && \
DEBIAN_FRONTEND=noninteractive apt-get install -y \
- cmake \
- ninja-build && \
+ abi-compliance-checker \
+ abi-dumper \
+ autoconf \
+ pkg-config && \
apt-get clean && \
rm -rf /var/lib/apt/lists/*
-# Install dependencies for 'pr-code-lint.yml' job
-COPY llvm/utils/git/requirements_linting.txt requirements_linting.txt
-RUN pip install -r requirements_linting.txt --break-system-packages && \
- rm requirements_linting.txt
-USER gha
-WORKDIR /home/gha
+RUN git clone https://github.com/universal-ctags/ctags.git && \
+ cd ctags && \
+ ./autogen.sh && \
+ ./configure && \
+ sudo make install && \
+ rm -Rf ../ctags
+
+
diff --git a/.github/workflows/llvm-abi-tests.yml b/.github/workflows/llvm-abi-tests.yml
index b0c2d32d4a41b..2418949d2ebb8 100644
--- a/.github/workflows/llvm-abi-tests.yml
+++ b/.github/workflows/llvm-abi-tests.yml
@@ -72,6 +72,8 @@ jobs:
if: github.repository_owner == 'llvm'
needs: abi-dump-setup
runs-on: ubuntu-24.04
+ container:
+ image: ${{ format('ghcr.io/{0}/ci-container-abi-tests',github.repository_owner) }}
strategy:
matrix:
name:
@@ -87,19 +89,6 @@ jobs:
ref: ${{ github.sha }}
repo: ${{ github.repository }}
steps:
- - name: Install Ninja
- uses: llvm/actions/install-ninja@42d80571b13f4599bbefbc7189728b64723c7f78 # main
- - name: Install abi-compliance-checker
- run: |
- sudo apt-get update
- sudo apt-get -y install abi-dumper autoconf pkg-config
- - name: Install universal-ctags
- run: |
- git clone https://github.com/universal-ctags/ctags.git
- cd ctags
- ./autogen.sh
- ./configure
- sudo make install
- name: Download source code
uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
with:
@@ -143,6 +132,8 @@ jobs:
abi-compare:
if: github.repository_owner == 'llvm'
runs-on: ubuntu-24.04
+ container:
+ image: ${{ format('ghcr.io/{0}/ci-container-abi-tests',github.repository_owner) }}
needs:
- abi-dump-setup
- abi-dump
@@ -163,10 +154,6 @@ jobs:
name: symbol-list
path: symbol-list
- - name: Install abi-compliance-checker
- run: |
- sudo apt-get update
- sudo apt-get -y install abi-compliance-checker
- name: Compare ABI
run: |
if [ -s symbol-list/llvm.symbols ]; then
|
.github/workflows/llvm-abi-tests.yml
Outdated
| needs: abi-dump-setup | ||
| runs-on: ubuntu-24.04 | ||
| container: | ||
| image: ${{ format('ghcr.io/{0}/ci-container-abi-tests',github.repository_owner) }} |
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.
Is this left over from testing?
Also, it looks like this doesn't exist yet since it hasn't been built/pushed. Do we want to split the workflow changes out?
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.
Do you think it should be hard-coded to llvm? I thought maybe there might be some advantage to using the repository owner here.
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.
I think I would prefer it that way. We don't support using it elsewhere, so this just adds (a small amount of) complexity for no real benefit from what I can see.
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.
I split out the workflow changes.
| test-command: 'cd $HOME && clang-format --version | grep version && git-clang-format -h | grep usage && black --version | grep black' | ||
| - container-name: code-lint | ||
| test-command: 'cd $HOME && clang-tidy --version | grep version && clang-tidy-diff.py -h | grep usage' | ||
|
|
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.
Nit: remove this newline to be consistent with the existing entries.
vbvictor
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.
Did you try running GHA action with the new container?
You can push locally-built container to Github (loginning with "classic token") and run workflow.
.github/workflows/containers/github-action-ci-tooling/Dockerfile
Outdated
Show resolved
Hide resolved
Yes, I ran a test of this on my fork. |
Co-authored-by: Baranov Victor <[email protected]>
This way we don't need to install dependencies every time we run the job and it allows us to use a PGO'd compiler which performs much better than the system compiler.