BUG: Correct Mac OS X framework behavior

  - Place the built library in foo.framework/Versions/A/foo
  - Do not create unused content symlinks (like PrivateHeaders)
  - Do not use VERSION/SOVERSION properties for frameworks
  - Make cmTarget::GetDirectory return by value
  - Remove the foo.framework part from cmTarget::GetDirectory
  - Correct install_name construction and conversion on install
  - Fix MACOSX_PACKAGE_LOCATION under Xcode to use the
    Versions/<version> directory for frameworks
  - Update the Framework test to try these things

Source/cmComputeLinkInformation.cxx

@@ -611,18 +611,8 @@ void cmComputeLinkInformation::AddItem(std::string const& item, cmTarget* tgt)
       std::string lib = tgt->GetFullPath(config, implib, true);
       this->Depends.push_back(lib);
 
-      if(tgt->IsFrameworkOnApple())
-        {
-        // Frameworks on OS X need only the framework directory to
-        // link.
-        std::string fw = tgt->GetDirectory(config, implib);
-        this->AddFrameworkItem(fw);
-        }
-      else
-        {
-        this->AddTargetItem(lib, tgt);
-        this->AddLibraryRuntimeInfo(lib, tgt);
-        }
+      this->AddTargetItem(lib, tgt);
+      this->AddLibraryRuntimeInfo(lib, tgt);
       }
     }
   else
