-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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
[libc][workflow] address permission concern and add more comments #119320
[libc][workflow] address permission concern and add more comments #119320
Conversation
@llvm/pr-subscribers-github-workflow @llvm/pr-subscribers-libc Author: Schrodinger ZHU Yifan (SchrodingerZhu) ChangesThis patch limits the permission of pre-commit libc pipelines. It also adds detailed comments for help future modifications. Full diff: https://github.com/llvm/llvm-project/pull/119320.diff 3 Files Affected:
diff --git a/.github/workflows/libc-fullbuild-tests.yml b/.github/workflows/libc-fullbuild-tests.yml
index fb73aa392f3433..b07e94244c2e8c 100644
--- a/.github/workflows/libc-fullbuild-tests.yml
+++ b/.github/workflows/libc-fullbuild-tests.yml
@@ -1,6 +1,7 @@
# This workflow is for pre-commit testing of the LLVM-libc project.
name: LLVM-libc Pre-commit Fullbuild Tests
-
+permissions:
+ contents: read
on:
pull_request:
branches: [ "main" ]
@@ -22,7 +23,13 @@ jobs:
# cpp_compiler: g++
steps:
- uses: actions/checkout@v4
-
+
+ # Libc's build is relatively small comparing with other components of LLVM.
+ # A fresh fullbuild takes about 190MiB of uncompressed disk space, which can
+ # be compressed into ~40MiB. Limiting the cache size to 1G should be enough.
+ # Prefer sccache as it is more modern.
+ # Do not use direct GHAC access even though it is supported by sccache. GHAC rejects
+ # frequent small object writes.
- name: Setup ccache
uses: hendrikmuhs/ccache-action@v1.2
with:
@@ -30,6 +37,10 @@ jobs:
key: libc_fullbuild_${{ matrix.c_compiler }}
variant: sccache
+ # Notice:
+ # - MPFR is required by some of the mathlib tests.
+ # - Debian has a multilib setup, so we need to symlink the asm directory.
+ # For more information, see https://wiki.debian.org/Multiarch/LibraryPathOverview
- name: Prepare dependencies (Ubuntu)
run: |
sudo apt-get update
@@ -42,7 +53,9 @@ jobs:
run: |
echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT"
echo "build-install-dir=${{ github.workspace }}/install" >> "$GITHUB_OUTPUT"
-
+
+ # Configure libc fullbuild with scudo.
+ # Use MinSizeRel to reduce the size of the build.
- name: Configure CMake
run: >
cmake -B ${{ steps.strings.outputs.build-output-dir }}
diff --git a/.github/workflows/libc-overlay-tests.yml b/.github/workflows/libc-overlay-tests.yml
index d0cdfdef99fe96..8b59d76aed4a88 100644
--- a/.github/workflows/libc-overlay-tests.yml
+++ b/.github/workflows/libc-overlay-tests.yml
@@ -1,6 +1,7 @@
# This workflow is for pre-commit testing of the LLVM-libc project.
name: LLVM-libc Pre-commit Overlay Tests
-
+permissions:
+ contents: read
on:
pull_request:
branches: [ "main" ]
@@ -32,7 +33,14 @@ jobs:
steps:
- uses: actions/checkout@v4
-
+
+ # Libc's build is relatively small comparing with other components of LLVM.
+ # A fresh linux overlay takes about 180MiB of uncompressed disk space, which can
+ # be compressed into ~40MiB. MacOS and Windows overlay builds are less than 10MiB
+ # after compression. Limiting the cache size to 1G should be enough.
+ # Prefer sccache as it is modern and it has a guarantee to work with MSVC.
+ # Do not use direct GHAC access even though it is supported by sccache. GHAC rejects
+ # frequent small object writes.
- name: Setup ccache
uses: hendrikmuhs/ccache-action@v1
with:
@@ -40,12 +48,15 @@ jobs:
key: libc_overlay_build_${{ matrix.os }}_${{ matrix.compiler.c_compiler }}
variant: sccache
+ # MPFR is required by some of the mathlib tests.
- name: Prepare dependencies (Ubuntu)
if: runner.os == 'Linux'
run: |
sudo apt-get update
sudo apt-get install -y libmpfr-dev libgmp-dev libmpc-dev ninja-build
+ # Chocolatey is shipped with Windows runners. Windows Server 2025 recommends WinGet.
+ # Consider migrating to WinGet when Windows Server 2025 is available.
- name: Prepare dependencies (Windows)
if: runner.os == 'Windows'
run: |
@@ -62,6 +73,9 @@ jobs:
run: |
echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT"
+ # Use MinSizeRel to reduce the size of the build.
+ # Notice that CMP0141=NEW and MSVC_DEBUG_INFORMATION_FORMAT=Embedded are required
+ # by the sccache tool.
- name: Configure CMake
run: >
cmake -B ${{ steps.strings.outputs.build-output-dir }}
diff --git a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
index 18b0631324f8fb..e41e7a2f5ba77e 100644
--- a/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
+++ b/libc/src/__support/FPUtil/aarch64/FEnvImpl.h
@@ -59,7 +59,7 @@ struct FEnv {
((excepts & FE_UNDERFLOW) ? UNDERFLOW_F : 0) |
((excepts & FE_INEXACT) ? INEXACT_F : 0);
}
-
+# 1 "/usr/include/x86_64-linux-gnu/bits/fcntl-linux.h" 1 3 4
LIBC_INLINE static int exceptionStatusToMacro(uint32_t status) {
return ((status & INVALID_F) ? FE_INVALID : 0) |
((status & DIVBYZERO_F) ? FE_DIVBYZERO : 0) |
|
f68ae15
to
301dc4a
Compare
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. Thank you.
…vm#119320) This patch limits the permission of pre-commit libc pipelines. It also adds detailed comments to help future modifications.
This patch limits the permission of pre-commit libc pipelines. It also adds detailed comments to help future modifications.