Page MenuHomePhabricator

[cmake] Don't add Support/Testing library if tests are not included
ClosedPublic

Authored by alan-baker on Jun 10 2019, 2:09 PM.

Details

Summary

Fixes the error:
CMake Error in <...>/llvm/cmake/modules/CMakeLists.txt:

export called with target "LLVMTestingSupport" which requires target
"gtest" that is not in the export set.

This occurs when LLVM is embedded in a larger project, but is configured not to include tests. If testing is disabled gtest isn't available and LLVM fails to configure.

Diff Detail

Repository
rL LLVM

Event Timeline

alan-baker created this revision.Jun 10 2019, 2:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 2:09 PM
tra added a subscriber: tra.Jun 17 2019, 10:40 AM
tra added inline comments.
lib/CMakeLists.txt
30 ↗(On Diff #203896)

Where does LLVM_INCLUDE_TESTING come from?
It should probably be an option() in llvm/CMakeLists.txt

Changed to use the top level LLVM build option.

alan-baker marked an inline comment as done.Jun 17 2019, 11:45 AM

LGTM, but I'm not familiar with the use of the stuff under Testing/. I've added @zturner who did some work there.

Friendly ping in case this fell off the radar.

jprice added a subscriber: jprice.Jul 5 2019, 1:41 PM
jprice added a comment.Jul 5 2019, 2:21 PM

FWIW I've just hit the same issue, and this patch works great for me.

I'm currently testing this patch. Will be committing after all tests are done.

lebedev.ri added inline comments.
lib/CMakeLists.txt
30 ↗(On Diff #203896)

if(LLVM_INCLUDE_TESTS) should work just fine too, and is cleaner.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 11 2019, 3:08 PM
Closed by commit rL365836: Fix build errors LLVM tests are disabled. (authored by dnovillo, committed by ). · Explain Why
This revision was automatically updated to reflect the committed changes.