Make run-clang-tidy.sh work on macOS (#8416)

alexreinking · web-flow · commit 07fecc9b6ff8 · 2024-09-09T19:08:09.000-04:00
diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml
@@ -1,13 +1,14 @@
 name: Halide Presubmit Checks
 on:
-  # We don't want 'edited' (that's basically just the description, title, etc)
+  # We don't want 'edited' (that's basically just the description, title, etc.)
   # We don't want 'review_requested' (that's redundant to the ones below for our purposes)
   pull_request:
-    types: [opened, synchronize, reopened]
+    types: [ opened, synchronize, reopened ]
     paths:
       - '**.h'
       - '**.c'
       - '**.cpp'
+      - 'run-clang-tidy.sh'
 
 permissions:
   contents: read
@@ -23,29 +24,17 @@ jobs:
           source: '.'
           extensions: 'h,c,cpp'
           clangFormatVersion: 17
-  # As of Aug 2023, the macOS runners have more RAM (14GB vs 7GB) and CPU (3 cores vs 2)
-  # than the Linux and Windows runners, so let's use those instead, since clang-tidy is
-  # a bit of a sluggard
   check_clang_tidy:
     name: Check clang-tidy
-    runs-on: ubuntu-20.04
+    runs-on: macos-14
     steps:
       - uses: actions/checkout@v3
       - name: Install clang-tidy
-        run: |
-          # from apt.llvm.org
-          # wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
-          sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 15CF4D18AF4F7421
-          sudo apt-add-repository "deb https://apt.llvm.org/$(lsb_release -sc)/ llvm-toolchain-$(lsb_release -sc)-17 main"
-          sudo apt-get update
-          sudo apt-get install llvm-17 clang-17 liblld-17-dev libclang-17-dev clang-tidy-17 ninja-build
+        run: brew install llvm@17 ninja
       - name: Run clang-tidy
-        run: |
-          export CC=clang-17
-          export CXX=clang++-17
-          export CLANG_TIDY_LLVM_INSTALL_DIR=/usr/lib/llvm-17
-          export CMAKE_GENERATOR=Ninja
-          ./run-clang-tidy.sh
+        run: ./run-clang-tidy.sh
+        env:
+          CLANG_TIDY_LLVM_INSTALL_DIR: /opt/homebrew/opt/llvm@17
   check_cmake_file_lists:
     name: Check CMake file lists
     runs-on: ubuntu-20.04
diff --git a/run-clang-format.sh b/run-clang-format.sh
@@ -11,13 +11,23 @@ ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 #
 # sudo apt-get install llvm-17 clang-17 libclang-17-dev clang-tidy-17
 # export CLANG_FORMAT_LLVM_INSTALL_DIR=/usr/lib/llvm-17
+#
+# On macOS:
+#
+# brew install llvm@17
+# export CLANG_FORMAT_LLVM_INSTALL_DIR=/opt/homebrew/opt/llvm@17
+
+if [ -z "$CLANG_FORMAT_LLVM_INSTALL_DIR" ]; then
+  echo "CLANG_FORMAT_LLVM_INSTALL_DIR must point to an LLVM installation dir for this script."
+  exit 1
+fi
+
+echo "CLANG_FORMAT_LLVM_INSTALL_DIR = ${CLANG_FORMAT_LLVM_INSTALL_DIR}"
 
-[ -z "$CLANG_FORMAT_LLVM_INSTALL_DIR" ] && echo "CLANG_FORMAT_LLVM_INSTALL_DIR must point to an LLVM installation dir for this script." && exit
-echo CLANG_FORMAT_LLVM_INSTALL_DIR = ${CLANG_FORMAT_LLVM_INSTALL_DIR}
+CLANG_FORMAT="${CLANG_FORMAT_LLVM_INSTALL_DIR}/bin/clang-format"
 
-VERSION=$(${CLANG_FORMAT_LLVM_INSTALL_DIR}/bin/clang-format --version)
-if [[ ${VERSION} =~ .*version\ 17.* ]]
-then
+VERSION=$("${CLANG_FORMAT}" --version)
+if [[ ${VERSION} =~ .*version\ 17.* ]]; then
     echo "clang-format version 17 found."
 else
     echo "CLANG_FORMAT_LLVM_INSTALL_DIR must point to an LLVM 17 install!"
@@ -33,5 +43,6 @@ find "${ROOT_DIR}/apps" \
      "${ROOT_DIR}/util" \
      "${ROOT_DIR}/python_bindings" \
      -not -path "${ROOT_DIR}/src/runtime/hexagon_remote/bin/src/*" \
-     \( -name "*.cpp" -o -name "*.h" -o -name "*.c" \) -and -not -wholename "*/.*" | \
-     xargs ${CLANG_FORMAT_LLVM_INSTALL_DIR}/bin/clang-format -i -style=file
+     \( -name "*.cpp" -o -name "*.h" -o -name "*.c" \) -and -not -wholename "*/.*" \
+     -print0 | \
+     xargs -0 "${CLANG_FORMAT}" -i -style=file
diff --git a/run-clang-tidy.sh b/run-clang-tidy.sh
@@ -6,7 +6,18 @@ ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 
 usage() { echo "Usage: $0 [-j MAX_PROCESS_COUNT] [-f]" 1>&2; exit 1; }
 
-J=$(nproc)
+get_thread_count () {
+  ([ -x "$(command -v nproc)" ] && nproc) ||
+  ([ -x "$(command -v sysctl)" ] && sysctl -n hw.physicalcpu)
+}
+
+if [ "$(uname)" == "Darwin" ]; then
+  patch_file () { sed -i '' -E "$@"; }
+else
+  patch_file () { sed -i -E "$@"; }
+fi
+
+J=$(get_thread_count)
 FIX=
 
 while getopts ":j:f" o; do
@@ -30,18 +41,27 @@ if [ -n "${FIX}" ]; then
     echo "Operating in -fix mode!"
 fi
 
-# We are currently standardized on using LLVM/Clang17 for this script.
+# We are currently standardized on using LLVM/Clang 17 for this script.
 # Note that this is totally independent of the version of LLVM that you
 # are using to build Halide itself. If you don't have LLVM17 installed,
 # you can usually install what you need easily via:
 #
 # sudo apt-get install llvm-17 clang-17 libclang-17-dev clang-tidy-17
 # export CLANG_TIDY_LLVM_INSTALL_DIR=/usr/lib/llvm-17
+#
+# On macOS:
+#
+# brew install llvm@17
+# export CLANG_TIDY_LLVM_INSTALL_DIR=/opt/homebrew/opt/llvm@17
+
+if [ -z "$CLANG_TIDY_LLVM_INSTALL_DIR" ]; then
+  echo "CLANG_TIDY_LLVM_INSTALL_DIR must point to an LLVM installation dir for this script."
+  exit
+fi
 
-[ -z "$CLANG_TIDY_LLVM_INSTALL_DIR" ] && echo "CLANG_TIDY_LLVM_INSTALL_DIR must point to an LLVM installation dir for this script." && exit
-echo CLANG_TIDY_LLVM_INSTALL_DIR = ${CLANG_TIDY_LLVM_INSTALL_DIR}
+echo "CLANG_TIDY_LLVM_INSTALL_DIR = ${CLANG_TIDY_LLVM_INSTALL_DIR}"
 
-VERSION=$(${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-tidy --version)
+VERSION=$("${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-tidy" --version)
 if [[ ${VERSION} =~ .*version\ 17.* ]]
 then
     echo "clang-tidy version 17 found."
@@ -52,28 +72,32 @@ fi
 
 
 # Use a temp folder for the CMake stuff here, so it's fresh & correct every time
-CLANG_TIDY_BUILD_DIR=`mktemp -d`
-echo CLANG_TIDY_BUILD_DIR = ${CLANG_TIDY_BUILD_DIR}
+CLANG_TIDY_BUILD_DIR=$(mktemp -d)
+echo "CLANG_TIDY_BUILD_DIR = ${CLANG_TIDY_BUILD_DIR}"
 
 # Specify Halide_LLVM_SHARED_LIBS=ON because some installers may provide only that.
 echo Building compile_commands.json...
-cmake -DCMAKE_BUILD_TYPE=Debug \
+cmake -G Ninja -S "${ROOT_DIR}" -B "${CLANG_TIDY_BUILD_DIR}" -Wno-dev \
+      -DCMAKE_C_COMPILER="${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang" \
+      -DCMAKE_CXX_COMPILER="${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang++" \
+      -DCMAKE_BUILD_TYPE=Debug \
       -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
       -DHalide_CLANG_TIDY_BUILD=ON \
-      -DHalide_LLVM_SHARED_LIBS=ON \
-      -DLLVM_DIR=${CLANG_TIDY_LLVM_INSTALL_DIR}/lib/cmake/llvm \
-      -S ${ROOT_DIR} \
-      -B ${CLANG_TIDY_BUILD_DIR} \
+      -DHalide_LLVM_ROOT="${CLANG_TIDY_LLVM_INSTALL_DIR}" \
       > /dev/null
 
-[ -a ${CLANG_TIDY_BUILD_DIR}/compile_commands.json ]
+[ -a "${CLANG_TIDY_BUILD_DIR}/compile_commands.json" ]
+
+# We need to remove -arch flags where -target flags also exist. These break our fake runtime compilation steps on macOS
+echo Patching compile_commands.json...
+patch_file '/-target/ s/-arch *[^ ]+//' "${CLANG_TIDY_BUILD_DIR}/compile_commands.json"
 
 # We must populate the includes directory to check things outside of src/
 echo Building HalideIncludes...
-cmake --build ${CLANG_TIDY_BUILD_DIR} -j $(nproc) --target HalideIncludes
+cmake --build "${CLANG_TIDY_BUILD_DIR}" -j "${J}" --target HalideIncludes
 
 echo Building flatbuffer stuff...
-cmake --build ${CLANG_TIDY_BUILD_DIR} -j $(nproc) --target generate_fb_header
+cmake --build "${CLANG_TIDY_BUILD_DIR}" -j "${J}" --target generate_fb_header
 
 RUN_CLANG_TIDY=${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/run-clang-tidy
 
@@ -101,19 +125,19 @@ CLANG_TIDY_HEADER_FILTER=".*/src/.*|.*/python_bindings/.*|.*/tools/.*|.*/util/.*
 echo Running clang-tidy...
 ${RUN_CLANG_TIDY} \
     ${FIX} \
-    -j ${J} \
+    -j "${J}" \
     -header-filter="${CLANG_TIDY_HEADER_FILTER}" \
     -quiet \
-    -p ${CLANG_TIDY_BUILD_DIR} \
-    -clang-tidy-binary ${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-tidy \
-    -clang-apply-replacements-binary ${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-apply-replacements \
+    -p "${CLANG_TIDY_BUILD_DIR}" \
+    -clang-tidy-binary "${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-tidy" \
+    -clang-apply-replacements-binary "${CLANG_TIDY_LLVM_INSTALL_DIR}/bin/clang-apply-replacements" \
     ${CLANG_TIDY_TARGETS} \
     2>&1 | grep -v "warnings generated" | sed "s|.*/||"
 
 RESULT=${PIPESTATUS[0]}
 
-echo run-clang-tidy finished with status ${RESULT}
+echo "run-clang-tidy finished with status ${RESULT}"
 
-rm -rf ${CLANG_TIDY_BUILD_DIR}
+rm -rf "${CLANG_TIDY_BUILD_DIR}"
 
-exit $RESULT
+exit "${RESULT}"
