Skip to content
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
10 changes: 10 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ common:clang --define is_clang=true
# https://github.com/llvm/llvm-project/issues/100909
common:clang --cxxopt=-Wno-unused-command-line-argument

# The configuration the code is INDEXED in: `compile_commands.json` (see
# `compile_commands-update.sh`) and therefore clangd and clang-tidy. It is
# `--config=clang` plus an explicit libc++, because that is what development
# actually targets and the compile DB must say so rather than inherit whatever
# the driver defaults to - libc++ on macOS but libstdc++ on Linux, which made
# the same clang-tidy check report differently per platform.
common:clang-tidy --config=clang
common:clang-tidy --cxxopt=-stdlib=libc++
common:clang-tidy --linkopt=-stdlib=libc++

common:symbolizer --strip=never
common:symbolizer --run_under=//tools:run_under_symbolizer

Expand Down
12 changes: 12 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,18 @@ repos:
entry: .pre-commit/check_version.sh
pass_filenames: False
types: [text]
- id: clang-tidy-targets-match-tag
name: clang-tidy targets match the tag
description: |
`CLANG_TIDY_MANUAL_TARGETS` in bazelmod/clang_tidy_targets.bzl must equal the set of
targets tagged `clang-tidy`. The list exists because aquery expands `//...` with
`manual` targets already removed, so //bazelmod:refresh_compile_commands has to name
them explicitly or they never reach compile_commands.json. The tag states the intent;
this stops the two drifting apart. Skips when bazel is unavailable.
language: system
entry: .pre-commit/check_clang_tidy_targets.sh
pass_filenames: false
files: ^(.*/BUILD\.bazel|bazelmod/clang_tidy_targets\.bzl)$
- id: no-do-not-merge
name: No 'DO NOT MERGE'
description: |
Expand Down
57 changes: 57 additions & 0 deletions .pre-commit/check_clang_tidy_targets.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#!/usr/bin/env bash

# SPDX-FileCopyrightText: Copyright (c) The helly25 authors (helly25.com)
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

# Verifies that `CLANG_TIDY_MANUAL_TARGETS` in bazelmod/clang_tidy_targets.bzl is
# exactly the set of targets tagged `clang-tidy`.
#
# The list has to exist because aquery expands `//...` with `manual` targets
# already removed, so `//bazelmod:refresh_compile_commands` must name them
# explicitly to get them into `compile_commands.json`. The tag stays the
# statement of intent; this check stops the two from drifting apart when a target
# is added, removed or retagged.
#
# Skips when bazel is unavailable rather than failing, so a checkout without a
# toolchain can still commit.

set -euo pipefail

if ! command -v bazel >/dev/null 2>&1; then
echo "clang-tidy-targets: skipped (no bazel on PATH)." 1>&2
exit 0
fi

readonly TARGETS_BZL="bazelmod/clang_tidy_targets.bzl"

# Tagged targets, as bazel sees them. `bazel query` (unlike aquery) does report
# `manual` targets, which is what makes this check possible at all. Strip the
# `@@//` canonical-repo prefix so both sides compare as `//pkg:target`.
TAGGED="$(bazel query 'attr(tags, "clang-tidy", //...)' 2>/dev/null | sed 's|^@@||' | sort)"

