[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

[John Ericson via Phabricator via cfe-commits]

Ericson2314 marked 4 inline comments as done.
Ericson2314 added inline comments.


================
Comment at: llvm/CMakeLists.txt:75
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
-	"Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or \"all\".")
+   "Semicolon-separated list of projects to build (${LLVM_KNOWN_PROJECTS}), or \"all\".")
 foreach(proj ${LLVM_ENABLE_PROJECTS})
----------------
mstorsjo wrote:
> Nit: This looks like a spurious unrelated change to whitespace?
Doing in D116510 instead.


================
Comment at: llvm/CMakeLists.txt:164
       set(LLVM_CCACHE_MAXSIZE "" CACHE STRING "Size of ccache")
-      set(LLVM_CCACHE_DIR "" CACHE STRING "Directory to keep ccached data")
+      set(LLVM_CCACHE_DIR "" CACHE PATH "Directory to keep ccached data")
       set(LLVM_CCACHE_PARAMS "CCACHE_CPP2=yes CCACHE_HASHDIR=yes"
----------------
mstorsjo wrote:
> Could this bit be split out to separate change, to keep this as small as possible - unless it's strictly needed and tied to this one?
Doing that, I will post the revision later as it is not that important.


================
Comment at: llvm/CMakeLists.txt:294
 
-set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE STRING
+set(LLVM_UTILS_INSTALL_DIR "${LLVM_TOOLS_INSTALL_DIR}" CACHE PATH
     "Path to install LLVM utilities (enabled by LLVM_INSTALL_UTILS=ON) (defaults to LLVM_TOOLS_INSTALL_DIR)")
----------------
mstorsjo wrote:
> Nit: Unrelated and can be split out?
Yes, will do later.


================
Comment at: llvm/CMakeLists.txt:408
 
-set(LLVM_Z3_INSTALL_DIR "" CACHE STRING "Install directory of the Z3 solver.")
+set(LLVM_Z3_INSTALL_DIR "" CACHE PATH "Install directory of the Z3 solver.")
 
----------------
mstorsjo wrote:
> Unrelated?
Yes, see above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100810