Progress indicator: Unify "copying" and "substituting"

They're the same thing after all.

Example:

  $ nix build --store local?root=/tmp/nix nixpkgs.firefox-unwrapped
  [0/1 built, 49/98 copied, 16.3/92.8 MiB DL, 55.8/309.2 MiB copied] downloading 'https://cache.nixos.org/nar/0pl9li1jigcj2dany47hpmn0r3r48wc4nz48v5mqhh426lgz3bz6.nar.xz'

src/libstore/build.cc

@@ -207,7 +207,7 @@ struct MaintainCount
 {
     T & counter;
     T delta;
-    MaintainCount(T & counter, T delta) : counter(counter), delta(delta) { counter += delta; }
+    MaintainCount(T & counter, T delta = 1) : counter(counter), delta(delta) { counter += delta; }
     ~MaintainCount() { counter -= delta; }
 };
 
@@ -256,6 +256,7 @@ private:
 public:
 
     const Activity act;
+    const Activity actSubstitutions;
 
     /* Set if at least one derivation had a BuildError (i.e. permanent
        failure). */
@@ -268,6 +269,8 @@ public:
 
     std::unique_ptr<HookInstance> hook;
 
+    uint64_t expectedSubstitutions = 0;
+    uint64_t doneSubstitutions = 0;
     uint64_t expectedDownloadSize = 0;
     uint64_t doneDownloadSize = 0;
     uint64_t expectedNarSize = 0;
@@ -334,6 +337,7 @@ public:
 
     void updateProgress()
     {
+        actSubstitutions.progress(doneSubstitutions, expectedSubstitutions + doneSubstitutions);
         logger->event(evSetExpected, act, actDownload, expectedDownloadSize + doneDownloadSize);
         logger->event(evSetExpected, act, actCopyPath, expectedNarSize + doneNarSize);
     }
@@ -3328,7 +3332,8 @@ private:
        storePath when doing a repair. */
     Path destPath;
 
-    std::unique_ptr<MaintainCount<uint64_t>> maintainExpectedNar, maintainExpectedDownload;
+    std::unique_ptr<MaintainCount<uint64_t>> maintainExpectedSubstitutions,
+        maintainExpectedNar, maintainExpectedDownload;
 
     typedef void (SubstitutionGoal::*GoalState)();
     GoalState state;
@@ -3364,7 +3369,6 @@ public:
 
     void amDone(ExitCode result) override
     {
-        logger->event(evSubstitutionFinished, act, result == ecSuccess);
         Goal::amDone(result);
     }
 };
@@ -3379,7 +3383,7 @@ SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, Repa
     state = &SubstitutionGoal::init;
     name = (format("substitution of '%1%'") % storePath).str();
     trace("created");
-    logger->event(evSubstitutionCreated, act, storePath);
+    maintainExpectedSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.expectedSubstitutions);
 }
 
 
@@ -3527,8 +3531,6 @@ void SubstitutionGoal::tryToRun()
 
     printInfo(format("fetching path '%1%'...") % storePath);
 
-    logger->event(evSubstitutionStarted, act);
-
     outPipe.create();
 
     promise = std::promise<void>();
@@ -3576,6 +3578,9 @@ void SubstitutionGoal::finished()
     printMsg(lvlChatty,
         format("substitution of path '%1%' succeeded") % storePath);
 
+    maintainExpectedSubstitutions.reset();
+    worker.doneSubstitutions++;
+
     if (maintainExpectedDownload) {
         auto fileSize = maintainExpectedDownload->delta;
         maintainExpectedDownload.reset();
@@ -3610,6 +3615,7 @@ static bool working = false;
 
 Worker::Worker(LocalStore & store)
     : act(actRealise)
+    , actSubstitutions(actCopyPaths)
     , store(store)
 {
     /* Debugging: prevent recursive workers. */
@@ -3632,6 +3638,7 @@ Worker::~Worker()
        their destructors). */
     topGoals.clear();
 
+    assert(expectedSubstitutions == 0);
     assert(expectedDownloadSize == 0);
     assert(expectedNarSize == 0);
 }

src/libstore/store-api.cc

@@ -621,16 +621,13 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, const PathSet & storePa
     for (auto & path : storePaths)
         if (!valid.count(path)) missing.insert(path);
 
-    Activity act(actUnknown);
+    Activity act(actCopyPaths, fmt("copying %d paths", missing.size()));
 
-    logger->event(evCopyStarted, act);
+    std::atomic<size_t> nrDone{0};
-
-    std::atomic<size_t> nrCopied{0};
-    std::atomic<size_t> nrDone{storePaths.size() - missing.size()};
     std::atomic<uint64_t> bytesExpected{0};
 
     auto showProgress = [&]() {
-        logger->event(evCopyProgress, act, storePaths.size(), nrCopied, nrDone);
+        act.progress(nrDone, missing.size());
     };
 
     ThreadPool pool;
@@ -659,11 +656,9 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, const PathSet & storePa
             if (!dstStore->isValidPath(storePath)) {
                 printInfo("copying '%s'...", storePath);
                 copyStorePath(srcStore, dstStore, storePath, repair, checkSigs);
-                nrCopied++;
             }
 
             nrDone++;
-
             showProgress();
         });
 }

src/libutil/logging.hh

