Ninja: Factor out per-dir "all" target computation into common generator

This will make it re-usable for the Makefile generator.

Source/cmGlobalCommonGenerator.cxx

@@ -2,6 +2,15 @@
    file Copyright.txt or https://cmake.org/licensing for details.  */
 #include "cmGlobalCommonGenerator.h"
 
+#include "cmGeneratorTarget.h"
+#include "cmLocalGenerator.h"
+#include "cmStateDirectory.h"
+#include "cmStateSnapshot.h"
+#include "cmStateTypes.h"
+#include "cmStringAlgorithms.h"
+
+#include <utility>
+
 class cmake;
 
 cmGlobalCommonGenerator::cmGlobalCommonGenerator(cmake* cm)
@@ -10,3 +19,60 @@ cmGlobalCommonGenerator::cmGlobalCommonGenerator(cmake* cm)
 }
 
 cmGlobalCommonGenerator::~cmGlobalCommonGenerator() = default;
+
+std::map<std::string, cmGlobalCommonGenerator::DirectoryTarget>
+cmGlobalCommonGenerator::ComputeDirectoryTargets() const
+{
+  std::map<std::string, DirectoryTarget> dirTargets;
+  for (cmLocalGenerator* lg : this->LocalGenerators) {
+    std::string const& currentBinaryDir(
+      lg->GetStateSnapshot().GetDirectory().GetCurrentBinary());
+    DirectoryTarget& dirTarget = dirTargets[currentBinaryDir];
+    dirTarget.LG = lg;
+
+    // The directory-level rule should depend on the target-level rules
+    // for all targets in the directory.
+    for (auto gt : lg->GetGeneratorTargets()) {
+      cmStateEnums::TargetType const type = gt->GetType();
+      if (type != cmStateEnums::EXECUTABLE &&
+          type != cmStateEnums::STATIC_LIBRARY &&
+          type != cmStateEnums::SHARED_LIBRARY &&
+          type != cmStateEnums::MODULE_LIBRARY &&
+          type != cmStateEnums::OBJECT_LIBRARY &&
+          type != cmStateEnums::UTILITY) {
+        continue;
+      }
+      DirectoryTarget::Target t;
+      t.GT = gt;
+      if (const char* exclude = gt->GetProperty("EXCLUDE_FROM_ALL")) {
+        if (cmIsOn(exclude)) {
+          // This target has been explicitly excluded.
+          t.ExcludeFromAll = true;
+        } else {
+          // This target has been explicitly un-excluded.  The directory-level
+          // rule for every directory between this and the root should depend
+          // on the target-level rule for this target.
+          for (cmStateSnapshot dir =
+                 lg->GetStateSnapshot().GetBuildsystemDirectoryParent();
+               dir.IsValid(); dir = dir.GetBuildsystemDirectoryParent()) {
+            std::string const& d = dir.GetDirectory().GetCurrentBinary();
+            dirTargets[d].Targets.emplace_back(t);
+          }
+        }
+      }
+      dirTarget.Targets.emplace_back(t);
+    }
+
+    // The directory-level rule should depend on the directory-level
+    // rules of the subdirectories.
+    for (cmStateSnapshot const& state : lg->GetStateSnapshot().GetChildren()) {
+      DirectoryTarget::Dir d;
+      d.Path = state.GetDirectory().GetCurrentBinary();
+      d.ExcludeFromAll =
+        state.GetDirectory().GetPropertyAsBool("EXCLUDE_FROM_ALL");
+      dirTarget.Children.emplace_back(std::move(d));
+    }
+  }
+
+  return dirTargets;
+}

Source/cmGlobalCommonGenerator.h

@@ -7,7 +7,13 @@
 
 #include "cmGlobalGenerator.h"
 
+#include <map>
+#include <string>
+#include <vector>
+
 class cmake;
