View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0013517CMakeCMakepublic2012-09-05 10:202016-06-10 14:31
ReporterSteven Oliver 
Assigned ToKitware Robot 
PrioritynormalSeverityfeatureReproducibilityalways
StatusclosedResolutionmoved 
PlatformOSFedora LinuxOS Version17
Product VersionCMake 2.8.9 
Target VersionFixed in Version 
Summary0013517: Add FindIconv module
DescriptionThis module, hopefully, will properly check to see if Iconv exists on the system.
TagsNo tags attached.
Attached Filespatch file icon 0001-Add-new-FindIconv-module.patch [^] (2,264 bytes) 2012-09-05 10:20 [Show Content]
patch file icon 0001-Add-new-FindIconv-module_v2.patch [^] (2,125 bytes) 2012-09-05 20:39 [Show Content]
patch file icon 0001-Add-FindIconv-file.patch [^] (1,965 bytes) 2012-09-12 21:13 [Show Content]
patch file icon 0001-Add-new-FindIconv-module_v3.patch [^] (1,977 bytes) 2012-09-26 21:49 [Show Content]

 Relationships

  Notes
(0030926)
Rolf Eike Beer (developer)
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 (reporter)
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 (developer)
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 (reporter)
2012-09-12 21:13

Hopefully this version cleans everything up to spec!
(0031024)
Rolf Eike Beer (developer)
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 (reporter)
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 (reporter)
2012-11-24 20:27

Is there any chance of this getting added to CMAKE?
(0031700)
Rolf Eike Beer (developer)
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 (reporter)
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 (administrator)
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.

 Issue History
Date Modified Username Field Change
2012-09-05 10:20 Steven Oliver New Issue
2012-09-05 10:20 Steven Oliver File Added: 0001-Add-new-FindIconv-module.patch
2012-09-05 10:44 Rolf Eike Beer Note Added: 0030926
2012-09-05 20:39 Steven Oliver File Added: 0001-Add-new-FindIconv-module_v2.patch
2012-09-05 20:40 Steven Oliver Note Added: 0030935
2012-09-06 02:54 Rolf Eike Beer Note Added: 0030937
2012-09-12 21:13 Steven Oliver File Added: 0001-Add-FindIconv-file.patch
2012-09-12 21:13 Steven Oliver Note Added: 0031023
2012-09-13 02:56 Rolf Eike Beer Note Added: 0031024
2012-09-26 21:49 Steven Oliver File Added: 0001-Add-new-FindIconv-module_v3.patch
2012-09-26 21:51 Steven Oliver Note Added: 0031106
2012-11-24 20:27 Steven Oliver Note Added: 0031699
2012-11-25 09:22 Rolf Eike Beer Note Added: 0031700
2014-11-26 09:35 Steven Oliver Note Added: 0037293
2016-06-10 14:28 Kitware Robot Note Added: 0042114
2016-06-10 14:28 Kitware Robot Status new => resolved
2016-06-10 14:28 Kitware Robot Resolution open => moved
2016-06-10 14:28 Kitware Robot Assigned To => Kitware Robot
2016-06-10 14:31 Kitware Robot Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team