Fixed layering check violations once and for all in upb bootstrapping. Our bootstrapping setup compiles multiple versions of the generated code for `descriptor.proto` and `plugin.proto`, one for each stage of the bootstrap. For source files (`.c`), we can always select the correct version of the file in the BUILD rules, but for header files we need to make sure the correct stage's file is always selected via `#include`. Previously we used `cc_library(includes=[])` to make it appear as though our bootstrapped headers had the same names as the "real" headers. This allowed a lot of the code to be agnostic to whether a bootstrap header was being used, which simplified things because we did not have to change the code performing the `#include`. Unfortunately, due to build system limitations, this sometimes led to the incorrect header getting included. This should not have been possible, because we had a clean BUILD graph that should have removed all ambiguity about which header should be available. But in non-sandboxed builds, the compiler was able to find headers that were not actually in `deps=[]`, and worse it preferred those headers over the headers that actually were in `deps=[]`. This led to unintended results and errors about layering check violations. This CL fixes the problem by removing all use of `includes=[]`. We now spell a full pathname to all bootstrap headers, so this class of errors is no longer possible. Unfortunately this adds some complexity, as we have to hard-code these full paths in several places. A nice improvement in this CL is that `bootstrap_upb_proto_library()` can now only be used for bootstrapping; it only exposes the `descriptor_bootstrap.h` / `plugin_bootstrap.h` files. Anyone wanting to use the normal `net/proto2/proto/descriptor.upb.h` file should depend on `//net/proto2/proto:descriptor_upb_c_proto` target instead. PiperOrigin-RevId: 664953196
diff --git a/benchmarks/BUILD b/benchmarks/BUILD index 59d3fbf..766f366 100644 --- a/benchmarks/BUILD +++ b/benchmarks/BUILD
@@ -74,7 +74,6 @@ "//:protobuf", "//src/google/protobuf/json", "//upb:base", - "//upb:descriptor_upb_proto", "//upb:json", "//upb:mem", "//upb:reflection",
diff --git a/python/BUILD.bazel b/python/BUILD.bazel index 3e34bbd..e8a7625 100644 --- a/python/BUILD.bazel +++ b/python/BUILD.bazel
@@ -191,9 +191,9 @@ ], target_compatible_with = select(_message_target_compatible_with), deps = [ + "//src/google/protobuf:descriptor_upb_reflection_proto", "//third_party/utf8_range", "//upb:base", - "//upb:descriptor_upb_proto_reflection", "//upb:eps_copy_input_stream", "//upb:message", "//upb:message_compare",
diff --git a/src/google/protobuf/BUILD.bazel b/src/google/protobuf/BUILD.bazel index 5aa68d4..239e318 100644 --- a/src/google/protobuf/BUILD.bazel +++ b/src/google/protobuf/BUILD.bazel
@@ -6,6 +6,9 @@ load("@rules_pkg//pkg:mappings.bzl", "pkg_files", "strip_prefix") load("//bazel:cc_proto_library.bzl", "cc_proto_library") load("//bazel:proto_library.bzl", "proto_library") +load("//bazel:upb_c_proto_library.bzl", "upb_c_proto_library") +load("//bazel:upb_minitable_proto_library.bzl", "upb_minitable_proto_library") +load("//bazel:upb_proto_reflection_library.bzl", "upb_proto_reflection_library") load("//build_defs:cpp_opts.bzl", "COPTS", "LINK_OPTS") load("//upb/cmake:build_defs.bzl", "staleness_test") @@ -223,6 +226,24 @@ ], ) +upb_c_proto_library( + name = "descriptor_upb_c_proto", + visibility = ["//:__subpackages__"], + deps = [":descriptor_proto"], +) + +upb_minitable_proto_library( + name = "descriptor_upb_minitable_proto", + visibility = ["//:__subpackages__"], + deps = [":descriptor_proto"], +) + +upb_proto_reflection_library( + name = "descriptor_upb_reflection_proto", + visibility = ["//:__subpackages__"], + deps = [":descriptor_proto"], +) + proto_library( name = "cpp_features_proto", srcs = ["cpp_features.proto"],
diff --git a/src/google/protobuf/compiler/BUILD.bazel b/src/google/protobuf/compiler/BUILD.bazel index 0f4c053..e9df8e4 100644 --- a/src/google/protobuf/compiler/BUILD.bazel +++ b/src/google/protobuf/compiler/BUILD.bazel
@@ -11,6 +11,8 @@ ) load("//bazel:cc_proto_library.bzl", "cc_proto_library") load("//bazel:proto_library.bzl", "proto_library") +load("//bazel:upb_c_proto_library.bzl", "upb_c_proto_library") +load("//bazel:upb_proto_reflection_library.bzl", "upb_proto_reflection_library") load("//build_defs:arch_tests.bzl", "aarch64_test", "x86_64_test") load("//build_defs:cpp_opts.bzl", "COPTS") load("test_plugin_injection.bzl", "inject_plugin_paths") @@ -32,6 +34,18 @@ deps = [":plugin_proto"], ) +upb_c_proto_library( + name = "plugin_upb_c_proto", + visibility = ["//upb_generator:__subpackages__"], + deps = [":plugin_proto"], +) + +upb_proto_reflection_library( + name = "plugin_upb_reflection_proto", + visibility = ["//upb_generator:__subpackages__"], + deps = [":plugin_proto"], +) + cc_library( name = "importer", srcs = [
diff --git a/upb/BUILD b/upb/BUILD index b46ee4b..48de962 100644 --- a/upb/BUILD +++ b/upb/BUILD
@@ -261,7 +261,6 @@ ], libs = [ ":base", - ":descriptor_upb_minitable_proto", ":descriptor_upb_proto", ":eps_copy_input_stream", ":generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -276,6 +275,8 @@ ":reflection", ":wire", ":wire_reader", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf:descriptor_upb_minitable_proto", "//upb/base:internal", "//upb/hash:hash", "//upb/lex:lex", @@ -307,8 +308,6 @@ ], libs = [ ":base", - ":descriptor_upb_minitable_proto", - ":descriptor_upb_proto_reflection", ":descriptor_upb_proto", ":eps_copy_input_stream", ":generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -324,6 +323,9 @@ ":reflection", ":wire", ":wire_reader", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf:descriptor_upb_minitable_proto", + "//src/google/protobuf:descriptor_upb_reflection_proto", "//upb/base:internal", "//upb/hash:hash", "//upb/lex:lex", @@ -356,7 +358,6 @@ ], libs = [ ":base", - ":descriptor_upb_minitable_proto", ":descriptor_upb_proto", ":eps_copy_input_stream", ":generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", @@ -372,6 +373,8 @@ ":reflection", ":wire", ":wire_reader", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf:descriptor_upb_minitable_proto", "//upb/base:internal", "//upb/hash:hash", "//upb/lex:lex",
diff --git a/upb/bazel/amalgamate.py b/upb/bazel/amalgamate.py index f4e83fc..96f142c 100755 --- a/upb/bazel/amalgamate.py +++ b/upb/bazel/amalgamate.py
@@ -106,6 +106,8 @@ if include.endswith("hpp"): # Skip, we don't support the amalgamation from C++. return True + if re.search(r"stage\d/", include): + return True elif include in self.included: return True else:
diff --git a/upb/cmake/BUILD.bazel b/upb/cmake/BUILD.bazel index fdf0f23..81338ed 100644 --- a/upb/cmake/BUILD.bazel +++ b/upb/cmake/BUILD.bazel
@@ -47,8 +47,8 @@ genrule( name = "copy_protos", srcs = [ - "//upb:descriptor_upb_proto", - "//upb:descriptor_upb_minitable_proto", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf:descriptor_upb_minitable_proto", ], outs = [ "generated-in/google/protobuf/descriptor.upb.h",
diff --git a/upb/port/def.inc b/upb/port/def.inc index 2bcdb88..efc6d5d 100644 --- a/upb/port/def.inc +++ b/upb/port/def.inc
@@ -338,10 +338,11 @@ // #define UPB_IS_GOOGLE3 // end:google_only -#if defined(UPB_IS_GOOGLE3) && !defined(UPB_BOOTSTRAP_STAGE0) +#if defined(UPB_IS_GOOGLE3) && \ + (!defined(UPB_BOOTSTRAP_STAGE) || UPB_BOOTSTRAP_STAGE != 0) #define UPB_DESC(sym) proto2_##sym #define UPB_DESC_MINITABLE(sym) &proto2__##sym##_msg_init -#elif defined(UPB_BOOTSTRAP_STAGE0) +#elif defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 0 #define UPB_DESC(sym) google_protobuf_##sym #define UPB_DESC_MINITABLE(sym) google__protobuf__##sym##_msg_init() #else
diff --git a/upb/reflection/BUILD b/upb/reflection/BUILD index 694b1f0..1251b57 100644 --- a/upb/reflection/BUILD +++ b/upb/reflection/BUILD
@@ -29,9 +29,7 @@ bootstrap_upb_proto_library( name = "descriptor_upb_proto", - base_dir = "", - # TODO: Export 'net/proto2/proto/descriptor.upb.h' and remove "-layering_check". - features = ["-layering_check"], + bootstrap_hdr = "descriptor_bootstrap.h", google3_src_files = ["net/proto2/proto/descriptor.proto"], google3_src_rules = ["//net/proto2/proto:descriptor_proto_source"], oss_src_files = ["google/protobuf/descriptor.proto"],
diff --git a/upb/reflection/common.h b/upb/reflection/common.h index ebe3282..e2d5221 100644 --- a/upb/reflection/common.h +++ b/upb/reflection/common.h
@@ -12,17 +12,7 @@ #ifndef UPB_REFLECTION_COMMON_H_ #define UPB_REFLECTION_COMMON_H_ -// begin:google_only -// #ifndef UPB_BOOTSTRAP_STAGE0 -// #include "net/proto2/proto/descriptor.upb.h" -// #else -// #include "google/protobuf/descriptor.upb.h" -// #endif -// end:google_only - -// begin:github_only -#include "google/protobuf/descriptor.upb.h" -// end:github_only +#include "upb/reflection/descriptor_bootstrap.h" // IWYU pragma: export typedef enum { kUpb_Syntax_Proto2 = 2,
diff --git a/upb/reflection/descriptor_bootstrap.h b/upb/reflection/descriptor_bootstrap.h new file mode 100644 index 0000000..c0b760c --- /dev/null +++ b/upb/reflection/descriptor_bootstrap.h
@@ -0,0 +1,19 @@ +#ifndef THIRD_PARTY_UPB_UPB_REFLECTION_DESCRIPTOR_BOOTSTRAP_H_ +#define THIRD_PARTY_UPB_UPB_REFLECTION_DESCRIPTOR_BOOTSTRAP_H_ + +// IWYU pragma: begin_exports + +#if defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 0 +// This header is checked in. +#include "upb/reflection/stage0/google/protobuf/descriptor.upb.h" +#elif UPB_BOOTSTRAP_STAGE == 1 +// This header is generated at build time by the bootstrapping process. +#include "upb/reflection/stage1/google/protobuf/descriptor.upb.h" +#else +// This is the normal header, generated by upb_c_proto_library(). +#include "google/protobuf/descriptor.upb.h" +#endif + +// IWYU pragma: end_exports + +#endif // THIRD_PARTY_UPB_UPB_REFLECTION_DESCRIPTOR_BOOTSTRAP_H_
diff --git a/upb/reflection/field_def.c b/upb/reflection/field_def.c index 59f1b0a..3ba3cba 100644 --- a/upb/reflection/field_def.c +++ b/upb/reflection/field_def.c
@@ -591,7 +591,7 @@ } // begin:google_only -// #ifndef UPB_BOOTSTRAP_STAGE0 +// #if UPB_BOOTSTRAP_STAGE != 0 // if (syntax == kUpb_Syntax_Proto3 && // UPB_DESC(FieldOptions_has_enforce_utf8)(options) && // !UPB_DESC(FieldOptions_enforce_utf8)(options)) {
diff --git a/upb/reflection/stage0/google/protobuf/descriptor.upb.c b/upb/reflection/stage0/google/protobuf/descriptor.upb.c index eb58624..3f5c673 100644 --- a/upb/reflection/stage0/google/protobuf/descriptor.upb.c +++ b/upb/reflection/stage0/google/protobuf/descriptor.upb.c
@@ -1,6 +1,6 @@ #include <stddef.h> #include "upb/generated_code_support.h" -#include "google/protobuf/descriptor.upb.h" +#include "upb/reflection/descriptor_bootstrap.h" static upb_Arena* upb_BootstrapArena() { static upb_Arena* arena = NULL;
diff --git a/upb/util/BUILD b/upb/util/BUILD index fe7e6c0..40b1f63 100644 --- a/upb/util/BUILD +++ b/upb/util/BUILD
@@ -18,8 +18,8 @@ hdrs = ["def_to_proto.h"], visibility = ["//visibility:public"], deps = [ + "//src/google/protobuf:descriptor_upb_c_proto", "//upb:base", - "//upb:descriptor_upb_proto", "//upb:port", "//upb:reflection", "//upb/reflection:internal", @@ -52,9 +52,9 @@ deps = [ ":def_to_proto", "//:protobuf", + "//src/google/protobuf:descriptor_upb_c_proto", "//src/google/protobuf/util:differencer", "//upb:base", - "//upb:descriptor_upb_proto", "//upb:mem", "//upb/reflection:internal", "@com_google_googletest//:gtest", @@ -72,10 +72,10 @@ ":def_to_proto_test_upb_proto", ":def_to_proto_test_upb_proto_reflection", "//:protobuf", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf:descriptor_upb_reflection_proto", "//src/google/protobuf/util:differencer", "//upb:base", - "//upb:descriptor_upb_proto", - "//upb:descriptor_upb_proto_reflection", "//upb:mem", "//upb:reflection", "//upb/test:parse_text_proto",
diff --git a/upb_generator/BUILD b/upb_generator/BUILD index 43be436..cff3974 100644 --- a/upb_generator/BUILD +++ b/upb_generator/BUILD
@@ -59,9 +59,7 @@ bootstrap_upb_proto_library( name = "plugin_upb_proto", - base_dir = "", - # TODO: Export 'net/proto2/proto/descriptor.upb.h' and remove "-layering_check". - features = ["-layering_check"], + bootstrap_hdr = "plugin_bootstrap.h", google3_src_files = [ "net/proto2/compiler/proto/profile.proto", "third_party/protobuf/compiler/plugin.proto", @@ -234,10 +232,10 @@ deps = [ ":code_generator_request_upb_proto", ":code_generator_request_upb_proto_reflection", - ":plugin_upb_proto", - ":plugin_upb_proto_reflection", + "//src/google/protobuf:descriptor_upb_c_proto", + "//src/google/protobuf/compiler:plugin_upb_c_proto", + "//src/google/protobuf/compiler:plugin_upb_reflection_proto", "//upb:base", - "//upb:descriptor_upb_proto", "//upb:json", "//upb:mem", "//upb:message", @@ -281,6 +279,7 @@ "//upb:mini_table", "//upb:port", "//upb:wire_reader", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/log:absl_check", @@ -400,7 +399,7 @@ ":common", ":file_layout", ":plugin", - "//upb:descriptor_upb_proto", + "//src/google/protobuf:descriptor_upb_c_proto", "//upb:reflection", "//upb/util:def_to_proto", ],
diff --git a/upb_generator/bootstrap_compiler.bzl b/upb_generator/bootstrap_compiler.bzl index e92fbaf..23b6fc0 100644 --- a/upb_generator/bootstrap_compiler.bzl +++ b/upb_generator/bootstrap_compiler.bzl
@@ -51,38 +51,38 @@ **kwargs ) -def _generated_srcs_for_suffix(prefix, srcs, suffix): - return [prefix + "/" + src[:-len(".proto")] + suffix for src in srcs] +def _generated_file(proto, stage, generator, suffix): + stripped = proto[:-len(".proto")] + return "{}/{}.{}.{}".format(stage, stripped, generator, suffix) -def _generated_srcs_for_generator(prefix, srcs, generator): - ret = _generated_srcs_for_suffix(prefix, srcs, ".{}.h".format(generator)) +def _generated_files(protos, stage, generator, suffix): + return [_generated_file(proto, stage, generator, suffix) for proto in protos] - if generator != "upb" or prefix.endswith("stage0"): - ret += _generated_srcs_for_suffix(prefix, srcs, ".{}.c".format(generator)) +def _generated_hdrs_and_srcs(protos, stage, generator): + ret = _generated_files(protos, stage, generator, "h") + if generator != "upb" or stage == "stage0": + ret += _generated_files(protos, stage, generator, "c") return ret -def _generated_srcs(prefix, srcs): - return _generated_srcs_for_generator(prefix, srcs, "upb") - -def _stage0_proto_staleness_test(name, base_dir, src_files, src_rules, strip_prefix): +def _stage0_proto_staleness_test(name, src_files, src_rules, strip_prefix): native.genrule( name = name + "_generate_bootstrap", srcs = src_rules, - outs = _generated_srcs("bootstrap_generated_sources/" + base_dir + "stage0", src_files), + outs = ["bootstrap_generated_sources/" + f for f in _generated_hdrs_and_srcs(src_files, "stage0", "upb")], tools = [_protoc, _upbc("upb", 0)], cmd = "$(location " + _protoc + ") " + "-I$(GENDIR)/" + strip_prefix + " " + _extra_proto_path + "--plugin=protoc-gen-upb=$(location " + _upbc("upb", 0) + ") " + - "--upb_out=bootstrap_upb:$(@D)/bootstrap_generated_sources/" + base_dir + "stage0 " + + "--upb_out=bootstrap_stage=0:$(@D)/bootstrap_generated_sources/stage0 " + " ".join(src_files), ) staleness_test( name = name + "_stage0_staleness_test", - outs = _generated_srcs(base_dir + "stage0", src_files), + outs = _generated_hdrs_and_srcs(src_files, "stage0", "upb"), generated_pattern = "bootstrap_generated_sources/%s", - target_files = native.glob([base_dir + "stage0/**"]), + target_files = native.glob(["stage0/**"]), # To avoid skew problems for descriptor.proto/pluging.proto between # GitHub repos. It's not critical that the checked-in protos are up to # date for every change, they just needs to be complete enough to have @@ -90,15 +90,15 @@ tags = ["manual"], ) -def _generate_stage1_proto(name, base_dir, src_files, src_rules, generator, kwargs): +def _generate_stage1_proto(name, src_files, src_rules, generator, kwargs): native.genrule( name = "gen_{}_{}_stage1".format(name, generator), srcs = src_rules, - outs = _generated_srcs_for_generator(base_dir + "stage1", src_files, generator), + outs = _generated_hdrs_and_srcs(src_files, "stage1", generator), cmd = "$(location " + _protoc + ") " + "--plugin=protoc-gen-" + generator + "=$(location " + _upbc(generator, 0) + ") " + _extra_proto_path + - "--" + generator + "_out=$(RULEDIR)/" + base_dir + "stage1 " + + "--" + generator + "_out=bootstrap_stage=1:$(RULEDIR)/stage1 " + " ".join(src_files), visibility = ["//upb_generator:__pkg__"], tools = [ @@ -109,7 +109,7 @@ ) # begin:github_only -def _cmake_staleness_test(name, base_dir, src_files, proto_lib_deps, **kwargs): +def _cmake_staleness_test(name, src_files, proto_lib_deps, **kwargs): upb_minitable_proto_library( name = name + "_minitable", deps = proto_lib_deps, @@ -117,15 +117,15 @@ ) # Copy the final gencode for staleness comparison - files = _generated_srcs("cmake" + base_dir, src_files) + \ - _generated_srcs_for_generator("cmake" + base_dir, src_files, "upb_minitable") + files = _generated_hdrs_and_srcs(src_files, "cmake", "upb") + \ + _generated_hdrs_and_srcs(src_files, "cmake", "upb_minitable") genrule = 0 for src in files: genrule += 1 native.genrule( name = name + "_copy_gencode_%d" % genrule, outs = ["generated_sources/" + src], - srcs = [name, name + "_minitable"], + srcs = [name + "_upb_proto", name + "_minitable"], cmd = """ mkdir -p $(@D) for src in $(SRCS); do @@ -148,7 +148,7 @@ def bootstrap_upb_proto_library( name, - base_dir, + bootstrap_hdr, google3_src_files, google3_src_rules, oss_src_files, @@ -162,7 +162,6 @@ Args: name: Name of this rule. This name will resolve to a upb_proto_library(). - base_dir: The directory that all generated files should be placed under. 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. @@ -177,17 +176,15 @@ **kwargs: Other arguments that will be passed through to cc_library(), genrule(), and upb_proto_library(). """ - _stage0_proto_staleness_test(name, base_dir, oss_src_files, oss_src_rules, oss_strip_prefix) + _stage0_proto_staleness_test(name, oss_src_files, oss_src_rules, oss_strip_prefix) # stage0 uses checked-in protos, and has no MiniTable. native.cc_library( name = name + "_stage0", - srcs = _generated_srcs_for_suffix(base_dir + "stage0", oss_src_files, ".upb.c"), - hdrs = _generated_srcs_for_suffix(base_dir + "stage0", oss_src_files, ".upb.h"), - includes = [base_dir + "stage0"], + srcs = _generated_hdrs_and_srcs(oss_src_files, "stage0", "upb"), + hdrs = [bootstrap_hdr], visibility = ["//upb_generator:__pkg__"], - # This macro signals to the runtime that it must use OSS APIs for descriptor.proto/plugin.proto. - defines = ["UPB_BOOTSTRAP_STAGE0"], + defines = ["UPB_BOOTSTRAP_STAGE=0"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", "//upb:mini_table", @@ -199,41 +196,48 @@ src_rules = google3_src_rules if _is_google3 else oss_src_rules # Generate stage1 protos (C API and MiniTables) using stage0 compiler. - _generate_stage1_proto(name, base_dir, src_files, src_rules, "upb", kwargs) - _generate_stage1_proto(name, base_dir, src_files, src_rules, "upb_minitable", kwargs) + _generate_stage1_proto(name, src_files, src_rules, "upb", kwargs) + _generate_stage1_proto(name, src_files, src_rules, "upb_minitable", kwargs) native.cc_library( name = name + "_minitable_stage1", - srcs = _generated_srcs_for_suffix(base_dir + "stage1", src_files, ".upb_minitable.c"), - hdrs = _generated_srcs_for_suffix(base_dir + "stage1", src_files, ".upb_minitable.h"), - includes = [base_dir + "stage1"], + srcs = _generated_files(src_files, "stage1", "upb_minitable", "c"), + hdrs = _generated_files(src_files, "stage1", "upb_minitable", "h"), visibility = ["//upb_generator:__pkg__"], + defines = ["UPB_BOOTSTRAP_STAGE=1"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", - ] + [dep + "_stage1" for dep in deps], + ] + [dep + "_minitable_stage1" for dep in deps], **kwargs ) native.cc_library( name = name + "_stage1", - hdrs = _generated_srcs_for_suffix(base_dir + "stage1", src_files, ".upb.h"), - includes = [base_dir + "stage1"], + srcs = _generated_files(src_files, "stage1", "upb", "h"), + hdrs = [bootstrap_hdr], visibility = ["//upb_generator:__pkg__"], + defines = ["UPB_BOOTSTRAP_STAGE=1"], deps = [ "//upb:generated_code_support__only_for_generated_code_do_not_use__i_give_permission_to_break_me", ":" + name + "_minitable_stage1", - ] + [dep + "_stage1" for dep in deps], + ] + [dep + "_minitable_stage1" for dep in deps], **kwargs ) # The final protos are generated via normal upb_proto_library(). upb_proto_library( - name = name, + name = name + "_upb_proto", deps = proto_lib_deps, + **kwargs + ) + native.cc_library( + name = name, + hdrs = [bootstrap_hdr], + deps = [name + "_upb_proto"], visibility = visibility, **kwargs ) # begin:github_only - _cmake_staleness_test(name, base_dir, src_files, proto_lib_deps, **kwargs) + _cmake_staleness_test(name, src_files, proto_lib_deps, **kwargs) # end:github_only
diff --git a/upb_generator/common.cc b/upb_generator/common.cc index 2dbe9d7..ef20383 100644 --- a/upb_generator/common.cc +++ b/upb_generator/common.cc
@@ -75,12 +75,36 @@ return ToCIdent(file.name()) + "_upb_file_layout"; } -std::string CApiHeaderFilename(upb::FileDefPtr file) { +bool HasFilename(upb::FileDefPtr file, absl::string_view filename) { + return file.name() == filename; +} + +bool IsDescriptorProto(upb::FileDefPtr file) { + return HasFilename(file, "net/proto2/proto/descriptor.proto") || + HasFilename(file, "google/protobuf/descriptor.proto"); +} + +std::string CApiHeaderFilename(upb::FileDefPtr file, bool bootstrap) { + if (bootstrap) { + if (IsDescriptorProto(file)) { + return "upb/reflection/descriptor_bootstrap.h"; + } else { + return "upb_generator/plugin_bootstrap.h"; + } + } return StripExtension(file.name()) + ".upb.h"; } -std::string MiniTableHeaderFilename(upb::FileDefPtr file) { - return StripExtension(file.name()) + ".upb_minitable.h"; +std::string MiniTableHeaderFilename(upb::FileDefPtr file, bool bootstrap) { + std::string base; + if (bootstrap) { + if (IsDescriptorProto(file)) { + base = "upb/reflection/stage1/"; + } else { + base = "upb_generator/stage1/"; + } + } + return base + StripExtension(file.name()) + ".upb_minitable.h"; } std::string EnumInit(upb::EnumDefPtr descriptor) {
diff --git a/upb_generator/common.h b/upb_generator/common.h index 2afa005..415fa22 100644 --- a/upb_generator/common.h +++ b/upb_generator/common.h
@@ -66,8 +66,8 @@ std::string MessageInitName(upb::MessageDefPtr descriptor); std::string MessageName(upb::MessageDefPtr descriptor); std::string FileLayoutName(upb::FileDefPtr file); -std::string MiniTableHeaderFilename(upb::FileDefPtr file); -std::string CApiHeaderFilename(upb::FileDefPtr file); +std::string MiniTableHeaderFilename(upb::FileDefPtr file, bool bootstrap); +std::string CApiHeaderFilename(upb::FileDefPtr file, bool bootstrap); std::string PadPrefix(absl::string_view tag); std::string EnumInit(upb::EnumDefPtr descriptor);
diff --git a/upb_generator/file_layout.h b/upb_generator/file_layout.h index a2aad33..4bf832e 100644 --- a/upb_generator/file_layout.h +++ b/upb_generator/file_layout.h
@@ -9,24 +9,14 @@ #define UPB_GENERATOR_FILE_LAYOUT_H #include <string> - -// begin:google_only -// #ifndef UPB_BOOTSTRAP_STAGE0 -// #include "google/protobuf/descriptor.upb.h" -// #else -// #include "google/protobuf/descriptor.upb.h" -// #endif -// end:google_only - -// begin:github_only -#include "google/protobuf/descriptor.upb.h" -// end:github_only +#include <vector> #include "absl/container/flat_hash_map.h" #include "upb/base/status.hpp" #include "upb/mini_descriptor/decode.h" #include "upb/reflection/def.h" #include "upb/reflection/def.hpp" +#include "upb/reflection/descriptor_bootstrap.h" // Must be last #include "upb/port/def.inc"
diff --git a/upb_generator/plugin.h b/upb_generator/plugin.h index 31ca530..b5edda1 100644 --- a/upb_generator/plugin.h +++ b/upb_generator/plugin.h
@@ -17,26 +17,13 @@ #include <io.h> #endif -// begin:google_only -// #ifndef UPB_BOOTSTRAP_STAGE0 -// #include "google/protobuf/descriptor.upb.h" -// #include "google/protobuf/compiler/plugin.upb.h" -// #else -// #include "google/protobuf/compiler/plugin.upb.h" -// #include "google/protobuf/descriptor.upb.h" -// #endif -// end:google_only - -// begin:github_only -#include "google/protobuf/compiler/plugin.upb.h" -#include "google/protobuf/descriptor.upb.h" -// end:github_only - #include "absl/container/flat_hash_set.h" #include "absl/log/absl_log.h" #include "absl/strings/str_split.h" #include "absl/strings/string_view.h" #include "upb/reflection/def.hpp" +#include "upb/reflection/descriptor_bootstrap.h" +#include "upb_generator/plugin_bootstrap.h" // Must be last. #include "upb/port/def.inc"
diff --git a/upb_generator/plugin_bootstrap.h b/upb_generator/plugin_bootstrap.h new file mode 100644 index 0000000..87afc06 --- /dev/null +++ b/upb_generator/plugin_bootstrap.h
@@ -0,0 +1,19 @@ +#ifndef THIRD_PARTY_UPB_UPB_GENERATOR_PLUGIN_BOOTSTRAP_H_ +#define THIRD_PARTY_UPB_UPB_GENERATOR_PLUGIN_BOOTSTRAP_H_ + +// IWYU pragma: begin_exports + +#if defined(UPB_BOOTSTRAP_STAGE) && UPB_BOOTSTRAP_STAGE == 0 +// This header is checked in. +#include "upb_generator/stage0/google/protobuf/compiler/plugin.upb.h" +#elif UPB_BOOTSTRAP_STAGE == 1 +// This header is generated at build time by the bootstrapping process. +#include "upb_generator/stage1/google/protobuf/compiler/plugin.upb.h" +#else +// This is the normal header, generated by upb_c_proto_library(). +#include "google/protobuf/compiler/plugin.upb.h" +#endif + +// IWYU pragma: end_exports + +#endif // THIRD_PARTY_UPB_UPB_GENERATOR_PLUGIN_BOOTSTRAP_H_
diff --git a/upb_generator/protoc-gen-upb.cc b/upb_generator/protoc-gen-upb.cc index 10b51f0..16ea022 100644 --- a/upb_generator/protoc-gen-upb.cc +++ b/upb_generator/protoc-gen-upb.cc
@@ -6,6 +6,7 @@ // https://developers.google.com/open-source/licenses/bsd #include <algorithm> +#include <cassert> #include <cmath> #include <cstddef> #include <cstdint> @@ -17,10 +18,12 @@ #include <utility> #include <vector> +#include "absl/base/macros.h" #include "absl/log/absl_check.h" #include "absl/log/absl_log.h" #include "absl/strings/escaping.h" #include "absl/strings/match.h" +#include "absl/strings/numbers.h" #include "absl/strings/str_cat.h" #include "absl/strings/str_replace.h" #include "absl/strings/string_view.h" @@ -31,7 +34,6 @@ #include "upb/base/string_view.h" #include "upb/mini_table/field.h" #include "upb/reflection/def.hpp" -#include "upb/wire/types.h" #include "upb_generator/common.h" #include "upb_generator/file_layout.h" #include "upb_generator/names.h" @@ -45,7 +47,7 @@ namespace { struct Options { - bool bootstrap = false; + int bootstrap_stage = -1; // -1 means not bootstrapped. bool strip_nonfunctional_codegen = false; }; @@ -55,7 +57,7 @@ std::string MessageMiniTableRef(upb::MessageDefPtr descriptor, const Options& options) { - if (options.bootstrap) { + if (options.bootstrap_stage == 0) { return absl::StrCat(MessageInitName(descriptor), "()"); } else { return absl::StrCat("&", MessageInitName(descriptor)); @@ -68,7 +70,7 @@ std::string EnumMiniTableRef(upb::EnumDefPtr descriptor, const Options& options) { - if (options.bootstrap) { + if (options.bootstrap_stage == 0) { return absl::StrCat(EnumInitName(descriptor), "()"); } else { return absl::StrCat("&", EnumInitName(descriptor)); @@ -887,21 +889,26 @@ if (i == 0) { output("/* Public Imports. */\n"); } - output("#include \"$0\"\n", CApiHeaderFilename(file.public_dependency(i))); + output("#include \"$0\"\n", + CApiHeaderFilename(file.public_dependency(i), + options.bootstrap_stage >= 0)); } if (file.public_dependency_count() > 0) { output("\n"); } - if (!options.bootstrap) { - output("#include \"$0\"\n\n", MiniTableHeaderFilename(file)); + if (options.bootstrap_stage != 0) { + output("#include \"$0\"\n\n", + MiniTableHeaderFilename(file, options.bootstrap_stage >= 0)); for (int i = 0; i < file.dependency_count(); i++) { if (options.strip_nonfunctional_codegen && google::protobuf::compiler::IsKnownFeatureProto(file.dependency(i).name())) { // Strip feature imports for editions codegen tests. continue; } - output("#include \"$0\"\n", MiniTableHeaderFilename(file.dependency(i))); + output("#include \"$0\"\n", + MiniTableHeaderFilename(file.dependency(i), + options.bootstrap_stage >= 0)); } output("\n"); } @@ -915,7 +922,7 @@ "#endif\n" "\n"); - if (options.bootstrap) { + if (options.bootstrap_stage == 0) { for (auto message : this_file_messages) { output("extern const upb_MiniTable* $0();\n", MessageInitName(message)); } @@ -1005,7 +1012,7 @@ const upb_MiniTableField* field64, const upb_MiniTableField* field32, const Options& options) { - if (options.bootstrap) { + if (options.bootstrap_stage == 0) { ABSL_CHECK(!field.is_extension()); return absl::Substitute( "*upb_MiniTable_FindFieldByNumber($0, $1)", @@ -1034,7 +1041,7 @@ upb::FieldDefPtr field, const Options& options) { std::string ret = FieldInitializer(pools, field, options); - if (!options.bootstrap && field.IsSubMessage()) { + if (options.bootstrap_stage != 0 && field.IsSubMessage()) { ret += ";\n" + StrongReference(field); } return ret; @@ -1109,14 +1116,16 @@ "#include <stddef.h>\n" "#include \"upb/generated_code_support.h\"\n" "#include \"$0\"\n\n", - CApiHeaderFilename(file)); + CApiHeaderFilename(file, options.bootstrap_stage >= 0)); for (int i = 0; i < file.dependency_count(); i++) { if (options.strip_nonfunctional_codegen && google::protobuf::compiler::IsKnownFeatureProto(file.dependency(i).name())) { continue; } - output("#include \"$0\"\n", CApiHeaderFilename(file.dependency(i))); + output( + "#include \"$0\"\n", + CApiHeaderFilename(file.dependency(i), options.bootstrap_stage >= 0)); } output( @@ -1143,9 +1152,9 @@ const Options& options, Plugin* plugin) { Output h_output; WriteHeader(pools, file, options, h_output); - plugin->AddOutputFile(CApiHeaderFilename(file), h_output.output()); + plugin->AddOutputFile(CApiHeaderFilename(file, false), h_output.output()); - if (options.bootstrap) { + if (options.bootstrap_stage == 0) { Output c_output; WriteMiniDescriptorSource(pools, file, options, c_output); plugin->AddOutputFile(SourceFilename(file), c_output.output()); @@ -1161,8 +1170,11 @@ bool ParseOptions(Plugin* plugin, Options* options) { for (const auto& pair : ParseGeneratorParameter(plugin->parameter())) { - if (pair.first == "bootstrap_upb") { - options->bootstrap = true; + if (pair.first == "bootstrap_stage") { + if (!absl::SimpleAtoi(pair.second, &options->bootstrap_stage)) { + plugin->SetError(absl::Substitute("Bad stage: $0", pair.second)); + return false; + } } else if (pair.first == "experimental_strip_nonfunctional_codegen") { options->strip_nonfunctional_codegen = true; } else {
diff --git a/upb_generator/protoc-gen-upb_minitable-main.cc b/upb_generator/protoc-gen-upb_minitable-main.cc index 2073f07..f970235 100644 --- a/upb_generator/protoc-gen-upb_minitable-main.cc +++ b/upb_generator/protoc-gen-upb_minitable-main.cc
@@ -35,8 +35,9 @@ void GenerateFile(const DefPoolPair& pools, upb::FileDefPtr file, const MiniTableOptions& options, Plugin* plugin) { Output h_output; - WriteMiniTableHeader(pools, file, h_output); - plugin->AddOutputFile(MiniTableHeaderFilename(file), h_output.output()); + WriteMiniTableHeader(pools, file, options, h_output); + plugin->AddOutputFile(MiniTableHeaderFilename(file, false), + h_output.output()); Output c_output; WriteMiniTableSource(pools, file, options, c_output); @@ -49,7 +50,9 @@ bool ParseOptions(MiniTableOptions* options, Plugin* plugin) { for (const auto& pair : ParseGeneratorParameter(plugin->parameter())) { - if (pair.first == "experimental_strip_nonfunctional_codegen") { + if (pair.first == "bootstrap_stage") { + options->bootstrap = true; + } else if (pair.first == "experimental_strip_nonfunctional_codegen") { options->strip_nonfunctional_codegen = true; } else if (pair.first == "one_output_per_message") { options->one_output_per_message = true;
diff --git a/upb_generator/protoc-gen-upb_minitable.cc b/upb_generator/protoc-gen-upb_minitable.cc index 333ab2e..a12f0cd 100644 --- a/upb_generator/protoc-gen-upb_minitable.cc +++ b/upb_generator/protoc-gen-upb_minitable.cc
@@ -488,7 +488,7 @@ } // namespace void WriteMiniTableHeader(const DefPoolPair& pools, upb::FileDefPtr file, - Output& output) { + const MiniTableOptions& options, Output& output) { EmitFileWarning(file.name(), output); output( "#ifndef $0_UPB_MINITABLE_H_\n" @@ -500,8 +500,9 @@ if (i == 0) { output("/* Public Imports. */\n"); } - output("#include \"$0\"\n", - MiniTableHeaderFilename(file.public_dependency(i))); + output( + "#include \"$0\"\n", + MiniTableHeaderFilename(file.public_dependency(i), options.bootstrap)); if (i == file.public_dependency_count() - 1) { output("\n"); } @@ -560,7 +561,7 @@ "#include <stddef.h>\n" "#include \"upb/generated_code_support.h\"\n" "#include \"$0\"\n", - MiniTableHeaderFilename(file)); + MiniTableHeaderFilename(file, options.bootstrap)); for (int i = 0; i < file.dependency_count(); i++) { if (options.strip_nonfunctional_codegen && @@ -568,7 +569,8 @@ // Strip feature imports for editions codegen tests. continue; } - output("#include \"$0\"\n", MiniTableHeaderFilename(file.dependency(i))); + output("#include \"$0\"\n", + MiniTableHeaderFilename(file.dependency(i), options.bootstrap)); } output(
diff --git a/upb_generator/protoc-gen-upb_minitable.h b/upb_generator/protoc-gen-upb_minitable.h index 5023771..358a157 100644 --- a/upb_generator/protoc-gen-upb_minitable.h +++ b/upb_generator/protoc-gen-upb_minitable.h
@@ -25,6 +25,7 @@ namespace generator { struct MiniTableOptions { + bool bootstrap = false; bool one_output_per_message = false; bool strip_nonfunctional_codegen = false; }; @@ -36,7 +37,7 @@ const MiniTableOptions& options, Plugin* plugin); void WriteMiniTableHeader(const DefPoolPair& pools, upb::FileDefPtr file, - Output& output); + const MiniTableOptions& options, Output& output); } // namespace generator } // namespace upb
diff --git a/upb_generator/protoc-gen-upbdefs.cc b/upb_generator/protoc-gen-upbdefs.cc index ac1b3e4..e2742a5 100644 --- a/upb_generator/protoc-gen-upbdefs.cc +++ b/upb_generator/protoc-gen-upbdefs.cc
@@ -84,7 +84,7 @@ output("#include \"upb/reflection/def.h\"\n"); output("#include \"$0\"\n", DefHeaderFilename(file)); - output("#include \"$0\"\n", MiniTableHeaderFilename(file)); + output("#include \"$0\"\n", MiniTableHeaderFilename(file, false)); output("\n"); for (int i = 0; i < file.dependency_count(); i++) {
diff --git a/upb_generator/stage0/google/protobuf/compiler/plugin.upb.c b/upb_generator/stage0/google/protobuf/compiler/plugin.upb.c index c1de6bd..ae9abc8 100644 --- a/upb_generator/stage0/google/protobuf/compiler/plugin.upb.c +++ b/upb_generator/stage0/google/protobuf/compiler/plugin.upb.c
@@ -1,8 +1,8 @@ #include <stddef.h> #include "upb/generated_code_support.h" -#include "google/protobuf/compiler/plugin.upb.h" +#include "upb_generator/plugin_bootstrap.h" -#include "google/protobuf/descriptor.upb.h" +#include "upb/reflection/descriptor_bootstrap.h" static upb_Arena* upb_BootstrapArena() { static upb_Arena* arena = NULL; if (!arena) arena = upb_Arena_New();