+class cmGeneratorTarget;
+class cmLocalGenerator;
 
 /** \class cmGlobalCommonGenerator
  * \brief Common infrastructure for Makefile and Ninja global generators.
@@ -17,6 +23,24 @@ class cmGlobalCommonGenerator : public cmGlobalGenerator
 public:
   cmGlobalCommonGenerator(cmake* cm);
   ~cmGlobalCommonGenerator() override;
+
+  struct DirectoryTarget
+  {
+    cmLocalGenerator* LG = nullptr;
+    struct Target
+    {
+      cmGeneratorTarget const* GT = nullptr;
+      bool ExcludeFromAll = false;
+    };
+    std::vector<Target> Targets;
+    struct Dir
+    {
+      std::string Path;
+      bool ExcludeFromAll = false;
+    };
+    std::vector<Dir> Children;
+  };
+  std::map<std::string, DirectoryTarget> ComputeDirectoryTargets() const;
 };
 
 #endif

Source/cmGlobalNinjaGenerator.cxx

@@ -1087,68 +1087,32 @@ void cmGlobalNinjaGenerator::WriteFolderTargets(std::ostream& os)
   cmGlobalNinjaGenerator::WriteDivider(os);
   os << "# Folder targets.\n\n";
 
-  std::map<std::string, cmNinjaDeps> targetsPerFolder;
-  for (cmLocalGenerator const* lg : this->LocalGenerators) {
-    std::string const& currentBinaryFolder(
-      lg->GetStateSnapshot().GetDirectory().GetCurrentBinary());
-
-    // The directory-level rule should depend on the target-level rules
-    // for all targets in the directory.
-    cmNinjaDeps& folderTargets = targetsPerFolder[currentBinaryFolder];
-    for (auto gt : lg->GetGeneratorTargets()) {
-      cmStateEnums::TargetType const type = gt->GetType();
-      if ((type == cmStateEnums::EXECUTABLE ||
-           type == cmStateEnums::STATIC_LIBRARY ||
-           type == cmStateEnums::SHARED_LIBRARY ||
-           type == cmStateEnums::MODULE_LIBRARY ||
-           type == cmStateEnums::OBJECT_LIBRARY ||
-           type == cmStateEnums::UTILITY)) {
-        if (const char* exclude = gt->GetProperty("EXCLUDE_FROM_ALL")) {
-          if (cmIsOn(exclude)) {
-            // This target has been explicitly excluded.
-            continue;
-          }
-          // This target has been explicitly un-excluded.  The directory-level
-          // rule for every directory between this and the root should depend
-          // on the target-level rule for this target.
-          for (cmStateSnapshot dir =
-                 lg->GetStateSnapshot().GetBuildsystemDirectoryParent();
-               dir.IsValid(); dir = dir.GetBuildsystemDirectoryParent()) {
-            std::string const& folder = dir.GetDirectory().GetCurrentBinary();
-            this->AppendTargetOutputs(gt, targetsPerFolder[folder]);
-          }
-        }
-        this->AppendTargetOutputs(gt, folderTargets);
-      }
-    }
+  std::map<std::string, DirectoryTarget> dirTargets =
+    this->ComputeDirectoryTargets();
 
-    // The directory-level rule should depend on the directory-level
-    // rules of the subdirectories.
-    for (cmStateSnapshot const& state : lg->GetStateSnapshot().GetChildren()) {
-      if (state.GetDirectory().GetPropertyAsBool("EXCLUDE_FROM_ALL")) {
-        continue;
+  for (auto const& it : dirTargets) {
+    cmNinjaBuild build("phony");
+    cmGlobalNinjaGenerator::WriteDivider(os);
+    std::string const& currentBinaryDir = it.first;
+    DirectoryTarget const& dt = it.second;
+
+    // Setup target
+    build.Comment = "Folder: " + currentBinaryDir;
+    build.Outputs.emplace_back(
+      this->ConvertToNinjaPath(currentBinaryDir + "/all"));
+    for (DirectoryTarget::Target const& t : dt.Targets) {
+      if (!t.ExcludeFromAll) {
+        this->AppendTargetOutputs(t.GT, build.ExplicitDeps);
       }
-      std::string const& currentBinaryDir =
-        state.GetDirectory().GetCurrentBinary();
-      folderTargets.push_back(
-        this->ConvertToNinjaPath(currentBinaryDir + "/all"));
     }
-  }
-
-  if (!targetsPerFolder.empty()) {
-    cmNinjaBuild build("phony");
-    build.Outputs.emplace_back("");
-    for (auto& it : targetsPerFolder) {
-      cmGlobalNinjaGenerator::WriteDivider(os);
-      std::string const& currentBinaryDir = it.first;
-
-      // Setup target
-      build.Comment = "Folder: " + currentBinaryDir;
-      build.Outputs[0] = this->ConvertToNinjaPath(currentBinaryDir + "/all");
-      build.ExplicitDeps = std::move(it.second);
-      // Write target
-      this->WriteBuild(os, build);
+    for (DirectoryTarget::Dir const& d : dt.Children) {
+      if (!d.ExcludeFromAll) {
+        build.ExplicitDeps.emplace_back(
+          this->ConvertToNinjaPath(d.Path + "/all"));
+      }
     }
+    // Write target
+    this->WriteBuild(os, build);
   }
 }