[compiler-rt] Create install targets for Darwin libraries

[smeenai]

Summary

Darwin targets were generating CMake install rules but not the
corresponding install targets. Centralize the existing install target
creation to a function and use that function for both Darwin and
non-Darwin builds.

[phosek]

LGTM

compiler-rt/cmake/Modules/CompilerRTUtils.cmake

423 ↗

(On Diff #198103)

IIUC, only the first invocation of this function will create the parent target and all the following ones will reuse it (for all systems/architectures with the same parent target that is). One possible alternative would be to split this into its function to avoid all those extra checks on subsequent invocations of this function.

[smeenai]

compiler-rt/cmake/Modules/CompilerRTUtils.cmake

423 ↗

(On Diff #198103)

Hmm, so the split function would just create the install-$parent targets? How would we ensure the split function gets called for each $parent without doing some sort of existence check?

Note that I've copied this logic from similar logic in AddCompilerRT.cmake: https://github.com/llvm/llvm-project/blob/491746a584723c147f8910c1ad5051c507afd735/compiler-rt/cmake/Modules/AddCompilerRT.cmake#L237

[smeenai]

Will address any additional comments post-commit. Thanks for the quick review!

[smeenai]

compiler-rt/cmake/Modules/CompilerRTUtils.cmake

423 ↗

(On Diff #198103)

Ah, I guess you meant having a second function that's called outside the loop in AddCompilerRT? I'm not sure if target checks are expensive enough to make that worth it ... I couldn't measure any non-noise CMake configure time difference before and after this patch.

compiler-rt/trunk/cmake/Modules/AddCompilerRT.cmake

function(add_compiler_rt_runtime name type)
 
   if(LIB_PARENT_TARGET)
     # If the parent targets aren't created we should create them
     if(NOT TARGET ${LIB_PARENT_TARGET})
       add_custom_target(${LIB_PARENT_TARGET})
       set_target_properties(${LIB_PARENT_TARGET} PROPERTIES
                             FOLDER "Compiler-RT Misc")
     endif()
-    if(NOT TARGET install-${LIB_PARENT_TARGET})
-      # The parent install target specifies the parent component to scrape up
-      # anything not installed by the individual install targets, and to handle
-      # installation when running the multi-configuration generators.
-      add_custom_target(install-${LIB_PARENT_TARGET}
-                        DEPENDS ${LIB_PARENT_TARGET}
-                        COMMAND "${CMAKE_COMMAND}"
-                                -DCMAKE_INSTALL_COMPONENT=${LIB_PARENT_TARGET}
-                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-      add_custom_target(install-${LIB_PARENT_TARGET}-stripped
-                        DEPENDS ${LIB_PARENT_TARGET}
-                        COMMAND "${CMAKE_COMMAND}"
-                                -DCMAKE_INSTALL_COMPONENT=${LIB_PARENT_TARGET}
-                                -DCMAKE_INSTALL_DO_STRIP=1
-                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-      set_target_properties(install-${LIB_PARENT_TARGET} PROPERTIES
-                            FOLDER "Compiler-RT Misc")
-      set_target_properties(install-${LIB_PARENT_TARGET}-stripped PROPERTIES
-                            FOLDER "Compiler-RT Misc")
-      add_dependencies(install-compiler-rt install-${LIB_PARENT_TARGET})
-      add_dependencies(install-compiler-rt-stripped install-${LIB_PARENT_TARGET}-stripped)
-    endif()
   endif()
 
   foreach(libname ${libnames})
     # If you are using a multi-configuration generator we don't generate
     # per-library install rules, so we fall back to the parent target COMPONENT
     if(CMAKE_CONFIGURATION_TYPES AND LIB_PARENT_TARGET)
       set(COMPONENT_OPTION COMPONENT ${LIB_PARENT_TARGET})
     else()
if(${type} STREQUAL "SHARED")
         add_custom_command(TARGET ${libname}
           POST_BUILD  
           COMMAND codesign --sign - $<TARGET_FILE:${libname}>
           WORKING_DIRECTORY ${COMPILER_RT_LIBRARY_OUTPUT_DIR}
         )
       endif()
     endif()
 
-    # We only want to generate per-library install targets if you aren't using
-    # an IDE because the extra targets get cluttered in IDEs.
-    if(NOT CMAKE_CONFIGURATION_TYPES)
-      add_custom_target(install-${libname}
-                        DEPENDS ${libname}
-                        COMMAND "${CMAKE_COMMAND}"
-                                -DCMAKE_INSTALL_COMPONENT=${libname}
-                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-      add_custom_target(install-${libname}-stripped
-                        DEPENDS ${libname}
-                        COMMAND "${CMAKE_COMMAND}"
-                                -DCMAKE_INSTALL_COMPONENT=${libname}
-                                -DCMAKE_INSTALL_DO_STRIP=1
-                                -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
-      # If you have a parent target specified, we bind the new install target
-      # to the parent install target.
-      if(LIB_PARENT_TARGET)
-        add_dependencies(install-${LIB_PARENT_TARGET} install-${libname})
-        add_dependencies(install-${LIB_PARENT_TARGET}-stripped install-${libname}-stripped)
-      endif()
+    set(parent_target_arg)
+      set(parent_target_arg PARENT_TARGET ${LIB_PARENT_TARGET})
     endif()
+    add_compiler_rt_install_targets(${libname} ${parent_target_arg})
+
     if(APPLE)
       set_target_properties(${libname} PROPERTIES
       OSX_ARCHITECTURES "${LIB_ARCHS_${libname}}")
     endif()
 
     if(type STREQUAL "SHARED")
       rt_externalize_debuginfo(${libname})
     endif()

compiler-rt/trunk/cmake/Modules/CompilerRTDarwinUtils.cmake

 include(CMakeParseArguments)
+include(CompilerRTUtils)
 
 # On OS X SDKs can be installed anywhere on the base system and xcode-select can
 # set the default Xcode to use. This function finds the SDKs that are present in
 # the current Xcode.
 function(find_darwin_sdk_dir var sdk_name)
   set(DARWIN_${sdk_name}_CACHED_SYSROOT "" CACHE STRING "Darwin SDK path for SDK ${sdk_name}.")
   set(DARWIN_PREFER_PUBLIC_SDK OFF CACHE BOOL "Prefer Darwin public SDK, even when an internal SDK is present.")
 
add_custom_command(OUTPUT ${LIB_OUTPUT_DIR}/lib${name}.a
       COMMAND ${CMAKE_COMMAND} -E make_directory ${LIB_OUTPUT_DIR}
       COMMAND lipo -output
               ${LIB_OUTPUT_DIR}/lib${name}.a
               -create ${LIB_LIPO_FLAGS}
       DEPENDS ${LIB_DEPENDS}
       )
     add_custom_target(${name}
       DEPENDS ${LIB_OUTPUT_DIR}/lib${name}.a)
+    set_target_properties(${name} PROPERTIES FOLDER "Compiler-RT Misc")
     add_dependencies(${LIB_PARENT_TARGET} ${name})
+
+    if(CMAKE_CONFIGURATION_TYPES)
+      set(install_component ${LIB_PARENT_TARGET})
+    else()
+      set(install_component ${name})
+    endif()
     install(FILES ${LIB_OUTPUT_DIR}/lib${name}.a
-      DESTINATION ${LIB_INSTALL_DIR})
-    set_target_properties(${name} PROPERTIES FOLDER "Compiler-RT Misc")
+      DESTINATION ${LIB_INSTALL_DIR}
+      COMPONENT ${install_component})
+    add_compiler_rt_install_targets(${name} PARENT_TARGET ${LIB_PARENT_TARGET})
   else()
     message(WARNING "Not generating lipo target for ${name} because no input libraries exist.")
   endif()
 endfunction()
 
 # Generates builtin libraries for all operating systems specified in ARGN. Each
 # OS library is constructed by lipo-ing together single-architecture libraries.
 macro(darwin_add_builtin_libraries)

compiler-rt/trunk/cmake/Modules/CompilerRTUtils.cmake

if (XCODE OR MSVC_IDE OR CMAKE_EXTRA_GENERATOR)
     list(LENGTH headers headers_length)
     if (${headers_length} GREATER 0)
       set_source_files_properties(${headers}
         PROPERTIES HEADER_FILE_ONLY ON)
     endif()
   endif()
   set("${OUTPUT_VAR}" ${sources} ${headers} PARENT_SCOPE)
 endfunction()
+
+# Create install targets for a library and its parent component (if specified).
+function(add_compiler_rt_install_targets name)
+  cmake_parse_arguments(ARG "" "PARENT_TARGET" "" ${ARGN})
+
+  if(ARG_PARENT_TARGET AND NOT TARGET install-${ARG_PARENT_TARGET})
+    # The parent install target specifies the parent component to scrape up
+    # anything not installed by the individual install targets, and to handle
+    # installation when running the multi-configuration generators.
+    add_custom_target(install-${ARG_PARENT_TARGET}
+                      DEPENDS ${ARG_PARENT_TARGET}
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=${ARG_PARENT_TARGET}
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+    add_custom_target(install-${ARG_PARENT_TARGET}-stripped
+                      DEPENDS ${ARG_PARENT_TARGET}
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=${ARG_PARENT_TARGET}
+                              -DCMAKE_INSTALL_DO_STRIP=1
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+    set_target_properties(install-${ARG_PARENT_TARGET} PROPERTIES
+                          FOLDER "Compiler-RT Misc")
+    set_target_properties(install-${ARG_PARENT_TARGET}-stripped PROPERTIES
+                          FOLDER "Compiler-RT Misc")
+    add_dependencies(install-compiler-rt install-${ARG_PARENT_TARGET})
+    add_dependencies(install-compiler-rt-stripped install-${ARG_PARENT_TARGET}-stripped)
+  endif()
+
+  # We only want to generate per-library install targets if you aren't using
+  # an IDE because the extra targets get cluttered in IDEs.
+  if(NOT CMAKE_CONFIGURATION_TYPES)
+    add_custom_target(install-${name}
+                      DEPENDS ${name}
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=${name}
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+    add_custom_target(install-${name}-stripped
+                      DEPENDS ${name}
+                      COMMAND "${CMAKE_COMMAND}"
+                              -DCMAKE_INSTALL_COMPONENT=${name}
+                              -DCMAKE_INSTALL_DO_STRIP=1
+                              -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+    # If you have a parent target specified, we bind the new install target
+    # to the parent install target.
+    if(LIB_PARENT_TARGET)
+      add_dependencies(install-${LIB_PARENT_TARGET} install-${name})
+      add_dependencies(install-${LIB_PARENT_TARGET}-stripped install-${name}-stripped)
+    endif()
+  endif()
+endfunction()