[PATCH] D28234: Support for custom install dirs in CMake build

[Milan Bouchet-Valat via Phabricator via llvm-commits]

nalimilan marked an inline comment as done.
nalimilan added inline comments.


================
Comment at: CMakeLists.txt:299
+    CACHE STRING "Define suffix of library directory name (32/64)" )
+set(CMAKE_INSTALL_BINDIR bin
+    CACHE STRING "Path for binary directory relative to CMAKE_INSTALL_PREFIX (defaults to 'bin')" )
----------------
beanz wrote:
> nalimilan wrote:
> > mgorny wrote:
> > > beanz wrote:
> > > > In keeping with our conventions these variables probably should start with `LLVM` instead of `CMAKE`.
> > > And anyway, why were you reinventing GNUInstallDirs? If we want the CMAKE* vars, I don't see a reason not to use the module completely. 
> > I'm not that familiar with CMake, so I don't really know how modules work.I thought I would propose the less invasive patch for a start.  But I can use GNUInstallDirs if you think that's the best approach.
> If you use the GNUInstallDirs module, then it makes sense to keep the `CMAKE_*` prefixes, however GNUInstallDirs may not play well with `LLVM_LIBDIR_SUFFIX`.
Yeah, so we cannot use GNUInstallDirs without breaking backward compatibility WRT `LLVM_LIBDIR_SUFFIX`. So I won't use it unless you're OK with dropping that variable.

Yet I suggest using the same names instead of adding an `LLVM` suffix: that's more discoverable for users; it doesn't really make sense to change the same of these standard variables for each project. In particular, Linux distributions offer convenience macros to build packages, which cannot work if variables don't follow a standard scheme.

Though e.g. Fedora currently uses a different convention, with `LIB_INSTALL_DIR`, `INCLUDE_INSTALL_DIR` and `SHARE_INSTALL_PREFIX`. I guess we could use that instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D28234