@@ -1023,7 +1013,7 @@ void cmComputeLinkInformation::AddTargetItem(std::string const& item,
 
   // For compatibility with CMake 2.4 include the item's directory in
   // the linker search path.
-  if(this->OldLinkDirMode &&
+  if(this->OldLinkDirMode && !target->IsFrameworkOnApple() &&
      this->OldLinkDirMask.find(cmSystemTools::GetFilenamePath(item)) ==
      this->OldLinkDirMask.end())
     {

Source/cmGlobalXCodeGenerator.cxx

@@ -803,8 +803,14 @@ cmGlobalXCodeGenerator::CreateXCodeTargets(cmLocalGenerator* gen,
         copyFilesBuildPhase->AddAttribute("dstSubfolderSpec",
           this->CreateString("6"));
         cmOStringStream ostr;
-        if ( mit->first != "MacOS" )
+        if (cmtarget.IsFrameworkOnApple())
           {
+          // dstPath in frameworks is relative to Versions/<version>
+          ostr << mit->first;
+          }
+        else if ( mit->first != "MacOS" )
+          {
+          // dstPath in bundles is relative to Contents/MacOS
           ostr << "../" << mit->first.c_str();
           }
         copyFilesBuildPhase->AddAttribute("dstPath",
@@ -1357,11 +1363,6 @@ void cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget& target,
      target.GetType() == cmTarget::EXECUTABLE)
     {
     std::string pndir = target.GetDirectory();
-    if(target.IsFrameworkOnApple())
-      {
-      pndir += "/..";
-      pndir = cmSystemTools::CollapseFullPath(pndir.c_str());
-      }
     buildSettings->AddAttribute("SYMROOT", 
                                 this->CreateString(pndir.c_str()));
     buildSettings->AddAttribute("EXECUTABLE_PREFIX", 
@@ -1429,17 +1430,9 @@ void cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget& target,
       fileType = "wrapper.framework";
       productType = "com.apple.product-type.framework";
 
-      const char* version = target.GetProperty("FRAMEWORK_VERSION");
-      if(!version)
-        {
-        version = target.GetProperty("VERSION");
-        }
-      if(!version)
-        {
-        version = "A";
-        }
+      std::string version = target.GetFrameworkVersion();
       buildSettings->AddAttribute("FRAMEWORK_VERSION",
-                                  this->CreateString(version));
+                                  this->CreateString(version.c_str()));
       }
     else
       {
@@ -1649,18 +1642,7 @@ void cmGlobalXCodeGenerator::CreateBuildSettings(cmTarget& target,
   if(target.GetType() == cmTarget::SHARED_LIBRARY)
     {
     // Get the install_name directory for the build tree.
-    install_name_dir = target.GetInstallNameDirForBuildTree(configName);
-    if(target.GetPropertyAsBool("FRAMEWORK"))
-      {
-      if(install_name_dir.find(".framework") != install_name_dir.npos)
-        {
-        install_name_dir = install_name_dir + "/..";
-        install_name_dir = 
-            cmSystemTools::CollapseFullPath(install_name_dir.c_str());
-        //std::cerr << "new install name " << install_name_dir << "\n";
-        }
-      }
-    
+    install_name_dir = target.GetInstallNameDirForBuildTree(configName, true);
     if(install_name_dir.empty())
       {
       // Xcode will not pass the -install_name option at all if INSTALL_PATH
@@ -2868,26 +2850,9 @@ cmGlobalXCodeGenerator
     {
     if(config)
       {
-      if(dir.find(".framework") != dir.npos)
-        {
-        // Remove trailing slashes (so that the rfind does not find the one at
-        // the very end...!)
-        //
-        cmSystemTools::ConvertToUnixSlashes(dir);
-        std::string::size_type pos = dir.rfind("/");
-        std::string framework = dir.substr(pos);
-        std::string newDir = dir.substr(0, pos);
-        newDir += "/";
-        newDir += config;
-        dir = newDir;
-        dir += framework;
-        }
-      else
-        {
-        dir += prefix;
-        dir += config;
-        dir += suffix;
-        }
+      dir += prefix;
+      dir += config;
+      dir += suffix;
       }
     }
 }

Source/cmInstallTargetGenerator.cxx

@@ -239,16 +239,17 @@ cmInstallTargetGenerator
 
       // Compute the build tree location of the framework directory
       std::string from1 = fromDirConfig;
-      // Remove trailing slashes... so that from1 ends with ".framework":
-      //
-      cmSystemTools::ConvertToUnixSlashes(from1);
+      from1 += targetName;
+      from1 += ".framework";
       files.push_back(from1);
 
       type = cmTarget::INSTALL_DIRECTORY;
 
       // Need to apply install_name_tool and stripping to binary
       // inside framework.
-      toInstallPath += ".framework/";
+      toInstallPath += ".framework/Versions/";
+      toInstallPath += this->Target->GetFrameworkVersion();
+      toInstallPath += "/";
       toInstallPath += this->GetInstallFilename(this->Target, config,
                                                 NameNormal);
 

Source/cmLocalVisualStudio7Generator.cxx

@@ -668,7 +668,7 @@ void cmLocalVisualStudio7Generator::WriteConfiguration(std::ostream& fout,
   if ( this->Version >= 8 )
     {
     // Check the filesystem type where the target will be written.
-    if(cmLVS6G_IsFAT(target.GetDirectory(configName)))
+    if(cmLVS6G_IsFAT(target.GetDirectory(configName).c_str()))
       {
       // Add a flag telling the manifest tool to use a workaround
       // for FAT32 file systems, which can cause an empty manifest

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -39,20 +39,11 @@ cmMakefileLibraryTargetGenerator
 
   if(this->Target->IsFrameworkOnApple())
     {
-    if(const char* fversion = this->Target->GetProperty("FRAMEWORK_VERSION"))
-      {
-      this->FrameworkVersion = fversion;
-      }
-    else if(const char* tversion = this->Target->GetProperty("VERSION"))
-      {
-      this->FrameworkVersion = tversion;
-      }
-    else
-      {
-      this->FrameworkVersion = "A";
-      }
+    this->FrameworkVersion = this->Target->GetFrameworkVersion();
     this->MacContentDirectory = this->Target->GetDirectory();
-    this->MacContentDirectory += "/Versions/";
+    this->MacContentDirectory += "/";
+    this->MacContentDirectory += this->TargetNameOut;
+    this->MacContentDirectory += ".framework/Versions/";
     this->MacContentDirectory += this->FrameworkVersion;
     this->MacContentDirectory += "/";
     }
@@ -244,50 +235,82 @@ void cmMakefileLibraryTargetGenerator::WriteFrameworkRules(bool relink)
 }
 
 //----------------------------------------------------------------------------
-void cmMakefileLibraryTargetGenerator::CreateFramework(
-  std::string& targetName,
-  std::string& outpath)
+void cmMakefileLibraryTargetGenerator::CreateFramework()
 {
-  std::string symlink;
-  std::string symlink2;
+  // TODO: Use the cmMakefileTargetGenerator::ExtraFiles vector to
+  // drive rules to create these files at build time.
+  std::string oldName;
+  std::string newName;
+
+  // Compute the location of the top-level foo.framework directory.
+  std::string top = this->Target->GetDirectory();
+  top += "/";
+  top += this->TargetNameOut;
+  top += ".framework/";
+
   // Make foo.framework/Versions
-  std::string dir = outpath;
-  dir += "Versions";
-  cmSystemTools::MakeDirectory(dir.c_str());
-  std::string cwd = cmSystemTools::GetCurrentWorkingDirectory();
-  // cd foo.framework to setup symlinks with relative paths
-  cmSystemTools::ChangeDirectory((outpath+"Versions").c_str());
+  std::string versions = top;
+  versions += "Versions";
+  cmSystemTools::MakeDirectory(versions.c_str());
+
+  // Make foo.framework/Versions/version
+  std::string version = versions;
+  version += "/";
+  version += this->FrameworkVersion;
+  cmSystemTools::MakeDirectory(version.c_str());
+
   // Current -> version
-  symlink = this->FrameworkVersion;
-  symlink2 = "Current";
-  cmSystemTools::RemoveFile("Current");
-  cmSystemTools::CreateSymlink(symlink.c_str(), symlink2.c_str());
-  this->Makefile->AddCMakeOutputFile((outpath + "Versions/Current").c_str());
-  // change to top level of framework to create next set of symlinks
-  cmSystemTools::ChangeDirectory(outpath.c_str());
+  oldName = this->FrameworkVersion;
+  newName = versions;
+  newName += "/Current";
+  cmSystemTools::RemoveFile(newName.c_str());
+  cmSystemTools::CreateSymlink(oldName.c_str(), newName.c_str());
+  this->Makefile->AddCMakeOutputFile(newName.c_str());
+
   // foo -> Versions/Current/foo
-  symlink = "Versions/Current/";
-  symlink += targetName;
-  symlink2 = targetName;
-  cmSystemTools::CreateSymlink(symlink.c_str(), symlink2.c_str());
-  this->Makefile->AddCMakeOutputFile((outpath + targetName).c_str());
-  // Resources -> Versions/Current/Resources 
-  symlink = "Versions/Current/Resources";
-  symlink2 = "Resources";
-  cmSystemTools::CreateSymlink(symlink.c_str(), symlink2.c_str());
-  this->Makefile->AddCMakeOutputFile((outpath + "Resources").c_str());
+  oldName = "Versions/Current/";
+  oldName += this->TargetNameOut;
+  newName = top;
+  newName += this->TargetNameOut;
+  cmSystemTools::RemoveFile(newName.c_str());
+  cmSystemTools::CreateSymlink(oldName.c_str(), newName.c_str());
+  this->Makefile->AddCMakeOutputFile(newName.c_str());
+
+  // Resources -> Versions/Current/Resources
+  if(this->MacContentFolders.find("Resources") !=
+     this->MacContentFolders.end())
+    {
+    oldName = "Versions/Current/Resources";
+    newName = top;
+    newName += "Resources";
+    cmSystemTools::RemoveFile(newName.c_str());
+    cmSystemTools::CreateSymlink(oldName.c_str(), newName.c_str());
+    this->Makefile->AddCMakeOutputFile(newName.c_str());
+    }
+
   // Headers -> Versions/Current/Headers
-  symlink = "Versions/Current/Headers";
-  symlink2 = "Headers";
-  cmSystemTools::CreateSymlink(symlink.c_str(), symlink2.c_str());
-  this->Makefile->AddCMakeOutputFile((outpath + "Headers").c_str());
+  if(this->MacContentFolders.find("Headers") !=
+     this->MacContentFolders.end())
+    {
+    oldName = "Versions/Current/Headers";
+    newName = top;
+    newName += "Headers";
+    cmSystemTools::RemoveFile(newName.c_str());
+    cmSystemTools::CreateSymlink(oldName.c_str(), newName.c_str());
+    this->Makefile->AddCMakeOutputFile(newName.c_str());
+    }
+
   // PrivateHeaders -> Versions/Current/PrivateHeaders
-  symlink = "Versions/Current/PrivateHeaders";
-  symlink2 = "PrivateHeaders";
-  cmSystemTools::CreateSymlink(symlink.c_str(), symlink2.c_str());
-  this->Makefile->AddCMakeOutputFile((outpath + "PrivateHeaders").c_str());
-  // go back to where we were
-  cmSystemTools::ChangeDirectory(cwd.c_str());
+  if(this->MacContentFolders.find("PrivateHeaders") !=
+     this->MacContentFolders.end())
+    {
+    oldName = "Versions/Current/PrivateHeaders";
+    newName = top;
+    newName += "PrivateHeaders";
+    cmSystemTools::RemoveFile(newName.c_str());
+    cmSystemTools::CreateSymlink(oldName.c_str(), newName.c_str());
+    this->Makefile->AddCMakeOutputFile(newName.c_str());
+    }
 }
 
 //----------------------------------------------------------------------------
@@ -354,7 +377,12 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
   // Construct the full path version of the names.
   std::string outpath;
   std::string outpathImp;
-  if(relink)
+  if(this->Target->IsFrameworkOnApple())
+    {
+    outpath = this->MacContentDirectory;
+    this->CreateFramework();
+    }
+  else if(relink)
     {
     outpath = this->Makefile->GetStartOutputDirectory();
     outpath += cmake::GetCMakeFilesDirectory();
@@ -379,12 +407,6 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
       }
     }
 
-  // If we're creating a framework, place the output into a framework directory
-  if(this->Target->IsFrameworkOnApple())
-    {
-    this->CreateFramework(targetName, outpath);
-    }
-
   std::string targetFullPath = outpath + targetName;
   std::string targetFullPathPDB = outpath + targetNamePDB;
   std::string targetFullPathSO = outpath + targetNameSO;

Source/cmMakefileLibraryTargetGenerator.h

@@ -37,8 +37,7 @@ protected:
                          bool relink);
   // MacOSX Framework support methods
   void WriteFrameworkRules(bool relink);
-  void CreateFramework(std::string& targetName,
-                       std::string& outpath);
+  void CreateFramework();
 
   // Store the computd framework version for OS X Frameworks.
   std::string FrameworkVersion;

Source/cmMakefileTargetGenerator.cxx

@@ -334,6 +334,9 @@ void cmMakefileTargetGenerator::WriteMacOSXContentRules(cmSourceFile& source,
   macdir += pkgloc;
   cmSystemTools::MakeDirectory(macdir.c_str());
 
+  // Record use of this content location.
+  this->MacContentFolders.insert(pkgloc);
+
   // Get the input file location.
   std::string input = source.GetFullPath();
 
@@ -1456,11 +1459,8 @@ void cmMakefileTargetGenerator
       if(cmTarget* tgt =
          this->GlobalGenerator->FindTarget(0, lib->first.c_str()))
         {
-        if(const char* location =
-           tgt->GetLocation(this->LocalGenerator->ConfigurationName.c_str()))
-          {
-          depends.push_back(location);
-          }
+        const char* config = this->LocalGenerator->ConfigurationName.c_str();
+        depends.push_back(tgt->GetFullPath(config, false));
         }
       // depend on full path libs as well
       else if(cmSystemTools::FileIsFullPath(lib->first.c_str()))

Source/cmMakefileTargetGenerator.h

@@ -204,6 +204,7 @@ protected:
 
   // Mac OS X content info.
   std::string MacContentDirectory;
+  std::set<cmStdString> MacContentFolders;
 
   // Target-wide Fortran module output directory.
   bool FortranModuleDirectoryComputed;

Source/cmTarget.cxx

@@ -1626,42 +1626,30 @@ void cmTarget::MarkAsImported()
 }
 
 //----------------------------------------------------------------------------
-const char* cmTarget::GetDirectory(const char* config, bool implib)
+std::string cmTarget::GetDirectory(const char* config, bool implib)
 {
   if (this->IsImported())
     {
-    return this->ImportedGetDirectory(config, implib);
-    }
-  else
-    {
-    return this->NormalGetDirectory(config, implib);
-    }
-}
-
-//----------------------------------------------------------------------------
-const char* cmTarget::ImportedGetDirectory(const char* config, bool implib)
-{
-  this->Directory =
-    cmSystemTools::GetFilenamePath(
+    // Return the directory from which the target is imported.
+    return
+      cmSystemTools::GetFilenamePath(
       this->ImportedGetFullPath(config, implib));
-  return this->Directory.c_str();
-}
-
-//----------------------------------------------------------------------------
-const char* cmTarget::NormalGetDirectory(const char* config, bool implib)
-{
-  if(config && *config)
-    {
-    // Do not create the directory when config is given:
-    this->Directory = this->GetOutputDir(implib);
-    // Add the configuration's subdirectory.
-    this->Makefile->GetLocalGenerator()->GetGlobalGenerator()->
-      AppendDirectoryForConfig("/", config, "", this->Directory);
-    return this->Directory.c_str();
     }
   else
     {
-    return this->GetOutputDir(implib);
+    // Return the directory in which the target will be built.
+    if(config && *config)
+      {
+      // Add the configuration's subdirectory.
+      std::string dir = this->GetOutputDir(implib);
+      this->Makefile->GetLocalGenerator()->GetGlobalGenerator()->
+        AppendDirectoryForConfig("/", config, "", dir);
+      return dir;
+      }
+    else
+      {
+      return this->GetOutputDir(implib);
+      }
     }
 }
 
@@ -1688,22 +1676,31 @@ const char* cmTarget::ImportedGetLocation(const char* config)
 //----------------------------------------------------------------------------
 const char* cmTarget::NormalGetLocation(const char* config)
 {
-  this->Location = this->GetDirectory(config);
+  // Handle the configuration-specific case first.
+  if(config)
+    {
+    this->Location = this->GetFullPath(config, false);
+    return this->Location.c_str();
+    }
+
+  // Now handle the deprecated build-time configuration location.
+  this->Location = this->GetDirectory();
   if(!this->Location.empty())
     {
     this->Location += "/";
     }
-  if(!config)
+  const char* cfgid = this->Makefile->GetDefinition("CMAKE_CFG_INTDIR");
+  if(cfgid && strcmp(cfgid, ".") != 0)
     {
-    // No specific configuration was given so it will not appear on
-    // the result of GetDirectory.  Add a name here to be replaced at
-    // build time.
-    const char* cfgid = this->Makefile->GetDefinition("CMAKE_CFG_INTDIR");
-    if(cfgid && strcmp(cfgid, ".") != 0)
-      {
-      this->Location += cfgid;
-      this->Location += "/";
-      }
+    this->Location += cfgid;
+    this->Location += "/";
+    }
+   if(this->IsFrameworkOnApple())
+    {
+    this->Location += this->GetFullName(config, false);
+    this->Location += ".framework/Versions/";
+    this->Location += this->GetFrameworkVersion();
+    this->Location += "/";
     }
   this->Location += this->GetFullName(config, false);
   return this->Location.c_str();
@@ -2203,6 +2200,14 @@ std::string cmTarget::NormalGetFullPath(const char* config, bool implib,
   std::string fpath = this->GetDirectory(config, implib);
   fpath += "/";
 
+  if(this->IsFrameworkOnApple())
+    {
+    fpath += this->GetFullName(config, false);
+    fpath += ".framework/Versions/";
+    fpath += this->GetFrameworkVersion();
+    fpath += "/";
+    }
+
   // Add the full name of the target.
   if(implib)
     {
@@ -2474,7 +2479,8 @@ void cmTarget::GetLibraryNamesInternal(std::string& name,
   const char* version = this->GetProperty("VERSION");
   const char* soversion = this->GetProperty("SOVERSION");
   if((type != cmTarget::SHARED_LIBRARY && type != cmTarget::MODULE_LIBRARY) ||
-     !this->Makefile->GetDefinition(sonameFlag.c_str()))
+     !this->Makefile->GetDefinition(sonameFlag.c_str()) ||
+     this->IsFrameworkOnApple())
     {
     // Versioning is supported only for shared libraries and modules,
     // and then only when the platform supports an soname flag.
@@ -2801,13 +2807,14 @@ bool cmTarget::NeedRelinkBeforeInstall()
 }
 
 //----------------------------------------------------------------------------
-std::string cmTarget::GetInstallNameDirForBuildTree(const char* config)
+std::string cmTarget::GetInstallNameDirForBuildTree(const char* config,
+                                                    bool for_xcode)
 {
   // If building directly for installation then the build tree install_name
   // is the same as the install tree.
   if(this->GetPropertyAsBool("BUILD_WITH_INSTALL_RPATH"))
     {
-    return GetInstallNameDirForInstallTree(config);
+    return GetInstallNameDirForInstallTree(config, for_xcode);
     }
 
   // Use the build tree directory for the target.
@@ -2817,6 +2824,13 @@ std::string cmTarget::GetInstallNameDirForBuildTree(const char* config)
     {
     std::string dir = this->GetDirectory(config);
     dir += "/";
+    if(this->IsFrameworkOnApple() && !for_xcode)
+      {
+      dir += this->GetFullName(config, false);
+      dir += ".framework/Versions/";
+      dir += this->GetFrameworkVersion();
+      dir += "/";
+      }
     return dir;
     }
   else
@@ -2826,7 +2840,8 @@ std::string cmTarget::GetInstallNameDirForBuildTree(const char* config)
 }
 
 //----------------------------------------------------------------------------
-std::string cmTarget::GetInstallNameDirForInstallTree(const char*)
+std::string cmTarget::GetInstallNameDirForInstallTree(const char* config,
+                                                      bool for_xcode)
 {
   // Lookup the target property.
   const char* install_name_dir = this->GetProperty("INSTALL_NAME_DIR");
@@ -2836,6 +2851,13 @@ std::string cmTarget::GetInstallNameDirForInstallTree(const char*)
     {
     std::string dir = install_name_dir;
     dir += "/";
+    if(this->IsFrameworkOnApple() && !for_xcode)
+      {
+      dir += this->GetFullName(config, false);
+      dir += ".framework/Versions/";
+      dir += this->GetFrameworkVersion();
+      dir += "/";
+      }
     return dir;
     }
   else
@@ -2845,7 +2867,7 @@ std::string cmTarget::GetInstallNameDirForInstallTree(const char*)
 }
 
 //----------------------------------------------------------------------------
-const char* cmTarget::GetOutputDir(bool implib)
+std::string cmTarget::GetOutputDir(bool implib)
 {
   // The implib option is only allowed for shared libraries, module
   // libraries, and executables.
@@ -2879,62 +2901,36 @@ const char* cmTarget::GetOutputDir(bool implib)
                    msg.c_str());
     }
 
+  return this->ComputeBaseOutputDir(implib);
+}
+
+//----------------------------------------------------------------------------
+std::string const& cmTarget::ComputeBaseOutputDir(bool implib)
+{
   // Select whether we are constructing the directory for the main
   // target or the import library.
-  std::string& out = implib? this->OutputDirImplib : this->OutputDir;
+  std::string& out = implib? this->BaseOutputDirImplib : this->BaseOutputDir;
 
-  if(out.empty())
+  // Return immediately if the directory has already been computed.
+  if(!out.empty())
+    {
+    return out;
+    }
+
+  // Look for a target property defining the target output directory
+  // based on the target type.
+  const char* propertyName = 0;
+  switch(this->GetType())
     {
-    // Look for a target property defining the target output directory
-    // based on the target type.
-    const char* propertyName = 0;
-    switch(this->GetType())
+    case cmTarget::SHARED_LIBRARY:
       {
-      case cmTarget::SHARED_LIBRARY:
-        {
-        // For non-DLL platforms shared libraries are treated as
-        // library targets.  For DLL platforms the DLL part of a
-        // shared library is treated as a runtime target and the
-        // corresponding import library is treated as an archive
-        // target.
-        if(this->DLLPlatform)
-          {
-          if(implib)
-            {
-            propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
-            }
-          else
-            {
-            propertyName = "RUNTIME_OUTPUT_DIRECTORY";
-            }
-          }
-        else
-          {
-          propertyName = "LIBRARY_OUTPUT_DIRECTORY";
-          }
-        } break;
-      case cmTarget::STATIC_LIBRARY:
-        {
-        // Static libraries are always treated as archive targets.
-        propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
-        } break;
-      case cmTarget::MODULE_LIBRARY:
-        {
-        // Module libraries are always treated as library targets.
-        // Module import libraries are treated as archive targets.
-        if(implib)
-          {
-          propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
-          }
-        else
-          {
-          propertyName = "LIBRARY_OUTPUT_DIRECTORY";
-          }
-        } break;
-      case cmTarget::EXECUTABLE:
+      // For non-DLL platforms shared libraries are treated as
+      // library targets.  For DLL platforms the DLL part of a
+      // shared library is treated as a runtime target and the
+      // corresponding import library is treated as an archive
+      // target.
+      if(this->DLLPlatform)
         {
-        // Executables are always treated as runtime targets.
-        // Executable import libraries are treated as archive targets.
         if(implib)
           {
           propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
@@ -2943,53 +2939,93 @@ const char* cmTarget::GetOutputDir(bool implib)
           {
           propertyName = "RUNTIME_OUTPUT_DIRECTORY";
           }
-        } break;
-      default: break;
-      }
-
-    // Select an output directory.
-    if(const char* outdir = this->GetProperty(propertyName))
-      {
-      // Use the user-specified output directory.
-      out = outdir;
-      }
-    else if(this->GetType() == cmTarget::EXECUTABLE)
-      {
-      // Lookup the output path for executables.
-      out = this->Makefile->GetSafeDefinition("EXECUTABLE_OUTPUT_PATH");
-      }
-    else if(this->GetType() == cmTarget::STATIC_LIBRARY ||
-            this->GetType() == cmTarget::SHARED_LIBRARY ||
-            this->GetType() == cmTarget::MODULE_LIBRARY)
+        }
+      else
+        {
+        propertyName = "LIBRARY_OUTPUT_DIRECTORY";
+        }
+      } break;
+    case cmTarget::STATIC_LIBRARY:
       {
-      // Lookup the output path for libraries.
-      out = this->Makefile->GetSafeDefinition("LIBRARY_OUTPUT_PATH");
-      }
-    if(out.empty())
+      // Static libraries are always treated as archive targets.
+      propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
+      } break;
+    case cmTarget::MODULE_LIBRARY:
       {
-      // Default to the current output directory.
-      out = ".";
-      }
-    // Convert the output path to a full path in case it is
-    // specified as a relative path.  Treat a relative path as
-    // relative to the current output directory for this makefile.
-    out =
-      cmSystemTools::CollapseFullPath
-      (out.c_str(), this->Makefile->GetStartOutputDirectory());
-
-    // TODO: Make AppBundle and Framework directory computation in
-    // target consistent.  Why do we add the .framework part here for
-    // frameworks but not the .app part for bundles?  We should
-    // probably not add it for either.
-    if(this->IsFrameworkOnApple())
+      // Module libraries are always treated as library targets.
+      // Module import libraries are treated as archive targets.
+      if(implib)
+        {
+        propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
+        }
+      else
+        {
+        propertyName = "LIBRARY_OUTPUT_DIRECTORY";
+        }
+      } break;
+    case cmTarget::EXECUTABLE:
       {
-      out += "/";
-      out += this->GetFullName(0, implib);
-      out += ".framework";
-      }
-  }
+      // Executables are always treated as runtime targets.
+      // Executable import libraries are treated as archive targets.
+      if(implib)
+        {
+        propertyName = "ARCHIVE_OUTPUT_DIRECTORY";
+        }
+      else
+        {
+        propertyName = "RUNTIME_OUTPUT_DIRECTORY";
+        }
+      } break;
+    default: break;
+    }
 
-  return out.c_str();
+  // Select an output directory.
+  if(const char* outdir = this->GetProperty(propertyName))
+    {
+    // Use the user-specified output directory.
+    out = outdir;
+    }
+  else if(this->GetType() == cmTarget::EXECUTABLE)
+    {
+    // Lookup the output path for executables.
+    out = this->Makefile->GetSafeDefinition("EXECUTABLE_OUTPUT_PATH");
+    }
+  else if(this->GetType() == cmTarget::STATIC_LIBRARY ||
+          this->GetType() == cmTarget::SHARED_LIBRARY ||
+          this->GetType() == cmTarget::MODULE_LIBRARY)
+    {
+    // Lookup the output path for libraries.
+    out = this->Makefile->GetSafeDefinition("LIBRARY_OUTPUT_PATH");
+    }
+  if(out.empty())
+    {
+    // Default to the current output directory.
+    out = ".";
+    }
+
+  // Convert the output path to a full path in case it is
+  // specified as a relative path.  Treat a relative path as
+  // relative to the current output directory for this makefile.
+  out = (cmSystemTools::CollapseFullPath
+         (out.c_str(), this->Makefile->GetStartOutputDirectory()));
+  return out;
+}
+
+//----------------------------------------------------------------------------
+std::string cmTarget::GetFrameworkVersion()
+{
+  if(const char* fversion = this->GetProperty("FRAMEWORK_VERSION"))
+    {
+    return fversion;
+    }
+  else if(const char* tversion = this->GetProperty("VERSION"))
+    {
+    return tversion;
+    }
+  else
+    {
+    return "A";
+    }
 }
 
 //----------------------------------------------------------------------------

Source/cmTarget.h

@@ -253,7 +253,7 @@ public:
       configuration name is given then the generator will add its
       subdirectory for that configuration.  Otherwise just the canonical
       output directory is given.  */
-  const char* GetDirectory(const char* config = 0, bool implib = false);
+  std::string GetDirectory(const char* config = 0, bool implib = false);
 
   /** Get the location of the target in the build tree for the given
       configuration.  This location is suitable for use as the LOCATION
@@ -348,8 +348,10 @@ public:
   /** Return true if builtin chrpath will work for this target */
   bool IsChrpathUsed();
 
-  std::string GetInstallNameDirForBuildTree(const char* config);
-  std::string GetInstallNameDirForInstallTree(const char* config);
+  std::string GetInstallNameDirForBuildTree(const char* config,
+                                            bool for_xcode = false);
+  std::string GetInstallNameDirForInstallTree(const char* config,
+                                              bool for_xcode = false);
 
   cmComputeLinkInformation* GetLinkInformation(const char* config);
 
@@ -384,6 +386,10 @@ public:
   /** Return whether this target is an executable Bundle on Apple.  */
   bool IsAppBundleOnApple();
 
+  /** Return the framework version string.  Undefined if
+      IsFrameworkOnApple returns false.  */
+  std::string GetFrameworkVersion();
+
   /** Get a backtrace from the creation of the target.  */
   cmListFileBacktrace const& GetBacktrace() const;
 
@@ -464,16 +470,14 @@ private:
   void SetPropertyDefault(const char* property, const char* default_value);
 
   // Get the full path to the target output directory.
-  const char* GetOutputDir(bool implib);
+  std::string GetOutputDir(bool implib);
+  std::string const& cmTarget::ComputeBaseOutputDir(bool implib);
 
   const char* ImportedGetLocation(const char* config);
   const char* NormalGetLocation(const char* config);
 
   std::string GetFullNameImported(const char* config, bool implib);
 
-  const char* ImportedGetDirectory(const char* config, bool implib);
-  const char* NormalGetDirectory(const char* config, bool implib);
-
   std::string ImportedGetFullPath(const char* config, bool implib);
   std::string NormalGetFullPath(const char* config, bool implib,
                                 bool realname);
@@ -500,9 +504,8 @@ private:
   bool HaveInstallRule;
   std::string InstallPath;
   std::string RuntimeInstallPath;
-  std::string OutputDir;
-  std::string OutputDirImplib;
-  std::string Directory;
+  std::string BaseOutputDir;
+  std::string BaseOutputDirImplib;
   std::string Location;
   std::string ExportMacro;
   std::set<cmStdString> Utilities;

Tests/Framework/CMakeLists.txt

@@ -10,8 +10,11 @@ add_library(foo SHARED
   fooNeither.h
   fooBoth.h
   test.lua
+  fooDeepPublic.h
 )
-
+set_property(SOURCE fooDeepPublic.h
+  PROPERTY MACOSX_PACKAGE_LOCATION Headers/Deep
+  )
 set(foo_ver ver4)
 
 set_target_properties(foo PROPERTIES
@@ -20,6 +23,7 @@ set_target_properties(foo PROPERTIES
   PRIVATE_HEADER "fooPrivate.h;fooBoth.h"
   PUBLIC_HEADER "foo.h;foo2.h;fooPublic.h;fooBoth.h"
   RESOURCE "test.lua"
+  INSTALL_NAME_DIR "@executable_path/../../../Library/Frameworks"
 )
 # fooBoth.h is listed as both public and private... (private wins...)
 # fooNeither.h is listed as neither public nor private...

Tests/Framework/fooDeepPublic.h

@@ -0,0 +1 @@
+fooDeepPublic