From 3ecb7a4163f637fb23d0dd3af70c4b8db210defd Mon Sep 17 00:00:00 2001 From: Protobuf Team Bot Date: Mon, 3 Jul 2023 01:27:18 -0700 Subject: [PATCH] Make `upb_proto_aspect` reexport all `deps` when `srcs` is empty. This special `proto_library` behavior is known as an *alias library*: https://bazel.build/reference/be/protocol-buffer#proto_library_args Without this CL, the compilation of the added test fails with: ``` test_import_empty_srcs.upb.c:12:10: error: module :test_import_empty_srcs_proto.upb does not depend on a module exporting 'test.upb.h' ``` PiperOrigin-RevId: 545153380 --- bazel/upb_proto_library.bzl | 91 +++++++++++++++------------ upb/test/BUILD | 29 +++++++++ upb/test/test_import_empty_srcs.cc | 34 ++++++++++ upb/test/test_import_empty_srcs.proto | 9 +++ 4 files changed, 124 insertions(+), 39 deletions(-) create mode 100644 upb/test/test_import_empty_srcs.cc create mode 100644 upb/test/test_import_empty_srcs.proto diff --git a/bazel/upb_proto_library.bzl b/bazel/upb_proto_library.bzl index 459dc5238c..8d4f4b6ef6 100644 --- a/bazel/upb_proto_library.bzl +++ b/bazel/upb_proto_library.bzl @@ -291,52 +291,65 @@ def _upb_proto_rule_impl(ctx): ] def _upb_proto_aspect_impl(target, ctx, generator, cc_provider, file_provider, provide_cc_shared_library_hints = True): - proto_info = target[ProtoInfo] - files = _compile_upb_protos(ctx, generator, proto_info, proto_info.direct_sources) - deps = ctx.rule.attr.deps + getattr(ctx.attr, "_" + generator) - dep_ccinfos = [dep[CcInfo] for dep in deps if CcInfo in dep] - dep_ccinfos += [dep[UpbWrappedCcInfo].cc_info for dep in deps if UpbWrappedCcInfo in dep] - dep_ccinfos += [dep[_UpbDefsWrappedCcInfo].cc_info for dep in deps if _UpbDefsWrappedCcInfo in dep] - if generator == "upbdefs": - if UpbWrappedCcInfo not in target: - fail("Target should have UpbWrappedCcInfo provider") - dep_ccinfos.append(target[UpbWrappedCcInfo].cc_info) name = ctx.rule.attr.name + "." + generator owners = [ctx.label.relative(name)] - cc_info = _cc_library_func( - ctx = ctx, - name = name, - hdrs = files.hdrs, - srcs = files.srcs, - includes = files.includes, - copts = ctx.attr._copts[UpbProtoLibraryCoptsInfo].copts, - dep_ccinfos = dep_ccinfos, - ) - if files.thunks: - name_thunks = ctx.rule.attr.name + "." + generator + ".thunks" - owners.append(ctx.label.relative(name_thunks)) - cc_info_with_thunks = _cc_library_func( + providers = [] + if not getattr(ctx.rule.attr, "srcs", []): + # This target doesn't declare any sources, reexport all its deps instead. + deps = ctx.rule.attr.deps + providers += [cc_provider(cc_info = dep[CcInfo]) for dep in deps if CcInfo in dep] + providers += [dep[UpbWrappedCcInfo] for dep in deps if UpbWrappedCcInfo in dep] + providers += [dep[_UpbDefsWrappedCcInfo] for dep in deps if _UpbDefsWrappedCcInfo in dep] + providers += [dep[_UpbWrappedGeneratedSrcsInfo] for dep in deps if _UpbWrappedGeneratedSrcsInfo in dep] + providers += [dep[_WrappedDefsGeneratedSrcsInfo] for dep in deps if _WrappedDefsGeneratedSrcsInfo in dep] + else: + proto_info = target[ProtoInfo] + files = _compile_upb_protos(ctx, generator, proto_info, proto_info.direct_sources) + deps = ctx.rule.attr.deps + getattr(ctx.attr, "_" + generator) + dep_ccinfos = [dep[CcInfo] for dep in deps if CcInfo in dep] + dep_ccinfos += [dep[UpbWrappedCcInfo].cc_info for dep in deps if UpbWrappedCcInfo in dep] + dep_ccinfos += [dep[_UpbDefsWrappedCcInfo].cc_info for dep in deps if _UpbDefsWrappedCcInfo in dep] + if generator == "upbdefs": + if UpbWrappedCcInfo not in target: + fail("Target should have UpbWrappedCcInfo provider") + dep_ccinfos.append(target[UpbWrappedCcInfo].cc_info) + + cc_info = _cc_library_func( ctx = ctx, - name = name_thunks, - hdrs = [], - srcs = files.thunks, + name = name, + hdrs = files.hdrs, + srcs = files.srcs, includes = files.includes, copts = ctx.attr._copts[UpbProtoLibraryCoptsInfo].copts, - dep_ccinfos = dep_ccinfos + [cc_info], - ) - wrapped_cc_info = cc_provider( - cc_info = cc_info, - cc_info_with_thunks = cc_info_with_thunks, + dep_ccinfos = dep_ccinfos, ) - else: - wrapped_cc_info = cc_provider( - cc_info = cc_info, - ) - providers = [ - wrapped_cc_info, - file_provider(srcs = files), - ] + + if files.thunks: + name_thunks = ctx.rule.attr.name + "." + generator + ".thunks" + owners.append(ctx.label.relative(name_thunks)) + cc_info_with_thunks = _cc_library_func( + ctx = ctx, + name = name_thunks, + hdrs = [], + srcs = files.thunks, + includes = files.includes, + copts = ctx.attr._copts[UpbProtoLibraryCoptsInfo].copts, + dep_ccinfos = dep_ccinfos + [cc_info], + ) + wrapped_cc_info = cc_provider( + cc_info = cc_info, + cc_info_with_thunks = cc_info_with_thunks, + ) + else: + wrapped_cc_info = cc_provider( + cc_info = cc_info, + ) + providers += [ + wrapped_cc_info, + file_provider(srcs = files), + ] + if provide_cc_shared_library_hints: if hasattr(cc_common, "CcSharedLibraryHintInfo"): providers.append(cc_common.CcSharedLibraryHintInfo(owners = owners)) diff --git a/upb/test/BUILD b/upb/test/BUILD index 64541133af..e8a0fb49a8 100644 --- a/upb/test/BUILD +++ b/upb/test/BUILD @@ -209,3 +209,32 @@ cc_test( "@com_google_googletest//:gtest_main", ], ) + +proto_library( + name = "empty_srcs_proto", + testonly = 1, + srcs = [], + deps = [":test_proto"], +) + +proto_library( + name = "test_import_empty_srcs_proto", + testonly = 1, + srcs = ["test_import_empty_srcs.proto"], + deps = [":empty_srcs_proto"], +) + +upb_proto_library( + name = "test_import_empty_srcs_upb_proto", + testonly = 1, + deps = [":test_import_empty_srcs_proto"], +) + +cc_test( + name = "test_import_empty_srcs", + srcs = ["test_import_empty_srcs.cc"], + deps = [ + ":test_import_empty_srcs_upb_proto", + "@com_google_googletest//:gtest_main", + ], +) diff --git a/upb/test/test_import_empty_srcs.cc b/upb/test/test_import_empty_srcs.cc new file mode 100644 index 0000000000..60190b48a2 --- /dev/null +++ b/upb/test/test_import_empty_srcs.cc @@ -0,0 +1,34 @@ +/* + * Copyright (c) 2009-2021, Google LLC + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of Google LLC nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL Google LLC BE LIABLE FOR ANY DIRECT, + * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +#include "gtest/gtest.h" +#include "upb/test/test_import_empty_srcs.upb.h" + +TEST(Test, Reexport) { + // This test really just ensures that compilation succeeds. + ASSERT_GT(sizeof(upb_test_ContainsImported_msg_init), 0); +} diff --git a/upb/test/test_import_empty_srcs.proto b/upb/test/test_import_empty_srcs.proto new file mode 100644 index 0000000000..42a8874dcb --- /dev/null +++ b/upb/test/test_import_empty_srcs.proto @@ -0,0 +1,9 @@ +syntax = "proto2"; + +package upb_test; + +import "upb/test/test.proto"; + +message ContainsImported { + optional MessageName message_name = 1; +}