stella.stamenova added inline comments.
================
Comment at: lit/CMakeLists.txt:10-13
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_C_COMPILER
${LLDB_TEST_C_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TEST_CXX_COMPILER
${LLDB_TEST_CXX_COMPILER})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_LIBS_DIR
${LLVM_LIBRARY_OUTPUT_INTDIR})
+string(REPLACE ${CMAKE_CFG_INTDIR} ${LLVM_BUILD_MODE} LLDB_TOOLS_DIR
${LLVM_RUNTIME_OUTPUT_INTDIR})
----------------
zturner wrote:
> This only works if you're using a just-built clang, which might not be the
> case. In fact, it's usually not the case, because it's common to want to run
> the test suite against a debug build of lldb but using a release build of
> clang (otherwise you'll be there all day waiting for it to finish).
>
> I feel like if the user specifies an absolute path to the test compiler on
> the command line, that should be what it uses -- always. If we want to use a
> just built toolchain, maybe we need something else, like
> `-DLLDB_TEST_BUILT_CLANG=ON`, which would trigger this logic.
>
> As I don't use this configuration though, I'm interested in hearing your
> thoughts.
It actually does work in the case when a user specifies a compiler on the
command line as well as when the just-built clang is used and the default today
is to use the just-built clang.
As far as I can tell, you can specify the compiler with LLDB_TEST_C_COMPILER
(or LLDB_TEST_CXX_COMPILER) when calling CMake or by passing a value to the
tests directly with -C. I assume that you are concerned about the first case -
when passing the property to CMake?
In that case LLDB_TEST_C_COMPILER is set to /path/to/compiler and these lines
won't actually affect it - unless for some reason the path contained
${CMAKE_CFG_INTDIR}. If ${CMAKE_CFG_INTDIR} is a period, which is the likely
scenario, it will just be replaced by another period since the build mode would
be the same.
The only case when it might not work is if ${CMAKE_CFG_INTDIR} is in the path
but not a period - but that is unlikely since the scenario would mean that
${CMAKE_CFG_INTDIR} is, say, $Configuration and the path that the user
specified contains $Configuration AND it is different than the one they're
using for LLDB.
On the other hand, without this change it is not possible to use the just-built
compiler when using Visual Studio, for example, because the path to the just
built compiler is not being set correctly and this particular substitution
needs to happen here.
The way to make sure that both scenarios work always is to guard against the
substitution when the user explicitly specifies a path or to enforce the
sibstitution when they're using the just-built clang so a property like
LLDB_TEST_BUILD_CLANG would work. If you think that the error scenario is
likely enough, then we should add the property.
https://reviews.llvm.org/D43096
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
