[llvm] 7226e7f - [cmake] Loosen multi-distribution restrictions

[Shoaib Meenai via llvm-commits]

Author: Shoaib Meenai
Date: 2022-04-13T20:33:28-07:00
New Revision: 7226e7fd14c4e57f8ab5729990ee04bcdb1579af

URL: https://github.com/llvm/llvm-project/commit/7226e7fd14c4e57f8ab5729990ee04bcdb1579af
DIFF: https://github.com/llvm/llvm-project/commit/7226e7fd14c4e57f8ab5729990ee04bcdb1579af.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/cmake/modules/LLVMDistributionSupport.cmake
    llvm/docs/BuildingADistribution.rst

Removed: 
    


################################################################################
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 
diff erent 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 
diff erent 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 "<DEFAULT>")
@@ -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>_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>_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
+
diff erent 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.