MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0009959CMakeCMakepublic2009-11-23 17:052010-12-14 18:49
ReporterPaul Oppenheim (Poppy Linden) 
Assigned ToDavid Cole 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-8 
Target VersionFixed in Version 
Summary0009959: new CMAKE_OSX_DEPLOYMENT_TARGET should set smarter if CMAKE_OSX_SYSROOT is defined
DescriptionI just got hosed by CMake 2.8's new CMAKE_OSX_DEPLOYMENT_TARGET code. I have read http://public.kitware.com/Bug/view.php?id=6195 [^] and feel that the code in Source/cmLocalGenerator.cxx doesn't match the (correct) research in that issue. The code naively assumes you want to set it to the current OS if it's not set, which is a bad, non-backwards-compatible idea. For instance, the Second Life project sets the CMAKE_OSX_SYSROOT to 10.4, clearly indicating we want 10.4 support. Modules/Platform/Darwin.cmake should not set anything by default, or check if the older variables are set, or at least issue a warning or error if one variable is set and not others.
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
related to 0006195closed David Cole Add a "CMAKE_OSX_DEPLOYMENT_TARGET" to pass -mmin-macosx-version to gcc 
related to 0009906closed Alex Neundorf CMake 2.8 applies OS X host information to all targets, ignoring cross-compilation use cases 
related to 0009898closed David Cole CMake OSX crash 
related to 0010155closed David Cole CMAKE_OSX_DEPLOYMENT_TARGET should be optional 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2009-11-23 17:05Paul Oppenheim (Poppy Linden)New Issue
2009-11-23 21:13Bill HoffmanStatusnew => assigned
2009-11-23 21:13Bill HoffmanAssigned To => Brad King
2009-11-24 08:01Brad KingAssigned ToBrad King => David Cole
2009-11-24 08:02Brad KingRelationship addedrelated to 0006195
2009-11-24 09:29Sean McBrideNote Added: 0018565
2009-11-24 15:27Paul Oppenheim (Poppy Linden)Note Added: 0018579
2009-11-24 17:18Sean McBrideNote Added: 0018583
2009-12-11 17:02David ColeNote Added: 0018830
2009-12-11 18:28Paul Oppenheim (Poppy Linden)Note Added: 0018836
2009-12-11 18:48Paul Oppenheim (Poppy Linden)Note Added: 0018837
2009-12-11 18:49Paul Oppenheim (Poppy Linden)Note Added: 0018838
2009-12-14 10:41Sean McBrideNote Added: 0018849
2009-12-14 11:09David ColeNote Added: 0018852
2009-12-14 11:39David ColeRelationship addedrelated to 0009906
2009-12-14 11:40David ColeRelationship addedrelated to 0009898
2009-12-15 11:10David ColeNote Added: 0018899
2009-12-15 11:10David ColeStatusassigned => resolved
2009-12-15 11:11David ColeResolutionopen => fixed
2010-01-29 11:34David ColeRelationship addedrelated to 0010155
2010-12-14 18:49David ColeNote Added: 0024059
2010-12-14 18:49David ColeStatusresolved => closed

Notes
(0018565)
Sean McBride   
2009-11-24 09:29   
Why would you explicitly set CMAKE_OSX_SYSROOT but not explicitly set CMAKE_OSX_DEPLOYMENT_TARGET? (I guess because the former is old, and has been set for a long time; and the latter is new.)

Still, theoretically, it could be a nice touch (if the user did not specify a CMAKE_OSX_DEPLOYMENT_TARGET) for CMake to set the deployment target to match the SDK (if the user did specify an SDK). I say theoretically because this seems fragile. CMake would need to hardcode a table of known SDK paths and the OS version they correspond to. What if the scheme changes in 10.7, 10.8? What is the current scheme on iPhone, would it work there?

Perhaps a safer idea would be for CMake to warn if one of CMAKE_OSX_DEPLOYMENT_TARGET or CMAKE_OSX_SYSROOT is set but the other is not?
(0018579)
Paul Oppenheim (Poppy Linden)   
2009-11-24 15:27   
Sean, exactly; legacy projects will likely have SYSROOT but not DEPLOYMENT_TARGET, because it is new as of 2.8.

And also true; the paths can change, but I imagine their names (last element of the path) will not. Not sure if that's a gamble worth putting in CMake. I'd imagine if no regex matched, then an error message could be thrown warning that one is set, and not the other.

I'm not sure if it's warning or error territory; with as much time as it took me and a coworker to debug (2 hrs each?), I'd be more for an error. Not setting both generates nearly impenetrable (and definitely non-googleable) libcrt errors, at least for the 10.4 / 10.5 mismatch. I can only daydream about how horrible the 10.6 / 10.3 error would be.
(0018583)
Sean McBride   
2009-11-24 17:18   
There is at least one precedent naming change. When Intel Macs came about, we got /Developer/SDKs/MacOSX10.4u.sdk. The "u" (for universal) was new. There had never been letters floating after the version number before.
(0018830)
David Cole   
2009-12-11 17:02   
sean, poppy, brad or bill (or anybody!):

