This is an archive of the discontinued LLVM Phabricator instance.

[lldb/test] Reduce API test tools configuration boilerplate
ClosedPublic

Authored by labath on Jan 22 2021, 12:54 PM.

Details

Summary

Replace the dotest command line options and various cmake variables,
which are used for passing the locations of llvm tools to the API tests
with a single variable, which points to the directory these tools are
placed in. Besides reducing repetition, this also makes things more
similar to how "normal" llvm tests are configured.

Diff Detail

Event Timeline

labath created this revision.Jan 22 2021, 12:54 PM
labath requested review of this revision.Jan 22 2021, 12:54 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 22 2021, 12:54 PM

I should add, this also ensures one does not need to introduce a similar amount of cruft for each new llvm tool that he wants to use in the tests.

lldb/test/API/CMakeLists.txt
59

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 (?)

JDevlieghere added inline comments.Jan 22 2021, 1:04 PM
lldb/test/API/CMakeLists.txt
59

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.

lldb/utils/lldb-dotest/CMakeLists.txt
92

I think here you just want to set it to LLVM_TOOLS_BINARY_DIR, right?

labath added inline comments.Jan 24 2021, 12:36 PM
lldb/test/API/CMakeLists.txt
59

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...

lldb/utils/lldb-dotest/CMakeLists.txt
92

Yes, oops. Thanks for catching that.

labath updated this revision to Diff 318862.Jan 24 2021, 12:54 PM

Keep dsymutil, fix typo.

lldb/utils/lldb-dotest/CMakeLists.txt
92

You also need to add a line for LLDB_TEST_DSYMUTIL_CONFIGURED now that it is back

labath updated this revision to Diff 321412.Feb 4 2021, 6:23 AM

Add back dsymutil in one more place

JDevlieghere accepted this revision.Feb 4 2021, 8:39 AM

I think you missed one LLVM_TOOLS_DIR_CONFIGURED but other than that this LGTM.

lldb/utils/lldb-dotest/CMakeLists.txt
29

Do you need to add LLVM_TOOLS_DIR_CONFIGURED here too?

This revision is now accepted and ready to land.Feb 4 2021, 8:39 AM
stella.stamenova accepted this revision.Feb 4 2021, 9:02 AM
labath added inline comments.Feb 5 2021, 12:02 AM
lldb/utils/lldb-dotest/CMakeLists.txt
29

I am assuming that LLVM_TOOLS_DIR can never point to the lldb build tree (in a standalone build). Though, with D96034, this point is moot.