tree-wide: unify progress bar inactive and paused states

Previously, the progress bar had two subtly different states in which the bar
would not actually render, both with their own shortcomings: inactive (which
was irreversible) and paused (reversible, but swallowing logs). Furthermore,
there was no way of resetting the statistics, so a very bad solution was
implemented (243c0f18dae2a08ea0e46f7ff33277c63f7506d7) that would create a new
logger for each line of the repl, leaking the previous one and discarding the
value of printBuildLogs. Finally, if stderr was not attached to a TTY, the
update thread was started even though the logger was not active, violating the
invariant required by the destructor (which is not observed because the logger
is leaked).

In this commit, the two aforementioned states are unified into a single one,
which can be exited again, correctly upholds the invariant that the update
thread is only running while the progress bar is active, and does not swallow
logs. The latter change in behavior is not expected to be a problems in the
rare cases where the paused state was used before, since other loggers (like
the simple one) don't exhibit it anyway. The startProgressBar/stopProgressBar
API is removed due to being a footgun, and a new method for properly resetting
the progress is added.

Co-Authored-By: Qyriad <qyriad@qyriad.me>
Change-Id: I2b7c3eb17d439cd0c16f7b896cfb61239ac7ff3a

13 files changed, 19 insertions, 33 deletions

diff --git a/src/nix/build.cc b/src/nix/build.cc
index 479100186..6de52c0b6 100644
--- a/src/nix/build.cc
+++ b/src/nix/build.cc
@@ -3,7 +3,6 @@
 #include "shared.hh"
 #include "store-api.hh"
 #include "local-fs-store.hh"
