diff --git a/.github/workflows/presubmit.yml b/.github/workflows/presubmit.yml index e30a606bd8d0..1b6b549214f4 100644 --- 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 index 9b5712c5e56a..33a0ac6d7152 100755 --- 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 \ No newline at end of file + \( -name "*.cpp" -o -name "*.h" -o -name "*.c" \) -and -not -wholename "*/.*" \ + -print0 | \ + xargs -0 "${CLANG_FORMAT}" -i -style=file \ No newline at end of file diff --git a/run-clang-tidy.sh b/run-clang-tidy.sh index baf896d3f78e..571b41a65503 100755 --- 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 \ No newline at end of file +exit "${RESULT}" \ No newline at end of file