ENH: Refactor find_* command framework/appbundle search order impl.

- CMAKE_FIND_FRAMEWORK and CMAKE_FIND_APPBUNDLE are supposed to specify
    whether to find frameworks/appbundles FIRST, LAST, ONLY, or NEVER.
  - Previously this affected only the placement of CMAKE_FRAMEWORK_PATH
    and CMAKE_APPBUNDLE_PATH with respect to the other path specifiers.
  - Now it behaves as documented.  The entire search path is inspected for
    each kind of program, library, or header before trying the next kind.
  - Additionally the ONLY mode is now honored for headers so that users
    do not end up with a library in framework and a header from elsewhere.

Source/cmFindBase.cxx

@@ -282,31 +282,10 @@ bool cmFindBase::ParseArguments(std::vector<std::string> const& argsIn)
 
 void cmFindBase::ExpandPaths()
 {
-  // if NO Default paths was not specified add the
+  this->AddCMakeEnvironmentPath();
-  // standard search paths.
+  this->AddCMakeVariablePath();
-  if(!this->NoDefaultPath)
+  this->AddSystemEnvironmentPath();
-    {
+  this->AddCMakeSystemVariablePath();
-    if(this->SearchFrameworkFirst || this->SearchFrameworkOnly)
-      {
-      this->AddFrameworkPath();
-      }
-    if(this->SearchAppBundleFirst || this->SearchAppBundleOnly)
-      {
-      this->AddAppBundlePath();
-      }
-    this->AddCMakeEnvironmentPath();
-    this->AddCMakeVariablePath();
-    this->AddSystemEnvironmentPath();
-    this->AddCMakeSystemVariablePath();
-    if(this->SearchAppBundleLast)
-      {
-      this->AddAppBundlePath();
-      }
-    if(this->SearchFrameworkLast)
-      {
-      this->AddFrameworkPath();
-      }
-    }
 
   // Add paths specified by the caller.
   this->AddPathsInternal(this->UserPaths, CMakePath);
@@ -380,50 +359,10 @@ void cmFindBase::AddEnvPrefixPath(const char* variable)
   this->AddPrefixPaths(tmp, EnvPath);
 }
 
