From e4b39f618089344784457930e8f1370e8f34ead9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 17 Aug 2020 11:37:51 +0200 Subject: [PATCH] add extra comments and TODOs --- .../extract_metadata_from_bazel_xml.py | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/tools/buildgen/extract_metadata_from_bazel_xml.py b/tools/buildgen/extract_metadata_from_bazel_xml.py index 6b7f14ea981..3cc65c3ad2e 100755 --- a/tools/buildgen/extract_metadata_from_bazel_xml.py +++ b/tools/buildgen/extract_metadata_from_bazel_xml.py @@ -26,6 +26,9 @@ # while useful as an overview, the doc does not act as formal spec # (formal spec does not exist in fact) and the doc can be incomplete, # inaccurate or slightly out of date. +# TODO(jtattermusch): In the future we want to get rid of the legacy build.yaml +# format entirely or simplify it to a point where it becomes self-explanatory +# and doesn't need any detailed documentation. import subprocess import yaml @@ -1012,7 +1015,23 @@ _populate_transitive_deps(bazel_rules) # Step 2: Extract the known bazel cc_test tests. While most tests # will be buildable with other build systems just fine, some of these tests # would be too difficult to build and run with other build systems, -# so we simply the ones we don't want. +# so we simply exclude the ones we don't want. +# Note that while making tests buildable with other build systems +# than just bazel is extra effort, we still need to do that for these +# reasons: +# - If our cmake build doesn't have any tests at all, it's hard to make +# sure that what it built actually works (we need at least some "smoke tests"). +# This is quite important because the build flags between bazel / non-bazel flag might differ +# (sometimes it's for interesting reasons that are not easy to overcome) +# which makes it even more important to have at least some tests for cmake/make +# - Our portability suite actually runs cmake tests and migration of portability +# suite fully towards bazel might be intricate (e.g. it's unclear whether it's +# possible to get a good enough coverage of different compilers / distros etc. +# with bazel) +# - some things that are considered "tests" in build.yaml-based builds are actually binaries +# we'd want to be able to build anyway (qps_json_worker, interop_client, interop_server, grpc_cli) +# so it's unclear how much make/cmake simplification we would gain by removing just some (but not all) test +# TODO(jtattermusch): Investigate feasibility of running portability suite with bazel. tests = _exclude_unwanted_cc_tests(_extract_cc_tests(bazel_rules)) # Step 3: Generate the "extra metadata" for all our build targets. @@ -1057,6 +1076,17 @@ all_extra_metadata.update( # - Some intermediate libraries get elided ("expanded") to better match the set # of targets provided by the legacy build.yaml build # +# Originally the target renaming was introduced to address these concerns: +# - avoid changing too many things at the same time and avoid people getting +# confused by some well know targets suddenly being missing +# - Makefile/cmake and also language-specific generators rely on some build +# targets being called exactly the way they they are. Some of our testing +# scrips also invoke executables (e.g. "qps_json_driver") by their name. +# - The autogenerated test name from bazel includes the package path +# (e.g. "test_cpp_TEST_NAME"). Without renaming, the target names would +# end up pretty ugly (e.g. test_cpp_qps_qps_json_driver). +# TODO(jtattermusch): reevaluate the need for target renaming in the future. +# # Example of a single generated target: # 'grpc' : { 'language': 'c', # 'public_headers': ['include/grpc/byte_buffer.h', ... ],