[cmake] Don't include symlinks to tools in Build-all when `LLVM_BUILD_TOOLS` is off

[zero9178]

Summary

When building LLVM with LLVM_BUILD_TOOLS as OFF, numerous tools such as llvm-ar or llvm-objcopy end up still being built. The reason for this is that the symlink targets are unconditionally included in a Build-all build, causing the tool they're symlinking to be built after all.

This patch changes that behaviour to be more intuative by only including including the symlink in a Build-all build if the target they're linking to is also included.

[Ericson2314]

I need to review more but I am suspicious of this exact fix before we are looking up a LLVM-specific env inside what I think is a reusable project also useable by downstream projects.

[zero9178]

In D132883#3756539, @Ericson2314 wrote:

I need to review more but I am suspicious of this exact fix before we are looking up a LLVM-specific env inside what I think is a reusable project also useable by downstream projects.

The code below sort of already makes this assumption, but I could change this patch and the line below to use ${project}_BUILD_TOOLS. That "enforces" common vocabulary while also respecting any other downstream projects making use of llvm_add_tool_symlink.

[smeenai]

Could we just read the EXCLUDE_FROM_ALL property of the target tool and use that to set the same property on the symlink target? We'd ideally use a generator expression for that so it works independently of the order the tool target and the symlink target are defined, but the CMake documentation makes it seem like EXCLUDE_FROM_ALL may only support generator expressions in 3.19 and above, whereas our minimum is 3.13.4. It's pretty conventional to always define the symlink target after the tool target though, so even just setting the property directly should work fine in practice.

[zero9178]

Change approach to querying EXCLUDE_FROM_ALL property of the target and using it to determine whether the resulting symlink should be included in a Build-all built.

[smeenai]

Thanks, LGTM

llvm/cmake/modules/AddLLVM.cmake

2139

Before committing, could you add a TODO comment to switch this to use a generator expression once our minimum required CMake version is >= 3.19?

llvm/cmake/modules/AddLLVM.cmake

if(ARG_ALWAYS_GENERATE)
     set_property(DIRECTORY APPEND PROPERTY
       ADDITIONAL_MAKE_CLEAN_FILES ${dest_binary})
     add_custom_command(TARGET ${target} POST_BUILD
       COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${dest_binary}" "${output_path}")
   else()
     add_custom_command(OUTPUT ${output_path}
                      COMMAND ${CMAKE_COMMAND} -E ${LLVM_LINK_OR_COPY} "${dest_binary}" "${output_path}"
                      DEPENDS ${target})
-    add_custom_target(${target_name} ALL DEPENDS ${target} ${output_path})
+
+    # TODO: Make use of generator expressions below once CMake 3.19 or higher is the minimum supported version.
+    set(should_build_all)
+    get_target_property(target_excluded_from_all ${target} EXCLUDE_FROM_ALL)
+    if (NOT target_excluded_from_all)
+      set(should_build_all ALL)
+    endif()
+    add_custom_target(${target_name} ${should_build_all} DEPENDS ${target} ${output_path})
     set_target_properties(${target_name} PROPERTIES FOLDER Tools)
 
     # Make sure both the link and target are toolchain tools
     if (${link_name} IN_LIST LLVM_TOOLCHAIN_TOOLS AND ${target} IN_LIST LLVM_TOOLCHAIN_TOOLS)
       set(TOOL_IS_TOOLCHAIN ON)
     endif()
 
     if ((TOOL_IS_TOOLCHAIN OR NOT LLVM_INSTALL_TOOLCHAIN_ONLY) AND LLVM_BUILD_TOOLS)