MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0008725CMakeCMakepublic2009-03-11 18:252009-10-22 13:06
ReporterSteve Wilson 
Assigned ToBrad King 
PrioritynormalSeverityfeatureReproducibilityN/A
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake-2-6 
Target VersionFixed in Version 
Summary0008725: Add OSX_ARCHITECTURES and OSX_ARCHITECTURES_<CONFIG> to target properties
DescriptionConsider the following source tree on OSX:

TopDir/
  - CMakeLists.txt
  - src1.c
  - src2.c

Where src1.c is compiled into a 32-bit(i386) binary that links with libExample1.a a non-Universal i386 architecture library.

Where src2.c is compiled into a 64-bit(x86_64) binary that links with libExample2.a a non-Universal x86_64 architecture library.

libExample1.a and libExample2.a exist outside of the scope of the source tree and cannot be rebuilt to be Universal libraries.

Here are the basic entries in CMakeLists.txt:

add_executable(src1 src1.c)

target_link_libraries(src1 Example1)

set_target_properties(src1 PROPERTIES COMPILE_FLAGS "-arch i386" LINK_FLAGS "-arch i386")

add_executable(src2 src2.c)

target_link_libraries(src2 Example2)

set_target_properties(src2 PROPERTIES COMPILE_FLAGS "-arch x86_64" LINK_FLAGS "-arch x86_64"

When generating a build system using the 'Unix Makefiles' generator this CMakeLists.txt will correctly build and link src1 and src2 as i386 and x86_64 binaries respectively.

When generating a build system using the Xcode generator this CMakeLists.txt will generate an Xcode project that cannot correctly build src2.

CMake provides default values for CMAKE_OSX_ARCHITECTURES. When the Xcode generator runs, it will use the default value for CMAKE_OSX_ARCHITECTURES (i386 in this case (assume a Core 2 Duo processor)) and the Xcode project will not have any explicit reference to i386 architectures when it is generated. Xcode will by default use the 'native' architecture for the given machine and automatically append -arch i386 to the compile/link commands for src1 and src2. Given that src2 must be an x86_64 architecture only binary (see dependency on libExample2.a) then src2 cannot link correctly.

Explicitly setting CMAKE_OSX_ARCHITECTURES to x86_64 in the CMakeLists.txt file does not solve the problem but rather switches the case to src1 which must be compiled as an i386 architecture only binary.

Explicitly setting CMAKE_OSX_ARCHITECTURES to x86_64;i386 violates the requirement that the binaries only have i386 and x86_64 architectures respectively.

This behavior makes it impossible to have working Makefile and Xcode build systems generated for this source tree. To put it another way, you cannot generate an Xcode project that contains targets with strict exclusive architecture requirements where those requirements vary by target.

In order to correct this deficiency, we need to have a property for targets that allows a target to override the project level CMAKE_OSX_ARCHITECTURES setting.

I propose the use of the following property names:

OSX_ARCHITECTURES and OSX_ARCHITECTURES_<CONFIG>

The CMAKE_OSX_ARCHITECTURES property sets the target binary architecture for targets on OSX. Use CMAKE_OSX_ARCHITECTURES_<CONFIG> to set the binary architectures on a per-configuration basis. <CONFIG> is an upper-case name (ex. CMAKE_OSX_ARCHITECTURES_DEBUG).

The upload tarball contains patches that implement this suggestion. The patches were created against the release sources of cmake 2.6.3.

I have built and used this patched version on OSX, Linux, Solaris, and Windows without any problems.

Let me know if I can provide any other help/comments/etc...

Steve
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Filestgz patches.tgz (2,436) 2009-03-11 18:25
https://public.kitware.com/Bug/file/2119/patches.tgz
tgz patches_CVS.tgz (2,475) 2009-10-14 16:58
https://public.kitware.com/Bug/file/2556/patches_CVS.tgz
Issue History
Date ModifiedUsernameFieldChange
2009-03-11 18:25Steve WilsonNew Issue
2009-03-11 18:25Steve WilsonFile Added: patches.tgz
2009-09-18 17:31Sean McBrideNote Added: 0017614
2009-09-18 17:54Steve WilsonNote Added: 0017615
2009-09-19 10:57Brad KingStatusnew => assigned
2009-09-19 10:57Brad KingAssigned To => Brad King
2009-09-19 11:15Brad KingNote Added: 0017641
2009-09-21 11:40Steve WilsonNote Added: 0017672
2009-10-14 16:58Steve WilsonNote Added: 0018070
2009-10-14 16:58Steve WilsonFile Added: patches_CVS.tgz
2009-10-19 16:28Brad KingNote Added: 0018114
2009-10-19 18:07Steve WilsonNote Added: 0018117
2009-10-19 18:24Brad KingNote Added: 0018118
2009-10-21 13:02Brad KingNote Added: 0018146
2009-10-21 13:03Brad KingNote Added: 0018147
2009-10-21 13:03Brad KingStatusassigned => resolved
2009-10-21 13:03Brad KingResolutionopen => fixed
2009-10-21 13:03Brad KingStatusresolved => feedback
2009-10-21 13:03Brad KingResolutionfixed => reopened
2009-10-22 13:00Steve WilsonNote Added: 0018193
2009-10-22 13:06Brad KingNote Added: 0018194
2009-10-22 13:06Brad KingStatusfeedback => closed
2009-10-22 13:06Brad KingResolutionreopened => fixed

Notes
(0017614)
Sean McBride   
2009-09-18 17:31   
Are libExample1.a and libExample2.a the same code but built for different architectures? If so, you could use lipo (see man lipo) to merge them into a Universal libExample.a.

And if you have arch-specific code (assembly perhaps?) in src1.c and src2.c you could wrap such code like so:

#if defined(__i386__)
#if defined(__x86_64__)

then build each universally (though one half will compile nothing). Does that help?
(0017615)
Steve Wilson   
2009-09-18 17:54   
These suggestions are not addressing the issue that I have raised. I am not looking for a work around, I am looking for a change in CMake that allows me to explicitly set the OSX architecture type on a per-target basis. My particular project has strict requirements where the src1 and src2 programs must be exclusively 32 and 64 bit and each component (ie libExample1 and libExample2) are also explicitly 32 and 64 bits. libExample1 and libExample2 are not combinable by lipo because they are different libraries.
One other way to work around this problem would be to move src1 and src2 into separate project directories with their own specific architecture settings, but I do not have the freedom to reorganize my companies source code in this manner.

The issue here is that we need a way to explicitly set the architecture for OSX on a per-target basis.
(0017641)
Brad King   
2009-09-19 11:15   
I'm sorry I didn't see this earlier. I wasn't aware of this issue's submission.

Support for per-target OSX_ARCHITECTURES is reasonable. Your patches look pretty good. However, we've just made some changes to the handling of CMAKE_OSX_ARCHITECTURES to support Snow Leopard. I've applied most of your patches cleanly on my local disk, but the actual AddLanguageFlags and ARCHS Xcode property code conflicts.

It may take a couple more days for our CMAKE_OSX_ARCHITECTURES changes to settle as they are tested. After that, I'll be happy to work with you on this feature. Do you mind updating the patches to apply against current CVS HEAD (perhaps after our changes settle)?

I have one comment on the approach. We have a convention for target properties that were converted from existing variable-based settings. Look in the "cmTarget::SetMakefile" method in cmTarget.cxx for lines like:

  this->SetPropertyDefault("INSTALL_NAME_DIR", "");
  this->SetPropertyDefault("INSTALL_RPATH", "");
  this->SetPropertyDefault("INSTALL_RPATH_USE_LINK_PATH", "OFF");
  ...

These initialize properties from variables named "CMAKE_<property-name>" at the point of creation of each target. This convention should be used to initialize the OSX_ARCHITECTURES property from CMAKE_OSX_ARCHITECTURES. The advantage of this approach is that it allows us to replace the variable-based implementation with a property-based one while retaining compatibility.
(0017672)
Steve Wilson   
2009-09-21 11:40   
Brad,

Thanks so much for looking at this proposal! I will happily revisit the patches in a day or two against the changes in the head branch. I will also examine the convention in cmTarget.cxx that you mentioned.

Steve
(0018070)
Steve Wilson   
2009-10-14 16:58   
Brad,

I am uploading a tarball with patches taken against my changes for OSX_ARCHITECTURES with the CMake tree directly from CVS. These sources were updated this afternoon (10-14-09). The patch file is patches_CVS.tgz.

I add the this->SetPropertyDefault() entry for OSX_ARCHITECTURES in cmTarget.cxx, but I wasn't entirely sure after viewing the properties that were initialized there if you meant that I should actually initialize the value based of the variable (CMAKE_OSX_ARCHITECTURES) or if I should model the other properties and just provide a default value. I chose to provide a default value.

Let me know if you need anything else.

Steve
(0018114)
Brad King   
2009-10-19 16:28   
I've applied the patches locally and made a few tweaks. Overall it looks good.

The purpose of SetPropertyDefault without a given default is to copy the value of the variable "CMAKE_<property-name>" when the target is created. This way we need not look up the value at generate time, and the variable can even be changed between creation of two targets. The patch can be simplified by dropping the lookup of CMAKE_OSX_ARCHITECTURES and using '0' in the SetPropertyDefault call. I've made these changes locally.

The remaining question is whether the per-configuration version of OSX_ARCHITECTURES (OSX_ARCHITECTURES_<CONFIG>) should append or replace the original value. Currently the patch appends. I think it should totally override the original property if set. Users can always simulate appending by getting the main property, appending, and then setting the per-config property.
(0018117)
Steve Wilson   
2009-10-19 18:07   
I think that overriding the value of OSX_ARCHITECTURES with the value of OSX_ARCHITECTURES_<CONFIG> is good idea.

Steve
(0018118)
Brad King   
2009-10-19 18:24   
Great, I'll make that change and test this as soon as I get a chance. Thanks for the patch!
(0018146)
Brad King   
2009-10-21 13:02   
I re-worked the patch somewhat extensively to implement the behavior differences I described. I also added a test.

Define per-target OSX_ARCHITECTURES property
/cvsroot/CMake/CMake/Source/cmGlobalXCodeGenerator.cxx,v <-- Source/cmGlobalXCodeGenerator.cxx
new revision: 1.235; previous revision: 1.234
/cvsroot/CMake/CMake/Source/cmLocalGenerator.cxx,v <-- Source/cmLocalGenerator.cxx
new revision: 1.321; previous revision: 1.320
/cvsroot/CMake/CMake/Source/cmLocalGenerator.h,v <-- Source/cmLocalGenerator.h
new revision: 1.120; previous revision: 1.119
/cvsroot/CMake/CMake/Source/cmMakefileExecutableTargetGenerator.cxx,v <-- Source/cmMakefileExecutableTargetGenerator.cxx
new revision: 1.67; previous revision: 1.66
/cvsroot/CMake/CMake/Source/cmMakefileLibraryTargetGenerator.cxx,v <-- Source/cmMakefileLibraryTargetGenerator.cxx
new revision: 1.83; previous revision: 1.82
/cvsroot/CMake/CMake/Source/cmMakefileTargetGenerator.cxx,v <-- Source/cmMakefileTargetGenerator.cxx
new revision: 1.130; previous revision: 1.129
/cvsroot/CMake/CMake/Source/cmTarget.cxx,v <-- Source/cmTarget.cxx
new revision: 1.282; previous revision: 1.281
/cvsroot/CMake/CMake/Source/cmTarget.h,v <-- Source/cmTarget.h
new revision: 1.149; previous revision: 1.148

Test OSX_ARCHITECTURES target property
/cvsroot/CMake/CMake/Tests/Architecture/CMakeLists.txt,v <-- Tests/Architecture/CMakeLists.txt
initial revision: 1.1
/cvsroot/CMake/CMake/Tests/Architecture/bar.c,v <-- Tests/Architecture/bar.c
initial revision: 1.1
/cvsroot/CMake/CMake/Tests/Architecture/foo.c,v <-- Tests/Architecture/foo.c
initial revision: 1.1
/cvsroot/CMake/CMake/Tests/CMakeLists.txt,v <-- Tests/CMakeLists.txt
new revision: 1.134; previous revision: 1.133
(0018147)
Brad King   
2009-10-21 13:03   
Please try out CMake from CVS HEAD to see if it implements the feature sufficiently.
(0018193)
Steve Wilson   
2009-10-22 13:00   
I tried out the changes today in my build that makes use of the OSX_ARCHITECTURES property and both the Unix Makefiles version and the Xcode version worked perfectly.

Thanks so much for working on these changes!
(0018194)
Brad King   
2009-10-22 13:06   
Cool. Thanks for testing.

FYI, the next 2.8.0 release candidate will contain only fixes for regressions from previous CMake versions (plus a couple trivial changes). This change is a new feature, so it may not make it until 2.8.1.