Add support for --no-progress option

5 files changed, 256 insertions, 94 deletions

diff --git a/bpkg/common.cli b/bpkg/common.cli
index 31580c0..4090ad3 100644
--- a/bpkg/common.cli
+++ b/bpkg/common.cli
@@ -100,6 +100,23 @@ namespace bpkg
        a command."
     }
 
+    // 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 --build
     {
       "<path>",
@@ -138,6 +155,14 @@ namespace bpkg
        \cb{wget}, and \cb{fetch}."
     }
 
+    strings --fetch-option
+    {
+      "<opt>",
+      "Additional option to be passed to the fetch program. See \cb{--fetch}
+       for more information on the fetch program. Repeat this option to
+       specify multiple fetch options."
+    }
+
     size_t --fetch-timeout
     {
       "<sec>",
@@ -157,14 +182,6 @@ namespace bpkg
        programs."
     }
 
-    strings --fetch-option
-    {
-      "<opt>",
-      "Additional option to be passed to the fetch program. See \cb{--fetch}
-       for more information on the fetch program. Repeat this option to
-       specify multiple fetch options."
-    }
-
     path --git = "git"
     {
       "<path>",
diff --git a/bpkg/fetch-git.cxx b/bpkg/fetch-git.cxx
index 1695bcf..ab3255c 100644
--- a/bpkg/fetch-git.cxx
+++ b/bpkg/fetch-git.cxx
@@ -35,8 +35,6 @@ namespace bpkg
 
   static const diag_noreturn_end<fail_git> endg;
 
-  using opt = optional<const char*>; // Program option.
-
   static strings
   timeout_opts (const common_options& co, repository_protocol proto)
   {
@@ -209,19 +207,77 @@ namespace bpkg
     }
   }
 
-  // Run git process.
+  // Run git process, optionally suppressing progress.
   //
   template <typename... A>
   static process_exit
-  run_git (const common_options& co, A&&... args)
+  run_git (const common_options& co, bool progress, A&&... args)
   {
+    // Unfortunately git doesn't have any kind of a no-progress option. The
+    // only way to suppress progress is to run quiet (-q) which also
+    // suppresses some potentially useful information. However, git suppresses
+    // progress automatically if its stderr is not a 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 ();
+
     process pr (start_git (co,
-                           1 /* stdout */, 2 /* stderr */,
+                           1                               /* stdout */,
+                           !progress ? pipe.out.get () : 2 /* stderr */,
                            forward<A> (args)...));
+
+    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, unless we decide to additionally mess with the
+        // diagnostics stream exception mask.
+        //
+        for (string l; !eof (getline (is, l)); )
+          *diag_stream << l << endl;
+
+        is.close ();
+
+        // Fall through.
+      }
+      catch (const io_error& e)
+      {
+        // Fail if git exited normally with zero code, so the issue won't go
+        // unnoticed. Otherwise, let the caller handle git's failure.
+        //
+        if (pr.wait ())
+          fail << "unable to read git diagnostics: " << e;
+
+        // Fall through.
+      }
+    }
+
     pr.wait ();
     return *pr.exit;
   }
 
+  template <typename... A>
+  static process_exit
+  run_git (const common_options& co, A&&... args)
+  {
+    return run_git (co, true /* progress */, forward<A> (args)...);
+  }
+
   // Run git process and return it's output as a string. Fail if the output
   // doesn't contain a single line.
   //
@@ -695,12 +751,15 @@ namespace bpkg
     if (i != repository_refs.end ())
       return i->second;
 
-    if (verb)
+    if (verb && !co.no_progress ())
       text << "querying " << url;
 
     refs rs;
     fdpipe pipe (open_pipe ());
 
+    // Note: ls-remote doesn't print anything to stderr, so no progress
+    // suppression is required.
+    //
     process pr (start_git (co,
                            pipe, 2 /* stderr */,
                            timeout_opts (co, url.scheme),
@@ -823,7 +882,7 @@ namespace bpkg
           ? strings ({"--separate-git-dir=" + git_dir.string ()})
           : strings (),
 
-          verb < 2 ? opt ("-q") : nullopt,
+          verb < 2 ? "-q" : nullptr,
           dir))
       fail << "unable to init " << dir << endg;
 
@@ -1262,27 +1321,44 @@ namespace bpkg
         }
       };
 
