ENH: Add InstallNameFixupPath to support installing built frameworks on the Mac. Change Application to Applications in the BundleTest. Also correct small typo (tcl->Tcl) noted in bug 4572.

8 files changed, 105 insertions, 43 deletions

diff --git a/Source/cmInstallCommand.cxx b/Source/cmInstallCommand.cxx
index 0c5b4d1..a27ef31 100644
--- a/Source/cmInstallCommand.cxx
+++ b/Source/cmInstallCommand.cxx
@@ -181,6 +181,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
   targetList.Follows(0);
   genericArgs.ArgumentGroup.Follows(&targetList);
   genericArgs.Parse(&genericArgVector.GetVector(), &unknownArgs);
+  bool success = genericArgs.Finalize();
 
   cmInstallCommandArguments archiveArgs;
   cmInstallCommandArguments libraryArgs;
@@ -219,7 +220,7 @@ bool cmInstallCommand::HandleTargetsMode(std::vector<std::string> const& args)
   publicHeaderArgs.SetGenericArguments(&genericArgs);
   privateHeaderArgs.SetGenericArguments(&genericArgs);
 
-  bool success = archiveArgs.Finalize();
+  success = success && archiveArgs.Finalize();
   success = success && libraryArgs.Finalize();
   success = success && runtimeArgs.Finalize();
   success = success && frameworkArgs.Finalize();
diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx
index 72dfa57..347d8fe 100644
--- a/Source/cmInstallTargetGenerator.cxx
+++ b/Source/cmInstallTargetGenerator.cxx
@@ -154,11 +154,10 @@ cmInstallTargetGenerator
                              Indent const& indent)
 {
   // Compute the full path to the main installed file for this target.
-  std::string toFullPath = "$ENV{DESTDIR}";
-  toFullPath += this->Destination;
-  toFullPath += "/";
-  toFullPath += this->GetInstallFilename(this->Target, config,
-                                         this->ImportLibrary, false);
+  std::string toInstallPath = this->Destination;
+  toInstallPath += "/";
+  toInstallPath += this->GetInstallFilename(this->Target, config,
+                                              this->ImportLibrary, false);
 
   // Compute the list of files to install for this target.
   std::vector<std::string> files;
@@ -198,9 +197,9 @@ cmInstallTargetGenerator
         type = cmTarget::INSTALL_DIRECTORY;
         // Need to apply install_name_tool and stripping to binary
         // inside bundle.
-        toFullPath += ".app/Contents/MacOS/";
-        toFullPath += this->GetInstallFilename(this->Target, config,
-                                               this->ImportLibrary, false);
+        toInstallPath += ".app/Contents/MacOS/";
+        toInstallPath += this->GetInstallFilename(this->Target, config,
+                                                  this->ImportLibrary, false);
         literal_args += " USE_SOURCE_PERMISSIONS";
         }
       else
@@ -234,6 +233,25 @@ cmInstallTargetGenerator
       // An import library looks like a static library.
       type = cmTarget::STATIC_LIBRARY;
       }
+    else if(this->Target->GetMakefile()->IsOn("APPLE") &&
+            this->Target->GetPropertyAsBool("FRAMEWORK"))
+      {
+      // Compute the build tree location of the framework directory
+      std::string from1 = fromDirConfig;
+      // Remove trailing slashes
+      cmSystemTools::ConvertToUnixSlashes(from1);
+      files.push_back(from1);
+
+      type = cmTarget::INSTALL_DIRECTORY;
+
+      // Need to apply install_name_tool and stripping to binary
+      // inside framework.
+      toInstallPath += ".framework/";
+      toInstallPath += this->GetInstallFilename(this->Target, config,
+                                                this->ImportLibrary, false);
+
+      literal_args += " USE_SOURCE_PERMISSIONS";
+      }
     else
       {
       std::string from1 = fromDirConfig;
@@ -266,11 +284,20 @@ cmInstallTargetGenerator
                        no_rename, literal_args.c_str(),
                        indent);
 
