serge-sans-paille added inline comments.

================
Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {
----------------
Ericson2314 wrote:
> mgorny wrote:
> > Well, one thing I immediately notice is that technically 
> > `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations you 
> > are assuming it will always be relative. Not saying that's a blocker but I 
> > think you should add checks for that into `CMakeLists.txt`.
> Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` with an 
> absolute path.
> 
> OK if this isn't supported initially but we should ovoid regressing where 
> possible.
Turns out LLVM_LIBDIR_SUFFIX is used in several situations: (a) for the library 
install dir or (b) for the location where build artifact as stored in 
CMAKE_BINARY_DIR. (a) could accept a path but not (b), unless we derive it from 
(a) but I can see a lot of complication there for the testing step. Would that 
be ok to choke on CMAKE_INSTALL_LIBDIR being a path and not a directory 
component?



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

https://reviews.llvm.org/D137337

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to