workflows/release-binaries: Use static ZSTD on macOS #109909

keith · 2024-09-25T05:38:48Z

On macOS the shared zstd library points to a homebrew install that isn't
very stable for users.

llvmbot · 2024-09-25T05:39:23Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-github-workflow

Author: Keith Smiley (keith)

Changes

On macOS the shared zstd library points to a homebrew install that isn't
very stable for users.

Full diff: https://github.com/llvm/llvm-project/pull/109909.diff

1 Files Affected:

(modified) .github/workflows/release-binaries.yml (+1-1)

diff --git a/.github/workflows/release-binaries.yml b/.github/workflows/release-binaries.yml
index 925912df6843e4..f284f04ece5b3d 100644
--- a/.github/workflows/release-binaries.yml
+++ b/.github/workflows/release-binaries.yml
@@ -126,7 +126,7 @@ jobs:
         # add extra CMake args to disable them.
         # See https://github.com/llvm/llvm-project/issues/99767
         if [ "${{ runner.os }}" = "macOS" ]; then
-          target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_COMPILER_RT_ENABLE_IOS=OFF"
+          target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_COMPILER_RT_ENABLE_IOS=OFF -DLLVM_USE_STATIC_ZSTD=ON"
           if [ "${{ runner.arch }}" = "ARM64" ]; then
             arches=arm64
           else

keith · 2024-09-25T05:39:31Z

I'm hoping we can verify from the CI jobs that run here that this fix is working e2e

tru · 2024-09-25T06:35:22Z

I wonder if we want enable static zlib while on it? otherwise LGTM.

keith · 2024-09-25T06:36:29Z

I think zlib is part of macOS, but I'd have to double check

keith · 2024-09-25T16:24:50Z

confirmed, so idk if that one is worth it or not. blah looks like testing is blocked on a real issue. i'll try to rebase and get lucky

tru · 2024-09-26T05:50:51Z

Go ahead and merge and make a cherry pick for the release branch and I can get it in before the next release.

keith · 2024-09-26T14:49:38Z

I was just hoping to get some binaries here first to do a final checkup

keith · 2024-09-26T21:31:04Z

I downloaded the binaries in from this step:

And in there build/bin/clang is fixed with this, but build/tools/clang/stage2-bins/bin/clang still has the issue, can you confirm that the former is the one we care about anyways?

tstellar · 2024-09-26T21:37:30Z

.github/workflows/release-binaries.yml

@@ -126,7 +126,7 @@ jobs:
        # add extra CMake args to disable them.
        # See https://github.com/llvm/llvm-project/issues/99767
        if [ "${{ runner.os }}" = "macOS" ]; then
-          target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_COMPILER_RT_ENABLE_IOS=OFF"
+          target_cmake_flags="$target_cmake_flags -DBOOTSTRAP_COMPILER_RT_ENABLE_IOS=OFF -DLLVM_USE_STATIC_ZSTD=ON"


You need to add a BOOTSTRAP_ prefix to this so that it is applied to the stage2 build. It may also make more sense to move this into the Release.cmake file.

