Merge topic 'generate-target-order'

aea465793e cmLocalVisualStudio7Generator: Consolidate target iteration
1527242745 cmLocalVisualStudio10Generator: Simplify target ordering by dependencies
48bf7192e7 cmLocalVisualStudio7Generator: Generate targets in dependency order
17aba9c9a6 cmLocalUnixMakefileGenerator3: Generate targets in dependency order
69ee18163b cmLocalGhsMultiGenerator: Generate targets in dependency order
c4e296a609 cmGlobalGenerator: Compute a global target ordering respecting dependencies

Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !5187

12 files changed, 90 insertions, 156 deletions

diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 74360b8..8688b1b 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -1471,6 +1471,7 @@ bool cmGlobalGenerator::Compute()
   if (!this->ComputeTargetDepends()) {
     return false;
   }
+  this->ComputeTargetOrder();
 
   if (this->CheckTargetsForType()) {
     return false;
@@ -1581,6 +1582,50 @@ bool cmGlobalGenerator::ComputeTargetDepends()
   return true;
 }
 
+std::vector<cmGeneratorTarget*>
+cmGlobalGenerator::GetLocalGeneratorTargetsInOrder(cmLocalGenerator* lg) const
+{
+  std::vector<cmGeneratorTarget*> gts;
+  cm::append(gts, lg->GetGeneratorTargets());
+  std::sort(gts.begin(), gts.end(),
+            [this](cmGeneratorTarget const* l, cmGeneratorTarget const* r) {
+              return this->TargetOrderIndex.at(l) <
+                this->TargetOrderIndex.at(r);
+            });
+  return gts;
+}
+
+void cmGlobalGenerator::ComputeTargetOrder()
+{
+  size_t index = 0;
+  auto const& lgens = this->GetLocalGenerators();
+  for (auto const& lgen : lgens) {
+    const auto& targets = lgen->GetGeneratorTargets();
+    for (const auto& gt : targets) {
+      this->ComputeTargetOrder(gt.get(), index);
+    }
+  }
+  assert(index == this->TargetOrderIndex.size());
+}
+
+void cmGlobalGenerator::ComputeTargetOrder(cmGeneratorTarget const* gt,
+                                           size_t& index)
+{
+  std::map<cmGeneratorTarget const*, size_t>::value_type value(gt, 0);
+  auto insertion = this->TargetOrderIndex.insert(value);
+  if (!insertion.second) {
+    return;
+  }
+  auto entry = insertion.first;
+
+  auto& deps = this->GetTargetDirectDepends(gt);
+  for (auto& d : deps) {
+    this->ComputeTargetOrder(d, index);
+  }
+
+  entry->second = index++;
+}
+
 bool cmGlobalGenerator::QtAutoGen()
 {
 #ifndef CMAKE_BOOTSTRAP
@@ -1760,6 +1805,7 @@ void cmGlobalGenerator::ClearGeneratorMembers()
   this->GeneratorTargetSearchIndex.clear();
   this->MakefileSearchIndex.clear();
   this->LocalGeneratorSearchIndex.clear();
+  this->TargetOrderIndex.clear();
   this->ProjectMap.clear();
   this->RuleHashes.clear();
   this->DirectoryContentMap.clear();
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index 6ae7e15..478028e 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -4,6 +4,7 @@
 
 #include "cmConfigure.h" // IWYU pragma: keep
 
+#include <cstddef>
 #include <iosfwd>
 #include <map>
 #include <memory>
@@ -264,6 +265,9 @@ public:
     return this->LocalGenerators;
   }
 
+  std::vector<cmGeneratorTarget*> GetLocalGeneratorTargetsInOrder(
+    cmLocalGenerator* lg) const;
+
   cmMakefile* GetCurrentMakefile() const
   {
     return this->CurrentConfigureMakefile;
@@ -613,6 +617,10 @@ private:
   // Its order is not deterministic.
   LocalGeneratorMap LocalGeneratorSearchIndex;
 
+  void ComputeTargetOrder();
+  void ComputeTargetOrder(cmGeneratorTarget const* gt, size_t& index);
+  std::map<cmGeneratorTarget const*, size_t> TargetOrderIndex;
+
   cmMakefile* TryCompileOuterMakefile;
   // If you add a new map here, make sure it is copied
   // in EnableLanguagesFromGenerator
diff --git a/Source/cmGlobalXCodeGenerator.cxx b/Source/cmGlobalXCodeGenerator.cxx
index dbc95e6..2f27128 100644
--- a/Source/cmGlobalXCodeGenerator.cxx
+++ b/Source/cmGlobalXCodeGenerator.cxx
@@ -424,37 +424,6 @@ void cmGlobalXCodeGenerator::AddExtraIDETargets()
   }
 }
 
