From a33cf6d08853ea4c79324bdd36c04f311a23f20a Mon Sep 17 00:00:00 2001 From: Adam Strzelecki Date: Fri, 27 Jun 2014 22:13:50 +0200 Subject: [PATCH 1/3] Ninja: Consider only custom commands deps as side-effects (#14972) Since commit v2.8.12~248^2 (Ninja: Custom Command file depends don't need to exist before building, 2013-06-07) all explicit dependencies inside build folder were considered as possible build command side-effects and phony rules were produced for them in case they don't exist when starting to build. This is unnecessary since regular compile inputs need to exist or cmake will fail. Moreover the exception for sources having GENERATED property that can be missing is already handled by WriteAssumedSourceDependencies. This fixes unwanted phony rules for all regular source files when doing in-source build, causing Ninja not complain when such files gets missing, i.e. during development. Also this reduces number of rules in ninja.build. Now only custom command dependencies are considered as possible side-effects. --- Source/cmGlobalNinjaGenerator.cxx | 22 ++++++++++++---------- Source/cmGlobalNinjaGenerator.h | 9 +++++---- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index 0facfeb66..eb0165482 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -151,11 +151,6 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os, ++i) { arguments += " " + EncodeIdent(EncodePath(*i), os); - - //we need to track every dependency that comes in, since we are trying - //to find dependencies that are side effects of build commands - // - this->CombinedBuildExplicitDependencies.insert( EncodePath(*i) ); } // Write implicit dependencies. @@ -280,6 +275,13 @@ cmGlobalNinjaGenerator::WriteCustomCommandBuild(const std::string& command, cmNinjaDeps(), orderOnly, vars); + + //we need to track every dependency that comes in, since we are trying + //to find dependencies that are side effects of build commands + for(cmNinjaDeps::const_iterator i = deps.begin(); i != deps.end(); ++i) + { + this->CombinedCustomCommandExplicitDependencies.insert( EncodePath(*i) ); + } } void @@ -1015,17 +1017,17 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os) //to keep this data around this->CombinedBuildOutputs.clear(); - //now we difference with CombinedBuildExplicitDependencies to find + //now we difference with CombinedCustomCommandExplicitDependencies to find //the list of items we know nothing about. - //We have encoded all the paths in CombinedBuildExplicitDependencies + //We have encoded all the paths in CombinedCustomCommandExplicitDependencies //and knownDependencies so no matter if unix or windows paths they //should all match now. std::vector unkownExplicitDepends; - this->CombinedBuildExplicitDependencies.erase("all"); + this->CombinedCustomCommandExplicitDependencies.erase("all"); - std::set_difference(this->CombinedBuildExplicitDependencies.begin(), - this->CombinedBuildExplicitDependencies.end(), + std::set_difference(this->CombinedCustomCommandExplicitDependencies.begin(), + this->CombinedCustomCommandExplicitDependencies.end(), knownDependencies.begin(), knownDependencies.end(), std::back_inserter(unkownExplicitDepends)); diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h index f2643af2d..ff110d7e5 100644 --- a/Source/cmGlobalNinjaGenerator.h +++ b/Source/cmGlobalNinjaGenerator.h @@ -363,10 +363,11 @@ private: /// The set of custom command outputs we have seen. std::set CustomCommandOutputs; - //The combined explicit dependencies of all build commands that the global - //generator has issued. When combined with CombinedBuildOutputs it allows - //us to detect the set of explicit dependencies that have - std::set CombinedBuildExplicitDependencies; + /// The combined explicit dependencies of custom build commands + std::set CombinedCustomCommandExplicitDependencies; + + /// When combined with CombinedCustomCommandExplicitDependencies it allows + /// us to detect the set of explicit dependencies that have std::set CombinedBuildOutputs; /// The mapping from source file to assumed dependencies. From 7243c95129fd8cd0d01495d33848663c796f91db Mon Sep 17 00:00:00 2001 From: Adam Strzelecki Date: Fri, 27 Jun 2014 22:13:51 +0200 Subject: [PATCH 2/3] Ninja: Don't limit custom cmd side-effects to build folder (#14972) Actually custom command can write wherever it wants to, such as temporary folder or source folder, possibly violating rules that only build folder should be affected. Therefore we should consider custom command dependency at any path as possible side effect adding phony rule. We avoid adding phony rules for regular source files (since the paraent commit) so we no longer need the in-build-tree test to avoid them. --- Source/cmGlobalNinjaGenerator.cxx | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx index eb0165482..09ee12872 100644 --- a/Source/cmGlobalNinjaGenerator.cxx +++ b/Source/cmGlobalNinjaGenerator.cxx @@ -1033,27 +1033,17 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os) std::back_inserter(unkownExplicitDepends)); - std::string const rootBuildDirectory = - this->GetCMakeInstance()->GetHomeOutputDirectory(); for (std::vector::const_iterator i = unkownExplicitDepends.begin(); i != unkownExplicitDepends.end(); ++i) { - //verify the file is in the build directory - std::string const absDepPath = cmSystemTools::CollapseFullPath( - i->c_str(), rootBuildDirectory.c_str()); - bool const inBuildDir = cmSystemTools::IsSubDirectory(absDepPath.c_str(), - rootBuildDirectory.c_str()); - if(inBuildDir) - { - cmNinjaDeps deps(1,*i); - this->WritePhonyBuild(os, - "", - deps, - deps); - } - } + cmNinjaDeps deps(1,*i); + this->WritePhonyBuild(os, + "", + deps, + deps); + } } void cmGlobalNinjaGenerator::WriteBuiltinTargets(std::ostream& os) From 93371ed592b85ccc179845dbd6e6018ca2193659 Mon Sep 17 00:00:00 2001 From: Adam Strzelecki Date: Fri, 27 Jun 2014 22:13:52 +0200 Subject: [PATCH 3/3] Ninja: Skip generating empty phony rules Ninja generator ensures that all custom commands being target dependencies are run before other source compilations. However in case there are no such dependencies it currently generates empty phony rules which clutter the build graph. Teach the Ninja generator to produce such rules only when necessary. --- Source/cmNinjaTargetGenerator.cxx | 32 ++++++++++++++++++------------- Source/cmNinjaTargetGenerator.h | 3 ++- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/Source/cmNinjaTargetGenerator.cxx b/Source/cmNinjaTargetGenerator.cxx index 24689fb58..816e6d827 100644 --- a/Source/cmNinjaTargetGenerator.cxx +++ b/Source/cmNinjaTargetGenerator.cxx @@ -542,22 +542,24 @@ cmNinjaTargetGenerator std::back_inserter(orderOnlyDeps), MapToNinjaPath()); } - cmNinjaDeps orderOnlyTarget; - orderOnlyTarget.push_back(this->OrderDependsTargetForTarget()); - this->GetGlobalGenerator()->WritePhonyBuild(this->GetBuildFileStream(), - "Order-only phony target for " - + this->GetTargetName(), - orderOnlyTarget, - cmNinjaDeps(), - cmNinjaDeps(), - orderOnlyDeps); - + if (!orderOnlyDeps.empty()) + { + cmNinjaDeps orderOnlyTarget; + orderOnlyTarget.push_back(this->OrderDependsTargetForTarget()); + this->GetGlobalGenerator()->WritePhonyBuild(this->GetBuildFileStream(), + "Order-only phony target for " + + this->GetTargetName(), + orderOnlyTarget, + cmNinjaDeps(), + cmNinjaDeps(), + orderOnlyDeps); + } std::vector objectSources; this->GeneratorTarget->GetObjectSources(objectSources, config); for(std::vector::const_iterator si = objectSources.begin(); si != objectSources.end(); ++si) { - this->WriteObjectBuildStatement(*si); + this->WriteObjectBuildStatement(*si, !orderOnlyDeps.empty()); } std::string def = this->GeneratorTarget->GetModuleDefinitionFile(config); if(!def.empty()) @@ -570,7 +572,8 @@ cmNinjaTargetGenerator void cmNinjaTargetGenerator -::WriteObjectBuildStatement(cmSourceFile const* source) +::WriteObjectBuildStatement( + cmSourceFile const* source, bool writeOrderDependsTargetForTarget) { std::string comment; const std::string language = source->GetLanguage(); @@ -599,7 +602,10 @@ cmNinjaTargetGenerator } cmNinjaDeps orderOnlyDeps; - orderOnlyDeps.push_back(this->OrderDependsTargetForTarget()); + if (writeOrderDependsTargetForTarget) + { + 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 94c420fb8..40a15a3ac 100644 --- a/Source/cmNinjaTargetGenerator.h +++ b/Source/cmNinjaTargetGenerator.h @@ -114,7 +114,8 @@ protected: void WriteLanguageRules(const std::string& language); void WriteCompileRule(const std::string& language); void WriteObjectBuildStatements(); - void WriteObjectBuildStatement(cmSourceFile const* source); + void WriteObjectBuildStatement(cmSourceFile const* source, + bool writeOrderDependsTargetForTarget); void WriteCustomCommandBuildStatement(cmCustomCommand *cc); cmNinjaDeps GetObjects() const