-      // Note that we suppress the (too detailed) fetch command output if the
-      // verbosity level is 1. However, we still want to see the progress in
-      // this case, unless stderr is not directed to a terminal.
+      // Map verbosity level. Suppress the (too detailed) fetch 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 vos;
+      bool progress (!co.no_progress ());
+
+      if (verb < 2)
+      {
+        vos.push_back ("-q");
+
+        if (progress)
+        {
+          if (verb == 1 && stderr_term)
+            vos.push_back ("--progress");
+        }
+        else
+          progress = true; // Already suppressed with -q.
+      }
+      else if (verb > 3)
+        vos.push_back ("-v");
+
       // Also note that we don't need to specify --refmap option since we can
       // rely on the init() function that properly sets the
       // remote.origin.fetch configuration option.
       //
       if (!run_git (co,
+                    progress,
                     timeout_opts (co, url ().scheme),
                     co.git_option (),
                     "-C", dir,
                     "fetch",
                     "--no-recurse-submodules",
-
-                    shallow         ? cstrings ({"--depth", "1"}) :
-                    shallow_repo () ? cstrings ({"--unshallow"})  :
-                    cstrings (),
-
-                    verb == 1 && fdterm (2) ? opt ("--progress") : nullopt,
-                    verb < 2 ? opt ("-q") : verb > 3 ? opt ("-v") : nullopt,
+                    (shallow         ? cstrings ({"--depth", "1"}) :
+                     shallow_repo () ? cstrings ({"--unshallow"})  :
+                     cstrings ()),
+                    vos,
                     "origin",
                     remapped_refspecs ? *remapped_refspecs : refspecs))
         fail << "unable to fetch " << dir << endg;
@@ -1294,72 +1370,74 @@ namespace bpkg
         fail << "unable to test if " << dir << " is shallow" << endg;
     };
 
-    // Print the progress indicator.
-    //
-    // Note that the clone command prints the following line prior to the
-    // progress lines:
-    //
-    // Cloning into '<dir>'...
-    //
-    // The fetch command doesn't print anything similar, for some reason.
-    // This makes it hard to understand which superproject/submodule is
-    // currently being fetched. Let's fix that.
+    // Print progress.
     //
-    // Also note that we have "fixed" that capital letter nonsense and stripped
-    // the trailing '...'.
-    //
-    if (verb)
+    if (verb && !co.no_progress ())
     {
-      diag_record dr (text);
-      dr << "fetching ";
+      // Note that the clone command prints the following line prior to the
+      // progress lines:
+      //
+      // Cloning into '<dir>'...
+      //
+      // The fetch command doesn't print anything similar, for some reason.
+      // This makes it hard to understand which superproject/submodule is
+      // currently being fetched. Let's fix that.
+      //
+      // Also note that we have "fixed" that capital letter nonsense and
+      // stripped the trailing '...'.
+      //
+      {
+        diag_record dr (text);
+        dr << "fetching ";
 
-      if (!submodule.empty ())
-        dr << "submodule '" << submodule.posix_string () << "' ";
+        if (!submodule.empty ())
+          dr << "submodule '" << submodule.posix_string () << "' ";
 
-      dr << "from " << url ();
+        dr << "from " << url ();
 
-      if (verb >= 2)
-        dr << " in '" << dir.posix_string () << "'"; // Is used by tests.
-    }
+        if (verb >= 2)
+          dr << " in '" << dir.posix_string () << "'"; // Is used by tests.
+      }
 
-    // First, we perform the deep fetching.
-    //
-    if (fetch_repo || !dcs.empty ())
-    {
       // Print warnings prior to the deep fetching.
       //
+      if (fetch_repo || !dcs.empty ())
       {
-        diag_record dr (warn);
-        dr << "fetching whole " << (fetch_repo ? "repository" : "reference")
-           << " history";
-
-        if (!submodule.empty ())
-          dr << " for submodule '" << submodule.posix_string () << "'";
+        {
+          diag_record dr (warn);
+          dr << "fetching whole " << (fetch_repo ? "repository" : "reference")
+             << " history";
+
+          if (!submodule.empty ())
+            dr << " for submodule '" << submodule.posix_string () << "'";
+
+          dr << " ("
+             << (caps () == capabilities::dumb
+                 ? "dumb HTTP"
+                 : "unadvertised commit") // There are no other reasons so far.
+             << ')';
+        }
 
-        dr << " ("
-           << (caps () == capabilities::dumb
-               ? "dumb HTTP"
-               : "unadvertised commit") // There are no other reasons so far.
-           << ')';
+        if (caps () == capabilities::dumb)
+          warn << "no progress will be shown (dumb HTTP)";
       }
+    }
 
-      if (caps () == capabilities::dumb)
-        warn << "no progress will be shown (dumb HTTP)";
-
-      // Fetch.
-      //
-      fetch (fetch_repo ? strings () : dcs, false);
+    // Fetch.
+    //
+    // First, we perform the deep fetching.
+    //
+    fetch (fetch_repo ? strings () : dcs, false);
 
