[libc][Take 2] Propagate entrypoint deps to downstream targets.

This reverts commit a8086ba. Setting couple of target properties to an empty string was missed in the previous commit.
dabrahams · Apr 21, 2020 · 7f12512 · 7f12512
1 parent be881e2
commit 7f12512
Show file tree

Hide file tree

Showing 10 changed files with 123 additions and 150 deletions.
diff --git a/libc/cmake/modules/LLVMLibCLibraryRules.cmake b/libc/cmake/modules/LLVMLibCLibraryRules.cmake
@@ -1,8 +1,46 @@
+# This is a helper function and not a build rule. It is to be used by the
+# the "add_entrypoint_library" rule to generate the full list of object files
+# recursively produced by "add_object_library" targets upstream in the
+# dependency tree. This function traverses up through the
+# "add_entrypoint_object" targets but does not collect the object files
+# produced by them.
+# Usage:
+#   get_object_files_for_test(<result var> <target0> [<target1> ...])
+#
+#   targetN is either an "add_entrypoint_target" target or an
+#   "add_object_library" target.
+function(get_object_files_for_entrypoint_library result)
+  set(object_files "")
+  foreach(dep IN LISTS ARGN)
+    get_target_property(dep_type ${dep} "TARGET_TYPE")
+    if (NOT dep_type)
+      continue()
+    endif()
+
+    if(${dep_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})
+      get_target_property(dep_object_files ${dep} "OBJECT_FILES")
+      if(dep_object_files)
+        list(APPEND object_files ${dep_object_files})
+      endif()
+    endif()
+
+    get_target_property(indirect_deps ${dep} "DEPS")
+    get_object_files_for_entrypoint_library(indirect_objfiles ${indirect_deps})
+    list(APPEND object_files ${indirect_objfiles})
+  endforeach(dep)
+  list(REMOVE_DUPLICATES object_files)
+  set(${result} ${object_files} PARENT_SCOPE)
+endfunction()
+
 # A rule to build a library from a collection of entrypoint objects.
 # Usage:
 #     add_entrypoint_library(
 #       DEPENDS <list of add_entrypoint_object targets>
 #     )
+#
+# NOTE: If one wants an entrypoint to be availabe in a library, then they will
+# have to list the entrypoint target explicitly in the DEPENDS list. Implicit
+# entrypoint dependencies will not be added to the library.
 function(add_entrypoint_library target_name)
   cmake_parse_arguments(
     "ENTRYPOINT_LIBRARY"
@@ -16,15 +54,16 @@ function(add_entrypoint_library target_name)
                         "of 'add_entrypoint_object' targets.")
   endif()
 
-  set(obj_list "")
-  foreach(dep IN LISTS ENTRYPOINT_LIBRARY_DEPENDS)
+  get_fq_deps_list(fq_deps_list ${ENTRYPOINT_LIBRARY_DEPENDS})
+  get_object_files_for_entrypoint_library(obj_list ${fq_deps_list})
+  foreach(dep IN LISTS fq_deps_list)
     get_target_property(dep_type ${dep} "TARGET_TYPE")
     if(NOT (${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE}))
       message(FATAL_ERROR "Dependency '${dep}' of 'add_entrypoint_collection' is "
                           "not an 'add_entrypoint_object' target.")
     endif()
-    get_target_property(target_obj_files ${dep} "OBJECT_FILES")
-    list(APPEND obj_list "${target_obj_files}")
+    get_target_property(objfile ${dep} "OBJECT_FILE")
+    list(APPEND obj_list ${objfile})
   endforeach(dep)
   list(REMOVE_DUPLICATES obj_list)
 
@@ -78,6 +117,8 @@ function(add_redirector_library target_name)
   )
 endfunction(add_redirector_library)
 
+set(HDR_LIBRARY_TARGET_TYPE "HDR_LIBRARY")
+
 # Rule to add header only libraries.
 # Usage
 #    add_header_library(
@@ -107,12 +148,12 @@ function(add_header_library target_name)
     list(APPEND FULL_HDR_PATHS ${CMAKE_CURRENT_SOURCE_DIR}/${hdr})
   endforeach()
 
-  set(interface_target_name "${fq_target_name}_header_library__")
+  set(interface_target_name "${fq_target_name}.__header_library__")
 
   add_library(${interface_target_name} INTERFACE)
   target_sources(${interface_target_name} INTERFACE ${FULL_HDR_PATHS})
+  get_fq_deps_list(fq_deps_list ${ADD_HEADER_DEPENDS})
   if(ADD_HEADER_DEPENDS)
