From ed034d0526f00c5a4b55a52e1bc1dda4de533304 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sat, 11 Oct 2025 21:59:56 +0200 Subject: [PATCH] libutil: make Activity a real, move-only class activities are scopes, and there's no reason scopes should not be able to move around. this also means we can *create* them elsewhere without also having to box them in some way, making parent relationships clear Change-Id: I5df036e12ebd8270feb4dca1f23b6bee1f08e906 --- lix/libstore/binary-cache-store.cc | 2 +- lix/libstore/filetransfer.cc | 6 +++--- lix/libstore/store-api.cc | 2 +- lix/libutil/logging.cc | 34 +++++++++++++++++++++++------- lix/libutil/logging.hh | 30 +++++++++++++++++++++----- 5 files changed, 56 insertions(+), 18 deletions(-) diff --git a/lix/libstore/binary-cache-store.cc b/lix/libstore/binary-cache-store.cc index f77d191d8..11dcdf004 100644 --- a/lix/libstore/binary-cache-store.cc +++ b/lix/libstore/binary-cache-store.cc @@ -469,7 +469,7 @@ try { actQueryPathInfo, fmt("querying info about '%s' on '%s'", storePathS, uri), Logger::Fields{storePathS, uri}, - context ? context->id : 0 + context ); auto narInfoFile = narInfoFileFor(storePath); diff --git a/lix/libstore/filetransfer.cc b/lix/libstore/filetransfer.cc index 4d5bd4f5a..44edfd09a 100644 --- a/lix/libstore/filetransfer.cc +++ b/lix/libstore/filetransfer.cc @@ -142,7 +142,7 @@ struct curlFileTransfer : public FileTransfer TransferItem( const std::string & uri, FileTransferOptions && options, - ActivityId parentAct, + const Activity * parentAct, std::optional uploadData, bool noBody, curl_off_t writtenToSink, @@ -904,7 +904,7 @@ struct curlFileTransfer : public FileTransfer FileTransferOptions options; std::optional data; bool noBody; - ActivityId parentAct; + const Activity * parentAct; std::shared_ptr transfer; FileTransferResult metadata; @@ -929,7 +929,7 @@ struct curlFileTransfer : public FileTransfer , options(options) , data(std::move(data)) , noBody(noBody) - , parentAct(context ? context->id : 0) + , parentAct(context) , backoff(backoffTimeouts( fileTransferSettings.tries, std::chrono::seconds(fileTransferSettings.maxConnectTimeout.get()), diff --git a/lix/libstore/store-api.cc b/lix/libstore/store-api.cc index 52bea0d33..5dc515292 100644 --- a/lix/libstore/store-api.cc +++ b/lix/libstore/store-api.cc @@ -1040,7 +1040,7 @@ try { actCopyPath, makeCopyPathMessage(srcUri, dstUri, storePathS), {storePathS, srcUri, dstUri}, - context ? context->id : 0 + context ); auto info = TRY_AWAIT(srcStore.queryPathInfo(storePath, &act)); diff --git a/lix/libutil/logging.cc b/lix/libutil/logging.cc index 7305700c8..fab6fed69 100644 --- a/lix/libutil/logging.cc +++ b/lix/libutil/logging.cc @@ -120,11 +120,18 @@ Logger * makeSimpleLogger(bool printBuildLogs) std::atomic nextId{0}; -Activity::Activity(Logger & logger, Verbosity lvl, ActivityType type, - const std::string & s, const Logger::Fields & fields, ActivityId parent) - : logger(logger), id(nextId++ + (((uint64_t) getpid()) << 32)) +Activity::Activity( + Logger & logger, + Verbosity lvl, + ActivityType type, + const std::string & s, + const Logger::Fields & fields, + const Activity * parent +) + : logger(&logger) + , id(nextId++ + (((uint64_t) getpid()) << 32)) { - logger.startActivity(id, lvl, type, s, fields, parent); + logger.startActivity(id, lvl, type, s, fields, parent ? parent->id : 0); } void to_json(JSON & json, std::shared_ptr pos) @@ -279,10 +286,18 @@ bool handleJSONLogMessage(JSON & json, if (action == "start") { auto type = (ActivityType) json["type"]; if (trusted || type == actFileTransfer) - activities.emplace(std::piecewise_construct, + activities.emplace( + std::piecewise_construct, std::forward_as_tuple(json["id"]), - std::forward_as_tuple(*logger, (Verbosity) json["level"], type, - json["text"], getFields(json["fields"]), act.id)); + std::forward_as_tuple( + *logger, + (Verbosity) json["level"], + type, + json["text"], + getFields(json["fields"]), + &act + ) + ); } else if (action == "stop") @@ -324,8 +339,11 @@ bool handleJSONLogMessage(const std::string & msg, Activity::~Activity() { + if (!logger) { + return; + } try { - logger.stopActivity(id); + logger->stopActivity(id); } catch (...) { ignoreExceptionInDestructor(); } diff --git a/lix/libutil/logging.hh b/lix/libutil/logging.hh index b587816f1..d8d347d2b 100644 --- a/lix/libutil/logging.hh +++ b/lix/libutil/logging.hh @@ -173,31 +173,51 @@ struct nop struct Activity { - Logger & logger; +private: + Logger * logger; - const ActivityId id; + ActivityId id; +public: Activity( Logger & logger, Verbosity lvl, ActivityType type, const std::string & s = "", const Logger::Fields & fields = {}, - ActivityId parent = 0 + const Activity * parent = nullptr ); Activity( Logger & logger, ActivityType type, const Logger::Fields & fields = {}, - ActivityId parent = 0 + const Activity * parent = nullptr ) : Activity(logger, lvlError, type, "", fields, parent) {}; + Activity(Activity && other) : logger(nullptr), id(0) + { + swap(other); + } + + Activity & operator=(Activity && other) + { + Activity(std::move(other)).swap(*this); + return *this; + } + Activity(const Activity & act) = delete; + Activity & operator=(const Activity & act) = delete; ~Activity(); + void swap(Activity & other) + { + std::swap(logger, other.logger); + std::swap(id, other.id); + } + void progress(uint64_t done = 0, uint64_t expected = 0, uint64_t running = 0, uint64_t failed = 0) const { result(resProgress, done, expected, running, failed); } @@ -214,7 +234,7 @@ struct Activity void result(ResultType type, const Logger::Fields & fields) const { - logger.result(id, type, fields); + logger->result(id, type, fields); } friend class Logger;