diff --git a/CMakeLists.txt b/CMakeLists.txt index 646e69ca4d0..22ac9eaebbd 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -5870,7 +5870,7 @@ target_include_directories(arena_test target_link_libraries(arena_test ${_gRPC_PROTOBUF_LIBRARIES} ${_gRPC_ALLTARGETS_LIBRARIES} - grpc_test_util + grpc ) @@ -8237,6 +8237,7 @@ target_include_directories(common_closures_test target_link_libraries(common_closures_test ${_gRPC_PROTOBUF_LIBRARIES} ${_gRPC_ALLTARGETS_LIBRARIES} + grpc grpc_test_util_unsecure ) diff --git a/build_autogenerated.yaml b/build_autogenerated.yaml index 6593185c73e..fde78011a28 100644 --- a/build_autogenerated.yaml +++ b/build_autogenerated.yaml @@ -4138,7 +4138,7 @@ targets: src: - test/core/resource_quota/arena_test.cc deps: - - grpc_test_util + - grpc uses_polling: false - name: async_end2end_test gtest: true @@ -5232,6 +5232,7 @@ targets: src: - test/core/event_engine/common_closures_test.cc deps: + - grpc - grpc_test_util_unsecure - name: completion_queue_threading_test gtest: true diff --git a/test/core/end2end/BUILD b/test/core/end2end/BUILD index 1690d9f7674..e25e8e5e6ce 100644 --- a/test/core/end2end/BUILD +++ b/test/core/end2end/BUILD @@ -34,7 +34,6 @@ grpc_cc_library( "//:debug_location", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:match", "//:time", "//test/core/util:grpc_test_util", @@ -68,7 +67,6 @@ grpc_cc_library( "//:config", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:httpcli", "//:iomgr_fwd", "//:resolved_address", @@ -87,7 +85,6 @@ grpc_cc_library( deps = [ "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//test/core/util:grpc_test_util", @@ -117,7 +114,6 @@ grpc_cc_test( "cq_verifier", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//test/core/util:grpc_test_util", @@ -132,7 +128,6 @@ grpc_cc_test( "cq_verifier", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//test/core/util:grpc_test_util", @@ -153,7 +148,6 @@ grpc_cc_test( "cq_verifier", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//:iomgr_port", @@ -179,7 +173,6 @@ grpc_cc_test( "//:default_event_engine_factory_hdrs", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//:grpc_resolver_dns_ares", @@ -271,7 +264,6 @@ grpc_cc_test( "cq_verifier", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_codegen", "//:grpc_public_hdrs", "//:tsi_ssl_credentials", diff --git a/test/core/event_engine/BUILD b/test/core/event_engine/BUILD index 4a4bb201569..72bea5d5230 100644 --- a/test/core/event_engine/BUILD +++ b/test/core/event_engine/BUILD @@ -28,7 +28,7 @@ grpc_cc_test( deps = [ "//:common_event_engine_closures", "//:gpr_platform", - "//:grpc_base", + "//:grpc", "//test/core/util:grpc_test_util_unsecure", ], ) @@ -42,7 +42,6 @@ grpc_cc_test( deps = [ "//:gpr_platform", "//:grpc", - "//:grpc_base", "//test/core/util:grpc_test_util_unsecure", ], ) @@ -78,7 +77,7 @@ grpc_cc_library( hdrs = ["test_init.h"], external_deps = ["absl/strings"], deps = [ + "//:event_engine_base_hdrs", "//:gpr_base", - "//:grpc_base", ], ) diff --git a/test/core/resource_quota/BUILD b/test/core/resource_quota/BUILD index 593b4d2c56f..c03bffc7c92 100644 --- a/test/core/resource_quota/BUILD +++ b/test/core/resource_quota/BUILD @@ -23,14 +23,19 @@ grpc_cc_test( name = "arena_test", srcs = ["arena_test.cc"], external_deps = [ + "absl/memory", + "absl/strings", "gtest", ], language = "C++", uses_event_engine = False, uses_polling = False, deps = [ - "//:gpr", - "//test/core/util:grpc_test_util", + "//:arena", + "//:gpr_base", + "//:grpc", + "//:ref_counted_ptr", + "//:resource_quota", ], ) @@ -39,21 +44,21 @@ grpc_cc_library( testonly = True, hdrs = ["call_checker.h"], language = "c++", - deps = ["//:gpr"], + deps = ["//:gpr_base"], ) grpc_cc_test( name = "memory_quota_test", srcs = ["memory_quota_test.cc"], - external_deps = [ - "gtest", - ], + external_deps = ["gtest"], language = "c++", uses_event_engine = False, uses_polling = False, deps = [ "call_checker", + "//:exec_ctx", "//:memory_quota", + "//:slice_refcount", ], ) @@ -61,12 +66,15 @@ grpc_cc_test( name = "periodic_update_test", srcs = ["periodic_update_test.cc"], external_deps = [ + "absl/memory", "gtest", ], language = "c++", uses_event_engine = False, uses_polling = False, deps = [ + "//:exec_ctx", + "//:gpr_base", "//:periodic_update", ], ) @@ -74,35 +82,32 @@ grpc_cc_test( grpc_cc_test( name = "thread_quota_test", srcs = ["thread_quota_test.cc"], - external_deps = [ - "gtest", - ], + external_deps = ["gtest"], language = "c++", uses_event_engine = False, uses_polling = False, - deps = [ - "//:thread_quota", - ], + deps = ["//:thread_quota"], ) grpc_cc_test( name = "resource_quota_test", srcs = ["resource_quota_test.cc"], - external_deps = [ - "gtest", - ], + external_deps = ["gtest"], language = "c++", uses_event_engine = False, uses_polling = False, - deps = [ - "//:resource_quota", - ], + deps = ["//:resource_quota"], ) grpc_cc_test( name = "memory_quota_stress_test", srcs = ["memory_quota_stress_test.cc"], - external_deps = ["gtest"], + external_deps = [ + "absl/base:core_headers", + "absl/strings", + "absl/types:optional", + "gtest", + ], language = "c++", # We only run this test under Linux, and really only care about the # TSAN results. @@ -113,6 +118,9 @@ grpc_cc_test( uses_event_engine = False, uses_polling = False, deps = [ + "//:event_engine_memory_allocator", + "//:exec_ctx", + "//:gpr_base", "//:memory_quota", ], ) @@ -121,6 +129,10 @@ grpc_proto_fuzzer( name = "memory_quota_fuzzer", srcs = ["memory_quota_fuzzer.cc"], corpus = "memory_quota_fuzzer_corpus", + external_deps = [ + "absl/strings", + "absl/types:optional", + ], language = "C++", proto = "memory_quota_fuzzer.proto", tags = ["no_windows"], @@ -128,7 +140,14 @@ grpc_proto_fuzzer( uses_polling = False, deps = [ "call_checker", + "//:closure", + "//:debug_location", + "//:error", + "//:event_engine_memory_allocator", + "//:exec_ctx", + "//:gpr_base", + "//:grpc_trace", "//:memory_quota", - "//test/core/util:grpc_test_util", + "//:useful", ], ) diff --git a/test/core/util/BUILD b/test/core/util/BUILD index 01f9019d4c0..43facf28328 100644 --- a/test/core/util/BUILD +++ b/test/core/util/BUILD @@ -84,6 +84,7 @@ grpc_cc_library( "absl/strings:str_format", ], language = "C++", + tags = ["nofixdeps"], deps = [ "//:arena", "//:debug_location", @@ -126,7 +127,6 @@ grpc_cc_library( "//:config", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:grpc_public_hdrs", "//:grpc_security_base", "//:grpc_sockaddr", @@ -163,7 +163,6 @@ grpc_cc_library( "//:channel_args_preconditioning", "//:config", "//:gpr_base", - "//:grpc_base", "//:grpc_security_base", "//:grpc_sockaddr", "//:grpc_unsecure", @@ -289,7 +288,6 @@ grpc_cc_library( "//:gpr_base", "//:grpc", "//:grpc_backend_metric_data", - "//:grpc_base", "//:grpc_client_channel", "//:json", "//:json_util", @@ -320,7 +318,6 @@ grpc_cc_library( "grpc_test_util", "//:gpr_base", "//:grpc", - "//:grpc_base", "//:resolved_address", "//:sockaddr_utils", ], diff --git a/test/core/util/grpc_fuzzer.bzl b/test/core/util/grpc_fuzzer.bzl index 80589815957..07ee2e4d63f 100644 --- a/test/core/util/grpc_fuzzer.bzl +++ b/test/core/util/grpc_fuzzer.bzl @@ -18,13 +18,14 @@ Includes fuzzer rules. load("//bazel:grpc_build_system.bzl", "grpc_cc_test", "grpc_proto_library") -def grpc_fuzzer(name, corpus, srcs = [], tags = [], deps = [], data = [], size = "large", **kwargs): +def grpc_fuzzer(name, corpus, srcs = [], tags = [], external_deps = [], deps = [], data = [], size = "large", **kwargs): """Instantiates a fuzzer test. Args: name: The name of the test. corpus: The corpus for the test. srcs: The source files for the test. + external_deps: External deps. deps: The dependencies of the test. data: The data dependencies of the test. size: The size of the test. @@ -41,7 +42,7 @@ def grpc_fuzzer(name, corpus, srcs = [], tags = [], deps = [], data = [], size = "//conditions:default": ["//test/core/util:fuzzer_corpus_test"], }), data = data + native.glob([corpus + "/**"]), - external_deps = [ + external_deps = external_deps + [ "gtest", ], size = size, @@ -52,7 +53,7 @@ def grpc_fuzzer(name, corpus, srcs = [], tags = [], deps = [], data = [], size = **kwargs ) -def grpc_proto_fuzzer(name, corpus, proto, proto_deps = [], srcs = [], tags = [], deps = [], data = [], size = "large", **kwargs): +def grpc_proto_fuzzer(name, corpus, proto, proto_deps = [], external_deps = [], srcs = [], tags = [], deps = [], data = [], size = "large", **kwargs): """Instantiates a protobuf mutator fuzzer test. Args: @@ -60,6 +61,7 @@ def grpc_proto_fuzzer(name, corpus, proto, proto_deps = [], srcs = [], tags = [] corpus: The corpus for the test. proto: The proto for the test. proto_deps: Deps for proto. + external_deps: External deps. srcs: The source files for the test. deps: The dependencies of the test. data: The data dependencies of the test. @@ -89,7 +91,7 @@ def grpc_proto_fuzzer(name, corpus, proto, proto_deps = [], srcs = [], tags = [] "//conditions:default": ["//test/core/util:fuzzer_corpus_test"], }), data = data + native.glob([corpus + "/**"]), - external_deps = [ + external_deps = external_deps + [ "gtest", ], size = size, diff --git a/tools/distrib/fix_build_deps.py b/tools/distrib/fix_build_deps.py index 723441c7199..cff53e5dc15 100755 --- a/tools/distrib/fix_build_deps.py +++ b/tools/distrib/fix_build_deps.py @@ -37,10 +37,13 @@ buildozer_commands = [] needs_codegen_base_src = set() original_deps = {} original_external_deps = {} +skip_headers = collections.defaultdict(set) # TODO(ctiller): ideally we wouldn't hardcode a bunch of paths here. # We can likely parse out BUILD files from dependencies to generate this index. EXTERNAL_DEPS = { + 'absl/algorithm/container.h': + 'absl/algorithm:container', 'absl/base/attributes.h': 'absl/base:core_headers', 'absl/base/call_once.h': @@ -66,6 +69,8 @@ EXTERNAL_DEPS = { 'absl/debugging:symbolize', 'absl/flags/flag.h': 'absl/flags:flag', + 'absl/flags/parse.h': + 'absl/flags:parse', 'absl/functional/any_invocable.h': 'absl/functional:any_invocable', 'absl/functional/bind_front.h': @@ -265,6 +270,7 @@ def grpc_cc_library(name, tags=[], deps=[], external_deps=[], + proto=None, **kwargs): global args global num_cc_libraries @@ -284,6 +290,10 @@ def grpc_cc_library(name, # other, whilst not biasing dependent projects if 'avoid_dep' in tags or 'grpc_avoid_dep' in tags: avoidness[name] += 10 + if proto: + proto_hdr = '%s%s' % ((parsing_path + '/' if parsing_path else ''), + proto.replace('.proto', '.pb.h')) + skip_headers[name].add(proto_hdr) for hdr in hdrs + public_hdrs: filename = '%s%s' % ((parsing_path + '/' if parsing_path else ''), hdr) vendors[filename].append(name) @@ -370,11 +380,24 @@ parser.add_argument('--whats_left', action='store_true', default=False, help='show what is left to opt in') +parser.add_argument('--explain', + action='store_true', + default=False, + help='try to explain some decisions') +parser.add_argument( + '--why', + type=str, + default=None, + help='with --explain, target why a given dependency is needed') args = parser.parse_args() for dirname in [ - "", "test/core/uri", "test/core/util", "test/core/end2end", - "test/core/event_engine" + "", + "test/core/uri", + "test/core/util", + "test/core/end2end", + "test/core/event_engine", + "test/core/resource_quota", ]: parsing_path = dirname exec( @@ -386,9 +409,11 @@ for dirname in [ 'config_setting': lambda **kwargs: None, 'selects': FakeSelects(), 'python_config_settings': lambda **kwargs: None, + 'grpc_cc_binary': grpc_cc_library, 'grpc_cc_library': grpc_cc_library, 'grpc_cc_test': grpc_cc_library, 'grpc_fuzzer': grpc_cc_library, + 'grpc_proto_fuzzer': grpc_cc_library, 'select': lambda d: d["//conditions:default"], 'grpc_end2end_tests': lambda: None, 'grpc_upb_proto_library': lambda name, **kwargs: None, @@ -423,23 +448,35 @@ if args.whats_left: # problem. (models the list monad in Haskell!) class Choices: - def __init__(self, library): + def __init__(self, library, substitutions): self.library = library self.to_add = [] self.to_remove = [] + self.substitutions = substitutions - def add_one_of(self, choices): + def add_one_of(self, choices, trigger): if not choices: return + choices = sum([self.apply_substitutions(choice) for choice in choices], + []) + if args.explain and (args.why is None or args.why in choices): + print("{}: Adding one of {} for {}".format(self.library, choices, + trigger)) self.to_add.append( tuple( make_relative_path(choice, self.library) for choice in choices)) - def add(self, choice): - self.add_one_of([choice]) + def add(self, choice, trigger): + self.add_one_of([choice], trigger) def remove(self, remove): - self.to_remove.append(make_relative_path(remove, self.library)) + for remove in self.apply_substitutions(remove): + self.to_remove.append(make_relative_path(remove, self.library)) + + def apply_substitutions(self, dep): + if dep in self.substitutions: + return self.substitutions[dep] + return [dep] def best(self, scorer): choices = set() @@ -468,30 +505,40 @@ class Choices: def make_library(library): error = False hdrs = sorted(consumes[library]) - deps = Choices(library) - external_deps = Choices(None) + # we need a little trickery here since grpc_base has channel.cc, which calls grpc_init + # which is in grpc, which is illegal but hard to change + # once event engine lands we can clean this up + deps = Choices(library, {'//:grpc_base': ['//:grpc', '//:grpc_unsecure']} + if library.startswith('//test/') else {}) + external_deps = Choices(None, {}) for hdr in hdrs: + if hdr in skip_headers[library]: + continue + if hdr == 'src/core/lib/profiling/stap_probes.h': continue + if hdr.startswith('src/libfuzzer/'): + continue + if hdr == 'grpc/grpc.h' and not library.startswith('//:'): # not the root build including grpc.h ==> //:grpc - deps.add_one_of(['//:grpc', '//:grpc_unsecure']) + deps.add_one_of(['//:grpc', '//:grpc_unsecure'], hdr) continue if hdr in INTERNAL_DEPS: dep = INTERNAL_DEPS[hdr] if not dep.startswith('//'): dep = '//:' + dep - deps.add(dep) + deps.add(dep, hdr) continue if hdr in vendors: - deps.add_one_of(vendors[hdr]) + deps.add_one_of(vendors[hdr], hdr) continue if 'include/' + hdr in vendors: - deps.add_one_of(vendors['include/' + hdr]) + deps.add_one_of(vendors['include/' + hdr], hdr) continue if '.' not in hdr: @@ -499,13 +546,13 @@ def make_library(library): continue if hdr in EXTERNAL_DEPS: - external_deps.add(EXTERNAL_DEPS[hdr]) + external_deps.add(EXTERNAL_DEPS[hdr], hdr) continue if hdr.startswith('opencensus/'): trail = hdr[len('opencensus/'):] trail = trail[:trail.find('/')] - external_deps.add('opencensus-' + trail) + external_deps.add('opencensus-' + trail, hdr) continue if hdr.startswith('envoy/'): @@ -513,11 +560,11 @@ def make_library(library): file = file.split('.') path = path.split('/') dep = '_'.join(path[:-1] + [file[1]]) - deps.add(dep) + deps.add(dep, hdr) continue if hdr.startswith('google/protobuf/') and not hdr.endswith('.upb.h'): - external_deps.add('protobuf_headers') + external_deps.add('protobuf_headers', hdr) continue if '/' not in hdr: @@ -548,7 +595,7 @@ def make_library(library): error = True if library in needs_codegen_base_src: - deps.add('grpc++_codegen_base_src') + deps.add('grpc++_codegen_base_src', '#needs_codegen_base_src') deps.remove(library)