-void cmGlobalXCodeGenerator::ComputeTargetOrder()
-{
-  size_t index = 0;
-  auto const& lgens = this->GetLocalGenerators();
-  for (auto const& lgen : lgens) {
-    const auto& targets = lgen->GetGeneratorTargets();
-    for (const auto& gt : targets) {
-      this->ComputeTargetOrder(gt.get(), index);
-    }
-  }
-  assert(index == this->TargetOrderIndex.size());
-}
-
-void cmGlobalXCodeGenerator::ComputeTargetOrder(cmGeneratorTarget const* gt,
-                                                size_t& index)
-{
-  std::map<cmGeneratorTarget const*, size_t>::value_type value(gt, 0);
-  auto insertion = this->TargetOrderIndex.insert(value);
-  if (!insertion.second) {
-    return;
-  }
-  auto entry = insertion.first;
-
-  auto& deps = this->GetTargetDirectDepends(gt);
-  for (auto& d : deps) {
-    this->ComputeTargetOrder(d, index);
-  }
-
-  entry->second = index++;
-}
-
 void cmGlobalXCodeGenerator::Generate()
 {
   this->cmGlobalGenerator::Generate();
@@ -462,8 +431,6 @@ void cmGlobalXCodeGenerator::Generate()
     return;
   }
 
