[libc-commits] [PATCH] D99484: Use `GNUInstallDirs` to support custom installation dirs.
John Ericson via Phabricator via libc-commits
libc-commits at lists.llvm.org
Fri Apr 30 08:48:34 PDT 2021
Ericson2314 added inline comments.
Comment at: compiler-rt/cmake/base-config-ix.cmake:72
+ set(COMPILER_RT_INSTALL_PATH "" CACHE PATH
+ "Prefix where built compiler-rt artifacts should be installed, comes before CMAKE_INSTALL_PREFIX.")
option(COMPILER_RT_INCLUDE_TESTS "Generate and build compiler-rt unit tests." OFF)
> compnerd wrote:
> > Ericson2314 wrote:
> > > compnerd wrote:
> > > > Please don't change the descriptions of the options as part of the `GNUInstallDirs` handling. The change to `COMPILER_RT_INSTALL_PATH` looks incorrect. Can you explain this change please?
> > > I tried explain this https://reviews.llvm.org/D99484#2655885 and the original description about prefixes. The GNU install dir variables are allowed to be absolute paths (and not necessary with the installation prefix) (and I needed that for the NixOS patch), so always prepending `COMPILER_RT_INSTALL_PATH` as is done doesn't work if that is `CMAKE_INSTALL_PREFIX` by default.
> > >
> > > If you do `git grep '_PREFIX ""'` and also `git grep GRPC_INSTALL_PATH` you will see that many similar variables also were already empty by default. I agree this thing is a bit ugly, but by that argument I am not doing a new hack for `GNUInstallDIrs` but rather applying an existing ideom consistently in all packages.
> > Sure, but the *descriptions* of the options are needed to changed for changing the value.
> > That said, I'm not convinced that this change is okay. It changes the way that compiler-rt can be built and used when building a toolchain image. The handling of the install prefix in compiler-rt is significantly different from the other parts of LLVM, and so, I think that it may need to be done as a separate larger effort.
> So just skip everything compile-rt related for now?
compile-rt is skipped in the latest version, and I have an independent https://reviews.llvm.org/D101497 laying the groundwork for a future `GNUInstallDirs`-adding patch to do the regular thing.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the libc-commits