From 1212b1fbfeee93ce7a04911a4085d796d6d9c72a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Feb 2025 14:59:07 +0100 Subject: [PATCH] JSONLogger: Log to a file descriptor instead of another Logger Logging to another Logger was kind of nonsensical - it was really just an easy way to get it to write its output to stderr, but that only works if the underlying logger writes to stderr. This change is needed to make it easy to log JSON output somewhere else (like a file or socket). --- src/build-remote/build-remote.cc | 2 +- src/libmain/loggers.cc | 2 +- src/libstore/unix/build/local-derivation-goal.cc | 2 +- src/libutil/logging.cc | 10 +++++----- src/libutil/logging.hh | 3 ++- 5 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 82ad7d862..2c3176724 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -51,7 +51,7 @@ static bool allSupportedLocally(Store & store, const std::set& requ static int main_build_remote(int argc, char * * argv) { { - logger = makeJSONLogger(*logger); + logger = makeJSONLogger(STDERR_FILENO); /* Ensure we don't get any SSH passphrase or host key popups. */ unsetenv("DISPLAY"); diff --git a/src/libmain/loggers.cc b/src/libmain/loggers.cc index a4e0530c8..ede5ddae3 100644 --- a/src/libmain/loggers.cc +++ b/src/libmain/loggers.cc @@ -27,7 +27,7 @@ Logger * makeDefaultLogger() { case LogFormat::rawWithLogs: return makeSimpleLogger(true); case LogFormat::internalJSON: - return makeJSONLogger(*makeSimpleLogger(true)); + return makeJSONLogger(STDERR_FILENO); case LogFormat::bar: return makeProgressBar(); case LogFormat::barWithLogs: { diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc index 5b9bc0bb0..805c3bbca 100644 --- a/src/libstore/unix/build/local-derivation-goal.cc +++ b/src/libstore/unix/build/local-derivation-goal.cc @@ -2219,7 +2219,7 @@ void LocalDerivationGoal::runChild() /* Execute the program. This should not return. */ if (drv->isBuiltin()) { try { - logger = makeJSONLogger(*logger); + logger = makeJSONLogger(STDERR_FILENO); std::map outputs; for (auto & e : drv->outputs) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index a5add5565..9caa83efe 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -167,9 +167,9 @@ void to_json(nlohmann::json & json, std::shared_ptr pos) } struct JSONLogger : Logger { - Logger & prevLogger; + Descriptor fd; - JSONLogger(Logger & prevLogger) : prevLogger(prevLogger) { } + JSONLogger(Descriptor fd) : fd(fd) { } bool isVerbose() override { return true; @@ -190,7 +190,7 @@ struct JSONLogger : Logger { void write(const nlohmann::json & json) { - prevLogger.log(lvlError, "@nix " + json.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace)); + writeLine(fd, "@nix " + json.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace)); } void log(Verbosity lvl, std::string_view s) override @@ -262,9 +262,9 @@ struct JSONLogger : Logger { } }; -Logger * makeJSONLogger(Logger & prevLogger) +Logger * makeJSONLogger(Descriptor fd) { - return new JSONLogger(prevLogger); + return new JSONLogger(fd); } static Logger::Fields getFields(nlohmann::json & json) diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 11e4033a5..e8112c6b0 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -3,6 +3,7 @@ #include "error.hh" #include "config.hh" +#include "file-descriptor.hh" #include @@ -183,7 +184,7 @@ extern Logger * logger; Logger * makeSimpleLogger(bool printBuildLogs = true); -Logger * makeJSONLogger(Logger & prevLogger); +Logger * makeJSONLogger(Descriptor fd); /** * @param source A noun phrase describing the source of the message, e.g. "the builder".