From 0785b499f95ca3d47395ca0815edcff062231b0e Mon Sep 17 00:00:00 2001 From: Fabian Ruffy <5960321+fruffy@users.noreply.github.com> Date: Mon, 12 Feb 2024 22:37:46 +0100 Subject: [PATCH] Add buildifier to Bazel workspace and run it on the bazel build files. (#4413) * Install buildifier. * Run buildifier. * Add a check and a fix target. * Linter complaints * CI. --- .github/workflows/ci-bazel.yml | 6 ++++ BUILD.bazel | 52 +++++++++++++++++++++++----------- WORKSPACE.bazel | 29 ++++++++++++++++++- backends/p4tools/BUILD.bazel | 10 +++---- bazel/BUILD.inja.bazel | 2 +- bazel/BUILD.json.bazel | 2 +- bazel/example/BUILD.bazel | 26 ++++++++--------- bazel/example/WORKSPACE.bazel | 43 ++++++++++++++++++++++------ bazel/example/p4/BUILD.bazel | 52 +++++++++++++++++----------------- bazel/p4_library.bzl | 10 +++---- bazel/p4c_deps.bzl | 17 ++++++----- 11 files changed, 165 insertions(+), 84 deletions(-) diff --git a/.github/workflows/ci-bazel.yml b/.github/workflows/ci-bazel.yml index f479bb4fad8..51aa02039b3 100644 --- a/.github/workflows/ci-bazel.yml +++ b/.github/workflows/ci-bazel.yml @@ -38,6 +38,9 @@ jobs: chmod +x $BAZEL sudo mv $BAZEL /usr/local/bin/bazel + - name: First, pass the lint test + run: bazel run //:buildifier_check + - name: Install Flex and Bison run: sudo apt-get update && sudo apt-get install bison flex libfl-dev @@ -64,6 +67,9 @@ jobs: restore-keys: | ${{ runner.os }}-build-indirect- + - name: First, pass the lint test + run: bazel run //:buildifier_check + - name: Install bazelisk run: | curl -LO "https://github.com/bazelbuild/bazelisk/releases/download/v1.18.0/$BAZEL" diff --git a/BUILD.bazel b/BUILD.bazel index d0196f9665a..6eedb12425a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,6 +1,7 @@ load("@rules_license//rules:license.bzl", "license") load("//:bazel/flex.bzl", "genlex") load("//:bazel/bison.bzl", "genyacc") +load("@com_github_bazelbuild_buildtools//buildifier:def.bzl", "buildifier") package( default_applicable_licenses = ["//:license"], @@ -134,10 +135,10 @@ cc_library( ":ir_generator_lex", ":ir_generator_yacc", ], + visibility = ["//visibility:private"], deps = [ ":lib", ], - visibility = ["//visibility:private"], ) # The next rule builds the ir-generator tool binary. @@ -206,6 +207,13 @@ cc_library( "backends/dpdk/dbprint-dpdk.cpp", "backends/dpdk/printUtils.cpp", ], + copts = [ + # Where p4c should look for p4include at runtime. + ("-DCONFIG_PKGDATADIR=\\\"external/%s\\\"" % repository_name()), + # This will work only if the binary is executed by Bazel. For a general + # solution, we would need to make p4c aware of Bazel, specifically: + # https://github.com/bazelbuild/bazel/blob/master/tools/cpp/runfiles/runfiles_src.h + ], textual_hdrs = glob([ "ir/**/*.h", "frontends/**/*.h", @@ -231,13 +239,6 @@ cc_library( "@com_github_p4lang_p4runtime//:p4types_cc_proto", "@com_google_protobuf//:protobuf", ], - copts = [ - # Where p4c should look for p4include at runtime. - ("-DCONFIG_PKGDATADIR=\\\"external/%s\\\"" % repository_name()), - # This will work only if the binary is executed by Bazel. For a general - # solution, we would need to make p4c aware of Bazel, specifically: - # https://github.com/bazelbuild/bazel/blob/master/tools/cpp/runfiles/runfiles_src.h - ], ) cc_library( @@ -258,9 +259,9 @@ cc_library( ), hdrs = glob(["backends/bmv2/simple_switch/*.h"]), deps = [ - ":p4c_bmv2_common_lib", ":ir_frontend_midend_control_plane", ":lib", + ":p4c_bmv2_common_lib", ], ) @@ -278,13 +279,13 @@ cc_binary( "backends/bmv2/simple_switch/main.cpp", "backends/bmv2/simple_switch/version.h", ], + data = [":p4include"], deps = [ - ":p4c_bmv2_common_lib", - ":p4c_bmv2_simple_lib", ":ir_frontend_midend_control_plane", ":lib", + ":p4c_bmv2_common_lib", + ":p4c_bmv2_simple_lib", ], - data = [":p4include"], ) # dpdk backend @@ -349,15 +350,14 @@ cc_binary( "backends/dpdk/main.cpp", "backends/dpdk/version.h", ], + data = [":p4include"], deps = [ - ":p4c_dpdk_lib", ":ir_frontend_midend_control_plane", ":lib", + ":p4c_dpdk_lib", ], - data = [":p4include"], ) - # This builds the p4test backend. cc_binary( name = "p4c_backend_p4test", @@ -421,15 +421,15 @@ cc_library( name = "p4c_backend_graphs_lib", srcs = [ "backends/graphs/controls.cpp", - "backends/graphs/graphs.cpp", "backends/graphs/graph_visitor.cpp", + "backends/graphs/graphs.cpp", "backends/graphs/p4c-graphs.cpp", "backends/graphs/parsers.cpp", ], hdrs = [ "backends/graphs/controls.h", - "backends/graphs/graphs.h", "backends/graphs/graph_visitor.h", + "backends/graphs/graphs.h", "backends/graphs/parsers.h", "backends/graphs/version.h", ], @@ -445,3 +445,21 @@ cc_library( "@boost//:graph", ], ) + +buildifier( + name = "buildifier_check", + diff_command = "diff -u", + exclude_patterns = [ + "./build/*", + ], + lint_mode = "warn", + mode = "diff", +) + +buildifier( + name = "buildifier_fix", + exclude_patterns = [ + "./build/*", + ], + lint_mode = "fix", +) diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel index 1bbf148fefa..74482170ccc 100644 --- a/WORKSPACE.bazel +++ b/WORKSPACE.bazel @@ -1,5 +1,32 @@ workspace(name = "com_github_p4lang_p4c") +# -- Load Buildifier ----------------------------------------------------- +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +# buildifier is written in Go and hence needs rules_go to be built. +# See https://github.com/bazelbuild/rules_go for the up to date setup instructions. +http_archive( + name = "io_bazel_rules_go", + sha256 = "6dc2da7ab4cf5d7bfc7c949776b1b7c733f05e56edc4bcd9022bb249d2e2a996", + urls = [ + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.39.1/rules_go-v0.39.1.zip", + "https://github.com/bazelbuild/rules_go/releases/download/v0.39.1/rules_go-v0.39.1.zip", + ], +) + +load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies") + +go_rules_dependencies() + +http_archive( + name = "com_github_bazelbuild_buildtools", + sha256 = "ae34c344514e08c23e90da0e2d6cb700fcd28e80c02e23e4d5715dddcb42f7b3", + strip_prefix = "buildtools-4.2.2", + urls = [ + "https://github.com/bazelbuild/buildtools/archive/refs/tags/4.2.2.tar.gz", + ], +) + # -- Direct dependencies. ------------------------------------------------------ load("//:bazel/p4c_deps.bzl", "p4c_deps") @@ -26,8 +53,8 @@ load("@com_google_googleapis//:repository_rules.bzl", "switched_rules_by_languag switched_rules_by_language( name = "com_google_googleapis_imports", - grpc = True, cc = True, + grpc = True, ) load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps") diff --git a/backends/p4tools/BUILD.bazel b/backends/p4tools/BUILD.bazel index 494346e7f41..90090fcb80c 100644 --- a/backends/p4tools/BUILD.bazel +++ b/backends/p4tools/BUILD.bazel @@ -80,9 +80,9 @@ cc_library( ], features = ["-use_header_modules"], deps = [ - "@boost//:multiprecision", "//:ir_frontend_midend_control_plane", "//:lib", + "@boost//:multiprecision", "@com_github_z3prover_z3//:api", ], ) @@ -149,11 +149,11 @@ cc_library( deps = [ ":common", ":register_testgen_targets", + "//:ir_frontend_midend_control_plane", + "//:lib", "@boost//:multiprecision", "@com_github_pantor_inja//:inja", "@nlohmann_json//:json", - "//:ir_frontend_midend_control_plane", - "//:lib", ], ) @@ -170,9 +170,9 @@ cc_binary( visibility = ["//visibility:public"], deps = [ ":testgen_lib", - "@boost//:multiprecision", - "@boost//:filesystem", "//:lib", + "@boost//:filesystem", + "@boost//:multiprecision", ], ) diff --git a/bazel/BUILD.inja.bazel b/bazel/BUILD.inja.bazel index 6abb068e4fe..7594b92ae96 100644 --- a/bazel/BUILD.inja.bazel +++ b/bazel/BUILD.inja.bazel @@ -1,7 +1,7 @@ cc_library( name = "inja", hdrs = ["inja/inja.hpp"], + includes = ["."], visibility = ["//visibility:public"], deps = ["@nlohmann_json//:json"], - includes = ["."], ) diff --git a/bazel/BUILD.json.bazel b/bazel/BUILD.json.bazel index 22c07f308fa..39dd4d7d6b1 100644 --- a/bazel/BUILD.json.bazel +++ b/bazel/BUILD.json.bazel @@ -1,6 +1,6 @@ cc_library( name = "json", hdrs = ["nlohmann/json.hpp"], - visibility = ["//visibility:public"], includes = ["."], + visibility = ["//visibility:public"], ) diff --git a/bazel/example/BUILD.bazel b/bazel/example/BUILD.bazel index 7fbee2d8c4e..316c63dba37 100644 --- a/bazel/example/BUILD.bazel +++ b/bazel/example/BUILD.bazel @@ -1,18 +1,18 @@ filegroup( - name = "ir_extension", - srcs = glob(["*.def"]), - visibility = ["//visibility:public"], # So p4c can compile these. + name = "ir_extension", + srcs = glob(["*.def"]), + visibility = ["//visibility:public"], # So p4c can compile these. ) cc_binary( - name = "main", - srcs = ["main.cc"], - data = ["//p4:program.p4info.txt"], - deps = [ - "@com_github_p4lang_p4c//:ir_frontend_midend_control_plane", - "@com_github_p4lang_p4runtime//:p4info_cc_proto", - "@com_google_protobuf//:protobuf", - ], - linkopts = [ - ], + name = "main", + srcs = ["main.cc"], + data = ["//p4:program.p4info.txt"], + linkopts = [ + ], + deps = [ + "@com_github_p4lang_p4c//:ir_frontend_midend_control_plane", + "@com_github_p4lang_p4runtime//:p4info_cc_proto", + "@com_google_protobuf//:protobuf", + ], ) diff --git a/bazel/example/WORKSPACE.bazel b/bazel/example/WORKSPACE.bazel index 5faf6087ec0..4be9dfe7230 100644 --- a/bazel/example/WORKSPACE.bazel +++ b/bazel/example/WORKSPACE.bazel @@ -2,13 +2,40 @@ workspace(name = "example_p4_project") # May replace this with `git_repository` or `http_archive` in your own project. local_repository( - name = "com_github_p4lang_p4c", - path = "../..", - # This part is optional: only needed for custom backends with IR extensions. - repo_mapping = { - # Tells p4c where to look for `:ir_extension` target: in this project. - "@com_github_p4lang_p4c_extension" : "@example_p4_project", - }, + name = "com_github_p4lang_p4c", + path = "../..", + # This part is optional: only needed for custom backends with IR extensions. + repo_mapping = { + # Tells p4c where to look for `:ir_extension` target: in this project. + "@com_github_p4lang_p4c_extension": "@example_p4_project", + }, +) + +# -- Load Buildifier ----------------------------------------------------- +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +# buildifier is written in Go and hence needs rules_go to be built. +# See https://github.com/bazelbuild/rules_go for the up to date setup instructions. +http_archive( + name = "io_bazel_rules_go", + sha256 = "6dc2da7ab4cf5d7bfc7c949776b1b7c733f05e56edc4bcd9022bb249d2e2a996", + urls = [ + "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.39.1/rules_go-v0.39.1.zip", + "https://github.com/bazelbuild/rules_go/releases/download/v0.39.1/rules_go-v0.39.1.zip", + ], +) + +load("@io_bazel_rules_go//go:deps.bzl", "go_rules_dependencies") + +go_rules_dependencies() + +http_archive( + name = "com_github_bazelbuild_buildtools", + sha256 = "ae34c344514e08c23e90da0e2d6cb700fcd28e80c02e23e4d5715dddcb42f7b3", + strip_prefix = "buildtools-4.2.2", + urls = [ + "https://github.com/bazelbuild/buildtools/archive/refs/tags/4.2.2.tar.gz", + ], ) load("@com_github_p4lang_p4c//:bazel/p4c_deps.bzl", "p4c_deps") @@ -35,8 +62,8 @@ load("@com_google_googleapis//:repository_rules.bzl", "switched_rules_by_languag switched_rules_by_language( name = "com_google_googleapis_imports", - grpc = True, cc = True, + grpc = True, ) load("@com_github_grpc_grpc//bazel:grpc_deps.bzl", "grpc_deps") diff --git a/bazel/example/p4/BUILD.bazel b/bazel/example/p4/BUILD.bazel index 615e3f8273a..bb50d174328 100644 --- a/bazel/example/p4/BUILD.bazel +++ b/bazel/example/p4/BUILD.bazel @@ -1,38 +1,38 @@ -load("@com_github_p4lang_p4c//:bazel/p4_library.bzl", "p4_library", "p4_graphs") +load("@com_github_p4lang_p4c//:bazel/p4_library.bzl", "p4_graphs", "p4_library") p4_library( - name = "program", - src = "program.p4", - deps = ["empty.p4"], - p4info_out = "program.p4info.txt", - visibility = ["//:__subpackages__"], + name = "program", + src = "program.p4", + p4info_out = "program.p4info.txt", + visibility = ["//:__subpackages__"], + deps = ["empty.p4"], ) p4_library( - name = "program_with_more_options", - src = "program.p4", - deps = ["empty.p4"], # Optional: #include-ed dependencies. - p4info_out = "program.p4info.2.txt", # Optional. - target = "bmv2", # Optional (default: "bmv2"). - target_out = "program.bmv2.json", # Optional. - arch = "v1model", # Optional (default: "v1model"). - visibility = ["//:__subpackages__"], # Optional. + name = "program_with_more_options", + src = "program.p4", + arch = "v1model", # Optional (default: "v1model"). + p4info_out = "program.p4info.2.txt", # Optional. + target = "bmv2", # Optional (default: "bmv2"). + target_out = "program.bmv2.json", # Optional. + visibility = ["//:__subpackages__"], # Optional. + deps = ["empty.p4"], # Optional: #include-ed dependencies. ) p4_library( - name = "vxlan_dpdk_pna", - src = "vxlan.p4", - p4c_backend = "@com_github_p4lang_p4c//:p4c_dpdk", - arch = "pna", - target = "dpdk", - context_out = "vxlan.context", - bf_rt_schema_out = "vxlan.bf-rt-schema", - target_out = "vxlan.spec", + name = "vxlan_dpdk_pna", + src = "vxlan.p4", + arch = "pna", + bf_rt_schema_out = "vxlan.bf-rt-schema", + context_out = "vxlan.context", + p4c_backend = "@com_github_p4lang_p4c//:p4c_dpdk", + target = "dpdk", + target_out = "vxlan.spec", ) p4_graphs( - name = "program_graphs", - src = "program.p4", - deps = ["empty.p4"], - out = "program_graphs.dot", + name = "program_graphs", + src = "program.p4", + out = "program_graphs.dot", + deps = ["empty.p4"], ) diff --git a/bazel/p4_library.bzl b/bazel/p4_library.bzl index caaf1c53f80..07e38eb62a7 100644 --- a/bazel/p4_library.bzl +++ b/bazel/p4_library.bzl @@ -25,12 +25,12 @@ def _extract_p4c_inputs(ctx): return ctx.files._p4include + ctx.files.deps + [ctx.file.src] def _run_shell_cmd_with_p4c(ctx, command, **run_shell_kwargs): - """Run given shell command using `run_shell` action after setting up - the C compiler toolchain. + """Run given shell command using the `run_shell` action. - This function also sets up the `tools` parameter for `run_shell` to - set up p4c and the cpp toolchain, and `kwargs` is passed to - `run_shell`. + This is done after setting up the C compiler toolchain. + This function also sets up the `tools` parameter for `run_shell` to + set up p4c and the cpp toolchain, and `kwargs` is passed to + `run_shell`. """ if not hasattr(ctx.executable, "p4c_backend"): diff --git a/bazel/p4c_deps.bzl b/bazel/p4c_deps.bzl index 96d380d884c..9ab4491f7ef 100644 --- a/bazel/p4c_deps.bzl +++ b/bazel/p4c_deps.bzl @@ -3,15 +3,19 @@ load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") -def p4c_deps(): - """Loads dependencies need to compile p4c.""" - # Third party projects can define the target - # @com_github_p4lang_p4c_extension:ir_extension with a `filegroup` - # containing their custom .def files. +def p4c_deps(name = "com_github_p4lang_p4c_extension"): + """Loads dependencies need to compile p4c. + + Args: + name: The name of the repository. + Third party projects can define the target + @com_github_p4lang_p4c_extension:ir_extension with a `filegroup` + containing their custom .def files. + """ if not native.existing_rule("com_github_p4lang_p4c_extension"): # By default, no IR extensions. native.new_local_repository( - name = "com_github_p4lang_p4c_extension", + name = name, path = ".", build_file_content = """ filegroup( @@ -106,4 +110,3 @@ filegroup( sha256 = "95651d7d1fcf2e5c3163c3d37df6d6b3e9e5027299e6bd050d157322ceda9ac9", build_file = "@//:bazel/BUILD.json.bazel", ) -