Skip to content

fix(ci): Use homebrew installed llvm to build clp dependencies in macOS GitHub action runner (fixes #1040). #1044

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.

Sign up for GitHub

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

Closed
wants to merge 51 commits into from
Closed

fix(ci): Use homebrew installed llvm to build clp dependencies in macOS GitHub action runner (fixes #1040). #1044

Changes from 50 commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
f324686
Temporarily remove other workflows
sitaowang1998 Jun 26, 2025
a51b899
Add setting of environment variables
sitaowang1998 Jun 26, 2025
04cc82a
Revert "Temporarily remove other workflows"
sitaowang1998 Jun 26, 2025
0e29499
Bug fix
sitaowang1998 Jun 26, 2025
cfad209
Temporarily remove other GH workflows
sitaowang1998 Jun 26, 2025
753e555
Add check on PATH variable
sitaowang1998 Jun 26, 2025
15f3097
Revert "Add check on PATH variable"
sitaowang1998 Jun 27, 2025
55516f5
Use llvm
sitaowang1998 Jun 27, 2025
2f7963d
Bug fix
sitaowang1998 Jun 27, 2025
0679303
Add llvm library
sitaowang1998 Jun 27, 2025
71c0a05
Remove ld flags
sitaowang1998 Jun 27, 2025
7d534bf
Revert "Temporarily remove other GH workflows"
sitaowang1998 Jun 27, 2025
072482f
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 27, 2025
cf2c37b
Experiment with extra clang-tidy flags
sitaowang1998 Jun 27, 2025
fa5dfc2
Temporarily remove other GH workflows
sitaowang1998 Jun 27, 2025
e2434cb
Revert "Experiment with extra clang-tidy flags"
sitaowang1998 Jun 27, 2025
39961b4
Add more environment flags
sitaowang1998 Jun 27, 2025
5e9d8c6
Experiment with environment variables
sitaowang1998 Jun 27, 2025
9478ae8
Add more experiment with environment variables AR and RANLIB
sitaowang1998 Jun 27, 2025
fca4857
Remove environment flags for flags
sitaowang1998 Jun 27, 2025
67d2363
Experiment with clang-tidy argument
sitaowang1998 Jun 27, 2025
2d82296
Try add llvm to clang-tidy sysroot
sitaowang1998 Jun 28, 2025
260b69d
Revert "Try add llvm to clang-tidy sysroot"
sitaowang1998 Jun 28, 2025
c91dc84
Not use llvm toolchain in clp
sitaowang1998 Jun 28, 2025
693e2d0
Revert "Not use llvm toolchain in clp"
sitaowang1998 Jun 28, 2025
234a1f3
Experiment with extra args
sitaowang1998 Jun 28, 2025
99cdacc
Remove errno_t=int from clang-tidy flags
sitaowang1998 Jun 28, 2025
0f1034a
Bug fix
sitaowang1998 Jun 28, 2025
db7cd48
Revert "Temporarily remove other GH workflows"
sitaowang1998 Jun 28, 2025
9e2c951
Add rsize clang-tidy flag in all clang-tidy tasks
sitaowang1998 Jun 28, 2025
7e528b4
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 28, 2025
d80898d
Merge branch 'main' into macos-llvm
sitaowang1998 Jun 30, 2025
d7f348f
Merge branch 'main' into macos-llvm
sitaowang1998 Jul 7, 2025
fcb664e
Test if setting GITHUB_ENV inside script works
sitaowang1998 Jul 7, 2025
e09f0b8
Add check for GITHUB_ENV
sitaowang1998 Jul 7, 2025
ba7cd2f
Fix test
sitaowang1998 Jul 7, 2025
2ce75bd
Remove test for GITHUB_ENV
sitaowang1998 Jul 7, 2025
accd3c4
Add tests for GITHUB_ENV
sitaowang1998 Jul 7, 2025
a222074
Temporarily remove all other workflows
sitaowang1998 Jul 7, 2025
146dc62
Add GITHUB_ENV check
sitaowang1998 Jul 7, 2025
d7ff1cb
Revert "Temporarily remove all other workflows"
sitaowang1998 Jul 7, 2025
2bbec2d
Use GITHUB_ACTIONS to check
sitaowang1998 Jul 7, 2025
9cf5f4d
Revert "Use GITHUB_ACTIONS to check"
sitaowang1998 Jul 7, 2025
b49789f
Fix the spacing and shell equal
sitaowang1998 Jul 7, 2025
893aa84
Put constant first when checking variables in shell script
sitaowang1998 Jul 7, 2025
c95039c
Temporarily remove other GH workflows
sitaowang1998 Jul 8, 2025
b4665a5
Try unset GITHUB_ENV
sitaowang1998 Jul 8, 2025
618d297
Revert "Temporarily remove other GH workflows"
sitaowang1998 Jul 8, 2025
2516396
Name the workflow step to unset env vars
sitaowang1998 Jul 8, 2025
6ddb51d
Move env var to GH workflow steps
sitaowang1998 Jul 9, 2025
dcbf362
Bug fix
sitaowang1998 Jul 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .github/workflows/clp-core-build-macos.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,21 @@ jobs:
- run: "./tools/scripts/deps-download/init.sh"
shell: "bash"

# NOTE: We set up the environment variables for LLVM so that the dependencies and CLP core
# are built with clang/clang++ from homebrew installed LLVM. Need to unset for clang-tidy.
# See https://github.com/y-scope/clp/issues/1080 for details and exploration of solutions.
- name: "Set up environment variables for LLVM"
shell: "bash"
run: >-
LLVM_PREFIX=$(brew --prefix llvm@16)
{
echo "LLVM_PREFIX=$LLVM_PREFIX"
echo "CC=$LLVM_PREFIX/bin/clang"
echo "CXX=$LLVM_PREFIX/bin/clang++"
echo "AR=$LLVM_PREFIX/bin/llvm-ar"
echo "RANLIB=$LLVM_PREFIX/bin/llvm-ranlib"
} >> "$GITHUB_ENV"

Copy link
Contributor

@coderabbitai coderabbitai bot Jul 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Set LLVM env vars before the install-all.sh compilation step

install-all.sh (lines 71-73) builds several third-party libraries from source.
Because the LLVM variables are only exported afterwards (lines 80-90), that script still executes with AppleClang – exactly the scenario this PR is meant to avoid (older AppleClang lacks C++20).

Move the “Set up environment variables for LLVM” step so it precedes any build/compile work (at minimum, before install-all.sh and deps-download/init.sh).

-      - name: "Install dependencies"
-        run: "./components/core/tools/scripts/lib_install/macos/install-all.sh"
-
-      - run: "./tools/scripts/deps-download/init.sh"
-        shell: "bash"
-
-      # Set up environment variables for LLVM
+      # Set up environment variables for LLVM
+      - name: "Set up environment variables for LLVM"
+        shell: bash
+        run: |
+          LLVM_PREFIX=$(brew --prefix llvm@16)
+          {
+            echo "LLVM_PREFIX=$LLVM_PREFIX"
+            echo "CC=$LLVM_PREFIX/bin/clang"
+            echo "CXX=$LLVM_PREFIX/bin/clang++"
+            echo "AR=$LLVM_PREFIX/bin/llvm-ar"
+            echo "RANLIB=$LLVM_PREFIX/bin/llvm-ranlib"
+          } >> "$GITHUB_ENV"
+
+      - name: "Install dependencies"
+        run: "./components/core/tools/scripts/lib_install/macos/install-all.sh"
+
+      - run: "./tools/scripts/deps-download/init.sh"
+        shell: "bash"

This guarantees every subsequent compilation (dependencies, core, tests) uses the Homebrew LLVM toolchain and resolves the original C++20 build failures.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# NOTE: We set up the environment variables for LLVM so that the dependencies and CLP core
# are built with clang/clang++ from homebrew installed LLVM. Need to unset for clang-tidy.
# See https://github.com/y-scope/clp/issues/1080 for details and exploration of solutions.
- name: "Set up environment variables for LLVM"
shell: "bash"
run: >-
LLVM_PREFIX=$(brew --prefix llvm@16)
{
echo "LLVM_PREFIX=$LLVM_PREFIX"
echo "CC=$LLVM_PREFIX/bin/clang"
echo "CXX=$LLVM_PREFIX/bin/clang++"
echo "AR=$LLVM_PREFIX/bin/llvm-ar"
echo "RANLIB=$LLVM_PREFIX/bin/llvm-ranlib"
} >> "$GITHUB_ENV"
# NOTE: We set up the environment variables for LLVM so that the dependencies and CLP core
# are built with clang/clang++ from homebrew installed LLVM. Need to unset for clang-tidy.
# See https://github.com/y-scope/clp/issues/1080 for details and exploration of solutions.
- name: "Set up environment variables for LLVM"
shell: bash
run: |
LLVM_PREFIX=$(brew --prefix llvm@16)
{
echo "LLVM_PREFIX=$LLVM_PREFIX"
echo "CC=$LLVM_PREFIX/bin/clang"
echo "CXX=$LLVM_PREFIX/bin/clang++"
echo "AR=$LLVM_PREFIX/bin/llvm-ar"
echo "RANLIB=$LLVM_PREFIX/bin/llvm-ranlib"
} >> "$GITHUB_ENV"
- name: "Install dependencies"
run: "./components/core/tools/scripts/lib_install/macos/install-all.sh"
- run: "./tools/scripts/deps-download/init.sh"
shell: "bash"
🤖 Prompt for AI Agents 
In .github/workflows/clp-core-build-macos.yaml around lines 77 to 91, the
environment variables for LLVM are set after the install-all.sh script runs,
causing that script to use AppleClang instead of the intended Homebrew LLVM
toolchain. To fix this, move the entire "Set up environment variables for LLVM"
step so that it occurs before any build or compilation steps, specifically
before the install-all.sh and deps-download/init.sh scripts, ensuring all
subsequent compilations use the correct LLVM environment.

- run: "CLP_CORE_MAX_PARALLELISM_PER_BUILD_TASK=$(getconf _NPROCESSORS_ONLN) task deps:core"
shell: "bash"

Expand Down Expand Up @@ -105,6 +120,17 @@ jobs:
# violations.
key: "main-branch-${{matrix.os}}-lint:check-cpp-static-full"

- name: "Unset LLVM environment variables"
shell: "bash"
run: >-
{
echo "LLVM_PREFIX="
echo "CC="
echo "CXX="
echo "AR="
echo "RANLIB="
} >> "$GITHUB_ENV"

# TODO: When enough files are passing clang-tidy, switch to a full pass on schedule only.
# - run: >-
# task lint:check-cpp-${{(github.event_name == 'schedule') && 'full' || 'diff'}}
Expand Down
Loading