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

[Roger Leigh]

On Wed, Aug 13, 2014 at 10:10:01AM -0400, Brad King wrote:
> On 08/12/2014 05:59 PM, Roger Leigh wrote:
> >> 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.
> > 
> > This turned out to be fairly simple at least for Ice versions
> > 3.4.0 - 3.5.1, which all have the same naming convention.
> > Earlier versions have odd naming conventions and are in
> > Wow6432Node so I've not included them (they are obsolete in
> > any case, and ICE_HOME can be set to use them).
> 
> The Wow6432Node is for distinguishing between 32-bit and 64-bit
> views of the registry.  Just omit it from any registry key path.
> When CMake find_* commands evaluate registry values they take
> into account whether the target architecture is 32-bit or 64-bit
> and prefer the matching registry view.
> 
> So, please go ahead and add the registry entries for the earlier
> versions.  Just convert any "\Wow6432Node\" to "\" in the keys.

OK, done now and it's working fine.

> Other comments:
> 
> > +# Components can include any of: Freeze Glacier2 Ice IceBox IceDB
> > +# IceGrid IcePatch IceSSL IceStorm IceUtil IceXML Slice.
> 
> I think each name in the list should be ``quoted`` to clarify
> that it is a name to appear in code.

Done.

> > +#   ICE_LIBRARIES - component libraries to be linked
> 
> Is that a typo in the docs?  It should be Ice_LIBRARIES.

Yes, fixed now.

> > +#   ICE_<C>_LIBRARY - component library
> 
> Although the individual find result is a singular-named cache entry
> the results presented to the project should still be plural-named
> normal variables:
> 
>  Ice_<C>_LIBRARY - cache entry
>  Ice_<C>_LIBRARIES - results (even if only one entry)

This is also fixed.

> > +set(ICE_HOME NOTFOUND
> > +    CACHE PATH "Location of the Ice installation")
> > +mark_as_advanced(FORCE ICE_HOME)
> > +
> > +set(ICE_BINARYDIR NOTFOUND
> > +    CACHE PATH "Location of the Ice programs")
> > +mark_as_advanced(FORCE ICE_BINARYDIR)
> > +
> > +set(ICE_INCLUDEDIR NOTFOUND
> > +    CACHE PATH "Location of the Ice headers")
> > +mark_as_advanced(FORCE ICE_INCLUDEDIR)
> > +
> > +set(ICE_SLICEDIR NOTFOUND
> > +    CACHE PATH "Location of the Ice slice interface definitions")
> > +mark_as_advanced(FORCE ICE_SLICEDIR)
> > +
> > +set(ICE_LIBRARYDIR NOTFOUND
> > +    CACHE PATH "Location of the Ice libraries")
> > +mark_as_advanced(FORCE ICE_LIBRARYDIR)
> 
> These hints should not need to be explicitly cached with such
> granularity.  The find_* commands create cache entries for
> their results both to save them persistently and to allow users
> to set them granularly.  The CMAKE_PREFIX_PATH variable allows
> users to add custom prefixes without a per-package _HOME or
> _ROOT variable.

These are all removed.

> > +  list(APPEND ice_slice_paths
> > +    "/usr/local/share/Ice-${Ice_VERSION_SLICE2CPP_FULL}/slice"
> > +    "/usr/local/share/Ice-${Ice_VERSION_SLICE2CPP_SHORT}/slice"
> > +    "/usr/local/share/Ice/slice"
> > +    "/usr/share/Ice-${Ice_VERSION_SLICE2CPP_FULL}/slice"
> > +    "/usr/share/Ice-${Ice_VERSION_SLICE2CPP_SHORT}/slice"
> > +    "/usr/share/Ice/slice")
> 
> It looks like a lot of the explicit search location construction
> could be simplified using PATH_SUFFIXES options to the find
> commands.  There should not need to be any explicit copies of
> paths for every common prefix.

I have changed all the find_* code to use PATH_SUFFIXES and the
correct cache variable names.  It certainly has simplified things
quite a bit.

> Another option you might be interested in exploring, but not
> as a requirement for our acceptance of this module, is creation
> of imported targets to hold the find results with usage reqs.
> See the cmake-buildsystem(7) manual:
> 
>  http://www.cmake.org/cmake/help/v3.0/manual/cmake-buildsystem.7.html
> 
> for more info.  See the FindQt4 module for an example.  This
> part could be added as a follow-up change once the main module
> is in though.

OK.  I'll have to read up on this and see what needs doing.
In the meantime, I've attached a revised patch with all the
above corrections included.


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: 18595 bytes
Desc: not available
URL: <http://public.kitware.com/pipermail/cmake-developers/attachments/20140815/39c1e50d/attachment-0002.patch>