ENH: Cleanup make progress rule generation code

This cleans up the Makefile generator's progress rule code.  Instead of
keeping every cmMakefileTargetGenerator instance alive to generate
progress, we keep only the information necessary in a single table.
This approach keeps most of the code in cmGlobalUnixMakefileGenerator3,
thus simplifying its public interface.

Source/cmGlobalUnixMakefileGenerator3.cxx

@@ -17,6 +17,7 @@
 
 #include "cmGlobalUnixMakefileGenerator3.h"
 #include "cmLocalUnixMakefileGenerator3.h"
+#include "cmMakefileTargetGenerator.h"
 #include "cmMakefile.h"
 #include "cmake.h"
 #include "cmGeneratedFileStream.h"
@@ -150,13 +151,11 @@ void cmGlobalUnixMakefileGenerator3::Generate()
   this->cmGlobalGenerator::Generate();
 
   // initialize progress
-  unsigned int i;
   unsigned long total = 0;
-  for (i = 0; i < this->LocalGenerators.size(); ++i)
+  for(ProgressMapType::const_iterator pmi = this->ProgressMap.begin();
+      pmi != this->ProgressMap.end(); ++pmi)
     {
-    cmLocalUnixMakefileGenerator3 *lg = 
+    total += pmi->second.NumberOfActions;
-      static_cast<cmLocalUnixMakefileGenerator3 *>(this->LocalGenerators[i]);
-    total += lg->GetNumberOfProgressActions();
     }
 
   // write each target's progress.make this loop is done twice. Bascially the
@@ -167,17 +166,21 @@ void cmGlobalUnixMakefileGenerator3::Generate()
   // well. This is because the all targets require more information that is
   // computed in the first loop.
   unsigned long current = 0;
-  for (i = 0; i < this->LocalGenerators.size(); ++i)
+  for(ProgressMapType::iterator pmi = this->ProgressMap.begin();
+      pmi != this->ProgressMap.end(); ++pmi)
     {
-    cmLocalUnixMakefileGenerator3 *lg = 
+    pmi->second.WriteProgressVariables(total, current);
-      static_cast<cmLocalUnixMakefileGenerator3 *>(this->LocalGenerators[i]);
-    lg->WriteProgressVariables(total,current);
     }
-  for (i = 0; i < this->LocalGenerators.size(); ++i)
+  for(unsigned int i = 0; i < this->LocalGenerators.size(); ++i)
     {
     cmLocalUnixMakefileGenerator3 *lg = 
       static_cast<cmLocalUnixMakefileGenerator3 *>(this->LocalGenerators[i]);
-    lg->WriteAllProgressVariable();
+    std::string markFileName = lg->GetMakefile()->GetStartOutputDirectory();
+    markFileName += "/";
+    markFileName += cmake::GetCMakeFilesDirectory();
+    markFileName += "/progress.marks";
+    cmGeneratedFileStream markFile(markFileName.c_str());
+    markFile << this->CountProgressMarksInAll(lg) << "\n";
     }
   
   // write the main makefile
@@ -752,7 +755,7 @@ cmGlobalUnixMakefileGenerator3
                                 cmLocalGenerator::FULL,
                                 cmLocalGenerator::SHELL);
         progCmd << " ";
-        std::vector<int> &progFiles = lg->ProgressFiles[t->first];
+        std::vector<int> &progFiles = this->ProgressMap[t->first].Marks;
         for (std::vector<int>::iterator i = progFiles.begin();
               i != progFiles.end(); ++i)
           {
@@ -794,8 +797,7 @@ cmGlobalUnixMakefileGenerator3
       //
       std::set<cmTarget *> emitted;
       progCmd << " " 
-              << this->GetTargetTotalNumberOfActions(t->second,
+              << this->CountProgressMarksInTarget(&t->second, emitted);
-                                                      emitted);
       commands.push_back(progCmd.str());
       }
       std::string tmp = cmake::GetCMakeFilesDirectoryPostSlash();
@@ -868,46 +870,77 @@ cmGlobalUnixMakefileGenerator3
 }
 
 //----------------------------------------------------------------------------