thanks, trying it in Release.cmake, still scoped to only macOS (although I think there's a reasonable argument to do this on Linux as well)

tstellar

This looks good to me. It looks like we need to add the Release.cmake file to the list of files that trigger the release-binaries-all test, but that can be done in another PR.

keith · 2024-09-29T01:07:36Z

I pushed a dummy change to that just to verify so we don't have to go back and forth on it again. I put up the PR for the path changes here #110394, I was surprised to see this file is in the clang/ specific subdir, but also didn't see any other Release.cmake files to add here

keith · 2024-09-29T19:37:23Z

I downloaded the binaries from this stage again:

and it looks like this change didn't apply to either of these binaries build/bin/clang build/tools/clang/stage2-bins/bin/clang. If it had passed would it have applied to stage 3 only? testing that now

On macOS the shared zstd library points to a homebrew install that isn't very stable for users.

keith · 2024-09-30T16:40:24Z

The build got further this time, but the binaries uploaded from this step:

still have the issue. I expected a new binary here, the same 2 clangs existed, am I looking in the right place?

tstellar · 2024-09-30T16:59:26Z

You want to be downloading and testing this artifact:

keith · 2024-09-30T17:20:28Z

ah thanks, looks like this didn't work then

tstellar · 2024-09-30T17:45:30Z

If you add these 3 options to the cmake invocation in release-binaries.yml, it should give us more verbose output and make the problem easier to debug:

        -DCMAKE_VERBOSE_MAKEFILE=ON \
        -DBOOTSTRAP_CMAKE_VERBOSE_MAKEFILE=ON \
        -DBOOTSTRAP_BOOTSTRAP_CMAKE_VERBOSE_MAKEFILE=ON

keith · 2024-09-30T19:53:22Z

It looks like at least 1 issue was that CMAKE_SYSTEM_NAME isn't set at the time this configuration is loaded, I swapped it to use the host system name, which is imperfect, but probably fine. Lets see if that works

tstellar · 2024-09-30T20:57:29Z

clang/cmake/caches/Release.cmake

+if(${CMAKE_HOST_SYSTEM_NAME} MATCHES "Darwin")
+  set_final_stage_var(LLVM_USE_STATIC_ZSTD "ON" BOOL)
+endif()


If this doesn't work, I think it would be OK to set this unconditionally too.

keith · 2024-10-01T17:02:17Z

the latest commit seems to have fixed it!

keith · 2024-10-01T17:02:30Z

should I cherry pick this for future releases?

tstellar · 2024-10-01T17:04:46Z

@keith Yes, this change needs to be in the release branch or it won't take effect.

keith · 2024-10-01T17:07:30Z

#110701

llvm-ci · 2024-10-01T17:22:21Z

LLVM Buildbot has detected a new failure on builder clang-hip-vega20 running on hip-vega20-0 while building clang at step 3 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/6757

Here is the relevant piece of the build log for the reference

Step 3 (annotate) failure: '../llvm-zorg/zorg/buildbot/builders/annotated/hip-build.sh --jobs=' (failure)
...
[38/40] : && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -O3 -DNDEBUG  External/HIP/CMakeFiles/memmove-hip-6.0.2.dir/memmove.hip.o -o External/HIP/memmove-hip-6.0.2  --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --hip-link -rtlib=compiler-rt -unwindlib=libgcc -frtlib-add-rpath && cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /usr/local/bin/cmake -E create_symlink /buildbot/llvm-test-suite/External/HIP/memmove.reference_output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/memmove.reference_output-hip-6.0.2
[39/40] /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --offload-arch=gfx908 --offload-arch=gfx90a --offload-arch=gfx1030 --offload-arch=gfx1100 -xhip -mfma -MD -MT External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -MF External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o.d -o External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -c /buildbot/llvm-test-suite/External/HIP/workload/ray-tracing/TheNextWeek/main.cc
[40/40] : && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/clang++ -O3 -DNDEBUG  External/HIP/CMakeFiles/TheNextWeek-hip-6.0.2.dir/workload/ray-tracing/TheNextWeek/main.cc.o -o External/HIP/TheNextWeek-hip-6.0.2  --rocm-path=/buildbot/Externals/hip/rocm-6.0.2 --hip-link -rtlib=compiler-rt -unwindlib=libgcc -frtlib-add-rpath && cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /usr/local/bin/cmake -E create_symlink /buildbot/llvm-test-suite/External/HIP/TheNextWeek.reference_output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek.reference_output-hip-6.0.2
+ build_step 'Testing HIP test-suite'
+ echo '@@@BUILD_STEP Testing HIP test-suite@@@'
+ ninja -v check-hip-simple
@@@BUILD_STEP Testing HIP test-suite@@@
[0/1] cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/llvm-lit -sv empty-hip-6.0.2.test with-fopenmp-hip-6.0.2.test saxpy-hip-6.0.2.test memmove-hip-6.0.2.test InOneWeekend-hip-6.0.2.test TheNextWeek-hip-6.0.2.test blender.test
-- Testing: 7 tests, 7 workers --
Testing:  0.. 10.. 20.. 30.. 40
FAIL: test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test (4 of 7)
******************** TEST 'test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test' FAILED ********************

/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out --redirect-input /dev/null --summary /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.time /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek-hip-6.0.2
cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP ; /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2

+ cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP
+ /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2
/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target: Comparison failed, textual difference between 'M' and 'R'

Input 1:
Memory access fault by GPU node-1 (Agent handle: 0x560624e00170) on address (nil). Reason: Page not present or supervisor privilege.
exit 134

Input 2:
Running quads
image width = 400 height = 400
block size = (16, 16) grid size = (25, 25)
Start rendering by GPU.
Done.
quads_gpu.ppm and quads_ref.ppm are the same.
Running earth
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
earth_gpu.ppm and earth_ref.ppm are the same.
Running two_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_spheres_gpu.ppm and two_spheres_ref.ppm are the same.
Running two_perlin_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_perlin_spheres_gpu.ppm and two_perlin_spheres_ref.ppm are the same.
Step 12 (Testing HIP test-suite) failure: Testing HIP test-suite (failure)
@@@BUILD_STEP Testing HIP test-suite@@@
[0/1] cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP && /buildbot/hip-vega20-0/clang-hip-vega20/llvm/bin/llvm-lit -sv empty-hip-6.0.2.test with-fopenmp-hip-6.0.2.test saxpy-hip-6.0.2.test memmove-hip-6.0.2.test InOneWeekend-hip-6.0.2.test TheNextWeek-hip-6.0.2.test blender.test
-- Testing: 7 tests, 7 workers --
Testing:  0.. 10.. 20.. 30.. 40
FAIL: test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test (4 of 7)
******************** TEST 'test-suite :: External/HIP/TheNextWeek-hip-6.0.2.test' FAILED ********************

/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/timeit-target --timeout 7200 --limit-core 0 --limit-cpu 7200 --limit-file-size 209715200 --limit-rss-size 838860800 --append-exitstatus --redirect-output /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out --redirect-input /dev/null --summary /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.time /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/TheNextWeek-hip-6.0.2
cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP ; /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2

+ cd /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP
+ /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target /buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/External/HIP/Output/TheNextWeek-hip-6.0.2.test.out TheNextWeek.reference_output-hip-6.0.2
/buildbot/hip-vega20-0/clang-hip-vega20/test-suite-build/tools/fpcmp-target: Comparison failed, textual difference between 'M' and 'R'

Input 1:
Memory access fault by GPU node-1 (Agent handle: 0x560624e00170) on address (nil). Reason: Page not present or supervisor privilege.
exit 134

Input 2:
Running quads
image width = 400 height = 400
block size = (16, 16) grid size = (25, 25)
Start rendering by GPU.
Done.
quads_gpu.ppm and quads_ref.ppm are the same.
Running earth
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
earth_gpu.ppm and earth_ref.ppm are the same.
Running two_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_spheres_gpu.ppm and two_spheres_ref.ppm are the same.
Running two_perlin_spheres
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
two_perlin_spheres_gpu.ppm and two_perlin_spheres_ref.ppm are the same.
Running simple_light
image width = 400 height = 225
block size = (16, 16) grid size = (25, 15)
Start rendering by GPU.
Done.
simple_light_gpu.ppm and simple_light_ref.ppm are the same.
Running random_spheres

On macOS the shared zstd library points to a homebrew install that isn't very stable for users.

keith requested a review from tstellar September 25, 2024 05:38

llvmbot added the github:workflow label Sep 25, 2024

keith requested a review from tru September 25, 2024 05:39

tru approved these changes Sep 25, 2024

View reviewed changes

keith force-pushed the ks/workflows-release-binaries-use-static-zstd-on-macos branch from dd5b383 to 26c8a32 Compare September 25, 2024 16:24

tstellar reviewed Sep 26, 2024

View reviewed changes

keith force-pushed the ks/workflows-release-binaries-use-static-zstd-on-macos branch from 4d4f8a2 to 338527b Compare September 28, 2024 16:11

llvmbot added the clang Clang issues not falling into any other category label Sep 28, 2024

tstellar approved these changes Sep 29, 2024

View reviewed changes

keith added 4 commits September 29, 2024 12:37

workflows/release-binaries: Use static ZSTD on macOS

8c6c06d

On macOS the shared zstd library points to a homebrew install that isn't very stable for users.

Move to release.cmake

f849215

add test commit to trigger

3398ee0

Discard changes to .github/workflows/release-binaries.yml

c4f0368

keith force-pushed the ks/workflows-release-binaries-use-static-zstd-on-macos branch from 00cce08 to c4f0368 Compare September 29, 2024 19:37

use host system name

0008b3d

tstellar reviewed Sep 30, 2024

View reviewed changes

keith merged commit 748f540 into llvm:main Oct 1, 2024
37 of 41 checks passed

keith deleted the ks/workflows-release-binaries-use-static-zstd-on-macos branch October 1, 2024 17:02

Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Oct 3, 2024

workflows/release-binaries: Use static ZSTD on macOS (llvm#109909)

a855a51

On macOS the shared zstd library points to a homebrew install that isn't very stable for users.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

workflows/release-binaries: Use static ZSTD on macOS #109909

workflows/release-binaries: Use static ZSTD on macOS #109909

keith commented Sep 25, 2024

llvmbot commented Sep 25, 2024 •

edited

Loading

keith commented Sep 25, 2024

tru commented Sep 25, 2024

keith commented Sep 25, 2024

keith commented Sep 25, 2024 •

edited

Loading

tru commented Sep 26, 2024

keith commented Sep 26, 2024

keith commented Sep 26, 2024

tstellar Sep 26, 2024

keith Sep 28, 2024

tstellar left a comment

keith commented Sep 29, 2024

keith commented Sep 29, 2024

keith commented Sep 30, 2024

tstellar commented Sep 30, 2024

keith commented Sep 30, 2024

tstellar commented Sep 30, 2024

keith commented Sep 30, 2024

tstellar Sep 30, 2024

keith commented Oct 1, 2024

keith commented Oct 1, 2024

tstellar commented Oct 1, 2024

keith commented Oct 1, 2024

llvm-ci commented Oct 1, 2024

workflows/release-binaries: Use static ZSTD on macOS #109909

workflows/release-binaries: Use static ZSTD on macOS #109909

Conversation

keith commented Sep 25, 2024

llvmbot commented Sep 25, 2024 • edited Loading

keith commented Sep 25, 2024

tru commented Sep 25, 2024

keith commented Sep 25, 2024

keith commented Sep 25, 2024 • edited Loading

tru commented Sep 26, 2024

keith commented Sep 26, 2024

keith commented Sep 26, 2024

tstellar Sep 26, 2024

Choose a reason for hiding this comment

keith Sep 28, 2024

Choose a reason for hiding this comment

tstellar left a comment

Choose a reason for hiding this comment

keith commented Sep 29, 2024

keith commented Sep 29, 2024

keith commented Sep 30, 2024

tstellar commented Sep 30, 2024

keith commented Sep 30, 2024

tstellar commented Sep 30, 2024

keith commented Sep 30, 2024

tstellar Sep 30, 2024

Choose a reason for hiding this comment

keith commented Oct 1, 2024

keith commented Oct 1, 2024

tstellar commented Oct 1, 2024

keith commented Oct 1, 2024

llvm-ci commented Oct 1, 2024

llvmbot commented Sep 25, 2024 •

edited

Loading

keith commented Sep 25, 2024 •

edited

Loading