Merge topic 'makefile-response-files'

c7a7c655 Makefile: Avoid link line object list lengths nearing system limits
34ba5c53 Makefile: Factor out response file checks into common helper

Source/cmMakefileExecutableTargetGenerator.cxx

@@ -277,27 +277,10 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
     }
   }
 
-  // Select whether to use a response file for objects.
-  bool useResponseFileForObjects = false;
-  {
-    std::string responseVar = "CMAKE_";
-    responseVar += linkLanguage;
-    responseVar += "_USE_RESPONSE_FILE_FOR_OBJECTS";
-    if (this->Makefile->IsOn(responseVar)) {
-      useResponseFileForObjects = true;
-    }
-  }
-
-  // Select whether to use a response file for libraries.
-  bool useResponseFileForLibs = false;
-  {
-    std::string responseVar = "CMAKE_";
-    responseVar += linkLanguage;
-    responseVar += "_USE_RESPONSE_FILE_FOR_LIBRARIES";
-    if (this->Makefile->IsOn(responseVar)) {
-      useResponseFileForLibs = true;
-    }
-  }
+  bool useResponseFileForObjects =
+    this->CheckUseResponseFileForObjects(linkLanguage);
+  bool const useResponseFileForLibs =
+    this->CheckUseResponseFileForLibraries(linkLanguage);
 
   // Expand the rule variables.
   {

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -429,27 +429,10 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules(
   // Determine whether a link script will be used.
   bool useLinkScript = this->GlobalGenerator->GetUseLinkScript();
 
-  // Select whether to use a response file for objects.
-  bool useResponseFileForObjects = false;
-  {
-    std::string responseVar = "CMAKE_";
-    responseVar += linkLanguage;
-    responseVar += "_USE_RESPONSE_FILE_FOR_OBJECTS";
-    if (this->Makefile->IsOn(responseVar)) {
-      useResponseFileForObjects = true;
-    }
-  }
-
-  // Select whether to use a response file for libraries.
-  bool useResponseFileForLibs = false;
-  {
-    std::string responseVar = "CMAKE_";
-    responseVar += linkLanguage;
-    responseVar += "_USE_RESPONSE_FILE_FOR_LIBRARIES";
-    if (this->Makefile->IsOn(responseVar)) {
-      useResponseFileForLibs = true;
-    }
-  }
+  bool useResponseFileForObjects =
+    this->CheckUseResponseFileForObjects(linkLanguage);
+  bool const useResponseFileForLibs =
+    this->CheckUseResponseFileForLibraries(linkLanguage);
 
   // For static libraries there might be archiving rules.
   bool haveStaticLibraryRule = false;

Source/cmMakefileTargetGenerator.cxx

@@ -31,6 +31,10 @@
 
 #include <ctype.h>
 
+#ifndef _WIN32
+#include <unistd.h>
+#endif
+
 cmMakefileTargetGenerator::cmMakefileTargetGenerator(cmGeneratorTarget* target)
   : cmCommonTargetGenerator(target)
   , OSXBundleGenerator(CM_NULLPTR)
@@ -1447,6 +1451,73 @@ void cmMakefileTargetGenerator::CreateLinkScript(
   makefile_depends.push_back(linkScriptName);
 }
 
+static size_t calculateCommandLineLengthLimit()
+{
+#if defined(_SC_ARG_MAX)
+  return ((size_t)sysconf(_SC_ARG_MAX)) - 1000;
+#else
+  return 0;
+#endif
+}
+
+bool cmMakefileTargetGenerator::CheckUseResponseFileForObjects(
+  std::string const& l) const
+{
+  // Check for an explicit setting one way or the other.
+  std::string const responseVar =
+    "CMAKE_" + l + "_USE_RESPONSE_FILE_FOR_OBJECTS";
+  if (const char* val = this->Makefile->GetDefinition(responseVar)) {
+    if (*val) {
+      return cmSystemTools::IsOn(val);
+    }
+  }
+
+  // Check for a system limit.
+  if (size_t const limit = calculateCommandLineLengthLimit()) {
+    // Compute the total length of our list of object files with room
+    // for argument separation and quoting.  This does not convert paths
+    // relative to START_OUTPUT like the final list will be, so the actual
+    // list will likely be much shorter than this.  However, in the worst
+    // case all objects will remain as absolute paths.
+    size_t length = 0;
+    for (std::vector<std::string>::const_iterator i = this->Objects.begin();
+         i != this->Objects.end(); ++i) {
+      length += i->size() + 3;
+    }
+    for (std::vector<std::string>::const_iterator i =
+           this->ExternalObjects.begin();
+         i != this->ExternalObjects.end(); ++i) {
+      length += i->size() + 3;
+    }
+
+    // We need to guarantee room for both objects and libraries, so
+    // if the objects take up more than half then use a response file
+    // for them.
+    if (length > (limit / 2)) {
+      return true;
+    }
+  }
+
+  // We do not need a response file for objects.
+  return false;
+}
+
+bool cmMakefileTargetGenerator::CheckUseResponseFileForLibraries(
+  std::string const& l) const
+{
+  // Check for an explicit setting one way or the other.
+  std::string const responseVar =
+    "CMAKE_" + l + "_USE_RESPONSE_FILE_FOR_LIBRARIES";
+  if (const char* val = this->Makefile->GetDefinition(responseVar)) {
+    if (*val) {
+      return cmSystemTools::IsOn(val);
+    }
+  }
+
+  // We do not need a response file for libraries.
+  return false;
+}
+
 std::string cmMakefileTargetGenerator::CreateResponseFile(
   const char* name, std::string const& options,
   std::vector<std::string>& makefile_depends)

Source/cmMakefileTargetGenerator.h

@@ -151,6 +151,9 @@ protected:
   std::string CreateResponseFile(const char* name, std::string const& options,
                                  std::vector<std::string>& makefile_depends);
 
+  bool CheckUseResponseFileForObjects(std::string const& l) const;
+  bool CheckUseResponseFileForLibraries(std::string const& l) const;
+
   /** Create list of flags for link libraries. */
   void CreateLinkLibs(std::string& linkLibs, bool relink, bool useResponseFile,
                       std::vector<std::string>& makefile_depends,