Merge topic 'makefiles_additional_clean_files'

f945c3e755 Tests: Extend MakeClean test to cover subdirectories without targets
1ded3599d6 Makefiles: Process ADDTIONAL_CLEAN_FILES dir prop at directory level
827da1119e Makefiles: Make build root targets "all", "clean" and "preinstall" recursive
adc3459707 Makefiles: Avoid pointer repurposing
0d41b45cc9 Makefiles: Inline range loop range arguments

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

5 files changed, 108 insertions, 97 deletions

diff --git a/Source/cmGlobalUnixMakefileGenerator3.cxx b/Source/cmGlobalUnixMakefileGenerator3.cxx
index 1ea3673..8053f61 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.cxx
+++ b/Source/cmGlobalUnixMakefileGenerator3.cxx
@@ -232,28 +232,16 @@ void cmGlobalUnixMakefileGenerator3::WriteMainMakefile2()
     depends.push_back(this->EmptyRuleHackDepends);
   }
 
-  // Write and empty all:
-  lg->WriteMakeRule(makefileStream, "The main recursive all target", "all",
-                    depends, no_commands, true);
-
-  // Write an empty preinstall:
-  lg->WriteMakeRule(makefileStream, "The main recursive preinstall target",
-                    "preinstall", depends, no_commands, true);
-
-  // Write an empty clean:
-  lg->WriteMakeRule(makefileStream, "The main recursive clean target", "clean",
-                    depends, no_commands, true);
-
   // Write out the "special" stuff
   lg->WriteSpecialTargetsTop(makefileStream);
 
-  // write the target convenience rules
+  // Write the target convenience rules
   for (cmLocalGenerator* localGen : this->LocalGenerators) {
-    lg = static_cast<cmLocalUnixMakefileGenerator3*>(localGen);
-    this->WriteConvenienceRules2(makefileStream, lg);
+    this->WriteConvenienceRules2(
+      makefileStream, static_cast<cmLocalUnixMakefileGenerator3*>(localGen));
   }
 
-  lg = static_cast<cmLocalUnixMakefileGenerator3*>(this->LocalGenerators[0]);
+  // Write special bottom targets
   lg->WriteSpecialTargetsBottom(makefileStream);
 }
 