-  os << indent << "IF(EXISTS \"" << toFullPath << "\")\n";
-  this->AddInstallNamePatchRule(os, indent.Next(), config, toFullPath);
-  this->AddRanlibRule(os, indent.Next(), type, toFullPath);
-  this->AddStripRule(os, indent.Next(), type, toFullPath);
-  os << indent << "ENDIF(EXISTS \"" << toFullPath << "\")\n";
+  std::string toDestDirPath = "$ENV{DESTDIR}";
+  if(toInstallPath[0] != '/')
+    {
+    toDestDirPath += "/";
+    }
+  toDestDirPath += toInstallPath;
+
+  this->Target->SetInstallNameFixupPath(toInstallPath.c_str());
+
+  os << indent << "IF(EXISTS \"" << toDestDirPath << "\")\n";
+  this->AddInstallNamePatchRule(os, indent.Next(), config, toDestDirPath);
+  this->AddRanlibRule(os, indent.Next(), type, toDestDirPath);
+  this->AddStripRule(os, indent.Next(), type, toDestDirPath);
+  os << indent << "ENDIF(EXISTS \"" << toDestDirPath << "\")\n";
 }
 
 //----------------------------------------------------------------------------
@@ -343,7 +370,7 @@ std::string cmInstallTargetGenerator::GetInstallFilename(cmTarget* target,
 void
 cmInstallTargetGenerator
 ::AddInstallNamePatchRule(std::ostream& os, Indent const& indent,
-                          const char* config, std::string const& toFullPath)
+                          const char* config, std::string const& toDestDirPath)
 {
   if(this->ImportLibrary ||
      !(this->Target->GetType() == cmTarget::SHARED_LIBRARY ||
@@ -354,9 +381,9 @@ cmInstallTargetGenerator
     }
 
   // Fix the install_name settings in installed binaries.
-    std::string installNameTool =
+  std::string installNameTool =
     this->Target->GetMakefile()->GetSafeDefinition("CMAKE_INSTALL_NAME_TOOL");
-  
+
   if(!installNameTool.size())
     {
     return;
@@ -393,16 +420,24 @@ cmInstallTargetGenerator
           std::string for_build = tgt->GetInstallNameDirForBuildTree(config);
           std::string for_install = 
             tgt->GetInstallNameDirForInstallTree(config);
-          if(for_build != for_install)
-            {
-            std::string fname =
-              this->GetInstallFilename(tgt, config, false, true);
-            // Map from the build-tree install_name.
-            for_build += fname;
+          std::string fname =
+            this->GetInstallFilename(tgt, config, false, true);
+
+          // Map from the build-tree install_name.
+          for_build += fname;
 
-            // Map to the install-tree install_name.
+          // Map to the install-tree install_name.
+          if (!for_install.empty())
+            {
             for_install += fname;
+            }
+          else
+            {
+            for_install = tgt->GetInstallNameFixupPath();
+            }
 
+          if(for_build != for_install)
+            {
             // Store the mapping entry.
             install_name_remap[for_build] = for_install;
             }
@@ -419,12 +454,22 @@ cmInstallTargetGenerator
       this->Target->GetInstallNameDirForBuildTree(config);
     std::string for_install = 
       this->Target->GetInstallNameDirForInstallTree(config);
+    std::string fname =
+      this->GetInstallFilename(this->Target, config, this->ImportLibrary,
+                               true);
+    for_build += fname;
+    if (!for_install.empty())
+      {
+      for_install += fname;
+      }
+    else
+      {
+      for_install = this->Target->GetInstallNameFixupPath();
+      }
     if(for_build != for_install)
       {
       // Prepare to refer to the install-tree install_name.
       new_id = for_install;
-      new_id += this->GetInstallFilename(this->Target, config,
-                                         this->ImportLibrary, true);
       }
     }
 
@@ -445,7 +490,7 @@ cmInstallTargetGenerator
       os << "\n" << indent << "  -change \""
          << i->first << "\" \"" << i->second << "\"";
       }
-    os << "\n" << indent << "  \"" << toFullPath << "\")\n";
+    os << "\n" << indent << "  \"" << toDestDirPath << "\")\n";
     }
 }
 