@@ -18,6 +18,7 @@ typedef enum {
     actCopyPath = 100,
     actDownload = 101,
     actRealise = 102,
+    actCopyPaths = 103,
 } ActivityType;
 
 class Activity
@@ -32,7 +33,7 @@ public:
     ~Activity();
 
     template<typename... Args>
-    void progress(const Args & ... args);
+    void progress(const Args & ... args) const;
 };
 
 typedef enum {
@@ -40,12 +41,6 @@ typedef enum {
     evBuildStarted = 1,
     evBuildOutput = 2,
     evBuildFinished = 3,
-    evSubstitutionCreated = 8,
-    evSubstitutionStarted = 9,
-    evSubstitutionFinished = 10,
-
-    evCopyStarted = 100,
-    evCopyProgress = 101,
 
     evStartActivity = 1000,
     evStopActivity = 1001,
@@ -152,7 +147,7 @@ void warnOnce(bool & haveWarned, const FormatOrString & fs);
 void writeToStderr(const string & s);
 
 template<typename... Args>
-void Activity::progress(const Args & ... args)
+void Activity::progress(const Args & ... args) const
 {
     Event ev;
     ev.type = evProgress;

src/nix/progress-bar.cc

@@ -25,13 +25,6 @@ private:
         std::map<ActivityType, uint64_t> expectedByType;
     };
 
-    struct CopyInfo
-    {
-        uint64_t expected = 0;
-        uint64_t copied = 0;
-        uint64_t done = 0;
-    };
-
     struct ActivitiesByType
     {
         std::map<Activity::t, std::list<ActInfo>::iterator> its;
@@ -46,12 +39,6 @@ private:
         uint64_t succeededBuilds = 0;
         uint64_t failedBuilds = 0;
 
-        std::map<Activity::t, Path> substitutions;
-        std::set<Activity::t> runningSubstitutions;
-        uint64_t succeededSubstitutions = 0;
-
-        std::map<Activity::t, CopyInfo> runningCopies;
-
         std::list<ActInfo> activities;
         std::map<Activity::t, std::list<ActInfo>::iterator> its;
 
@@ -185,25 +172,7 @@ public:
                 state.succeededBuilds, state.succeededBuilds + state.builds.size());
         }
 
-        if (!state.substitutions.empty() || state.succeededSubstitutions) {
+        auto showActivity = [&](ActivityType type, const std::string & f, double unit) {
-            if (!res.empty()) res += ", ";
-            if (!state.runningSubstitutions.empty())
-                res += fmt(ANSI_BLUE "%d" "/" ANSI_NORMAL, state.runningSubstitutions.size());
-            res += fmt(ANSI_GREEN "%d/%d fetched" ANSI_NORMAL,
-                state.succeededSubstitutions,
-                state.succeededSubstitutions + state.substitutions.size());
-        }
-
-        if (!state.runningCopies.empty()) {
-            uint64_t copied = 0, expected = 0;
-            for (auto & i : state.runningCopies) {
-                copied += i.second.copied;
-                expected += i.second.expected - (i.second.done - i.second.copied);
-            }
-            add(fmt("%d/%d copied", copied, expected));
-        }
-
-        auto showActivity = [&](ActivityType type, const std::string & f) {
             auto & act = state.activitiesByType[type];
             uint64_t done = act.done, expected = act.done;
             for (auto & j : act.its) {
@@ -214,11 +183,12 @@ public:
             expected = std::max(expected, act.expected);
 
             if (done || expected)
-                add(fmt(f, done / MiB, expected / MiB));
+                add(fmt(f, done / unit, expected / unit));
         };
 
-        showActivity(actDownload, "%1$.1f/%2$.1f MiB DL");
+        showActivity(actCopyPaths, ANSI_GREEN "%d" ANSI_NORMAL "/%d copied", 1);
-        showActivity(actCopyPath, "%1$.1f/%2$.1f MiB copied");
+        showActivity(actDownload, "%1$.1f/%2$.1f MiB DL", MiB);
+        showActivity(actCopyPath, "%1$.1f/%2$.1f MiB copied", MiB);
 
         return res;
     }
@@ -287,36 +257,6 @@ public:
             updateActivity(*state, act, ev.getS(1));
         }
 
-        if (ev.type == evSubstitutionCreated) {
-            state->substitutions[ev.getI(0)] = ev.getS(1);
-        }
-
-        if (ev.type == evSubstitutionStarted) {
-            Activity::t act = ev.getI(0);
-            state->runningSubstitutions.insert(act);
-            auto name = storePathToName(state->substitutions[act]);
-            createActivity(*state, act, fmt("fetching " ANSI_BOLD "%s" ANSI_NORMAL, name));
-        }
-
-        if (ev.type == evSubstitutionFinished) {
-            Activity::t act = ev.getI(0);
-            if (ev.getI(1)) {
-                if (state->runningSubstitutions.count(act))
-                    state->succeededSubstitutions++;
-            }
-            state->runningSubstitutions.erase(act);
-            state->substitutions.erase(act);
-            deleteActivity(*state, act);
-        }
-
-        if (ev.type == evCopyProgress) {
-            Activity::t act = ev.getI(0);
-            auto & i = state->runningCopies[act];
-            i.expected = ev.getI(1);
-            i.copied = ev.getI(2);
-            i.done = ev.getI(3);
-        }
-
         update(*state);
     }
 };