-#include "progress-bar.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -143,7 +142,7 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile
                 createOutLinks(outLink, buildables, *store2);
 
         if (printOutputPaths) {
-            stopProgressBar();
+            logger->pause();
             for (auto & buildable : buildables) {
                 std::visit(overloaded {
                     [&](const BuiltPath::Opaque & bo) {
diff --git a/src/nix/cat.cc b/src/nix/cat.cc
index 678edd9a1..81c21e2ad 100644
--- a/src/nix/cat.cc
+++ b/src/nix/cat.cc
@@ -2,7 +2,6 @@
 #include "store-api.hh"
 #include "fs-accessor.hh"
 #include "nar-accessor.hh"
-#include "progress-bar.hh"
 
 using namespace nix;
 
@@ -20,7 +19,7 @@ struct MixCat : virtual Args
 
         auto file = accessor->readFile(path);
 
-        stopProgressBar();
+        logger->pause();
         writeFull(STDOUT_FILENO, file);
     }
 };
diff --git a/src/nix/develop.cc b/src/nix/develop.cc
index 353bf0110..fb144c904 100644
--- a/src/nix/develop.cc
+++ b/src/nix/develop.cc
@@ -6,7 +6,6 @@
 #include "store-api.hh"
 #include "outputs-spec.hh"
 #include "derivations.hh"
-#include "progress-bar.hh"
 #include "run.hh"
 
 #include <iterator>
@@ -690,7 +689,7 @@ struct CmdPrintDevEnv : Common, MixJSON
     {
         auto buildEnvironment = getBuildEnvironment(store, installable).first;
 
-        stopProgressBar();
+        logger->pause();
 
         if (json) {
             logger->writeToStdout(buildEnvironment.toJSON());
diff --git a/src/nix/dump-path.cc b/src/nix/dump-path.cc
index fb32dddb7..99ff05dcc 100644
--- a/src/nix/dump-path.cc
+++ b/src/nix/dump-path.cc
@@ -1,7 +1,6 @@
 #include "command.hh"
 #include "store-api.hh"
 #include "archive.hh"
-#include "progress-bar.hh"
 
 using namespace nix;
 
@@ -21,7 +20,7 @@ struct CmdDumpPath : StorePathCommand
 
     void run(ref<Store> store, const StorePath & storePath) override
     {
-        stopProgressBar();
+        logger->pause();
         FdSink sink(STDOUT_FILENO);
         store->narFromPath(storePath, sink);
         sink.flush();
@@ -57,7 +56,7 @@ struct CmdDumpPath2 : Command
 
     void run() override
     {
-        stopProgressBar();
+        logger->pause();
         FdSink sink(STDOUT_FILENO);
         dumpPath(path, sink);
         sink.flush();
diff --git a/src/nix/edit.cc b/src/nix/edit.cc
index 2f701f145..8352c26e8 100644
--- a/src/nix/edit.cc
+++ b/src/nix/edit.cc
@@ -2,7 +2,6 @@
 #include "shared.hh"
 #include "eval.hh"
 #include "attr-path.hh"
-#include "progress-bar.hh"
 #include "editor-for.hh"
 #include "current-process.hh"
 
@@ -42,7 +41,7 @@ struct CmdEdit : InstallableCommand
             }
         }();
 
-        stopProgressBar();
+        logger->pause();
 
         auto args = editorFor(file, line);
 
diff --git a/src/nix/eval.cc b/src/nix/eval.cc
index 9f265930b..a027b9a58 100644
--- a/src/nix/eval.cc
+++ b/src/nix/eval.cc
@@ -6,7 +6,6 @@
 #include "eval.hh"
 #include "eval-inline.hh"
 #include "value-to-json.hh"
-#include "progress-bar.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -76,7 +75,7 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption
         }
 
         if (writeTo) {
-            stopProgressBar();
+            logger->pause();
 
             if (pathExists(*writeTo))
                 throw Error("path '%s' already exists", *writeTo);
@@ -114,7 +113,7 @@ struct CmdEval : MixJSON, InstallableCommand, MixReadOnlyOption
         }
 
         else if (raw) {
-            stopProgressBar();
+            logger->pause();
             writeFull(STDOUT_FILENO, *state->coerceToString(noPos, *v, context, "while generating the eval command output"));
         }
 
diff --git a/src/nix/log.cc b/src/nix/log.cc
index 9a9bd30f9..b291489b5 100644
--- a/src/nix/log.cc
+++ b/src/nix/log.cc
@@ -3,7 +3,6 @@
 #include "shared.hh"
 #include "store-api.hh"
 #include "log-store.hh"
-#include "progress-bar.hh"
 
 using namespace nix;
 
@@ -55,7 +54,7 @@ struct CmdLog : InstallableCommand
 
             auto log = logSub.getBuildLog(path);
             if (!log) continue;
-            stopProgressBar();
+            logger->pause();
             printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri());
             writeFull(STDOUT_FILENO, *log);
             return;
diff --git a/src/nix/main.cc b/src/nix/main.cc
index 55f8d59ba..2f52a352f 100644
--- a/src/nix/main.cc
+++ b/src/nix/main.cc
@@ -346,8 +346,6 @@ void mainWrapped(int argc, char * * argv)
     }
     #endif
 
-    Finally f([] { logger->stop(); });
-
     programPath = argv[0];
     auto programName = std::string(baseNameOf(programPath));
 
@@ -363,7 +361,8 @@ void mainWrapped(int argc, char * * argv)
 
     evalSettings.pureEval = true;
 
-    setLogFormat("bar");
+    setLogFormat(LogFormat::bar);
+    Finally f([] { logger->pause(); });
     settings.verboseBuild = false;
     if (isatty(STDERR_FILENO)) {
         verbosity = lvlNotice;
diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc
index cad70e726..13d94d645 100644
--- a/src/nix/prefetch.cc
+++ b/src/nix/prefetch.cc
@@ -1,10 +1,10 @@
 #include "command.hh"
 #include "common-args.hh"
+#include "loggers.hh"
 #include "shared.hh"
 #include "store-api.hh"
 #include "filetransfer.hh"
 #include "finally.hh"
-#include "progress-bar.hh"
 #include "tarfile.hh"
 #include "attr-path.hh"
 #include "eval-inline.hh"
@@ -180,10 +180,8 @@ static int main_nix_prefetch_url(int argc, char * * argv)
         if (args.size() > 2)
             throw UsageError("too many arguments");
 
-        Finally f([]() { stopProgressBar(); });
-
         if (isatty(STDERR_FILENO))
-          startProgressBar();
+            setLogFormat(LogFormat::bar);
 
         auto store = openStore();
         auto state = std::make_unique<EvalState>(myArgs.searchPath, store);
@@ -237,7 +235,7 @@ static int main_nix_prefetch_url(int argc, char * * argv)
         auto [storePath, hash] = prefetchFile(
             store, resolveMirrorUrl(*state, url), name, ht, expectedHash, unpack, executable);
 
-        stopProgressBar();
+        logger->pause();
 
         if (!printPath)
             printInfo("path is '%s'", store->printStorePath(storePath));
diff --git a/src/nix/run.cc b/src/nix/run.cc
index 1e4406df5..824201fdf 100644
--- a/src/nix/run.cc
+++ b/src/nix/run.cc
@@ -8,7 +8,6 @@
 #include "local-store.hh"
 #include "finally.hh"
 #include "fs-accessor.hh"
-#include "progress-bar.hh"
 #include "eval.hh"
 #include "build/personality.hh"
 #include "current-process.hh"
@@ -31,7 +30,7 @@ void runProgramInStore(ref<Store> store,
     const Strings & args,
     std::optional<std::string_view> system)
 {
-    stopProgressBar();
+    logger->pause();
 
     restoreProcessContext();
 
diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc
index eeb14e29a..948844e22 100644
--- a/src/nix/sigs.cc
+++ b/src/nix/sigs.cc
@@ -3,7 +3,6 @@
 #include "store-api.hh"
 #include "thread-pool.hh"
 #include "signals.hh"
-#include "progress-bar.hh"
 
 #include <atomic>
 
@@ -222,7 +221,7 @@ struct CmdKey : NixMultiCommand
         if (!command)
             throw UsageError("'nix key' requires a sub-command.");
 
-        stopProgressBar();
+        logger->pause();
         command->second->run();
     }
 };
diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc
index c7f31f3fb..371879791 100644
--- a/src/nix/upgrade-nix.cc
+++ b/src/nix/upgrade-nix.cc
@@ -13,7 +13,6 @@
 #include "eval-settings.hh"
 #include "attr-path.hh"
 #include "names.hh"
-#include "progress-bar.hh"
 
 using namespace nix;
 
@@ -88,7 +87,7 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand
         auto version = DrvName(storePath.name()).version;
 
         if (dryRun) {
-            stopProgressBar();
+            logger->pause();
             warn("would upgrade to version %s", version);
             return;
         }
@@ -106,7 +105,7 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand
                 throw Error("could not verify that '%s' works", program);
         }
 
-        stopProgressBar();
+        logger->pause();
 
         auto const fullStorePath = store->printStorePath(storePath);
 
diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc
index 055cf6d0d..5bef11c4d 100644
--- a/src/nix/why-depends.cc
+++ b/src/nix/why-depends.cc
@@ -1,6 +1,5 @@
 #include "command.hh"
 #include "store-api.hh"
-#include "progress-bar.hh"
 #include "fs-accessor.hh"
 #include "shared.hh"
 
@@ -110,7 +109,7 @@ struct CmdWhyDepends : SourceExprCommand, MixOperateOnOptions
         auto dependencyPath = *optDependencyPath;
         auto dependencyPathHash = dependencyPath.hashPart();
 
-        stopProgressBar(); // FIXME
+        logger->pause(); // FIXME
 
         auto accessor = store->getFSAccessor();