-  this->ComputeTargetOrder();
-
   for (auto keyVal : this->ProjectMap) {
     cmLocalGenerator* root = keyVal.second[0];
 
@@ -1243,12 +1210,8 @@ bool cmGlobalXCodeGenerator::CreateXCodeTargets(
   cmLocalGenerator* gen, std::vector<cmXCodeObject*>& targets)
 {
   this->SetCurrentLocalGenerator(gen);
-  std::vector<cmGeneratorTarget*> gts;
-  cm::append(gts, this->CurrentLocalGenerator->GetGeneratorTargets());
-  std::sort(gts.begin(), gts.end(),
-            [this](cmGeneratorTarget const* l, cmGeneratorTarget const* r) {
-              return this->TargetOrderIndex[l] < this->TargetOrderIndex[r];
-            });
+  std::vector<cmGeneratorTarget*> gts =
+    this->GetLocalGeneratorTargetsInOrder(gen);
   for (auto gtgt : gts) {
     if (!this->CreateXCodeTarget(gtgt, targets)) {
       return false;
diff --git a/Source/cmGlobalXCodeGenerator.h b/Source/cmGlobalXCodeGenerator.h
index c9f9926..c7524e8 100644
--- a/Source/cmGlobalXCodeGenerator.h
+++ b/Source/cmGlobalXCodeGenerator.h
@@ -115,8 +115,6 @@ public:
 
 protected:
   void AddExtraIDETargets() override;
-  void ComputeTargetOrder();
-  void ComputeTargetOrder(cmGeneratorTarget const* gt, size_t& index);
   void Generate() override;
 
 private:
@@ -303,7 +301,6 @@ private:
   std::string ObjectDirArch;
   std::string SystemName;
   std::string GeneratorToolset;
-  std::map<cmGeneratorTarget const*, size_t> TargetOrderIndex;
   std::vector<std::string> EnabledLangs;
   std::map<cmGeneratorTarget const*, std::set<cmSourceFile const*>>
     CommandsVisited;
diff --git a/Source/cmLocalGhsMultiGenerator.cxx b/Source/cmLocalGhsMultiGenerator.cxx
index a23ad57..b223813 100644
--- a/Source/cmLocalGhsMultiGenerator.cxx
+++ b/Source/cmLocalGhsMultiGenerator.cxx
@@ -2,10 +2,8 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmLocalGhsMultiGenerator.h"
 
-#include <algorithm>
 #include <utility>
-
-#include <cmext/algorithm>
+#include <vector>
 
 #include "cmGeneratorTarget.h"
 #include "cmGhsMultiTargetGenerator.h"
@@ -29,34 +27,16 @@ std::string cmLocalGhsMultiGenerator::GetTargetDirectory(
   return dir;
 }
 
-void cmLocalGhsMultiGenerator::GenerateTargetsDepthFirst(
-  cmGeneratorTarget* target, std::vector<cmGeneratorTarget*>& remaining)
-{
-  if (!target->IsInBuildSystem()) {
-    return;
-  }
-  // Find this target in the list of remaining targets.
-  auto it = std::find(remaining.begin(), remaining.end(), target);
-  if (it == remaining.end()) {
-    // This target was already handled.
-    return;
-  }
-  // Remove this target from the list of remaining targets because
-  // we are handling it now.
-  *it = nullptr;
-
-  cmGhsMultiTargetGenerator tg(target);
-  tg.Generate();
-}
-
 void cmLocalGhsMultiGenerator::Generate()
 {
-  std::vector<cmGeneratorTarget*> remaining;
-  cm::append(remaining, this->GetGeneratorTargets());
-  for (auto& t : remaining) {
-    if (t) {
-      this->GenerateTargetsDepthFirst(t, remaining);
+  for (cmGeneratorTarget* gt :
+       this->GlobalGenerator->GetLocalGeneratorTargetsInOrder(this)) {
+    if (!gt->IsInBuildSystem()) {
+      continue;
     }
+
+    cmGhsMultiTargetGenerator tg(gt);
+    tg.Generate();
   }
 }
 
diff --git a/Source/cmLocalGhsMultiGenerator.h b/Source/cmLocalGhsMultiGenerator.h
index 1b6f109..be32a94 100644
--- a/Source/cmLocalGhsMultiGenerator.h
+++ b/Source/cmLocalGhsMultiGenerator.h
@@ -4,7 +4,6 @@
 
 #include <map>
 #include <string>
-#include <vector>
 
 #include "cmLocalGenerator.h"
 
@@ -37,8 +36,4 @@ public:
   void ComputeObjectFilenames(
     std::map<cmSourceFile const*, std::string>& mapping,
     cmGeneratorTarget const* gt = nullptr) override;
-
-private:
-  void GenerateTargetsDepthFirst(cmGeneratorTarget* target,
-                                 std::vector<cmGeneratorTarget*>& remaining);
 };
diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index 9825f1c..c449450 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/Source/cmLocalUnixMakefileGenerator3.cxx
@@ -100,12 +100,13 @@ void cmLocalUnixMakefileGenerator3::Generate()
   // Generate the rule files for each target.
   cmGlobalUnixMakefileGenerator3* gg =
     static_cast<cmGlobalUnixMakefileGenerator3*>(this->GlobalGenerator);
-  for (const auto& target : this->GetGeneratorTargets()) {
-    if (!target->IsInBuildSystem()) {
+  for (cmGeneratorTarget* gt :
+       this->GlobalGenerator->GetLocalGeneratorTargetsInOrder(this)) {
+    if (!gt->IsInBuildSystem()) {
       continue;
     }
     std::unique_ptr<cmMakefileTargetGenerator> tg(
-      cmMakefileTargetGenerator::New(target.get()));
+      cmMakefileTargetGenerator::New(gt));
     if (tg) {
       tg->WriteRuleFiles();
       gg->RecordTargetProgress(tg.get());
diff --git a/Source/cmLocalVisualStudio10Generator.cxx b/Source/cmLocalVisualStudio10Generator.cxx
index 6c7d6c6..98e9db9 100644
--- a/Source/cmLocalVisualStudio10Generator.cxx
+++ b/Source/cmLocalVisualStudio10Generator.cxx
@@ -66,33 +66,18 @@ cmLocalVisualStudio10Generator::~cmLocalVisualStudio10Generator()
 {
 }
 
-void cmLocalVisualStudio10Generator::GenerateTargetsDepthFirst(
-  cmGeneratorTarget* target, std::vector<cmGeneratorTarget*>& remaining)
+void cmLocalVisualStudio10Generator::GenerateTarget(cmGeneratorTarget* target)
 {
-  if (!target->IsInBuildSystem()) {
-    return;
-  }
-  // Find this target in the list of remaining targets.
-  auto it = std::find(remaining.begin(), remaining.end(), target);
-  if (it == remaining.end()) {
-    // This target was already handled.
-    return;
-  }
-  // Remove this target from the list of remaining targets because
-  // we are handling it now.
-  *it = nullptr;
+  auto& targetVisited = this->GetSourcesVisited(target);
   auto& deps = this->GlobalGenerator->GetTargetDirectDepends(target);
   for (auto& d : deps) {
-    // FIXME: Revise CreateSingleVCProj so we do not have to drop `const` here.
-    auto dependee = const_cast<cmGeneratorTarget*>(&*d);
-    GenerateTargetsDepthFirst(dependee, remaining);
     // Take the union of visited source files of custom commands
-    auto visited = GetSourcesVisited(dependee);
-    GetSourcesVisited(target).insert(visited.begin(), visited.end());
+    auto depVisited = this->GetSourcesVisited(d);
+    targetVisited.insert(depVisited.begin(), depVisited.end());
   }
   if (static_cast<cmGlobalVisualStudioGenerator*>(this->GlobalGenerator)
         ->TargetIsFortranOnly(target)) {
-    this->CreateSingleVCProj(target->GetName(), target);
+    this->cmLocalVisualStudio7Generator::GenerateTarget(target);
   } else {
     cmVisualStudio10TargetGenerator tg(
       target,
@@ -102,18 +87,6 @@ void cmLocalVisualStudio10Generator::GenerateTargetsDepthFirst(
   }
 }
 
-void cmLocalVisualStudio10Generator::Generate()
-{
-  std::vector<cmGeneratorTarget*> remaining;
-  cm::append(remaining, this->GetGeneratorTargets());
-  for (auto& t : remaining) {
-    if (t) {
-      this->GenerateTargetsDepthFirst(t, remaining);
-    }
-  }
-  this->WriteStampFiles();
-}
-
 void cmLocalVisualStudio10Generator::ReadAndStoreExternalGUID(
   const std::string& name, const char* path)
 {
diff --git a/Source/cmLocalVisualStudio10Generator.h b/Source/cmLocalVisualStudio10Generator.h
index 84216c8..631132e 100644
--- a/Source/cmLocalVisualStudio10Generator.h
+++ b/Source/cmLocalVisualStudio10Generator.h
@@ -25,14 +25,11 @@ public:
 
   virtual ~cmLocalVisualStudio10Generator();
 
-  /**
-   * Generate the makefile for this directory.
-   */
-  void Generate() override;
   void ReadAndStoreExternalGUID(const std::string& name,
                                 const char* path) override;
 
-  std::set<cmSourceFile const*>& GetSourcesVisited(cmGeneratorTarget* target)
+  std::set<cmSourceFile const*>& GetSourcesVisited(
+    cmGeneratorTarget const* target)
   {
     return SourcesVisited[target];
   };
@@ -42,8 +39,8 @@ protected:
   bool CustomCommandUseLocal() const override { return true; }
 
 private:
-  void GenerateTargetsDepthFirst(cmGeneratorTarget* target,
-                                 std::vector<cmGeneratorTarget*>& remaining);
+  void GenerateTarget(cmGeneratorTarget* target) override;
 
-  std::map<cmGeneratorTarget*, std::set<cmSourceFile const*>> SourcesVisited;
+  std::map<cmGeneratorTarget const*, std::set<cmSourceFile const*>>
+    SourcesVisited;
 };
diff --git a/Source/cmLocalVisualStudio7Generator.cxx b/Source/cmLocalVisualStudio7Generator.cxx
index fb10022..1ebd5da 100644
--- a/Source/cmLocalVisualStudio7Generator.cxx
+++ b/Source/cmLocalVisualStudio7Generator.cxx
@@ -80,7 +80,15 @@ void cmLocalVisualStudio7Generator::AddHelperCommands()
 
 void cmLocalVisualStudio7Generator::Generate()
 {
-  this->WriteProjectFiles();
+  // Create the project file for each target.
+  for (cmGeneratorTarget* gt :
+       this->GlobalGenerator->GetLocalGeneratorTargetsInOrder(this)) {
+    if (!gt->IsInBuildSystem() || gt->GetProperty("EXTERNAL_MSPROJECT")) {
+      continue;
+    }
+    this->GenerateTarget(gt);
+  }
+
   this->WriteStampFiles();
 }
 
@@ -111,35 +119,6 @@ void cmLocalVisualStudio7Generator::FixGlobalTargets()
   }
 }
 
-// TODO
-// for CommandLine= need to repleace quotes with &quot
-// write out configurations
-void cmLocalVisualStudio7Generator::WriteProjectFiles()
-{
-  // If not an in source build, then create the output directory
-  if (this->GetCurrentBinaryDirectory() != this->GetSourceDirectory()) {
-    if (!cmSystemTools::MakeDirectory(this->GetCurrentBinaryDirectory())) {
-      cmSystemTools::Error("Error creating directory " +
-                           this->GetCurrentBinaryDirectory());
-    }
-  }
-
-  // Get the set of targets in this directory.
-  const auto& tgts = this->GetGeneratorTargets();
-
-  // Create the project file for each target.
-  for (const auto& l : tgts) {
-    if (!l->IsInBuildSystem()) {
-      continue;
-    }
-    // INCLUDE_EXTERNAL_MSPROJECT command only affects the workspace
-    // so don't build a projectfile for it
-    if (!l->GetProperty("EXTERNAL_MSPROJECT")) {
-      this->CreateSingleVCProj(l->GetName(), l.get());
-    }
-  }
-}
-
 void cmLocalVisualStudio7Generator::WriteStampFiles()
 {
   // Touch a timestamp file used to determine when the project file is
@@ -178,9 +157,9 @@ void cmLocalVisualStudio7Generator::WriteStampFiles()
   }
 }
 
-void cmLocalVisualStudio7Generator::CreateSingleVCProj(
-  const std::string& lname, cmGeneratorTarget* target)
+void cmLocalVisualStudio7Generator::GenerateTarget(cmGeneratorTarget* target)
 {
+  std::string const& lname = target->GetName();
   cmGlobalVisualStudioGenerator* gg =
     static_cast<cmGlobalVisualStudioGenerator*>(this->GlobalGenerator);
   this->FortranProject = gg->TargetIsFortranOnly(target);
diff --git a/Source/cmLocalVisualStudio7Generator.h b/Source/cmLocalVisualStudio7Generator.h
index d76fc82..9ccd1a1 100644
--- a/Source/cmLocalVisualStudio7Generator.h
+++ b/Source/cmLocalVisualStudio7Generator.h
@@ -84,7 +84,7 @@ public:
                                         const char* path);
 
 protected:
-  void CreateSingleVCProj(const std::string& lname, cmGeneratorTarget* tgt);
+  virtual void GenerateTarget(cmGeneratorTarget* target);
 
 private:
   using Options = cmVS7GeneratorOptions;
@@ -92,7 +92,6 @@ private:
   std::string GetBuildTypeLinkerFlags(std::string rootLinkerFlags,
                                       const std::string& configName);
   void FixGlobalTargets();
-  void WriteProjectFiles();
   void WriteVCProjHeader(std::ostream& fout, const std::string& libName,
                          cmGeneratorTarget* tgt,
                          std::vector<cmSourceGroup>& sgs);
diff --git a/Source/cmVisualStudio10TargetGenerator.cxx b/Source/cmVisualStudio10TargetGenerator.cxx
index 0b2b5d9..ae4a5be 100644
--- a/Source/cmVisualStudio10TargetGenerator.cxx
+++ b/Source/cmVisualStudio10TargetGenerator.cxx
@@ -314,10 +314,6 @@ std::ostream& cmVisualStudio10TargetGenerator::Elem::WriteString(
 
 void cmVisualStudio10TargetGenerator::Generate()
 {
-  // do not generate external ms projects
-  if (this->GeneratorTarget->GetProperty("EXTERNAL_MSPROJECT")) {
-    return;
-  }
   const std::string ProjectFileExtension =
     computeProjectFileExtension(this->GeneratorTarget);
   if (ProjectFileExtension == ".vcxproj") {