-int cmGlobalUnixMakefileGenerator3
+size_t
-::GetTargetTotalNumberOfActions(cmTarget &target,
+cmGlobalUnixMakefileGenerator3
-                                std::set<cmTarget *> &emitted)
+::CountProgressMarksInTarget(cmTarget* target,
+                             std::set<cmTarget*>& emitted)
 {
-  // do not double count
+  size_t count = 0;
-  int result = 0;
+  if(emitted.insert(target).second)
-
-  if(emitted.insert(&target).second)
     {
-    cmLocalUnixMakefileGenerator3 *lg = 
+    count = this->ProgressMap[target->GetName()].Marks.size();
-      static_cast<cmLocalUnixMakefileGenerator3 *>
+    TargetDependSet const& depends = this->GetTargetDirectDepends(*target);
-      (target.GetMakefile()->GetLocalGenerator());
+    for(TargetDependSet::const_iterator di = depends.begin();
-    result = static_cast<int>(lg->ProgressFiles[target.GetName()].size());
+        di != depends.end(); ++di)
-    
-    TargetDependSet & depends = this->GetTargetDirectDepends(target);
-    
-    TargetDependSet::iterator i;
-    for (i = depends.begin(); i != depends.end(); ++i)
       {
-      result += this->GetTargetTotalNumberOfActions(**i, emitted);
+      count += this->CountProgressMarksInTarget(*di, emitted);
       }
     }
-  
+  return count;
-  return result;
 }
 