# The list the build actually uses.
LISTED="$(sed -n 's|^ *"\(//[^"]*\)",$|\1|p' "${TARGETS_BZL}" | sort)"

if [ "${TAGGED}" = "${LISTED}" ]; then
exit 0
fi

echo "ERROR: ${TARGETS_BZL} and the 'clang-tidy' tag disagree." 1>&2
echo " Tagged but not listed (these would be MISSING from compile_commands.json):" 1>&2
comm -23 <(printf '%s\n' "${TAGGED}") <(printf '%s\n' "${LISTED}") | sed 's/^/ /' 1>&2
echo " Listed but not tagged (stale entries):" 1>&2
comm -13 <(printf '%s\n' "${TAGGED}") <(printf '%s\n' "${LISTED}") | sed 's/^/ /' 1>&2
echo " Fix by editing ${TARGETS_BZL}, or by adding/removing the 'clang-tidy' tag." 1>&2
exit 1
27 changes: 27 additions & 0 deletions bazelmod/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,36 @@
# See the License for the specific language governing permissions and
# limitations under the License.

load("@hedron_compile_commands//:refresh_compile_commands.bzl", "refresh_compile_commands")
load(":clang_tidy_targets.bzl", "CLANG_TIDY_MANUAL_TARGETS")

package(default_visibility = [])

exports_files([
"dev.MODULE.bazel",
"llvm.MODULE.bazel",
])

# Generates `compile_commands.json`; run it through `//compile_commands-update.sh`.
#
# Lives here rather than in the root package because the extractor is a
# `dev_dependency`: the root package defines `//:clang-format` and `//:is_clang`,
# which library targets reference, so a module consumer would load it and fail on
# the missing dev dep. Nothing outside MODULE.bazel references `//bazelmod`.
#
# One entry, because the key is interpolated into `deps(...)` and so may be a
# query EXPRESSION rather than a single pattern. The union is required: aquery
# expands `//...` with `manual` targets already excluded, so the benchmarks and
# other manual targets we still want indexed have to be named explicitly (a
# `attr(tags, "clang-tidy", //...)` filter returns nothing extra, because the
# exclusion happens before the filter). `//bazelmod:clang_tidy_manual_targets`
# below keeps that list honest against the tag.
refresh_compile_commands(
name = "refresh_compile_commands",
tags = ["manual"],
targets = {
" + ".join([
"@//...",
] + CLANG_TIDY_MANUAL_TARGETS): "--config=clang-tidy",
},
)
44 changes: 44 additions & 0 deletions bazelmod/clang_tidy_targets.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# SPDX-FileCopyrightText: Copyright (c) The helly25 authors (helly25.com)
# SPDX-License-Identifier: Apache-2.0
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Manual targets that must still be indexed for clang-tidy.

A `manual` target is excluded from wildcard patterns, and aquery applies that
exclusion while expanding `//...` - before any `attr(tags, ...)` filter can see
it. So a tag alone cannot pull these into `compile_commands.json`; the labels
have to be named explicitly in `//bazelmod:refresh_compile_commands`.

The `clang-tidy` tag on each of these targets remains the source of truth for
INTENT ("this manual target is meant to be linted"), and the
`clang-tidy-targets-match-tag` pre-commit hook fails if this list and the tag
query disagree - so the list cannot silently drift as targets are added,
removed or retagged.

Note this covers everything bazel builds. The one repository source that is not
in `compile_commands.json`, `mbo/hash/measurements/smhasher3/mbohash.cpp`, cannot
be: it is a plugin for SMHasher3, copied into that project by
`mbo/hash/measurements/build_smhasher3.sh` and compiled by ITS cmake. There is no
BUILD file for it here and thus nothing to tag, and linting it against this
repository's flags would only produce bogus errors for the SMHasher3 headers
(`Platform.h`, `Hashlib.h`) it includes.
"""

CLANG_TIDY_MANUAL_TARGETS = [
"//mbo/container:limited_set_benchmark",
"//mbo/diff:diff_benchmark",
"//mbo/hash:hash_benchmark",
"//mbo/hash:hash_differential_test",
"//tools:show_compiler",
]
2 changes: 1 addition & 1 deletion bazelmod/dev.MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

"""Extra development modules."""

bazel_dep(name = "hedron_compile_commands", dev_dependency = True, repo_name = "bazel_compile_commands_extractor")
bazel_dep(name = "hedron_compile_commands", dev_dependency = True)
git_override(
module_name = "hedron_compile_commands",
commit = "6eb3ff1de6445355552ef4230be0caf3e1dd27e5",
Expand Down
75 changes: 17 additions & 58 deletions compile_commands-update.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,18 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# Generate `compile_commands.json` describing the HERMETIC clang, so that clangd
# and `tools/clang_tidy.sh` parse with the same compiler the `--config=clang`
# builds use. Note that `--config=clang` cannot do this: it only configures the
# build of the extractor tool itself, and never reaches the `aquery` the tool
# runs internally, so the recorded commands still named the autodetected local
# (Apple) clang. The fork's runtime flags below are the supported override; they
# must follow `--`, or `bazel run` hands them to bazel rather than to the tool.
# Generate `compile_commands.json` so that clangd and `tools/clang_tidy.sh` parse
# in the configuration development actually targets.
#
# The extraction runs through `//bazelmod:refresh_compile_commands`, whose target
# list carries `--config=clang-tidy` (`.bazelrc`: `--config=clang` plus an
# explicit libc++). That is what puts the flags right at the source, so no
# compiler override is needed: passing `--config=clang` on THIS command line
# would not work, as it only configures the build of the extractor tool itself
# and never reaches the `aquery` the tool runs internally.
#
# The runtime `--bcce-*` flags below must follow `--`, or `bazel run` hands them
# to bazel rather than to the tool.

set -euo pipefail

Expand All @@ -30,67 +35,21 @@ function die() {
exit 1
}

OUTPUT_BASE="$(bazel info output_base 2>/dev/null || true)"
[ -n "${OUTPUT_BASE}" ] || die "'bazel info output_base' failed; is bazel on PATH?"

# The hermetic LLVM install (headers + libs) lives in the `_llvm_llvm` repo; the
# plain `_llvm` repo only re-exports clang-format/clang-tidy/clangd.
declare -a CLANG_LOCS=(
"${OUTPUT_BASE}/external/toolchains_llvm++llvm+llvm_toolchain_llvm_llvm/bin/clang++"
"${OUTPUT_BASE}/external/toolchains_llvm~~llvm~llvm_toolchain_llvm_llvm/bin/clang++"
"${OUTPUT_BASE}/external/llvm_toolchain_llvm/bin/clang++"
)

CLANG=""
function resolve_clang() {
for LOC in "${CLANG_LOCS[@]}"; do
if [ -x "${LOC}" ]; then
CLANG="${LOC}"
return
fi
done
}

resolve_clang
if [ -z "${CLANG}" ]; then
# A fresh checkout (or CI runner) has not materialized the toolchain yet: it is
# only fetched once something is actually built with `--config=clang`. Build the
# smallest cc target there is to trigger that, then look again.
echo "Hermetic clang++ not present; fetching the toolchain via a probe build ..." 1>&2
bazel build --config=clang //tools:show_compiler >/dev/null \
|| die "probe build '//tools:show_compiler --config=clang' failed; cannot fetch the LLVM toolchain"
resolve_clang
fi

[ -n "${CLANG}" ] || die "Cannot find the hermetic clang++ even after a '--config=clang' build"

# Sources reachable both normally and through a build-machine tool are compiled
# twice (target + exec configuration), and both commands would be emitted. Keep
# only the target-configuration one: clang-tidy works per entry, so the exec copy
# is duplicate linting for a near-identical result. Files compiled ONLY in the
# exec configuration keep their command, so nothing leaves the compile DB.
declare -a BCCE_ARGS=("--bcce-compiler=${CLANG}" "--bcce-prefer-target-config")

# Parse against libc++ on EVERY platform, so clang-tidy sees the same standard
# library everywhere. The extracted commands come from the default build
# configuration, not `--config=clang`, so they carry no `-stdlib`; the hermetic
# clang then falls back to each platform's default - libc++ on macOS, libstdc++
# on Linux. That made the same check report differently per platform: a finding
# fixed on a Mac could be absent on the Linux CI runner and the reverse. The
# hermetic toolchain ships its own libc++ on both, so asking for it is enough
# (a no-op on macOS, the actual switch on Linux).
BCCE_ARGS+=("--bcce-copt=-stdlib=libc++")
declare -a BCCE_ARGS=("--bcce-prefer-target-config")

# The hermetic clang carries its own libc++ but no system C headers: without the
# SDK sysroot its <locale> support headers fail on `'time.h' file not found`.
# The bazel `--config=clang` toolchain supplies this itself; the extracted
# commands come from the autodetected toolchain, which relies on Apple clang's
# built-in default, so it has to be made explicit here. Linux needs no such flag.
# macOS only: the hermetic clang carries its own libc++ but no system C headers,
# so without the SDK sysroot its <locale> support headers fail on
# `'time.h' file not found`. Linux needs no such flag.
if [ "$(uname -s)" = "Darwin" ]; then
SDKROOT_PATH="$(xcrun --show-sdk-path 2>/dev/null || true)"
[ -n "${SDKROOT_PATH}" ] || die "'xcrun --show-sdk-path' failed; install the Xcode command line tools"
BCCE_ARGS+=("--bcce-copt=-isysroot${SDKROOT_PATH}")
fi

bazel run @bazel_compile_commands_extractor//:refresh_all -- "${BCCE_ARGS[@]}"
bazel run //bazelmod:refresh_compile_commands -- "${BCCE_ARGS[@]}"
echo "OK"
5 changes: 4 additions & 1 deletion mbo/container/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ cc_binary(
name = "limited_set_benchmark",
testonly = 1,
srcs = ["limited_set_benchmark_main.cc"],
tags = ["manual"],
tags = [
"clang-tidy",
"manual",
],
visibility = ["//visibility:private"],
deps = [
":limited_options_cc",
Expand Down
5 changes: 4 additions & 1 deletion mbo/diff/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ cc_binary(
name = "diff_benchmark",
testonly = True,
srcs = ["diff_benchmark.cc"],
tags = ["manual"],
tags = [
"clang-tidy",
"manual",
],
deps = [
":diff_cc",
"//mbo/file:artefact_cc",
Expand Down
10 changes: 8 additions & 2 deletions mbo/hash/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,10 @@ cc_test(
# module is loaded (bazelmod/dev.MODULE.bazel) - i.e. when helly25_mbo is
# the root module. Wildcards used by consumers must not analyze it; CI runs
# it explicitly (see .github/workflows/test.yml).
tags = ["manual"],
tags = [
"clang-tidy",
"manual",
],
deps = [
":hash_cc",
":hash_test_util",
Expand All @@ -204,7 +207,10 @@ cc_binary(
name = "hash_benchmark",
testonly = True,
srcs = ["hash_benchmark.cc"],
tags = ["manual"],
tags = [
"clang-tidy",
"manual",
],
deps = [
":hash_test_util",
"@abseil-cpp//absl/strings",
Expand Down
5 changes: 4 additions & 1 deletion tools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,8 @@ sh_binary(
cc_binary(
name = "show_compiler",
srcs = ["show_compiler.cc"],
tags = ["manual"],
tags = [
"clang-tidy",
"manual",
],
)
Loading