Do any of you have a suggestion for how to fix this?

Is it as easy as this in Darwin.cmake?
  if(DEFINED CMAKE_OSX_SYSROOT)
    # set default deployment target to be the same as that implied by the SDK used by CMAKE_OSX_SYSROOT...
  else()
    # use current sw_vers technique to get default value of deployment target
  endif()

poppy:
You say "the code in Source/cmLocalGenerator.cxx" does not match the research in the related bug... How so? Looking in there, I see we only add "-mmacosx-version-min=" flag if there is a deployment target value. What is it that you think is incorrect about the code in cmLocalGenerator?

What was the original problem that you encountered? It is unclear to me what bad effect you encountered by having an old SDK but a newer deployment target. Did you get the FATAL_ERROR from the sanity check function that's in there...?
(0018836)
Paul Oppenheim (Poppy Linden)   
2009-12-11 18:28   
David,

You are correct, the C doesn't do anything if the value isn't set, but Darwin.cmake *always* sets it (line 75 in 2.8.0 release), which is part of the bad behavior. It should not do that. Furthermore, there should be a configure error if one is set and not the other.
(0018837)
Paul Oppenheim (Poppy Linden)   
2009-12-11 18:48   
Also, Sean,

For the record, the Darwin.cmake code already assumes the SDK name will be consistent in the future, and already does the reverse of what i'm suggesting; that is, it sets SYSROOT from DEPLOYMENT_TARGET. I would merely like to

* remove the behavior that causes DEPLOYMENT_TARGET to default to *anything*
* add behavior that causes DEPLOYMENT_TARGET to set SYSROOT if it's not set, or error out if that's not possible.
(0018838)
Paul Oppenheim (Poppy Linden)   
2009-12-11 18:49   
(you know, assuming that's the right decision in the first place ;P)
(0018849)
Sean McBride   
2009-12-14 10:41   
Some of your recent points were previously discussed in 0006195. Personally, I find it fragile to attempt to pick a default for these settings, but IIRC I was in the minority there. I'm just as ambivalent towards your proposed defaults as the existing ones. :)

However, I do think a warning/error is a good idea if one of deployment target / sdk is set and the other is not.
(0018852)
David Cole   
2009-12-14 11:09   
So just a note to explain our rationale and our thinking and why we did it this way in the first place... (I am rethinking it right now, but want the rest of you reading this issue to understand why we did it this way.)

When we are choosing behaviors like this for CMake, we often ask ourselves: what does the native tool do by default? CMake should do the same thing.

It seems to us that Xcode, when creating a new project through its UI, will result in a project that sets *both* deployment target *and* SDK to the same as the current OS that you are running on. So... we chose to set the deployment target *default* value to be the same as the current OS, and then choose the SDK that goes with that value of deployment target.

You are correct in reporting this bug that we should have considered the case about when an SDK is already set: in which case, it makes sense to set the default value of deployment target to be the same as that SDK value, or to leave it blank.

One of the goals of Darwin.cmake is to expose the following variables as *cache* entries in Mac-based CMake projects so that it is obvious-to/easy-to-tell people how to adjust these things: CMAKE_OSX_SYSROOT, CMAKE_OSX_ARCHITECTURES, CMAKE_OSX_DEPLOYMENT_TARGET.

So I think we want to end up with CMAKE_OSX_DEPLOYMENT_TARGET in the cache at the end of Darwin.cmake, but what its value should be needs some slight adjustment.

If CMAKE_OSX_SYSROOT is not set when we enter Darwin.cmake, then I think we should leave it as it is and choose deployment target according to current os, and then sysroot to match. And then cache them both.

If CMAKE_OSX_SYSROOT is set, then if there is no deployment target set, we should set the deployment target default value to the empty string. And then cache them both.

Something like this:
# Set CMAKE_OSX_DEPLOYMENT_TARGET_DEFAULT - if user has already specified an SDK
# with CMAKE_OSX_SYSROOT, then deployment target should default to "", otherwise,
# default it to the current OSX version.
#
IF(CMAKE_OSX_SYSROOT)
  SET(CMAKE_OSX_DEPLOYMENT_TARGET_DEFAULT "")
ELSE(CMAKE_OSX_SYSROOT)
  STRING(REGEX REPLACE "^.*(10)\\.([0-9]+)\\.*([0-9]+)*.*$" "\\1.\\2"
    CMAKE_OSX_DEPLOYMENT_TARGET_DEFAULT ${CURRENT_OSX_VERSION})
ENDIF(CMAKE_OSX_SYSROOT)


Does this logic hold water?

If I implement it, will you try it out with CVS CMake to make sure it works for you?
(0018899)
David Cole   
2009-12-15 11:10   
Fixed in CVS HEAD of CMake.

/cvsroot/CMake/CMake/Modules/Platform/Darwin.cmake,v <-- Modules/Platform/Darwin.cmake
new revision: 1.63; previous revision: 1.62
(0024059)
David Cole   
2010-12-14 18:49   
Closing bugs that have been resolved for more than 3 months without any further updates.