-//----------------------------------------------------------------------------
-void cmFindBase::AddMacPath(const char* var, const char* sysvar)
-{
-  if(this->NoDefaultPath)
-    {
-    return;
-    }
-
-  // first environment variables
-  if(!this->NoCMakeEnvironmentPath)
-    {
-    this->AddEnvPath(var);
-    }
-
-  // add cmake variables
-  if(!this->NoCMakePath)
-    {
-    this->AddCMakePath(var);
-    }
-
-  // add cmake system variables
-  if(!this->NoCMakeSystemPath)
-    {
-    this->AddCMakePath(sysvar);
-    }
-}
-
-//----------------------------------------------------------------------------
-void cmFindBase::AddFrameworkPath()
-{
-  this->AddMacPath("CMAKE_FRAMEWORK_PATH", "CMAKE_SYSTEM_FRAMEWORK_PATH");
-}
-
-//----------------------------------------------------------------------------
-void cmFindBase::AddAppBundlePath()
-{
-  this->AddMacPath("CMAKE_APPBUNDLE_PATH", "CMAKE_SYSTEM_APPBUNDLE_PATH");
-}
-
 //----------------------------------------------------------------------------
 void cmFindBase::AddCMakeEnvironmentPath()
 {
-  if(!this->NoCMakeEnvironmentPath &&
+  if(!this->NoCMakeEnvironmentPath && !this->NoDefaultPath)
-     !(this->SearchFrameworkOnly || this->SearchAppBundleOnly))
     {
     // Add CMAKE_*_PATH environment variables
     std::string var = "CMAKE_";
@@ -431,14 +370,22 @@ void cmFindBase::AddCMakeEnvironmentPath()
     var += "_PATH";
     this->AddEnvPrefixPath("CMAKE_PREFIX_PATH");
     this->AddEnvPath(var.c_str());
+
+    if(this->CMakePathName == "PROGRAM")
+      {
+      this->AddEnvPath("CMAKE_APPBUNDLE_PATH");
+      }
+    else
+      {
+      this->AddEnvPath("CMAKE_FRAMEWORK_PATH");
+      }
     }
 }
 
 //----------------------------------------------------------------------------
 void cmFindBase::AddCMakeVariablePath()
 {
-  if(!this->NoCMakePath &&
+  if(!this->NoCMakePath && !this->NoDefaultPath)
-     !(this->SearchFrameworkOnly || this->SearchAppBundleOnly))
     {
     // Add CMake varibles of the same name as the previous environment
     // varibles CMAKE_*_PATH to be used most of the time with -D
@@ -448,14 +395,22 @@ void cmFindBase::AddCMakeVariablePath()
     var += "_PATH";
     this->AddCMakePrefixPath("CMAKE_PREFIX_PATH");
     this->AddCMakePath(var.c_str());
+
+    if(this->CMakePathName == "PROGRAM")
+      {
+      this->AddCMakePath("CMAKE_APPBUNDLE_PATH");
+      }
+    else
+      {
+      this->AddCMakePath("CMAKE_FRAMEWORK_PATH");
+      }
     }
 }
 
 //----------------------------------------------------------------------------
 void cmFindBase::AddSystemEnvironmentPath()
 {
-  if(!this->NoSystemEnvironmentPath &&
+  if(!this->NoSystemEnvironmentPath && !this->NoDefaultPath)
-     !(this->SearchFrameworkOnly || this->SearchAppBundleOnly))
     {
     // Add LIB or INCLUDE
     if(!this->EnvironmentPath.empty())
@@ -470,14 +425,22 @@ void cmFindBase::AddSystemEnvironmentPath()
 //----------------------------------------------------------------------------
 void cmFindBase::AddCMakeSystemVariablePath()
 {
-  if(!this->NoCMakeSystemPath &&
+  if(!this->NoCMakeSystemPath && !this->NoDefaultPath)
-     !(this->SearchFrameworkOnly || this->SearchAppBundleOnly))
     {
     std::string var = "CMAKE_SYSTEM_";
     var += this->CMakePathName;
     var += "_PATH";
     this->AddCMakePrefixPath("CMAKE_SYSTEM_PREFIX_PATH");
     this->AddCMakePath(var.c_str());
+
+    if(this->CMakePathName == "PROGRAM")
+      {
+      this->AddCMakePath("CMAKE_SYSTEM_APPBUNDLE_PATH");
+      }
+    else
+      {
+      this->AddCMakePath("CMAKE_SYSTEM_FRAMEWORK_PATH");
+      }
     }
 }
 

Source/cmFindBase.h

@@ -62,15 +62,12 @@ protected:
   bool AlreadyInCacheWithoutMetaInfo;
 private:
   // Add pieces of the search.
-  void AddFrameworkPath();
-  void AddAppBundlePath();
   void AddCMakeEnvironmentPath();
   void AddCMakeVariablePath();
   void AddSystemEnvironmentPath();
   void AddCMakeSystemVariablePath();
 
   // Helpers.
-  void AddMacPath(const char* var, const char* sysvar);
   void AddCMakePrefixPath(const char* variable);
   void AddEnvPrefixPath(const char* variable);
   void AddPrefixPaths(std::vector<std::string> const& in_paths,

Source/cmFindLibraryCommand.cxx

@@ -96,19 +96,15 @@ bool cmFindLibraryCommand
     this->AddLib64Paths();
     }
 
-  std::string library;
+  std::string library = this->FindLibrary();
-  for(std::vector<std::string>::iterator i = this->Names.begin();
+  if(library != "")
-      i != this->Names.end() ; ++i)
     {
-    library = this->FindLibrary(i->c_str());
+    // Save the value in the cache
-    if(library != "")
+    this->Makefile->AddCacheDefinition(this->VariableName.c_str(),
-      {
+                                       library.c_str(),
-      this->Makefile->AddCacheDefinition(this->VariableName.c_str(),
+                                       this->VariableDocumentation.c_str(),
-                                         library.c_str(),
+                                       cmCacheManager::FILEPATH);
-                                         this->VariableDocumentation.c_str(),
+    return true;
-                                         cmCacheManager::FILEPATH);
-      return true;
-      } 
     }
   std::string notfound = this->VariableName + "-NOTFOUND";
   this->Makefile->AddCacheDefinition(this->VariableName.c_str(),
@@ -211,21 +207,29 @@ void cmFindLibraryCommand::AddLib64Paths()
     }
 }
 
-std::string cmFindLibraryCommand::FindLibrary(const char* name)
+//----------------------------------------------------------------------------
+std::string cmFindLibraryCommand::FindLibrary()
 {
-  bool supportFrameworks = false;
+  std::string library;
-  bool onlyFrameworks = false;
+  if(this->SearchFrameworkFirst || this->SearchFrameworkOnly)
-  std::string ff = this->Makefile->GetSafeDefinition("CMAKE_FIND_FRAMEWORK");
-  if(ff == "FIRST" || ff == "LAST")
     {
-    supportFrameworks = true;
+    library = this->FindFrameworkLibrary();
     }
-  if(ff == "ONLY")
+  if(library.empty() && !this->SearchFrameworkOnly)
     {
-    onlyFrameworks = true;
+    library = this->FindNormalLibrary();
-    supportFrameworks = true;
     }
-  
+  if(library.empty() && this->SearchFrameworkLast)
+    {
+    library = this->FindFrameworkLibrary();
+    }
+  return library;
+}
+
+//----------------------------------------------------------------------------
+std::string cmFindLibraryCommand::FindNormalLibrary()
+{
+  // Collect the list of library name prefixes/suffixes to try.
   const char* prefixes_list =
     this->Makefile->GetRequiredDefinition("CMAKE_FIND_LIBRARY_PREFIXES");
   const char* suffixes_list =
@@ -235,41 +239,29 @@ std::string cmFindLibraryCommand::FindLibrary(const char* name)
   cmSystemTools::ExpandListArgument(prefixes_list, prefixes, true);
   cmSystemTools::ExpandListArgument(suffixes_list, suffixes, true);
 
-  // If the original library name provided by the user matches one of
+  // Search the entire path for each name.
-  // the suffixes, try it first.
-  bool tryOrig = false;
-  {
-  std::string nm = name;
-  for(std::vector<std::string>::const_iterator si = suffixes.begin();
-      !tryOrig && si != suffixes.end(); ++si)
-    {
-    std::string const& suffix = *si;
-    if(nm.length() > suffix.length() &&
-       nm.substr(nm.size()-suffix.length()) == suffix)
-      {
-      tryOrig = true;
-      }
-    }
-  }
-
   std::string tryPath;
-  for(std::vector<std::string>::const_iterator p = this->SearchPaths.begin();
+  for(std::vector<std::string>::const_iterator ni = this->Names.begin();
-      p != this->SearchPaths.end(); ++p)
+      ni != this->Names.end() ; ++ni)
     {
-    if(supportFrameworks)
+    // If the original library name provided by the user matches one of
+    // the suffixes, try it first.
+    bool tryOrig = false;
+    std::string const& name = *ni;
+    for(std::vector<std::string>::const_iterator si = suffixes.begin();
+        !tryOrig && si != suffixes.end(); ++si)
       {
-      tryPath = *p;
+      std::string const& suffix = *si;
-      tryPath += name;
+      if(name.length() > suffix.length() &&
-      tryPath += ".framework";
+         name.substr(name.size()-suffix.length()) == suffix)
-      if(cmSystemTools::FileExists(tryPath.c_str())
-         && cmSystemTools::FileIsDirectory(tryPath.c_str()))
         {
-        tryPath = cmSystemTools::CollapseFullPath(tryPath.c_str());
+        tryOrig = true;
-        cmSystemTools::ConvertToUnixSlashes(tryPath);
-        return tryPath;
         }
       }
-    if(!onlyFrameworks)
+
+    for(std::vector<std::string>::const_iterator
+          p = this->SearchPaths.begin();
+        p != this->SearchPaths.end(); ++p)
       {
       // Try the original library name as specified by the user.
       if(tryOrig)
@@ -309,3 +301,26 @@ std::string cmFindLibraryCommand::FindLibrary(const char* name)
   // Couldn't find the library.
   return "";
 }
+
+//----------------------------------------------------------------------------
+std::string cmFindLibraryCommand::FindFrameworkLibrary()
+{
+  // Search for a framework of each name in the entire search path.
+  for(std::vector<std::string>::const_iterator ni = this->Names.begin();
+      ni != this->Names.end() ; ++ni)
+    {
+    // Search the paths for a framework with this name.
+    std::string fwName = *ni;
+    fwName += ".framework";
+    std::string fwPath = cmSystemTools::FindDirectory(fwName.c_str(),
+                                                      this->SearchPaths,
+                                                      true);
+    if(!fwPath.empty())
+      {
+      return fwPath;
+      }
+    }
+
+  // No framework found.
+  return "";
+}

Source/cmFindLibraryCommand.h

@@ -68,7 +68,10 @@ public:
 protected:
   void AddArchitecturePaths(const char* suffix);
   void AddLib64Paths();
-  std::string FindLibrary(const char* name);
+  std::string FindLibrary();
+private:
+  std::string FindNormalLibrary();
+  std::string FindFrameworkLibrary();
 };
 
 

Source/cmFindPathCommand.cxx

@@ -95,57 +95,16 @@ bool cmFindPathCommand
       }
     return true;
     }
