[PATCH] Adds a CMake package configuration file for libclang

[Damien Buhl]

On 03/07/2013 03:42 PM, Brad King wrote:> Note that exported targets can 
put arch-specific information into the
 > targets file it generates.  Therefore lib/ is better than share/ as
 > a place for the package configuration file.
 >
 > Also it will take some manual work to get imported targets to work
 > when building without CMake.  A fallback would be to just produce the
 > basic file with autotools and do the full target exports only when
 > building with CMake.

Ok, so we should also patch location of package configuration for llvm 
in this case. I'll make an additional patch in correlation to fix the 
location in llvm.

 >
 > The new patch looks good, though I haven't had time to try building it.
 >
Thanks ;) This time I tried to build several times, and cmake found the 
package without any problem and I was able to link to it.

On 03/07/2013 08:17 PM, Brad King wrote:
> On 03/07/2013 02:07 PM, Argyrios Kyrtzidis wrote:
>> On Mar 7, 2013, at 9:59 AM, Brad King <brad.king at kitware.com <mailto:brad.king at kitware.com>> wrote:
>>> On 03/07/2013 12:32 PM, Jordan Rose wrote:
>>>> AFAIK libclang is forward-compatible across minor versions, but not
>>>> backwards (i.e. because new features can be introduced with minor versions).
>>
>> That is correct.
>>
>>> In that case the logic in the package version file needs to be
>>> something like:
>>>
>>> set(PACKAGE_VERSION @LIBCLANG_LIBRARY_VERSION@)
>>> if("${PACKAGE_FIND_VERSION_MAJOR}" EQUAL @CLANG_VERSION_MAJOR@ AND
>>>     NOT "${PACKAGE_FIND_VERSION_MINOR}" GREATER @CLANG_VERSION_MINOR@)
>>>    set(PACKAGE_VERSION_COMPATIBLE 1)
>>>    if("${PACKAGE_FIND_VERSION_MINOR}" EQUAL @CLANG_VERSION_MINOR@)
>>>      set(PACKAGE_VERSION_EXACT 1) # TODO: check patch level too?
>>>    endif()
>>> endif()
>>
>> The versions for the libclang API are
>>
>> CINDEX_VERSION_MAJOR
>> CINDEX_VERSION_MINOR
>>
>> in $clang_source/include/clang-c/Index.h
>
> Great.  Damien, please fold this into your next patch revision.
> You can use file(STRINGS) to parse the values out of the header.

Thanks for the information Jordan & Argyrios, and thanks Brad for the 
package version file logic and the tip about file(STRINGS), I already 
used it for most FindPackage file I wrote.

However I believe the CINDEX_VERSION_MAJOR and CINDEX_VERSION_MINOR 
values shouldn't be used here, because currently libclang version is 
defined through the variable LIBCLANG_LIBRARY_VERSION, which is even 
used as target property version in 
$clang_source/tools/libclang/CMakeLists.txt, lines 88-92 :

     set_target_properties(libclang
       PROPERTIES
       OUTPUT_NAME "clang"
       VERSION ${LIBCLANG_LIBRARY_VERSION}
       DEFINE_SYMBOL _CINDEX_LIB_)

And the problem is that ${LIBCLANG_LIBRARY_VERSION} is defined as the 
concatenation of ${CLANG_VERSION_MAJOR}.${CLANG_VERSION_MINOR}, so the 
package version file should have an history telling the mapping between 
LIBCLANG_LIBRARY_VERSION and CINDEX_VERSION_MAJOR+CINDEX_VERSION_MINOR.

This isn't pretty at all, and I don't think it's maintainable, so 
shouldn't LIBCLANG_LIBRARY_VERSION be made of 
CINDEX_VERSION_MAJOR+CINDEX_VERSION_MINOR ? I don't think that this is 
doable, in fact we need to know how the policy is between the 
CINDEX_VERSION_MAJOR+CINDEX_VERSION_MINOR and LIBCLANG_LIBRARY_VERSION. 
Because users of the lib will never find_package(libclang 0.12) if they 
install libclang-3.3.

What do you think that would be better to do here ? Define some policy 
for the LIBCLANG_LIBRARY_VERSION change in relation to 
CINDEX_VERSION_MAJOR+CINDEX_VERSION_MINOR or be ready to store in the 
future some complicated mapping between these information in the package 
version file, and find a way that someone makes sure that this get 
updated in the file when tagging a new libclang version.

I believe this is dangerous and error-prone, but I could be missing 
something.

As a consequence if we cannot have any policy for the mapping between 
library version and cindex version, I would rather tend to a test for 
exact version equality than expecting libclang maintainers update cmake 
package version file each time they change CINDEX_VERSION_MAJOR.

Tell me what you prefer and you think it's correct and I'll do it.

Thanks for your collaboration on this :),
Cheers,
--
Damien Buhl
alias daminetreg