-    get_fq_deps_list(fq_deps_list ${ADD_HEADER_DEPENDS})
     add_dependencies(${interface_target_name} ${fq_deps_list})
   endif()
 
@@ -121,6 +162,7 @@ function(add_header_library target_name)
   set_target_properties(
     ${fq_target_name}
     PROPERTIES
-      "TARGET_TYPE" "HDR_LIBRARY"
+      "TARGET_TYPE" "${HDR_LIBRARY_TARGET_TYPE}"
+      "DEPS" "${fq_deps_list}"
   )
 endfunction(add_header_library)
diff --git a/libc/cmake/modules/LLVMLibCObjectRules.cmake b/libc/cmake/modules/LLVMLibCObjectRules.cmake
@@ -43,36 +43,17 @@ function(add_object_library target_name)
     )
   endif()
 
-  set(all_object_files $<TARGET_OBJECTS:${fq_target_name}>)
-  if(ADD_OBJECT_DEPENDS)
-    get_fq_deps_list(fq_deps_list ${ADD_OBJECT_DEPENDS})
-    add_dependencies(
-      ${fq_target_name}
-      ${fq_deps_list}
-    )
-    foreach(obj_target IN LISTS fq_deps_list)
-      if(NOT TARGET obj_target)
-        # Not all targets will be visible. So, we will ignore those which aren't
-        # visible yet.
-        continue()
-      endif()
-      get_target_property(obj_type ${obj_target} "TARGET_TYPE")
-      if((NOT obj_type) OR (NOT (${obj_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})))
-        continue()
-      endif()
-      # If a dependency is also a object file library, we will collect the list of
-      # object files from it.
-      get_target_property(obj_files ${obj_target} "OBJECT_FILES")
-      list(APPEND all_object_files ${obj_files})
-    endforeach(obj_target)
+  get_fq_deps_list(fq_deps_list ${ADD_OBJECT_DEPENDS})
+  if(fq_deps_list)
+    add_dependencies(${fq_target_name} ${fq_deps_list})
   endif()
-  list(REMOVE_DUPLICATES all_object_files)
 
   set_target_properties(
     ${fq_target_name}
     PROPERTIES
       "TARGET_TYPE" ${OBJECT_LIBRARY_TARGET_TYPE}
-      "OBJECT_FILES" "${all_object_files}"
+      "OBJECT_FILES" "$<TARGET_OBJECTS:${fq_target_name}>"
+      "DEPS" "${fq_deps_list}"
   )
 endfunction(add_object_library)
 
@@ -123,14 +104,15 @@ function(add_entrypoint_object target_name)
 
     add_custom_target(${fq_target_name})
     add_dependencies(${fq_target_name} ${fq_dep_name})
-    get_target_property(all_objects ${fq_dep_name} "OBJECT_FILES")
-    get_target_property(all_objects_raw ${fq_dep_name} "OBJECT_FILES_RAW")
+    get_target_property(object_file ${fq_dep_name} "OBJECT_FILE")
+    get_target_property(object_file_raw ${fq_dep_name} "OBJECT_FILE_RAW")
     set_target_properties(
       ${fq_target_name}
       PROPERTIES
         "TARGET_TYPE" ${ENTRYPOINT_OBJ_TARGET_TYPE}
-        "OBJECT_FILES" "${all_objects}"
-        "OBJECT_FILES_RAW" "${all_objects_raw}"
+        "OBJECT_FILE" ""
+        "OBJECT_FILE_RAW" ""
+        "DEPS" "${fq_dep_name}"
     )
     return()
   endif()
@@ -170,38 +152,12 @@ function(add_entrypoint_object target_name)
       ${LIBC_SOURCE_DIR}
       ${LIBC_BUILD_DIR}
   )
+  get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
   add_dependencies(
     ${objects_target_name}
     libc.src.__support.common
+    ${fq_deps_list}
   )
-  set(dep_objects "")
-  if(ADD_ENTRYPOINT_OBJ_DEPENDS)
-    get_fq_deps_list(fq_deps_list ${ADD_ENTRYPOINT_OBJ_DEPENDS})
-    add_dependencies(
-      ${objects_target_name}
-      ${fq_deps_list}
-    )
-    foreach(dep_target IN LISTS fq_deps_list)
-      if(NOT TARGET ${dep_target})
-        # Not all targets will be visible. So, we will ignore those which aren't
-        # visible yet.
-        continue()
-      endif()
-      get_target_property(obj_type ${dep_target} "TARGET_TYPE")
-      if((NOT obj_type) OR (NOT (${obj_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})))
-        # Even from among the visible targets, we will collect object files
-        # only from add_object_library targets.
-        continue()
-      endif()
-      # Calling get_target_property requires that the target be visible at this
-      # point. For object library dependencies, this is a reasonable requirement.
-      # We can revisit this in future if we need cases which break under this
-      # requirement.
-      get_target_property(obj_files ${dep_target} "OBJECT_FILES")
-      list(APPEND dep_objects ${obj_files})
-    endforeach(dep_target)
-  endif()
-  list(REMOVE_DUPLICATES dep_objects)
 
   if(ADD_ENTRYPOINT_OBJ_COMPILE_OPTIONS)
     target_compile_options(
@@ -239,16 +195,13 @@ function(add_entrypoint_object target_name)
     ALL
     DEPENDS ${object_file}
   )
