MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0012576CMakeModulespublic2011-11-12 09:192012-11-05 14:33
ReporterHans Johnson 
Assigned ToRolf Eike Beer 
PrioritynormalSeverityminorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake 2.8.6 
Target VersionCMake 2.8.9Fixed in VersionCMake 2.8.9 
Summary0012576: CHECK_C_COMPILER_FLAG is inflexible for FAIL_REGEX
DescriptionThe end user that wants to use a new compiler has no way to add a new FAIL_REGEX option to the
CHECK_C_COMPILER_FLAG macro.

In ITK we created a custom ITK_CHECK_C_COMPILER_FLAG that adds only one line in order to properly handle the intel compiler.

It would be very nice if this macro were refactored so that one could append new FAIL_REGEX strings from end-user build environments without the need to copy and alter this macro set.

Hans
Steps To Reproduce    MACRO (ITK_CHECK_C_COMPILER_FLAG _FLAG _RESULT)
26 SET(SAFE_CMAKE_REQUIRED_DEFINITIONS "${CMAKE_REQUIRED_DEFINITIONS}")
27 SET(CMAKE_REQUIRED_DEFINITIONS "${_FLAG}")
28 CHECK_C_SOURCE_COMPILES("int main(void) { return 0; }" ${_RESULT}
29 # Some compilers do not fail with a bad flag
30 FAIL_REGEX "warning: command line option .* is valid for .* but not for C"
31 # Apple gcc
32 FAIL_REGEX "unrecognized .*option" # GNU
33 FAIL_REGEX "unknown .*option" # Clang
34 FAIL_REGEX "ignoring unknown option" # MSVC
35 FAIL_REGEX "warning D9002" # MSVC, any lang
36 FAIL_REGEX "[Uu]nknown option" # HP
37 FAIL_REGEX "[Ww]arning: [Oo]ption" # SunPro
38 FAIL_REGEX "command option .* is not recognized" # XL
39 FAIL_REGEX "warning 0010156: ignoring option" # INTEL compilers
40 )
Additional Information
TagsNo tags attached.
Relationships
has duplicate 0013294closed Brad King patch to CheckC(XX)CompilerFlag.cmake to match ICC output 
Attached Filespatch CheckCCompilerFlag.cmake.patch (320) 2012-04-15 18:34
https://public.kitware.com/Bug/file/4303/CheckCCompilerFlag.cmake.patch
patch CheckCXXCompilerFlag.cmake.patch (320) 2012-04-15 18:34
https://public.kitware.com/Bug/file/4304/CheckCXXCompilerFlag.cmake.patch
Issue History
Date ModifiedUsernameFieldChange
2011-11-12 09:19Hans JohnsonNew Issue
2011-11-12 10:13David ColeNote Added: 0027787
2012-01-13 14:40Szilárd PállNote Added: 0028295
2012-02-15 16:04Rolf Eike BeerNote Added: 0028596
2012-04-15 18:34Szilárd PállFile Added: CheckCCompilerFlag.cmake.patch
2012-04-15 18:34Szilárd PállFile Added: CheckCXXCompilerFlag.cmake.patch
2012-04-15 18:35Szilárd PállNote Added: 0029161
2012-04-16 03:36Rolf Eike BeerNote Added: 0029163
2012-04-16 16:38Rolf Eike BeerNote Added: 0029188
2012-04-16 16:38Rolf Eike BeerAssigned To => Rolf Eike Beer
2012-04-16 16:38Rolf Eike BeerStatusnew => resolved
2012-04-16 16:38Rolf Eike BeerResolutionopen => fixed
2012-04-16 16:38Rolf Eike BeerFixed in Version => CMake 2.8.8
2012-04-16 16:38Rolf Eike BeerTarget Version => CMake 2.8.8
2012-04-19 08:54Rolf Eike BeerStatusresolved => feedback
2012-04-19 08:54Rolf Eike BeerResolutionfixed => reopened
2012-04-19 08:55Rolf Eike BeerStatusfeedback => resolved
2012-04-19 08:55Rolf Eike BeerResolutionreopened => fixed
2012-04-19 08:55Rolf Eike BeerFixed in VersionCMake 2.8.8 => CMake 2.8.9
2012-04-19 08:55Rolf Eike BeerTarget VersionCMake 2.8.8 => CMake 2.8.9
2012-04-19 09:09Hans JohnsonNote Added: 0029240
2012-04-19 09:09Hans JohnsonStatusresolved => feedback
2012-04-19 09:09Hans JohnsonResolutionfixed => reopened
2012-04-19 09:40Szilárd PállNote Added: 0029242
2012-04-19 10:30David ColeNote Added: 0029251
2012-04-19 10:58Szilárd PállNote Added: 0029252
2012-04-19 11:34Rolf Eike BeerNote Added: 0029254
2012-04-19 12:06David ColeNote Added: 0029255
2012-04-26 18:55Rolf Eike BeerNote Added: 0029352
2012-06-04 17:02David ColeNote Added: 0029608
2012-06-04 17:02David ColeStatusfeedback => resolved
2012-06-04 17:02David ColeResolutionreopened => fixed
2012-06-12 16:39Brad KingRelationship addedhas duplicate 0013294
2012-11-05 14:33David ColeNote Added: 0031450
2012-11-05 14:33David ColeStatusresolved => closed