-  std::string ff = this->Makefile->GetSafeDefinition("CMAKE_FIND_FRAMEWORK");
+
-  bool supportFrameworks = true;
+  std::string result = this->FindHeader();
-  if( ff.size() == 0 || ff == "NEVER" )
+  if(result.size() != 0)
     {
-    supportFrameworks = false;
+    this->Makefile->AddCacheDefinition
-    }
+      (this->VariableName.c_str(), result.c_str(),
-  std::string framework;
+       this->VariableDocumentation.c_str(),
-  // Use the search path to find the file.
+       (this->IncludeFileInPath) ?
-  unsigned int k;
+       cmCacheManager::FILEPATH :cmCacheManager::PATH);
-  std::string result;
+    return true;
-  for(k=0; k < this->SearchPaths.size(); k++)
-    {
-    for(unsigned int j =0; j < this->Names.size(); ++j)
-      {
-      // if frameworks are supported try to find the header in a framework
-      std::string tryPath;
-      if(supportFrameworks)
-        {
-        tryPath = this->FindHeaderInFramework(this->Names[j],
-                                              this->SearchPaths[k]);
-        if(tryPath.size())
-          {
-          result = tryPath;
-          }
-        }
-      if(result.size() == 0)
-        {
-        tryPath = this->SearchPaths[k];
-        tryPath += this->Names[j];
-        if(cmSystemTools::FileExists(tryPath.c_str()))
-          {
-          if(this->IncludeFileInPath)
-            {
-            result = tryPath;
-            }
-          else
-            {
-            result = this->SearchPaths[k];
-            }
-          }
-        }
-      if(result.size() != 0)
-        {
-        this->Makefile->AddCacheDefinition
-          (this->VariableName.c_str(), result.c_str(),
-           this->VariableDocumentation.c_str(),
-           (this->IncludeFileInPath) ? 
-           cmCacheManager::FILEPATH :cmCacheManager::PATH);
-        return true;
-        }
-      }
     }
   this->Makefile->AddCacheDefinition
     (this->VariableName.c_str(),
@@ -156,8 +115,28 @@ bool cmFindPathCommand
   return true;
 }
 
