Add support for --no-progress option

14 files changed, 341 insertions, 147 deletions

diff --git a/bdep/bdep.cxx b/bdep/bdep.cxx
index 6e17a3f..00d70f7 100644
--- a/bdep/bdep.cxx
+++ b/bdep/bdep.cxx
@@ -36,6 +36,7 @@
 #include <bdep/clean.hxx>
 
 using namespace std;
+using namespace butl;
 using namespace bdep;
 
 namespace bdep
@@ -157,6 +158,8 @@ try
 {
   using namespace cli;
 
+  stderr_term = fdterm (stderr_fd ());
+
   argv0 = argv[0];
   exec_dir = path (argv0).directory ();
 
diff --git a/bdep/ci.cxx b/bdep/ci.cxx
index 304818d..08193b3 100644
--- a/bdep/ci.cxx
+++ b/bdep/ci.cxx
@@ -253,7 +253,7 @@ namespace bdep
     {
       // Print progress unless we had a prompt.
       //
-      if (verb && o.yes ())
+      if (verb && o.yes () && !o.no_progress ())
         text << "submitting to " << srv;
 
       url u (srv);
diff --git a/bdep/common.cli b/bdep/common.cli
index 4736159..2db2677 100644
--- a/bdep/common.cli
+++ b/bdep/common.cli
@@ -90,6 +90,23 @@ namespace bdep
        system."
     }
 
+    // When it comes to external programs (such as curl, git, etc), if stderr
+    // is not a terminal, the logic is actually tri-state: With --no-progress
+    // we suppress any progress. With --progress (which we may add in the
+    // future), we request full progress. Finally, without any --*progress
+    // options we let the external program decide what to do: it may do
+    // something intelligent (like curl) and produce non-terminal-friendly
+    // progress (such as status lines printed periodically) or it may disable
+    // progress all together (like git). Of course, it may also do no
+    // detection and dump non-terminal-unfriendly progress in which case we
+    // should probably do the detection ourselves and suppress it.
+    //
+    bool --no-progress
+    {
+      "Suppress progress indicators for long-lasting operations, such as
+       network transfers, building, etc."
+    }
+
     path --bpkg
     {
       "<path>",
diff --git a/bdep/git.cxx b/bdep/git.cxx
index 311a557..91964aa 100644
--- a/bdep/git.cxx
+++ b/bdep/git.cxx
@@ -257,22 +257,24 @@ namespace bdep
     // git-status --porcelain=2 (available since git 2.11.0) gives us all the
     // information with a single invocation.
     //
-    process pr;
+    fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
+
+    process pr (start_git (semantic_version {2, 11, 0},
+                           repo,
+                           0    /* stdin  */,
+                           pipe /* stdout */,
+                           2    /* stderr */,
+                           "status",
+                           "--porcelain=2",
+                           "--branch"));
+
+    // Shouldn't throw, unless something is severely damaged.
+    //
+    pipe.out.close ();
+
     bool io (false);
     try
     {
-      fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate.
-
-      pr = start_git (semantic_version {2, 11, 0},
-                      repo,
-                      0    /* stdin  */,
-                      pipe /* stdout */,
-                      2    /* stderr */,
-                      "status",
-                      "--porcelain=2",
-                      "--branch");
-
-      pipe.out.close ();
       ifdstream is (move (pipe.in), fdstream_mode::skip, ifdstream::badbit);
 
       // Lines starting with '#' are headers (come first) with any other line
diff --git a/bdep/git.hxx b/bdep/git.hxx
index 9f968dd..e699947 100644
--- a/bdep/git.hxx
+++ b/bdep/git.hxx
@@ -9,6 +9,7 @@
 
 #include <bdep/types.hxx>
 #include <bdep/utility.hxx>
+#include <bdep/common-options.hxx>
 
 namespace bdep
 {
@@ -39,7 +40,17 @@ namespace bdep
   //
   template <typename... A>
   void
-  run_git (const semantic_version&, const dir_path& repo, A&&... args);
+  run_git (const semantic_version&,
+           bool progress,
+           const dir_path& repo,
+           A&&... args);
+
+  template <typename... A>
+  inline void
+  run_git (const semantic_version& min_ver, const dir_path& repo, A&&... args)
+  {
+    run_git (min_ver, true /* progress */, repo, forward<A> (args)...);
+  }
 
   // Return the first line of the git output. If ignore_error is true, then
   // suppress stderr, ignore (normal) error exit status, and return nullopt.
@@ -95,6 +106,12 @@ namespace bdep
   //
   git_repository_status
   git_status (const dir_path& repo);
+
+  // Run the git push command.
+  //
+  template <typename... A>
+  void
+  git_push (const common_options&, const dir_path& repo, A&&... args);
 }
 
 #include <bdep/git.ixx>
diff --git a/bdep/git.txx b/bdep/git.txx
index 4624102..5ff5e6c 100644
--- a/bdep/git.txx
+++ b/bdep/git.txx
@@ -6,21 +6,69 @@ namespace bdep
 {
   template <typename... A>
   void
-  run_git (const semantic_version& min_ver, const dir_path& repo, A&&... args)
+  run_git (const semantic_version& min_ver,
+           bool progress,
+           const dir_path& repo,
+           A&&... args)
   {
-    // We don't expect git to print anything to stdout, as the caller would use
-    // start_git() and pipe otherwise. Thus, let's redirect stdout to stderr
-    // for good measure, as git is known to print some informational messages
-    // to stdout.
+    // Unfortunately git doesn't have any kind of a no-progress option but
+    // suppresses progress automatically for a non-terminal. So we use this
+    // feature for the progress suppression by redirecting git's stderr to our
+    // own diagnostics stream via a proxy pipe.
+    //
+    fdpipe pipe;
+
+    if (!progress)
+      pipe = open_pipe ();
+
+    int err (!progress ? pipe.out.get () : 2);
+
+    // We don't expect git to print anything to stdout, as the caller would
+    // use start_git() and pipe otherwise. Thus, let's redirect stdout to
+    // stderr for good measure, as git is known to print some informational
+    // messages to stdout.
     //
     process pr (start_git (min_ver,
                            repo,
-                           0 /* stdin  */,
-                           2 /* stdout */,
-                           2 /* stderr */,
+                           0   /* stdin  */,
+                           err /* stdout */,
+                           err /* stderr */,
                            forward<A> (args)...));
 
-    finish_git (pr);
+    bool io (false);
+
+    if (!progress)
+    {
+      // Shouldn't throw, unless something is severely damaged.
+      //
+      pipe.out.close ();
+
+      try
+      {
+        ifdstream is (move (pipe.in), fdstream_mode::skip, ifdstream::badbit);
+
+        // We could probably write something like this, instead:
+        //
+        // *diag_stream << is.rdbuf () << flush;
+        //
+        // However, it would never throw and we could potentially miss the
+        // reading failure.
+        //
+        for (string l; !eof (getline (is, l)); )
+          *diag_stream << l << std::endl;
+
+        is.close ();
+      }
+      catch (const io_error&)
+      {
+        // Presumably the child process failed and issued diagnostics so let
+        // finish_git() try to deal with that.
+        //
+        io = true;
+      }
+    }
+
+    finish_git (pr, io);
   }
 
   void
@@ -43,8 +91,8 @@ namespace bdep
   optional<string>
   git_line (const semantic_version& min_ver, bool ie, A&&... args)
   {
-    fdpipe pipe (fdopen_pipe ());
-    auto_fd null (ie ? fdnull () : auto_fd ());
+    fdpipe pipe (open_pipe ());
+    auto_fd null (ie ? open_dev_null () : auto_fd ());
 
     process pr (start_git (min_ver,
                            0                    /* stdin  */,
@@ -54,4 +102,39 @@ namespace bdep
 
     return git_line (move (pr), move (pipe), ie);
   }
+
+  template <typename... A>
+  void
+  git_push (const common_options& o, const dir_path& repo, A&&... args)
+  {
+    // Map verbosity level. Suppress the (too detailed) push command output if
+    // the verbosity level is 1. However, we still want to see the progress in
+    // this case, unless we were asked to suppress it (git also suppress
+    // progress for a non-terminal stderr).
+    //
+    cstrings v;
+    bool progress (!o.no_progress ());
+
+    if (verb < 2)
+    {
+      v.push_back ("-q");
+
+      if (progress)
+      {
+        if (verb == 1 && stderr_term)
+          v.push_back ("--progress");
+      }
+      else
+        progress = true; // No need to suppress (already done with -q).
+    }
+    else if (verb > 3)
+      v.push_back ("-v");
+
+    run_git (semantic_version {2, 1, 0},
+             progress,
+             repo,
+             "push",
+             v,
+             forward<A> (args)...);
+  }
 }
diff --git a/bdep/http-service.cxx b/bdep/http-service.cxx
index 59598c0..8b4f059 100644
--- a/bdep/http-service.cxx
+++ b/bdep/http-service.cxx
@@ -53,6 +53,52 @@ namespace bdep
       //
       optional<url> location;
 
+      // Map the verbosity level.
+      //
+      cstrings v;
+      bool progress (!o.no_progress ());
+
+      auto suppress_progress = [&v] ()
+      {
+        v.push_back ("-s");
+        v.push_back ("-S"); // But show errors.
+      };
+
+      if (verb < 1)
+      {
+        suppress_progress ();
+        progress = true;      // No need to suppress (already done).
+      }
+      else if (verb == 1 && fdterm (2))
+      {
+        if (progress)
+          v.push_back ("--progress-bar");
+      }
+      else if (verb > 3)
+        v.push_back ("-v");
+
+      // Suppress progress.
+      //
+      // Note: the `-v -s` options combination is valid and results in a
+      // verbose output without progress.
+      //
+      if (!progress)
+        suppress_progress ();
+
+      // Convert the submit arguments to curl's --form* options.
+      //
+      strings fos;
+      for (const parameter& p: params)
+      {
+        fos.emplace_back (p.type == parameter::file
+                          ? "--form"
+                          : "--form-string");
+
+        fos.emplace_back (p.type == parameter::file
+                          ? p.name + "=@" + p.value
+                          : p.name + "="  + p.value);
+      }
+
       // Note that it's a bad idea to issue the diagnostics while curl is
       // running, as it will be messed up with the progress output. Thus, we
       // throw the runtime_error exception on the HTTP response parsing error
@@ -63,65 +109,39 @@ namespace bdep
       // running curl over using butl::curl because in this context it is
       // restrictive and inconvenient.
       //
-      process pr;
-      bool io (false);
-      try
-      {
-        // Map the verbosity level.
-        //
-        cstrings v;
-        if (verb < 1)
-        {
-          v.push_back ("-s");
-          v.push_back ("-S"); // But show errors.
-        }
-        else if (verb == 1 && fdterm (2))
-          v.push_back ("--progress-bar");
-        else if (verb > 3)
-          v.push_back ("-v");
-
-        // Convert the submit arguments to curl's --form* options.
-        //
-        strings fos;
-        for (const parameter& p: params)
-        {
-          fos.emplace_back (p.type == parameter::file
-                            ? "--form"
-                            : "--form-string");
-
-          fos.emplace_back (p.type == parameter::file
-                            ? p.name + "=@" + p.value
-                            : p.name + "="  + p.value);
-        }
-
-        // Start curl program.
-        //
-        fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate.
+      // Start curl program.
+      //
+      fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
 
-        // Note that we don't specify any default timeouts, assuming that bdep
-        // is an interactive program and the user can always interrupt the
-        // command (or pass the timeout with --curl-option).
-        //
-        pr = start (0          /* stdin  */,
-                    pipe       /* stdout */,
-                    2          /* stderr */,
-                    o.curl (),
-                    v,
-                    "-A", (BDEP_USER_AGENT " curl"),
+      // Note that we don't specify any default timeouts, assuming that bdep
+      // is an interactive program and the user can always interrupt the
+      // command (or pass the timeout with --curl-option).
+      //
+      process pr (start (0          /* stdin  */,
+                         pipe       /* stdout */,
+                         2          /* stderr */,
+                         o.curl (),
+                         v,
+                         "-A", (BDEP_USER_AGENT " curl"),
 
-                    o.curl_option (),
+                         o.curl_option (),
 
-                    // Include the response headers in the output so we can
-                    // get the status code/reason, content type, and the
-                    // redirect location.
-                    //
-                    "--include",
+                         // Include the response headers in the output so we
+                         // can get the status code/reason, content type, and
+                         // the redirect location.
+                         //
+                         "--include",
 
-                    fos,
-                    u.string ());
+                         fos,
+                         u.string ()));
 
-        pipe.out.close ();
+      // Shouldn't throw, unless something is severely damaged.
+      //
+      pipe.out.close ();
 
+      bool io (false);
+      try
+      {
         // First we read the HTTP response status line and headers. At this
         // stage we will read until the empty line (containing just CRLF). Not
         // being able to reach such a line is an error, which is the reason
diff --git a/bdep/http-service.hxx b/bdep/http-service.hxx
index 247fe94..75fd603 100644
--- a/bdep/http-service.hxx
+++ b/bdep/http-service.hxx
@@ -52,7 +52,7 @@ namespace bdep
     // present, then it is included in the diagnostics.
     //
     result
-    post (const common_options& o, const url&, const parameters&);
+    post (const common_options&, const url&, const parameters&);
   }
 }
 
diff --git a/bdep/publish.cxx b/bdep/publish.cxx
index c97c652..d3ea1c0 100644
--- a/bdep/publish.cxx
+++ b/bdep/publish.cxx
@@ -273,26 +273,28 @@ namespace bdep
       // Verify that archive name/content all match and while at it extract
       // its manifest.
       //
-      process pr;
+      fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
+
+      // Pass the --deep option to make sure that the *-file manifest values
+      // are resolvable, so rep-create will not fail due to this package
+      // down the road.
+      //
+      process pr (start_bpkg (2    /* verbosity */,
+                              o,
+                              pipe /* stdout */,
+                              2    /* stderr */,
+                              "pkg-verify",
+                              "--deep",
+                              "--manifest",
+                              a));
+
+      // Shouldn't throw, unless something is severely damaged.
+      //
+      pipe.out.close ();
+
       bool io (false);
       try
       {
-        fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate.
-
-        // Pass the --deep option to make sure that the *-file manifest values
-        // are resolvable, so rep-create will not fail due to this package
-        // down the road.
-        //
-        pr = start_bpkg (2    /* verbosity */,
-                         o,
-                         pipe /* stdout */,
-                         2    /* stderr */,
-                         "pkg-verify",
-                         "--deep",
-                         "--manifest",
-                         a);
-
-        pipe.out.close ();
         ifdstream is (move (pipe.in), fdstream_mode::skip);
 
         manifest_parser mp (is, manifest_file.string ());
@@ -375,7 +377,7 @@ namespace bdep
       auto worktree_add = [&prj, &wd] ()
       {
         bool q (verb < 2);
-        auto_fd null (q ? fdnull () : auto_fd ());
+        auto_fd null (q ? open_dev_null () : auto_fd ());
 
         process pr (start_git (git_ver,
                                prj,
@@ -461,14 +463,14 @@ namespace bdep
         {
           // Create the empty tree object.
           //
-          auto_fd null (fdnull ());
-          fdpipe pipe (fdopen_pipe ());
+          auto_fd null (open_dev_null ());
+          fdpipe pipe (open_pipe ());
 
           process pr (start_git (git_ver,
                                  prj,
-                                 null.get () /* stdin  */,
-                                 pipe        /* stdout */,
-                                 2           /* stderr */,
+                                 null /* stdin  */,
+                                 pipe /* stdout */,
+                                 2    /* stderr */,
                                  "hash-object",
                                  "-wt", "tree",
                                  "--stdin"));
@@ -729,23 +731,14 @@ namespace bdep
               }
             }));
 
-        if (verb)
-          text << "pushing build2-control";
-
-        // Note that we suppress the (too detailed) push command output if
-        // the verbosity level is 1. However, we still want to see the
-        // progress in this case.
-        //
-        run_git (git_ver,
-                 wd,
-                 "push",
+        if (verb && !o.no_progress ())
+          text << "pushing branch build2-control";
 
-                 verb < 2 ? "-q" : verb > 3 ? "-v" : nullptr,
-                 verb == 1 ? "--progress" : nullptr,
-
-                 !remote_exists
-                 ? cstrings ({"--set-upstream", "origin", "build2-control"})
-                 : cstrings ());
+        git_push (o,
+                  wd,
+                  (!remote_exists
+                   ? cstrings ({"--set-upstream", "origin", "build2-control"})
+                   : cstrings ()));
       }
 
       worktree_remove ();
@@ -758,7 +751,7 @@ namespace bdep
       // The path points into the temporary directory so let's omit the
       // directory part.
       //
-      if (verb)
+      if (verb && !o.no_progress ())
         text << "submitting " << p.archive.leaf ();
 
       url u (o.repository ());
diff --git a/bdep/release.cxx b/bdep/release.cxx
index cb4cac9..e550d32 100644
--- a/bdep/release.cxx
+++ b/bdep/release.cxx
@@ -888,17 +888,20 @@ namespace bdep
         brspec  = st.branch + ':' + string (st.upstream, p + 1);
       }
 
-      // Note that we suppress the (too detailed) push command output if
-      // the verbosity level is 1. However, we still want to see the
-      // progress in this case.
-      //
-      run_git (git_ver,
-               prj.path,
-               "push",
-               verb < 1 ? "-q" : verb >= 2 ? "-v" : nullptr,
-               remote,
-               brspec,
-               !tagspec.empty () ? tagspec.c_str () : nullptr);
+      if (verb && !o.no_progress ())
+      {
+        diag_record dr (text);
+        dr << "pushing branch " << st.branch;
+
+        if (prj.tag)
+          dr << ", tag " << *prj.tag;
+      }
+
+      git_push (o,
+                prj.path,
+                remote,
+                brspec,
+                !tagspec.empty () ? tagspec.c_str () : nullptr);
     }
 
     return 0;
diff --git a/bdep/sync.cxx b/bdep/sync.cxx
index a8514ab..d4f3983 100644
--- a/bdep/sync.cxx
+++ b/bdep/sync.cxx
@@ -33,20 +33,22 @@ namespace bdep
 
     // Use bpkg-rep-list to discover the list of project directories.
     //
-    process pr;
+    fdpipe pipe (open_pipe ()); // Text mode seems appropriate.
+
+    process pr (start_bpkg (3,
+                            co,
+                            pipe /* stdout */,
+                            2    /* stderr */,
+                            "rep-list",
+                            "-d", cfg));
+
+    // Shouldn't throw, unless something is severely damaged.
+    //
+    pipe.out.close ();
+
     bool io (false);
     try
     {
-      fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate.
-
-      pr = start_bpkg (3,
-                       co,
-                       pipe /* stdout */,
-                       2    /* stderr */,
-                       "rep-list",
-                       "-d", cfg);
-
-      pipe.out.close ();
       ifdstream is (move (pipe.in), fdstream_mode::skip, ifdstream::badbit);
 
       for (string l; !eof (getline (is, l)); )
diff --git a/bdep/utility.cxx b/bdep/utility.cxx
index b670587..8caedc6 100644
--- a/bdep/utility.cxx
+++ b/bdep/utility.cxx
@@ -80,6 +80,8 @@ namespace bdep
     }
   }
 
+  bool stderr_term;
+
   bool
   exists (const path& f, bool ignore_error)
   {
@@ -174,6 +176,32 @@ namespace bdep
     }
   }
 
+  fdpipe
+  open_pipe ()
+  {
+    try
+    {
+      return fdopen_pipe ();
+    }
+    catch (const io_error& e)
+    {
+      fail << "unable to open pipe: " << e << endf;
+    }
+  }
+
+  auto_fd
+  open_dev_null ()
+  {
+    try
+    {
+      return fdnull ();
+    }
+    catch (const io_error& e)
+    {
+      fail << "unable to open null device: " << e << endf;
+    }
+  }
+
   const char*
   name_bpkg (const common_options& co)
   {
diff --git a/bdep/utility.hxx b/bdep/utility.hxx
index fcf0963..c4d406c 100644
--- a/bdep/utility.hxx
+++ b/bdep/utility.hxx
@@ -67,11 +67,6 @@ namespace bdep
   using butl::auto_rmfile;
   using butl::auto_rmdir;
 
-  // <libbutl/fdstream.mxx>
-  //
-  using butl::fdnull;
-  using butl::fdopen_pipe;
-
   // Empty string and path.
   //
   extern const string   empty_string;
@@ -119,6 +114,10 @@ namespace bdep
   //
   extern dir_path exec_dir;
 
+  // Progress.
+  //
+  extern bool stderr_term; // True if stderr is a terminal.
+
   // Filesystem.
   //
   bool
@@ -147,6 +146,14 @@ namespace bdep
         uint16_t verbosity = 3,
         rm_error_mode = rm_error_mode::fail);
 
+  // File descriptor streams.
+  //
+  fdpipe
+  open_pipe ();
+
+  auto_fd
+  open_dev_null ();
+
   // Run a process.
   //
   template <typename I, typename O, typename E, typename P, typename... A>
diff --git a/bdep/utility.txx b/bdep/utility.txx
index 4686ba2..2927002 100644
--- a/bdep/utility.txx
+++ b/bdep/utility.txx
@@ -2,6 +2,7 @@
 // copyright : Copyright (c) 2014-2019 Code Synthesis Ltd
 // license   : MIT; see accompanying LICENSE file
 
+#include <cstring>  // strcmp()
 #include <iostream> // cin
 
 #include <bdep/diagnostics.hxx>
@@ -91,6 +92,7 @@ namespace bdep
       string vl;
       {
         const char* o (nullptr);
+        bool progress (!co.no_progress ());
 
         switch (verb)
         {
@@ -106,7 +108,15 @@ namespace bdep
         }
 
         if (o != nullptr)
+        {
           ops.push_back (o);
+
+          if (strcmp (o, "-q") == 0)
+            progress = true; // No need to suppress (already done with -q).
+        }
+
+        if (!progress)
+          ops.push_back ("--no-progress");
       }
 
       // Forward our --build* options.
@@ -180,6 +190,7 @@ namespace bdep
       string vl;
       {
         const char* o (nullptr);
+        bool progress (!co.no_progress ());
 
         switch (verb)
         {
@@ -195,7 +206,15 @@ namespace bdep
         }
 
         if (o != nullptr)
+        {
           ops.push_back (o);
+
+          if (strcmp (o, "-q") == 0)
+            progress = true; // No need to suppress (already done with -q).
+        }
+
+        if (!progress)
+          ops.push_back ("--no-progress");
       }
 
       return process_start_callback (