Quantcast

Why is XXX_INCLUDE_DIRS plural and should it be deep?

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Why is XXX_INCLUDE_DIRS plural and should it be deep?

esatel
Hi all,
I am trying to adapt an existing FindNetCDF.cmake file to work with NetCDF 4.2. I have read the Modules/Readme.txt but I am still a little unclear to the best approach in my case.

In 4.2 the Fortran bindings and C bindings are not necessarily installed in the same place. One motive for NetCDF_INCLUDE_DIRS to be plural is that there might be a NetCDF_C_INCLUDE_DIR and a NetCDF_Fortran_INCLUDE_DIR that together form NetCDF_INCLUDE_DIRS.

There is a second motive that I am less clear on. The FindNetCDF.cmake files I have seen handle the HDF5 dependency differently:
1. One sets a variable NETCDF_HAS_HDF5 by querying a config executable but does nothing else.
2. The other sets a similar variable but then does find_package on HDF5 and loads the dependencies into NetCDF_INCLUDE_DIRS and NetCDF_LIBRARIES.

This design (2) cascades. FindHDF5.cmake represents a similar decision about whether to find_package its dependencies like the zlib compression library. I believe the one in the cmake distro doesn't do that.

What is the best practice here? To handle dual locations for the library at hand or to accumulate or both? The first seems unavoidable ... the second seems to be an unevenly applied standard.

Thanks
Eli
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Why is XXX_INCLUDE_DIRS plural and should it be deep?

Rolf Eike Beer
esatel wrote:

> 2. The other sets a similar variable but then does find_package on HDF5 and
> loads the dependencies into NetCDF_INCLUDE_DIRS and NetCDF_LIBRARIES.
>
> This design (2) cascades. FindHDF5.cmake represents a similar decision about
> whether to find_package its dependencies like the zlib compression library.
> I believe the one in the cmake distro doesn't do that.
>
> What is the best practice here? To handle dual locations for the library at
> hand or to accumulate or both? The first seems unavoidable ... the second
> seems to be an unevenly applied standard.
My personal vote on this is: if the public headers of package X include
headers from one of it's dependencies then you should cascade. So if
<netcdf.h> does

#ifdef WITH_HDF5
#include <hdf5.h>
#endif

then you would get a compile error if the HDF5 include directories are not
set. If this is just an internal dependency or you would explicitely need to
include e.g. netcdf/hdf5.h to get that sort of stuff then I would vote for not
becoming recursive. Which is a "sane" default as most packages will not need
that recursive approach then.

Eike
--
--

Powered by www.kitware.com

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

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

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake

signature.asc (205 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Why is XXX_INCLUDE_DIRS plural and should it be deep?

Lori Pritchett-Sheats
In reply to this post by esatel
My $0.02......I just completed a CMake system for a code project that
required several TPLs (third party libraries) two of which were the
NetCDF and HDF5.  I struggled with the same problems you mention here.
When I wrote my FindXXX modules, the standard I adopted was,
XXX_INCLUDE_DIR only contained the path to the XXX include directory
while XXX_INCLUDE_DIRS contained XXX include directory any other TPL
include directory required to compile.   Parts of the code was calling
NetCDF directly but not HDF5. I wanted the project CMakeLists.txt files
to be as clean as possible and placed the logic to determine if NetCDF
had HDF5 and adjusted the NetCDF_INCLUDE_DIRS if needed in our
FindNetCDF module. This allowed the developers to simply put

find_package(NetCDF)
include_directories(NetCDF_INCLUDE_DIRS)

in their CMakeLists.txt files not worry about testing for HDF5, etc.
This complicated the FindNetCDF module, but I think it improved the
stability of our CMake system.

If you decide not to cascade, then you will need to provide a flag like
you mention in (1) to help out whoever calls your module,  so they can
call find_package(HDF5) when needed.

When NetCDF split the language APIs, I used  the FindMPI module as a
template and created NetCDF_C_* NetCDF_CXX_* NetCDF_Fortran_* variables
in the FindNetCDF module. You may what to look at how that module
handles different language libraries and include directories  for
inspiration.


Good luck!




On 12/12/12 17:35, esatel wrote:

> Hi all,
> I am trying to adapt an existing FindNetCDF.cmake file to work with NetCDF
> 4.2. I have read the Modules/Readme.txt but I am still a little unclear to
> the best approach in my case.
>
> In 4.2 the Fortran bindings and C bindings are not necessarily installed in
> the same place. One motive for NetCDF_INCLUDE_DIRS to be plural is that
> there might be a NetCDF_C_INCLUDE_DIR and a NetCDF_Fortran_INCLUDE_DIR that
> together form NetCDF_INCLUDE_DIRS.
>
> There is a second motive that I am less clear on. The FindNetCDF.cmake files
> I have seen handle the HDF5 dependency differently:
> 1. One sets a variable NETCDF_HAS_HDF5 by querying a config executable but
> does nothing else.
> 2. The other sets a similar variable but then does find_package on HDF5 and
> loads the dependencies into NetCDF_INCLUDE_DIRS and NetCDF_LIBRARIES.
>
> This design (2) cascades. FindHDF5.cmake represents a similar decision about
> whether to find_package its dependencies like the zlib compression library.
> I believe the one in the cmake distro doesn't do that.
>
> What is the best practice here? To handle dual locations for the library at
> hand or to accumulate or both? The first seems unavoidable ... the second
> seems to be an unevenly applied standard.
>
> Thanks
> Eli
>
>
>
> --
> View this message in context: http://cmake.3232098.n2.nabble.com/Why-is-XXX-INCLUDE-DIRS-plural-and-should-it-be-deep-tp7582623.html
> Sent from the CMake mailing list archive at Nabble.com.
> --
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.cmake.org/mailman/listinfo/cmake


--
Lori A. Pritchett-Sheats, PhD.
CCS-2, Computational Physics and Methods
Office: 505-665-6675
Fax: 505-665-4972

Los Alamos National Laboratory
P.O. Box 1663
MS D413
Los Alamos, NM 87544

--

Powered by www.kitware.com

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

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

Follow this link to subscribe/unsubscribe:
http://www.cmake.org/mailman/listinfo/cmake
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Why is XXX_INCLUDE_DIRS plural and should it be deep?

esatel
In reply to this post by Rolf Eike Beer
Eike --
Your suggestion makes a lot of sense.

This is also a difference between includes and libraries. You need the whole library chain to link ... so is the ideal to accumulate the libraries?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Why is XXX_INCLUDE_DIRS plural and should it be deep?

esatel
In reply to this post by Lori Pritchett-Sheats

I see the advantages. I think I will do what you suggest for languages.

As far as cascading, I see it both ways and I think I'll be more productive if I abandon the idea that there is a best practice for this. It isn't THAT hard to make it right either way. If you know your dependencies (e.g. HDF5) didn't do it you can always patch it up at a higher level in FindNetCDF or CMakeLists.txt. It is a surprising gap though since so much of how FindXXX is written is standarized.
Loading...