-unsigned long cmGlobalUnixMakefileGenerator3
+//----------------------------------------------------------------------------
-::GetNumberOfProgressActionsInAll(cmLocalUnixMakefileGenerator3 *lg)
+size_t
+cmGlobalUnixMakefileGenerator3
+::CountProgressMarksInAll(cmLocalUnixMakefileGenerator3* lg)
 {
-  unsigned long result = 0;
+  size_t count = 0;
-  std::set<cmTarget *> emitted;
+  std::set<cmTarget*> emitted;
-  std::set<cmTarget *>& targets = this->LocalGeneratorToTargetMap[lg];
+  std::set<cmTarget*> const& targets = this->LocalGeneratorToTargetMap[lg];
-  for(std::set<cmTarget *>::iterator t = targets.begin();
+  for(std::set<cmTarget*>::const_iterator t = targets.begin();
       t != targets.end(); ++t)
     {
-    result += this->GetTargetTotalNumberOfActions(**t,emitted);
+    count += this->CountProgressMarksInTarget(*t, emitted);
     }
-  return result;
+  return count;
 }
 
+//----------------------------------------------------------------------------
+void
+cmGlobalUnixMakefileGenerator3::RecordTargetProgress(
+  cmMakefileTargetGenerator* tg)
+{
+  TargetProgress& tp = this->ProgressMap[tg->GetTarget()->GetName()];
+  tp.NumberOfActions = tg->GetNumberOfProgressActions();
+  tp.VariableFile = tg->GetProgressFileNameFull();
+}
+
+//----------------------------------------------------------------------------
+void
+cmGlobalUnixMakefileGenerator3::TargetProgress
+::WriteProgressVariables(unsigned long total, unsigned long &current)
+{
+  cmGeneratedFileStream fout(this->VariableFile.c_str());
+  for(unsigned long i = 1; i <= this->NumberOfActions; ++i)
+    {
+    fout << "CMAKE_PROGRESS_" << i << " = ";
+    if (total <= 100)
+      {
+      unsigned long num = i + current;
+      fout << num;
+      this->Marks.push_back(num);
+      }
+    else if (((i+current)*100)/total > ((i-1+current)*100)/total)
+      {
+      unsigned long num = ((i+current)*100)/total;
+      fout << num;
+      this->Marks.push_back(num);
+      }
+    fout << "\n";
+    }
+  fout << "\n";
+  current += this->NumberOfActions;
+}
 
 //----------------------------------------------------------------------------
 void

Source/cmGlobalUnixMakefileGenerator3.h

@@ -20,6 +20,7 @@
 #include "cmGlobalGenerator.h"
 
 class cmGeneratedFileStream;
+class cmMakefileTargetGenerator;
 class cmLocalUnixMakefileGenerator3;
 
 /** \class cmGlobalUnixMakefileGenerator3
@@ -113,11 +114,8 @@ public:
    const char *targetName,
    const char* config, bool ignoreErrors, bool fast);
 
-  // returns some progress informaiton
+  /** Record per-target progress information.  */
-  int GetTargetTotalNumberOfActions(cmTarget & target,
+  void RecordTargetProgress(cmMakefileTargetGenerator* tg);
-                                    std::set<cmTarget *> &emitted);
-  unsigned long GetNumberOfProgressActionsInAll
-  (cmLocalUnixMakefileGenerator3 *lg);
 
   /**
    * If true, the CMake variable CMAKE_VERBOSE_MAKEFILES doesn't have effect
@@ -177,6 +175,22 @@ protected:
   std::string EmptyRuleHackCommand;
 
   bool ForceVerboseMakefiles;
+
+  // Store per-target progress counters.
+  struct TargetProgress
+  {
+    TargetProgress(): NumberOfActions(0) {}
+    unsigned long NumberOfActions;
+    std::string VariableFile;
+    std::vector<int> Marks;
+    void WriteProgressVariables(unsigned long total, unsigned long& current);
+  };
+  typedef std::map<cmStdString, TargetProgress> ProgressMapType;
+  ProgressMapType ProgressMap;
+
+  size_t CountProgressMarksInTarget(cmTarget* target,
+                                    std::set<cmTarget*>& emitted);
+  size_t CountProgressMarksInAll(cmLocalUnixMakefileGenerator3* lg);
 };
 
 #endif

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -35,6 +35,8 @@
 # include <cmsys/Terminal.h>
 #endif
 
+#include <cmsys/auto_ptr.hxx>
+
 #include <memory> // auto_ptr
 #include <queue>
 
@@ -126,14 +128,16 @@ void cmLocalUnixMakefileGenerator3::Generate()
 
   // Generate the rule files for each target.
   cmTargets& targets = this->Makefile->GetTargets();
+  cmGlobalUnixMakefileGenerator3* gg =
+    static_cast<cmGlobalUnixMakefileGenerator3*>(this->GlobalGenerator);
   for(cmTargets::iterator t = targets.begin(); t != targets.end(); ++t)
     {
-    cmMakefileTargetGenerator *tg =
+    cmsys::auto_ptr<cmMakefileTargetGenerator> tg(
-      cmMakefileTargetGenerator::New(&(t->second));
+      cmMakefileTargetGenerator::New(&(t->second)));
-    if (tg)
+    if (tg.get())
       {
-      this->TargetGenerators.push_back(tg);
       tg->WriteRuleFiles();
+      gg->RecordTargetProgress(tg.get());
       }
     }
 
@@ -144,75 +148,6 @@ void cmLocalUnixMakefileGenerator3::Generate()
   this->WriteDirectoryInformationFile();
 }
 
-//----------------------------------------------------------------------------
-// return info about progress actions
-unsigned long cmLocalUnixMakefileGenerator3::GetNumberOfProgressActions()
-{
-  unsigned long result = 0;
-
-  for (std::vector<cmMakefileTargetGenerator *>::iterator mtgIter = 
-         this->TargetGenerators.begin();
-       mtgIter != this->TargetGenerators.end(); ++mtgIter)
-    {
-    result += (*mtgIter)->GetNumberOfProgressActions();
-    }  
-  return result;
-}
-
-//----------------------------------------------------------------------------
-// return info about progress actions
-unsigned long cmLocalUnixMakefileGenerator3
-::GetNumberOfProgressActionsForTarget(const char *name)
-{
-  for (std::vector<cmMakefileTargetGenerator *>::iterator mtgIter = 
-         this->TargetGenerators.begin();
-       mtgIter != this->TargetGenerators.end(); ++mtgIter)
-    {
-    if (!strcmp(name,(*mtgIter)->GetTarget()->GetName()))
-      {
-      return (*mtgIter)->GetNumberOfProgressActions();
-      }
-    }  
-  return 0;
-}
-
-
-//----------------------------------------------------------------------------
-// writes the progreess variables and also closes out the targets
-void cmLocalUnixMakefileGenerator3
-::WriteProgressVariables(unsigned long total,
-                         unsigned long &current)
-{
-  // delete the makefile target generator objects
-  for (std::vector<cmMakefileTargetGenerator *>::iterator mtgIter = 
-         this->TargetGenerators.begin();
-       mtgIter != this->TargetGenerators.end(); ++mtgIter)
-    {
-    (*mtgIter)->WriteProgressVariables(total,current);
-    delete *mtgIter;
-    }  
-  this->TargetGenerators.clear();
-}
-
-void cmLocalUnixMakefileGenerator3::WriteAllProgressVariable()
-{
-  // write the top level progress for the all target
-  std::string progressFile = cmake::GetCMakeFilesDirectory();
-  progressFile += "/progress.make";
-  std::string progressFileNameFull = 
-    this->ConvertToFullPath(progressFile.c_str());
-  cmGeneratedFileStream ruleFileStream(progressFileNameFull.c_str());
-  if(!ruleFileStream)
-    {
-    return;
-    }
-
-  cmGlobalUnixMakefileGenerator3 *gg = 
-    static_cast<cmGlobalUnixMakefileGenerator3*>(this->GlobalGenerator);
-
-  ruleFileStream << gg->GetNumberOfProgressActionsInAll(this) << "\n";
-}
-
 //----------------------------------------------------------------------------
 void cmLocalUnixMakefileGenerator3::WriteLocalMakefile()
 {
@@ -1717,7 +1652,7 @@ void cmLocalUnixMakefileGenerator3
                              cmLocalGenerator::SHELL);
 
     std::string progressFile = cmake::GetCMakeFilesDirectory();
-    progressFile += "/progress.make";
+    progressFile += "/progress.marks";
     std::string progressFileNameFull =
       this->ConvertToFullPath(progressFile.c_str());
     progCmd << " " << this->Convert(progressFileNameFull.c_str(),

Source/cmLocalUnixMakefileGenerator3.h

@@ -66,10 +66,6 @@ public:
   // write the main variables used by the makefiles
   void WriteMakeVariables(std::ostream& makefileStream);
 
-  // write the progress variables used by the makefiles
-  void WriteProgressVariables(unsigned long total, unsigned long &current);
-  void WriteAllProgressVariable();
-
   /**
    * If true, then explicitly pass MAKEFLAGS on the make all target for makes
    * that do not use environment variables.
@@ -256,10 +252,6 @@ public:
 
   std::vector<cmStdString> const& GetLocalHelp() { return this->LocalHelp; }
 
-  // return info about progress actions
-  unsigned long GetNumberOfProgressActions();
-  unsigned long GetNumberOfProgressActionsForTarget(const char *);
-
   /** Get whether to create rules to generate preprocessed and
       assembly sources.  This could be converted to a variable lookup
       later.  */
@@ -340,8 +332,6 @@ protected:
                           const std::vector<std::string>& files,
                           cmTarget& target, const char* filename =0);
 
