[cmake-developers] Adding MacroWriteBasicCMakeVersionFile.cmake file to cmake ?

Brad King brad.king at kitware.com
Thu Aug 4 17:37:00 EDT 2011


On 8/3/2011 4:04 PM, Brad King wrote:
> On 8/3/2011 4:00 PM, Alexander Neundorf wrote:
>> Are you ok with this branch or are there issues left (...since it wasn't
>> merged into master on Tuesday) ?
>
> I still need to find time to do further review and try it out.

Okay, here are a few comments from a quick review.

(1) The pointer size sanity check should report failure with
"PACKAGE_VERSION_UNSUITABLE" as documented here:

   http://www.cmake.org/cmake/help/cmake-2-8-docs.html#command:find_package
   http://www.cmake.org/Wiki/CMake/Tutorials/Packaging#Package_Version_Files

The version might be compatible but the package is unsuitable anyway.
With this approach you don't need to have the whole version check
inside an else() block so the code should look simpler.

(2) The documentation of the module has a few typos.  Please
proofread it again.  Also, please use the word "compatible" rather
than "suitable" to refer version acceptability.  This makes the
distinction noted above in (1).

(3) After noting the implementation detail with configure_file the
documentation should state that users should never directly configure
the files themselves.

(4) I wonder if we should just make AnyNewerVersion the default for
COMPATIBILITY so it does not have to be specified at all in the most
obvious and (probably) common case.  Comments?

Thanks,
-Brad



More information about the cmake-developers mailing list