This is an archive of the discontinued LLVM Phabricator instance.

Bug 19462 - Use the INSTALL(EXPORT ...) to export CMake definitions
ClosedPublic

Authored by beanz on Feb 13 2015, 1:58 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

beanz updated this revision to Diff 19930.Feb 13 2015, 1:58 PM
beanz retitled this revision from to Bug 19462 - Use the INSTALL(EXPORT ...) to export CMake definitions.
beanz updated this object.
beanz edited the test plan for this revision. (Show Details)
beanz added a reviewer: rnk.
beanz added a subscriber: Unknown Object (MLST).
beanz added a comment.May 28 2015, 8:18 AM

Stephen Kelly's comments in the bug (https://llvm.org/bugs/show_bug.cgi?id=19462) from 2015-02-25 indicate that these patches are fine as-is.

He also recommends we may want to consider namespacing our LLVM and Clang targets in our exported target files. According to the CMake documentation (http://www.cmake.org/cmake/help/v3.0/manual/cmake-packages.7.html):

"A NAMESPACE with double-colons is specified when exporting the targets for installation. This convention of double-colons gives CMake a hint that the name is an IMPORTED target when it is used by downstreams with the target_link_libraries() command. This way, CMake can issue a diagnostic if the package providing it has not yet been found."

This might be desirable, however it will require more testing.

Does anyone have any objection to these patches as they are now?

rnk accepted this revision.May 28 2015, 3:13 PM
rnk edited edge metadata.

lgtm, let's do it, I think this just got lost for a bit.

This revision is now accepted and ready to land.May 28 2015, 3:13 PM
This revision was automatically updated to reflect the committed changes.