-std::string cmFindPathCommand::FindHeaderInFramework(std::string& file,
+//----------------------------------------------------------------------------
-                                                     std::string& dir)
+std::string cmFindPathCommand::FindHeader()
+{
+  std::string header;
+  if(this->SearchFrameworkFirst || this->SearchFrameworkOnly)
+    {
+    header = this->FindFrameworkHeader();
+    }
+  if(header.empty() && !this->SearchFrameworkOnly)
+    {
+    header = this->FindNormalHeader();
+    }
+  if(header.empty() && this->SearchFrameworkLast)
+    {
+    header = this->FindFrameworkHeader();
+    }
+  return header;
+}
+
+std::string
+cmFindPathCommand::FindHeaderInFramework(std::string const& file,
+                                         std::string const& dir)
 {
   cmStdString fileName = file;
   cmStdString frameWorkName;
@@ -217,3 +196,51 @@ std::string cmFindPathCommand::FindHeaderInFramework(std::string& file,
   return "";
 }
 
+//----------------------------------------------------------------------------
+std::string cmFindPathCommand::FindNormalHeader()
+{
+  std::string tryPath;
+  for(std::vector<std::string>::const_iterator ni = this->Names.begin();
+      ni != this->Names.end() ; ++ni)
+    {
+    for(std::vector<std::string>::const_iterator
+          p = this->SearchPaths.begin();
+        p != this->SearchPaths.end(); ++p)
+      {
+      tryPath = *p;
+      tryPath += *ni;
+      if(cmSystemTools::FileExists(tryPath.c_str()))
+        {
+        if(this->IncludeFileInPath)
+          {
+          return tryPath;
+          }
+        else
+          {
+          return *p;
+          }
+        }
+      }
+    }
+  return "";
+}
+
+//----------------------------------------------------------------------------
+std::string cmFindPathCommand::FindFrameworkHeader()
+{
+  for(std::vector<std::string>::const_iterator ni = this->Names.begin();
+      ni != this->Names.end() ; ++ni)
+    {
+    for(std::vector<std::string>::const_iterator
+          p = this->SearchPaths.begin();
+        p != this->SearchPaths.end(); ++p)
+      {
+      std::string fwPath = this->FindHeaderInFramework(*ni, *p);
+      if(!fwPath.empty())
+        {
+        return fwPath;
+        }
+      }
+    }
+  return "";
+}

Source/cmFindPathCommand.h

@@ -64,12 +64,16 @@ public:
     return "Find the directory containing a file.";
     }
 
