Page MenuHomePhabricator

[CMake] Add special case for processing LLDB_DOTEST_ARGS
ClosedPublic

Authored by sgraenitz on Jun 4 2019, 7:51 AM.

Details

Summary

Allow to run the test suite when building LLDB Standalone with Xcode against a provided LLVM build-tree that used a single-configuration generator like Ninja.

So far both test drivers, lit-based check-lldb as well as lldb-dotest, were looking for test dependencies (clang/dsymutil/etc.) in a subdirectory with the configuration name (Debug/Release/etc.). It was implicitly assuming that both, LLDB and the provided LLVM used the same generator. In practice, however, the opposite is quite common: build the dependencies with Ninja and LLDB with Xcode for development*. With this patch it becomes the default.

(* In fact, it turned out that the Xcode<->Xcode variant didn't even build out of the box. It's fixed since D62879)

Once this is sound, I'm planning the following steps:

  • add stage to the lldb-cmake-standalone bot to build and test it
  • update Building LLDB with Xcode section in the docs
  • bring the same mechanism to swift-lldb
  • fade out the manually maintained Xcode project

On macOS build and test like this:

$ git clone https://github.com/llvm/llvm-project.git /path/to/llvm-project

$ cd /path/to/lldb-dev-deps
$ cmake -GNinja -C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake -DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi" /path/to/llvm-project/llvm
$ ninja

$ cd /path/to/lldb-dev-xcode
$ cmake -GXcode -C/path/to/llvm-project/lldb/cmake/caches/Apple-lldb-macOS.cmake -DLLDB_BUILD_FRAMEWORK=Off -DLLVM_DIR=/path/to/lldb-dev-deps/lib/cmake/llvm -DClang_DIR=/path/to/lldb-dev-deps/lib/cmake/clang /path/to/llvm-project/lldb
$ xcodebuild -configuration Debug -target check-lldb
$ xcodebuild -configuration Debug -target lldb-dotest
$ ./Debug/bin/lldb-dotest

Event Timeline

sgraenitz created this revision.Jun 4 2019, 7:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2019, 7:51 AM
Herald added a subscriber: mgorny. · View Herald Transcript
stella.stamenova requested changes to this revision.Jun 4 2019, 9:52 AM

I don't think that this would apply very well to Visual Studio with the current change because it is likely that if you are using VS to build LLDB, you also used VS to build the dependencies. Even for xcode, I am sure that there are people who build the dependencies standalone with xcode and then are building LLDB standalone with xcode, so any change that absolutely treats the scenario as one or the other is going to break someone.

I think a better change would be to allow a parameter to be passed like your suggested LLDB_PROVIDED_NINJA_BUILD_TREE=On (except don't use Ninja in the name, how the tree came about is not important and it could also be make or a tree copied from another build) and then use that and LLDB_BUILT_STANDALONE for the pivot. Then if someone provides the new parameter, they know what they are doing and we can expect it to work for Xcode and VS both.

This revision now requires changes to proceed.Jun 4 2019, 9:52 AM
labath added a subscriber: labath.EditedJun 4 2019, 11:24 AM

LLDB_PROVIDED_NINJA_BUILD_TREE=On

An even better option would be to get the llvm build to export the fact whether it was built with a multi-config generator or not. Then the user wouldn't have to provide anything, and things would "just work". :)

Thanks for your feedback. I should have mentioned that this is pretty much a draft. For now I was happy it works at all.

I don't think that this would apply very well to Visual Studio with the current change because it is likely that if you are using VS to build LLDB, you also used VS to build the dependencies.

Good point

I think a better change would be to allow a parameter to be passed like your suggested LLDB_PROVIDED_NINJA_BUILD_TREE=On (except don't use Ninja in the name, how the tree came about is not important and it could also be make or a tree copied from another build) and then use that and LLDB_BUILT_STANDALONE for the pivot. Then if someone provides the new parameter, they know what they are doing and we can expect it to work for Xcode and VS both.

Yes, then it's either that or what Pavel proposed.

LLDB_PROVIDED_NINJA_BUILD_TREE=On

An even better option would be to get the llvm build to export the fact whether it was built with a multi-config generator or not. Then the user wouldn't have to provide anything, and things would "just work". :)

Yes, I will check if I can store it in LLVMConfig.cmake

sgraenitz updated this revision to Diff 203016.Jun 4 2019, 1:54 PM

Chose the replacement for remaining matches of CMAKE_CFG_INTDIR depdending on LLVM_CONFIGURATION_TYPES

lldb/lit/CMakeLists.txt
13

It looks like you need the update here also. Is there a way to share the logic between the two cmake files?

You probably no longer need the xcode pivot either, right?

sgraenitz updated this revision to Diff 203020.Jun 4 2019, 2:06 PM

Patch the replacement logic also in lit/CMakeLists.txt

sgraenitz marked an inline comment as done.Jun 4 2019, 2:10 PM

After figuring out D62879 that came up on the way, this wasn't too complicated indeed. I put the patch for exporting LLVM_CONFIGURATION_TYPES here: D62878

lldb/lit/CMakeLists.txt
13

Thanks for catching this one! Ok deal, I remove the Xcode restriction and you run a test with VS? ;)

sgraenitz marked an inline comment as done.Jun 4 2019, 2:10 PM
stella.stamenova accepted this revision.Jun 4 2019, 2:14 PM
stella.stamenova added inline comments.
lldb/lit/CMakeLists.txt
13

I can run some tests with VS, probably tomorrow though.

This revision is now accepted and ready to land.Jun 4 2019, 2:14 PM
sgraenitz updated this revision to Diff 203106.Jun 5 2019, 2:09 AM

Remove Xcode restriction and polish

sgraenitz marked 2 inline comments as done.Jun 5 2019, 2:24 AM
sgraenitz added inline comments.
lldb/lit/CMakeLists.txt
7

I will have to fix this. We only need one match here.

Is there a way to share the logic between the two cmake files?

Unfortunately they're slightly different. lldb-dotest generates the actual driver scripts. Here, we only write the (one) cfg file and it relies on replacements of %(build_mode)s at runtime in ( see https://github.com/llvm/llvm-project/blob/6fc4c1cc54/lldb/lit/Suite/lit.site.cfg.in#L35).

sgraenitz updated this revision to Diff 203118.Jun 5 2019, 4:16 AM

Unfortunately lit is a little more complicated

sgraenitz edited the summary of this revision. (Show Details)Jun 5 2019, 4:23 AM
sgraenitz updated this revision to Diff 203121.Jun 5 2019, 4:28 AM

Polishing

This comment was removed by sgraenitz.
Closed by commit rL362803: [CMake] Add special case for processing LLDB_DOTEST_ARGS (authored by stefan.graenitz, committed by ). · Explain WhyJun 7 2019, 7:31 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2019, 7:31 AM