MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0011469CMakeCMakepublic2010-11-17 09:112011-01-31 16:10
ReporterVladislav Vaintroub 
Assigned ToBrad King 
PriorityurgentSeverityblockReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake 2.8.3 
Target VersionCMake 2.8.4Fixed in VersionCMake 2.8.4 
Summary0011469: CMAKE_USER_MAKE_RULES_OVERRIDE with system checks goes into recursion and aborts
DescriptionWe (MySQL) use some system checks in CMAKE_USER_MAKE_RULES_OVERRIDE file - header file existence, bitness etc.

It worked fine in 2.6.1 to 2.8.2, but it broke in 2.8.3.

It looks like cmake would go into recursion, and break with "no current directory" when stack is exhausted.
Steps To ReproduceI attach a sample project to demonstrate the behavior.
Basically, having just "CHECK_TYPE_SIZE" macro in CMAKE_USER_MAKE_RULES_OVERRIDE file will cause the trouble, e.g file with contents:

MESSAGE("in rules.cmake")
INCLUDE(CheckTypeSize)
CHECK_TYPE_SIZE("void *" SIZEOF_VOIDP)
Additional Informationhttp://www.vtk.org/Bug/view.php?id=10902 [^] might be the reason for breakage
TagsNo tags attached.
Relationships
related to 0010902closed Brad King USER_MAKE_RULES_OVERRIDE variable is not used in initial try_compile 
related to 0011942closed David Cole INCLUDE inside file defined by CMAKE_USER_MAKE_RULES_OVERRIDE broken since 2.8.3 
Attached Filesgz my_project.tar.gz (441) 2010-11-17 09:11
https://public.kitware.com/Bug/file/3507/my_project.tar.gz
Issue History
Date ModifiedUsernameFieldChange
2010-11-17 09:11Vladislav VaintroubNew Issue
2010-11-17 09:11Vladislav VaintroubFile Added: my_project.tar.gz
2010-11-17 09:53Brad KingRelationship addedrelated to 0010902
2010-11-17 09:56Brad KingNote Added: 0023333
2010-11-17 10:18Vladislav VaintroubNote Added: 0023335
2010-11-17 10:26Brad KingNote Added: 0023340
2010-11-17 10:43Vladislav VaintroubNote Added: 0023346
2010-11-17 10:49Brad KingNote Added: 0023348
2010-11-17 12:45Rolf Eike BeerNote Added: 0023351
2010-11-17 12:52Vladislav VaintroubNote Added: 0023352
2010-11-17 16:15Vladislav VaintroubNote Added: 0023353
2010-11-17 16:30Brad KingAssigned To => Bill Hoffman
2010-11-17 16:30Brad KingStatusnew => assigned
2010-11-17 16:32Brad KingNote Added: 0023354
2010-11-17 16:42Vladislav VaintroubNote Added: 0023355
2010-11-17 17:18Brad KingNote Added: 0023361
2011-01-21 19:12David ColeAssigned ToBill Hoffman => Brad King
2011-01-21 19:14David ColeNote Added: 0025015
2011-01-24 10:05Brad KingNote Added: 0025023
2011-01-24 10:05Brad KingStatusassigned => closed
2011-01-24 10:05Brad KingResolutionopen => fixed
2011-01-31 16:10David ColeFixed in Version => CMake 2.8.4
2011-01-31 16:10David ColeTarget Version => CMake 2.8.4
2011-03-31 12:36Brad KingRelationship addedrelated to 0011942

Notes
(0023333)
Brad King   
2010-11-17 09:56   
Yes, 0010902 is the cause. CMAKE_USER_MAKE_RULES_OVERRIDE *has* to be used in try_compiles to support them in toolchains that need them (hence 0010902).

Running a try_compile inside the user rules script was never an anticipated, documented, or supported behavior. I can't believe it even worked accidentally before. The language initialization isn't finished at the time the script is loaded so the generators do not have enough information to produce fully correct build files.

What was the reason for this? There must be another way to do it.
(0023335)
Vladislav Vaintroub   
2010-11-17 10:18   
>Running a try_compile inside the user rules script was never an anticipated, documented, or supported behavior.

Well, the documentation never stated what was anticipated or supported. This is a cmake file, and we added statements typical for CMake files and used it because it worked. And it is used to make MySQL releases, too

