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.

6 files changed, 104 insertions, 176 deletions

diff --git a/Source/cmGlobalUnixMakefileGenerator3.cxx b/Source/cmGlobalUnixMakefileGenerator3.cxx
index 2b2f31a..6538936 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.cxx
+++ b/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 = 
-      static_cast<cmLocalUnixMakefileGenerator3 *>(this->LocalGenerators[i]);
-    total += lg->GetNumberOfProgressActions();
+    total += pmi->second.NumberOfActions;
     }
 
   // 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 = 
-      static_cast<cmLocalUnixMakefileGenerator3 *>(this->LocalGenerators[i]);
-    lg->WriteProgressVariables(total,current);
+    pmi->second.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,
-                                                      emitted);
+              << this->CountProgressMarksInTarget(&t->second, emitted);
       commands.push_back(progCmd.str());
       }
       std::string tmp = cmake::GetCMakeFilesDirectoryPostSlash();
@@ -868,46 +870,77 @@ cmGlobalUnixMakefileGenerator3
 }
 
 //----------------------------------------------------------------------------
-int cmGlobalUnixMakefileGenerator3
-::GetTargetTotalNumberOfActions(cmTarget &target,
-                                std::set<cmTarget *> &emitted)
+size_t
+cmGlobalUnixMakefileGenerator3
+::CountProgressMarksInTarget(cmTarget* target,
+                             std::set<cmTarget*>& emitted)
 {
-  // do not double count
-  int result = 0;
-
-  if(emitted.insert(&target).second)
+  size_t count = 0;
+  if(emitted.insert(target).second)
     {
-    cmLocalUnixMakefileGenerator3 *lg = 
-      static_cast<cmLocalUnixMakefileGenerator3 *>
-      (target.GetMakefile()->GetLocalGenerator());
-    result = static_cast<int>(lg->ProgressFiles[target.GetName()].size());
-    
-    TargetDependSet & depends = this->GetTargetDirectDepends(target);
-    
-    TargetDependSet::iterator i;
-    for (i = depends.begin(); i != depends.end(); ++i)
+    count = this->ProgressMap[target->GetName()].Marks.size();
+    TargetDependSet const& depends = this->GetTargetDirectDepends(*target);
+    for(TargetDependSet::const_iterator di = depends.begin();
+        di != depends.end(); ++di)
       {
-      result += this->GetTargetTotalNumberOfActions(**i, emitted);
+      count += this->CountProgressMarksInTarget(*di, emitted);
       }
     }
-  
-  return result;
+  return count;
 }
 
-unsigned long cmGlobalUnixMakefileGenerator3
-::GetNumberOfProgressActionsInAll(cmLocalUnixMakefileGenerator3 *lg)
+//----------------------------------------------------------------------------
+size_t
+cmGlobalUnixMakefileGenerator3
+::CountProgressMarksInAll(cmLocalUnixMakefileGenerator3* lg)
 {
-  unsigned long result = 0;
-  std::set<cmTarget *> emitted;
-  std::set<cmTarget *>& targets = this->LocalGeneratorToTargetMap[lg];
-  for(std::set<cmTarget *>::iterator t = targets.begin();
+  size_t count = 0;
+  std::set<cmTarget*> emitted;
+  std::set<cmTarget*> const& targets = this->LocalGeneratorToTargetMap[lg];
+  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
diff --git a/Source/cmGlobalUnixMakefileGenerator3.h b/Source/cmGlobalUnixMakefileGenerator3.h
index 5cfe26f..d21b3e8 100644
--- a/Source/cmGlobalUnixMakefileGenerator3.h
+++ b/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
-  int GetTargetTotalNumberOfActions(cmTarget & target,
-                                    std::set<cmTarget *> &emitted);
-  unsigned long GetNumberOfProgressActionsInAll
-  (cmLocalUnixMakefileGenerator3 *lg);
+  /** Record per-target progress information.  */
+  void RecordTargetProgress(cmMakefileTargetGenerator* tg);
 
   /**
    * 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
diff --git a/Source/cmLocalUnixMakefileGenerator3.cxx b/Source/cmLocalUnixMakefileGenerator3.cxx
index c37a80b..b5779bc 100644
--- a/Source/cmLocalUnixMakefileGenerator3.cxx
+++ b/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 =
-      cmMakefileTargetGenerator::New(&(t->second));
-    if (tg)
+    cmsys::auto_ptr<cmMakefileTargetGenerator> tg(
+      cmMakefileTargetGenerator::New(&(t->second)));
+    if (tg.get())
       {
-      this->TargetGenerators.push_back(tg);
       tg->WriteRuleFiles();
+      gg->RecordTargetProgress(tg.get());
       }
     }
 
@@ -145,75 +149,6 @@ void cmLocalUnixMakefileGenerator3::Generate()
 }
 
 //----------------------------------------------------------------------------
-// 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()
 {
   // generate the includes
@@ -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(),
diff --git a/Source/cmLocalUnixMakefileGenerator3.h b/Source/cmLocalUnixMakefileGenerator3.h
index 15fafa5..0bb0314 100644
--- a/Source/cmLocalUnixMakefileGenerator3.h
+++ b/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;
 };
diff --git a/Source/cmMakefileTargetGenerator.cxx b/Source/cmMakefileTargetGenerator.cxx
index 4b9c724..f38874d 100644
--- a/Source/cmMakefileTargetGenerator.cxx
+++ b/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
diff --git a/Source/cmMakefileTargetGenerator.h b/Source/cmMakefileTargetGenerator.h
index 0c0d21d..815dc8d 100644
--- a/Source/cmMakefileTargetGenerator.h
+++ b/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;