ENH: Cleanup building of OS X bundle content

 - Fixes repeated rebuild of bundles by Makefile generators
 - Add special rules to copy sources to their
   MACOSX_PACKAGE_LOCATION bundle directory
 - Remove MacOSX_Content language hack
   - Remove EXTRA_CONTENT property
   - Remove MACOSX_CONTENT
   - Remove corresponding special cases in object names

Modules/Platform/Darwin.cmake

@@ -109,8 +109,6 @@ ENDIF(XCODE)
 # line works around the problem.
 SET(CMAKE_LINK_DEPENDENT_LIBRARY_FILES 1)
 
-SET(CMAKE_MacOSX_Content_COMPILE_OBJECT "\"${CMAKE_COMMAND}\" -E copy_if_different <SOURCE> <OBJECT>")
-
 SET(CMAKE_C_CREATE_SHARED_LIBRARY_FORBIDDEN_FLAGS -w)
 SET(CMAKE_CXX_CREATE_SHARED_LIBRARY_FORBIDDEN_FLAGS -w)
 SET(CMAKE_C_CREATE_SHARED_LIBRARY

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -1887,74 +1887,32 @@ cmLocalUnixMakefileGenerator3
                     std::string* nameWithoutTargetDir,
                     bool* hasSourceExtension)
 {
-  if(const char* fileTargetDirectory =
-     source.GetProperty("MACOSX_PACKAGE_LOCATION"))
-    {
-    // Special handling for OSX package files.
-    std::string objectName =
-      this->GetObjectFileNameWithoutTarget(source, 0,
-                                           hasSourceExtension);
-    if(nameWithoutTargetDir)
-      {
-      *nameWithoutTargetDir = objectName;
-      }
-    objectName = cmSystemTools::GetFilenameName(objectName.c_str());
-    std::string targetName;
-    std::string targetNameReal;
-    std::string targetNameImport;
-    std::string targetNamePDB;
-    target.GetExecutableNames(targetName, targetNameReal, targetNameImport,
-                              targetNamePDB, this->ConfigurationName.c_str());
-    std::string obj;
-
-    // Construct the full path version of the names.
-    //
-    // If target is a MACOSX_BUNDLE target, then the package location is
-    // relative to "${targetDir}/${targetName}.app/Contents"... else it is
-    // relative to "${targetDir}"...
-    //
-    obj = target.GetDirectory();
-    obj += "/";
-    if ( target.GetPropertyAsBool("MACOSX_BUNDLE") )
-      {
-      obj += targetName + ".app/Contents/";
-      }
-    else
-      {
-      // Emit warning here...? MACOSX_PACKAGE_LOCATION is "most useful" in a
-      // MACOSX_BUNDLE...
-      }
-    obj += fileTargetDirectory;
-
-    // Object names are specified relative to the current build dir.
-    obj = this->Convert(obj.c_str(), START_OUTPUT);
-    obj += "/";
-    obj += objectName;
-    return obj;
-    }
-  else
+  // Make sure we never hit this old case.
+  if(source.GetProperty("MACOSX_PACKAGE_LOCATION"))
     {
-    // Start with the target directory.
-    std::string obj = this->GetTargetDirectory(target);
-    obj += "/";
-
-    // Get the object file name without the target directory.
-    std::string::size_type dir_len = 0;
-    dir_len += strlen(this->Makefile->GetCurrentOutputDirectory());
-    dir_len += 1;
-    dir_len += obj.size();
-    std::string objectName =
-      this->GetObjectFileNameWithoutTarget(source, dir_len,
-                                           hasSourceExtension);
-    if(nameWithoutTargetDir)
-      {
-      *nameWithoutTargetDir = objectName;
-      }
+    abort();
+    }
+
+  // Start with the target directory.
+  std::string obj = this->GetTargetDirectory(target);
+  obj += "/";
 
-    // Append the object name to the target directory.
-    obj += objectName;
-    return obj;
+  // Get the object file name without the target directory.
+  std::string::size_type dir_len = 0;
+  dir_len += strlen(this->Makefile->GetCurrentOutputDirectory());
+  dir_len += 1;
+  dir_len += obj.size();
+  std::string objectName =
+    this->GetObjectFileNameWithoutTarget(source, dir_len,
+                                         hasSourceExtension);
+  if(nameWithoutTargetDir)
+    {
+    *nameWithoutTargetDir = objectName;
     }
+
+  // Append the object name to the target directory.
+  obj += objectName;
+  return obj;
 }
 
 //----------------------------------------------------------------------------

