This is an archive of the discontinued LLVM Phabricator instance.

Differential D63097

[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

Reviewers
chandlerc
zturner
Commits
rGa35a7d49e5e9: Fix build errors LLVM tests are disabled.
rL365836: Fix build errors LLVM tests are disabled.
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
Herald added subscribers: llvm-commits, mgorny. · View Herald Transcript
tra added a subscriber: tra.Jun 17 2019, 10:40 AM
tra added inline comments.
lib/CMakeLists.txt
30

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

alan-baker updated this revision to Diff 205137.Jun 17 2019, 11:44 AM

Changed to use the top level LLVM build option.

alan-baker marked an inline comment as done.Jun 17 2019, 11:45 AM
tra added a reviewer: zturner.Jun 17 2019, 12:15 PM

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

alan-baker added a comment.Jun 26 2019, 6:07 AM

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.

MaskRay added a subscriber: MaskRay.Jul 10 2019, 1:27 AM

LG

dnovillo added a subscriber: dnovillo.Jul 11 2019, 1:42 PM

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

lebedev.ri added a subscriber: lebedev.ri.Jul 11 2019, 1:59 PM
lebedev.ri added inline comments.
lib/CMakeLists.txt
30

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). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
lib/
4 lines

Diff 205137

lib/CMakeLists.txt

Context not available.
add_subdirectory(TextAPI) add_subdirectory(TextAPI)
add_subdirectory(ToolDrivers) add_subdirectory(ToolDrivers)
add_subdirectory(XRay) add_subdirectory(XRay)
add_subdirectory(Testing) if(${LLVM_INCLUDE_TESTS})
traUnsubmitted
Done
ReplyInline Actions

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

tra: Where does LLVM_INCLUDE_TESTING come from? It should probably be an option() in llvm/CMakeLists.
lebedev.riUnsubmitted
Not Done
ReplyInline Actions

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

lebedev.ri: `if(LLVM_INCLUDE_TESTS)` should work just fine too, and is cleaner.
add_subdirectory(Testing)
endif()
add_subdirectory(WindowsManifest) add_subdirectory(WindowsManifest)
Context not available.