[cmake-developers] [PATCH] New module: FindIce.cmake
Brad King
brad.king at kitware.com
Mon Aug 11 11:07:57 EDT 2014
On 08/08/2014 08:56 AM, Roger Leigh wrote:
> On Thu, Aug 07, 2014 at 06:49:28PM +0100, Roger Leigh wrote:
>> Hi,
>>
>> I've written a module for finding the details of a ZeroC ICE installation,
>> attached, which I thought might be of interest to a wider audience and be
>> suitable for direct inclusion into cmake. I've attached the patch for this.
>> The docs should be correct, but I'm not yet totally familiar with the cmake
>> docs build, so it might possibly need some minor tweaking.
>
> I have added a few portability and documentation fixes. Updated copy
> attached.
Thanks for working on this. The patch looks pretty complete.
Is it possible to convince ZeroC ICE to provide a CMake
Package Configuration File as documented here:
http://www.cmake.org/cmake/help/v3.0/manual/cmake-packages.7.html
? That would be much more maintainable and allows for much more
powerful CMake interfaces to be used. It is the CMake equivalent
of a pkg-config .pc file, but is much more powerful.
Otherwise we can consider the module here, but we ask that you
commit to regular maintenance as the upstream changes.
As for the module itself, there are a few problems:
1. The legal notice block is not of the proper format and
fails the CMake.ModuleNotices test.
2. The module provides singular names like <comp>_LIBRARY as its
output. Please read
http://www.cmake.org/cmake/help/v3.0/manual/cmake-developer.7.html#find-modules
for conventions on variable naming. The find_* command
cached result variables should not overlap with the output
variables from the module.
3. There is a lot of hard-coded version-specific information
that will require constant maintenance and new releases
of CMake as the upstream versions change. This is not
maintainable, and is one reason the package configuration
file approach linked above is much preferred to a Find
module. Are there Windows Registry entries available
that specify the install location?
4. Rather than repeatedly testing CMAKE_SIZEOF_VOID_P,
save the "/x64" suffix in a "${_x64}" variable.
Thanks,
-Brad
More information about the cmake-developers
mailing list