@@ -454,7 +499,7 @@ void
 cmInstallTargetGenerator::AddStripRule(std::ostream& os,
                                        Indent const& indent,
                                        cmTarget::TargetType type,
-                                       const std::string& toFullPath)
+                                       const std::string& toDestDirPath)
 {
 
   // don't strip static libraries, because it removes the only symbol table
@@ -479,7 +524,7 @@ cmInstallTargetGenerator::AddStripRule(std::ostream& os,
   os << indent << "IF(CMAKE_INSTALL_DO_STRIP)\n";
   os << indent << "  EXECUTE_PROCESS(COMMAND \""
      << this->Target->GetMakefile()->GetDefinition("CMAKE_STRIP")
-     << "\" \"" << toFullPath << "\")\n";
+     << "\" \"" << toDestDirPath << "\")\n";
   os << indent << "ENDIF(CMAKE_INSTALL_DO_STRIP)\n";
 }
 
@@ -488,7 +533,7 @@ void
 cmInstallTargetGenerator::AddRanlibRule(std::ostream& os,
                                         Indent const& indent,
                                         cmTarget::TargetType type,
-                                        const std::string& toFullPath)
+                                        const std::string& toDestDirPath)
 {
   // Static libraries need ranlib on this platform.
   if(type != cmTarget::STATIC_LIBRARY)
@@ -511,5 +556,5 @@ cmInstallTargetGenerator::AddRanlibRule(std::ostream& os,
     }
 
   os << indent << "EXECUTE_PROCESS(COMMAND \""
-     << ranlib << "\" \"" << toFullPath << "\")\n";
+     << ranlib << "\" \"" << toDestDirPath << "\")\n";
 }
diff --git a/Source/cmInstallTargetGenerator.h b/Source/cmInstallTargetGenerator.h
index d1907bb..d03ea3d 100644
--- a/Source/cmInstallTargetGenerator.h
+++ b/Source/cmInstallTargetGenerator.h
@@ -53,13 +53,13 @@ protected:
                                   Indent const& indent);
   void AddInstallNamePatchRule(std::ostream& os, Indent const& indent,
                                const char* config,
-                               const std::string& toFullPath);
+                               const std::string& toDestDirPath);
   void AddStripRule(std::ostream& os, Indent const& indent,
                     cmTarget::TargetType type,
-                    const std::string& toFullPath);
+                    const std::string& toDestDirPath);
   void AddRanlibRule(std::ostream& os, Indent const& indent,
                      cmTarget::TargetType type,
-                     const std::string& toFullPath);
+                     const std::string& toDestDirPath);
 
   cmTarget* Target;
   bool ImportLibrary;
