Merge topic 'LINK_LIBRARY-framework'

a2cfa2da4f GenEx/LINK_LIBRARY: Add features for framework support on Apple
40178f3c90 cmGlobalGenerator: Add helper to split framework path

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !6967

6 files changed, 164 insertions, 85 deletions

diff --git a/Source/cmComputeLinkInformation.cxx b/Source/cmComputeLinkInformation.cxx
index 15e9d60..cf8c9a1 100644
--- a/Source/cmComputeLinkInformation.cxx
+++ b/Source/cmComputeLinkInformation.cxx
@@ -8,7 +8,9 @@
 #include <utility>
 
 #include <cm/memory>
+#include <cm/optional>
 #include <cmext/algorithm>
+#include <cmext/string_view>
 
 #include "cmComputeLinkDepends.h"
 #include "cmGeneratorTarget.h"
@@ -18,7 +20,6 @@
 #include "cmMakefile.h"
 #include "cmMessageType.h"
 #include "cmOrderDirectories.h"
-#include "cmOutputConverter.h"
 #include "cmPlaceholderExpander.h"
 #include "cmPolicies.h"
 #include "cmState.h"
@@ -1044,12 +1045,14 @@ void cmComputeLinkInformation::AddItem(LinkEntry const& entry)
     }
   } else {
     // This is not a CMake target.  Use the name given.
-    if (cmSystemTools::FileIsFullPath(item.Value)) {
-      if (cmSystemTools::IsPathToFramework(item.Value) &&
-          this->Makefile->IsOn("APPLE")) {
-        // This is a framework.
-        this->AddFrameworkItem(entry);
-      } else if (cmSystemTools::FileIsDirectory(item.Value)) {
+    if (cmHasSuffix(entry.Feature, "FRAMEWORK"_s) ||
+        (entry.Feature == DEFAULT &&
+         cmSystemTools::IsPathToFramework(item.Value) &&
+         this->Makefile->IsOn("APPLE"))) {
+      // This is a framework.
+      this->AddFrameworkItem(entry);
+    } else if (cmSystemTools::FileIsFullPath(item.Value)) {
+      if (cmSystemTools::FileIsDirectory(item.Value)) {
         // This is a directory.
         this->DropDirectoryItem(item);
       } else {
@@ -1424,11 +1427,41 @@ void cmComputeLinkInformation::AddTargetItem(LinkEntry const& entry)
     this->OldLinkDirItems.push_back(item.Value);
   }
 
-  // Now add the full path to the library.
-  this->Items.emplace_back(item, ItemIsPath::Yes, target,
-                           this->FindLibraryFeature(entry.Feature == DEFAULT
-                                                      ? "__CMAKE_LINK_LIBRARY"
-                                                      : entry.Feature));
+  if (target->IsFrameworkOnApple() && this->GlobalGenerator->IsXcode() &&
+      entry.Feature == DEFAULT) {
+    // ensure FRAMEWORK feature is loaded
+    this->AddLibraryFeature("FRAMEWORK");
+  }
+
+  if (cmHasSuffix(entry.Feature, "FRAMEWORK"_s) &&
+      target->IsFrameworkOnApple() && !this->GlobalGenerator->IsXcode()) {
+    // Add the framework directory and the framework item itself
+    auto fwItems = this->GlobalGenerator->SplitFrameworkPath(item.Value, true);
+    if (!fwItems) {
+      this->CMakeInstance->IssueMessage(
+        MessageType::FATAL_ERROR,
+        cmStrCat("Could not parse framework path \"", item.Value,
+                 "\" linked by target ", this->Target->GetName(), '.'),
+        item.Backtrace);
+      return;
+    }
+    if (!fwItems->first.empty()) {
+      // Add the directory portion to the framework search path.
+      this->AddFrameworkPath(fwItems->first);
+    }
+    this->Items.emplace_back(fwItems->second, ItemIsPath::Yes, target,
+                             this->FindLibraryFeature(entry.Feature));
+  } else {
+    // Now add the full path to the library.
+    this->Items.emplace_back(
+      item, ItemIsPath::Yes, target,
+      this->FindLibraryFeature(
+        entry.Feature == DEFAULT
+          ? (target->IsFrameworkOnApple() && this->GlobalGenerator->IsXcode()
+               ? "FRAMEWORK"
+               : "__CMAKE_LINK_LIBRARY")
+          : entry.Feature));
+  }
 }
 
 void cmComputeLinkInformation::AddFullItem(LinkEntry const& entry)
@@ -1679,7 +1712,9 @@ void cmComputeLinkInformation::AddFrameworkItem(LinkEntry const& entry)
   std::string const& item = entry.Item.Value;
 
   // Try to separate the framework name and path.
-  if (!this->SplitFramework.find(item)) {
+  auto fwItems =
+    this->GlobalGenerator->SplitFrameworkPath(item, entry.Feature != DEFAULT);
+  if (!fwItems) {
     std::ostringstream e;
     e << "Could not parse framework path \"" << item << "\" "
       << "linked by target " << this->Target->GetName() << ".";
@@ -1687,26 +1722,36 @@ void cmComputeLinkInformation::AddFrameworkItem(LinkEntry const& entry)
     return;
   }
 
-  std::string fw_path = this->SplitFramework.match(1);
-  std::string fw = this->SplitFramework.match(2);
-  std::string full_fw = cmStrCat(fw_path, '/', fw, ".framework/", fw);
+  std::string fw_path = std::move(fwItems->first);
+  std::string fw = std::move(fwItems->second);
+  std::string full_fw = cmStrCat(fw, ".framework/", fw);
 
-  // Add the directory portion to the framework search path.
-  this->AddFrameworkPath(fw_path);
+  if (!fw_path.empty()) {
+    full_fw = cmStrCat(fw_path, '/', full_fw);
+    // Add the directory portion to the framework search path.
+    this->AddFrameworkPath(fw_path);
+  }
 
   // add runtime information
   this->AddLibraryRuntimeInfo(full_fw);
 
+  if (entry.Feature == DEFAULT) {
+    // ensure FRAMEWORK feature is loaded
+    this->AddLibraryFeature("FRAMEWORK");
+  }
+
   if (this->GlobalGenerator->IsXcode()) {
     // Add framework path - it will be handled by Xcode after it's added to
     // "Link Binary With Libraries" build phase
-    this->Items.emplace_back(item, ItemIsPath::Yes);
+    this->Items.emplace_back(item, ItemIsPath::Yes, nullptr,
+                             this->FindLibraryFeature(entry.Feature == DEFAULT
+                                                        ? "FRAMEWORK"
+                                                        : entry.Feature));
   } else {
-    // Add the item using the -framework option.
-    this->Items.emplace_back(std::string("-framework"), ItemIsPath::No);
-    cmOutputConverter converter(this->Makefile->GetStateSnapshot());
-    fw = converter.EscapeForShell(fw);
-    this->Items.emplace_back(fw, ItemIsPath::No);
+    this->Items.emplace_back(fw, ItemIsPath::Yes, nullptr,
+                             this->FindLibraryFeature(entry.Feature == DEFAULT
+                                                        ? "FRAMEWORK"
+                                                        : entry.Feature));
   }
 }
 
@@ -1739,9 +1784,6 @@ void cmComputeLinkInformation::ComputeFrameworkInfo()
 
   this->FrameworkPathsEmitted.insert(implicitDirVec.begin(),
                                      implicitDirVec.end());
-
-  // Regular expression to extract a framework path and name.
-  this->SplitFramework.compile("(.*)/(.*)\\.framework$");
 }
 
 void cmComputeLinkInformation::AddFrameworkPath(std::string const& p)
diff --git a/Source/cmComputeLinkInformation.h b/Source/cmComputeLinkInformation.h
index ce9f393..4b7fb1a 100644
--- a/Source/cmComputeLinkInformation.h
+++ b/Source/cmComputeLinkInformation.h
@@ -63,6 +63,11 @@ public:
     cmGeneratorTarget const* Target = nullptr;
 
     bool HasFeature() const { return this->Feature != nullptr; }
+    const std::string& GetFeatureName() const
+    {
+      return HasFeature() ? this->Feature->Name
+                          : cmComputeLinkDepends::LinkEntry::DEFAULT;
+    }
 
     BT<std::string> GetFormattedItem(std::string const& path) const
     {
@@ -205,7 +210,6 @@ private:
   void ComputeFrameworkInfo();
   void AddFrameworkPath(std::string const& p);
   std::set<std::string> FrameworkPathsEmitted;
-  cmsys::RegularExpression SplitFramework;
 
   // Linker search path computation.
   std::unique_ptr<cmOrderDirectories> OrderLinkerSearchPath;
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 156ecce..baa54e5 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -19,6 +19,7 @@
 
 #include "cmsys/Directory.hxx"
 #include "cmsys/FStream.hxx"
+#include "cmsys/RegularExpression.hxx"
 
 #if defined(_WIN32) && !defined(__CYGWIN__)
 #  include <windows.h>
@@ -2522,6 +2523,47 @@ bool cmGlobalGenerator::NameResolvesToFramework(
   return false;
 }
 
+// If the file has no extension it's either a raw executable or might
+// be a direct reference to a binary within a framework (bad practice!).
+// This is where we change the path to point to the framework directory.
+// .tbd files also can be located in SDK frameworks (they are
+// placeholders for actual libraries shipped with the OS)
+cm::optional<std::pair<std::string, std::string>>
+cmGlobalGenerator::SplitFrameworkPath(const std::string& path,
+                                      bool extendedFormat) const
+{
+  // Check for framework structure:
+  //    (/path/to/)?FwName.framework
+  // or (/path/to/)?FwName.framework/FwName(.tbd)?
+  // or (/path/to/)?FwName.framework/Versions/*/FwName(.tbd)?
+  static cmsys::RegularExpression frameworkPath(
+    "((.+)/)?(.+)\\.framework(/Versions/[^/]+)?(/(.+))?$");
+
+  auto ext = cmSystemTools::GetFilenameLastExtension(path);
+  if ((ext.empty() || ext == ".tbd" || ext == ".framework") &&
+      frameworkPath.find(path)) {
+    auto name = frameworkPath.match(3);
+    auto libname =
+      cmSystemTools::GetFilenameWithoutExtension(frameworkPath.match(6));
+    if (!libname.empty() && name != libname) {
+      return cm::nullopt;
+    }
+    return std::pair<std::string, std::string>{ frameworkPath.match(2), name };
+  }
+
+  if (extendedFormat) {
+    // path format can be more flexible: (/path/to/)?fwName(.framework)?
+    auto fwDir = cmSystemTools::GetParentDirectory(path);
+    auto name = cmSystemTools::GetFilenameLastExtension(path) == ".framework"
+      ? cmSystemTools::GetFilenameWithoutExtension(path)
+      : cmSystemTools::GetFilenameName(path);
+
+    return std::pair<std::string, std::string>{ fwDir, name };
+  }
+
+  return cm::nullopt;
+}
+
 bool cmGlobalGenerator::CheckCMP0037(std::string const& targetName,
                                      std::string const& reason) const
 {
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index a43d4a6..a4b2ae3 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -367,6 +367,13 @@ public:
   /** Determine if a name resolves to a framework on disk or a built target
       that is a framework. */
   bool NameResolvesToFramework(const std::string& libname) const;
+  /** Split a framework path to the directory and name of the framework
+   * returns std::nullopt if the path does not match with framework format
+   * when extendedFormat is true, required format is relaxed (i.e. extension
+   * `.framework' is optional). Used when FRAMEWORK link feature is
+   * specified */
+  cm::optional<std::pair<std::string, std::string>> SplitFrameworkPath(
+    const std::string& path, bool extendedFormat = false) const;
 
   cmMakefile* FindMakefile(const std::string& start_dir) const;
   cmLocalGenerator* FindLocalGenerator(cmDirectoryId const& id) const;
diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx
index 489c7fb..b752c41 100644
--- a/Source/cmGlobalXCodeGenerator.cxx
+++ b/Source/cmGlobalXCodeGenerator.cxx
@@ -1154,47 +1154,25 @@ std::string GetSourcecodeValueFromFileExtension(
   return sourcecode;
 }
 
-// If the file has no extension it's either a raw executable or might
-// be a direct reference to a binary within a framework (bad practice!).
-// This is where we change the path to point to the framework directory.
-// .tbd files also can be located in SDK frameworks (they are
-// placeholders for actual libraries shipped with the OS)
-std::string GetLibraryOrFrameworkPath(const std::string& path)
+} // anonymous
+
+// Extracts the framework directory, if path matches the framework syntax
+// otherwise returns the path untouched
+std::string cmGlobalXCodeGenerator::GetLibraryOrFrameworkPath(
+  const std::string& path) const
 {
-  auto ext = cmSystemTools::GetFilenameLastExtension(path);
-  if (ext.empty() || ext == ".tbd") {
-    auto name = cmSystemTools::GetFilenameWithoutExtension(path);
-    // Check for iOS framework structure:
-    //    FwName.framework/FwName (and also on macOS where FwName lib is a
-    //    symlink)
-    auto parentDir = cmSystemTools::GetParentDirectory(path);
-    auto parentName = cmSystemTools::GetFilenameWithoutExtension(parentDir);
-    ext = cmSystemTools::GetFilenameLastExtension(parentDir);
-    if (ext == ".framework" && name == parentName) {
-      return parentDir;
-    }
-    // Check for macOS framework structure:
-    //    FwName.framework/Versions/*/FwName
-    std::vector<std::string> components;
-    cmSystemTools::SplitPath(path, components);
-    if (components.size() > 3 &&
-        components[components.size() - 3] == "Versions") {
-      ext = cmSystemTools::GetFilenameLastExtension(
-        components[components.size() - 4]);
-      parentName = cmSystemTools::GetFilenameWithoutExtension(
-        components[components.size() - 4]);
-      if (ext == ".framework" && name == parentName) {
-        components.erase(components.begin() + components.size() - 3,
-                         components.end());
-        return cmSystemTools::JoinPath(components);
-      }
+  auto fwItems = this->SplitFrameworkPath(path);
+  if (fwItems) {
+    if (fwItems->first.empty()) {
+      return cmStrCat(fwItems->second, ".framework");
+    } else {
+      return cmStrCat(fwItems->first, '/', fwItems->second, ".framework");
     }
   }
+
   return path;
 }
 
-} // anonymous
-
 cmXCodeObject* cmGlobalXCodeGenerator::CreateXCodeFileReferenceFromPath(
   const std::string& fullpath, cmGeneratorTarget* target,
   const std::string& lang, cmSourceFile* sf)
@@ -1217,7 +1195,7 @@ cmXCodeObject* cmGlobalXCodeGenerator::CreateXCodeFileReferenceFromPath(
     ext = ext.substr(1);
   }
   if (fileType.empty()) {
-    path = GetLibraryOrFrameworkPath(path);
+    path = this->GetLibraryOrFrameworkPath(path);
     ext = cmSystemTools::GetFilenameLastExtension(path);
     if (!ext.empty()) {
       ext = ext.substr(1);
@@ -3541,13 +3519,14 @@ void cmGlobalXCodeGenerator::AddDependAndLinkInformation(cmXCodeObject* target)
     } else {
       linkDir = libItem->Value.Value;
     }
-    linkDir = GetLibraryOrFrameworkPath(linkDir);
-    bool isFramework = cmSystemTools::IsPathToFramework(linkDir);
-    linkDir = cmSystemTools::GetParentDirectory(linkDir);
-    if (isFramework) {
-      if (std::find(frameworkSearchPaths.begin(), frameworkSearchPaths.end(),
-                    linkDir) == frameworkSearchPaths.end()) {
-        frameworkSearchPaths.push_back(linkDir);
+    if (cmHasSuffix(libItem->GetFeatureName(), "FRAMEWORK"_s)) {
+      auto fwItems = this->SplitFrameworkPath(linkDir, true);
+      if (fwItems && !fwItems->first.empty()) {
+        linkDir = std::move(fwItems->first);
+        if (std::find(frameworkSearchPaths.begin(), frameworkSearchPaths.end(),
+                      linkDir) == frameworkSearchPaths.end()) {
+          frameworkSearchPaths.push_back(linkDir);
+        }
       }
     } else {
       if (std::find(linkSearchPaths.begin(), linkSearchPaths.end(), linkDir) ==
@@ -3555,7 +3534,7 @@ void cmGlobalXCodeGenerator::AddDependAndLinkInformation(cmXCodeObject* target)
         linkSearchPaths.push_back(linkDir);
       }
     }
-    // Add target dependency
+
     if (libItem->Target && !libItem->Target->IsImported()) {
       for (auto const& configName : this->CurrentConfigurationTypes) {
         target->AddDependTarget(configName, libItem->Target->GetName());
@@ -3729,24 +3708,27 @@ void cmGlobalXCodeGenerator::AddDependAndLinkInformation(cmXCodeObject* target)
           if (cmSystemTools::FileIsFullPath(cleanPath)) {
             cleanPath = cmSystemTools::CollapseFullPath(cleanPath);
           }
-          const auto libPath = GetLibraryOrFrameworkPath(cleanPath);
-          if (cmSystemTools::StringEndsWith(libPath.c_str(), ".framework")) {
-            const auto fwName =
-              cmSystemTools::GetFilenameWithoutExtension(libPath);
-            const auto fwDir = cmSystemTools::GetParentDirectory(libPath);
-            if (emitted.insert(fwDir).second) {
-              // This is a search path we had not added before and it isn't an
-              // implicit search path, so we need it
-              libPaths.Add("-F " + this->XCodeEscapePath(fwDir));
+          bool isFramework =
+            cmHasSuffix(libName.GetFeatureName(), "FRAMEWORK"_s);
+          if (isFramework) {
+            const auto fwItems =
+              this->SplitFrameworkPath(cleanPath, isFramework);
+            if (!fwItems->first.empty() &&
+                emitted.insert(fwItems->first).second) {
+              // This is a search path we had not added before and it isn't
+              // an implicit search path, so we need it
+              libPaths.Add("-F " + this->XCodeEscapePath(fwItems->first));
             }
-            libPaths.Add("-framework " + this->XCodeEscapePath(fwName));
+            libPaths.Add(
+              libName.GetFormattedItem(this->XCodeEscapePath(fwItems->second))
+                .Value);
           } else {
             libPaths.Add(
               libName.GetFormattedItem(this->XCodeEscapePath(cleanPath))
                 .Value);
           }
           if ((!libName.Target || libName.Target->IsImported()) &&
-              IsLinkPhaseLibraryExtension(libPath)) {
+              (isFramework || IsLinkPhaseLibraryExtension(cleanPath))) {
             // Create file reference for embedding
             auto it = this->ExternalLibRefs.find(cleanPath);
             if (it == this->ExternalLibRefs.end()) {
@@ -3914,8 +3896,8 @@ void cmGlobalXCodeGenerator::AddEmbeddedFrameworks(cmXCodeObject* target)
 {
   static const auto dstSubfolderSpec = "10";
 
-  // Despite the name, by default Xcode uses "Embed Frameworks" build phase for
-  // both frameworks and dynamic libraries
+  // Despite the name, by default Xcode uses "Embed Frameworks" build phase
+  // for both frameworks and dynamic libraries
   this->AddEmbeddedObjects(target, "Embed Frameworks",
                            "XCODE_EMBED_FRAMEWORKS", dstSubfolderSpec,
                            NoActionOnCopyByDefault);
diff --git a/Source/cmGlobalXCodeGenerator.h b/Source/cmGlobalXCodeGenerator.h
index ff6ffe8..98cebef 100644
--- a/Source/cmGlobalXCodeGenerator.h
+++ b/Source/cmGlobalXCodeGenerator.h
@@ -330,6 +330,8 @@ private:
   {
   }
 
+  std::string GetLibraryOrFrameworkPath(const std::string& path) const;
+
   std::string GetObjectsDirectory(const std::string& projName,
                                   const std::string& configName,
                                   const cmGeneratorTarget* t,