-      // After the deep fetching some of the shallow commits might also be
-      // fetched, so we drop them from the fetch list.
-      //
-      for (auto i (scs.begin ()); i != scs.end (); )
-      {
-        if (commit_fetched (co, dir, *i))
-          i = scs.erase (i);
-        else
-          ++i;
-      }
+    // After the deep fetching some of the shallow commits might also be
+    // fetched, so we drop them from the fetch list.
+    //
+    for (auto i (scs.begin ()); i != scs.end (); )
+    {
+      if (commit_fetched (co, dir, *i))
+        i = scs.erase (i);
+      else
+        ++i;
     }
 
     // Finally, we perform the shallow fetching.
@@ -1425,7 +1503,7 @@ namespace bpkg
           : strings (),
 
           "submodule--helper", "init",
-          verb < 2 ? opt ("-q") : nullopt))
+          verb < 2 ? "-q" : nullptr))
       failure ("unable to initialize submodules");
 
     repository_url orig_url (origin_url (co, dir));
@@ -1589,7 +1667,7 @@ namespace bpkg
         // Let's make the message match the git-submodule script output
         // (again, except for capitalization).
         //
-        if (verb)
+        if (verb && !co.no_progress ())
           text << "submodule path '" << psd << "': checked out '" << commit
                << "'";
 
@@ -1716,7 +1794,7 @@ namespace bpkg
                            "-C", dir,
                            "reset",
                            "--hard",
-                           verb < 2 ? opt ("-q") : nullopt,
+                           verb < 2 ? "-q" : nullptr,
                            commit));
     if (!pr.wait ())
       fail << "unable to reset to " << commit << endg;
@@ -1729,7 +1807,7 @@ namespace bpkg
           "-d",
           "-x",
           "-ff",
-          verb < 2 ? opt ("-q") : nullopt))
+          verb < 2 ? "-q" : nullptr))
       fail << "unable to clean " << dir << endg;
   }
 
diff --git a/bpkg/fetch.cxx b/bpkg/fetch.cxx
index 1aaff37..aaabffe 100644
--- a/bpkg/fetch.cxx
+++ b/bpkg/fetch.cxx
@@ -91,6 +91,7 @@ namespace bpkg
   static process
   start_wget (const path& prog,
               const optional<size_t>& timeout,
+              bool no_progress,
               const strings& ops,
               const string& url,
               const path& out)
@@ -119,7 +120,10 @@ namespace bpkg
     // user re-runs the command with -v to see all the gory details.
     //
     if (verb < (fo ? 1 : 2))
+    {
       args.push_back ("-q");
+      no_progress = false; // Already suppressed with -q.
+    }
     else if (fo && verb == 1)
     {
       // Wget 1.16 introduced the --show-progress option which in the
@@ -129,12 +133,24 @@ namespace bpkg
       if (wget_major > 1 || (wget_major == 1 && wget_minor >= 16))
       {
         args.push_back ("-q");
-        args.push_back ("--show-progress");
+
+        if (!no_progress)
+          args.push_back ("--show-progress");
+        else
+          no_progress = false; // Already suppressed with -q.
       }
     }
     else if (verb > 3)
       args.push_back ("-d");
 
+    // Suppress progress.
+    //
+    // Note: the `--no-verbose -d` options combination is valid and results in
+    // debug messages with the progress meter suppressed.
+    //
+    if (no_progress)
+      args.push_back ("--no-verbose");
+
     // Set download timeout if requested.
     //
     string tm;
@@ -224,6 +240,7 @@ namespace bpkg
   static process
   start_curl (const path& prog,
               const optional<size_t>& timeout,
+              bool no_progress,
               const strings& ops,
               const string& url,
               const path& out)
@@ -237,6 +254,12 @@ namespace bpkg
       "-A", (BPKG_USER_AGENT " curl")
     };
 
+    auto suppress_progress = [&args] ()
+    {
+      args.push_back ("-s");
+      args.push_back ("-S"); // But show errors.
+    };
+
     // Map verbosity level. If we are running quiet or at level 1
     // and the output is stdout, then run curl quiet. If at level
     // 1 and the output is a file, then show the progress bar. At
@@ -246,14 +269,25 @@ namespace bpkg
     //
     if (verb < (fo ? 1 : 2))
     {
-      args.push_back ("-s");
-      args.push_back ("-S"); // But show errors.
+      suppress_progress ();
+      no_progress = false; // Already suppressed.
     }
     else if (fo && verb == 1)
-      args.push_back ("--progress-bar");
+    {
+      if (!no_progress)
+        args.push_back ("--progress-bar");
+    }
     else if (verb > 3)
       args.push_back ("-v");
 
