[cmake-developers] Patch for FindIconv support

Brad King brad.king at kitware.com
Wed Nov 26 10:26:44 EST 2014


On 11/26/2014 9:39 AM, Steven Oliver wrote:
> The patch is attached. I actually wrote it quite a while ago but never
> got around to submitting it. Please take a good look at it and let me
> know what issues you see with it.

Thanks for working on this!  Here are some comments:

* Please read the cmake-developer(7) manual section on modules:

   http://www.cmake.org/cmake/help/v3.1/manual/cmake-developer.7.html#modules

* The documentation needs to be updated to the reStructuredText system.
  See the man page for details.

* The find_library cache entry should be ICONV_LIBRARY, but may the be
  copied to ICONV_LIBRARIES for consumption by callers.
  See the man page section on standard variable names.

* The CMAKE_REQUIRED_ variable settings around the check code should
  be saved and restored in case the calling project is accumulating them.

* Does the ICONV_SECOND_ARGUMENT_IS_CONST check code actually fail to
  compile when the second argument is not const?  Some C compilers may
  just warn about the incompatible pointer type but still work.

* Please avoid trailing whitespace on lines.

Thanks,
-Brad


More information about the cmake-developers mailing list