Notes
(0027787)
David Cole   
2011-11-12 10:13   
Yes, but if we extend this to allow "user defined regexes" to be passed in, then people will just use their own user defined regexes when they need to, and they will not tell the CMake team about them, and then this function will start to *require* use of user defined regexes just to be useful.

Wouldn't it be better to submit the new required regexes to the CMake build, so that it can be merged into the next release of CMake for all projects to benefit from?
(0028295)
Szilárd Páll   
2012-01-13 14:40   
At the same time, if you don't allow any flexibility, one expects that it just works. Submitting new required regexes is not a bad solution, but some compilers change error/warning messages (and even options) more often than what I'd consider sane. The Intel Compiler is among these.

Anyway, where can I submit the changes? I have access to tons of different compilers/versions!
(0028596)
Rolf Eike Beer   
2012-02-15 16:04   
Right here. Just attach a patch.
(0029161)
Szilárd Páll   
2012-04-15 18:35   
Attached are patches with some of the common the Intel Compilers and Open64 warnings. Note that this is still not optimal as e.g. the Intel Compilers have 150+ non-fatal errors (can be listed with icc -diag-error warn).
(0029163)
Rolf Eike Beer   
2012-04-16 03:36   
The first and the last expression is already part of the current file:

     FAIL_REGEX "option .*not supported" # Intel
     FAIL_REGEX "WARNING: unknown flag:" # Open64

http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/CheckCCompilerFlag.cmake;h=1c08c595903232ac93762d1dee6e2faba828f381;hb=HEAD [^]
http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/CheckCXXCompilerFlag.cmake;h=6fa69b12d80071be9fa5524a431c0b075de9aff7;hb=HEAD [^]

I'll merge the others tonight. Just a note for further contributions: please use "diff -u" and use a single patch for multiple files if possible. That makes applying things easier.
(0029188)
Rolf Eike Beer   
2012-04-16 16:38   
Patch merged to next:

http://cmake.org/gitweb?p=cmake.git;a=commit;h=bbb895959f54a9dec9f5132313f4a71fafc458e5 [^]
(0029240)
Hans Johnson   
2012-04-19 09:09   
This patch fixes the exemplar case, but the systemic problem is still there.

As new compilers/compiler flags/build environments are used, one must change the source code of CMake in order to adapt. This process takes at least 5 months.

I was hoping for a fexible "Runtime" way to augment the FAIL_REGEX rules.

