[Lldb-commits] [PATCH] D95261: [lldb/test] Reduce API test tools configuration boilerplate

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/test/API/CMakeLists.txt:59
 set(LLDB_TEST_COMPILER "${LLDB_DEFAULT_TEST_COMPILER}" CACHE PATH "C Compiler to use for building LLDB test inferiors")
-set(LLDB_TEST_DSYMUTIL "${LLDB_DEFAULT_TEST_DSYMUTIL}" CACHE PATH "dsymutil used for generating dSYM bundles")
-set(LLDB_TEST_FILECHECK "${LLDB_DEFAULT_TEST_FILECHECK}" CACHE PATH "FileCheck used for testing purposes")
----------------
JDevlieghere wrote:
> labath wrote:
> > Dsymutil is in compilation of inferiors, so it may make sense to leave it individually configurable, like we have it for the compiler, but I don't know if that would actually be useful (?)
> Yeah, I recently did some work to make it possible to run the test suite against an Xcode installation, which contains a `dsymutil` but no `FileCheck` or `yaml2obj`. To keep that working, I need a way to configure the latter two separately. To me it doesn't really matter whether that's by setting the tools dir and having dsymutil specified separately, but that one way this would continue working. 
Ok, cool. I'll leave the dsymutil as a separate setting then.

While we're on this subject, what do you think about the `LLDB_TEST_SERVER` variable? Do we need that, or could we find it via `SBHostOS::GetLLDBPath(ePathTypeSupportExecutableDir)` ? The real problem I'm trying to solve is the inability to reference both lldb-server and debugserver from the test suite (on darwin we have both), and I'd hate to introduce separate variables for both...


================
Comment at: lldb/utils/lldb-dotest/CMakeLists.txt:87
   set(LLDB_LIBS_DIR_CONFIGURED "${LLDB_LIBS_DIR}")
+  set(LLVM_TOOLS_DIR_CONFIGURED "${LLVM_TOOLS_DIR_CONFIGURED}")
 
----------------
stella.stamenova wrote:
> I think here you just want to set it to LLVM_TOOLS_BINARY_DIR, right?
Yes, oops. Thanks for catching that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95261