Merge topic 'makefile-progress-improvements'

63668954 Help: Add notes for topic 'makefile-progress-improvements'
ae775fe8 Makefile: Change link step message color to bold green
7bb50e4a Makefile: Add progress to link step messages
c6ada827 Makefile: Print all color escape sequences before newline
8521fdf5 Makefile: Fix output during parallel builds (#12991)
69ac6d27 bootstrap: Enable color Makefile output

Help/release/dev/makefile-progress-improvements.rst

@@ -0,0 +1,7 @@
+makefile-progress-improvements
+------------------------------
+
+* With Makefile generators, the build-time progress output has been improved.
+  It no longer mixes progress and build rule messages during parallel builds.
+  The link rule messages now have progress and are displayed as bold green
+  instead of bold red (since red is often associated with an error message).

Source/cmGlobalUnixMakefileGenerator3.cxx

@@ -779,29 +779,24 @@ cmGlobalUnixMakefileGenerator3
       localName += "/all";
       depends.clear();
 
-      std::string progressDir =
-        lg->GetMakefile()->GetHomeOutputDirectory();
-      progressDir += cmake::GetCMakeFilesDirectory();
+      cmLocalUnixMakefileGenerator3::EchoProgress progress;
+      progress.Dir = lg->GetMakefile()->GetHomeOutputDirectory();
+      progress.Dir += cmake::GetCMakeFilesDirectory();
+      {
+      std::ostringstream progressArg;
+      const char* sep = "";
+      std::vector<unsigned long>& progFiles =
+        this->ProgressMap[gtarget->Target].Marks;
+      for (std::vector<unsigned long>::iterator i = progFiles.begin();
+           i != progFiles.end(); ++i)
         {
-        std::ostringstream progCmd;
-        progCmd << "$(CMAKE_COMMAND) -E cmake_progress_report ";
-        // all target counts
-        progCmd << lg->Convert(progressDir,
-                                cmLocalGenerator::FULL,
-                                cmLocalGenerator::SHELL);
-        progCmd << " ";
-        std::vector<unsigned long>& progFiles =
-          this->ProgressMap[gtarget->Target].Marks;
-        for (std::vector<unsigned long>::iterator i = progFiles.begin();
-              i != progFiles.end(); ++i)
-          {
-          progCmd << " " << *i;
-          }
-        commands.push_back(progCmd.str());
+        progressArg << sep << *i;
+        sep = ",";
         }
-      progressDir = "Built target ";
-      progressDir += name;
-      lg->AppendEcho(commands,progressDir.c_str());
+      progress.Arg = progressArg.str();
+      }
+      lg->AppendEcho(commands, "Built target " + name,
+        cmLocalUnixMakefileGenerator3::EchoNormal, &progress);
 
       this->AppendGlobalTargetDepends(depends,*gtarget->Target);
       lg->WriteMakeRule(ruleFileStream, "All Build rule for target.",
@@ -819,15 +814,13 @@ cmGlobalUnixMakefileGenerator3
 
       // Write the rule.
       commands.clear();
-      progressDir = lg->GetMakefile()->GetHomeOutputDirectory();
-      progressDir += cmake::GetCMakeFilesDirectory();
 
       {
       // TODO: Convert the total progress count to a make variable.
       std::ostringstream progCmd;
       progCmd << "$(CMAKE_COMMAND) -E cmake_progress_start ";
       // # in target
-      progCmd << lg->Convert(progressDir,
+      progCmd << lg->Convert(progress.Dir,
                               cmLocalGenerator::FULL,
                               cmLocalGenerator::SHELL);
       //
@@ -843,7 +836,7 @@ cmGlobalUnixMakefileGenerator3
       {
       std::ostringstream progCmd;
       progCmd << "$(CMAKE_COMMAND) -E cmake_progress_start "; // # 0
-      progCmd << lg->Convert(progressDir,
+      progCmd << lg->Convert(progress.Dir,
                               cmLocalGenerator::FULL,
                               cmLocalGenerator::SHELL);
       progCmd << " 0";

Source/cmLocalUnixMakefileGenerator3.cxx

@@ -27,10 +27,10 @@
 #ifdef CMAKE_BUILD_WITH_CMAKE
 # include "cmDependsFortran.h"
 # include "cmDependsJava.h"
-# include <cmsys/Terminal.h>
 #endif
 
 #include <cmsys/auto_ptr.hxx>
+#include <cmsys/Terminal.h>
 
 #include <queue>
 
@@ -1346,12 +1346,12 @@ cmLocalUnixMakefileGenerator3
 //----------------------------------------------------------------------------
 void
 cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
-                                          const char* text,
-                                          EchoColor color)
+                                          std::string const& text,
+                                          EchoColor color,
+                                          EchoProgress const* progress)
 {
   // Choose the color for the text.
   std::string color_name;
-#ifdef CMAKE_BUILD_WITH_CMAKE
   if(this->GlobalGenerator->GetToolSupportsColor() && this->ColorMakefile)
     {
     // See cmake::ExecuteEchoColor in cmake.cxx for these options.
@@ -1367,7 +1367,7 @@ cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
         color_name = "--green ";
         break;
       case EchoLink:
-        color_name = "--red --bold ";
+        color_name = "--green --bold ";
         break;
       case EchoGenerate:
         color_name = "--blue --bold ";
@@ -1377,14 +1377,11 @@ cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
         break;
       }
     }
-#else
-  (void)color;
-#endif
 
   // Echo one line at a time.
   std::string line;
   line.reserve(200);
-  for(const char* c = text;; ++c)
+  for(const char* c = text.c_str();; ++c)
     {
     if(*c == '\n' || *c == '\0')
       {
@@ -1393,7 +1390,7 @@ cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
         {
         // Add a command to echo this line.
         std::string cmd;
-        if(color_name.empty())
+        if(color_name.empty() && !progress)
           {
           // Use the native echo command.
           cmd = "@echo ";
@@ -1404,6 +1401,17 @@ cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
           // Use cmake to echo the text in color.
           cmd = "@$(CMAKE_COMMAND) -E cmake_echo_color --switch=$(COLOR) ";
           cmd += color_name;
+          if (progress)
+            {
+            cmd += "--progress-dir=";
+            cmd += this->Convert(progress->Dir,
+                                 cmLocalGenerator::FULL,
+                                 cmLocalGenerator::SHELL);
+            cmd += " ";
+            cmd += "--progress-num=";
+            cmd += progress->Arg;
+            cmd += " ";
+            }
           cmd += this->EscapeForShell(line);
           }
         commands.push_back(cmd);
@@ -1412,6 +1420,9 @@ cmLocalUnixMakefileGenerator3::AppendEcho(std::vector<std::string>& commands,
       // Reset the line to emtpy.
       line = "";
 
+      // Progress appears only on first line.
+      progress = 0;
+
       // Terminate on end-of-string.
       if(*c == '\0')
         {
@@ -1617,14 +1628,10 @@ bool cmLocalUnixMakefileGenerator3::UpdateDependencies(const char* tgtInfo,
     targetName = targetName.substr(0, targetName.length()-4);
     std::string message = "Scanning dependencies of target ";
     message += targetName;
-#ifdef CMAKE_BUILD_WITH_CMAKE
     cmSystemTools::MakefileColorEcho(
       cmsysTerminal_Color_ForegroundMagenta |
       cmsysTerminal_Color_ForegroundBold,
       message.c_str(), true, color);
-#else
-    fprintf(stdout, "%s\n", message.c_str());
-#endif
 
     return this->ScanDependencies(dir.c_str(), validDependencies);
     }

Source/cmLocalUnixMakefileGenerator3.h

@@ -184,8 +184,9 @@ public:
   // append an echo command
   enum EchoColor { EchoNormal, EchoDepend, EchoBuild, EchoLink,
                    EchoGenerate, EchoGlobal };
-  void AppendEcho(std::vector<std::string>& commands, const char* text,
-                  EchoColor color = EchoNormal);
+  struct EchoProgress { std::string Dir; std::string Arg; };
+  void AppendEcho(std::vector<std::string>& commands, std::string const& text,
+                  EchoColor color = EchoNormal, EchoProgress const* = 0);
 
   /** Get whether the makefile is to have color.  */
   bool GetColorMakefile() const { return this->ColorMakefile; }

Source/cmMakefileExecutableTargetGenerator.cxx

@@ -171,15 +171,19 @@ void cmMakefileExecutableTargetGenerator::WriteExecutableRule(bool relink)
     return;
     }
 
+  this->NumberOfProgressActions++;
   if(!this->NoRuleMessages)
     {
+    cmLocalUnixMakefileGenerator3::EchoProgress progress;
+    this->MakeEchoProgress(progress);
     // Add the link message.
     std::string buildEcho = "Linking ";
     buildEcho += linkLanguage;
     buildEcho += " executable ";
     buildEcho += targetOutPath;
     this->LocalGenerator->AppendEcho(commands, buildEcho.c_str(),
-                                     cmLocalUnixMakefileGenerator3::EchoLink);
+                                     cmLocalUnixMakefileGenerator3::EchoLink,
+                                     &progress);
     }
 
   // Build a list of compiler flags and linker flags.

Source/cmMakefileLibraryTargetGenerator.cxx

@@ -341,8 +341,11 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
     this->Convert(targetFullPathImport,cmLocalGenerator::START_OUTPUT,
                   cmLocalGenerator::SHELL);
 
+  this->NumberOfProgressActions++;
   if(!this->NoRuleMessages)
     {
+    cmLocalUnixMakefileGenerator3::EchoProgress progress;
+    this->MakeEchoProgress(progress);
     // Add the link message.
     std::string buildEcho = "Linking ";
     buildEcho += linkLanguage;
@@ -365,7 +368,8 @@ void cmMakefileLibraryTargetGenerator::WriteLibraryRules
       }
     buildEcho += targetOutPath.c_str();
     this->LocalGenerator->AppendEcho(commands, buildEcho.c_str(),
-                                     cmLocalUnixMakefileGenerator3::EchoLink);
+                                     cmLocalUnixMakefileGenerator3::EchoLink,
+                                     &progress);
     }
 
   const char* forbiddenFlagVar = 0;

Source/cmMakefileTargetGenerator.cxx

@@ -618,16 +618,19 @@ cmMakefileTargetGenerator
   std::vector<std::string> commands;
 
   // add in a progress call if needed
-  this->AppendProgress(commands);
+  this->NumberOfProgressActions++;
 
   if(!this->NoRuleMessages)
     {
+    cmLocalUnixMakefileGenerator3::EchoProgress progress;
+    this->MakeEchoProgress(progress);
     std::string buildEcho = "Building ";
     buildEcho += lang;
     buildEcho += " object ";
     buildEcho += relativeObj;
     this->LocalGenerator->AppendEcho
-      (commands, buildEcho.c_str(), cmLocalUnixMakefileGenerator3::EchoBuild);
+      (commands, buildEcho.c_str(), cmLocalUnixMakefileGenerator3::EchoBuild,
+       &progress);
     }
 
   std::string targetOutPathReal;
@@ -1200,12 +1203,15 @@ void cmMakefileTargetGenerator
   if(!comment.empty())
     {
     // add in a progress call if needed
-    this->AppendProgress(commands);
+    this->NumberOfProgressActions++;
     if(!this->NoRuleMessages)
       {
+      cmLocalUnixMakefileGenerator3::EchoProgress progress;
+      this->MakeEchoProgress(progress);
       this->LocalGenerator
         ->AppendEcho(commands, comment.c_str(),
-                     cmLocalUnixMakefileGenerator3::EchoGenerate);
+                     cmLocalUnixMakefileGenerator3::EchoGenerate,
+                     &progress);
       }
     }
 
@@ -1263,22 +1269,14 @@ void cmMakefileTargetGenerator
 
 //----------------------------------------------------------------------------
 void
-cmMakefileTargetGenerator::AppendProgress(std::vector<std::string>& commands)
+cmMakefileTargetGenerator
+::MakeEchoProgress(cmLocalUnixMakefileGenerator3::EchoProgress& progress) const
 {
-  this->NumberOfProgressActions++;
-  if(this->NoRuleMessages)
-    {
-    return;
-    }
-  std::string progressDir = this->Makefile->GetHomeOutputDirectory();
-  progressDir += cmake::GetCMakeFilesDirectory();
-  std::ostringstream progCmd;
-  progCmd << "$(CMAKE_COMMAND) -E cmake_progress_report ";
-  progCmd << this->LocalGenerator->Convert(progressDir,
-                                           cmLocalGenerator::FULL,
-                                           cmLocalGenerator::SHELL);
-  progCmd << " $(CMAKE_PROGRESS_" << this->NumberOfProgressActions << ")";
-  commands.push_back(progCmd.str());
+  progress.Dir = this->Makefile->GetHomeOutputDirectory();
+  progress.Dir += cmake::GetCMakeFilesDirectory();
+  std::ostringstream progressArg;
+  progressArg << "$(CMAKE_PROGRESS_" << this->NumberOfProgressActions << ")";
+  progress.Arg = progressArg.str();
 }
 
 //----------------------------------------------------------------------------

Source/cmMakefileTargetGenerator.h

@@ -109,7 +109,7 @@ protected:
   void GenerateExtraOutput(const char* out, const char* in,
                            bool symbolic = false);
 
-  void AppendProgress(std::vector<std::string>& commands);
+  void MakeEchoProgress(cmLocalUnixMakefileGenerator3::EchoProgress&) const;
 
   // write out the variable that lists the objects for this target
   void WriteObjectsVariable(std::string& variableName,

Source/cmSystemTools.cxx

@@ -28,10 +28,10 @@
 # include "cmArchiveWrite.h"
 # include "cmLocale.h"
 # include <cm_libarchive.h>
-# include <cmsys/Terminal.h>
 #endif
 #include <cmsys/stl/algorithm>
 #include <cmsys/FStream.hxx>
+#include <cmsys/Terminal.h>
 
 #if defined(_WIN32)
 # include <windows.h>
@@ -2287,7 +2287,6 @@ std::string const& cmSystemTools::GetCMakeRoot()
 }
 
 //----------------------------------------------------------------------------
-#if defined(CMAKE_BUILD_WITH_CMAKE)
 void cmSystemTools::MakefileColorEcho(int color, const char* message,
                                       bool newline, bool enabled)
 {
@@ -2308,16 +2307,21 @@ void cmSystemTools::MakefileColorEcho(int color, const char* message,
 
   if(enabled)
     {
-    cmsysTerminal_cfprintf(color | assumeTTY, stdout, "%s%s",
-                           message, newline? "\n" : "");
+    // Print with color.  Delay the newline until later so that
+    // all color restore sequences appear before it.
+    cmsysTerminal_cfprintf(color | assumeTTY, stdout, "%s", message);
     }
   else
     {
     // Color is disabled.  Print without color.
-    fprintf(stdout, "%s%s", message, newline? "\n" : "");
+    fprintf(stdout, "%s", message);
+    }
+
+  if(newline)
+    {
+    fprintf(stdout, "\n");
     }
 }
-#endif
 
 //----------------------------------------------------------------------------
 bool cmSystemTools::GuessLibrarySOName(std::string const& fullPath,

Source/cmSystemTools.h

@@ -428,11 +428,9 @@ public:
   static std::string const& GetCMakeCursesCommand();
   static std::string const& GetCMakeRoot();
 
-#if defined(CMAKE_BUILD_WITH_CMAKE)
   /** Echo a message in color using KWSys's Terminal cprintf.  */
   static void MakefileColorEcho(int color, const char* message,
                                 bool newLine, bool enabled);
-#endif
 
   /** Try to guess the soname of a shared library.  */
   static bool GuessLibrarySOName(std::string const& fullPath,

Source/cmcmd.cxx

@@ -18,12 +18,12 @@
 
 #if defined(CMAKE_BUILD_WITH_CMAKE)
 # include "cmDependsFortran.h" // For -E cmake_copy_f90_mod callback.
-# include <cmsys/Terminal.h>
 #endif
 
 #include <cmsys/Directory.hxx>
 #include <cmsys/Process.h>
 #include <cmsys/FStream.hxx>
+#include <cmsys/Terminal.h>
 
 #if defined(CMAKE_HAVE_VS_GENERATORS)
 #include "cmCallVisualStudioMacro.h"
@@ -534,48 +534,9 @@ int cmcmd::ExecuteCMakeCommand(std::vector<std::string>& args)
     // Command to report progress for a build
     else if (args[1] == "cmake_progress_report" && args.size() >= 3)
       {
-      std::string dirName = args[2];
-      dirName += "/Progress";
-      std::string fName;
-      FILE *progFile;
-
-      // read the count
-      fName = dirName;
-      fName += "/count.txt";
-      progFile = cmsys::SystemTools::Fopen(fName,"r");
-      int count = 0;
-      if (!progFile)
-        {
-        return 0;
-        }
-      else
-        {
-        if (1!=fscanf(progFile,"%i",&count))
-          {
-          cmSystemTools::Message("Could not read from progress file.");
-          }
-        fclose(progFile);
-        }
-      unsigned int i;
-      for (i = 3; i < args.size(); ++i)
-        {
-        fName = dirName;
-        fName += "/";
-        fName += args[i];
-        progFile = cmsys::SystemTools::Fopen(fName,"w");
-        if (progFile)
-          {
-          fprintf(progFile,"empty");
-          fclose(progFile);
-          }
-        }
-      int fileNum = static_cast<int>
-        (cmsys::Directory::GetNumberOfFilesInDirectory(dirName));
-      if (count > 0)
-        {
-        // print the progress
-        fprintf(stdout,"[%3i%%] ",((fileNum-3)*100)/count);
-        }
+      // This has been superseded by cmake_echo_color --progress-*
+      // options.  We leave it here to avoid errors if somehow this
+      // is invoked by an existing makefile without regenerating.
       return 0;
       }
 
@@ -753,12 +714,12 @@ int cmcmd::ExecuteCMakeCommand(std::vector<std::string>& args)
       {
       return cmcmd::VisualStudioLink(args, 2);
       }
-#ifdef CMAKE_BUILD_WITH_CMAKE
     // Internal CMake color makefile support.
     else if (args[1] == "cmake_echo_color")
       {
       return cmcmd::ExecuteEchoColor(args);
       }
+#ifdef CMAKE_BUILD_WITH_CMAKE
     else if (args[1] == "cmake_autogen" && args.size() >= 4)
       {
         cmQtAutoGenerators autogen;
@@ -987,7 +948,65 @@ bool cmcmd::SymlinkInternal(std::string const& file, std::string const& link)
 }
 
 //----------------------------------------------------------------------------
-#ifdef CMAKE_BUILD_WITH_CMAKE
+static void cmcmdProgressReport(std::string const& dir,
+                                std::string const& num)
+{
+  std::string dirName = dir;
+  dirName += "/Progress";
+  std::string fName;
+  FILE *progFile;
+
+  // read the count
+  fName = dirName;
+  fName += "/count.txt";
+  progFile = cmsys::SystemTools::Fopen(fName,"r");
+  int count = 0;
+  if (!progFile)
+    {
+    return;
+    }
+  else
+    {
+    if (1!=fscanf(progFile,"%i",&count))
+      {
+      cmSystemTools::Message("Could not read from progress file.");
+      }
+    fclose(progFile);
+    }
+  const char* last = num.c_str();
+  for(const char* c = last;; ++c)
+    {
+    if (*c == ',' || *c == '\0')
+      {
+      if (c != last)
+        {
+        fName = dirName;
+        fName += "/";
+        fName.append(last, c-last);
+        progFile = cmsys::SystemTools::Fopen(fName,"w");
+        if (progFile)
+          {
+          fprintf(progFile,"empty");
+          fclose(progFile);
+          }
+        }
+      if(*c == '\0')
+        {
+        break;
+        }
+      last = c + 1;
+      }
+    }
+  int fileNum = static_cast<int>
+    (cmsys::Directory::GetNumberOfFilesInDirectory(dirName));
+  if (count > 0)
+    {
+    // print the progress
+    fprintf(stdout,"[%3i%%] ",((fileNum-3)*100)/count);
+    }
+}
+
+//----------------------------------------------------------------------------
 int cmcmd::ExecuteEchoColor(std::vector<std::string>& args)
 {
   // The arguments are
@@ -997,6 +1016,7 @@ int cmcmd::ExecuteEchoColor(std::vector<std::string>& args)
   bool enabled = true;
   int color = cmsysTerminal_Color_Normal;
   bool newline = true;
+  std::string progressDir;
   for(unsigned int i=2; i < args.size(); ++i)
     {
     if(args[i].find("--switch=") == 0)
@@ -1015,6 +1035,18 @@ int cmcmd::ExecuteEchoColor(std::vector<std::string>& args)
           }
         }
       }
+    else if(cmHasLiteralPrefix(args[i], "--progress-dir="))
+      {
+      progressDir = args[i].substr(15);
+      }
+    else if(cmHasLiteralPrefix(args[i], "--progress-num="))
+      {
+      if (!progressDir.empty())
+        {
+        std::string const& progressNum = args[i].substr(15);
+        cmcmdProgressReport(progressDir, progressNum);
+        }
+      }
     else if(args[i] == "--normal")
       {
       color = cmsysTerminal_Color_Normal;
@@ -1073,12 +1105,6 @@ int cmcmd::ExecuteEchoColor(std::vector<std::string>& args)
 
   return 0;
 }
-#else
-int cmcmd::ExecuteEchoColor(std::vector<std::string>&)
-{
-  return 1;
-}
-#endif
 
 //----------------------------------------------------------------------------
 int cmcmd::ExecuteLinkScript(std::vector<std::string>& args)

bootstrap

@@ -333,13 +333,15 @@ if ${cmake_system_mingw}; then
     EncodingC \
     ProcessWin32 \
     String \
-    System"
+    System \
+    Terminal"
 else
   KWSYS_C_SOURCES="\
     EncodingC \
     ProcessUNIX \
     String \
-    System"
+    System \
+    Terminal"
 fi
 
 KWSYS_CXX_SOURCES="\
@@ -362,7 +364,8 @@ KWSYS_FILES="\
   String.h \
   String.hxx \
   System.h \
-  SystemTools.hxx"
+  SystemTools.hxx \
+  Terminal.h"
 
 KWSYS_IOS_FILES="
   fstream \