[PATCH] D89177: [cmake] Add support for multiple distributions

Fri Nov 6 14:57:02 PST 2020

phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits.

================
Comment at: clang/cmake/modules/AddClang.cmake:121
+          set(export_to_clangtargets EXPORT Clang${distribution}Targets)
+          set_property(GLOBAL PROPERTY CLANG${distribution}_HAS_EXPORTS True)
         endif()
----------------
Should we make this uppercase and join it with `_` to match the existing conventions? For example, you'd have `ClangToolchainTargets` and `CLANG_TOOLCHAIN_HAS_EXPORTS`. With the change as is, we would end up with `CLANGToolchain_HAS_EXPORT` which is a bit unusual.

================
Comment at: lld/cmake/modules/LLDConfig.cmake.in:13
 # Provide all our library targets to users.
-include("@LLD_CONFIG_EXPORTS_FILE@")
+ at lld_config_include_exports@
----------------
We use upper-case variables for the `*_CONFIG_CODE` so using lower case here is a bit unfortunate, could we use upper case for these variables?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89177/new/

https://reviews.llvm.org/D89177