diff --git a/.clang-tidy b/.clang-tidy new file mode 100644 index 0000000..717253e --- /dev/null +++ b/.clang-tidy @@ -0,0 +1,36 @@ +# Shared clang-tidy baseline for dd-trace-cpp, httpd-datadog, and nginx-datadog. +# Keep this file in sync across those repositories. +Checks: > + -*, + bugprone-assert-side-effect, + bugprone-bool-pointer-implicit-conversion, + bugprone-copy-constructor-init, + bugprone-dangling-handle, + bugprone-forwarding-reference-overload, + bugprone-inaccurate-erase, + bugprone-infinite-loop, + bugprone-macro-repeated-side-effects, + bugprone-move-forwarding-reference, + bugprone-parent-virtual-call, + bugprone-posix-return, + bugprone-string-constructor, + bugprone-undelegated-constructor, + bugprone-unused-raii, + bugprone-virtual-near-miss, + misc-unused-using-decls, + readability-delete-null-pointer, + readability-redundant-control-flow, + readability-redundant-string-cstr +# Deferred until existing findings are cleaned up: +# bugprone-argument-comment +# bugprone-use-after-move +# misc-redundant-expression +# misc-static-assert +# modernize-redundant-void-arg +# performance-move-const-arg +# performance-noexcept-move-constructor +# readability-redundant-member-init +# readability-simplify-boolean-expr +WarningsAsErrors: '*' +HeaderFilterRegex: 'mod_datadog/src/' +FormatStyle: file diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index c8cc33f..1acf570 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -20,7 +20,7 @@ ENV SYSROOT=/sysroot/${ARCH}-none-linux-musl COPY deps/nginx-datadog/build_env/CHECKSUMS /CHECKSUMS RUN apk --no-cache add alpine-sdk coreutils sudo bash samurai python3 linux-headers \ - compiler-rt clang llvm lld wget cmake make binutils musl-dev git patchelf xz lit + compiler-rt clang clang-extra-tools llvm lld wget cmake make binutils musl-dev git patchelf xz lit RUN wget https://github.com/llvm/llvm-project/releases/download/llvmorg-${LLVM_VERSION}/llvm-project-${LLVM_VERSION}.src.tar.xz && \ grep -F llvm-project-${LLVM_VERSION}.src.tar.xz /CHECKSUMS | sha512sum --check && \ tar -xvf llvm-project-${LLVM_VERSION}.src.tar.xz diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 7e542b1..39116ac 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -1,5 +1,6 @@ name: Development on: + pull_request: push: workflow_dispatch: schedule: @@ -19,6 +20,18 @@ jobs: - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 - run: ./scripts/codestyle.sh lint + clang-tidy: + runs-on: ubuntu-22.04 + container: + image: ghcr.io/datadog/httpd-datadog/devcontainer:main + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + steps: + - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1 + - name: clang-tidy + run: ./scripts/clang-tidy.sh + build: needs: format runs-on: ubuntu-22.04 diff --git a/.gitignore b/.gitignore index 8f1cd2d..c707d2e 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,7 @@ compile_commands.json __pycache__/ build-container/ build-rum/ +.clang-tidy-build/ build/ dist-container/ httpd-*/ diff --git a/CMakeLists.txt b/CMakeLists.txt index 025570b..89dbb78 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -13,6 +13,9 @@ set(ZLIB_USE_STATIC_LIBS ON) option(HTTPD_DATADOG_ENABLE_RUM "Enable RUM product" OFF) option(HTTPD_DATADOG_ENABLE_COVERAGE "Enable code coverage instrumentation" OFF) option(HTTPD_DATADOG_PATCH_AWAY_LIBC "Patch away libc dependency" OFF) +option(HTTPD_DATADOG_ENABLE_CLANG_TIDY "Run clang-tidy on first-party sources during build" OFF) +set(HTTPD_DATADOG_CLANG_TIDY "clang-tidy-17" CACHE STRING + "clang-tidy executable when HTTPD_DATADOG_ENABLE_CLANG_TIDY is ON") if (NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE ReleaseWithDeb) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9e106c2..3733efe 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,6 +4,34 @@ Follow [docs/conventions.md](doc/conventions.md). +## Format + +C++ is formatted with `clang-format-14` and `.clang-format`. + +```sh +./scripts/codestyle.sh lint # check; fails on drift +./scripts/codestyle.sh format # rewrite files +``` + +## Static Analysis + +C++ is analyzed with **clang-tidy-17** (pinned to the LLVM 17 toolchain in the +devcontainer) using the shared `.clang-tidy` baseline. Warnings are errors. + +Do not run clang-tidy on the host. Tidy must use the same compiler, flags, and +sysroot as the real build (`ci-dev`). Locally: + +```sh +make lint-tidy +``` + +That builds/pulls the devcontainer, configures CMake inside it (`ci-dev`, RUM +off, `HTTPD_DATADOG_ENABLE_CLANG_TIDY`), and builds `mod_datadog` so CMake +invokes clang-tidy-17 with the exact compile line. `rum/`, `deps/`, and tests +are not on that target. `./scripts/clang-tidy.sh` re-execs through +`make lint-tidy` when run on the host. CI runs the same script in +`ghcr.io/datadog/httpd-datadog/devcontainer:main`. + ## Clone When cloning the repo, initialize the submodules you need. For a standard build: diff --git a/Makefile b/Makefile index 1e493f1..f47cb6c 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,11 @@ test-integration: dev-image # GitHub build entrypoint. RUM is off, so inject-browser-sdk is unnecessary. PRESET ?= ci-dev + +.PHONY: lint-tidy +lint-tidy: dev-image + $(IN_DEVCONTAINER) env HTTPD_DATADOG_TIDY_IN_CONTAINER=1 ./scripts/clang-tidy.sh + .PHONY: ci-build ci-build: git config --global --add safe.directory "$(CURDIR)" diff --git a/mod_datadog/CMakeLists.txt b/mod_datadog/CMakeLists.txt index 7703647..7f5d6cc 100644 --- a/mod_datadog/CMakeLists.txt +++ b/mod_datadog/CMakeLists.txt @@ -27,6 +27,10 @@ add_library( ) set_property(TARGET mod_datadog PROPERTY POSITION_INDEPENDENT_CODE ON) +if(HTTPD_DATADOG_ENABLE_CLANG_TIDY) + set_property(TARGET mod_datadog PROPERTY CXX_CLANG_TIDY + "${HTTPD_DATADOG_CLANG_TIDY};--quiet;--use-color") +endif() if (HTTPD_DATADOG_ENABLE_RUM) target_compile_definitions( diff --git a/scripts/clang-tidy.sh b/scripts/clang-tidy.sh new file mode 100755 index 0000000..8f10da8 --- /dev/null +++ b/scripts/clang-tidy.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash +# Run the pinned clang-tidy through CMake in the httpd-datadog devcontainer. +# +# clang-tidy must use the same compiler, flags, and sysroot as the real +# build (ci-dev). CMake's CXX_CLANG_TIDY on mod_datadog does that: it +# invokes tidy with the exact compile line after `--`. Do not run tidy +# against a host compilation database. +# +# Usage: +# make lint-tidy +# ./scripts/clang-tidy.sh + +set -euo pipefail + +SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd) +REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) +cd "$REPO_ROOT" + +# Must match LLVM_VERSION in .devcontainer/Dockerfile (major). +CLANG_TIDY_VERSION=17 + +in_container() { + [[ -f /.dockerenv ]] || [[ -n "${KUBERNETES_SERVICE_HOST:-}" ]] || \ + [[ "${HTTPD_DATADOG_TIDY_IN_CONTAINER:-}" == "1" ]] +} + +if ! in_container; then + if ! command -v docker >/dev/null 2>&1; then + >&2 echo "docker is required to run clang-tidy-${CLANG_TIDY_VERSION}." + exit 1 + fi + exec make -C "$REPO_ROOT" lint-tidy +fi + +build_dir=${BUILD_DIR:-.clang-tidy-build} +tidy="clang-tidy-${CLANG_TIDY_VERSION}" + +if ! command -v "$tidy" >/dev/null 2>&1 && command -v clang-tidy >/dev/null 2>&1; then + found_major=$(clang-tidy --version | sed -n 's/.*version \([0-9][0-9]*\).*/\1/p' | head -n 1) + if [[ "$found_major" == "$CLANG_TIDY_VERSION" ]]; then + tidy=clang-tidy + fi +fi + +if ! command -v "$tidy" >/dev/null 2>&1; then + apk add --no-cache clang-extra-tools + if command -v clang-tidy >/dev/null 2>&1; then + found_major=$(clang-tidy --version | sed -n 's/.*version \([0-9][0-9]*\).*/\1/p' | head -n 1) + if [[ "$found_major" != "$CLANG_TIDY_VERSION" ]]; then + >&2 echo "clang-tidy ${found_major} is installed, but ${CLANG_TIDY_VERSION} is pinned." + exit 1 + fi + tidy=clang-tidy + fi +fi + +if ! command -v "$tidy" >/dev/null 2>&1; then + >&2 echo "clang-tidy-${CLANG_TIDY_VERSION} is required (pinned)." + exit 1 +fi + +compiler=${CXX:-clang++} +if ! command -v "$compiler" >/dev/null 2>&1; then + >&2 echo "$compiler is required (same toolchain as CMake)." + exit 1 +fi + +compiler_major=$("$compiler" -dumpversion | cut -d. -f1) +tidy_major=$("$tidy" --version | sed -n 's/.*version \([0-9][0-9]*\).*/\1/p' | head -n 1) +if [[ "$compiler_major" != "$CLANG_TIDY_VERSION" || "$tidy_major" != "$CLANG_TIDY_VERSION" ]]; then + >&2 echo "clang-tidy and the CMake compiler must both be LLVM ${CLANG_TIDY_VERSION}." + >&2 echo " ${compiler}: ${compiler_major}" + >&2 echo " ${tidy}: ${tidy_major}" + exit 1 +fi + +git config --global --add safe.directory "$REPO_ROOT" >/dev/null 2>&1 || true +if [[ ! -f deps/dd-trace-cpp/CMakeLists.txt ]] || [[ ! -f deps/nginx-datadog/CMakeLists.txt ]]; then + git submodule update --init --depth=1 deps/dd-trace-cpp deps/nginx-datadog +fi + +# Reconfigure so CXX_CLANG_TIDY is attached to mod_datadog, then compile +# that target. CMake passes the exact ci-dev compile line to tidy. +# RUM is off in ci-dev, so rum/ is not a source of mod_datadog. +cmake --preset=ci-dev -B "$build_dir" . \ + -DHTTPD_DATADOG_ENABLE_CLANG_TIDY=ON \ + -DHTTPD_DATADOG_CLANG_TIDY="$tidy" + +cmake --build "$build_dir" --target mod_datadog