[cmake] Cleanup and simplify the cmake build. (grpc#34087)

Followup for grpc#33803 More cmake cleanup. See commit descriptions for individual cleanup items.
maniksurtani · Aug 17, 2023 · 3a2bd22 · 3a2bd22
1 parent 724ef46
commit 3a2bd22
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 1,254 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
diff --git a/Makefile b/Makefile
diff --git a/build_handwritten.yaml b/build_handwritten.yaml
@@ -131,20 +131,20 @@ configs:
 defaults:
   abseil:
     CPPFLAGS: -g -Ithird_party/abseil-cpp
-  ares:
-    CFLAGS: -g
-    CPPFLAGS: -Ithird_party/cares/cares/include -Ithird_party/cares -Ithird_party/cares/cares
-      -fvisibility=hidden -D_GNU_SOURCE $(if $(subst Darwin,,$(SYSTEM)),,-Ithird_party/cares/config_darwin)
-      $(if $(subst FreeBSD,,$(SYSTEM)),,-Ithird_party/cares/config_freebsd) $(if $(subst
-      Linux,,$(SYSTEM)),,-Ithird_party/cares/config_linux) $(if $(subst OpenBSD,,$(SYSTEM)),,-Ithird_party/cares/config_openbsd)
-      -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX $(if $(subst MINGW32,,$(SYSTEM)),-DHAVE_CONFIG_H,)
   benchmark:
     CPPFLAGS: -Ithird_party/benchmark/include -DHAVE_POSIX_REGEX
   boringssl:
     CFLAGS: -g
     CPPFLAGS: -Ithird_party/boringssl-with-bazel/src/include -fvisibility=hidden -DOPENSSL_NO_ASM
       -D_GNU_SOURCE -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX
     CXXFLAGS: -fno-exceptions
+  cares:
+    CFLAGS: -g
+    CPPFLAGS: -Ithird_party/cares/cares/include -Ithird_party/cares -Ithird_party/cares/cares
+      -fvisibility=hidden -D_GNU_SOURCE $(if $(subst Darwin,,$(SYSTEM)),,-Ithird_party/cares/config_darwin)
+      $(if $(subst FreeBSD,,$(SYSTEM)),,-Ithird_party/cares/config_freebsd) $(if $(subst
+      Linux,,$(SYSTEM)),,-Ithird_party/cares/config_linux) $(if $(subst OpenBSD,,$(SYSTEM)),,-Ithird_party/cares/config_openbsd)
+      -DWIN32_LEAN_AND_MEAN -D_HAS_EXCEPTIONS=0 -DNOMINMAX $(if $(subst MINGW32,,$(SYSTEM)),-DHAVE_CONFIG_H,)
   global:
     CFLAGS: -g
     COREFLAGS: -fno-exceptions
@@ -190,15 +190,15 @@ python_dependencies:
   deps:
   - grpc
   - address_sorting
-  - ares
+  - cares
   - boringssl
   - re2
   - z
 ruby_gem:
   deps:
   - grpc
   - address_sorting
-  - ares
+  - cares
   - boringssl
   - re2
   - z

diff --git a/src/c-ares/gen_build_yaml.py b/src/c-ares/gen_build_yaml.py
@@ -51,8 +51,8 @@ def ares_build(x):
 
     out["libs"] = [
         {
-            "name": "ares",
-            "defaults": "ares",
+            "name": "cares",
+            "defaults": "cares",
             "build": "private",
             "language": "c",
             "secure": False,

diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template
@@ -48,7 +48,9 @@
     return lib_name.startswith("absl/")
 
   def get_absl_dep(lib_name):
-      return lib_map[lib_name].cmake_target
+    """Gets the corresponding cmake target for given absl library."""
+    # The "cmake_target" field originates from src/abseil-cpp/preprocessed_builds.yaml.gen.py
+    return lib_map[lib_name].cmake_target
 
   def get_transitive_deps(lib_name):
     """Get list of transitive deps for given library."""
@@ -148,27 +150,9 @@
       private_libs.append("-l" + lib_name)
     return private_libs
 
-  def is_shared_only_lib(lib_name):
-    """Returns True if only shared library should be generated."""
-    # grpc_csharp_ext is loaded by C# runtime and it
-    # only makes sense as a shared lib.
-    return lib_name in ['grpc_csharp_ext']
-
   def get_deps(target_dict):
+    # TODO(jtattermusch): remove special cases based on target names
     deps = []
-    deps.append("${_gRPC_BASELIB_LIBRARIES}")
-    if target_dict.get('build', None) in ['protoc']:
-      deps.append("${_gRPC_PROTOBUF_PROTOC_LIBRARIES}")
-    if target_dict.language == 'c++':
-      deps.append("${_gRPC_PROTOBUF_LIBRARIES}")
-    if target_dict['name'] in ['grpc', 'grpc_cronet', 'grpc_unsecure']:
-      deps.append("${_gRPC_UPB_LIBRARIES}")
-    # TODO(yijiem): These targets depend on grpc_base instead of grpc. Since we don't populate grpc_base as a cmake target, the sources all get collapsed into these targets. This workaround adds c-ares and/or re2 dependencies to these targets. We should clean this up.
-    if target_dict['name'] in ['frame_test']:
-      deps.append("${_gRPC_CARES_LIBRARIES}")
-    if target_dict['name'] in ['grpc_authorization_provider']:
-      deps.append("${_gRPC_CARES_LIBRARIES}")
-      deps.append("${_gRPC_RE2_LIBRARIES}")
     deps.append("${_gRPC_ALLTARGETS_LIBRARIES}")
     for d in target_dict.get('deps', []):
       if d == 'z':
@@ -194,6 +178,22 @@
         deps.append(d)
     return deps
 
+  def is_generate_cmake_target(lib_or_target):
+    """Returns True if a cmake target should be generated for given library/target."""
+    # TODO(jtattermusch): extract the metadata to a centralized location.
+    if lib_or_target.build not in ["all", "protoc", "tool", "test", "private"]:
+      return False
+    if lib_or_target.boringssl:
+      # Don't generate target for boringssl libs or tests
+      return False
+    if lib_or_target.name in ['cares', 'benchmark', 're2', 'xxhash', 'z']:
+      # we rely on these target to be created by external cmake builds.
+      return False
+    if is_absl_lib(lib_or_target.name):
+      # we rely on absl targets to be created by an external cmake build.
+      return False
+    return True
+
   def get_platforms_condition_begin(platforms):
     if all(platform in platforms for platform in ['linux', 'mac', 'posix', 'windows']):
       return ''
@@ -260,8 +260,6 @@
   set(gRPC_CORE_SOVERSION   "${settings.core_version.major}")
   set(gRPC_CPP_VERSION      "${settings.cpp_version}")
   set(gRPC_CPP_SOVERSION    "${settings.cpp_version.major}.${settings.cpp_version.minor}")
-  set(gRPC_CSHARP_VERSION   "${settings.csharp_version}")
-  set(gRPC_CSHARP_SOVERSION "${settings.csharp_version.major}.${settings.csharp_version.minor}")
   set(PACKAGE_STRING        "<%text>${PACKAGE_NAME} ${PACKAGE_VERSION}</%text>")
   set(PACKAGE_TARNAME       "<%text>${PACKAGE_NAME}-${PACKAGE_VERSION}</%text>")
   set(PACKAGE_BUGREPORT     "https://github.com/grpc/grpc/issues/")
@@ -277,7 +275,6 @@
   # Options
   option(gRPC_BUILD_TESTS "Build tests" OFF)
   option(gRPC_BUILD_CODEGEN "Build codegen" ON)
-  option(gRPC_BUILD_CSHARP_EXT "Build C# extensions" ON)
   option(gRPC_BACKWARDS_COMPATIBILITY_MODE "Build libraries that are binary compatible across a larger number of OS and libc versions" OFF)
 
   set(gRPC_INSTALL_default ON)
@@ -420,12 +417,6 @@
     set(_gRPC_CORE_NOSTDCXX_FLAGS "")
   endif()
 
-  if (gRPC_XDS_USER_AGENT_IS_CSHARP)
-    # The value of the defines needs to contain quotes.
-    # See https://github.com/grpc/grpc/blob/fbf32836a418cc84f58786700273b65cb9174e1d/src/core/ext/xds/xds_api.cc#L854
-    add_definitions("-DGRPC_XDS_USER_AGENT_NAME_SUFFIX=\"csharp\"" "-DGRPC_XDS_USER_AGENT_VERSION_SUFFIX=\"${settings.csharp_version}\"")
-  endif()
-
   if(UNIX AND NOT HAIKU)
     # -pthread does more than -lpthread
     set(THREADS_PREFER_PTHREAD_FLAG ON)
@@ -476,7 +467,7 @@
   % endfor
 
   if(WIN32)
-    set(_gRPC_BASELIB_LIBRARIES ws2_32 crypt32)
+    set(_gRPC_ALLTARGETS_LIBRARIES <%text>${_gRPC_ALLTARGETS_LIBRARIES}</%text> ws2_32 crypt32)
   endif()
 
   # Create directory for proto source files
@@ -638,7 +629,7 @@
   if(gRPC_BUILD_TESTS)
     add_custom_target(buildtests_c)
     % for tgt in targets:
-    % if tgt.build == 'test' and not tgt.language == 'c++' and not tgt.get('external_deps', None) and not tgt.boringssl:
+    % if is_generate_cmake_target(tgt) and tgt.build == 'test' and not tgt.language == 'c++':
     <%block filter='platforms_condition_block(tgt.platforms)'>
     add_dependencies(buildtests_c ${tgt.name})
     </%block>
@@ -647,7 +638,7 @@
 
     add_custom_target(buildtests_cxx)
     % for tgt in targets:
-    % if tgt.build == 'test' and tgt.language == 'c++' and not tgt.get('external_deps', None) and not tgt.boringssl:
+    % if is_generate_cmake_target(tgt) and tgt.build == 'test' and tgt.language == 'c++':
     <%block filter='platforms_condition_block(tgt.platforms)'>
     add_dependencies(buildtests_cxx ${tgt.name})
     </%block>
@@ -660,21 +651,14 @@
   <%
     cmake_libs = []
     for lib in libs:
-      if lib.build not in ["all", "protoc", "tool", "test", "private"] or lib.boringssl: continue
-      if lib.get('build_system', []) and 'cmake' not in lib.get('build_system', []): continue
-      if lib.name in ['ares', 'benchmark', 're2', 'xxhash', 'z']: continue  # we build these using CMake instead
-      if is_absl_lib(lib.name): continue  # we build these using CMake instead
+      if not is_generate_cmake_target(lib): continue
       cmake_libs.append(lib)
   %>
   % for lib in cmake_libs:
   %   if lib.build in ["test", "private"]:
   if(gRPC_BUILD_TESTS)
   ${cc_library(lib)}
   endif()
-  %   elif lib.name in ['grpc_csharp_ext']:
-  if(gRPC_BUILD_CSHARP_EXT)
-  ${cc_library(lib)}
-  endif()
   %   else:
   ${cc_library(lib)}
   %     if not lib.build in ["tool"]:
@@ -690,7 +674,7 @@
   % endfor
 
   % for tgt in targets:
-  % if tgt.build in ["all", "protoc", "tool", "test", "private"] and not tgt.boringssl:
+  % if is_generate_cmake_target(tgt):
   % if tgt.build in ["test", "private"]:
   if(gRPC_BUILD_TESTS)
   <%block filter='platforms_condition_block(tgt.platforms)'>
@@ -717,15 +701,15 @@
 
   <%def name="cc_library(lib)">
   % if any(proto_re.match(src) for src in lib.src):
-  % if lib.name == 'grpcpp_channelz':
+  % if lib.name == 'grpcpp_channelz':  # TODO(jtattermusch): remove special case based on target name
   # grpcpp_channelz doesn't build with protobuf-lite
   # See https://github.com/grpc/grpc/issues/19473
   if(gRPC_BUILD_CODEGEN AND NOT gRPC_USE_PROTO_LITE)
   % else:
   if(gRPC_BUILD_CODEGEN)
   % endif
   % endif
-  add_library(${lib.name}${' SHARED' if is_shared_only_lib(lib.name) else ''}
+  add_library(${lib.name}
   % for src in lib.src:
   % if not proto_re.match(src):
     ${src}
@@ -747,9 +731,6 @@
   % if lib.language == 'c++':
     VERSION <%text>${gRPC_CPP_VERSION}</%text>
     SOVERSION <%text>${gRPC_CPP_SOVERSION}</%text>
-  % elif lib.language == 'csharp':
-    VERSION <%text>${gRPC_CSHARP_VERSION}</%text>
-    SOVERSION <%text>${gRPC_CSHARP_SOVERSION}</%text>
   % else:
     VERSION <%text>${gRPC_CORE_VERSION}</%text>
     SOVERSION <%text>${gRPC_CORE_SOVERSION}</%text>
@@ -779,7 +760,7 @@
       <%text>${_gRPC_UPB_INCLUDE_DIR}</%text>
       <%text>${_gRPC_XXHASH_INCLUDE_DIR}</%text>
       <%text>${_gRPC_ZLIB_INCLUDE_DIR}</%text>
-  % if 'gtest' in lib.transitive_deps or lib.name == 'gtest':
+  % if 'gtest' in lib.transitive_deps or lib.name == 'gtest':  # TODO(jtattermusch): avoid special case based on target name
       third_party/googletest/googletest/include
       third_party/googletest/googletest
       third_party/googletest/googlemock/include
@@ -796,7 +777,7 @@
   % endfor
   )
   % endif
-  % if lib.name in ["gpr"]:
+  % if lib.name in ["gpr"]:  # TODO(jtattermusch): avoid special case based on target name
   if(_gRPC_PLATFORM_ANDROID)
     target_link_libraries(gpr
       android
@@ -806,7 +787,7 @@
   % endif
   % if lib.name in ["grpc", "grpc_cronet", "grpc_test_util", \
                     "grpc_test_util_unsecure", "grpc_unsecure", \
-                    "grpc++_cronet"]:
+                    "grpc++_cronet"]: # TODO(jtattermusch): remove special cases based on target names
   if(_gRPC_PLATFORM_IOS OR _gRPC_PLATFORM_MAC)
     target_link_libraries(${lib.name} "-framework CoreFoundation")
   endif()

diff --git a/templates/Makefile.template b/templates/Makefile.template
@@ -47,8 +47,6 @@
       """For given dependency, return the expression to be used in Makefile rule dependencies."""
       if dep == 'z':
           return "$(ZLIB_DEP)"
-      elif dep == 'cares':
-          return "$(CARES_DEP)"
       elif dep == 'grpc_abseil':
           return "$(GRPC_ABSEIL_DEP)"
       elif dep == 'libssl':
@@ -61,8 +59,6 @@
       """For given dependency, return the lib archive expression to be used when linking."""
       if dep == 'z':
           return "$(ZLIB_MERGE_LIBS)"
-      elif dep == 'cares':
-          return "$(CARES_MERGE_LIBS)"
       elif dep == 'grpc_abseil':
           return "$(GRPC_ABSEIL_MERGE_LIBS)"
       elif dep == 'libssl':
@@ -75,8 +71,6 @@
       """For given dependency, return the expression with variable that has list of all object files."""
       if dep == 'z':
           return "$(ZLIB_MERGE_OBJS)"
-      elif dep == 'cares':
-          return "$(CARES_MERGE_OBJS)"
       elif dep == 'grpc_abseil':
           return "$(LIBGRPC_ABSEIL_OBJS)"
       elif dep == 'libssl':
@@ -183,7 +177,7 @@
     # Currently it is necessary because some dependencies are marked as "build: private" in build.yaml
     # (which itself is correct, as they are not "public" libraries from our perspective and cmake
     # needs to have them marked as such)
-    filtered_libs = [lib for lib in libs if (lib.build in ['all'] and lib.language != 'c++') or lib.name in ['ares', 'boringssl', 're2', 'upb', 'z']]
+    filtered_libs = [lib for lib in libs if (lib.build in ['all'] and lib.language != 'c++') or lib.name in ['cares', 'boringssl', 'z']]
     filtered_targets = [tgt for tgt in targets if tgt.build in ['all'] and lib.language != 'c++']
   %>
 
@@ -542,9 +536,6 @@
   endif
 
   ifeq ($(EMBED_CARES),true)
-  CARES_DEP = $(LIBDIR)/$(CONFIG)/libares.a
-  CARES_MERGE_OBJS = $(LIBARES_OBJS)
-  CARES_MERGE_LIBS = $(LIBDIR)/$(CONFIG)/libares.a
   CPPFLAGS := -Ithird_party/cares/cares/include -Ithird_party/cares -Ithird_party/cares/cares $(CPPFLAGS)
   endif
 

diff --git a/templates/config.m4.template b/templates/config.m4.template
@@ -52,8 +52,8 @@
       lib = lib_maps.get(dep, None)
       if lib:
         php_full_deps.extend(lib.transitive_deps)
-    # construct list of all source files, but explicitly exclude zlib
-    for dep in set(php_full_deps) - set({'z'}):
+    # construct list of all source files, but explicitly exclude zlib and cares
+    for dep in set(php_full_deps) - set({'z', 'cares'}):
       lib = lib_maps.get(dep, None)
       if lib:
         srcs.extend(lib.src)

diff --git a/templates/config.w32.template b/templates/config.w32.template
@@ -17,7 +17,8 @@
       lib = lib_maps.get(dep, None)
       if lib:
         php_full_deps.extend(lib.transitive_deps)
-    for dep in php_full_deps:
+    # construct list of all source files, but explicitly exclude cares
+    for dep in set(php_full_deps) - set({'cares'}):
       lib = lib_maps.get(dep, None)
       if lib:
         srcs.extend(lib.src)