|
Notes |
|
|
(0030926)
|
|
Rolf Eike Beer
|
|
2012-09-05 10:44
|
|
-iconv is a C library, no? Why then require a C++ compiler to check for the const argument?
-please use FindPackageHandleStandardArgs for the found/not found/silent/required checking
-since after 2.8.9 all commands in CMake modules shipped by CMake itself should use lowercase commands (i.e. set instead of SET) |
|
|
|
(0030935)
|
|
Steven Oliver
|
|
2012-09-05 20:40
|
|
|
I have added a new file that I believe addresses the comments from Rolf Eike Beer. Thank you very much for your suggestions!! |
|
|
|
(0030937)
|
|
Rolf Eike Beer
|
|
2012-09-06 02:54
|
|
Better, but still too much code ;)
-remove the 2 block with "if(ICONV_INCLUDE_DIR AND ICONV_LIBRARIES)", it doesn't hurt if the other code will be run twice, especially find_library() and find_path() will just do nothing if the result variable is already set.
-don't set ICONV_FOUND yourself, let FPHSA handle that
-move the compile check below the call to FPHSA so it will see the ICONV_FOUND then
-please use the second (newer, more flexible) interface of FPHSA using REQUIRED_VARS and friends |
|
|
|
(0031023)
|
|
Steven Oliver
|
|
2012-09-12 21:13
|
|
|
Hopefully this version cleans everything up to spec! |
|
|
|
(0031024)
|
|
Rolf Eike Beer
|
|
2012-09-13 02:56
|
|
You set CMAKE_REQUIRED_INCLUDES to immediately unset it again. I think this is by accident. Move those variables into the if() that guards compile check, this is the only place where they are needed. And unset _after_ the check as it doesn't make any sense otherwise.
Looks good to me otherwise. |
|
|
|
(0031106)
|
|
Steven Oliver
|
|
2012-09-26 21:51
|
|
|
Uploaded one last version. At this point you (Rolf Beer) should get all the credit! I just provided the typing. |
|
|
|
(0031699)
|
|
Steven Oliver
|
|
2012-11-24 20:27
|
|
|
Is there any chance of this getting added to CMAKE? |
|
|
|
(0031700)
|
|
Rolf Eike Beer
|
|
2012-11-25 09:22
|
|
Ok, 2 things. First, the procedure for submitting new modules is described here:
http://cmake.org/Wiki/CMake:Module_Maintainers [^]
Another thing is that this will now compile the test code every time iconv is found, i.e. if I touch a CMakeLists.txt it will rerun the test (at least I think it will). Can you test this? If it will run this every time my first try of a cure would be a cached advanced variable that remembers if that test has already been run. |
|
|
|
(0037293)
|
|
Steven Oliver
|
|
2014-11-26 09:35
|
|
Please close this issue. I will be posting to the developer's mailing list shortly.
Thank you for all your help. |
|
|
|
(0042114)
|
|
Kitware Robot
|
|
2016-06-10 14:28
|
|
Resolving issue as `moved`.
This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page. |
|
0001-Add-new-FindIconv-module.patch (2,264) 2012-09-05 10:20