clang/tools: Use python distributed versions (envoyproxy#29455)

Signed-off-by: Ryan Northey <[email protected]>

tools/base/requirements.in

@@ -2,6 +2,8 @@ abstracts>=0.0.12
 aio.api.bazel
 aiohttp>=3.8.1
 cffi>=1.15.0
+clang-format==14.0.6
+clang-tidy==14.0.6
 colorama
 coloredlogs
 cryptography>=41.0.1

tools/base/requirements.txt

@@ -368,6 +368,27 @@ charset-normalizer==3.2.0 \
     # via
     #   aiohttp
     #   requests
+clang-format==14.0.6 \
+    --hash=sha256:13f2d6d4a2af004a783c65f0921afa8f0384bffcdaf500b6c2cb542edeb0b4a5 \
+    --hash=sha256:810c649ab97d208cd418c897d50ab6e958eb8d96854527edd80d0dd21a75e914 \
+    --hash=sha256:aaf4edecc46a24f0b572b82cf5827e292ad1c137903427627c4d5f671668cc2b \
+    --hash=sha256:bd400c47665dd19afc03f98e747f78ed828abab99c6a1b07e137b35c1cd3cc26 \
+    --hash=sha256:c93580945f75de7e01996f1fb3cf67e4dc424f1c864e237c85614fb99a48c7a4 \
+    --hash=sha256:d5c96b500d7f8b5d2db5b75ac035be387512850ad589cdc3019666b861382136 \
+    --hash=sha256:d780c04334bca80f2b60d25bf53c37bd0618520ee295a7888a11f25bde114ac4 \
+    --hash=sha256:d7c1c5e404c58e55f0170f01b3c5611dce6c119e62b5d1020347e0ad97d5a047 \
+    --hash=sha256:dbfd60528eb3bb7d7cfe8576faa70845fbf93601f815ef75163d36606e87f388
+    # via -r requirements.in
+clang-tidy==14.0.6 \
+    --hash=sha256:02bce40a56cc344e20d2f63bef6b85acf9837954559e0091804d6e748dfc0359 \
+    --hash=sha256:173a757415108095b541eb9a2d0c222d41f5624e7bb5b98772476957228ce2c7 \
+    --hash=sha256:4635f6553f9e3eb7a81fec29d15e4e70b49c1780f31a17550c11007fc9bba4b3 \
+    --hash=sha256:5b56edb6b7215eb79fede7ab8a4f9b94454bdfe1091d026acc1afdc7696abb68 \
+    --hash=sha256:7f75eb4839dc996dea494a07814b3a70200be75bc7d9acb54d3d5916f24bcd8d \
+    --hash=sha256:c9ffcb91f17ee920fdd7a83f30484f3cb4c183f7b490d092373e4a6f2c82729d \
+    --hash=sha256:d595b8e9a155d63b6b9dec0afa62725590626c9f0e945c3d9e448a28e0082b39 \
+    --hash=sha256:fef62fb706adccef94128761ca0796973a196e2d60fb938a312cfa2bc59730bd
+    # via -r requirements.in
 colorama==0.4.6 \
     --hash=sha256:08695f5cb7ed6e0531a20572697297273c47b8cae5a63ffc6d6ed5c201be6e44 \
     --hash=sha256:4f1d9991f5acc0ca119f9d443620b77f9d6b33703e51011c16baf57afb285fc6
@@ -1235,7 +1256,7 @@ sphinxcontrib-httpdomain==1.8.1 \
     --hash=sha256:21eefe1270e4d9de8d717cc89ee92cc4871b8736774393bafc5e38a6bb77b1d5 \
     --hash=sha256:6c2dfe6ca282d75f66df333869bb0ce7331c01b475db6809ff9d107b7cdfe04b
     # via envoy-docs-sphinx-runner
-sphinxcontrib-jquery @ https://github.com/sphinx-contrib/jquery/archive/refs/tags/v3.0.0.zip \
+sphinxcontrib.jquery @ https://github.com/sphinx-contrib/jquery/archive/refs/tags/v3.0.0.zip \
     --hash=sha256:562ad9ac0ac3d8f04a363eb3507ae4b2b856aa04aabab6df7543530fafb849ca
     # via
     #   -r requirements.in

tools/clang-format/BUILD

@@ -0,0 +1,12 @@
+load("@base_pip3//:requirements.bzl", "requirement")
+load("//bazel:envoy_build_system.bzl", "envoy_package")
+load(":clang_format.bzl", "clang_format")
+
+licenses(["notice"])  # Apache 2
+
+envoy_package()
+
+clang_format(
+    name = "clang-format",
+    target = requirement("clang-format"),
+)

tools/clang-format/clang_format.bzl

@@ -0,0 +1,60 @@
+#
+# This fishes the clang-format binary out of the related python package.
+#
+# This is useful as using the binary through the python entry_point adds a lot of overhead.
+#
+# ```starlark
+#
+# load("@base_pip3//:requirements.bzl", "requirement")
+#
+# clang_format(
+#    name = "clang-format",
+#    target = requirement("clang-format"),
+# )
+#
+# ```
+#
+# The exposed binary can also be run directly:
+#
+# ```console
+#
+# $ bazel run //tools/clang-format -- --version
+#
+# ```
+#
+
+def _clang_format_impl(ctx):
+    clang_bin = None
+    for file in ctx.attr.target[DefaultInfo].data_runfiles.files.to_list():
+        if file.basename == "clang-format" and file.dirname.split("/").pop() == "bin":
+            clang_bin = file
+            break
+
+    if not clang_bin:
+        fail("Unable to find clang-format file in package")
+
+    output_file = ctx.actions.declare_file("clang-format")
+    args = ctx.actions.args()
+    args.add(clang_bin.path)
+    args.add(output_file.path)
+    ctx.actions.run(
+        outputs = [output_file],
+        inputs = [clang_bin],
+        arguments = [args],
+        executable = "cp",
+        mnemonic = "ClangFormatGetter",
+    )
+    return [DefaultInfo(
+        executable = output_file,
+        files = depset([output_file]),
+    )]
+
+clang_format = rule(
+    implementation = _clang_format_impl,
+    attrs = {
+        "target": attr.label(
+            allow_files = True,
+        ),
+    },
+    executable = True,
+)

tools/code_format/BUILD

@@ -22,11 +22,13 @@ py_binary(
     srcs = ["check_format.py"],
     args = [
         "--path=%s" % PATH,
+        "--clang_format_path=$(location //tools/clang-format)",
         "--buildifier_path=$(location @com_github_bazelbuild_buildtools//buildifier)",
         "--buildozer_path=$(location @com_github_bazelbuild_buildtools//buildozer)",
     ],
     data = [
         ":config.yaml",
+        "//tools/clang-format",
         "@com_github_bazelbuild_buildtools//buildifier",
         "@com_github_bazelbuild_buildtools//buildozer",
     ],

tools/code_format/check_format.py

@@ -55,7 +55,10 @@ def buildozer_path(self) -> str:
     @cached_property
     def clang_format_path(self) -> str:
         """Path to the clang-format binary."""
-        return os.getenv("CLANG_FORMAT", "clang-format-14")
+        path = (
+            os.path.join(self.source_path, self.args.clang_format_path)
+            if self.source_path else self.args.clang_format_path)
+        return path
 
     @cached_property
     def config(self) -> Dict:
@@ -128,7 +131,6 @@ def __init__(self, args):
         self.source_path = os.getcwd()
         if self.args.path:
             os.chdir(self.args.path)
-        os.environ["BAZEL_EXECROOT"] = self.source_path
         self._include_dir_order = self.args.include_dir_order
 
     @property
@@ -217,6 +219,8 @@ def args(self):
             nargs="+",
             default=[],
             help="exclude paths from bazel_tools check.")
+        parser.add_argument(
+            "--clang_format_path", type=str, help="Path to clang-format executable.")
         parser.add_argument("--buildifier_path", type=str, help="Path to buildifier executable.")
         parser.add_argument("--buildozer_path", type=str, help="Path to buildozer executable.")
         parser.add_argument(
@@ -322,55 +326,6 @@ def executable_by_others(self, executable):
         st = os.stat(os.path.expandvars(executable))
         return bool(st.st_mode & stat.S_IXOTH)
 
-    # Check whether all needed external tools (clang-format, buildifier, buildozer) are
-    # available.
-    def check_tools(self):
-        error_messages = []
-        clang_format_abs_path = self.look_path(self.config.clang_format_path)
-
-        if clang_format_abs_path:
-            if not self.executable_by_others(clang_format_abs_path):
-                error_messages.append(
-                    "command {} exists, but cannot be executed by other "
-                    "users".format(self.config.clang_format_path))
-        else:
-            error_messages.append(
-                "Command {} not found. If you have clang-format in version 12.x.x "
-                "installed, but the binary name is different or it's not available in "
-                "PATH, please use CLANG_FORMAT environment variable to specify the path. "
-                "Examples:\n"
-                "    export CLANG_FORMAT=clang-format-14.0.0\n"
-                "    export CLANG_FORMAT=/opt/bin/clang-format-14\n"
-                "    export CLANG_FORMAT=/usr/local/opt/llvm@14/bin/clang-format".format(
-                    self.config.clang_format_path))
-
-        def check_bazel_tool(name, path, var):
-            bazel_tool_abs_path = self.look_path(path)
-            if bazel_tool_abs_path:
-                if not self.executable_by_others(bazel_tool_abs_path):
-                    error_messages.append(
-                        "command {} exists, but cannot be executed by other "
-                        "users".format(path))
-            elif self.path_exists(path):
-                if not self.executable_by_others(path):
-                    error_messages.append(
-                        "command {} exists, but cannot be executed by other "
-                        "users".format(path))
-            else:
-                error_messages.append(
-                    "Command {} not found. If you have {} installed, but the binary "
-                    "name is different or it's not available in $GOPATH/bin, please use "
-                    "{} environment variable to specify the path. Example:\n"
-                    "    export {}=`which {}`\n"
-                    "If you don't have {} installed, you can install it by:\n"
-                    "    go install github.com/bazelbuild/buildtools/{}@latest".format(
-                        path, name, var, var, name, name, name))
-
-        check_bazel_tool('buildifier', self.config.buildifier_path, 'BUILDIFIER_BIN')
-        check_bazel_tool('buildozer', self.config.buildozer_path, 'BUILDOZER_BIN')
-
-        return error_messages
-
     def check_namespace(self, file_path):
         for excluded_path in self.namespace_check_excluded_paths:
             if file_path.startswith(excluded_path):
@@ -905,7 +860,6 @@ def fix_source_path(self, file_path):
 
     def check_source_path(self, file_path):
         error_messages = self.check_file_contents(file_path, self.check_source_line)
-
         if not file_path.endswith(self.config.suffixes["proto"]):
             error_messages += self.check_namespace(file_path)
             command = (
@@ -914,8 +868,7 @@ def check_source_path(self, file_path):
                     file_path))
             error_messages += self.execute_command(
                 command, "header_order.py check failed", file_path)
-        command = ("%s %s | diff %s -" % (self.config.clang_format_path, file_path, file_path))
-        error_messages += self.execute_command(command, "clang-format check failed", file_path)
+        error_messages.extend(self.clang_format(file_path, check=True))
         return error_messages
 
     # Example target outputs are:
@@ -946,11 +899,19 @@ def fix_header_order(self, file_path):
             return ["header_order.py rewrite error: %s" % (file_path)]
         return []
 
-    def clang_format(self, file_path):
-        command = "%s -i %s" % (self.config.clang_format_path, file_path)
-        if os.system(command) != 0:
-            return ["clang-format rewrite error: %s" % (file_path)]
-        return []
+    def clang_format(self, file_path, check=False):
+        result = []
+        command = (
+            f"{self.config.clang_format_path} {file_path} | diff {file_path} -"
+            if check else f"{self.config.clang_format_path} -i {file_path}")
+
+        if check:
+            result = self.execute_command(command, "clang-format check failed", file_path)
+        else:
+            if os.system(command) != 0:
+                result = [f"clang-format rewrite error: {file_path}"]
+
+        return result
 
     def check_format(self, file_path, fail_on_diff=False):
         error_messages = []

tools/local_fix_format.sh

@@ -43,6 +43,10 @@ if [[ $# -gt 0 && "$1" == "-skip-bazel" ]]; then
     shift
     use_bazel=0
 
+    CLANG_FORMAT_BIN="$(command -v clang-format)" || {
+        echo "Local clang-format not found, exiting" >&2
+        exit 1
+    }
     BUILDIFIER_BIN="$(command -v buildifier)" || {
         echo "Local buildifier not found, exiting" >&2
         exit 1
@@ -72,7 +76,10 @@ format_some () {
         ./tools/spelling/check_spelling_pedantic.py fix "$@"
     else
       for arg in "$@"; do
-          ./tools/code_format/check_format.py --buildozer_path "$BUILDOZER_BIN" --buildifier_path "$BUILDIFIER_BIN" fix "$arg"
+          ./tools/code_format/check_format.py \
+              --clang_format_path "$CLANG_FORMAT_BIN" \
+              --buildozer_path "$BUILDOZER_BIN" \
+              --buildifier_path "$BUILDIFIER_BIN" fix "$arg"
           ./tools/spelling/check_spelling_pedantic.py fix "$arg"
       done
     fi