-  std::map<cmStdString, std::vector<int> > ProgressFiles;
-
   // Helper methods for dependeny updates.
   bool ScanDependencies(const char* targetDir);
   void CheckMultipleOutputs(bool verbose);
@@ -390,7 +380,6 @@ private:
   std::vector<cmStdString> LocalHelp;
 
   /* does the work for each target */
-  std::vector<cmMakefileTargetGenerator *> TargetGenerators;
   std::map<cmStdString, cmStdString> MakeVariableMap;
   std::map<cmStdString, cmStdString> ShortMakeVariableMap;
 };

Source/cmMakefileTargetGenerator.cxx

@@ -96,8 +96,6 @@ void cmMakefileTargetGenerator::CreateRuleFile()
   this->BuildFileNameFull += "/build.make";
 
   // Construct the rule file name.
-  this->ProgressFileName = this->TargetBuildDirectory;
-  this->ProgressFileName += "/progress.make";
   this->ProgressFileNameFull = this->TargetBuildDirectoryFull;
   this->ProgressFileNameFull += "/progress.make";
 
@@ -1535,43 +1533,6 @@ void cmMakefileTargetGenerator::RemoveForbiddenFlags(const char* flagVar,
     }
 }
 
-void cmMakefileTargetGenerator::WriteProgressVariables(unsigned long total,
-                                                       unsigned long &current)
-{
-  cmGeneratedFileStream *progressFileStream = 
-    new cmGeneratedFileStream(this->ProgressFileNameFull.c_str());
-  if(!progressFileStream)
-    {
-    return;
-    }
-
-  unsigned long num;
-  unsigned long i;
-  for (i = 1; i <= this->NumberOfProgressActions; ++i)
-    {
-    *progressFileStream
-      << "CMAKE_PROGRESS_" << i << " = ";
-    if (total <= 100)
-      {
-      num = i + current;
-      *progressFileStream << num;
-      this->LocalGenerator->ProgressFiles[this->Target->GetName()]
-        .push_back(num);
-      }
-    else if (((i+current)*100)/total > ((i-1+current)*100)/total)
-      {
-      num = ((i+current)*100)/total;
-      *progressFileStream << num;
-      this->LocalGenerator->ProgressFiles[this->Target->GetName()]
-        .push_back(num);
-      }
-    *progressFileStream << "\n";
-    }
-  *progressFileStream << "\n";
-  current += this->NumberOfProgressActions;
-  delete progressFileStream;
-}
-
 //----------------------------------------------------------------------------
 void
 cmMakefileTargetGenerator

Source/cmMakefileTargetGenerator.h

@@ -47,14 +47,11 @@ public:
      with this target */
   virtual void WriteRuleFiles() = 0;
 
-  /* the main entry point for this class. Writes the Makefiles associated
-     with this target */
-  virtual void WriteProgressVariables(unsigned long total, 
-                                      unsigned long &current);
-  
   /* return the number of actions that have progress reporting on them */
   virtual unsigned long GetNumberOfProgressActions() {
     return this->NumberOfProgressActions;}
+  std::string GetProgressFileNameFull()
+    { return this->ProgressFileNameFull; }
 
   cmTarget* GetTarget() { return this->Target;}
 protected:
@@ -167,7 +164,6 @@ protected:
   std::string BuildFileNameFull;
 
   // the full path to the progress file
-  std::string ProgressFileName;
   std::string ProgressFileNameFull;
   unsigned long NumberOfProgressActions;
   bool NoRuleMessages;