Merge topic 'ninja-require-byproducts'

bd9c7f9b Ninja: Add policy to require explicit custom command byproducts
ed8e30b0 cmGlobalNinjaGenerator: Optimize handling of known build outputs
ad094f43 cmGlobalNinjaGenerator: Fix spelling of "unknown"
82a37d3c cmGlobalNinjaGenerator: Drop unused member

4 files changed, 77 insertions, 22 deletions

diff --git a/Source/cmGlobalNinjaGenerator.cxx b/Source/cmGlobalNinjaGenerator.cxx
index ac7a6eb..f74f1e0 100644
--- a/Source/cmGlobalNinjaGenerator.cxx
+++ b/Source/cmGlobalNinjaGenerator.cxx
@@ -17,6 +17,7 @@
 #include "cmLocalNinjaGenerator.h"
 #include "cmMakefile.h"
 #include "cmVersion.h"
+#include "cmAlgorithms.h"
 
 #include <algorithm>
 #include <assert.h>
@@ -183,7 +184,10 @@ void cmGlobalNinjaGenerator::WriteBuild(std::ostream& os,
       i != outputs.end(); ++i)
     {
     build += " " + EncodeIdent(EncodePath(*i), os);
-    this->CombinedBuildOutputs.insert( EncodePath(*i) );
+    if (this->ComputingUnknownDependencies)
+      {
+      this->CombinedBuildOutputs.insert( EncodePath(*i) );
+      }
     }
   build += ":";
 
@@ -281,11 +285,14 @@ cmGlobalNinjaGenerator::WriteCustomCommandBuild(const std::string& command,
                    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)
+  if (this->ComputingUnknownDependencies)
     {
-    this->CombinedCustomCommandExplicitDependencies.insert( EncodePath(*i) );
+    //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));
+      }
     }
 }
 
@@ -477,6 +484,8 @@ cmGlobalNinjaGenerator::cmGlobalNinjaGenerator()
   , CompileCommandsStream(0)
   , Rules()
   , AllDependencies()
+  , ComputingUnknownDependencies(false)
+  , PolicyCMP0058(cmPolicies::WARN)
 {
   // // Ninja is not ported to non-Unix OS yet.
   // this->ForceUnixPaths = true;
@@ -510,6 +519,13 @@ void cmGlobalNinjaGenerator::Generate()
   this->OpenBuildFileStream();
   this->OpenRulesFileStream();
 
+  this->PolicyCMP0058 =
+    this->LocalGenerators[0]->GetMakefile()
+    ->GetPolicyStatus(cmPolicies::CMP0058);
+  this->ComputingUnknownDependencies =
+    (this->PolicyCMP0058 == cmPolicies::OLD ||
+     this->PolicyCMP0058 == cmPolicies::WARN);
+
   this->cmGlobalGenerator::Generate();
 
   this->WriteAssumedSourceDependencies();
@@ -955,6 +971,18 @@ void cmGlobalNinjaGenerator::WriteTargetAliases(std::ostream& os)
 
 void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
 {
+  if (!this->ComputingUnknownDependencies)
+    {
+    return;
+    }
+
+  // We need to collect the set of known build outputs.
+  // Start with those generated by WriteBuild calls.
+  // No other method needs this so we can take ownership
+  // of the set locally and throw it out when we are done.
+  std::set<std::string> knownDependencies;
+  knownDependencies.swap(this->CombinedBuildOutputs);
+
   //now write out the unknown explicit dependencies.
 
   //union the configured files, evaluations files and the CombinedBuildOutputs,
@@ -971,7 +999,6 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
   cmLocalNinjaGenerator *ng =
     static_cast<cmLocalNinjaGenerator *>(this->LocalGenerators[0]);
 
-  std::set<std::string> knownDependencies;
   for (std::vector<cmLocalGenerator *>::const_iterator i =
        this->LocalGenerators.begin(); i != this->LocalGenerators.end(); ++i)
     {
@@ -1026,36 +1053,29 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
     knownDependencies.insert( ng->ConvertToNinjaPath(i->first) );
     }
 
-  //insert outputs from all WirteBuild commands
-  //these paths have already be encoded when added to CombinedBuildOutputs
-  knownDependencies.insert(this->CombinedBuildOutputs.begin(),
-                           this->CombinedBuildOutputs.end());
-
-  //after we have combined the data into knownDependencies we have no need
-  //to keep this data around
-  this->CombinedBuildOutputs.clear();
-
   //now we difference with CombinedCustomCommandExplicitDependencies to find
   //the list of items we know nothing about.
   //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<std::string> unkownExplicitDepends;
+  std::vector<std::string> unknownExplicitDepends;
   this->CombinedCustomCommandExplicitDependencies.erase("all");
 
   std::set_difference(this->CombinedCustomCommandExplicitDependencies.begin(),
                       this->CombinedCustomCommandExplicitDependencies.end(),
                       knownDependencies.begin(),
                       knownDependencies.end(),
-                      std::back_inserter(unkownExplicitDepends));
-
+                      std::back_inserter(unknownExplicitDepends));
 
   std::string const rootBuildDirectory =
       this->GetCMakeInstance()->GetHomeOutputDirectory();
+  bool const inSourceBuild =
+    (rootBuildDirectory == this->GetCMakeInstance()->GetHomeDirectory());
+  std::vector<std::string> warnExplicitDepends;
   for (std::vector<std::string>::const_iterator
-       i = unkownExplicitDepends.begin();
-       i != unkownExplicitDepends.end();
+       i = unknownExplicitDepends.begin();
+       i != unknownExplicitDepends.end();
        ++i)
     {
     //verify the file is in the build directory
@@ -1070,8 +1090,34 @@ void cmGlobalNinjaGenerator::WriteUnknownExplicitDependencies(std::ostream& os)
                             "",
                             deps,
                             cmNinjaDeps());
+      if (this->PolicyCMP0058 == cmPolicies::WARN &&
+          !inSourceBuild && warnExplicitDepends.size() < 10)
+        {
+        warnExplicitDepends.push_back(*i);
+        }
       }
    }
