Progress indicator: Cleanup

7 files changed, 105 insertions, 151 deletions

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index 257f02a49196..1a1fc1dee54d 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -335,8 +335,8 @@ public:
     {
         actDerivations.progress(doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds);
         actSubstitutions.progress(doneSubstitutions, expectedSubstitutions + doneSubstitutions, runningSubstitutions, failedSubstitutions);
-        logger->event(evSetExpected, act, actDownload, expectedDownloadSize + doneDownloadSize);
-        logger->event(evSetExpected, act, actCopyPath, expectedNarSize + doneNarSize);
+        act.setExpected(actDownload, expectedDownloadSize + doneDownloadSize);
+        act.setExpected(actCopyPath, expectedNarSize + doneNarSize);
     }
 };
 
@@ -1386,7 +1386,7 @@ void DerivationGoal::tryToBuild()
     bool buildLocally = buildMode != bmNormal || drv->willBuildLocally();
 
     auto started = [&]() {
-        act = std::make_unique<Activity>(actBuild, fmt("building '%s'", drvPath));
+        act = std::make_unique<Activity>(*logger, actBuild, fmt("building '%s'", drvPath));
         mcRunningBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.runningBuilds);
         worker.updateProgress();
     };
@@ -3257,7 +3257,7 @@ void DerivationGoal::flushLine()
         logTail.push_back(currentLogLine);
         if (logTail.size() > settings.logLines) logTail.pop_front();
     }
-    logger->event(evBuildOutput, *act, currentLogLine);
+    act->progress(currentLogLine);
     currentLogLine = "";
     currentLogLinePos = 0;
 }
@@ -3647,9 +3647,9 @@ static bool working = false;
 
 
 Worker::Worker(LocalStore & store)
-    : act(actRealise)
-    , actDerivations(actBuilds)
-    , actSubstitutions(actCopyPaths)
+    : act(*logger, actRealise)
+    , actDerivations(*logger, actBuilds)
+    , actSubstitutions(*logger, actCopyPaths)
     , store(store)
 {
     /* Debugging: prevent recursive workers. */
diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index 9ac4a65d5706..71b720a3b59e 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -85,7 +85,7 @@ struct CurlDownloader : public Downloader
         DownloadItem(CurlDownloader & downloader, const DownloadRequest & request)
             : downloader(downloader)
             , request(request)
-            , act(actDownload, fmt("downloading '%s'", request.uri))
+            , act(*logger, actDownload, fmt("downloading '%s'", request.uri))
         {
             if (!request.expectedETag.empty())
                 requestHeaders = curl_slist_append(requestHeaders, ("If-None-Match: " + request.expectedETag).c_str());
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 5aa3dcfd26a3..f520210615a3 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -565,7 +565,7 @@ void Store::buildPaths(const PathSet & paths, BuildMode buildMode)
 void copyStorePath(ref<Store> srcStore, ref<Store> dstStore,
     const Path & storePath, RepairFlag repair, CheckSigsFlag checkSigs)
 {
-    Activity act(actCopyPath, fmt("copying path '%s'", storePath));
+    Activity act(*logger, actCopyPath, fmt("copying path '%s'", storePath));
 
     auto info = srcStore->queryPathInfo(storePath);
 
@@ -621,7 +621,7 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, const PathSet & storePa
     for (auto & path : storePaths)
         if (!valid.count(path)) missing.insert(path);
 
-    Activity act(actCopyPaths, fmt("copying %d paths", missing.size()));
+    Activity act(*logger, actCopyPaths, fmt("copying %d paths", missing.size()));
 
     std::atomic<size_t> nrDone{0};
     std::atomic<uint64_t> bytesExpected{0};
@@ -646,7 +646,7 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, const PathSet & storePa
             auto info = srcStore->queryPathInfo(storePath);
 
             bytesExpected += info->narSize;
-            logger->event(evSetExpected, act, actCopyPath, bytesExpected);
+            act.setExpected(actCopyPath, bytesExpected);
 
             return info->references;
         },
@@ -655,7 +655,6 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, const PathSet & storePa
             checkInterrupt();
 
             if (!dstStore->isValidPath(storePath)) {
-                printInfo("copying '%s'...", storePath);
                 MaintainCount<decltype(nrRunning)> mc(nrRunning);
                 showProgress();
                 copyStorePath(srcStore, dstStore, storePath, repair, checkSigs);
diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc
index ed83770a14aa..900f24e4cbdf 100644
--- a/src/libutil/logging.cc
+++ b/src/libutil/logging.cc
@@ -43,10 +43,6 @@ public:
 
         writeToStderr(prefix + (tty ? fs.s : filterANSIEscapes(fs.s)) + "\n");
     }
-
-    void event(const Event & ev) override
-    {
-    }
 };
 
 Verbosity verbosity = lvlInfo;
@@ -78,22 +74,10 @@ Logger * makeDefaultLogger()
 
 std::atomic<uint64_t> nextId{(uint64_t) getpid() << 32};
 
-Activity::Activity() : id(nextId++) { };
-
-Activity::Activity(ActivityType type, std::string msg)
-    : Activity()
-{
-    logger->event(evStartActivity, id, type, msg);
-}
-
-Activity::~Activity()
-{
-    logger->event(evStopActivity, id);
-}
-
-void Activity::progress(uint64_t done, uint64_t expected, uint64_t running, uint64_t failed) const
+Activity::Activity(Logger & logger, ActivityType type, const std::string & s)
+    : logger(logger), id(nextId++)
 {
-    logger->event(evProgress, id, done, expected, running, failed);
+    logger.startActivity(id, type, s);
 }
 
 }
diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh
index 097d486eff9c..edc3553f6942 100644
--- a/src/libutil/logging.hh
+++ b/src/libutil/logging.hh
@@ -23,63 +23,7 @@ typedef enum {
     actBuild = 105,
 } ActivityType;
 
-class Activity
-{
-public:
-    typedef uint64_t t;
-    const t id;
-    Activity();
-    Activity(const Activity & act) : id(act.id) { };
-    Activity(uint64_t id) : id(id) { };
-    Activity(ActivityType type, std::string msg = "");
-    ~Activity();
-
-    void progress(uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) const;
-};
-
-typedef enum {
-    evBuildOutput = 2,
-
-    evStartActivity = 1000,
-    evStopActivity = 1001,
-    evProgress = 1002,
-    evSetExpected = 1003,
-
-} EventType;
-
-struct Event
-{
-    struct Field
-    {
-        // FIXME: use std::variant.
-        enum { tInt, tString } type;
-        uint64_t i = 0;
-        std::string s;
-        Field(const std::string & s) : type(tString), s(s) { }
-        Field(const char * s) : type(tString), s(s) { }
-        Field(const uint64_t & i) : type(tInt), i(i) { }
-        Field(const Activity & act) : type(tInt), i(act.id) { }
-    };
-
-    typedef std::vector<Field> Fields;
-
-    EventType type;
-    Fields fields;
-
-    std::string getS(size_t n) const
-    {
-        assert(n < fields.size());
-        assert(fields[n].type == Field::tString);
-        return fields[n].s;
-    }
-
-    uint64_t getI(size_t n) const
-    {
-        assert(n < fields.size());
-        assert(fields[n].type == Field::tInt);
-        return fields[n].i;
-    }
-};
+typedef uint64_t ActivityId;
 
 class Logger
 {
@@ -98,16 +42,38 @@ public:
 
     virtual void warn(const std::string & msg);
 
-    template<typename... Args>
-    void event(EventType type, const Args & ... args)
-    {
-        Event ev;
-        ev.type = type;
-        nop{(ev.fields.emplace_back(Event::Field(args)), 1)...};
-        event(ev);
-    }
+    virtual void startActivity(ActivityId act, ActivityType type, const std::string & s) { };
+
+    virtual void stopActivity(ActivityId act) { };
+
+    virtual void progress(ActivityId act, uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) { };
+
+    virtual void progress(ActivityId act, const std::string & s) { };
+
+    virtual void setExpected(ActivityId act, ActivityType type, uint64_t expected) { };
+};
+
+struct Activity
+{
+    Logger & logger;
+
+    const ActivityId id;
+
+    Activity(Logger & logger, ActivityType type, const std::string & s = "");
+
+    ~Activity()
+    { logger.stopActivity(id); }
+
+    void progress(uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) const
+    { logger.progress(id, done, expected, running, failed); }
+
+    void progress(const std::string & s) const
+    { logger.progress(id, s); }
+
+    void setExpected(ActivityType type2, uint64_t expected) const
+    { logger.setExpected(id, type2, expected); }
 
-    virtual void event(const Event & ev) = 0;
+    friend class Logger;
 };
 
 extern Logger * logger;
diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc
index 2b223c882647..7e6f3aa25b5e 100644
--- a/src/nix-daemon/nix-daemon.cc
+++ b/src/nix-daemon/nix-daemon.cc
@@ -81,10 +81,6 @@ class TunnelLogger : public Logger
         } else
             defaultLogger->log(lvl, fs);
     }
-
-    void event(const Event & ev) override
-    {
-    }
 };
 
 
diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc
index c79a0d6d1db9..6288737088a2 100644
--- a/src/nix/progress-bar.cc
+++ b/src/nix/progress-bar.cc
@@ -29,7 +29,7 @@ private:
 
     struct ActivitiesByType
     {
-        std::map<Activity::t, std::list<ActInfo>::iterator> its;
+        std::map<ActivityId, std::list<ActInfo>::iterator> its;
         uint64_t done = 0;
         uint64_t expected = 0;
         uint64_t failed = 0;
@@ -38,7 +38,7 @@ private:
     struct State
     {
         std::list<ActInfo> activities;
-        std::map<Activity::t, std::list<ActInfo>::iterator> its;
+        std::map<ActivityId, std::list<ActInfo>::iterator> its;
 
         std::map<ActivityType, ActivitiesByType> activitiesByType;
     };
@@ -77,7 +77,61 @@ public:
         update(state);
     }
 
-    void createActivity(State & state, Activity::t activity, const std::string & s, ActivityType type = actUnknown)
+    void startActivity(ActivityId act, ActivityType type, const std::string & s) override
+    {
+        auto state(state_.lock());
+        createActivity(*state, act, s, type);
+        update(*state);
+    }
+
+    void stopActivity(ActivityId act) override
+    {
+        auto state(state_.lock());
+        deleteActivity(*state, act);
+        update(*state);
+    }
+
+    void progress(ActivityId act, uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) override
+    {
+        auto state(state_.lock());
+
+        auto i = state->its.find(act);
+        assert(i != state->its.end());
+        ActInfo & actInfo = *i->second;
+        actInfo.done = done;
+        actInfo.expected = expected;
+        actInfo.running = running;
+        actInfo.failed = failed;
+
+        update(*state);
+    }
+
+    void progress(ActivityId act, const std::string & s) override
+    {
+        auto state(state_.lock());
+        auto s2 = trim(s);
+        if (!s2.empty()) {
+            updateActivity(*state, act, s2);
+            update(*state);
+        }
+    }
+
+    void setExpected(ActivityId act, ActivityType type, uint64_t expected) override
+    {
+        auto state(state_.lock());
+
+        auto i = state->its.find(act);
+        assert(i != state->its.end());
+        ActInfo & actInfo = *i->second;
+        auto & j = actInfo.expectedByType[type];
+        state->activitiesByType[type].expected -= j;
+        j = expected;
+        state->activitiesByType[type].expected += j;
+
+        update(*state);
+    }
+
+    void createActivity(State & state, ActivityId activity, const std::string & s, ActivityType type = actUnknown)
     {
         state.activities.emplace_back(ActInfo{s, "", type});
         auto i = std::prev(state.activities.end());
@@ -85,7 +139,7 @@ public:
         state.activitiesByType[type].its.emplace(activity, i);
     }
 
-    void deleteActivity(State & state, Activity::t activity)
+    void deleteActivity(State & state, ActivityId activity)
     {
         auto i = state.its.find(activity);
         if (i != state.its.end()) {
@@ -102,7 +156,7 @@ public:
         }
     }
 
-    void updateActivity(State & state, Activity::t activity, const std::string & s2)
+    void updateActivity(State & state, ActivityId activity, const std::string & s2)
     {
         auto i = state.its.find(activity);
         assert(i != state.its.end());
@@ -210,51 +264,6 @@ public:
 
         return res;
     }
-
-    void event(const Event & ev) override
-    {
-        auto state(state_.lock());
-
-        if (ev.type == evStartActivity) {
-            Activity::t act = ev.getI(0);
-            createActivity(*state, act, ev.getS(2), (ActivityType) ev.getI(1));
-        }
-
-        if (ev.type == evStopActivity) {
-            Activity::t act = ev.getI(0);
-            deleteActivity(*state, act);
-        }
-
-        if (ev.type == evProgress) {
-            auto i = state->its.find(ev.getI(0));
-            assert(i != state->its.end());
-            ActInfo & actInfo = *i->second;
-            actInfo.done = ev.getI(1);
-            actInfo.expected = ev.getI(2);
-            actInfo.running = ev.getI(3);
-            actInfo.failed = ev.getI(4);
-        }
-
-        if (ev.type == evSetExpected) {
-            auto i = state->its.find(ev.getI(0));
-            assert(i != state->its.end());
-            ActInfo & actInfo = *i->second;
-            auto type = (ActivityType) ev.getI(1);
-            auto & j = actInfo.expectedByType[type];
-            state->activitiesByType[type].expected -= j;
-            j = ev.getI(2);
-            state->activitiesByType[type].expected += j;
-        }
-
-        if (ev.type == evBuildOutput) {
-            Activity::t act = ev.getI(0);
-            auto s = trim(ev.getS(1));
-            if (!s.empty())
-                updateActivity(*state, act, s);
-        }
-
-        update(*state);
-    }
 };
 
 StartProgressBar::StartProgressBar()