From 689616785f76acd844fd448c51c5b2a0711aafa2 Mon Sep 17 00:00:00 2001
From: Brad King <brad.king@kitware.com>
Date: Tue, 28 Mar 2023 17:28:44 -0400
Subject: macOS: Do not pass Apple-specific flags to llvm-strip

Since commit cf82300a63 (BinUtils: Clarify search logic and make it more
consistent, 2021-05-27, v3.21.0-rc1~119^2~2) we prefer `llvm-strip` over
`strip` when using Clang.  However, since commit 20291e8e72 (install:
Fix stripping on macOS, 2019-01-30, v3.14.0-rc1~31^2) on macOS we add
flags `-u -r`, needed by Apple's `strip` for executables, but that
`llvm-strip` does not need or support.  Improve the condition to add
Apple-specific flags only when the selected `strip` tool is Apple's.

Note that Apple dylibs must be stripped with `-x` with either Apple's
`strip` or `llvm-strip`.

Fixes: #24601
---
 Source/cmGlobalGenerator.cxx                      | 29 +++++++++++++++++++++++
 Source/cmGlobalGenerator.h                        | 11 +++++++++
 Source/cmInstallRuntimeDependencySetGenerator.cxx |  3 +--
 Source/cmInstallTargetGenerator.cxx               | 17 +++++++------
 4 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 0e9f78e..08022b3 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -3542,3 +3542,32 @@ cmInstallRuntimeDependencySet* cmGlobalGenerator::GetNamedRuntimeDependencySet(
   }
   return it->second;
 }
+
+cmGlobalGenerator::StripCommandStyle cmGlobalGenerator::GetStripCommandStyle(
+  std::string const& strip)
+{
+#ifdef __APPLE__
+  auto i = this->StripCommandStyleMap.find(strip);
+  if (i == this->StripCommandStyleMap.end()) {
+    StripCommandStyle style = StripCommandStyle::Default;
+
+    // Try running strip tool with Apple-specific options.
+    std::vector<std::string> cmd{ strip, "-u", "-r" };
+    std::string out;
+    std::string err;
+    int ret;
+    if (cmSystemTools::RunSingleCommand(cmd, &out, &err, &ret, nullptr,
+                                        cmSystemTools::OUTPUT_NONE) &&
+        // Check for Apple-specific output.
+        ret != 0 && cmHasLiteralPrefix(err, "fatal error: /") &&
+        err.find("/usr/bin/strip: no files specified") != std::string::npos) {
+      style = StripCommandStyle::Apple;
+    }
+    i = this->StripCommandStyleMap.emplace(strip, style).first;
+  }
+  return i->second;
+#else
+  static_cast<void>(strip);
+  return StripCommandStyle::Default;
+#endif
+}
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 4d321b5..4bbbdfa 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -607,6 +607,13 @@ public:
   cmInstallRuntimeDependencySet* GetNamedRuntimeDependencySet(
     const std::string& name);
 
+  enum class StripCommandStyle
+  {
+    Default,
+    Apple,
+  };
+  StripCommandStyle GetStripCommandStyle(std::string const& strip);
+
 protected:
   // for a project collect all its targets by following depend
   // information, and also collect all the targets
@@ -737,6 +744,10 @@ private:
   std::map<std::string, int> LanguageToLinkerPreference;
   std::map<std::string, std::string> LanguageToOriginalSharedLibFlags;
 
+#ifdef __APPLE__
+  std::map<std::string, StripCommandStyle> StripCommandStyleMap;
+#endif
+
   mutable bool DiagnosedCxxModuleSupport = false;
 
   // Deferral id generation.
diff --git a/Source/cmInstallRuntimeDependencySetGenerator.cxx b/Source/cmInstallRuntimeDependencySetGenerator.cxx
index 44f03e1..1e2e663 100644
--- a/Source/cmInstallRuntimeDependencySetGenerator.cxx
+++ b/Source/cmInstallRuntimeDependencySetGenerator.cxx
@@ -256,8 +256,7 @@ void cmInstallRuntimeDependencySetGenerator::GenerateStripFixup(
   if (!strip.empty()) {
     os << indent << "if(CMAKE_INSTALL_DO_STRIP)\n"
        << indent.Next() << "execute_process(COMMAND \"" << strip << "\" ";
-    if (this->LocalGenerator->GetMakefile()->GetSafeDefinition(
-          "CMAKE_HOST_SYSTEM_NAME") == "Darwin") {
+    if (this->LocalGenerator->GetMakefile()->IsOn("APPLE")) {
       os << "-x ";
     }
     os << "\""
diff --git a/Source/cmInstallTargetGenerator.cxx b/Source/cmInstallTargetGenerator.cxx
index 6c31da6..d967d50 100644
--- a/Source/cmInstallTargetGenerator.cxx
+++ b/Source/cmInstallTargetGenerator.cxx
@@ -826,26 +826,29 @@ void cmInstallTargetGenerator::AddStripRule(std::ostream& os, Indent indent,
     return;
   }
 
-  if (!this->Target->Target->GetMakefile()->IsSet("CMAKE_STRIP")) {
+  std::string const& strip =
+    this->Target->Target->GetMakefile()->GetSafeDefinition("CMAKE_STRIP");
+  if (strip.empty()) {
     return;
   }
 
   std::string stripArgs;
-
-  // macOS 'strip' is picky, executables need '-u -r' and dylibs need '-x'.
   if (this->Target->IsApple()) {
     if (this->Target->GetType() == cmStateEnums::SHARED_LIBRARY ||
         this->Target->GetType() == cmStateEnums::MODULE_LIBRARY) {
+      // Strip tools need '-x' to strip Apple dylibs correctly.
       stripArgs = "-x ";
-    } else if (this->Target->GetType() == cmStateEnums::EXECUTABLE) {
+    } else if (this->Target->GetType() == cmStateEnums::EXECUTABLE &&
+               this->Target->GetGlobalGenerator()->GetStripCommandStyle(
+                 strip) == cmGlobalGenerator::StripCommandStyle::Apple) {
+      // Apple's strip tool needs '-u -r' to strip executables correctly.
       stripArgs = "-u -r ";
     }
   }
 
   os << indent << "if(CMAKE_INSTALL_DO_STRIP)\n";
-  os << indent << "  execute_process(COMMAND \""
-     << this->Target->Target->GetMakefile()->GetSafeDefinition("CMAKE_STRIP")
-     << "\" " << stripArgs << "\"" << toDestDirPath << "\")\n";
+  os << indent << "  execute_process(COMMAND \"" << strip << "\" " << stripArgs
+     << "\"" << toDestDirPath << "\")\n";
   os << indent << "endif()\n";
 }
 
-- 
cgit v0.12