[llvm] 1d773a4 - [CMake] Support inter-proto dependencies in generate_protos.

[Sam McCall via llvm-commits]

Author: Sam McCall
Date: 2020-10-29T10:04:20+01:00
New Revision: 1d773a4ff05d0dcfab112719b82b2bd5d0c93ff5

URL: https://github.com/llvm/llvm-project/commit/1d773a4ff05d0dcfab112719b82b2bd5d0c93ff5
DIFF: https://github.com/llvm/llvm-project/commit/1d773a4ff05d0dcfab112719b82b2bd5d0c93ff5.diff

LOG: [CMake] Support inter-proto dependencies in generate_protos.

Differential Revision: https://reviews.llvm.org/D90215

Added: 
    

Modified: 
    clang-tools-extra/clangd/index/remote/CMakeLists.txt
    llvm/cmake/modules/FindGRPC.cmake

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/index/remote/CMakeLists.txt b/clang-tools-extra/clangd/index/remote/CMakeLists.txt
index 554288df0bcb..a07dd994b5a3 100644
--- a/clang-tools-extra/clangd/index/remote/CMakeLists.txt
+++ b/clang-tools-extra/clangd/index/remote/CMakeLists.txt
@@ -1,13 +1,8 @@
 if (CLANGD_ENABLE_REMOTE)
-  generate_protos(RemoteIndexServiceProto "Service.proto" GRPC)
   generate_protos(RemoteIndexProto "Index.proto")
-  # Ensure dependency headers are generated before dependent protos are built.
-  # FIXME: this should be encapsulated in generate_protos.
-  # FIXME: CMake docs say OBJECT_DEPENDS isn't needed, but I can't get the
-  #        recommended add_dependencies() approach to work.
-  set_source_files_properties(
-    ${CMAKE_CURRENT_BINARY_DIR}/Service.pb.cc
-    PROPERTIES OBJECT_DEPENDS ${CMAKE_CURRENT_BINARY_DIR}/Index.pb.h)
+  generate_protos(RemoteIndexServiceProto "Service.proto"
+    DEPENDS "Index.proto"
+    GRPC)
   include_directories(${CMAKE_CURRENT_BINARY_DIR})
   include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../../)
 

diff  --git a/llvm/cmake/modules/FindGRPC.cmake b/llvm/cmake/modules/FindGRPC.cmake
index 9e837a80661c..f2c9bee38c93 100644
--- a/llvm/cmake/modules/FindGRPC.cmake
+++ b/llvm/cmake/modules/FindGRPC.cmake
@@ -84,8 +84,10 @@ endif()
 # Proto headers are generated in ${CMAKE_CURRENT_BINARY_DIR}.
 # Libraries that use these headers should adjust the include path.
 # If the "GRPC" argument is given, services are also generated.
+# The DEPENDS list should name *.proto source files that are imported.
+# They may be relative to the source dir or absolute (for generated protos).
 function(generate_protos LibraryName ProtoFile)
-  cmake_parse_arguments(PARSE_ARGV 2 PROTO "GRPC" "" "")
+  cmake_parse_arguments(PARSE_ARGV 2 PROTO "GRPC" "" "DEPENDS")
   get_filename_component(ProtoSourceAbsolutePath "${CMAKE_CURRENT_SOURCE_DIR}/${ProtoFile}" ABSOLUTE)
   get_filename_component(ProtoSourcePath ${ProtoSourceAbsolutePath} PATH)
   get_filename_component(Basename ${ProtoSourceAbsolutePath} NAME_WLE)
@@ -111,4 +113,23 @@ function(generate_protos LibraryName ProtoFile)
   add_clang_library(${LibraryName} ${GeneratedProtoSource}
     PARTIAL_SOURCES_INTENDED
     LINK_LIBS grpc++ protobuf)
+
+  # Ensure dependency headers are generated before dependent protos are built.
+  # DEPENDS arg is a list of "Foo.proto". While they're logically relative to
+  # the source dir, the generated headers we need are in the binary dir.
+  foreach(ImportedProto IN LISTS PROTO_DEPENDS)
+    # Foo.proto -> Foo.pb.h
+    STRING(REGEX REPLACE "\\.proto$" ".pb.h" ImportedHeader "${ImportedProto}")
+    # Foo.pb.h -> ${CMAKE_CURRENT_BINARY_DIR}/Foo.pb.h
+    get_filename_component(ImportedHeader "${ImportedHeader}"
+      ABSOLUTE
+      BASE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+    # Compilation of each generated source depends on ${BINARY}/Foo.pb.h.
+    foreach(Generated IN LISTS GeneratedProtoSource)
+      # FIXME: CMake docs suggest OBJECT_DEPENDS isn't needed, but I can't get
+      #        the recommended add_dependencies() approach to work.
+      set_source_files_properties("${Generated}"
+        PROPERTIES OBJECT_DEPENDS "${ImportedHeader}")
+    endforeach(Generated)
+  endforeach(ImportedProto)
 endfunction()