From 7226e7fd14c4e57f8ab5729990ee04bcdb1579af Mon Sep 17 00:00:00 2001 From: Shoaib Meenai Date: Wed, 13 Apr 2022 18:28:27 -0700 Subject: [PATCH] [cmake] Loosen multi-distribution restrictions We've found that there are cases where it's useful to be able to include the same target in multiple distributions (e.g. if you want a distribution that's a superset of another distribution, for convenience purposes), and that there are cases where the distribution of a target and its umbrella can legitimately differ (e.g. the LTO library would commonly be distributed alongside your tools, but it also falls under the llvm-libraries umbrella, which would commonly be distributed separately). Relax the restrictions while providing an option to restore them (which is mostly useful to ensure you aren't accidentally placing targets in the wrong distributions). There could be further refinements here (e.g. excluding a target from an umbrella if it's explicitly included in some other distribution, or having variables to control which targets are allowed to be duplicated or placed in a separate distribution than their umbrellas), but we can punt on those until there's an actual need. --- .../modules/LLVMDistributionSupport.cmake | 41 +++++++++++++++---- llvm/docs/BuildingADistribution.rst | 13 +++++- 2 files changed, 44 insertions(+), 10 deletions(-) diff --git a/llvm/cmake/modules/LLVMDistributionSupport.cmake b/llvm/cmake/modules/LLVMDistributionSupport.cmake index e1c9a6cce5e67..a2fe69df1e7e6 100644 --- a/llvm/cmake/modules/LLVMDistributionSupport.cmake +++ b/llvm/cmake/modules/LLVMDistributionSupport.cmake @@ -41,11 +41,17 @@ function(llvm_distribution_build_target_map) foreach(distribution ${LLVM_DISTRIBUTIONS}) foreach(target ${LLVM_${distribution}_DISTRIBUTION_COMPONENTS}) - # We don't allow a target to be in multiple distributions, because we - # wouldn't know which export set to place it in. - get_property(current_distribution GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target}) - if(current_distribution AND NOT current_distribution STREQUAL distribution) - message(SEND_ERROR "Target ${target} cannot be in multiple distributions ${distribution} and ${current_distribution}") + # By default, we allow a target to be in multiple distributions, and use + # the last one to determine its export set. We disallow this in strict + # mode, emitting a single error at the end for readability. + if(LLVM_STRICT_DISTRIBUTIONS) + get_property(current_distribution GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target}) + if(current_distribution AND NOT current_distribution STREQUAL distribution) + set_property(GLOBAL APPEND_STRING PROPERTY LLVM_DISTRIBUTION_ERRORS + "Target ${target} cannot be in multiple distributions \ + ${distribution} and ${current_distribution}\n" + ) + endif() endif() set_property(GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target} ${distribution}) endforeach() @@ -78,13 +84,18 @@ function(get_llvm_distribution target in_distribution_var distribution_var) get_property(distribution GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${target}) if(ARG_UMBRELLA) get_property(umbrella_distribution GLOBAL PROPERTY LLVM_DISTRIBUTION_FOR_${ARG_UMBRELLA}) - if(distribution AND umbrella_distribution AND NOT distribution STREQUAL umbrella_distribution) - message(SEND_ERROR "Target ${target} has different distribution ${distribution} from its" - " umbrella target ${ARG_UMBRELLA} distribution ${umbrella_distribution}") - elseif(NOT distribution) + if(LLVM_STRICT_DISTRIBUTIONS AND distribution AND umbrella_distribution AND + NOT distribution STREQUAL umbrella_distribution) + set_property(GLOBAL APPEND_STRING PROPERTY LLVM_DISTRIBUTION_ERRORS + "Target ${target} has different distribution ${distribution} from its \ + umbrella target's (${ARG_UMBRELLA}) distribution ${umbrella_distribution}\n" + ) + endif() + if(NOT distribution) set(distribution ${umbrella_distribution}) endif() endif() + if(distribution) set(${in_distribution_var} YES PARENT_SCOPE) if(distribution STREQUAL "") @@ -212,6 +223,18 @@ endfunction() # where ${distribution} is the distribution name in lowercase, or "distribution" # for the default distribution. function(llvm_distribution_add_targets) + # This function is called towards the end of LLVM's CMakeLists.txt, so all + # errors will have been seen by now. + if(LLVM_STRICT_DISTRIBUTIONS) + get_property(errors GLOBAL PROPERTY LLVM_DISTRIBUTION_ERRORS) + if(errors) + string(PREPEND errors + "Strict distribution errors (turn off LLVM_STRICT_DISTRIBUTIONS to bypass):\n" + ) + message(FATAL_ERROR "${errors}") + endif() + endif() + set(distributions "${LLVM_DISTRIBUTIONS}") if(NOT distributions) # CMake seemingly doesn't distinguish between an empty list and a list diff --git a/llvm/docs/BuildingADistribution.rst b/llvm/docs/BuildingADistribution.rst index e52e55cb996c0..d2f81f6fc3d22 100644 --- a/llvm/docs/BuildingADistribution.rst +++ b/llvm/docs/BuildingADistribution.rst @@ -102,7 +102,7 @@ then setting the *LLVM__DISTRIBUTION_COMPONENTS* variable to the list of targets for that distribution. For each distribution, the build system generates an ``install-${distribution}-distribution`` target, where ``${distribution}`` is the name of the distribution in lowercase, to install -that distribution. Each target can only be in one distribution. +that distribution. Each distribution creates its own set of CMake exports, and the target to install the CMake exports for a particular distribution for a project is named @@ -118,6 +118,17 @@ any components specified in *LLVM_RUNTIME_DISTRIBUTION_COMPONENTS* are not automatically added to any distribution. Instead, you must include the targets explicitly in some *LLVM__DISTRIBUTION_COMPONENTS* list. +By default, each target can appear in multiple distributions; a target will be +installed as part of all distributions it appears in, and it'll be exported by +the last distribution it appears in (the order of distributions is the order +they appear in *LLVM_DISTRIBUTIONS*). We also define some umbrella targets (e.g. +``llvm-libraries`` to install all LLVM libraries); a target can appear in a +different distribution than its umbrella, in which case the target will be +exported by the distribution it appears in (and not the distribution its +umbrella appears in). Set *LLVM_STRICT_DISTRIBUTIONS* to ``On`` if you want to +enforce a target appearing in only one distribution and umbrella distributions +being consistent with target distributions. + We strongly encourage looking at ``clang/cmake/caches/MultiDistributionExample.cmake`` as an example of configuring multiple distributions.