[cmake-developers] [PATCH] New module: FindIce.cmake

[Roger Leigh]

On Mon, Aug 11, 2014 at 11:07:57AM -0400, Brad King wrote:
> 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

I will certainly ask them if they will consider it.

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

I'm currently working on this for my job, and so will be keeping this
up to date.  I just thought it would benefit being upstream so that
others can benefit from it.  I'll also make the upstream aware of it.
They release fairly infrequently, so I don't expect updates generally
more than once or twice a year, if that.

While a configuration file would be ideal for new releases, the
proposed find script will cater for their most recent releases
to date, and as such will support all current Linux, Mac and
Windows users.

> As for the module itself, there are a few problems:

I hope that the new patch (attached) addresses these problems.  I
hope this is more acceptable, and I'll be happy to make any additional
changes.

> 1. The legal notice block is not of the proper format and
>    fails the CMake.ModuleNotices test.

This should be fixed.

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

I read through the conventions and fixed things to work as best as
I understood things.  I also switched to supporting COMPONENTS
and OPTIONAL_COMPONENTS for all the separate libraries.

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

I have reworked this so that it loops through all the variants
which are compatible with the generator in use, plus fallbacks.
The disadvantage is that this now means that on Windows you
won't get an outright failure if the visual studio version
mismatches with the detected libraries whereas the previous
approach hardcoded all that knowledge.  That said, it's a
whole lot more flexible and maintainable this way and so is an
acceptable tradeoff.

Regarding the Windows Registry, I've taken a look and it looks
like there might be some usable keys from the installer which
could be used, but I'll need to do further digging with all
the different versions to see what's most usable here.

> 4. Rather than repeatedly testing CMAKE_SIZEOF_VOID_P,
>    save the "/x64" suffix in a "${_x64}" variable.

Also fixed now.

If you had any further comments on the attached revision, I'll
be happy to make any further changes as needed, and as noted
above I'll also look at the registry stuff.


Regards,
Roger

-- 
  .''`.  Roger Leigh
 : :' :  Debian GNU/Linux    http://people.debian.org/~rleigh/
 `. `'   schroot and sbuild  http://alioth.debian.org/projects/buildd-tools
   `-    GPG Public Key      F33D 281D 470A B443 6756 147C 07B3 C8BC 4083 E800
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-FindIce-New-module-to-find-ZeroC-Ice.patch
Type: text/x-diff
Size: 21436 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140812/478f0b53/attachment-0002.patch>