[PATCH] D117973: [cmake] Support custom package install paths

John Ericson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 25 08:57:37 PDT 2022


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


================
Comment at: cmake/Modules/FindPrefixFromConfig.cmake:30
+  if(IS_ABSOLUTE "${path_to_leave}")
+    set(prefix_var
+      "# Installation prefix is fixed absolute path"
----------------
sebastian-ne wrote:
> Shouldn’t this set `config_code` instead of `prefix_var`, which is the output variable in the generated code?
Yes! Silly mistake me cleaning the code up for upstreaming but not yet testing it with our configuration again. Thanks for noticing.


================
Comment at: cmake/Modules/FindPrefixFromConfig.cmake:32
+      "# Installation prefix is fixed absolute path"
+      "set(${prefix_var} \"${CMAKE_INSTALL_PREFIX}\"")
+  else()
----------------
sebastian-ne wrote:
> This could use a comment to why it ignores `path_to_leave` and why `CMAKE_INSTALL_PREFIX` is the correct choice.
Added comments to both branches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117973



More information about the cfe-commits mailing list