From c137c0a5ebc0d58c53f86986ab66967ac8629cbe Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Aug 2017 17:49:40 +0200 Subject: [PATCH] Allow activities to be nested In particular, this allows more relevant activities ("substituting X") to supersede inferior ones ("downloading X"). --- src/libstore/build.cc | 9 ++++++--- src/libstore/download.cc | 2 +- src/libstore/download.hh | 4 +++- src/libstore/store-api.cc | 4 +++- src/libutil/logging.cc | 6 ++++-- src/libutil/logging.hh | 14 ++++++++++++-- src/nix/progress-bar.cc | 28 ++++++++++++++++++++++++++-- 7 files changed, 55 insertions(+), 12 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 77dee2914..cb67d7a6c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2407,14 +2407,14 @@ struct BuilderLogger : Logger } void startActivity(ActivityId act, ActivityType type, - const std::string & s, const Fields & fields) override + const std::string & s, const Fields & fields, ActivityId parent) override { nlohmann::json json; json["action"] = "start"; json["id"] = act; json["type"] = type; json["text"] = s; - // FIXME: handle fields + // FIXME: handle fields, parent log(lvlError, "@nix " + json.dump()); } @@ -3313,7 +3313,7 @@ void DerivationGoal::flushLine() if (type == actDownload) builderActivities.emplace(std::piecewise_construct, std::forward_as_tuple(json["id"]), - std::forward_as_tuple(*logger, type, json["text"])); + std::forward_as_tuple(*logger, type, json["text"], Logger::Fields{}, act->id)); } else if (action == "stop") @@ -3655,6 +3655,9 @@ void SubstitutionGoal::tryToRun() /* Wake up the worker loop when we're done. */ Finally updateStats([this]() { outPipe.writeSide = -1; }); + Activity act(*logger, actSubstitute, "", Logger::Fields{storePath, sub->getUri()}); + PushActivity pact(act.id); + copyStorePath(ref(sub), ref(worker.store.shared_from_this()), storePath, repair); diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 71b720a3b..a20999176 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(*logger, actDownload, fmt("downloading '%s'", request.uri)) + , act(*logger, actDownload, fmt("downloading '%s'", request.uri), {}, request.parentAct) { if (!request.expectedETag.empty()) requestHeaders = curl_slist_append(requestHeaders, ("If-None-Match: " + request.expectedETag).c_str()); diff --git a/src/libstore/download.hh b/src/libstore/download.hh index 7d8982d64..752bf3723 100644 --- a/src/libstore/download.hh +++ b/src/libstore/download.hh @@ -16,8 +16,10 @@ struct DownloadRequest bool head = false; size_t tries = 5; unsigned int baseRetryTimeMs = 250; + ActivityId parentAct; - DownloadRequest(const std::string & uri) : uri(uri) { } + DownloadRequest(const std::string & uri) + : uri(uri), parentAct(curActivity) { } }; struct DownloadResult diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index f52021061..f07376852 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -565,7 +565,9 @@ void Store::buildPaths(const PathSet & paths, BuildMode buildMode) void copyStorePath(ref srcStore, ref dstStore, const Path & storePath, RepairFlag repair, CheckSigsFlag checkSigs) { - Activity act(*logger, actCopyPath, fmt("copying path '%s'", storePath)); + Activity act(*logger, actCopyPath, fmt("copying path '%s'", storePath), + {storePath, srcStore->getUri(), dstStore->getUri()}); + PushActivity pact(act.id); auto info = srcStore->queryPathInfo(storePath); diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 87f20664e..b103b902e 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -5,6 +5,8 @@ namespace nix { +thread_local ActivityId curActivity = 0; + Logger * logger = makeDefaultLogger(); void Logger::warn(const std::string & msg) @@ -75,10 +77,10 @@ Logger * makeDefaultLogger() std::atomic nextId{(uint64_t) getpid() << 32}; Activity::Activity(Logger & logger, ActivityType type, - const std::string & s, const Logger::Fields & fields) + const std::string & s, const Logger::Fields & fields, ActivityId parent) : logger(logger), id(nextId++) { - logger.startActivity(id, type, s, fields); + logger.startActivity(id, type, s, fields, parent); } } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 567af361e..9427f2682 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -23,6 +23,7 @@ typedef enum { actBuild = 105, actOptimiseStore = 106, actVerifyPaths = 107, + actSubstitute = 108, } ActivityType; typedef enum { @@ -65,7 +66,7 @@ public: virtual void warn(const std::string & msg); virtual void startActivity(ActivityId act, ActivityType type, - const std::string & s, const Fields & fields) { }; + const std::string & s, const Fields & fields, ActivityId parent) { }; virtual void stopActivity(ActivityId act) { }; @@ -76,6 +77,8 @@ public: virtual void result(ActivityId act, ResultType type, const Fields & fields) { }; }; +extern thread_local ActivityId curActivity; + struct Activity { Logger & logger; @@ -83,7 +86,7 @@ struct Activity const ActivityId id; Activity(Logger & logger, ActivityType type, const std::string & s = "", - const Logger::Fields & fields = {}); + const Logger::Fields & fields = {}, ActivityId parent = curActivity); Activity(const Activity & act) = delete; @@ -107,6 +110,13 @@ struct Activity friend class Logger; }; +struct PushActivity +{ + const ActivityId prevAct; + PushActivity(ActivityId act) : prevAct(curActivity) { curActivity = act; } + ~PushActivity() { curActivity = prevAct; } +}; + extern Logger * logger; Logger * makeDefaultLogger(); diff --git a/src/nix/progress-bar.cc b/src/nix/progress-bar.cc index 17d7109e7..1b597433b 100644 --- a/src/nix/progress-bar.cc +++ b/src/nix/progress-bar.cc @@ -73,6 +73,8 @@ private: uint64_t running = 0; uint64_t failed = 0; std::map expectedByType; + bool visible = true; + ActivityId parent; }; struct ActivitiesByType @@ -125,7 +127,7 @@ public: } void startActivity(ActivityId act, ActivityType type, const std::string & s, - const Fields & fields) override + const Fields & fields, ActivityId parent) override { auto state(state_.lock()); @@ -133,6 +135,7 @@ public: auto i = std::prev(state->activities.end()); i->s = s; i->type = type; + i->parent = parent; state->its.emplace(act, i); state->activitiesByType[type].its.emplace(act, i); @@ -143,9 +146,30 @@ public: i->s = fmt("building " ANSI_BOLD "%s" ANSI_NORMAL, name); } + if (type == actSubstitute) { + auto name = storePathToName(getS(fields, 0)); + i->s = fmt("fetching " ANSI_BOLD "%s" ANSI_NORMAL " from %s", name, getS(fields, 1)); + } + + if ((type == actDownload && hasAncestor(*state, actCopyPath, parent)) + || (type == actCopyPath && hasAncestor(*state, actSubstitute, parent))) + i->visible = false; + update(*state); } + /* Check whether an activity has an ancestore with the specified + type. */ + bool hasAncestor(State & state, ActivityType type, ActivityId act) + { + while (act != 0) { + auto i = state.its.find(act); + if (i == state.its.end()) break; + if (i->second->type == type) return true; + } + return false; + } + void stopActivity(ActivityId act) override { auto state(state_.lock()); @@ -253,7 +277,7 @@ public: if (!status.empty()) line += " "; auto i = state.activities.rbegin(); - while (i != state.activities.rend() && i->s.empty() && i->s2.empty()) + while (i != state.activities.rend() && (!i->visible || (i->s.empty() && i->s2.empty()))) ++i; if (i != state.activities.rend()) {