-  set(all_objects ${object_file})
-  list(APPEND all_objects ${dep_objects})
-  set(all_objects_raw ${object_file_raw})
-  list(APPEND all_objects_raw ${dep_objects})
   set_target_properties(
     ${fq_target_name}
     PROPERTIES
       "TARGET_TYPE" ${ENTRYPOINT_OBJ_TARGET_TYPE}
-      "OBJECT_FILES" "${all_objects}"
-      "OBJECT_FILES_RAW" "${all_objects_raw}"
+      "OBJECT_FILE" "${object_file}"
+      "OBJECT_FILE_RAW" "${object_file_raw}"
+      "DEPS" "${fq_deps_list}"
   )
 
   if(LLVM_LIBC_ENABLE_LINTING)
@@ -310,4 +263,3 @@ function(add_redirector_object target_name)
     BEFORE PRIVATE -fPIC
   )
 endfunction(add_redirector_object)
-
diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -1,3 +1,42 @@
+# This is a helper function and not a build rule. It is to be used by the
+# various test rules to generate the full list of object files
+# recursively produced by "add_entrypoint_object" and "add_object_library"
+# targets.
+# Usage:
+#   get_object_files_for_test(<result var> <target0> [<target1> ...])
+#
+#   targetN is either an "add_entrypoint_target" target or an
+#   "add_object_library" target.
+function(get_object_files_for_test result)
+  set(object_files "")
+  foreach(dep IN LISTS ARGN)
+    get_target_property(dep_type ${dep} "TARGET_TYPE")
+    if(NOT dep_type)
+      # Target for which TARGET_TYPE property is not set do not
+      # provide any object files.
+      continue()
+    endif()
+
+    if(${dep_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})
+      get_target_property(dep_object_files ${dep} "OBJECT_FILES")
+      if(dep_object_files)
+        list(APPEND object_files ${dep_object_files})
+      endif()
+    elseif(${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE})
+      get_target_property(object_file_raw ${dep} "OBJECT_FILE_RAW")
+      if(object_file_raw)
+        list(APPEND object_files ${object_file_raw})
+      endif()
+    endif()
+
+    get_target_property(indirect_deps ${dep} "DEPS")
+    get_object_files_for_test(indirect_objfiles ${indirect_deps})
+    list(APPEND object_files ${indirect_objfiles})
+  endforeach(dep)
+  list(REMOVE_DUPLICATES object_files)
+  set(${result} ${object_files} PARENT_SCOPE)
+endfunction(get_object_files_for_test)
+
 # Rule to add a libc unittest.
 # Usage
 #    add_libc_unittest(
@@ -29,21 +68,6 @@ function(add_libc_unittest target_name)
                         "'add_entrypoint_object' targets.")
   endif()
 
-  set(library_deps "")
-  get_fq_deps_list(fq_deps_list ${LIBC_UNITTEST_DEPENDS})
-  foreach(dep IN LISTS fq_deps_list)
-    get_target_property(dep_type ${dep} "TARGET_TYPE")
-    if(${dep_type} STREQUAL ${ENTRYPOINT_OBJ_TARGET_TYPE})
-      get_target_property(obj_files ${dep} "OBJECT_FILES_RAW")
-      list(APPEND library_deps ${obj_files})
-    elseif(${dep_type} STREQUAL ${OBJECT_LIBRARY_TARGET_TYPE})
-      get_target_property(obj_files ${dep} "OBJECT_FILES")
-      list(APPEND library_deps ${obj_files})
-    endif()
-    # TODO: Check if the dep is a normal CMake library target. If yes, then add it
-    # to the list of library_deps.
-  endforeach(dep)
-  list(REMOVE_DUPLICATES library_deps)
 
   get_fq_target_name(${target_name} fq_target_name)
   add_executable(
@@ -66,9 +90,9 @@ function(add_libc_unittest target_name)
     )
   endif()
 