Source/cmMakefileExecutableTargetGenerator.cxx

@@ -83,11 +83,6 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
       obj != this->Objects.end(); ++obj)
     {
     objTarget = relPath;
-    // Handle extra content on Mac bundles
-    if ( this->ExtraContent.find(*obj) != this->ExtraContent.end() )
-      {
-      objTarget = "";
-      }
     objTarget += *obj;
     depends.push_back(objTarget);
     }
@@ -120,8 +115,7 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
   // Construct the full path version of the names.
   std::string outpath = this->Target->GetDirectory();
   outpath += "/";
-#ifdef __APPLE__
-  if(this->Target->GetPropertyAsBool("MACOSX_BUNDLE"))
+  if(this->Target->IsAppBundleOnApple())
     {
     // Compute bundle directory names.
     std::string macdir = outpath;
@@ -132,27 +126,6 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
     cmSystemTools::MakeDirectory(outpath.c_str());
     outpath += "/";
 
-    // Make bundle directories
-    std::vector<cmSourceFile*>::const_iterator sourceIt;
-    for ( sourceIt = this->Target->GetSourceFiles().begin();
-      sourceIt != this->Target->GetSourceFiles().end();
-      ++ sourceIt )
-      {
-      const char* subDir = 
-        (*sourceIt)->GetProperty("MACOSX_PACKAGE_LOCATION");
-      if ( subDir )
-        {
-        std::string newDir = macdir;
-        newDir += subDir;
-        if ( !cmSystemTools::MakeDirectory(newDir.c_str()) )
-          {
-          cmSystemTools::Error("Cannot create a subdirectory for \"",
-            newDir.c_str(), "\".");
-          return;
-          }
-        }
-      }
-
     // Configure the Info.plist file.  Note that it needs the executable name
     // to be set.
     std::string plist = macdir + "Info.plist";
@@ -160,7 +133,6 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
                                                  targetName.c_str(),
                                                  plist.c_str());
     }
-#endif
   std::string outpathImp;
   if(relink)
     {

Source/cmMakefileTargetGenerator.cxx

@@ -150,6 +150,11 @@ void cmMakefileTargetGenerator::WriteTargetBuildRules()
           }
         }
       }
+    else if(const char* pkgloc =
+            (*source)->GetProperty("MACOSX_PACKAGE_LOCATION"))
+      {
+      this->WriteMacOSXContentRules(*(*source), pkgloc);
+      }
     else if(!(*source)->GetPropertyAsBool("HEADER_FILE_ONLY"))
       {
       if(!this->GlobalGenerator->IgnoreFile
@@ -313,6 +318,62 @@ void cmMakefileTargetGenerator::WriteTargetLanguageFlags()
     }
 }
 