==================
Feel free to close this issue as "won't fix" if there are no resources.
(0029242)
Szilárd Páll   
2012-04-19 09:40   
Rolf, note the lack of space in my original patch:
     FAIL_REGEX "option.*not supported"
which matches two more warnings which use "options" iso "option".
Why has the patch been delayed to 2.8.9?


Hans, actually I do agree that this implementation is not very useful; as I mentioned earlier, the Intel Compilers have 150+ non-fatal errors triggered by invalid command line options. The current implementation covers exactly 19 of them. Of course, one would not test tons of random options/flags, but I would suggest that the CMake folks seriously rethink the goal of this macro as ATM it is crippled and I see little chance that such an approach can ever reach a decent level of support for common compilers.

Also note that projects that make use of this feature have already started to systematically replicate the files in question and just make the module's *basic* features work for their cases of interest.

It is in fact a pretty common thing that projects just replace CMake modules due to the annoyance of such, way to often met, half-baked features.

Sure, you can hope that everybody will contribute back and things will quickly get better, but we both know that's not the case. Heavy replication is happening as it is the only way for CMake users to get things fixed in a timely manner -- no sane person would require the latest version as soon as it comes out.
(0029251)
David Cole   
2012-04-19 10:30   
The patch has been delayed to 2.8.9 because we do not make last minute merges to 'master' except for absolutely critical issues that MUST be fixed. (Crashes, horrific regressions in behavior, last minute bug fixes for new features that went unnoticed until we started doing release candidates, ...)

If we had accepted it, would you be happy?

As you noted, the patch that was merged differs from yours and misses some of your intended matches.

If you want a patch definitely in a certain CMake release, then it absolutely MUST be in 'master' *before* we cut our first release candidate for a release. After that point, we are extremely careful about the patches that we allow in, in order to guarantee as stable a release as possible.
(0029252)
Szilárd Páll   
2012-04-19 10:58   
David, I was just wondering why was the patch first nominated for 2.8.8, but then moved to 2.8.9. In fact, this was not the reason for my ranting and getting this minor fix into 2.8.8 won't make much difference; I have anyway pushed fixed files into our project's source repo.

Although I think I was pretty clear, but let me reiterate. The source of my annoyance is that the issue of half-baked features and "half-bottomed" portability implementations results in having to maintain custom versions of various CMake modules. I can assure you that this affects a large part of the user base, just look around how many of projects that use CMake include fixed/improved versions of various modules in their source tree -- and we haven't even considered the cases when the developers chose to implement workarounds and not replicate code.
(0029254)
Rolf Eike Beer   
2012-04-19 11:34   
The reason this was moved to 2.8.9 is just that I merged it too late to next so it wasn't picked up for 2.8.8 as I initially had expected.
(0029255)
David Cole   
2012-04-19 12:06   
Sorry for your annoyance.

Half-baked is certainly not our intention.

I think you should move your rant / expression of annoyance to the CMake mailing list. (Nobody but me and Eike are reading the comments in this bug report.)

This is partly simply due to the nature of the distributed contributions made to CMake from a very diverse set of contributors. We try as hard as we can to maintain "green" on our dashboards, and if there things that slip into our releases half-baked, it's because we don't have perfect coverage on our dashboards.

The best way to address this is to add those platforms and compilers to our nightly dashboard regimen so that we can notice what commits introduce what problems on the dashboards. If we don't have a submission there for a given compiler, then we're not going to notice if we unintentionally break something related to it.

Hopefully, despite all this, CMake is still useful for you, and we welcome further discussion. (I'd like to see it on the mailing list, though, so the wider community may participate more easily.)
(0029352)
Rolf Eike Beer   
2012-04-26 18:55   
Updated patch merged as f621ead8b78862a00e841dc336f601bbe267364c into next, this time attributed to the correct author.
(0029608)
David Cole   
2012-06-04 17:02   
see previous notes
(0031450)
David Cole   
2012-11-05 14:33   
Closing resolved issues that have not been updated in more than 4 months.