diff --git a/Source/cmTarget.h b/Source/cmTarget.h
index 4b59e8b..be0ed65 100644
--- a/Source/cmTarget.h
+++ b/Source/cmTarget.h
@@ -137,7 +137,7 @@ public:
    */
   std::string GetRuntimeInstallPath() {return this->RuntimeInstallPath;}
   void SetRuntimeInstallPath(const char *name) {
-  this->RuntimeInstallPath = name;}
+    this->RuntimeInstallPath = name; }
 
   /**
    * Get/Set whether there is an install rule for this target.
@@ -145,6 +145,18 @@ public:
   bool GetHaveInstallRule() { return this->HaveInstallRule; }
   void SetHaveInstallRule(bool h) { this->HaveInstallRule = h; }
 
+  /**
+   * Get/Set the path needed for calls to install_name_tool regarding this
+   * target. Used to support fixing up installed libraries and executables on
+   * the Mac (including bundles and frameworks). Only used if the target does
+   * not have an INSTALL_NAME_DIR property.
+   * See cmInstallTargetGenerator::AddInstallNamePatchRule and callers for
+   * more information.
+   */
+  std::string GetInstallNameFixupPath() { return this->InstallNameFixupPath; }
+  void SetInstallNameFixupPath(const char *path) {
+    this->InstallNameFixupPath = path; }
+
   /** Add a utility on which this project depends. A utility is an executable
    * name as would be specified to the ADD_EXECUTABLE or UTILITY_SOURCE
    * commands. It is not a full path nor does it have an extension.
@@ -382,6 +394,7 @@ private:
   std::vector<std::string> LinkDirectories;
   std::vector<std::string> ExplicitLinkDirectories;
   bool HaveInstallRule;
+  std::string InstallNameFixupPath;
   std::string InstallPath;
   std::string RuntimeInstallPath;
   std::string OutputDir;
diff --git a/Tests/BundleTest/BundleSubDir/CMakeLists.txt b/Tests/BundleTest/BundleSubDir/CMakeLists.txt
index 299c42e..3f7a5f1 100644
--- a/Tests/BundleTest/BundleSubDir/CMakeLists.txt
+++ b/Tests/BundleTest/BundleSubDir/CMakeLists.txt
@@ -27,7 +27,7 @@ ADD_EXECUTABLE(SecondBundle
 TARGET_LINK_LIBRARIES(SecondBundle BundleTestLib)
  
 # Test bundle installation.
-INSTALL(TARGETS SecondBundle DESTINATION Application)
+INSTALL(TARGETS SecondBundle DESTINATION Applications)
 
 # Test whether bundles respect the output name.  Since the library is
 # installed into a location that uses this output name this will fail if the
diff --git a/Tests/BundleTest/CMakeLists.txt b/Tests/BundleTest/CMakeLists.txt
index ff9626f..2eec9ea 100644
--- a/Tests/BundleTest/CMakeLists.txt
+++ b/Tests/BundleTest/CMakeLists.txt
@@ -36,9 +36,9 @@ ADD_EXECUTABLE(BundleTest
 TARGET_LINK_LIBRARIES(BundleTest BundleTestLib)
 
 # Test bundle installation.
-#INSTALL(TARGETS BundleTestLib DESTINATION Application/BundleTestExe.app/Contents/Plugins)
-INSTALL(TARGETS BundleTestLib DESTINATION Application/SecondBundleExe.app/Contents/Plugins)
-INSTALL(TARGETS BundleTest DESTINATION Application)
+#INSTALL(TARGETS BundleTestLib DESTINATION Applications/BundleTestExe.app/Contents/Plugins)
+INSTALL(TARGETS BundleTestLib DESTINATION Applications/SecondBundleExe.app/Contents/Plugins)
+INSTALL(TARGETS BundleTest DESTINATION Applications)
 
 # Test whether bundles respect the output name.  Since the library is
 # installed into a location that uses this output name this will fail if the
@@ -59,7 +59,7 @@ INCLUDE(CPack)
 
 # test the framework find stuff
 IF(EXISTS /usr/lib/libtcl.dylib
-    AND EXISTS /System/Library/Frameworks/tcl.framework)
+    AND EXISTS /System/Library/Frameworks/Tcl.framework)
   SET(TCL NOTFOUND)
   FIND_LIBRARY(TCL tcl)
   MESSAGE("frame: ${TCL}")
@@ -87,6 +87,6 @@ IF(EXISTS /usr/lib/libtcl.dylib
   ENDIF(NOT "${TCL}" MATCHES .framework)
   MESSAGE("frame: ${TCL}")
 ENDIF(EXISTS /usr/lib/libtcl.dylib
-  AND EXISTS /System/Library/Frameworks/tcl.framework)
+  AND EXISTS /System/Library/Frameworks/Tcl.framework)
 
 SUBDIRS(BundleSubDir)
diff --git a/Tests/CMakeLists.txt b/Tests/CMakeLists.txt
index 9e530d3..eb5267f 100644
--- a/Tests/CMakeLists.txt
+++ b/Tests/CMakeLists.txt
@@ -597,7 +597,7 @@ IF(BUILD_TESTING)
       --build-options "-DCMAKE_INSTALL_PREFIX:PATH=${BundleTestInstallDir}"
        "-DCMake_SOURCE_DIR:PATH=${CMAKE_SOURCE_DIR}"
       --test-command
-      ${BundleTestInstallDir}/Application/SecondBundleExe.app/Contents/MacOS/SecondBundleExe)
+      ${BundleTestInstallDir}/Applications/SecondBundleExe.app/Contents/MacOS/SecondBundleExe)
 
     ADD_TEST_MACRO(ObjC++ ObjC++)
   ENDIF (APPLE AND CMAKE_COMPILER_IS_GNUCXX)
diff --git a/Tests/Framework/CMakeLists.txt b/Tests/Framework/CMakeLists.txt
index f1f82cb..57028d7 100644
--- a/Tests/Framework/CMakeLists.txt
+++ b/Tests/Framework/CMakeLists.txt
@@ -27,7 +27,10 @@ set_source_files_properties(test.lua PROPERTIES
 )
 add_executable(bar bar.cxx)
 target_link_libraries(bar foo)
-
+install(TARGETS foo bar
+  RUNTIME DESTINATION /Applications/CMakeTestsFramework/bin
+  FRAMEWORK DESTINATION /Library/Frameworks
+)
 
 # Make a static library and apply the framework properties to it to verify
 # that everything still builds correctly, but it will not actually produce