Skip to content

Commit

Permalink
Remove ODR violation from WKT codegen (#12406) (#12419)
Browse files Browse the repository at this point in the history
* Remove ODR violation from WKT codegen (#12406)

Closes #12406

COPYBARA_INTEGRATE_REVIEW=#12406 from mkruskal-google:wkt 1c6748e
PiperOrigin-RevId: 522418175

* Removed legacy target.

---------

Co-authored-by: Mike Kruskal <[email protected]>
  • Loading branch information
haberman and mkruskal-google authored Apr 6, 2023
1 parent f1c7820 commit 5ddb1fc
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 44 deletions.
2 changes: 1 addition & 1 deletion BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ proto_lang_toolchain(
"//:descriptor_proto",
],
command_line = "--cpp_out=$(OUT)",
runtime = ":protobuf",
runtime = "//src/google/protobuf:protobuf_nowkt",
visibility = ["//visibility:public"],
)

Expand Down
3 changes: 3 additions & 0 deletions conformance/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ cc_library(
includes = ["."],
deps = [
":conformance_cc_proto",
"//src/google/protobuf/util:differencer",
"//src/google/protobuf/util:json_util",
"//src/google/protobuf/util:type_resolver_util",
"@com_google_absl//absl/strings",
"@com_google_absl//absl/strings:str_format",
],
Expand Down
10 changes: 8 additions & 2 deletions examples/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,19 @@ cc_proto_library(
cc_binary(
name = "add_person_cpp",
srcs = ["add_person.cc"],
deps = [":addressbook_cc_proto"],
deps = [
":addressbook_cc_proto",
"@com_google_protobuf//:protobuf",
],
)

cc_binary(
name = "list_people_cpp",
srcs = ["list_people.cc"],
deps = [":addressbook_cc_proto"],
deps = [
":addressbook_cc_proto",
"@com_google_protobuf//:protobuf",
],
)

# Similar to cc_proto_library but for Java.
Expand Down
4 changes: 2 additions & 2 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ cc_dist_library(
deps = [
"//src/google/protobuf:arena_align",
"//src/google/protobuf:protobuf_nowkt",
"//src/google/protobuf:wkt_cc_proto",
"//src/google/protobuf:cmake_wkt_cc_proto",
"//src/google/protobuf/compiler:importer",
"//src/google/protobuf/json",
"//src/google/protobuf/util:delimited_message_util",
Expand Down Expand Up @@ -416,7 +416,7 @@ cc_dist_library(
testonly = 1,
tags = ["manual"],
deps = ["//src/google/protobuf:lite_test_util"],
dist_deps = [":protobuf_lite"],
dist_deps = [":protobuf"],
)

cc_dist_library(
Expand Down
33 changes: 30 additions & 3 deletions src/google/protobuf/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ WELL_KNOWN_TYPES = [
"wrappers",
]

proto_library(
name = "wkt_proto",
visibility = ["//visibility:private"],
deps = [wkt + "_proto" for wkt in WELL_KNOWN_TYPES],
)

cc_proto_library(
name = "wkt_cc_proto",
visibility = ["//pkg:__pkg__"],
deps = ["wkt_proto"],
)

# When we generate code for the well-known types, we put the resulting files in
# wkt/google/protobuf and add ./wkt to the include paths below. This is a
# somewhat strange setup but is necessary to satisfy these two constraints:
Expand All @@ -121,12 +133,12 @@ genrule(
["wkt/google/protobuf/" + wkt + ".pb.h" for wkt in WELL_KNOWN_TYPES] +
["wkt/google/protobuf/" + wkt + ".pb.cc" for wkt in WELL_KNOWN_TYPES],
cmd = """
$(execpath //src/google/protobuf/compiler:protoc_nowkt) \
$(execpath //:protoc) \
--cpp_out=dllexport_decl=PROTOBUF_EXPORT:$(RULEDIR)/wkt \
--proto_path=$$(dirname $$(dirname $$(dirname $(location any.proto)))) \
$(SRCS)
""",
exec_tools = ["//src/google/protobuf/compiler:protoc_nowkt"],
exec_tools = ["//:protoc"],
visibility = ["//visibility:private"],
)

Expand All @@ -139,8 +151,11 @@ staleness_test(
tags = ["manual"],
)

# This is necessary for our generated cmake configs to pick up the checked in
# WKT files.
# TODO(b/246826624) Remove this once we generate WKT code from cmake.
cc_library(
name = "wkt_cc_proto",
name = "cmake_wkt_cc_proto",
srcs = ["wkt/google/protobuf/" + wkt + ".pb.cc" for wkt in WELL_KNOWN_TYPES],
hdrs = ["wkt/google/protobuf/" + wkt + ".pb.h" for wkt in WELL_KNOWN_TYPES],
copts = COPTS,
Expand Down Expand Up @@ -425,6 +440,7 @@ cc_library(
include_prefix = "google/protobuf",
linkopts = LINK_OPTS,
visibility = [
"//:__pkg__",
"//pkg:__pkg__",
"//src/google/protobuf:__subpackages__",
],
Expand Down Expand Up @@ -784,6 +800,7 @@ cc_library(
visibility = ["//:__subpackages__"],
deps = [
"//src/google/protobuf/io",
"//src/google/protobuf/util:differencer",
"@com_google_googletest//:gtest",
],
)
Expand Down Expand Up @@ -935,9 +952,11 @@ cc_test(
":cc_test_protos",
":protobuf",
":test_util",
":test_util2",
"//src/google/protobuf/io",
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
"//src/google/protobuf/util:differencer",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
],
Expand Down Expand Up @@ -1057,6 +1076,9 @@ cc_test(
":cc_test_protos",
":protobuf",
":test_util",
":test_util2",
"//src/google/protobuf/util:differencer",
"//src/google/protobuf/util:time_util",
"@com_google_absl//absl/container:flat_hash_map",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_googletest//:gtest",
Expand All @@ -1078,6 +1100,7 @@ cc_test(
"//src/google/protobuf/io",
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
"//src/google/protobuf/util:differencer",
"@com_google_absl//absl/log:scoped_mock_log",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
Expand Down Expand Up @@ -1235,6 +1258,7 @@ cc_test(
"//src/google/protobuf/io",
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
"@com_google_absl//absl/log:die_if_null",
"@com_google_absl//absl/log:scoped_mock_log",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
Expand Down Expand Up @@ -1296,6 +1320,7 @@ cc_test(
"//src/google/protobuf/io",
"//src/google/protobuf/stubs",
"//src/google/protobuf/testing",
"//src/google/protobuf/util:differencer",
"@com_google_absl//absl/log:scoped_mock_log",
"@com_google_googletest//:gtest",
"@com_google_googletest//:gtest_main",
Expand All @@ -1308,7 +1333,9 @@ cc_test(
deps = [
":cc_test_protos",
":protobuf",
"//src/google/protobuf/compiler:importer",
"//src/google/protobuf/compiler:retention",
"//src/google/protobuf/util:differencer",
"@com_google_googletest//:gtest_main",
],
)
Expand Down
29 changes: 5 additions & 24 deletions src/google/protobuf/compiler/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,15 @@ cc_library(
)

cc_library(
name = "protoc_lib_nowkt",
name = "protoc_lib",
srcs = [
"main.cc",
],
copts = COPTS,
visibility = [
"//:__pkg__",
"//pkg:__pkg__",
],
deps = [
":code_generator",
":command_line_interface",
Expand All @@ -120,29 +124,6 @@ cc_library(
],
)

cc_binary(
name = "protoc_nowkt",
copts = COPTS,
linkopts = LINK_OPTS,
visibility = [
"//src/google/protobuf:__pkg__",
],
deps = [":protoc_lib_nowkt"],
)

cc_library(
name = "protoc_lib",
copts = COPTS,
visibility = [
"//:__pkg__",
"//pkg:__pkg__",
],
deps = [
":protoc_lib_nowkt",
"//:protobuf",
],
)

# Note: this is an alias for now. In the future, this rule will become the
# cc_binary for protoc, and //:protoc will become an alias.
alias(
Expand Down
9 changes: 0 additions & 9 deletions src/google/protobuf/util/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,6 @@ proto_library(
testonly = 1,
srcs = ["json_format.proto"],
strip_import_prefix = "/src",
deps = [
"//:any_proto",
"//:duration_proto",
"//:field_mask_proto",
"//:struct_proto",
"//:test_protos",
"//:timestamp_proto",
"//:wrappers_proto",
],
)

cc_proto_library(
Expand Down
3 changes: 0 additions & 3 deletions src/google/protobuf/util/json_format.proto
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,6 @@ syntax = "proto2";

package protobuf_unittest;

import "google/protobuf/any.proto";
import "google/protobuf/struct.proto";

message TestFlagsAndStrings {
required int32 A = 1;
repeated group RepeatedGroup = 2 {
Expand Down

0 comments on commit 5ddb1fc

Please sign in to comment.