From 654608600474a27eeddafd50ea25c7e3882fd460 Mon Sep 17 00:00:00 2001 From: Peter Kuemmel Date: Thu, 14 Jun 2012 14:15:16 +0200 Subject: 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 | 10 +++- Source/cmGlobalNinjaGenerator.h | 1 + Source/cmLocalNinjaGenerator.cxx | 13 ++++- Source/cmLocalNinjaGenerator.h | 1 + Source/cmNinjaNormalTargetGenerator.cxx | 88 ++++++++++++++++++++------------- Source/cmNinjaNormalTargetGenerator.h | 2 +- 6 files changed, 77 insertions(+), 38 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 11d8653093..2c00e06057 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/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; diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index 2055375bfa..244fd3b1c8 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -84,6 +84,7 @@ public: const cmNinjaDeps& implicitDeps, const cmNinjaDeps& orderOnlyDeps, const cmNinjaVars& variables, + bool suppressShell = false, int cmdLineLimit = -1); /** diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 5d193cde41..71f0913439 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/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 &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; diff --git a/Source/cmLocalNinjaGenerator.h b/Source/cmLocalNinjaGenerator.h index ea44b2fe29..9df4afb570 100644 --- a/Source/cmLocalNinjaGenerator.h +++ b/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 &cmdLines); void AppendCustomCommandLines(const cmCustomCommand *cc, std::vector &cmdLines); diff --git a/Source/cmNinjaNormalTargetGenerator.cxx b/Source/cmNinjaNormalTargetGenerator.cxx index 81715dcd4e..3030cf9a03 100644 --- a/Source/cmNinjaNormalTargetGenerator.cxx +++ b/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::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) { diff --git a/Source/cmNinjaNormalTargetGenerator.h b/Source/cmNinjaNormalTargetGenerator.h index 1ef95675b9..299b3e15f8 100644 --- a/Source/cmNinjaNormalTargetGenerator.h +++ b/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 ComputeLinkCmd(); -- cgit v1.2.1