[PATCH] Adds a CMake package configuration file for libclang

[Damien Buhl]

On 03/07/2013 01:17 AM, Manuel Klimek wrote:
> No, I meant: if that's what cmake integration should look like; that's
> why I pulled in Brad, who was nice enough to put in a lot of feedback,
> so I can basically say: please address Brad's comments :)
In fact in my previous mail I was adressing both comments, but I sent 
the mail accidentally. :|

Thank you for taking time for this, it's nice Manuel that you cc'ed Brad 
King.

Brad it's really nice that you replied so fast and that you help me in a 
great and detailed way like this.

On 03/06/2013 05:26 PM, Brad King wrote:
 >> +++ b/tools/libclang/cmake/modules/CMakeLists.txt
 >> ...
 >> +set(libclang_cmake_builddir "${CMAKE_BINARY_DIR}/share/clang/cmake")
 >
 > FYI, the location inside the build tree is just a staging area
 > for install(FILES) and the files should never be loaded from
 > there.

I'm aware of this. I tried to make something equal to 
llvm/cmake/modules/CMakeLists.txt which also declares a variable in the 
same way to provide package configuration file for llvm libraries.

 >
 >> +set(LLVM_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX})
 >
 > You don't seem to actually use this variable anywhere in
 > the configure_file inputs.
 >

Yes this is silly indeed to have it defined. :) It was to have the same 
as llvm/cmake/modules/CMakeLists.txt, I removed it in the new patch.

 > Also CMake will not look in "clang" for a package called "libclang"
 > without extra options to find_package.

Hmm... Indeed I thought I tested it without, but when I repeat my test 
now I see that I should always have had a define for libclang_DIR 
because it ran through. Thanks for pointing this out, I fixed it.

 > Finally, there could be more
 > than one version of libclang in the same prefix (someday).  I suggest
 > installing the package configuration file and package version file to
 >
 >   lib/cmake/libclang-<version>
 >
 > where <version> is "${CLANG_VERSION_MAJOR}.${@CLANG_VERSION_MINOR}".
 >
 >> +#       target_link_libraries(youTarget clang)
 >

It's nice to think to the future indeed, I changed this, but took 
LIBCLANG_LIBRARY_VERSION instead of 
${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}, since it's also used in 
the package version file. :)

 > Using link_directories/target_link_libraries with a library name
 > will work but it will not hook up a dependency to cause the app
 > to re-link when the clang library file changes.  To achieve that
 > you need full target export/import support:
 >
 > 
http://www.cmake.org/Wiki/CMake/Tutorials/Exporting_and_Importing_Targets
 >
 > That can be added later though.  The basic find_package(libclang)
 > functionality will work without this.

This would be nice and especially for people patching libclang, testing 
their custom build of libclang.

It should be pretty easy to add this, by adding an EXPORT param to the 
install call in the add_clang_library macro (see root CMakeLists.txt), 
and we could add an install(EXPORT...) call there, to put a full 
import/export support per library. I'll try to make a new patch for this 
once this first patch get merged, because this requires same 
modifications for the install of llvm/ libraries too.

 >
 >> +++ b/tools/libclang/cmake/modules/libclang-config.cmake.in
 >> ...
 >> +message(STATUS "libclang version: ${LIBCLANG_PACKAGE_VERSION}")
 >
 > Package configuration files should not display any messages.
 > If the project loading it wants to display that kind of verbose
 > information it can do so itself.
 >

Ihis makes sense, I didn't know, because often FindPackages file 
performs status outputs. Thanks. :)

 >> The other question is that I think that there are still
 >> many (basically all) package releases that use the
 >> configure-based build. We'll need a solution for those as
 >> well. Is the idea that we just copy the correctly versioned
 > Yes, you need to configure the two files
 >
 >   libclang-config.cmake.in
 >   libclang-config-version.cmake.in
 >
 > with proper @...@ replacements and install them.
 >
 >> files into <prefix>/share/clang/cmake?
 >
 > Yes, though see above recommendation about install destination:
 >
 >    lib/cmake/libclang-<version>
 >

@Manuel: I would be happy to add this to the patch, is there already any 
starting point ? Because the llvm libs also have package configuration 
file for cmake, but I didn't found any place (i.e. I fgrep'ed in 
llvm/autoconf/ without any match) where this was already configured 
through the autoreconf files.

Please find a new patch with the different modifications discussed as 
attachment.

Cheers and thank you for your help,
--
Damien Buhl
alias daminetreg

P.S. The patch is also available on github here : 
https://github.com/daminetreg/clang/tree/libclang-cmake-package-config
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Adds-a-CMake-package-configuration-file-for-libclang.patch
Type: text/x-patch
Size: 4427 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130307/3510bd75/attachment.bin>