This page http://www.perkin.org.uk/ [^] describes how MySQL releases are done
(the -DBUILD_CONFIG=mysql_release parameter will cause inclusion of cmake/build_configurations/mysql_release.cmake, which has the CHECK_TYPE_SIZE and other stuff inside)


>I can't believe it even worked accidentally before.
You can check it, I can provide instructions for MySQL.

>What was the reason for this? There must be another way to do it.
The main reason is to find out the compiler bitness, some flags are set dependent on bitness (don't why, it was always been that way on Solaris)
(0023340)
Brad King   
2010-11-17 10:26   
Why can't those "bitness" flags be added after the fact:

 set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fancybitnessflag")

? Do they *have* to be in the cache for the user to edit/break?
(0023346)
Vladislav Vaintroub   
2010-11-17 10:43   
They can be set later in theory, but what would be the purpose of the MAKE_USER_MAKE_RULES_OVERRIDE then ? We used this file to define all flags used by release builds, so that the flags are not spread around in different places.
(0023348)
Brad King   
2010-11-17 10:49   
The variable is meant as a last-resort for projects to pick alternative *default* values for the *cache* entries like CMAKE_C_FLAGS. The scripts can also override the platform file rules for running the compiler, but the need for this is generally outdated now by better per-compiler platform files.

Instead, just add your own release flags script inclusion:

  cmake_minimum_required(VERSION ...)
  project(MySQL ...)
  if(MySQL_RELEASE_RULES)
    include("${MySQL_RELEASE_RULES}")
  endif()
(0023351)
Rolf Eike Beer   
2010-11-17 12:45   
Why did you check the pointer size yourself? There is a CMAKE_SIZEOF_VOID_P.
(0023352)
Vladislav Vaintroub   
2010-11-17 12:52   
There is CMAKE_SIZEOF_VOID_P, but not prior to PROJECT
(0023353)
Vladislav Vaintroub   
2010-11-17 16:15   
Brad, yes, I think using INCLUDE would be the way to go. The single downside of it is not having flags in the cache. It is merely cosmetics, however the cached flags were used (at least in the past) for automation and verification work.
 
I created a bug for MySQL http://bugs.mysql.com/bug.php?id=58272 [^] and proposed a fix http://lists.mysql.com/commits/124202 [^] to fix the problem in MySQL, lets see if it gets accepted.

I guess the priority of that bug can be lowered now.
(0023354)
Brad King   
2010-11-17 16:32   
Okay, the "new" behavior of CMAKE_USER_MAKE_RULES_OVERRIDE is correct from CMake's point of view. Since it does not affect end-user MySQL builds but only packaging it is not as critical that we deal with this on the CMake end. If we do need to deal with it we will need to introduce a new policy.

Otherwise, this issue now remains open only as a task to get the MySQL contract test added to CMake.
(0023355)
Vladislav Vaintroub   
2010-11-17 16:42   
I still think there is a better way to handle TRY_COMPILE inside CMAKE_USER_MAKE_RULES_OVERRIDE than the recursion and stack exhaustion. On Windows the output looks even more weird than on Linux..

It does affect our users somewhat, since we documented "How to build like we do in releases" on some places, but maybe if we manage to fix it quickly hopefully not many will notice.
(0023361)
Brad King   
2010-11-17 17:18   
Bill, cmGlobalGenerator::EnableLanguage should somehow block generation of build rules for a language until it is enabled fully (at least enough to load CMakeTest*Compiler.cmake). That way any try_compile command inside the override script would report an error from the generator.
(0025015)
David Cole   
2011-01-21 19:14   
Brad, is this fixed by your recent CMAKE_USER_MAKE_RULES_OVERRIDE change, or do we still need include blockers? (Or some other solution for this issue...?)
(0025023)
Brad King   
2011-01-24 10:05   
Blocker added:

  http://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=c83a834d [^]

Now if the override file does a try_compile one gets

CMake Error at /.../Modules/CheckIncludeFile.cmake:48 (TRY_COMPILE):
  The test project needs language C which is not enabled.
Call Stack (most recent call first):
  /.../Modules/CheckTypeSize.cmake:158 (check_include_file)
  UserOverride.cmake:2 (check_type_size)
  /.../Modules/CMakeCInformation.cmake:78 (INCLUDE)
  CMakeLists.txt:3 (project)