[runtimes] Handle interface targets in runtimes distribution components

[ldionne]

Summary

As reported in [1], cxx-headers is not a valid distribution target
because it is an interface target in CMake. This breaks the most
basic MultiDistributionExample of the runtimes build.

This patch handles interface targets by getting rid of the assumption
that all distribution components have a target associated to them. It
is valid for a distribution component to only have a install-FOO
target.

In the case where only cxx-headers is provided as a distribution
component, ninja toolchain-distribution will not build anything
after this patch, since there is effectively nothing to build for
the cxx-headers target. However, running ninja install-toolchain-distribution
will build everything, as expected.

[1]: https://discord.com/channels/636084430946959380/636732894974312448/1010013287464828968

[phosek]

LGTM

[JamesNagurne]

Hey thanks for looking into this @ldionne

Maybe I'm missing some scope here (I'm very much not an expert on LLVMDistributionSupport), but is the intent here that not every component has a build, but every component should have an install (that may depend on a build, but also might not)?

If that's the case, then this looks good, and is a fairly simple and elegant solution, as opposed to my force-a-build-target solution I'm doing downstream at the moment.

[ldionne]

In D132387#3741080, @JamesNagurne wrote:

Hey thanks for looking into this @ldionne

Maybe I'm missing some scope here (I'm very much not an expert on LLVMDistributionSupport), but is the intent here that not every component has a build, but every component should have an install (that may depend on a build, but also might not)?

If that's the case, then this looks good, and is a fairly simple and elegant solution, as opposed to my force-a-build-target solution I'm doing downstream at the moment.

Yeah, basically, I see build targets as being implementation details of the build. What folks should really care about is having correct dependencies between install-FOO targets. If you build install-cxx-headers, then *that* needs to do the right thing, but who cares whether cxx-headers even exists? The fact that it's an interface (or not) target should not matter. This patch attempts to implement that point of view.

llvm/cmake/modules/LLVMDistributionSupport.cmake

else()
       set(distribution_components ${LLVM_${distribution}_DISTRIBUTION_COMPONENTS})
     endif()
 
     add_custom_target(${distribution_target})
     add_custom_target(install-${distribution_target})
     add_custom_target(install-${distribution_target}-stripped)
 
     foreach(target ${distribution_components})
+      # Note that some distribution components may not have an actual target, but only an install-FOO target.
+      # This happens for example if a target is an INTERFACE target.
       if(TARGET ${target})
         add_dependencies(${distribution_target} ${target})
-      else()
-        message(SEND_ERROR "Specified distribution component '${target}' doesn't have a target")
       endif()
 
       if(TARGET install-${target})
         add_dependencies(install-${distribution_target} install-${target})
       else()
         message(SEND_ERROR "Specified distribution component '${target}' doesn't have an install target")
       endif()
 

llvm/runtimes/CMakeLists.txt

list(APPEND extra_targets
       install-${runtime_name}
       install-${runtime_name}-stripped)
     if(LLVM_INCLUDE_TESTS)
       list(APPEND test_targets check-${runtime_name})
     endif()
   endforeach()
   foreach(component ${LLVM_RUNTIME_DISTRIBUTION_COMPONENTS})
     if(NOT ${component} IN_LIST SUB_COMPONENTS)
-      list(APPEND extra_targets ${component} install-${component} install-${component}-stripped)
+      list(APPEND extra_targets install-${component} install-${component}-stripped)
     endif()
   endforeach()
 
   if(LLVM_INCLUDE_TESTS)
     include(${LLVM_BINARY_DIR}/runtimes/Tests.cmake OPTIONAL RESULT_VARIABLE have_tests)
     set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS ${LLVM_BINARY_DIR}/runtimes/Tests.cmake)
     if(have_tests)
       set_property(GLOBAL APPEND PROPERTY LLVM_ALL_LIT_TESTSUITES ${SUB_LIT_TESTSUITES})