|Show All 16 Lines|
|if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )||if ( CMAKE_SYSTEM_NAME MATCHES "Windows" )|
|add_definitions( -DIMPORT_LIBLLDB )||add_definitions( -DIMPORT_LIBLLDB )|
Would it make sense/work to do the check inside this function?
JDevlieghere: Would it make sense/work to do the check inside this function?
Yes would be possible and it would save some lines of code, but this way it's more visible that it's a very special use-case and shouldn't be copy/pasted into each and every tool. For the general cases the default RPATHs from LLVM are exactly what is necessary. I have the impression that there was some confusing about RPATHs in this code before.
I thought about the alternative approach to query the dependencies for each target in lldb_add_executable and apply the RPATH settings to those targets that depend on liblldb (if LLDB_BUILD_FRAMEWORK). However, it feels like a bit too much magic under the hood and also it goes a little bit too far in the direction of building our own dependency management in CMake, which is not what CMake is made for.
I made the function name a little bit more explicit and extended the documentation. I would keep it like this now. What do you think?
sgraenitz: Yes would be possible and it would save some lines of code, but this way it's more visible that…