Skip to content
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

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

Merged
merged 2 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 8 additions & 19 deletions .github/workflows/presubmit.yml
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
25 changes: 18 additions & 7 deletions run-clang-format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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!"
Expand All @@ -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
68 changes: 46 additions & 22 deletions run-clang-tidy.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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."
Expand All @@ -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

Expand Down Expand Up @@ -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}"
Loading