diff options
35 files changed, 268 insertions, 130 deletions
diff --git a/Help/release/dev/ninja-loosen-object-deps.rst b/Help/release/dev/ninja-loosen-object-deps.rst new file mode 100644 index 0000000..c47fb93 --- /dev/null +++ b/Help/release/dev/ninja-loosen-object-deps.rst @@ -0,0 +1,8 @@ +ninja-loosen-object-deps +------------------------ + +* The :generator:`Ninja` generator has loosened dependencies on object + compilation to depend on the custom targets and commands of dependent + libraries instead of the libraries themselves. This helps projects with deep + dependency graphs to be blocked only on their link steps at the deeper + levels rather than also blocking object compilation on dependent link steps. diff --git a/Source/CMakeVersion.cmake b/Source/CMakeVersion.cmake index ea6e0e2..fcb0b8b 100644 --- a/Source/CMakeVersion.cmake +++ b/Source/CMakeVersion.cmake @@ -1,5 +1,5 @@ # CMake version number components. set(CMake_VERSION_MAJOR 3) set(CMake_VERSION_MINOR 8) -set(CMake_VERSION_PATCH 20170421) +set(CMake_VERSION_PATCH 20170424) #set(CMake_VERSION_RC 1) diff --git a/Source/CPack/IFW/cmCPackIFWInstaller.cxx b/Source/CPack/IFW/cmCPackIFWInstaller.cxx index 3aebbc8..57b47f1 100644 --- a/Source/CPack/IFW/cmCPackIFWInstaller.cxx +++ b/Source/CPack/IFW/cmCPackIFWInstaller.cxx @@ -288,7 +288,7 @@ public: hasFiles = false; hasErrors = false; - basePath = cmSystemTools::GetFilenamePath(installer->Resources[r].data()); + basePath = cmSystemTools::GetFilenamePath(installer->Resources[r]); ParseFile(installer->Resources[r].data()); @@ -360,8 +360,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() cmSystemTools::GetFilenameName(InstallerApplicationIcon); std::string path = Directory + "/config/" + name; name = cmSystemTools::GetFilenameWithoutExtension(name); - cmsys::SystemTools::CopyFileIfDifferent(InstallerApplicationIcon.data(), - path.data()); + cmsys::SystemTools::CopyFileIfDifferent(InstallerApplicationIcon, path); xout.Element("InstallerApplicationIcon", name); } @@ -369,8 +368,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!InstallerWindowIcon.empty()) { std::string name = cmSystemTools::GetFilenameName(InstallerWindowIcon); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(InstallerWindowIcon.data(), - path.data()); + cmsys::SystemTools::CopyFileIfDifferent(InstallerWindowIcon, path); xout.Element("InstallerWindowIcon", name); } @@ -378,7 +376,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!Logo.empty()) { std::string name = cmSystemTools::GetFilenameName(Logo); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Logo.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Logo, path); xout.Element("Logo", name); } @@ -386,7 +384,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!Banner.empty()) { std::string name = cmSystemTools::GetFilenameName(Banner); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Banner.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Banner, path); xout.Element("Banner", name); } @@ -394,7 +392,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!Watermark.empty()) { std::string name = cmSystemTools::GetFilenameName(Watermark); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Watermark.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Watermark, path); xout.Element("Watermark", name); } @@ -402,7 +400,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!Background.empty()) { std::string name = cmSystemTools::GetFilenameName(Background); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Background.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Background, path); xout.Element("Background", name); } @@ -480,7 +478,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (!IsVersionLess("2.0") && !ControlScript.empty()) { std::string name = cmSystemTools::GetFilenameName(ControlScript); std::string path = Directory + "/config/" + name; - cmsys::SystemTools::CopyFileIfDifferent(ControlScript.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(ControlScript, path); xout.Element("ControlScript", name); } @@ -492,8 +490,7 @@ void cmCPackIFWInstaller::GenerateInstallerFile() if (parser.ParseResource(i)) { std::string name = cmSystemTools::GetFilenameName(Resources[i]); std::string path = Directory + "/resources/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Resources[i].data(), - path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Resources[i], path); resources.push_back(name); } else { cmCPackLogger(cmCPackLog::LOG_WARNING, "Can't copy resources from \"" diff --git a/Source/CPack/IFW/cmCPackIFWPackage.cxx b/Source/CPack/IFW/cmCPackIFWPackage.cxx index 2a95ba8..99e8b9e 100644 --- a/Source/CPack/IFW/cmCPackIFWPackage.cxx +++ b/Source/CPack/IFW/cmCPackIFWPackage.cxx @@ -590,7 +590,7 @@ void cmCPackIFWPackage::GeneratePackageFile() if (!Script.empty()) { std::string name = cmSystemTools::GetFilenameName(Script); std::string path = Directory + "/meta/" + name; - cmsys::SystemTools::CopyFileIfDifferent(Script.data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(Script, path); xout.Element("Script", name); } @@ -599,8 +599,7 @@ void cmCPackIFWPackage::GeneratePackageFile() for (size_t i = 0; i < userInterfaces.size(); i++) { std::string name = cmSystemTools::GetFilenameName(userInterfaces[i]); std::string path = Directory + "/meta/" + name; - cmsys::SystemTools::CopyFileIfDifferent(userInterfaces[i].data(), - path.data()); + cmsys::SystemTools::CopyFileIfDifferent(userInterfaces[i], path); userInterfaces[i] = name; } if (!userInterfaces.empty()) { @@ -616,8 +615,7 @@ void cmCPackIFWPackage::GeneratePackageFile() for (size_t i = 0; i < translations.size(); i++) { std::string name = cmSystemTools::GetFilenameName(translations[i]); std::string path = Directory + "/meta/" + name; - cmsys::SystemTools::CopyFileIfDifferent(translations[i].data(), - path.data()); + cmsys::SystemTools::CopyFileIfDifferent(translations[i], path); translations[i] = name; } if (!translations.empty()) { @@ -675,7 +673,7 @@ void cmCPackIFWPackage::GeneratePackageFile() for (size_t i = 1; i < licenses.size(); i += 2) { std::string name = cmSystemTools::GetFilenameName(licenses[i]); std::string path = Directory + "/meta/" + name; - cmsys::SystemTools::CopyFileIfDifferent(licenses[i].data(), path.data()); + cmsys::SystemTools::CopyFileIfDifferent(licenses[i], path); licenses[i] = name; } if (!licenses.empty()) { diff --git a/Source/CTest/cmCTestCoverageHandler.cxx b/Source/CTest/cmCTestCoverageHandler.cxx index 4d970d5..87c532c 100644 --- a/Source/CTest/cmCTestCoverageHandler.cxx +++ b/Source/CTest/cmCTestCoverageHandler.cxx @@ -2222,7 +2222,8 @@ void cmCTestCoverageHandler::LoadLabels(const char* dir) if (line.empty() || line[0] == '#') { // Ignore blank and comment lines. continue; - } else if (line[0] == ' ') { + } + if (line[0] == ' ') { // Label lines appear indented by one space. std::string label = line.substr(1); int id = this->GetLabelId(label); diff --git a/Source/CTest/cmCTestLaunch.cxx b/Source/CTest/cmCTestLaunch.cxx index 7195bb3..a782150 100644 --- a/Source/CTest/cmCTestLaunch.cxx +++ b/Source/CTest/cmCTestLaunch.cxx @@ -306,7 +306,8 @@ void cmCTestLaunch::LoadLabels() if (line.empty() || line[0] == '#') { // Ignore blank and comment lines. continue; - } else if (line[0] == ' ') { + } + if (line[0] == ' ') { // Label lines appear indented by one space. if (inTarget || inSource) { this->Labels.insert(line.c_str() + 1); diff --git a/Source/CTest/cmCTestMultiProcessHandler.cxx b/Source/CTest/cmCTestMultiProcessHandler.cxx index d738a1b..fdc16b1 100644 --- a/Source/CTest/cmCTestMultiProcessHandler.cxx +++ b/Source/CTest/cmCTestMultiProcessHandler.cxx @@ -561,7 +561,7 @@ void cmCTestMultiProcessHandler::CreateParallelTestCostList() // In parallel test runs repeatedly move dependencies of the tests on // the current dependency level to the next level until no // further dependencies exist. - while (priorityStack.back().size()) { + while (!priorityStack.back().empty()) { TestSet& previousSet = priorityStack.back(); priorityStack.push_back(TestSet()); TestSet& currentSet = priorityStack.back(); diff --git a/Source/CTest/cmParseDelphiCoverage.cxx b/Source/CTest/cmParseDelphiCoverage.cxx index 9ae48d8..9cdd50b 100644 --- a/Source/CTest/cmParseDelphiCoverage.cxx +++ b/Source/CTest/cmParseDelphiCoverage.cxx @@ -46,7 +46,8 @@ public: beginSet.push_back("begin"); coverageVector.push_back(-1); continue; - } else if (line.find('{') != line.npos) { + } + if (line.find('{') != line.npos) { blockComFlag = true; } else if (line.find('}') != line.npos) { blockComFlag = false; diff --git a/Source/CursesDialog/cmCursesLongMessageForm.cxx b/Source/CursesDialog/cmCursesLongMessageForm.cxx index d299547..7fb065d 100644 --- a/Source/CursesDialog/cmCursesLongMessageForm.cxx +++ b/Source/CursesDialog/cmCursesLongMessageForm.cxx @@ -159,7 +159,8 @@ void cmCursesLongMessageForm::HandleInput() // quit if (key == 'o' || key == 'e') { break; - } else if (key == KEY_DOWN || key == ctrl('n')) { + } + if (key == KEY_DOWN || key == ctrl('n')) { form_driver(this->Form, REQ_SCR_FLINE); } else if (key == KEY_UP || key == ctrl('p')) { form_driver(this->Form, REQ_SCR_BLINE); diff --git a/Source/CursesDialog/cmCursesMainForm.cxx b/Source/CursesDialog/cmCursesMainForm.cxx index 939c736..ca824c0 100644 --- a/Source/CursesDialog/cmCursesMainForm.cxx +++ b/Source/CursesDialog/cmCursesMainForm.cxx @@ -764,9 +764,8 @@ void cmCursesMainForm::HandleInput() // quit if (key == 'q') { break; - } else { - continue; } + continue; } currentField = current_field(this->Form); @@ -826,7 +825,7 @@ void cmCursesMainForm::HandleInput() // (index always corresponds to the value field) // scroll down with arrow down, ctrl+n (emacs binding), or j (vim // binding) - else if (key == KEY_DOWN || key == ctrl('n') || key == 'j') { + if (key == KEY_DOWN || key == ctrl('n') || key == 'j') { FIELD* cur = current_field(this->Form); size_t findex = field_index(cur); if (findex == 3 * this->NumberOfVisibleEntries - 1) { diff --git a/Source/cmAddSubDirectoryCommand.cxx b/Source/cmAddSubDirectoryCommand.cxx index 34ec0e3..dbd4dd1 100644 --- a/Source/cmAddSubDirectoryCommand.cxx +++ b/Source/cmAddSubDirectoryCommand.cxx @@ -32,7 +32,8 @@ bool cmAddSubDirectoryCommand::InitialPass( if (*i == "EXCLUDE_FROM_ALL") { excludeFromAll = true; continue; - } else if (binArg.empty()) { + } + if (binArg.empty()) { binArg = *i; } else { this->SetError("called with incorrect number of arguments"); diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx index 034a266..fa166a0 100644 --- a/Source/cmFileCommand.cxx +++ b/Source/cmFileCommand.cxx @@ -613,8 +613,8 @@ bool cmFileCommand::HandleStringsCommand(std::vector<std::string> const& args) continue; } - else if ((c >= 0x20 && c < 0x7F) || c == '\t' || - (c == '\n' && newline_consume)) { + if ((c >= 0x20 && c < 0x7F) || c == '\t' || + (c == '\n' && newline_consume)) { // This is an ASCII character that may be part of a string. // Cast added to avoid compiler warning. Cast is ok because // c is guaranteed to fit in char by the above if... diff --git a/Source/cmFileMonitor.cxx b/Source/cmFileMonitor.cxx index 9e66035..ed794c3 100644 --- a/Source/cmFileMonitor.cxx +++ b/Source/cmFileMonitor.cxx @@ -236,7 +236,7 @@ public: cmFileMonitor::Callback cb) : Parent(p) , PathSegment(ps) - , CbList({ cb }) + , CbList({ std::move(cb) }) { assert(p); assert(!ps.empty()); diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index 10a7706..a0f677b 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -3973,15 +3973,14 @@ PropertyType checkInterfacePropertyCompatibility(cmGeneratorTarget const* tgt, << theTarget->GetName() << "\".\n"; cmSystemTools::Error(e.str().c_str()); break; - } else { - propContent = consistent.second; - continue; } - } else { - // Explicitly set on target and not set in iface. Can't disagree. + propContent = consistent.second; continue; } - } else if (impliedByUse) { + // Explicitly set on target and not set in iface. Can't disagree. + continue; + } + if (impliedByUse) { propContent = impliedValue<PropertyType>(propContent); if (ifaceIsSet) { @@ -3999,43 +3998,36 @@ PropertyType checkInterfacePropertyCompatibility(cmGeneratorTarget const* tgt, << "\" is in conflict.\n"; cmSystemTools::Error(e.str().c_str()); break; - } else { - propContent = consistent.second; - continue; } - } else { - // Implicitly set on target and not set in iface. Can't disagree. + propContent = consistent.second; continue; } - } else { - if (ifaceIsSet) { - if (propInitialized) { - std::pair<bool, PropertyType> consistent = - consistentProperty(propContent, ifacePropContent, t); - report += reportEntry; - report += compatibilityAgree(t, propContent != consistent.second); - if (!consistent.first) { - std::ostringstream e; - e << "The INTERFACE_" << p << " property of \"" - << theTarget->GetName() << "\" does\nnot agree with the value " - "of " - << p << " already determined\nfor \"" << tgt->GetName() - << "\".\n"; - cmSystemTools::Error(e.str().c_str()); - break; - } else { - propContent = consistent.second; - continue; - } - } else { - report += reportEntry + "(Interface set)\n"; - propContent = ifacePropContent; - propInitialized = true; + // Implicitly set on target and not set in iface. Can't disagree. + continue; + } + if (ifaceIsSet) { + if (propInitialized) { + std::pair<bool, PropertyType> consistent = + consistentProperty(propContent, ifacePropContent, t); + report += reportEntry; + report += compatibilityAgree(t, propContent != consistent.second); + if (!consistent.first) { + std::ostringstream e; + e << "The INTERFACE_" << p << " property of \"" + << theTarget->GetName() << "\" does\nnot agree with the value of " + << p << " already determined\nfor \"" << tgt->GetName() << "\".\n"; + cmSystemTools::Error(e.str().c_str()); + break; } - } else { - // Not set. Nothing to agree on. + propContent = consistent.second; continue; } + report += reportEntry + "(Interface set)\n"; + propContent = ifacePropContent; + propInitialized = true; + } else { + // Not set. Nothing to agree on. + continue; } } diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index e61cbd9..1a77d7c 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -966,8 +966,14 @@ void cmGlobalNinjaGenerator::WriteAssumedSourceDependencies() } } +std::string OrderDependsTargetForTarget(cmGeneratorTarget const* target) +{ + return "cmake_object_order_depends_target_" + target->GetName(); +} + void cmGlobalNinjaGenerator::AppendTargetOutputs( - cmGeneratorTarget const* target, cmNinjaDeps& outputs) + cmGeneratorTarget const* target, cmNinjaDeps& outputs, + cmNinjaTargetDepends depends) { std::string configName = target->Target->GetMakefile()->GetSafeDefinition("CMAKE_BUILD_TYPE"); @@ -979,15 +985,27 @@ void cmGlobalNinjaGenerator::AppendTargetOutputs( bool realname = target->IsFrameworkOnApple(); switch (target->GetType()) { - case cmStateEnums::EXECUTABLE: case cmStateEnums::SHARED_LIBRARY: case cmStateEnums::STATIC_LIBRARY: case cmStateEnums::MODULE_LIBRARY: { + if (depends == DependOnTargetOrdering) { + outputs.push_back(OrderDependsTargetForTarget(target)); + break; + } + } + // FALLTHROUGH + case cmStateEnums::EXECUTABLE: { outputs.push_back(this->ConvertToNinjaPath(target->GetFullPath( configName, cmStateEnums::RuntimeBinaryArtifact, realname))); break; } - case cmStateEnums::OBJECT_LIBRARY: + case cmStateEnums::OBJECT_LIBRARY: { + if (depends == DependOnTargetOrdering) { + outputs.push_back(OrderDependsTargetForTarget(target)); + break; + } + } + // FALLTHROUGH case cmStateEnums::GLOBAL_TARGET: case cmStateEnums::UTILITY: { std::string path = @@ -1003,7 +1021,8 @@ void cmGlobalNinjaGenerator::AppendTargetOutputs( } void cmGlobalNinjaGenerator::AppendTargetDepends( - cmGeneratorTarget const* target, cmNinjaDeps& outputs) + cmGeneratorTarget const* target, cmNinjaDeps& outputs, + cmNinjaTargetDepends depends) { if (target->GetType() == cmStateEnums::GLOBAL_TARGET) { // These depend only on other CMake-provided targets, e.g. "all". @@ -1023,7 +1042,7 @@ void cmGlobalNinjaGenerator::AppendTargetDepends( if ((*i)->GetType() == cmStateEnums::INTERFACE_LIBRARY) { continue; } - this->AppendTargetOutputs(*i, outs); + this->AppendTargetOutputs(*i, outs, depends); } std::sort(outs.begin(), outs.end()); outputs.insert(outputs.end(), outs.begin(), outs.end()); diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index e06afb0..b1d6155 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -316,10 +316,12 @@ public: ASD.insert(deps.begin(), deps.end()); } - void AppendTargetOutputs(cmGeneratorTarget const* target, - cmNinjaDeps& outputs); - void AppendTargetDepends(cmGeneratorTarget const* target, - cmNinjaDeps& outputs); + void AppendTargetOutputs( + cmGeneratorTarget const* target, cmNinjaDeps& outputs, + cmNinjaTargetDepends depends = DependOnTargetArtifact); + void AppendTargetDepends( + cmGeneratorTarget const* target, cmNinjaDeps& outputs, + cmNinjaTargetDepends depends = DependOnTargetArtifact); void AppendTargetDependsClosure(cmGeneratorTarget const* target, cmNinjaDeps& outputs); void AddDependencyToAll(cmGeneratorTarget* target); diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index 16297f3..929b1cd 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -16,7 +16,7 @@ struct cmListFileParser { - cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, + cmListFileParser(cmListFile* lf, cmListFileBacktrace const& lfbt, cmMessenger* messenger, const char* filename); ~cmListFileParser(); void IssueFileOpenError(std::string const& text) const; @@ -39,7 +39,8 @@ struct cmListFileParser } Separation; }; -cmListFileParser::cmListFileParser(cmListFile* lf, cmListFileBacktrace lfbt, +cmListFileParser::cmListFileParser(cmListFile* lf, + cmListFileBacktrace const& lfbt, cmMessenger* messenger, const char* filename) : ListFile(lf) diff --git a/Source/cmLocalNinjaGenerator.cxx b/Source/cmLocalNinjaGenerator.cxx index 00f5e4b..e0e3e54 100644 --- a/Source/cmLocalNinjaGenerator.cxx +++ b/Source/cmLocalNinjaGenerator.cxx @@ -281,9 +281,11 @@ void cmLocalNinjaGenerator::AppendTargetOutputs(cmGeneratorTarget* target, } void cmLocalNinjaGenerator::AppendTargetDepends(cmGeneratorTarget* target, - cmNinjaDeps& outputs) + cmNinjaDeps& outputs, + cmNinjaTargetDepends depends) { - this->GetGlobalNinjaGenerator()->AppendTargetDepends(target, outputs); + this->GetGlobalNinjaGenerator()->AppendTargetDepends(target, outputs, + depends); } void cmLocalNinjaGenerator::AppendCustomCommandDeps( diff --git a/Source/cmLocalNinjaGenerator.h b/Source/cmLocalNinjaGenerator.h index fda4578..a45e018 100644 --- a/Source/cmLocalNinjaGenerator.h +++ b/Source/cmLocalNinjaGenerator.h @@ -63,7 +63,9 @@ public: std::string BuildCommandLine(const std::vector<std::string>& cmdLines); void AppendTargetOutputs(cmGeneratorTarget* target, cmNinjaDeps& outputs); - void AppendTargetDepends(cmGeneratorTarget* target, cmNinjaDeps& outputs); + void AppendTargetDepends( + cmGeneratorTarget* target, cmNinjaDeps& outputs, + cmNinjaTargetDepends depends = DependOnTargetArtifact); void AddCustomCommandTarget(cmCustomCommand const* cc, cmGeneratorTarget* target); diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 7c417a4..e0b2217 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -117,7 +117,7 @@ bool cmNinjaTargetGenerator::NeedDyndep(std::string const& lang) const std::string cmNinjaTargetGenerator::OrderDependsTargetForTarget() { - return "cmake_order_depends_target_" + this->GetTargetName(); + return "cmake_object_order_depends_target_" + this->GetTargetName(); } // TODO: Most of the code is picked up from @@ -718,8 +718,8 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() } cmNinjaDeps orderOnlyDeps; - this->GetLocalGenerator()->AppendTargetDepends(this->GeneratorTarget, - orderOnlyDeps); + this->GetLocalGenerator()->AppendTargetDepends( + this->GeneratorTarget, orderOnlyDeps, DependOnTargetOrdering); // Add order-only dependencies on other files associated with the target. orderOnlyDeps.insert(orderOnlyDeps.end(), this->ExtraFiles.begin(), @@ -740,7 +740,11 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() std::back_inserter(orderOnlyDeps), MapToNinjaPath()); } - if (!orderOnlyDeps.empty()) { + std::sort(orderOnlyDeps.begin(), orderOnlyDeps.end()); + orderOnlyDeps.erase(std::unique(orderOnlyDeps.begin(), orderOnlyDeps.end()), + orderOnlyDeps.end()); + + { cmNinjaDeps orderOnlyTarget; orderOnlyTarget.push_back(this->OrderDependsTargetForTarget()); this->GetGlobalGenerator()->WritePhonyBuild( @@ -753,7 +757,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() for (std::vector<cmSourceFile const*>::const_iterator si = objectSources.begin(); si != objectSources.end(); ++si) { - this->WriteObjectBuildStatement(*si, !orderOnlyDeps.empty()); + this->WriteObjectBuildStatement(*si); } if (!this->DDIFiles.empty()) { @@ -770,6 +774,17 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() ddOutputs.push_back(this->GetDyndepFilePath("Fortran")); + // Make sure dyndep files for all our dependencies have already + // been generated so that the 'FortranModules.json' files they + // produced as side-effects are available for us to read. + // Ideally we should depend on the 'FortranModules.json' files + // from our dependencies directly, but we don't know which of + // our dependencies produces them. Fixing this will require + // refactoring the Ninja generator to generate targets in + // dependency order so that we can collect the needed information. + this->GetLocalGenerator()->AppendTargetDepends( + this->GeneratorTarget, ddOrderOnlyDeps, DependOnTargetArtifact); + this->GetGlobalGenerator()->WriteBuild( this->GetBuildFileStream(), ddComment, ddRule, ddOutputs, ddImplicitOuts, ddExplicitDeps, ddImplicitDeps, ddOrderOnlyDeps, ddVars); @@ -779,7 +794,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatements() } void cmNinjaTargetGenerator::WriteObjectBuildStatement( - cmSourceFile const* source, bool writeOrderDependsTargetForTarget) + cmSourceFile const* source) { std::string const language = source->GetLanguage(); std::string const sourceFileName = @@ -830,9 +845,7 @@ void cmNinjaTargetGenerator::WriteObjectBuildStatement( } cmNinjaDeps orderOnlyDeps; - if (writeOrderDependsTargetForTarget) { - orderOnlyDeps.push_back(this->OrderDependsTargetForTarget()); - } + orderOnlyDeps.push_back(this->OrderDependsTargetForTarget()); // If the source file is GENERATED and does not have a custom command // (either attached to this source file or another one), assume that one of diff --git a/Source/cmNinjaTargetGenerator.h b/Source/cmNinjaTargetGenerator.h index 9ce8651..5eb7a9a 100644 --- a/Source/cmNinjaTargetGenerator.h +++ b/Source/cmNinjaTargetGenerator.h @@ -119,8 +119,7 @@ protected: void WriteLanguageRules(const std::string& language); void WriteCompileRule(const std::string& language); void WriteObjectBuildStatements(); - void WriteObjectBuildStatement(cmSourceFile const* source, - bool writeOrderDependsTargetForTarget); + void WriteObjectBuildStatement(cmSourceFile const* source); void WriteTargetDependInfo(std::string const& lang); void ExportObjectCompileCommand( diff --git a/Source/cmNinjaTypes.h b/Source/cmNinjaTypes.h index b4af70e..ec435d9 100644 --- a/Source/cmNinjaTypes.h +++ b/Source/cmNinjaTypes.h @@ -9,6 +9,12 @@ #include <string> #include <vector> +enum cmNinjaTargetDepends +{ + DependOnTargetArtifact, + DependOnTargetOrdering +}; + typedef std::vector<std::string> cmNinjaDeps; typedef std::map<std::string, std::string> cmNinjaVars; diff --git a/Source/cmOutputRequiredFilesCommand.cxx b/Source/cmOutputRequiredFilesCommand.cxx index 8b8cf07..2339d68 100644 --- a/Source/cmOutputRequiredFilesCommand.cxx +++ b/Source/cmOutputRequiredFilesCommand.cxx @@ -188,9 +188,8 @@ protected: if (qstart == std::string::npos) { cmSystemTools::Error("unknown include directive ", line.c_str()); continue; - } else { - qend = line.find('>', qstart + 1); } + qend = line.find('>', qstart + 1); } else { qend = line.find('\"', qstart + 1); } diff --git a/Source/cmRST.cxx b/Source/cmRST.cxx index 70ffc7d..938cad9 100644 --- a/Source/cmRST.cxx +++ b/Source/cmRST.cxx @@ -96,14 +96,14 @@ void cmRST::ProcessModule(std::istream& is) if (line == "#") { this->ProcessLine(""); continue; - } else if (line.substr(0, 2) == "# ") { + } + if (line.substr(0, 2) == "# ") { this->ProcessLine(line.substr(2, line.npos)); continue; - } else { - rst = ""; - this->Reset(); - this->OutputLinePending = true; } + rst = ""; + this->Reset(); + this->OutputLinePending = true; } if (line == "#.rst:") { rst = "#"; diff --git a/Source/cmSetTargetPropertiesCommand.cxx b/Source/cmSetTargetPropertiesCommand.cxx index da26972..6425913 100644 --- a/Source/cmSetTargetPropertiesCommand.cxx +++ b/Source/cmSetTargetPropertiesCommand.cxx @@ -32,9 +32,8 @@ bool cmSetTargetPropertiesCommand::InitialPass( } propertyPairs.insert(propertyPairs.end(), j, args.end()); break; - } else { - numFiles++; } + numFiles++; } if (propertyPairs.empty()) { this->SetError("called with illegal arguments, maybe missing " diff --git a/Source/cmSetTestsPropertiesCommand.cxx b/Source/cmSetTestsPropertiesCommand.cxx index 4fd379f..e27c675 100644 --- a/Source/cmSetTestsPropertiesCommand.cxx +++ b/Source/cmSetTestsPropertiesCommand.cxx @@ -32,9 +32,8 @@ bool cmSetTestsPropertiesCommand::InitialPass( } propertyPairs.insert(propertyPairs.end(), j, args.end()); break; - } else { - numFiles++; } + numFiles++; } if (propertyPairs.empty()) { this->SetError("called with illegal arguments, maybe " diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx index 39625ae..6d620d9 100644 --- a/Source/cmSystemTools.cxx +++ b/Source/cmSystemTools.cxx @@ -1707,7 +1707,8 @@ int cmSystemTools::WaitForLine(cmsysProcess* process, std::string& line, for (; outiter != out.end(); ++outiter) { if ((*outiter == '\r') && ((outiter + 1) == out.end())) { break; - } else if (*outiter == '\n' || *outiter == '\0') { + } + if (*outiter == '\n' || *outiter == '\0') { std::vector<char>::size_type length = outiter - out.begin(); if (length > 1 && *(outiter - 1) == '\r') { --length; @@ -1724,7 +1725,8 @@ int cmSystemTools::WaitForLine(cmsysProcess* process, std::string& line, for (; erriter != err.end(); ++erriter) { if ((*erriter == '\r') && ((erriter + 1) == err.end())) { break; - } else if (*erriter == '\n' || *erriter == '\0') { + } + if (*erriter == '\n' || *erriter == '\0') { std::vector<char>::size_type length = erriter - err.begin(); if (length > 1 && *(erriter - 1) == '\r') { --length; @@ -2623,29 +2625,28 @@ bool cmSystemTools::RemoveRPath(std::string const& file, std::string* emsg, it = dentries.erase(it); entriesErased++; continue; - } else { - if (cmELF::TagMipsRldMapRel != 0 && - it->first == cmELF::TagMipsRldMapRel) { - // Background: debuggers need to know the "linker map" which contains - // the addresses each dynamic object is loaded at. Most arches use - // the DT_DEBUG tag which the dynamic linker writes to (directly) and - // contain the location of the linker map, however on MIPS the - // .dynamic section is always read-only so this is not possible. MIPS - // objects instead contain a DT_MIPS_RLD_MAP tag which contains the - // address where the dyanmic linker will write to (an indirect - // version of DT_DEBUG). Since this doesn't work when using PIE, a - // relative equivalent was created - DT_MIPS_RLD_MAP_REL. Since this - // version contains a relative offset, moving it changes the - // calculated address. This may cause the dyanmic linker to write - // into memory it should not be changing. - // - // To fix this, we adjust the value of DT_MIPS_RLD_MAP_REL here. If - // we move it up by n bytes, we add n bytes to the value of this tag. - it->second += entriesErased * sizeof_dentry; - } - - it++; } + if (cmELF::TagMipsRldMapRel != 0 && + it->first == cmELF::TagMipsRldMapRel) { + // Background: debuggers need to know the "linker map" which contains + // the addresses each dynamic object is loaded at. Most arches use + // the DT_DEBUG tag which the dynamic linker writes to (directly) and + // contain the location of the linker map, however on MIPS the + // .dynamic section is always read-only so this is not possible. MIPS + // objects instead contain a DT_MIPS_RLD_MAP tag which contains the + // address where the dyanmic linker will write to (an indirect + // version of DT_DEBUG). Since this doesn't work when using PIE, a + // relative equivalent was created - DT_MIPS_RLD_MAP_REL. Since this + // version contains a relative offset, moving it changes the + // calculated address. This may cause the dyanmic linker to write + // into memory it should not be changing. + // + // To fix this, we adjust the value of DT_MIPS_RLD_MAP_REL here. If + // we move it up by n bytes, we add n bytes to the value of this tag. + it->second += entriesErased * sizeof_dentry; + } + + it++; } // Encode new entries list diff --git a/Source/cmcmd.cxx b/Source/cmcmd.cxx index 974dd5f..cc954e6 100644 --- a/Source/cmcmd.cxx +++ b/Source/cmcmd.cxx @@ -1450,7 +1450,8 @@ bool cmVSLink::Parse(std::vector<std::string>::const_iterator argBeg, if (*arg == "--") { ++arg; break; - } else if (*arg == "--manifests") { + } + if (*arg == "--manifests") { for (++arg; arg != argEnd && !cmHasLiteralPrefix(*arg, "-"); ++arg) { this->UserManifests.push_back(*arg); } diff --git a/Tests/CustomCommandWorkingDirectory/CMakeLists.txt b/Tests/CustomCommandWorkingDirectory/CMakeLists.txt index f917cd7..4975feb 100644 --- a/Tests/CustomCommandWorkingDirectory/CMakeLists.txt +++ b/Tests/CustomCommandWorkingDirectory/CMakeLists.txt @@ -19,6 +19,7 @@ add_executable(working "${TestWorkingDir_BINARY_DIR}/working.c" add_custom_target( Custom ALL COMMAND "${CMAKE_COMMAND}" -E copy_if_different ./customTarget.c "${TestWorkingDir_BINARY_DIR}/customTarget.c" + BYPRODUCTS "${TestWorkingDir_BINARY_DIR}/customTarget.c" WORKING_DIRECTORY "${TestWorkingDir_SOURCE_DIR}" ) @@ -36,6 +37,7 @@ add_executable(working2 working2.c ${TestWorkingDir_BINARY_DIR}/customTarget2.c) add_custom_target( Custom2 ALL COMMAND "${CMAKE_COMMAND}" -E copy_if_different ${TestWorkingDir_SOURCE_DIR}/customTarget.c ../customTarget2.c + BYPRODUCTS "${TestWorkingDir_BINARY_DIR}/customTarget2.c" WORKING_DIRECTORY work/ # Relative to build tree, trailing slash ) diff --git a/Tests/RunCMake/CMakeLists.txt b/Tests/RunCMake/CMakeLists.txt index 32c4be8..21e6259 100644 --- a/Tests/RunCMake/CMakeLists.txt +++ b/Tests/RunCMake/CMakeLists.txt @@ -119,6 +119,10 @@ if(CMAKE_GENERATOR MATCHES "Make") add_RunCMake_test(Make) endif() if(CMAKE_GENERATOR STREQUAL "Ninja") + set(Ninja_ARGS + -DCMAKE_C_OUTPUT_EXTENSION=${CMAKE_C_OUTPUT_EXTENSION} + -DCMAKE_SHARED_LIBRARY_PREFIX=${CMAKE_SHARED_LIBRARY_PREFIX} + -DCMAKE_SHARED_LIBRARY_SUFFIX=${CMAKE_SHARED_LIBRARY_SUFFIX}) add_RunCMake_test(Ninja) endif() add_RunCMake_test(CTest) diff --git a/Tests/RunCMake/Ninja/AssumedSources.cmake b/Tests/RunCMake/Ninja/AssumedSources.cmake new file mode 100644 index 0000000..5fb0219 --- /dev/null +++ b/Tests/RunCMake/Ninja/AssumedSources.cmake @@ -0,0 +1,20 @@ +cmake_minimum_required(VERSION 3.8) +project(AssumedSources) + +set_source_files_properties( + "${CMAKE_CURRENT_BINARY_DIR}/target.c" + "${CMAKE_CURRENT_BINARY_DIR}/target-no-depends.c" + PROPERTIES GENERATED 1) + +add_executable(working + "${CMAKE_CURRENT_BINARY_DIR}/target.c" + "${CMAKE_CURRENT_BINARY_DIR}/target-no-depends.c") + +add_custom_target( + gen-target.c ALL + COMMAND "${CMAKE_COMMAND}" -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/dep.c" "${CMAKE_CURRENT_BINARY_DIR}/target.c") +add_custom_target( + gen-target-no-depends.c ALL + COMMAND "${CMAKE_COMMAND}" -E copy_if_different "${CMAKE_CURRENT_SOURCE_DIR}/dep.c" "${CMAKE_CURRENT_BINARY_DIR}/target-no-depends.c") + +add_dependencies(working gen-target.c) diff --git a/Tests/RunCMake/Ninja/LooseObjectDepends.cmake b/Tests/RunCMake/Ninja/LooseObjectDepends.cmake new file mode 100644 index 0000000..360c7ba --- /dev/null +++ b/Tests/RunCMake/Ninja/LooseObjectDepends.cmake @@ -0,0 +1,26 @@ +cmake_minimum_required(VERSION 3.8) +project(LooseObjectDepends C) + +add_custom_command( + OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/command.h" + COMMAND "${CMAKE_COMMAND}" -E touch + "${CMAKE_CURRENT_BINARY_DIR}/command.h" + COMMENT "Creating command.h") +add_custom_target(create-command.h + DEPENDS + "${CMAKE_CURRENT_BINARY_DIR}/command.h") + +add_custom_target(create-target.h + BYPRODUCTS "${CMAKE_CURRENT_BINARY_DIR}/target.h" + COMMAND "${CMAKE_COMMAND}" -E touch + "${CMAKE_CURRENT_BINARY_DIR}/target.h" + COMMENT "Creating target.h") + +add_library(dep SHARED dep.c) +add_dependencies(dep create-command.h create-target.h) +target_include_directories(dep + PUBLIC + "${CMAKE_CURRENT_BINARY_DIR}") + +add_library(top top.c) +target_link_libraries(top PRIVATE dep) diff --git a/Tests/RunCMake/Ninja/RunCMakeTest.cmake b/Tests/RunCMake/Ninja/RunCMakeTest.cmake index 7b4e51e..8c3bc20 100644 --- a/Tests/RunCMake/Ninja/RunCMakeTest.cmake +++ b/Tests/RunCMake/Ninja/RunCMakeTest.cmake @@ -73,7 +73,7 @@ run_SubDir() function(run_ninja dir) execute_process( - COMMAND "${RunCMake_MAKE_PROGRAM}" + COMMAND "${RunCMake_MAKE_PROGRAM}" ${ARGN} WORKING_DIRECTORY "${dir}" OUTPUT_VARIABLE ninja_stdout ERROR_VARIABLE ninja_stderr @@ -95,6 +95,39 @@ ${ninja_stderr} endif() endfunction(run_ninja) +function (run_LooseObjectDepends) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/LooseObjectDepends-build) + run_cmake(LooseObjectDepends) + run_ninja("${RunCMake_TEST_BINARY_DIR}" "CMakeFiles/top.dir/top.c${CMAKE_C_OUTPUT_EXTENSION}") + if (EXISTS "${RunCMake_TEST_BINARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}dep${CMAKE_SHARED_LIBRARY_SUFFIX}") + message(FATAL_ERROR + "The `dep` library was created when requesting an object file to be " + "built; this should no longer be necessary.") + endif () + if (EXISTS "${RunCMake_TEST_BINARY_DIR}/CMakeFiles/dep.dir/dep.c${CMAKE_C_OUTPUT_EXTENSION}") + message(FATAL_ERROR + "The `dep.c` object file was created when requesting an object file to " + "be built; this should no longer be necessary.") + endif () +endfunction () +run_LooseObjectDepends() + +function (run_AssumedSources) + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/AssumedSources-build) + run_cmake(AssumedSources) + run_ninja("${RunCMake_TEST_BINARY_DIR}" "target.c") + if (NOT EXISTS "${RunCMake_TEST_BINARY_DIR}/target.c") + message(FATAL_ERROR + "Dependencies for an assumed source did not hook up properly for 'target.c'.") + endif () + run_ninja("${RunCMake_TEST_BINARY_DIR}" "target-no-depends.c") + if (EXISTS "${RunCMake_TEST_BINARY_DIR}/target-no-depends.c") + message(FATAL_ERROR + "Dependencies for an assumed source were magically hooked up for 'target-no-depends.c'.") + endif () +endfunction () +run_AssumedSources() + function(sleep delay) execute_process( COMMAND ${CMAKE_COMMAND} -E sleep ${delay} diff --git a/Tests/RunCMake/Ninja/dep.c b/Tests/RunCMake/Ninja/dep.c new file mode 100644 index 0000000..728f031 --- /dev/null +++ b/Tests/RunCMake/Ninja/dep.c @@ -0,0 +1,4 @@ +int dep() +{ + return 0; +} diff --git a/Tests/RunCMake/Ninja/top.c b/Tests/RunCMake/Ninja/top.c new file mode 100644 index 0000000..4a88eb2 --- /dev/null +++ b/Tests/RunCMake/Ninja/top.c @@ -0,0 +1,7 @@ +#include "command.h" +#include "target.h" + +int top() +{ + return 0; +} |