Skip to content

Commit f8ed573

Browse files
committed
build: Fix format targets for envoy 1.37 LLVM toolchain change
Envoy 1.37 switched clang-format from a pip package to @llvm_toolchain_llvm, which is not created when BAZEL_LLVM_PATH is set. Use hermetic LLVM for format targets and add a fallback repo rule for CI builds. Signed-off-by: Tam Mach <tam.mach@cilium.io>
1 parent 93b6a7e commit f8ed573

File tree

3 files changed

+48
-3
lines changed

3 files changed

+48
-3
lines changed

Makefile.dev

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ veryclean: force-non-root clean
4242
precheck: force-non-root
4343
tools/check_repositories.sh
4444

45-
FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/" "./vendor/" "./go/" "./envoy_build_config/"
45+
FORMAT_EXCLUDED_PREFIXES = "./linux/" "./proxylib/" "./starter/" "./vendor/" "./go/" "./envoy_build_config/" "./work/" "./bin/"
4646

4747
# The default set of sources assumes all relevant sources are dependecies of some tests!
4848
TIDY_SOURCES ?= $(shell bazel query 'kind("source file", deps(//tests/...))' 2>/dev/null | sed -n "s/\/\/cilium:/cilium\//p; s/\/\/tests:/tests\//p")
@@ -71,10 +71,10 @@ tidy-fix-head: compile_commands.json force-non-root
7171
run-clang-tidy-18 -fix -format -style=file -quiet -extra-arg="-Wno-unknown-pragmas" -checks=misc-include-cleaner -j $(TIDY_JOBS) $(shell git diff-tree --no-commit-id --name-only -r --diff-filter=d HEAD) || echo "clang-tidy fix produced changes, please commit them."
7272

7373
format: force-non-root
74-
CARGO_BAZEL_REPIN=true $(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check || echo "Format check failed, run 'make format-fix' locally to fix formatting errors."
74+
CARGO_BAZEL_REPIN=true $(BAZEL) $(BAZEL_OPTS) run --repo_env=BAZEL_LLVM_PATH= @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="./" --build_fixer_check_excluded_paths="./" check || echo "Format check failed, run 'make format-fix' locally to fix formatting errors."
7575

7676
format-fix: force-non-root
77-
CARGO_BAZEL_REPIN=true $(BAZEL) $(BAZEL_OPTS) run $(BAZEL_BUILD_OPTS) @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
77+
CARGO_BAZEL_REPIN=true $(BAZEL) $(BAZEL_OPTS) run --repo_env=BAZEL_LLVM_PATH= @envoy//tools/code_format:check_format -- --path "$(PWD)" --skip_envoy_build_rule_check --add-excluded-prefixes $(FORMAT_EXCLUDED_PREFIXES) --bazel_tools_check_excluded_paths="." --build_fixer_check_excluded_paths="./" fix
7878

7979
# Run tests without debug by default.
8080
tests: $(COMPILER_DEP) force-non-root SOURCE_VERSION proxylib/libcilium.so install-bazelisk

WORKSPACE

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ load("@envoy//bazel:toolchains.bzl", "envoy_toolchains")
135135

136136
envoy_toolchains()
137137

138+
# When BAZEL_LLVM_PATH is set, envoy_toolchains() skips creating the
139+
# llvm_toolchain_llvm repo, but envoy's clang-format target still depends on it.
140+
# Provide it only if it wasn't already created.
141+
load("//bazel:local_llvm.bzl", "local_llvm_repo")
142+
143+
local_llvm_repo(name = "llvm_toolchain_llvm")
144+
138145
load("@envoy//bazel:dependency_imports_extra.bzl", "envoy_dependency_imports_extra")
139146

140147
envoy_dependency_imports_extra()

bazel/local_llvm.bzl

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
"""Repository rule to provide llvm_toolchain_llvm when using a local LLVM toolchain.
2+
3+
When BAZEL_LLVM_PATH is set (local toolchain mode), the toolchains_llvm
4+
llvm_toolchain() macro skips creating the llvm_toolchain_llvm repository.
5+
Envoy's tools/clang-format target still depends on @llvm_toolchain_llvm//:clang-format,
6+
so we need to provide it.
7+
"""
8+
9+
def _local_llvm_repo_impl(repository_ctx):
10+
llvm_path = repository_ctx.os.environ.get("BAZEL_LLVM_PATH", "")
11+
clang_format = None
12+
13+
if llvm_path:
14+
candidate = repository_ctx.path(llvm_path + "/bin/clang-format")
15+
if candidate.exists:
16+
clang_format = candidate
17+
18+
if not clang_format:
19+
clang_format = repository_ctx.which("clang-format")
20+
21+
if not clang_format:
22+
fail("Could not find clang-format. Set BAZEL_LLVM_PATH or ensure clang-format is on PATH.")
23+
24+
repository_ctx.symlink(clang_format, "bin/clang-format")
25+
repository_ctx.file("BUILD.bazel", """\
26+
package(default_visibility = ["//visibility:public"])
27+
28+
filegroup(
29+
name = "clang-format",
30+
srcs = ["bin/clang-format"],
31+
)
32+
""")
33+
34+
local_llvm_repo = repository_rule(
35+
implementation = _local_llvm_repo_impl,
36+
environ = ["BAZEL_LLVM_PATH", "PATH"],
37+
local = True,
38+
)

0 commit comments

Comments
 (0)