From bc394d2c6df4fa9c69641b351f86786abac2547f Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Wed, 21 Aug 2024 17:04:54 -0700 Subject: [PATCH] Moved the MiniTable generator into a separate directory. This is the first in a series of changes intended to improve the code structure of the generator. PiperOrigin-RevId: 666093310 --- bazel/upb_minitable_proto_library.bzl | 2 +- pkg/BUILD.bazel | 2 +- upb/reflection/BUILD | 1 - upb_generator/BUILD | 75 --------------- upb_generator/bootstrap_compiler.bzl | 76 +++++++++++++--- upb_generator/minitable/BUILD | 91 +++++++++++++++++++ .../generator.cc} | 2 +- .../generator.h} | 0 .../main.cc} | 2 +- 9 files changed, 157 insertions(+), 94 deletions(-) create mode 100644 upb_generator/minitable/BUILD rename upb_generator/{protoc-gen-upb_minitable.cc => minitable/generator.cc} (99%) rename upb_generator/{protoc-gen-upb_minitable.h => minitable/generator.h} (100%) rename upb_generator/{protoc-gen-upb_minitable-main.cc => minitable/main.cc} (98%) diff --git a/bazel/upb_minitable_proto_library.bzl b/bazel/upb_minitable_proto_library.bzl index bba8c18870..0a178421a2 100644 --- a/bazel/upb_minitable_proto_library.bzl +++ b/bazel/upb_minitable_proto_library.bzl @@ -45,7 +45,7 @@ upb_minitable_proto_library_aspect = aspect( default = "//upb:upb_proto_library_copts__for_generated_code_only_do_not_use", ), "_upb_minitable_toolchain": attr.label( - default = Label("//upb_generator:protoc-gen-upb_minitable_toolchain"), + default = Label("//upb_generator/minitable:toolchain"), ), "_cc_toolchain": attr.label( default = "@bazel_tools//tools/cpp:current_cc_toolchain", diff --git a/pkg/BUILD.bazel b/pkg/BUILD.bazel index 4f4de34c01..96c449018b 100644 --- a/pkg/BUILD.bazel +++ b/pkg/BUILD.bazel @@ -256,7 +256,7 @@ cc_dist_library( ], tags = ["manual"], deps = [ - "//upb_generator:protoc-gen-upb_minitable_lib_lib", + "//upb_generator/minitable:generator_with_main", ], ) diff --git a/upb/reflection/BUILD b/upb/reflection/BUILD index 1251b57ec9..57dd22a43f 100644 --- a/upb/reflection/BUILD +++ b/upb/reflection/BUILD @@ -36,7 +36,6 @@ bootstrap_upb_proto_library( oss_src_rules = ["//:descriptor_proto_srcs"], oss_strip_prefix = "third_party/protobuf/github/bootstrap/src", proto_lib_deps = ["//:descriptor_proto"], - visibility = ["//visibility:public"], ) upb_proto_reflection_library( diff --git a/upb_generator/BUILD b/upb_generator/BUILD index cff397481a..5ad00f9b86 100644 --- a/upb_generator/BUILD +++ b/upb_generator/BUILD @@ -76,7 +76,6 @@ bootstrap_upb_proto_library( ], oss_strip_prefix = "third_party/protobuf/github/bootstrap/src", proto_lib_deps = ["//:compiler_plugin_proto"], - visibility = ["//upb:friends"], deps = ["//upb/reflection:descriptor_upb_proto"], ) @@ -299,80 +298,6 @@ proto_lang_toolchain( visibility = ["//visibility:public"], ) -bootstrap_cc_library( - name = "protoc-gen-upb_minitable_lib", - srcs = ["protoc-gen-upb_minitable.cc"], - hdrs = ["protoc-gen-upb_minitable.h"], - bootstrap_deps = [ - ":common", - ":file_layout", - ":names", - ":plugin", - ":plugin_upb_proto", - "//upb/reflection:descriptor_upb_proto", - "//upb/reflection:reflection", - ], - copts = UPB_DEFAULT_CPPOPTS, - visibility = ["//upb:friends"], - deps = [ - "//src/google/protobuf/compiler:code_generator", - "//upb:base", - "//upb:mem", - "//upb:mini_table", - "//upb:port", - "//upb:wire_reader", - "//upb/mini_table:internal", - "@com_google_absl//absl/container:flat_hash_map", - "@com_google_absl//absl/container:flat_hash_set", - "@com_google_absl//absl/log:absl_check", - "@com_google_absl//absl/log:absl_log", - "@com_google_absl//absl/strings", - ], -) - -bootstrap_cc_binary( - name = "protoc-gen-upb_minitable", - bootstrap_deps = [ - ":protoc-gen-upb_minitable_lib_lib", - ], - copts = UPB_DEFAULT_CPPOPTS, - visibility = ["//visibility:public"], -) - -# TODO: This wrapper lib is a hack that we need because of how CcInfo works in Bazel 6. -# In Bazel 7, our cmake dependency scraping works fine with cc_binary. -bootstrap_cc_library( - name = "protoc-gen-upb_minitable_lib_lib", - srcs = ["protoc-gen-upb_minitable-main.cc"], - bootstrap_deps = [ - ":file_layout", - ":common", - ":plugin", - ":protoc-gen-upb_minitable_lib", - "//upb/reflection:reflection", - ], - copts = UPB_DEFAULT_CPPOPTS, - visibility = ["//pkg:__pkg__"], - deps = [ - "//upb:base", - "//upb:port", - "@com_google_absl//absl/log:absl_check", - "@com_google_absl//absl/log:absl_log", - "@com_google_absl//absl/strings", - ], -) - -proto_lang_toolchain( - name = "protoc-gen-upb_minitable_toolchain", - command_line = "--upb_minitable_out=$(OUT)", - output_files = "multiple", - plugin = ":protoc-gen-upb_minitable_stage1", - plugin_format_flag = "--plugin=protoc-gen-upb_minitable=%s", - progress_message = "Generating upb minitables", - runtime = "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", - visibility = ["//visibility:public"], -) - cc_binary( name = "protoc-gen-upbdefs", linkopts = ["-lm"], diff --git a/upb_generator/bootstrap_compiler.bzl b/upb_generator/bootstrap_compiler.bzl index 23b6fc0827..95fc98e0ea 100644 --- a/upb_generator/bootstrap_compiler.bzl +++ b/upb_generator/bootstrap_compiler.bzl @@ -18,7 +18,6 @@ load( _stages = ["_stage0", "_stage1", ""] _protoc = "//:protoc" -_upbc_base = "//upb_generator:protoc-gen-" # begin:google_only # _is_google3 = True @@ -30,24 +29,70 @@ _is_google3 = False _extra_proto_path = "-I$$(dirname $(location @com_google_protobuf//:descriptor_proto_srcs))/../.. " # end:github_only +# This visibility is used automatically for anything used by the bootstrapping process. +_bootstrap_visibility = [ + "//upb_generator:__subpackages__", + "//upb/reflection:__pkg__", + # begin:github_only + "//upb:__pkg__", # For the amalgamations. + "//python/dist:__pkg__", # For the Python source package. + # end:github_only +] + +def _stage_visibility(stage, visibility): + return visibility if stage == "" else _bootstrap_visibility + def _upbc(generator, stage): - return _upbc_base + generator + _stages[stage] + if generator == "upb": + return "//upb_generator:protoc-gen-upb" + _stages[stage] + else: + return "//upb_generator/minitable:protoc-gen-upb_minitable" + _stages[stage] + +def bootstrap_cc_library(name, visibility = [], deps = [], bootstrap_deps = [], **kwargs): + """A version of cc_library() that is augmented to allow for bootstrapping the compiler. -def bootstrap_cc_library(name, visibility, deps = [], bootstrap_deps = [], **kwargs): + In addition to the normal cc_library() target, this rule will also generate _stage0 and _stage1 + targets that are used internally for bootstrapping, and will automatically have bootstrap + visibility. However the final target will use the normal visibility, and will behave like a + normal cc_library() target. + + Args: + name: Name of this rule. This name will resolve to a upb_proto_library(). + deps: Normal cc_library() deps. + bootstrap_deps: Special bootstrap_upb_proto_library() or bootstrap_cc_library() deps. + visibility: Visibility of the final target. + **kwargs: Other arguments that will be passed through to cc_library(). + upb_proto_library(). + """ for stage in _stages: - stage_visibility = visibility if stage == "" else ["//upb_generator:__pkg__"] native.cc_library( name = name + stage, deps = deps + [dep + stage for dep in bootstrap_deps], - visibility = stage_visibility, + visibility = _stage_visibility(stage, visibility), **kwargs ) -def bootstrap_cc_binary(name, deps = [], bootstrap_deps = [], **kwargs): +def bootstrap_cc_binary(name, visibility = [], deps = [], bootstrap_deps = [], **kwargs): + """A version of cc_binary() that is augmented to allow for bootstrapping the compiler. + + In addition to the normal cc_binary() target, this rule will also generate _stage0 and _stage1 + targets that are used internally for bootstrapping, and will automatically have bootstrap + visibility. However the final target will use the normal visibility, and will behave like a + normal cc_binary() target. + + Args: + name: Name of this rule. This name will resolve to a upb_proto_library(). + deps: Normal cc_library() deps. + bootstrap_deps: Special bootstrap_upb_proto_library() or bootstrap_cc_library() deps. + visibility: Visibility of the final target. + **kwargs: Other arguments that will be passed through to cc_binary(). + upb_proto_library(). + """ for stage in _stages: native.cc_binary( name = name + stage, deps = deps + [dep + stage for dep in bootstrap_deps], + visibility = _stage_visibility(stage, visibility), **kwargs ) @@ -100,7 +145,7 @@ def _generate_stage1_proto(name, src_files, src_rules, generator, kwargs): "=$(location " + _upbc(generator, 0) + ") " + _extra_proto_path + "--" + generator + "_out=bootstrap_stage=1:$(RULEDIR)/stage1 " + " ".join(src_files), - visibility = ["//upb_generator:__pkg__"], + visibility = _bootstrap_visibility, tools = [ _protoc, _upbc(generator, 0), @@ -155,13 +200,18 @@ def bootstrap_upb_proto_library( oss_src_rules, oss_strip_prefix, proto_lib_deps, - visibility, deps = [], **kwargs): """A version of upb_proto_library() that is augmented to allow for bootstrapping the compiler. + Note that this rule is only intended to be used by bootstrap_cc_library() targets. End users + should use the normal upb_proto_library() targets. As a result, we don't have a visibility + parameter: all targets will automatically have bootstrap visibility. + Args: name: Name of this rule. This name will resolve to a upb_proto_library(). + bootstrap_hdr: The forwarding header that exposes the generated code, taking into account + the current stage. google3_src_files: Google3 filenames of .proto files that should be built by this rule. The names should be relative to the depot base. google3_src_rules: Target names of the Blaze rules that will provide these filenames. @@ -170,8 +220,6 @@ def bootstrap_upb_proto_library( oss_strip_prefix: Prefix that should be stripped from OSS file names. proto_lib_deps: proto_library() rules that we will use to build the protos when we are not bootstrapping. - visibility: Visibility list for the final upb_proto_library() rule. Bootstrapping rules - will always be hidden, and will not honor the visibility parameter passed here. deps: other bootstrap_upb_proto_library() rules that this one depends on. **kwargs: Other arguments that will be passed through to cc_library(), genrule(), and upb_proto_library(). @@ -183,7 +231,7 @@ def bootstrap_upb_proto_library( name = name + "_stage0", srcs = _generated_hdrs_and_srcs(oss_src_files, "stage0", "upb"), hdrs = [bootstrap_hdr], - visibility = ["//upb_generator:__pkg__"], + visibility = _bootstrap_visibility, defines = ["UPB_BOOTSTRAP_STAGE=0"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -203,7 +251,7 @@ def bootstrap_upb_proto_library( name = name + "_minitable_stage1", srcs = _generated_files(src_files, "stage1", "upb_minitable", "c"), hdrs = _generated_files(src_files, "stage1", "upb_minitable", "h"), - visibility = ["//upb_generator:__pkg__"], + visibility = _bootstrap_visibility, defines = ["UPB_BOOTSTRAP_STAGE=1"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -214,7 +262,7 @@ def bootstrap_upb_proto_library( name = name + "_stage1", srcs = _generated_files(src_files, "stage1", "upb", "h"), hdrs = [bootstrap_hdr], - visibility = ["//upb_generator:__pkg__"], + visibility = _bootstrap_visibility, defines = ["UPB_BOOTSTRAP_STAGE=1"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -233,7 +281,7 @@ def bootstrap_upb_proto_library( name = name, hdrs = [bootstrap_hdr], deps = [name + "_upb_proto"], - visibility = visibility, + visibility = _bootstrap_visibility, **kwargs ) diff --git a/upb_generator/minitable/BUILD b/upb_generator/minitable/BUILD new file mode 100644 index 0000000000..1b597adbbe --- /dev/null +++ b/upb_generator/minitable/BUILD @@ -0,0 +1,91 @@ +# Copyright (c) 2009-2024, Google LLC +# All rights reserved. +# +# Use of this source code is governed by a BSD-style +# license that can be found in the LICENSE file or at +# https://developers.google.com/open-source/licenses/bsd + +load("//upb/bazel:build_defs.bzl", "UPB_DEFAULT_CPPOPTS") +load( + "//upb_generator:bootstrap_compiler.bzl", + "bootstrap_cc_binary", + "bootstrap_cc_library", +) + +bootstrap_cc_library( + name = "generator", + srcs = ["generator.cc"], + hdrs = ["generator.h"], + bootstrap_deps = [ + "//upb_generator:common", + "//upb_generator:file_layout", + "//upb_generator:names", + "//upb_generator:plugin", + "//upb_generator:plugin_upb_proto", + "//upb/reflection:descriptor_upb_proto", + "//upb/reflection:reflection", + ], + copts = UPB_DEFAULT_CPPOPTS, + deps = [ + "//src/google/protobuf/compiler:code_generator", + "//upb:base", + "//upb:mem", + "//upb:mini_table", + "//upb:port", + "//upb:wire_reader", + "//upb/mini_table:internal", + "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/log:absl_check", + "@com_google_absl//absl/log:absl_log", + "@com_google_absl//absl/strings", + ], +) + +bootstrap_cc_binary( + name = "protoc-gen-upb_minitable", + bootstrap_deps = [ + ":generator_with_main", + ], + copts = UPB_DEFAULT_CPPOPTS, + visibility = [ + "//editions/codegen_tests:__pkg__", + "//net/proto2/contrib/protoc_explorer:__pkg__", + "//third_party/prototiller/transformer:__pkg__", + ], +) + +# TODO: This wrapper lib is a hack that we need because of how CcInfo works in Bazel 6. +# In Bazel 7, our cmake dependency scraping works fine with cc_binary. +bootstrap_cc_library( + name = "generator_with_main", + srcs = ["main.cc"], + bootstrap_deps = [ + "//upb_generator:file_layout", + "//upb_generator:common", + "//upb_generator:plugin", + ":generator", + "//upb/reflection:reflection", + ], + copts = UPB_DEFAULT_CPPOPTS, + visibility = ["//pkg:__pkg__"], + deps = [ + "//upb:base", + "//upb:port", + "@com_google_absl//absl/log:absl_check", + "@com_google_absl//absl/log:absl_log", + "@com_google_absl//absl/strings", + ], +) + +proto_lang_toolchain( + name = "toolchain", + command_line = "--upb_minitable_out=$(OUT)", + output_files = "multiple", + plugin = ":protoc-gen-upb_minitable_stage1", + plugin_format_flag = "--plugin=protoc-gen-upb_minitable=%s", + progress_message = "Generating upb minitables", + runtime = "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", + # TODO: Restrict to "//bazel:__pkg__" once we are on Bazel >=6.5. + visibility = ["//visibility:public"], +) diff --git a/upb_generator/protoc-gen-upb_minitable.cc b/upb_generator/minitable/generator.cc similarity index 99% rename from upb_generator/protoc-gen-upb_minitable.cc rename to upb_generator/minitable/generator.cc index a12f0cdc97..9a03e91632 100644 --- a/upb_generator/protoc-gen-upb_minitable.cc +++ b/upb_generator/minitable/generator.cc @@ -5,7 +5,7 @@ // license that can be found in the LICENSE file or at // https://developers.google.com/open-source/licenses/bsd -#include "upb_generator/protoc-gen-upb_minitable.h" +#include "upb_generator/minitable/generator.h" #include diff --git a/upb_generator/protoc-gen-upb_minitable.h b/upb_generator/minitable/generator.h similarity index 100% rename from upb_generator/protoc-gen-upb_minitable.h rename to upb_generator/minitable/generator.h diff --git a/upb_generator/protoc-gen-upb_minitable-main.cc b/upb_generator/minitable/main.cc similarity index 98% rename from upb_generator/protoc-gen-upb_minitable-main.cc rename to upb_generator/minitable/main.cc index f970235ba1..00a943b2a8 100644 --- a/upb_generator/protoc-gen-upb_minitable-main.cc +++ b/upb_generator/minitable/main.cc @@ -15,8 +15,8 @@ #include "upb/reflection/def.hpp" #include "upb_generator/common.h" #include "upb_generator/file_layout.h" +#include "upb_generator/minitable/generator.h" #include "upb_generator/plugin.h" -#include "upb_generator/protoc-gen-upb_minitable.h" // Must be last. #include "upb/port/def.inc"