-  std::string FindHeaderInFramework( std::string& file,
-                                     std::string& dir);
   virtual const char* GetFullDocumentation();
   cmTypeMacro(cmFindPathCommand, cmFindBase);
   bool IncludeFileInPath;
   bool ExtraDocAdded;
+private:
+  std::string FindHeaderInFramework(std::string const& file,
+                                    std::string const& dir);
+  std::string FindHeader();
+  std::string FindNormalHeader();
+  std::string FindFrameworkHeader();
 };
 
 

Source/cmFindProgramCommand.cxx

@@ -97,8 +97,7 @@ std::string cmFindProgramCommand::FindProgram(std::vector<std::string> names)
 {
   std::string program = "";
 
-  // First/last order taken care of in cmFindBase when the paths are setup.
+  if(this->SearchAppBundleFirst || this->SearchAppBundleOnly)
-  if(this->SearchAppBundleFirst || this->SearchAppBundleLast)
     {
     program = FindAppBundle(names);
     }
@@ -107,6 +106,10 @@ std::string cmFindProgramCommand::FindProgram(std::vector<std::string> names)
     program = cmSystemTools::FindProgram(names, this->SearchPaths, true);
     }
 
+  if(program.empty() && this->SearchAppBundleLast)
+    {
+    program = this->FindAppBundle(names);
+    }
   return program;
 }