+//----------------------------------------------------------------------------
+void cmMakefileTargetGenerator::WriteMacOSXContentRules(cmSourceFile& source,
+                                                        const char* pkgloc)
+{
+  // Skip OS X bundle content when not building a bundle.
+  if(!this->Target->IsAppBundleOnApple()) { return; }
+
+  // Create the directory in which the content is to be placed.
+  std::string targetName;
+  std::string targetNameReal;
+  std::string targetNameImport;
+  std::string targetNamePDB;
+  this->Target->GetExecutableNames
+    (targetName, targetNameReal, targetNameImport, targetNamePDB,
+     this->LocalGenerator->ConfigurationName.c_str());
+  std::string macdir = this->Target->GetDirectory();
+  macdir += "/";
+  macdir += targetName;
+  macdir += ".app/Contents/";
+  macdir += pkgloc;
+  cmSystemTools::MakeDirectory(macdir.c_str());
+
+  // Get the input file location.
+  std::string input = source.GetFullPath();
+
+  // Get the output file location.
+  std::string output = macdir;
+  output += "/";
+  output += cmSystemTools::GetFilenameName(input);
+  this->CleanFiles.push_back(this->Convert(output.c_str(),
+                                           cmLocalGenerator::START_OUTPUT));
+  output = this->Convert(output.c_str(), cmLocalGenerator::HOME_OUTPUT);
+
+  // Create a rule to copy the content into the bundle.
+  std::vector<std::string> depends;
+  std::vector<std::string> commands;
+  depends.push_back(input);
+  std::string copyEcho = "Copying Bundle content ";
+  copyEcho += output;
+  this->LocalGenerator->AppendEcho(commands, copyEcho.c_str(),
+                                   cmLocalUnixMakefileGenerator3::EchoBuild);
+  std::string copyCommand = "$(CMAKE_COMMAND) -E copy ";
+  copyCommand += this->Convert(input.c_str(),
+                               cmLocalGenerator::NONE,
+                               cmLocalGenerator::SHELL);
+  copyCommand += " ";
+  copyCommand += this->Convert(output.c_str(),
+                               cmLocalGenerator::NONE,
+                               cmLocalGenerator::SHELL);
+  commands.push_back(copyCommand);
+  this->LocalGenerator->WriteMakeRule(*this->BuildFileStream, 0,
+                                      output.c_str(),
+                                      depends, commands, false);
+  this->ExtraFiles.insert(output);
+}
+
 //----------------------------------------------------------------------------
 void cmMakefileTargetGenerator::WriteObjectRuleFiles(cmSourceFile& source)
 {
@@ -356,10 +417,6 @@ void cmMakefileTargetGenerator::WriteObjectRuleFiles(cmSourceFile& source)
     (this->LocalGenerator->ConvertToFullPath(dir).c_str());
 
   // Save this in the target's list of object files.
-  if ( source.GetPropertyAsBool("EXTRA_CONTENT") )
-    {
-    this->ExtraContent.insert(obj);
-    }
   this->Objects.push_back(obj);
   this->CleanFiles.push_back(obj);
 
@@ -417,10 +474,6 @@ cmMakefileTargetGenerator
   this->WriteObjectDependRules(source, depends);
 
   std::string relativeObj = this->LocalGenerator->GetHomeRelativeOutputPath();
-  if ( source.GetPropertyAsBool("MACOSX_CONTENT") )
-    {
-    relativeObj = "";
-    }
   relativeObj += obj;
   // Write the build rule.
 
@@ -1143,10 +1196,6 @@ cmMakefileTargetGenerator
   for(std::vector<std::string>::const_iterator i = this->Objects.begin();
       i != this->Objects.end(); ++i)
     {
-    if ( this->ExtraContent.find(i->c_str()) != this->ExtraContent.end() )
-      {
-      continue;
-      }
     *this->BuildFileStream << " " << lineContinue << "\n";
     if(objName)
       {
@@ -1277,10 +1326,6 @@ cmMakefileTargetGenerator
   for(std::vector<std::string>::const_iterator i = this->Objects.begin();
       i != this->Objects.end(); ++i)
     {
-    if ( this->ExtraContent.find(i->c_str()) != this->ExtraContent.end() )
-      {
-      continue;
-      }
     helper.Feed(*i);
     }
   for(std::vector<std::string>::const_iterator i =
@@ -1328,6 +1373,13 @@ void cmMakefileTargetGenerator::WriteTargetDriverRule(const char* main_output,
       {
       this->DriveCustomCommands(depends);
       }
+
+    // Make sure the extra files are built.
+    for(std::set<cmStdString>::const_iterator i = this->ExtraFiles.begin();
+        i != this->ExtraFiles.end(); ++i)
+      {
+      depends.push_back(*i);
+      }
     }
 
   // Write the driver rule.

Source/cmMakefileTargetGenerator.h

@@ -82,6 +82,9 @@ protected:
   // write the depend rules for this target
   void WriteTargetDependRules();
 
+  // write rules for Mac OS X Application Bundle content.
+  void WriteMacOSXContentRules(cmSourceFile& source, const char* pkgloc);
+
   // write the rules for an object
   void WriteObjectRuleFiles(cmSourceFile& source);
 
@@ -178,11 +181,13 @@ protected:
   // objects used by this target
   std::vector<std::string> Objects;
   std::vector<std::string> ExternalObjects;
-  std::set<std::string> ExtraContent;
 
   // Set of object file names that will be built in this directory.
   std::set<cmStdString> ObjectFiles;
 
+  // Set of extra output files to be driven by the build.
+  std::set<cmStdString> ExtraFiles;
+
   typedef std::map<cmStdString, cmStdString> MultipleOutputPairsType;
   MultipleOutputPairsType MultipleOutputPairs;
 

Source/cmSetPropertyCommand.cxx

@@ -295,11 +295,6 @@ bool cmSetPropertyCommand::HandleSource(cmSourceFile* sf)
     {
     sf->SetProperty(name, this->PropertyValue.c_str());
     }
-
-  // TODO: MACOSX_PACKAGE_LOCATION special case in
-  // cmSetSourceFilesPropertiesCommand
-  // The logic should be moved to cmSourceFile.
-
   return true;
 }
 

Source/cmSetSourceFilesPropertiesCommand.cxx

@@ -119,7 +119,6 @@ bool cmSetSourceFilesPropertiesCommand
       {
       // now loop through the rest of the arguments, new style
       ++j;
-      bool dontPush = false;
       while (j != propend)
         {
         propertyPairs.push_back(*j);
@@ -131,26 +130,6 @@ bool cmSetSourceFilesPropertiesCommand
             generated = true;
             }
           }
-        else if(*j == "MACOSX_PACKAGE_LOCATION")
-          {
-          ++j;
-          if(j == propend)
-            {
-            errors = "called with incorrect number of arguments "
-              "MACOSX_PACKAGE_LOCATION with no flags";
-            return false;
-            }
-          propertyPairs.push_back(*j);
-          propertyPairs.push_back("EXTRA_CONTENT");
-          propertyPairs.push_back("1");
-          propertyPairs.push_back("MACOSX_CONTENT");
-          propertyPairs.push_back("1");
-          propertyPairs.push_back("KEEP_EXTENSION");
-          propertyPairs.push_back("1");
-          propertyPairs.push_back("LANGUAGE");
-          propertyPairs.push_back("MacOSX_Content");
-          dontPush = true;
-          }
         else
           {
           ++j;
@@ -160,12 +139,8 @@ bool cmSetSourceFilesPropertiesCommand
           errors = "called with incorrect number of arguments.";
           return false;
           }
-        if ( !dontPush )
-          {
-          propertyPairs.push_back(*j);
-          }
+        propertyPairs.push_back(*j);
         ++j;
-        dontPush = false;
         }
       // break out of the loop because j is already == end
       break;

Source/cmSourceFile.cxx

@@ -421,13 +421,6 @@ void cmSourceFile::DefineProperties(cmake *cm)
      "is really an object file and should not be compiled.  "
      "It will still be linked into the target though.");
 
-  cm->DefineProperty
-    ("EXTRA_CONTENT", cmProperty::SOURCE_FILE, 
-     "Is this file part of a target's extra content.",
-     "If this property is set, the source file will be added to the "
-     "target's list of extra content. This is used by makefile "
-     "generators for some sort of Mac budle framework support.");
-
   cm->DefineProperty
     ("GENERATED", cmProperty::SOURCE_FILE, 
      "Is this source file generated as part of the build process.",
@@ -467,17 +460,15 @@ void cmSourceFile::DefineProperties(cmake *cm)
 
   cm->DefineProperty
     ("MACOSX_PACKAGE_LOCATION", cmProperty::SOURCE_FILE, 
-     "Location for MACOSX bundles and frameworks.",
-     "MACOSX_PACKAGE_LOCATION is the property of a file within a mac osx "
-     "bundle or framework that specifies where this file should be "
-     "copied. This makes sense for things like icons and other "
-     "resources.");
-
-  cm->DefineProperty
-    ("MACOSX_CONTENT", cmProperty::SOURCE_FILE, 
-     "If true then this is part of a MACOSX bundle or framework.",
-     "MACOSX_CONTENT is a flag that if true this file will be copied "
-     "to the bundle or framework.");
+     "Place a source file inside a Mac OS X application bundle.",
+     "Executable targets with the MACOSX_BUNDLE property set are built "
+     "as Mac OS X application bundles on Apple platforms.  "
+     "Source files specified for the target with the "
+     "MACOSX_PACKAGE_LOCATION property set will be placed in the "
+     "application bundle Contents folder under the directory specified "
+     "by the value of the property.  "
+     "Typically this is set to \"Resources\" for icon files and other "
+     "bundle resources.");
 
   cm->DefineProperty
     ("OBJECT_DEPENDS", cmProperty::SOURCE_FILE,