+    // Suppress progress.
+    //
+    // Note: the `-v -s` options combination is valid and results in a verbose
+    // output without progress.
+    //
+    if (no_progress)
+      suppress_progress ();
+
     // Set download timeout if requested.
     //
     string tm;
@@ -285,7 +319,7 @@ namespace bpkg
 
     if (verb >= 2)
       print_process (args);
-    else if (verb == 1 && fo)
+    else if (verb == 1 && fo && !no_progress)
       //
       // Unfortunately curl doesn't print the filename being fetched
       // next to the progress bar. So the best we can do is print it
@@ -350,6 +384,7 @@ namespace bpkg
   static process
   start_fetch (const path& prog,
                const optional<size_t>& timeout,
+               bool no_progress,
                const strings& ops,
                const string& url,
                const path& out)
@@ -369,10 +404,28 @@ namespace bpkg
     // level 2 or 3, then run it at the default level (so it will display
     // the progress). Higher than that -- run it verbose.
     //
+    // Note that the only way to suppress progress for the fetch program is to
+    // run it quiet (-q). However, it prints nothing but the progress by
+    // default and some additional information in the verbose mode (-v).
+    // Therefore, if the progress suppression is requested we will run quiet
+    // unless the verbosity level is greater than three, in which case we will
+    // run verbose (and with progress). That's the best we can do.
+    //
     if (verb < (fo ? 1 : 2))
+    {
       args.push_back ("-q");
+      no_progress = false;   // Already suppressed with -q.
+    }
     else if (verb > 3)
+    {
       args.push_back ("-v");
+      no_progress = false; // Don't be quiet in the verbose mode (see above).
+    }
+
+    // Suppress progress.
+    //
+    if (no_progress)
+      args.push_back ("-q");
 
     // Set download timeout if requested.
     //
@@ -417,8 +470,8 @@ namespace bpkg
 
   // The dispatcher.
   //
-  // Cache the result of finding/testing the fetch program. Sometimes
-  // a simple global variable is really the right solution...
+  // Cache the result of finding/testing the fetch program. Sometimes a simple
+  // global variable is really the right solution...
   //
   enum kind {wget, curl, fetch};
 
@@ -509,6 +562,7 @@ namespace bpkg
   {
     process (*f) (const path&,
                   const optional<size_t>&,
+                  bool,
                   const strings&,
                   const string&,
                   const path&) = nullptr;
@@ -526,7 +580,8 @@ namespace bpkg
 
     try
     {
-      return f (fetch_path, timeout, o.fetch_option (), url, out);
+      return f (
+        fetch_path, timeout, o.no_progress (), o.fetch_option (), url, out);
     }
     catch (const process_error& e)
     {
diff --git a/bpkg/pkg-checkout.cxx b/bpkg/pkg-checkout.cxx
index 6e0f92a..a91cb92 100644
--- a/bpkg/pkg-checkout.cxx
+++ b/bpkg/pkg-checkout.cxx
@@ -42,7 +42,7 @@ namespace bpkg
           // Print the progress indicator to attribute the possible fetching
           // progress.
           //
-          if (verb)
+          if (verb && !o.no_progress ())
             text << "checking out "
                  << package_string (ap->id.name, ap->version);
 
@@ -194,7 +194,7 @@ namespace bpkg
       // At verbosity level 1 we want our (nicer) progress header but the
       // build system's actual progress.
       //
-      if (verb == 1)
+      if (verb == 1 && !o.no_progress ())
         text << "distributing " << n << '/' << v;
 
       run_b (o,
diff --git a/bpkg/utility.txx b/bpkg/utility.txx
index 93699ab..71701bc 100644
--- a/bpkg/utility.txx
+++ b/bpkg/utility.txx
@@ -36,17 +36,26 @@ namespace bpkg
       // as us.
       //
       string vl;
+      bool no_progress (co.no_progress ());
 
       if (verb == 0)
+      {
         ops.push_back ("-q");
+        no_progress = false;  // Already suppressed with -q.
+      }
       else if (verb == 1)
       {
         if (v != verb_b::normal)
         {
           ops.push_back ("-q");
 
-          if (v == verb_b::progress && stderr_term)
-            ops.push_back ("--progress");
+          if (!no_progress)
+          {
+            if (v == verb_b::progress && stderr_term)
+              ops.push_back ("--progress");
+          }
+          else
+            no_progress = false; // Already suppressed with -q.
         }
       }
       else if (verb == 2)
@@ -58,6 +67,9 @@ namespace bpkg
         ops.push_back (vl.c_str ());
       }
 
+      if (no_progress)
+        ops.push_back ("--no-progress");
+
       return process_start_callback (
         [] (const char* const args[], size_t n)
         {