+
+  if (!warnExplicitDepends.empty())
+    {
+    std::ostringstream w;
+    w <<
+      (this->GetCMakeInstance()->GetPolicies()->
+       GetPolicyWarning(cmPolicies::CMP0058)) << "\n"
+      "This project specifies custom command DEPENDS on files "
+      "in the build tree that are not specified as the OUTPUT or "
+      "BYPRODUCTS of any add_custom_command or add_custom_target:\n"
+      " " << cmJoin(warnExplicitDepends, "\n ") <<
+      "\n"
+      "For compatibility with versions of CMake that did not have "
+      "the BYPRODUCTS option, CMake is generating phony rules for "
+      "such files to convince 'ninja' to build."
+      "\n"
+      "Project authors should add the missing BYPRODUCTS or OUTPUT "
+      "options to the custom commands that produce these files."
+      ;
+    this->GetCMakeInstance()->IssueMessage(cmake::AUTHOR_WARNING, w.str());
+    }
 }
 
 void cmGlobalNinjaGenerator::WriteBuiltinTargets(std::ostream& os)
diff --git a/Source/cmGlobalNinjaGenerator.h b/Source/cmGlobalNinjaGenerator.h
index c7bb782..6aa76f9 100644
--- a/Source/cmGlobalNinjaGenerator.h
+++ b/Source/cmGlobalNinjaGenerator.h
@@ -368,6 +368,11 @@ private:
   /// The set of custom command outputs we have seen.
   std::set<std::string> CustomCommandOutputs;
 
+  /// Whether we are collecting known build outputs and needed
+  /// dependencies to determine unknown dependencies.
+  bool ComputingUnknownDependencies;
+  cmPolicies::PolicyStatus PolicyCMP0058;
+
   /// The combined explicit dependencies of custom build commands
   std::set<std::string> CombinedCustomCommandExplicitDependencies;
 
@@ -381,8 +386,6 @@ private:
   typedef std::map<std::string, cmTarget*> TargetAliasMap;
   TargetAliasMap TargetAliases;
 
-  static cmLocalGenerator* LocalGenerator;
-
   static bool UsingMinGW;
 
 };
diff --git a/Source/cmPolicies.cxx b/Source/cmPolicies.cxx
index 720030b..592df8f 100644
--- a/Source/cmPolicies.cxx
+++ b/Source/cmPolicies.cxx
@@ -380,6 +380,11 @@ cmPolicies::cmPolicies()
     CMP0057, "CMP0057",
     "Disallow multiple MAIN_DEPENDENCY specifications for the same file.",
     3,3,0, cmPolicies::WARN);
+
+  this->DefinePolicy(
+    CMP0058, "CMP0058",
+    "Ninja requires custom command byproducts to be explicit.",
+    3,3,0, cmPolicies::WARN);
 }
 
 cmPolicies::~cmPolicies()
diff --git a/Source/cmPolicies.h b/Source/cmPolicies.h
index 854b132..b18b337 100644
--- a/Source/cmPolicies.h
+++ b/Source/cmPolicies.h
@@ -115,6 +115,7 @@ public:
     CMP0056, ///< Honor link flags in try_compile() source-file signature.
     CMP0057, ///< Disallow multiple MAIN_DEPENDENCY specifications
     /// for the same file.
+    CMP0058, ///< Ninja requires custom command byproducts to be explicit
 
     /** \brief Always the last entry.
      *