From e0aa060352f0ea663500e87086192f52440952b9 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Apr 2018 11:04:23 -0400 Subject: cmNinjaTargetGenerator: Move depfile logic to later in its function Clarify that other compile rule generation logic does not depend on it. --- Source/cmNinjaTargetGenerator.cxx | 96 ++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index fa7d95a..d982020 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -465,52 +465,6 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) vars.Includes = ""; } - // Tell ninja dependency format so all deps can be loaded into a database - std::string deptype; - std::string depfile; - std::string cldeps; - if (explicitPP) { - // The explicit preprocessing step will handle dependency scanning. - } else if (this->NeedDepTypeMSVC(lang)) { - deptype = "msvc"; - depfile.clear(); - flags += " /showIncludes"; - } else if (mf->IsOn("CMAKE_NINJA_CMCLDEPS_" + lang)) { - // For the MS resource compiler we need cmcldeps, but skip dependencies - // for source-file try_compile cases because they are always fresh. - if (!mf->GetIsSourceFileTryCompile()) { - deptype = "gcc"; - depfile = "$DEP_FILE"; - const std::string cl = mf->GetDefinition("CMAKE_C_COMPILER") - ? mf->GetSafeDefinition("CMAKE_C_COMPILER") - : mf->GetSafeDefinition("CMAKE_CXX_COMPILER"); - cldeps = "\""; - cldeps += cmSystemTools::GetCMClDepsCommand(); - cldeps += "\" " + lang + " " + vars.Source + " $DEP_FILE $out \""; - cldeps += mf->GetSafeDefinition("CMAKE_CL_SHOWINCLUDES_PREFIX"); - cldeps += "\" \"" + cl + "\" "; - } - } else { - deptype = "gcc"; - const char* langdeptype = mf->GetDefinition("CMAKE_NINJA_DEPTYPE_" + lang); - if (langdeptype) { - deptype = langdeptype; - } - depfile = "$DEP_FILE"; - const std::string flagsName = "CMAKE_DEPFILE_FLAGS_" + lang; - std::string depfileFlags = mf->GetSafeDefinition(flagsName); - if (!depfileFlags.empty()) { - cmSystemTools::ReplaceString(depfileFlags, "", "$DEP_FILE"); - cmSystemTools::ReplaceString(depfileFlags, "", "$out"); - cmSystemTools::ReplaceString(depfileFlags, "", - mf->GetDefinition("CMAKE_C_COMPILER")); - flags += " " + depfileFlags; - } - } - - vars.Flags = flags.c_str(); - vars.DependencyFile = depfile.c_str(); - std::unique_ptr rulePlaceholderExpander( this->GetLocalGenerator()->CreateRulePlaceholderExpander()); @@ -550,7 +504,7 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) vars.Source = "$in"; // Preprocessing and compilation use the same flags. - ppVars.Flags = vars.Flags; + std::string ppFlags = flags; // Move preprocessor definitions to the preprocessor rule. ppVars.Defines = vars.Defines; @@ -560,6 +514,8 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) // compilation rule still needs them for the INCLUDE directive. ppVars.Includes = vars.Includes; + ppVars.Flags = ppFlags.c_str(); + // Rule for preprocessing source file. std::vector ppCmds; cmSystemTools::ExpandListArgument(ppCmd, ppCmds); @@ -631,6 +587,52 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) /*generator*/ false); } + // Tell ninja dependency format so all deps can be loaded into a database + std::string deptype; + std::string depfile; + std::string cldeps; + if (explicitPP) { + // The explicit preprocessing step will handle dependency scanning. + } else if (this->NeedDepTypeMSVC(lang)) { + deptype = "msvc"; + depfile.clear(); + flags += " /showIncludes"; + } else if (mf->IsOn("CMAKE_NINJA_CMCLDEPS_" + lang)) { + // For the MS resource compiler we need cmcldeps, but skip dependencies + // for source-file try_compile cases because they are always fresh. + if (!mf->GetIsSourceFileTryCompile()) { + deptype = "gcc"; + depfile = "$DEP_FILE"; + const std::string cl = mf->GetDefinition("CMAKE_C_COMPILER") + ? mf->GetSafeDefinition("CMAKE_C_COMPILER") + : mf->GetSafeDefinition("CMAKE_CXX_COMPILER"); + cldeps = "\""; + cldeps += cmSystemTools::GetCMClDepsCommand(); + cldeps += "\" " + lang + " " + vars.Source + " $DEP_FILE $out \""; + cldeps += mf->GetSafeDefinition("CMAKE_CL_SHOWINCLUDES_PREFIX"); + cldeps += "\" \"" + cl + "\" "; + } + } else { + deptype = "gcc"; + const char* langdeptype = mf->GetDefinition("CMAKE_NINJA_DEPTYPE_" + lang); + if (langdeptype) { + deptype = langdeptype; + } + depfile = "$DEP_FILE"; + const std::string flagsName = "CMAKE_DEPFILE_FLAGS_" + lang; + std::string depfileFlags = mf->GetSafeDefinition(flagsName); + if (!depfileFlags.empty()) { + cmSystemTools::ReplaceString(depfileFlags, "", "$DEP_FILE"); + cmSystemTools::ReplaceString(depfileFlags, "", "$out"); + cmSystemTools::ReplaceString(depfileFlags, "", + mf->GetDefinition("CMAKE_C_COMPILER")); + flags += " " + depfileFlags; + } + } + + vars.Flags = flags.c_str(); + vars.DependencyFile = depfile.c_str(); + // Rule for compiling object file. std::vector compileCmds; if (lang == "CUDA") { -- cgit v0.12 From 8592c6326bb43ec46cf43e77e51d62212c145dfe Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Apr 2018 11:37:32 -0400 Subject: cmNinjaTargetGenerator: Move force-rspfile check to earlier --- Source/cmNinjaTargetGenerator.cxx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index d982020..f213cc0 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -867,6 +867,11 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( std::string const objectFileDir = cmSystemTools::GetFilenamePath(objectFileName); + bool const lang_supports_response = + !(language == "RC" || language == "CUDA"); + int const commandLineLengthLimit = + ((lang_supports_response && this->ForceResponseFile())) ? -1 : 0; + cmNinjaVars vars; vars["FLAGS"] = this->ComputeFlagsForObject(source, language); vars["DEFINES"] = this->ComputeDefines(source, language); @@ -1036,10 +1041,6 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( this->SetMsvcTargetPdbVariable(vars); - bool const lang_supports_response = - !(language == "RC" || language == "CUDA"); - int const commandLineLengthLimit = - ((lang_supports_response && this->ForceResponseFile())) ? -1 : 0; std::string const rspfile = objectFileName + ".rsp"; this->GetGlobalGenerator()->WriteBuild( -- cgit v0.12 From d6390ce26e6575d055f2ecda8642a2ad3568cb87 Mon Sep 17 00:00:00 2001 From: Brad King Date: Thu, 5 Apr 2018 11:38:36 -0400 Subject: Ninja: Fix Fortran support with response files The Ninja generator splits preprocessing and compilation steps for Fortran. Fix this logic to work when using response files for compilation so that it works for the preprocessing step too. This fixes behavior under `CMAKE_NINJA_FORCE_RESPONSE_FILE`. Issue: #17877 --- Source/cmNinjaTargetGenerator.cxx | 53 ++++++++++++++++++++----------- Tests/RunCMake/CMakeLists.txt | 3 ++ Tests/RunCMake/Ninja/RspFileC.cmake | 2 ++ Tests/RunCMake/Ninja/RspFileCXX.cmake | 2 ++ Tests/RunCMake/Ninja/RspFileFortran.cmake | 2 ++ Tests/RunCMake/Ninja/RunCMakeTest.cmake | 6 ++++ 6 files changed, 50 insertions(+), 18 deletions(-) create mode 100644 Tests/RunCMake/Ninja/RspFileC.cmake create mode 100644 Tests/RunCMake/Ninja/RspFileCXX.cmake create mode 100644 Tests/RunCMake/Ninja/RspFileFortran.cmake diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index f213cc0..ddb1d54 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -445,24 +445,16 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) cmMakefile* mf = this->GetMakefile(); std::string flags = "$FLAGS"; - std::string rspfile; - std::string rspcontent; + std::string responseFlag; bool const lang_supports_response = !(lang == "RC" || lang == "CUDA"); if (lang_supports_response && this->ForceResponseFile()) { std::string const responseFlagVar = "CMAKE_" + lang + "_RESPONSE_FILE_FLAG"; - std::string responseFlag = - this->Makefile->GetSafeDefinition(responseFlagVar); + responseFlag = this->Makefile->GetSafeDefinition(responseFlagVar); if (responseFlag.empty()) { responseFlag = "@"; } - rspfile = "$RSP_FILE"; - responseFlag += rspfile; - rspcontent = " $DEFINES $INCLUDES $FLAGS"; - flags = std::move(responseFlag); - vars.Defines = ""; - vars.Includes = ""; } std::unique_ptr rulePlaceholderExpander( @@ -514,6 +506,18 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) // compilation rule still needs them for the INCLUDE directive. ppVars.Includes = vars.Includes; + // If using a response file, move defines, includes, and flags into it. + std::string ppRspFile; + std::string ppRspContent; + if (!responseFlag.empty()) { + ppRspFile = "$RSP_FILE"; + ppRspContent = std::string(" ") + ppVars.Defines + " " + + ppVars.Includes + " " + ppFlags; + ppFlags = responseFlag + ppRspFile; + ppVars.Defines = ""; + ppVars.Includes = ""; + } + ppVars.Flags = ppFlags.c_str(); // Rule for preprocessing source file. @@ -544,13 +548,11 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) ppComment << "Rule for preprocessing " << lang << " files."; std::ostringstream ppDesc; ppDesc << "Building " << lang << " preprocessed $out"; - this->GetGlobalGenerator()->AddRule(this->LanguagePreprocessRule(lang), - ppCmdLine, ppDesc.str(), - ppComment.str(), ppDepfile, ppDeptype, - /*rspfile*/ "", - /*rspcontent*/ "", - /*restat*/ "", - /*generator*/ false); + this->GetGlobalGenerator()->AddRule( + this->LanguagePreprocessRule(lang), ppCmdLine, ppDesc.str(), + ppComment.str(), ppDepfile, ppDeptype, ppRspFile, ppRspContent, + /*restat*/ "", + /*generator*/ false); } if (needDyndep) { @@ -587,6 +589,18 @@ void cmNinjaTargetGenerator::WriteCompileRule(const std::string& lang) /*generator*/ false); } + // If using a response file, move defines, includes, and flags into it. + std::string rspfile; + std::string rspcontent; + if (!responseFlag.empty()) { + rspfile = "$RSP_FILE"; + rspcontent = + std::string(" ") + vars.Defines + " " + vars.Includes + " " + flags; + flags = responseFlag + rspfile; + vars.Defines = ""; + vars.Includes = ""; + } + // Tell ninja dependency format so all deps can be loaded into a database std::string deptype; std::string depfile; @@ -1019,9 +1033,12 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( this->addPoolNinjaVariable("JOB_POOL_COMPILE", this->GetGeneratorTarget(), ppVars); + std::string const ppRspFile = ppFileName + ".rsp"; + this->GetGlobalGenerator()->WriteBuild( this->GetBuildFileStream(), ppComment, ppRule, ppOutputs, ppImplicitOuts, - ppExplicitDeps, ppImplicitDeps, ppOrderOnlyDeps, ppVars); + ppExplicitDeps, ppImplicitDeps, ppOrderOnlyDeps, ppVars, ppRspFile, + commandLineLengthLimit); } if (needDyndep) { std::string const dyndep = this->GetDyndepFilePath(language); diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index c0c361e..8837821 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -121,6 +121,9 @@ if(CMAKE_GENERATOR STREQUAL "Ninja") -DCMAKE_C_OUTPUT_EXTENSION=${CMAKE_C_OUTPUT_EXTENSION} -DCMAKE_SHARED_LIBRARY_PREFIX=${CMAKE_SHARED_LIBRARY_PREFIX} -DCMAKE_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) + if(CMAKE_Fortran_COMPILER) + list(APPEND Ninja_ARGS -DTEST_Fortran=1) + endif() add_RunCMake_test(Ninja) endif() add_RunCMake_test(CTest) diff --git a/Tests/RunCMake/Ninja/RspFileC.cmake b/Tests/RunCMake/Ninja/RspFileC.cmake new file mode 100644 index 0000000..4a40682 --- /dev/null +++ b/Tests/RunCMake/Ninja/RspFileC.cmake @@ -0,0 +1,2 @@ +set(ENV{CMAKE_NINJA_FORCE_RESPONSE_FILE} 1) +enable_language(C) diff --git a/Tests/RunCMake/Ninja/RspFileCXX.cmake b/Tests/RunCMake/Ninja/RspFileCXX.cmake new file mode 100644 index 0000000..9e61ffe --- /dev/null +++ b/Tests/RunCMake/Ninja/RspFileCXX.cmake @@ -0,0 +1,2 @@ +set(ENV{CMAKE_NINJA_FORCE_RESPONSE_FILE} 1) +enable_language(CXX) diff --git a/Tests/RunCMake/Ninja/RspFileFortran.cmake b/Tests/RunCMake/Ninja/RspFileFortran.cmake new file mode 100644 index 0000000..8c18e37 --- /dev/null +++ b/Tests/RunCMake/Ninja/RspFileFortran.cmake @@ -0,0 +1,2 @@ +set(ENV{CMAKE_NINJA_FORCE_RESPONSE_FILE} 1) +enable_language(Fortran) diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index b3720fb..3bb2b6b 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -40,6 +40,12 @@ run_CMP0058(NEW-by) run_cmake(CustomCommandDepfile) +run_cmake(RspFileC) +run_cmake(RspFileCXX) +if(TEST_Fortran) + run_cmake(RspFileFortran) +endif() + function(run_CommandConcat) set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/CommandConcat-build) set(RunCMake_TEST_NO_CLEAN 1) -- cgit v0.12