[PATCH] Adds a CMake package configuration file for libclang

[Brad King]

On 03/06/2013 10:01 AM, Manuel Klimek wrote:
> Looking at your patch, it generally looks good if the high
> level direction is right... I'm cc'ing Brad King, who has
> responded to your original pull request, in the hope that he
> can confirm that this looks good from a cmake perspective.

I just looked at the patch as posted here:

 http://thread.gmane.org/gmane.comp.compilers.clang.scm/68052

though I have not actually tried building with it.

It is a great start.  Thanks for working on it.

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

> +set(LLVM_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX})

You don't seem to actually use this variable anywhere in
the configure_file inputs.

> +++ b/tools/libclang/cmake/modules/CMakeLists.txt
> ...
> +install(FILES
> +    ${libclang_cmake_builddir}/libclang-config.cmake
> +    ${libclang_cmake_builddir}/libclang-config-version.cmake
> +    DESTINATION share/clang/cmake)

In the future the file may contain architecture-specific information.
Also CMake will not look in "clang" for a package called "libclang"
without extra options to find_package.  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)

s/you/your/ ?

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.

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

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

> If yes, how does cmake find them there?

The CMake find_package command:

 http://www.cmake.org/cmake/help/v2.8.10/cmake.html#command:find_package

will search in <prefix>/lib/cmake/<name>* where <name> is the
package to be found (case insensitive) and "*" globs any suffix
on the directory name (like "-<version>").  The command documents
how it constructs the list of possible <prefix>es.

-Brad