@@ -354,9 +342,7 @@ void cmGlobalUnixMakefileGenerator3::WriteMainCMakefile()
   {
     cmakefileStream << "# Byproducts of CMake generate step:\n"
                     << "set(CMAKE_MAKEFILE_PRODUCTS\n";
-    const std::vector<std::string>& outfiles =
-      lg->GetMakefile()->GetOutputFiles();
-    for (std::string const& outfile : outfiles) {
+    for (std::string const& outfile : lg->GetMakefile()->GetOutputFiles()) {
       cmakefileStream << "  \""
                       << lg->MaybeConvertToRelativePath(binDir, outfile)
                       << "\"\n";
@@ -392,8 +378,7 @@ void cmGlobalUnixMakefileGenerator3::WriteMainCMakefileLanguageRules(
   for (cmLocalGenerator* lGenerator : lGenerators) {
     lg = static_cast<cmLocalUnixMakefileGenerator3*>(lGenerator);
     // for all of out targets
-    const std::vector<cmGeneratorTarget*>& tgts = lg->GetGeneratorTargets();
-    for (cmGeneratorTarget* tgt : tgts) {
+    for (cmGeneratorTarget* tgt : lg->GetGeneratorTargets()) {
       if ((tgt->GetType() == cmStateEnums::EXECUTABLE) ||
           (tgt->GetType() == cmStateEnums::STATIC_LIBRARY) ||
           (tgt->GetType() == cmStateEnums::SHARED_LIBRARY) ||
@@ -413,18 +398,18 @@ void cmGlobalUnixMakefileGenerator3::WriteMainCMakefileLanguageRules(
 
 void cmGlobalUnixMakefileGenerator3::WriteDirectoryRule2(
   std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3* lg,
-  const char* pass, bool check_all, bool check_relink)
+  const char* pass, bool check_all, bool check_relink,
+  std::vector<std::string> const& commands)
 {
   // Get the relative path to the subdirectory from the top.
   std::string makeTarget = lg->GetCurrentBinaryDirectory();
-  makeTarget += "/";
+  makeTarget += '/';
   makeTarget += pass;
 
   // The directory-level rule should depend on the target-level rules
   // for all targets in the directory.
   std::vector<std::string> depends;
-  const std::vector<cmGeneratorTarget*>& targets = lg->GetGeneratorTargets();
-  for (cmGeneratorTarget* gtarget : targets) {
+  for (cmGeneratorTarget* gtarget : lg->GetGeneratorTargets()) {
     int type = gtarget->GetType();
     if ((type == cmStateEnums::EXECUTABLE) ||
         (type == cmStateEnums::STATIC_LIBRARY) ||
@@ -446,10 +431,9 @@ void cmGlobalUnixMakefileGenerator3::WriteDirectoryRule2(
 
   // The directory-level rule should depend on the directory-level
   // rules of the subdirectories.
-  std::vector<cmStateSnapshot> children = lg->GetStateSnapshot().GetChildren();
-  for (cmStateSnapshot const& c : children) {
+  for (cmStateSnapshot const& c : lg->GetStateSnapshot().GetChildren()) {
     std::string subdir = c.GetDirectory().GetCurrentBinary();
-    subdir += "/";
+    subdir += '/';
     subdir += pass;
     depends.push_back(std::move(subdir));
   }
@@ -461,34 +445,46 @@ void cmGlobalUnixMakefileGenerator3::WriteDirectoryRule2(
   }
 
   // Write the rule.
-  std::string doc = "Convenience name for \"";
-  doc += pass;
-  doc += "\" pass in the directory.";
-  std::vector<std::string> no_commands;
-  lg->WriteMakeRule(ruleFileStream, doc.c_str(), makeTarget, depends,
-                    no_commands, true);
+  std::string doc;
+  if (lg->IsRootMakefile()) {
+    doc = "The main recursive \"";
+    doc += pass;
+    doc += "\" target.";
+  } else {
+    doc = "Recursive \"";
+    doc += pass;
+    doc += "\" directory target.";
+  }
+  lg->WriteMakeRule(ruleFileStream, doc.c_str(), makeTarget, depends, commands,
+                    true);
 }
 
 void cmGlobalUnixMakefileGenerator3::WriteDirectoryRules2(
   std::ostream& ruleFileStream, cmLocalUnixMakefileGenerator3* lg)
 {
-  // Only subdirectories need these rules.
-  if (lg->IsRootMakefile()) {
-    return;
-  }
-
   // Begin the directory-level rules section.
-  std::string dir =
-    cmSystemTools::ConvertToOutputPath(lg->MaybeConvertToRelativePath(
-      lg->GetBinaryDirectory(), lg->GetCurrentBinaryDirectory()));
-  lg->WriteDivider(ruleFileStream);
-  ruleFileStream << "# Directory level rules for directory " << dir << "\n\n";
+  {
+    std::string dir =
+      cmSystemTools::ConvertToOutputPath(lg->MaybeConvertToRelativePath(
+        lg->GetBinaryDirectory(), lg->GetCurrentBinaryDirectory()));
+    lg->WriteDivider(ruleFileStream);
+    if (lg->IsRootMakefile()) {
+      ruleFileStream << "# Directory level rules for the build root directory";
+    } else {
+      ruleFileStream << "# Directory level rules for directory " << dir;
+    }
+    ruleFileStream << "\n\n";
+  }
 
   // Write directory-level rules for "all".
   this->WriteDirectoryRule2(ruleFileStream, lg, "all", true, false);
 
   // Write directory-level rules for "clean".
-  this->WriteDirectoryRule2(ruleFileStream, lg, "clean", false, false);
+  {
+    std::vector<std::string> cmds;
+    lg->AppendDirectoryCleanCommand(cmds);
+    this->WriteDirectoryRule2(ruleFileStream, lg, "clean", false, false, cmds);
+  }
 
   // Write directory-level rules for "preinstall".
   this->WriteDirectoryRule2(ruleFileStream, lg, "preinstall", true, true);
@@ -567,8 +563,7 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules(
     cmLocalUnixMakefileGenerator3* lg =
       static_cast<cmLocalUnixMakefileGenerator3*>(localGen);
     // for each target Generate the rule files for each target.
-    const std::vector<cmGeneratorTarget*>& targets = lg->GetGeneratorTargets();
-    for (cmGeneratorTarget* gtarget : targets) {
+    for (cmGeneratorTarget* gtarget : lg->GetGeneratorTargets()) {
       // Don't emit the same rule twice (e.g. two targets with the same
       // simple name)
       int type = gtarget->GetType();
@@ -652,8 +647,7 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
   }
 
   // for each target Generate the rule files for each target.
-  const std::vector<cmGeneratorTarget*>& targets = lg->GetGeneratorTargets();
-  for (cmGeneratorTarget* gtarget : targets) {
+  for (cmGeneratorTarget* gtarget : lg->GetGeneratorTargets()) {
     int type = gtarget->GetType();
     std::string name = gtarget->GetName();
     if (!name.empty() &&
@@ -693,9 +687,7 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
       {
         std::ostringstream progressArg;
         const char* sep = "";
-        std::vector<unsigned long> const& progFiles =
-          this->ProgressMap[gtarget].Marks;
-        for (unsigned long progFile : progFiles) {
+        for (unsigned long progFile : this->ProgressMap[gtarget].Marks) {
           progressArg << sep << progFile;
           sep = ",";
         }
@@ -718,15 +710,6 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
       lg->WriteMakeRule(ruleFileStream, "All Build rule for target.",
                         localName, depends, commands, true);
 
-      // add the all/all dependency
-      if (!this->IsExcluded(gtarget)) {
-        depends.clear();
-        depends.push_back(localName);
-        commands.clear();
-        lg->WriteMakeRule(ruleFileStream, "Include target in all.", "all",
-                          depends, commands, true);
-      }
-
       // Write the rule.
       commands.clear();
 
@@ -803,9 +786,6 @@ void cmGlobalUnixMakefileGenerator3::WriteConvenienceRules2(
       lg->WriteMakeRule(ruleFileStream, "clean rule for target.",
                         makeTargetName, depends, commands, true);
       commands.clear();
-      depends.push_back(makeTargetName);
-      lg->WriteMakeRule(ruleFileStream, "clean rule for target.", "clean",
-                        depends, commands, true);
     }
   }
 }
@@ -817,8 +797,7 @@ void cmGlobalUnixMakefileGenerator3::InitializeProgressMarks()
   this->DirectoryTargetsMap.clear();
   // Loop over all targets in all local generators.
   for (cmLocalGenerator* lg : this->LocalGenerators) {
-    const std::vector<cmGeneratorTarget*>& targets = lg->GetGeneratorTargets();
-    for (cmGeneratorTarget* gt : targets) {
+    for (cmGeneratorTarget* gt : lg->GetGeneratorTargets()) {
       cmLocalGenerator* tlg = gt->GetLocalGenerator();
 
       if (gt->GetType() == cmStateEnums::INTERFACE_LIBRARY ||
@@ -841,8 +820,7 @@ void cmGlobalUnixMakefileGenerator3::InitializeProgressMarks()
         // Add dependencies of the included target.  An excluded
         // target may still be included if it is a dependency of a
         // non-excluded target.
-        TargetDependSet const& tgtdeps = this->GetTargetDirectDepends(gt);
-        for (cmTargetDepend const& tgtdep : tgtdeps) {
+        for (cmTargetDepend const& tgtdep : this->GetTargetDirectDepends(gt)) {
           targetSet.insert(tgtdep);
         }
       }
@@ -856,8 +834,7 @@ size_t cmGlobalUnixMakefileGenerator3::CountProgressMarksInTarget(
   size_t count = 0;
   if (emitted.insert(target).second) {
     count = this->ProgressMap[target].Marks.size();
-    TargetDependSet const& depends = this->GetTargetDirectDepends(target);
-    for (cmTargetDepend const& depend : depends) {
+    for (cmTargetDepend const& depend : this->GetTargetDirectDepends(target)) {
       if (depend->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
         continue;
       }
@@ -872,9 +849,8 @@ size_t cmGlobalUnixMakefileGenerator3::CountProgressMarksInAll(
 {
   size_t count = 0;
   std::set<cmGeneratorTarget const*> emitted;
-  std::set<cmGeneratorTarget const*> const& targets =
-    this->DirectoryTargetsMap[lg->GetStateSnapshot()];
-  for (cmGeneratorTarget const* target : targets) {
+  for (cmGeneratorTarget const* target :
+       this->DirectoryTargetsMap[lg->GetStateSnapshot()]) {
     count += this->CountProgressMarksInTarget(target, emitted);
   }
   return count;
@@ -913,8 +889,7 @@ void cmGlobalUnixMakefileGenerator3::TargetProgress::WriteProgressVariables(
 void cmGlobalUnixMakefileGenerator3::AppendGlobalTargetDepends(
   std::vector<std::string>& depends, cmGeneratorTarget* target)
 {
-  TargetDependSet const& depends_set = this->GetTargetDirectDepends(target);
-  for (cmTargetDepend const& i : depends_set) {
+  for (cmTargetDepend const& i : this->GetTargetDirectDepends(target)) {
     // Create the target-level dependency.
     cmGeneratorTarget const* dep = i;
     if (dep->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
@@ -956,9 +931,7 @@ void cmGlobalUnixMakefileGenerator3::WriteHelpRule(
     // the targets
     if (lg2 == lg || lg->IsRootMakefile()) {
       // for each target Generate the rule files for each target.
-      const std::vector<cmGeneratorTarget*>& targets =
-        lg2->GetGeneratorTargets();
-      for (cmGeneratorTarget* target : targets) {
+      for (cmGeneratorTarget* target : lg2->GetGeneratorTargets()) {
         cmStateEnums::TargetType type = target->GetType();
         if ((type == cmStateEnums::EXECUTABLE) ||
             (type == cmStateEnums::STATIC_LIBRARY) ||
diff --git a/Source/cmGlobalUnixMakefileGenerator3.h b/Source/cmGlobalUnixMakefileGenerator3.h
index e919d38..287472c 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.h
+++ b/Source/cmGlobalUnixMakefileGenerator3.h
@@ -165,7 +165,8 @@ protected:
 
   void WriteDirectoryRule2(std::ostream& ruleFileStream,
                            cmLocalUnixMakefileGenerator3* lg, const char* pass,
-                           bool check_all, bool check_relink);
+                           bool check_all, bool check_relink,
+                           std::vector<std::string> const& commands = {});
   void WriteDirectoryRules2(std::ostream& ruleFileStream,
                             cmLocalUnixMakefileGenerator3* lg);
 
diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index 88966c8..6a08840 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/Source/cmLocalUnixMakefileGenerator3.cxx
@@ -15,6 +15,7 @@
 #include "cmCustomCommandGenerator.h"
 #include "cmFileTimeCache.h"
 #include "cmGeneratedFileStream.h"
+#include "cmGeneratorExpression.h"
 #include "cmGeneratorTarget.h"
 #include "cmGlobalGenerator.h"
 #include "cmGlobalUnixMakefileGenerator3.h"
@@ -115,10 +116,9 @@ void cmLocalUnixMakefileGenerator3::Generate()
     this->Makefile->IsOn("CMAKE_SKIP_ASSEMBLY_SOURCE_RULES");
 
   // Generate the rule files for each target.
-  const std::vector<cmGeneratorTarget*>& targets = this->GetGeneratorTargets();
   cmGlobalUnixMakefileGenerator3* gg =
     static_cast<cmGlobalUnixMakefileGenerator3*>(this->GlobalGenerator);
-  for (cmGeneratorTarget* target : targets) {
+  for (cmGeneratorTarget* target : this->GetGeneratorTargets()) {
     if (target->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
       continue;
     }
@@ -154,8 +154,7 @@ void cmLocalUnixMakefileGenerator3::ComputeHomeRelativeOutputPath()
 void cmLocalUnixMakefileGenerator3::GetLocalObjectFiles(
   std::map<std::string, LocalObjectInfo>& localObjectFiles)
 {
-  const std::vector<cmGeneratorTarget*>& targets = this->GetGeneratorTargets();
-  for (cmGeneratorTarget* gt : targets) {
+  for (cmGeneratorTarget* gt : this->GetGeneratorTargets()) {
     if (gt->GetType() == cmStateEnums::INTERFACE_LIBRARY) {
       continue;
     }
@@ -353,9 +352,8 @@ void cmLocalUnixMakefileGenerator3::WriteLocalMakefileTargets(
 
   // for each target we just provide a rule to cd up to the top and do a make
   // on the target
-  const std::vector<cmGeneratorTarget*>& targets = this->GetGeneratorTargets();
   std::string localName;
-  for (cmGeneratorTarget* target : targets) {
+  for (cmGeneratorTarget* target : this->GetGeneratorTargets()) {
     if ((target->GetType() == cmStateEnums::EXECUTABLE) ||
         (target->GetType() == cmStateEnums::STATIC_LIBRARY) ||
         (target->GetType() == cmStateEnums::SHARED_LIBRARY) ||
@@ -1092,6 +1090,56 @@ void cmLocalUnixMakefileGenerator3::AppendCleanCommand(
   }
 }
 
+void cmLocalUnixMakefileGenerator3::AppendDirectoryCleanCommand(
+  std::vector<std::string>& commands)
+{
+  std::vector<std::string> cleanFiles;
+  // Look for additional files registered for cleaning in this directory.
+  if (const char* prop_value =
+        this->Makefile->GetProperty("ADDITIONAL_CLEAN_FILES")) {
+    cmGeneratorExpression ge;
+    std::unique_ptr<cmCompiledGeneratorExpression> cge = ge.Parse(prop_value);
+    cmSystemTools::ExpandListArgument(
+      cge->Evaluate(this,
+                    this->Makefile->GetSafeDefinition("CMAKE_BUILD_TYPE")),
+      cleanFiles);
+  }
+  if (cleanFiles.empty()) {
+    return;
+  }
+
+  cmLocalGenerator* rootLG =
+    this->GetGlobalGenerator()->GetLocalGenerators().at(0);
+  std::string const& binaryDir = rootLG->GetCurrentBinaryDirectory();
+  std::string const& currentBinaryDir = this->GetCurrentBinaryDirectory();
+  std::string cleanfile = currentBinaryDir;
+  cleanfile += "/CMakeFiles/cmake_directory_clean.cmake";
+  // Write clean script
+  {
+    std::string cleanfilePath = cmSystemTools::CollapseFullPath(cleanfile);
+    cmsys::ofstream fout(cleanfilePath.c_str());
+    if (!fout) {
+      cmSystemTools::Error("Could not create " + cleanfilePath);
+      return;
+    }
+    fout << "file(REMOVE_RECURSE\n";
+    for (std::string const& cfl : cleanFiles) {
+      std::string fc = rootLG->MaybeConvertToRelativePath(
+        binaryDir, cmSystemTools::CollapseFullPath(cfl, currentBinaryDir));
+      fout << "  " << cmOutputConverter::EscapeForCMake(fc) << "\n";
+    }
+    fout << ")\n";
+  }
+  // Create command
+  {
+    std::string remove = "$(CMAKE_COMMAND) -P ";
+    remove += this->ConvertToOutputFormat(
+      rootLG->MaybeConvertToRelativePath(binaryDir, cleanfile),
+      cmOutputConverter::SHELL);
+    commands.push_back(std::move(remove));
+  }
+}
+
 void cmLocalUnixMakefileGenerator3::AppendEcho(
   std::vector<std::string>& commands, std::string const& text, EchoColor color,
   EchoProgress const* progress)
diff --git a/Source/cmLocalUnixMakefileGenerator3.h b/Source/cmLocalUnixMakefileGenerator3.h
index fed25e1..c8e4b0e 100644
--- a/Source/cmLocalUnixMakefileGenerator3.h
+++ b/Source/cmLocalUnixMakefileGenerator3.h
@@ -227,6 +227,7 @@ protected:
                           const std::set<std::string>& files,
                           cmGeneratorTarget* target,
                           const char* filename = nullptr);
+  void AppendDirectoryCleanCommand(std::vector<std::string>& commands);
 
   // Helper methods for dependency updates.
   bool ScanDependencies(std::string const& targetDir,
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index e62b107..3a89d75 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/Source/cmMakefileTargetGenerator.cxx
@@ -172,18 +172,6 @@ void cmMakefileTargetGenerator::WriteTargetBuildRules()
     this->CleanFiles.insert(files.begin(), files.end());
   }
 
-  // Look for additional files registered for cleaning in this directory.
-  if (const char* prop_value =
-        this->Makefile->GetProperty("ADDITIONAL_CLEAN_FILES")) {
-    std::vector<std::string> const files = evaluatedFiles(prop_value);
-    // For relative path support
-    std::string const& binaryDir =
-      this->LocalGenerator->GetCurrentBinaryDirectory();
-    for (std::string const& cfl : files) {
-      this->CleanFiles.insert(cmSystemTools::CollapseFullPath(cfl, binaryDir));
-    }
-  }
-
   // Look for additional files registered for cleaning in this target.
   if (const char* prop_value =
         this->GeneratorTarget->GetProperty("ADDITIONAL_CLEAN_FILES")) {