From 3f493d9e52779c7df60dc37f994932982ffbaa05 Mon Sep 17 00:00:00 2001 From: Alyssa Haroldsen Date: Fri, 12 Apr 2024 16:47:49 -0700 Subject: [PATCH] Use the same set of exports as regular users in shared tests The shared tests which access `protobuf_upb` or `protobuf_cpp` have access to more items than the `protobuf` library itself. This is because the former don't go through the same re-exporting based on kernel. I fix this by creating two test-only libraries that perform the same re-exporting as the `protobuf` library, but with the kernel explicitly set, and changing the shared tests to reference that instead of the inner runtime library. This is needed to reliably test macros, where item paths are relative to the invocation, not eagerly checked at the macro source. PiperOrigin-RevId: 624328817 --- rust/BUILD | 32 ++++++++++++++++++++--- rust/protobuf.rs | 6 +++-- rust/test/cpp/BUILD | 3 ++- rust/test/shared/BUILD | 58 ++++++++++++++++++++++-------------------- rust/test/upb/BUILD | 3 ++- 5 files changed, 67 insertions(+), 35 deletions(-) diff --git a/rust/BUILD b/rust/BUILD index 9bc42f3294..89231cf44f 100644 --- a/rust/BUILD +++ b/rust/BUILD @@ -40,9 +40,9 @@ rust_library( # This list contains kernel-agnostic logic and simple kernel-specific logic controlled by # `#[cfg(...)]` attributes. That forces us to compile these files twice, once for each kernel. As a -# result these files are included in both `:protobuf_upb` and `:protobuf_cpp`. This is in principle -# identical to how we compile regular Rust source files twice (once for production, and once for -# unittesting). +# result these files are included in both `:protobuf_upb` and `:protobuf_cpp`. +# This is in principle identical to how we compile regular Rust source files twice +# (once for production, and once for unit testing). # # shared.rs is the root of the crate and has public items re-exported in protobuf.rs for user use. PROTOBUF_SHARED = [ @@ -93,6 +93,19 @@ rust_test( ], ) +# This provides an identical set of re-exports as `:protobuf` with `:use_upb_kernel` active. +# This is only used for tests shared between runtimes. +rust_library( + name = "protobuf_upb_export", + testonly = True, + srcs = ["protobuf.rs"], + rustc_flags = ["--cfg=upb_kernel"], + visibility = [ + "//src/google/protobuf:__subpackages__", + ], + deps = [":protobuf_upb"], +) + # The Rust Protobuf runtime using the cpp kernel. # # `rust_cpp_proto_library` implicitly depends on this target. This target cannot depend on @@ -123,6 +136,19 @@ rust_test( ], ) +# This provides an identical set of re-exports as `:protobuf` with `:use_upb_kernel` inactive. +# This is only used for tests shared between runtimes. +rust_library( + name = "protobuf_cpp_export", + testonly = True, + srcs = ["protobuf.rs"], + rustc_flags = ["--cfg=cpp_kernel"], + visibility = [ + "//src/google/protobuf:__subpackages__", + ], + deps = [":protobuf_cpp"], +) + rust_library( name = "utf8", srcs = ["utf8.rs"], diff --git a/rust/protobuf.rs b/rust/protobuf.rs index 9e22dd865b..c83b94953a 100644 --- a/rust/protobuf.rs +++ b/rust/protobuf.rs @@ -9,8 +9,10 @@ //! //! This file forwards to `shared.rs`, which exports a kernel-specific //! `__runtime`. Rust Protobuf gencode actually depends directly on kernel -//! specific crates. The only reason this crate exists is to be able to use -//! `protobuf` as a crate name for both cpp and upb kernels from user code. +//! specific crates. This crate exists for two reasons: +//! - To be able to use `protobuf` as a crate name for both cpp and upb kernels +//! from user code. +//! - To make it more difficult to access internal-only items by default. #[cfg(cpp_kernel)] use protobuf_cpp as kernel; diff --git a/rust/test/cpp/BUILD b/rust/test/cpp/BUILD index feaed8d728..0e141c7113 100644 --- a/rust/test/cpp/BUILD +++ b/rust/test/cpp/BUILD @@ -8,7 +8,8 @@ # # To do that use: # * `rust_cc_proto_library` instead of `rust_proto_library`. -# * `//rust:protobuf_cpp` instead of `//rust:protobuf``. +# * `//rust:protobuf_cpp_export` instead of +# `//rust:protobuf`. load("@rules_rust//rust:defs.bzl", "rust_test") load( diff --git a/rust/test/shared/BUILD b/rust/test/shared/BUILD index 055c3168ac..0daf000c93 100644 --- a/rust/test/shared/BUILD +++ b/rust/test/shared/BUILD @@ -7,9 +7,9 @@ # To do that: # * Declare 2 rust_test targets and share the same sources from both. # * First copy will only depend on `rust_cc_proto_library` Rust proto targets, and if needed will -# only depend on `//rust:protobuf_cpp. +# only depend on `//rust:protobuf_cpp_export`. # * Second copy will only depend on `rust_upb_proto_library` Rust proto targets, and if needed will -# only depend on `//rust:protobuf_upb +# only depend on `//rust:protobuf_upb_export`. # # Once we have a couple of these tests we will investigate ways to remove boilerplate (for example # by introducing a build macro that registers 2 rust_test targets). @@ -18,24 +18,26 @@ load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") rust_library( name = "matchers_upb", + testonly = True, srcs = ["matchers.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", }, deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "@crate_index//:googletest", ], ) rust_library( name = "matchers_cpp", + testonly = True, srcs = ["matchers.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", }, deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "@crate_index//:googletest", ], ) @@ -82,10 +84,10 @@ rust_test( name = "enum_cpp_test", srcs = ["enum_test.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", }, deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:enums_cc_rust_proto", "//rust/test:unittest_cc_rust_proto", "@crate_index//:googletest", @@ -96,10 +98,10 @@ rust_test( name = "enum_upb_test", srcs = ["enum_test.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", }, deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:enums_upb_rust_proto", "//rust/test:unittest_upb_rust_proto", "@crate_index//:googletest", @@ -190,14 +192,14 @@ rust_test( name = "accessors_cpp_test", srcs = ["accessors_test.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", "//rust/test/shared:matchers_cpp": "matchers", }, proc_macro_deps = [ "@crate_index//:paste", ], deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:unittest_cc_rust_proto", "//rust/test/shared:matchers_cpp", "@crate_index//:googletest", @@ -208,14 +210,14 @@ rust_test( name = "accessors_upb_test", srcs = ["accessors_test.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", "//rust/test/shared:matchers_upb": "matchers", }, proc_macro_deps = [ "@crate_index//:paste", ], deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:unittest_upb_rust_proto", "//rust/test/shared:matchers_upb", "@crate_index//:googletest", @@ -226,11 +228,11 @@ rust_test( name = "accessors_proto3_cpp_test", srcs = ["accessors_proto3_test.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", "//rust/test/shared:matchers_cpp": "matchers", }, deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:unittest_proto3_cc_rust_proto", "//rust/test:unittest_proto3_optional_cc_rust_proto", "//rust/test/shared:matchers_cpp", @@ -242,11 +244,11 @@ rust_test( name = "accessors_proto3_upb_test", srcs = ["accessors_proto3_test.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", "//rust/test/shared:matchers_upb": "matchers", }, deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:unittest_proto3_optional_upb_rust_proto", "//rust/test:unittest_proto3_upb_rust_proto", "//rust/test/shared:matchers_upb", @@ -276,10 +278,10 @@ rust_test( name = "simple_nested_cpp_test", srcs = ["simple_nested_test.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", }, deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:nested_cc_rust_proto", "@crate_index//:googletest", ], @@ -289,10 +291,10 @@ rust_test( name = "simple_nested_upb_test", srcs = ["simple_nested_test.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", }, deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:nested_upb_rust_proto", "@crate_index//:googletest", ], @@ -302,13 +304,13 @@ rust_test( name = "accessors_repeated_cpp_test", srcs = ["accessors_repeated_test.rs"], aliases = { - "//rust:protobuf_cpp": "protobuf", + "//rust:protobuf_cpp_export": "protobuf", }, proc_macro_deps = [ "@crate_index//:paste", ], deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:unittest_cc_rust_proto", "@crate_index//:googletest", ], @@ -318,13 +320,13 @@ rust_test( name = "accessors_repeated_upb_test", srcs = ["accessors_repeated_test.rs"], aliases = { - "//rust:protobuf_upb": "protobuf", + "//rust:protobuf_upb_export": "protobuf", }, proc_macro_deps = [ "@crate_index//:paste", ], deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:unittest_upb_rust_proto", "@crate_index//:googletest", ], @@ -378,7 +380,7 @@ rust_test( name = "fields_with_imported_types_cpp_test", srcs = ["fields_with_imported_types_test.rs"], deps = [ - "//rust:protobuf_cpp", + "//rust:protobuf_cpp_export", "//rust/test:fields_with_imported_types_cc_rust_proto", "//rust/test:imported_types_cc_rust_proto", "@crate_index//:googletest", @@ -389,7 +391,7 @@ rust_test( name = "fields_with_imported_types_upb_test", srcs = ["fields_with_imported_types_test.rs"], deps = [ - "//rust:protobuf_upb", + "//rust:protobuf_upb_export", "//rust/test:fields_with_imported_types_upb_rust_proto", "//rust/test:imported_types_upb_rust_proto", "@crate_index//:googletest", diff --git a/rust/test/upb/BUILD b/rust/test/upb/BUILD index 33f296217a..959e736cc5 100644 --- a/rust/test/upb/BUILD +++ b/rust/test/upb/BUILD @@ -8,4 +8,5 @@ # # To do that use: # * `rust_upb_proto_library` instead of `rust_proto_library`. -# * `//rust:protobuf_upb` instead of `//rust:protobuf``. +# * `//rust:protobuf_upb_export` instead of +# `//rust:protobuf`.