-  if(library_deps)
-    target_link_libraries(${fq_target_name} PRIVATE ${library_deps})
-  endif()
+  get_fq_deps_list(fq_deps_list ${LIBC_UNITTEST_DEPENDS})
+  get_object_files_for_test(link_object_files ${fq_deps_list})
+  target_link_libraries(${fq_target_name} PRIVATE ${link_object_files})
 
   set_target_properties(${fq_target_name}
     PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
@@ -123,22 +147,6 @@ function(add_libc_fuzzer target_name)
                         "'add_entrypoint_object' targets.")
   endif()
 
-  get_fq_deps_list(fq_deps_list ${LIBC_FUZZER_DEPENDS})
-  set(library_deps "")
-  foreach(dep IN LISTS fq_deps_list)
-    get_target_property(dep_type ${dep} "TARGET_TYPE")
-    if (dep_type)
-      string(COMPARE EQUAL ${dep_type} ${ENTRYPOINT_OBJ_TARGET_TYPE} dep_is_entrypoint)
-      if(dep_is_entrypoint)
-        get_target_property(obj_file ${dep} "OBJECT_FILES_RAW")
-        list(APPEND library_deps ${obj_file})
-        continue()
-      endif()
-    endif()
-    # TODO: Check if the dep is a normal CMake library target. If yes, then add it
-    # to the list of library_deps.
-  endforeach(dep)
-
   get_fq_target_name(${target_name} fq_target_name)
   add_executable(
     ${fq_target_name}
@@ -154,9 +162,9 @@ function(add_libc_fuzzer target_name)
       ${LIBC_BUILD_DIR}/include
   )
 
-  if(library_deps)
-    target_link_libraries(${fq_target_name} PRIVATE ${library_deps})
-  endif()
+  get_fq_deps_list(fq_deps_list ${LIBC_FUZZER_DEPENDS})
+  get_object_files_for_test(link_object_files ${fq_deps_list})
+  target_link_libraries(${fq_target_name} PRIVATE ${link_object_files})
 
   set_target_properties(${fq_target_name}
       PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})

diff --git a/libc/loader/linux/CMakeLists.txt b/libc/loader/linux/CMakeLists.txt
@@ -8,7 +8,7 @@ function(add_loader_object name)
   )
 
   get_fq_target_name(${name} fq_target_name)
-
+  get_fq_deps_list(fq_deps_list ${ADD_LOADER_OBJECT_DEPENDS})
   if(ADD_LOADER_OBJECT_ALIAS)
     list(LENGTH ADD_LOADER_OBJECT_DEPENDS deps_size)
     if(NOT (${deps_size} EQUAL "1"))
@@ -23,14 +23,15 @@ function(add_loader_object name)
     set_target_properties(
       ${fq_target_name}
       PROPERTIES
-        "TARGET_TYPE" "LOADER_OBJECT"
-        "OBJECT_FILES" ${dep_objfile}
+        "TARGET_TYPE" "${OBJECT_LIBRARY_TARGET_TYPE}"
+        "OBJECT_FILES" ""
+        "DEPS" "${fq_dep_name}"
     )
     return()
   endif()
 
   add_object_library(
-    ${name}_objects
+    ${name}.__objects__
     SRCS ${ADD_LOADER_OBJECT_SRC}
     DEPENDS ${ADD_LOADER_OBJECT_DEPENDS}
     COMPILE_OPTIONS ${ADD_LOADER_OBJECT_COMPILE_OPTIONS}
@@ -39,8 +40,8 @@ function(add_loader_object name)
   set(objfile ${LIBC_BUILD_DIR}/lib/${name}.o)
   add_custom_command(
     OUTPUT ${objfile}
-    COMMAND cp $<TARGET_OBJECTS:${fq_target_name}_objects> ${objfile}
-    DEPENDS $<TARGET_OBJECTS:${fq_target_name}_objects>
+    COMMAND cp $<TARGET_OBJECTS:${fq_target_name}.__objects__> ${objfile}
+    DEPENDS $<TARGET_OBJECTS:${fq_target_name}.__objects__>
   )
   add_custom_target(
     ${fq_target_name}
@@ -49,8 +50,9 @@ function(add_loader_object name)
   set_target_properties(
     ${fq_target_name}
     PROPERTIES
-      "TARGET_TYPE" "LOADER_OBJECT"
-      "OBJECT_FILES" ${objfile}
+      "TARGET_TYPE" "${OBJECT_LIBRARY_TARGET_TYPE}"
+      "OBJECT_FILES" ""
+      "DEPS" "${fq_target_name}.__objects__"
   )
 endfunction()