Ninja: don't use shell when cmake is called directly

When linking with cmake and vs_link_* the command line
could be too long for cmd.exe, which needs not to be
called in this case. (was not cached by a test)

Introduce rules which don't use the shell and use this
rule when there are no pre or post step.

For free we get a small speedup, because cmd is then
not called.

Also be more accurate when estimating the
command line length.

Source/cmGlobalNinjaGenerator.cxx

@@ -106,6 +106,7 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
                                         const cmNinjaDeps& implicitDeps,
                                         const cmNinjaDeps& orderOnlyDeps,
                                         const cmNinjaVars& variables,
+                                        bool suppressShell,
                                         int cmdLineLimit)
 {
   // Make sure there is a rule.
@@ -177,8 +178,15 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
 
   // check if a response file rule should be used
   const std::string args = arguments.str();
-  if (cmdLineLimit > 0 && args.size() > (size_t)cmdLineLimit)
+  if (suppressShell)
+    {
+    builds << "_NOSHELL";
+    }
+  else if (cmdLineLimit > 0 &&
+      args.size() + builds.str().size() > (size_t)cmdLineLimit)
+    {
     builds << "_RSPFILE";
+    }
 
   os << builds.str() << args;
 

Source/cmGlobalNinjaGenerator.h

@@ -84,6 +84,7 @@ public:
                          const cmNinjaDeps& implicitDeps,
                          const cmNinjaDeps& orderOnlyDeps,
                          const cmNinjaVars& variables,
+                         bool suppressShell = false,
                          int cmdLineLimit = -1);
 
   /**

Source/cmLocalNinjaGenerator.cxx

@@ -264,6 +264,14 @@ void cmLocalNinjaGenerator::AppendCustomCommandDeps(const cmCustomCommand *cc,
   }
 }
 
+std::string cmLocalNinjaGenerator::nopCommand() const {
+#ifdef _WIN32
+    return "cd .";
+#else
+    return ":";
+#endif
+}
+
 std::string cmLocalNinjaGenerator::BuildCommandLine(
                                     const std::vector<std::string> &cmdLines)
 {
@@ -272,9 +280,10 @@ std::string cmLocalNinjaGenerator::BuildCommandLine(
   // don't use POST_BUILD.
   if (cmdLines.empty())
 #ifdef _WIN32
-    return "cd.";
+    return "";
 #else
-    return ":";
+    // TODO use _NOSHELL rule also on Linux
+    return nopCommand();
 #endif
 
   std::ostringstream cmd;

Source/cmLocalNinjaGenerator.h

@@ -111,6 +111,7 @@ private:
 
   void AppendCustomCommandDeps(const cmCustomCommand *cc,
                                cmNinjaDeps &ninjaDeps);
+  std::string nopCommand() const;
   std::string BuildCommandLine(const std::vector<std::string> &cmdLines);
   void AppendCustomCommandLines(const cmCustomCommand *cc,
                                 std::vector<std::string> &cmdLines);

Source/cmNinjaNormalTargetGenerator.cxx

@@ -75,9 +75,13 @@ void cmNinjaNormalTargetGenerator::Generate()
     }
   else
     {
-    this->WriteLinkRule(false);
-#ifdef _WIN32 // TODO response file support only Linux
-    this->WriteLinkRule(true);
+    this->WriteLinkRule();
+#ifdef _WIN32
+    // TODO remove hardcoded strings
+    this->WriteLinkRule("_RSPFILE");
+    this->WriteLinkRule("_NOSHELL");
+#else
+     // TODO response file support only Linux
 #endif
     this->WriteLinkStatement();
     }
@@ -131,12 +135,18 @@ cmNinjaNormalTargetGenerator
 
 void
 cmNinjaNormalTargetGenerator
-::WriteLinkRule(bool useResponseFile)
+::WriteLinkRule(const std::string& postfix)
 {
   cmTarget::TargetType targetType = this->GetTarget()->GetType();
   std::string ruleName = this->LanguageLinkerRule();
-  if (useResponseFile)
-    ruleName += "_RSPFILE";
+
+  bool useResponseFile = false;
+  bool suppressShell = false;
+  if (!postfix.empty()) {
+    ruleName += postfix;
+    useResponseFile = (postfix == "_RSPFILE");
+    suppressShell   = (postfix == "_NOSHELL");
+  }
 
   // Select whether to use a response file for objects.
   std::string rspfile;
@@ -210,8 +220,10 @@ cmNinjaNormalTargetGenerator
       {
       this->GetLocalGenerator()->ExpandRuleVariables(*i, vars);
       }
-    linkCmds.insert(linkCmds.begin(), "$PRE_LINK");
-    linkCmds.push_back("$POST_BUILD");
+    if (!suppressShell) {
+      linkCmds.insert(linkCmds.begin(), "$PRE_LINK");
+      linkCmds.push_back("$POST_BUILD");
+    }
     std::string linkCmd =
       this->GetLocalGenerator()->BuildCommandLine(linkCmds);
 
@@ -331,6 +343,7 @@ cmNinjaNormalTargetGenerator
 
 void cmNinjaNormalTargetGenerator::WriteLinkStatement()
 {
+  cmLocalNinjaGenerator* locGtor = this->GetLocalGenerator();
   cmTarget::TargetType targetType = this->GetTarget()->GetType();
 
   // Write comments.
@@ -368,18 +381,17 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
   cmNinjaDeps explicitDeps = this->GetObjects(),
               implicitDeps = this->ComputeLinkDeps();
 
-  this->GetLocalGenerator()->GetTargetFlags(vars["LINK_LIBRARIES"],
-                                            vars["FLAGS"],
-                                            vars["LINK_FLAGS"],
-                                            *this->GetTarget());
+  locGtor->GetTargetFlags(vars["LINK_LIBRARIES"],
+                          vars["FLAGS"],
+                          vars["LINK_FLAGS"],
+                          *this->GetTarget());
 
   this->AddModuleDefinitionFlag(vars["LINK_FLAGS"]);
 
   // Compute architecture specific link flags.  Yes, these go into a different
   // variable for executables, probably due to a mistake made when duplicating
   // code between the Makefile executable and library generators.
-  this->GetLocalGenerator()
-      ->AddArchitectureFlags(targetType == cmTarget::EXECUTABLE
+  locGtor->AddArchitectureFlags(targetType == cmTarget::EXECUTABLE
                                ? vars["FLAGS"]
                                : vars["ARCH_FLAGS"],
                              this->GetTarget(),
@@ -395,16 +407,16 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
 
       if (!install_name_dir.empty()) {
         vars["INSTALLNAME_DIR"] =
-          this->GetLocalGenerator()->Convert(install_name_dir.c_str(),
-              cmLocalGenerator::NONE,
-              cmLocalGenerator::SHELL, false);
+          locGtor->Convert(install_name_dir.c_str(),
+                           cmLocalGenerator::NONE,
+                           cmLocalGenerator::SHELL, false);
       }
     }
   }
 
   std::string path;
   if (!this->TargetNameImport.empty()) {
-    path = this->GetLocalGenerator()->ConvertToOutputFormat(
+    path = locGtor->ConvertToOutputFormat(
                     targetOutputImplib.c_str(), cmLocalGenerator::SHELL);
     vars["TARGET_IMPLIB"] = path;
     EnsureParentDirectoryExists(path);
@@ -416,7 +428,7 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
       mf->GetDefinition("MSVC_CXX_ARCHITECTURE_ID"))
     {
     path = this->GetTargetPDB();
-    vars["TARGET_PDB"] = this->GetLocalGenerator()->ConvertToOutputFormat(
+    vars["TARGET_PDB"] = locGtor->ConvertToOutputFormat(
                           ConvertToNinjaPath(path.c_str()).c_str(),
                           cmLocalGenerator::SHELL);
     EnsureParentDirectoryExists(path);
@@ -446,38 +458,45 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
     for (std::vector<cmCustomCommand>::const_iterator
          ci = cmdLists[i]->begin();
          ci != cmdLists[i]->end(); ++ci) {
-      this->GetLocalGenerator()->AppendCustomCommandLines(&*ci,
-                                                          *cmdLineLists[i]);
+      locGtor->AppendCustomCommandLines(&*ci, *cmdLineLists[i]);
     }
   }
 
   // If we have any PRE_LINK commands, we need to go back to HOME_OUTPUT for
   // the link commands.
   if (!preLinkCmdLines.empty()) {
-    path = this->GetLocalGenerator()->ConvertToOutputFormat(
+    path = locGtor->ConvertToOutputFormat(
       this->GetMakefile()->GetHomeOutputDirectory(),
       cmLocalGenerator::SHELL);
     preLinkCmdLines.push_back("cd " + path);
+    vars["PRE_LINK"] = locGtor->BuildCommandLine(preLinkCmdLines);
   }
 
-  vars["PRE_LINK"] =
-    this->GetLocalGenerator()->BuildCommandLine(preLinkCmdLines);
-  std::string postBuildCmdLine =
-    this->GetLocalGenerator()->BuildCommandLine(postBuildCmdLines);
-
   cmNinjaVars symlinkVars;
-  if (targetOutput == targetOutputReal) {
-    vars["POST_BUILD"] = postBuildCmdLine;
-  } else {
-    vars["POST_BUILD"] = ":";
-    symlinkVars["POST_BUILD"] = postBuildCmdLine;
+  if (!postBuildCmdLines.empty()) {
+    std::string postBuildCmdLine =
+      locGtor->BuildCommandLine(postBuildCmdLines);
+
+    if (targetOutput == targetOutputReal) {
+      vars["POST_BUILD"] = postBuildCmdLine;
+    } else {
+      vars["POST_BUILD"] = ":";
+      symlinkVars["POST_BUILD"] = postBuildCmdLine;
+    }
+    if (preLinkCmdLines.empty()) {
+      // rule with PRE_LINK will be selected, feed it
+      vars["PRE_LINK"] = locGtor->nopCommand();
+    }
   }
 
+  bool suppressShell = preLinkCmdLines.empty() && postBuildCmdLines.empty();
+
   int cmdLineLimit = -1;
 #ifdef _WIN32
-  cmdLineLimit = 8100;
+  cmdLineLimit = 8000;
 #else
-  // TODO
+  // cmdLineLimit = ?? TODO
+  isCmdSequenc = true;
 #endif
   // Write the build statement for this target.
   cmGlobalNinjaGenerator::WriteBuild(this->GetBuildFileStream(),
@@ -488,6 +507,7 @@ void cmNinjaNormalTargetGenerator::WriteLinkStatement()
                                      implicitDeps,
                                      emptyDeps,
                                      vars,
+                                     suppressShell,
                                      cmdLineLimit);
 
   if (targetOutput != targetOutputReal) {

Source/cmNinjaNormalTargetGenerator.h

@@ -30,7 +30,7 @@ private:
   std::string LanguageLinkerRule() const;
   const char* GetVisibleTypeName() const;
   void WriteLanguagesRules();
-  void WriteLinkRule(bool useResponseFile);
+  void WriteLinkRule(const std::string& postfix = "");
   void WriteLinkStatement();
   void WriteObjectLibStatement();
   std::vector<std::string> ComputeLinkCmd();