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

-- 

Powered by www.kitware.com

Please keep messages on-topic and check the CMake FAQ at: 
http://www.cmake.org/Wiki/CMake_FAQ

Kitware offers various services to support the CMake community. For more 
information on each offering, please visit:

CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html

Visit other Kitware open-source projects at 
http://www.kitware.com/opensource/opensource.html

Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers

Reply via email to