View Issue Details Jump to Notes ] Print ]
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 )
TagsNo tags attached.
Attached Filespatch file icon CheckCCompilerFlag.cmake.patch [^] (320 bytes) 2012-04-15 18:34 [Show Content]
patch file icon CheckCXXCompilerFlag.cmake.patch [^] (320 bytes) 2012-04-15 18:34 [Show Content]

 Relationships
has duplicate 0013294closedBrad King patch to CheckC(XX)CompilerFlag.cmake to match ICC output 

  Notes
(0027787)
David Cole (manager)
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 (reporter)
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 (developer)
2012-02-15 16:04

Right here. Just attach a patch.
(0029161)
Szilárd Páll (reporter)
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 (developer)
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 (developer)
2012-04-16 16:38

Patch merged to next:

http://cmake.org/gitweb?p=cmake.git;a=commit;h=bbb895959f54a9dec9f5132313f4a71fafc458e5 [^]
(0029240)
Hans Johnson (reporter)
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 (reporter)
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 (manager)
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 (reporter)
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 (developer)
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 (manager)
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 (developer)
2012-04-26 18:55

Updated patch merged as f621ead8b78862a00e841dc336f601bbe267364c into next, this time attributed to the correct author.
(0029608)
David Cole (manager)
2012-06-04 17:02

see previous notes
(0031450)
David Cole (manager)
2012-11-05 14:33

Closing resolved issues that have not been updated in more than 4 months.

 Issue History
Date Modified Username Field Change
2011-11-12 09:19 Hans Johnson New Issue
2011-11-12 10:13 David Cole Note Added: 0027787
2012-01-13 14:40 Szilárd Páll Note Added: 0028295
2012-02-15 16:04 Rolf Eike Beer Note Added: 0028596
2012-04-15 18:34 Szilárd Páll File Added: CheckCCompilerFlag.cmake.patch
2012-04-15 18:34 Szilárd Páll File Added: CheckCXXCompilerFlag.cmake.patch
2012-04-15 18:35 Szilárd Páll Note Added: 0029161
2012-04-16 03:36 Rolf Eike Beer Note Added: 0029163
2012-04-16 16:38 Rolf Eike Beer Note Added: 0029188
2012-04-16 16:38 Rolf Eike Beer Assigned To => Rolf Eike Beer
2012-04-16 16:38 Rolf Eike Beer Status new => resolved
2012-04-16 16:38 Rolf Eike Beer Resolution open => fixed
2012-04-16 16:38 Rolf Eike Beer Fixed in Version => CMake 2.8.8
2012-04-16 16:38 Rolf Eike Beer Target Version => CMake 2.8.8
2012-04-19 08:54 Rolf Eike Beer Status resolved => feedback
2012-04-19 08:54 Rolf Eike Beer Resolution fixed => reopened
2012-04-19 08:55 Rolf Eike Beer Status feedback => resolved
2012-04-19 08:55 Rolf Eike Beer Resolution reopened => fixed
2012-04-19 08:55 Rolf Eike Beer Fixed in Version CMake 2.8.8 => CMake 2.8.9
2012-04-19 08:55 Rolf Eike Beer Target Version CMake 2.8.8 => CMake 2.8.9
2012-04-19 09:09 Hans Johnson Note Added: 0029240
2012-04-19 09:09 Hans Johnson Status resolved => feedback
2012-04-19 09:09 Hans Johnson Resolution fixed => reopened
2012-04-19 09:40 Szilárd Páll Note Added: 0029242
2012-04-19 10:30 David Cole Note Added: 0029251
2012-04-19 10:58 Szilárd Páll Note Added: 0029252
2012-04-19 11:34 Rolf Eike Beer Note Added: 0029254
2012-04-19 12:06 David Cole Note Added: 0029255
2012-04-26 18:55 Rolf Eike Beer Note Added: 0029352
2012-06-04 17:02 David Cole Note Added: 0029608
2012-06-04 17:02 David Cole Status feedback => resolved
2012-06-04 17:02 David Cole Resolution reopened => fixed
2012-06-12 16:39 Brad King Relationship added has duplicate 0013294
2012-11-05 14:33 David Cole Note Added: 0031450
2012-11-05 14:33 David Cole Status resolved => closed


Copyright © 2000 - 2018 MantisBT Team