Push log source description out of libutil and report build hook @nix warning correctly

roberth · roberth · commit 03d4bfd852dc · 2024-12-09T15:59:59.000+01:00
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
@@ -1161,7 +1161,7 @@ HookReply DerivationGoal::tryBuildHook()
                     throw;
                 }
             }();
-            if (handleJSONLogMessage(s, worker.act, worker.hook->activities, true))
+            if (handleJSONLogMessage(s, worker.act, worker.hook->activities, "the build hook", true))
                 ;
             else if (s.substr(0, 2) == "# ") {
                 reply = s.substr(2);
@@ -1346,9 +1346,9 @@ void DerivationGoal::handleChildOutput(Descriptor fd, std::string_view data)
     if (hook && fd == hook->fromHook.readSide.get()) {
         for (auto c : data)
             if (c == '\n') {
-                auto json = parseJSONMessage(currentHookLine);
+                auto json = parseJSONMessage(currentHookLine, "the derivation builder");
                 if (json) {
-                    auto s = handleJSONLogMessage(*json, worker.act, hook->activities, true);
+                    auto s = handleJSONLogMessage(*json, worker.act, hook->activities, "the derivation builder", true);
                     // ensure that logs from a builder using `ssh-ng://` as protocol
                     // are also available to `nix log`.
                     if (s && !isWrittenToLog && logSink) {
@@ -1390,7 +1390,7 @@ void DerivationGoal::handleEOF(Descriptor fd)
 
 void DerivationGoal::flushLine()
 {
-    if (handleJSONLogMessage(currentLogLine, *act, builderActivities, false))
+    if (handleJSONLogMessage(currentLogLine, *act, builderActivities, "the derivation builder", false))
         ;
 
     else {
diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc
@@ -280,20 +280,22 @@ static Logger::Fields getFields(nlohmann::json & json)
     return fields;
 }
 
-std::optional<nlohmann::json> parseJSONMessage(const std::string & msg)
+std::optional<nlohmann::json> parseJSONMessage(const std::string & msg, std::string_view source)
 {
     if (!hasPrefix(msg, "@nix ")) return std::nullopt;
     try {
         return nlohmann::json::parse(std::string(msg, 5));
     } catch (std::exception & e) {
-        printError("bad JSON log message from builder: %s", e.what());
+        printError("bad JSON log message from %s: %s",
+            Uncolored(source),
+            e.what());
     }
     return std::nullopt;
 }
 
 bool handleJSONLogMessage(nlohmann::json & json,
     const Activity & act, std::map<ActivityId, Activity> & activities,
-    bool trusted)
+    std::string_view source, bool trusted)
 {
     try {
         std::string action = json["action"];
@@ -329,20 +331,21 @@ bool handleJSONLogMessage(nlohmann::json & json,
         return true;
     } catch (const nlohmann::json::exception &e) {
         warn(
-            "warning: Unable to handle a JSON message from the builder: %s",
+            "warning: Unable to handle a JSON message from %s: %s",
+            Uncolored(source),
             e.what()
         );
         return false;
     }
 }
 
 bool handleJSONLogMessage(const std::string & msg,
-    const Activity & act, std::map<ActivityId, Activity> & activities, bool trusted)
+    const Activity & act, std::map<ActivityId, Activity> & activities, std::string_view source, bool trusted)
 {
-    auto json = parseJSONMessage(msg);
+    auto json = parseJSONMessage(msg, source);
     if (!json) return false;
 
-    return handleJSONLogMessage(*json, act, activities, trusted);
+    return handleJSONLogMessage(*json, act, activities, source, trusted);
 }
 
 Activity::~Activity()
diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh
@@ -185,14 +185,25 @@ Logger * makeSimpleLogger(bool printBuildLogs = true);
 
 Logger * makeJSONLogger(Logger & prevLogger);
 
-std::optional<nlohmann::json> parseJSONMessage(const std::string & msg);
+/**
+ * @param source A noun phrase describing the source of the message, e.g. "the builder".
+ */
+std::optional<nlohmann::json> parseJSONMessage(const std::string & msg, std::string_view source);
 
+/**
+ * @param source A noun phrase describing the source of the message, e.g. "the builder".
+ */
 bool handleJSONLogMessage(nlohmann::json & json,
     const Activity & act, std::map<ActivityId, Activity> & activities,
+    std::string_view source,
     bool trusted);
 
+/**
+ * @param source A noun phrase describing the source of the message, e.g. "the builder".
+ */
 bool handleJSONLogMessage(const std::string & msg,
     const Activity & act, std::map<ActivityId, Activity> & activities,
+    std::string_view source,
     bool trusted);
 
 /**

Original file line number	Diff line number	Diff line change
`@@ -280,20 +280,22 @@ static Logger::Fields getFields(nlohmann::json & json)`
`280`	`280`	`return fields;`
`281`	`281`	`}`
`282`	`282`
`283`		`-std::optional<nlohmann::json> parseJSONMessage(const std::string & msg)`
	`283`	`+std::optional<nlohmann::json> parseJSONMessage(const std::string & msg, std::string_view source)`
`284`	`284`	`{`
`285`	`285`	`if (!hasPrefix(msg, "@nix ")) return std::nullopt;`
`286`	`286`	`try {`
`287`	`287`	`return nlohmann::json::parse(std::string(msg, 5));`
`288`	`288`	`} catch (std::exception & e) {`
`289`		`- printError("bad JSON log message from builder: %s", e.what());`
	`289`	`+ printError("bad JSON log message from %s: %s",`
	`290`	`+ Uncolored(source),`
	`291`	`+ e.what());`
`290`	`292`	`}`
`291`	`293`	`return std::nullopt;`
`292`	`294`	`}`
`293`	`295`
`294`	`296`	`bool handleJSONLogMessage(nlohmann::json & json,`
`295`	`297`	`const Activity & act, std::map<ActivityId, Activity> & activities,`
`296`		`- bool trusted)`
	`298`	`+ std::string_view source, bool trusted)`
`297`	`299`	`{`
`298`	`300`	`try {`
`299`	`301`	`std::string action = json["action"];`
`@@ -329,20 +331,21 @@ bool handleJSONLogMessage(nlohmann::json & json,`
`329`	`331`	`return true;`
`330`	`332`	`} catch (const nlohmann::json::exception &e) {`
`331`	`333`	`warn(`
`332`		`- "warning: Unable to handle a JSON message from the builder: %s",`
	`334`	`+ "warning: Unable to handle a JSON message from %s: %s",`
	`335`	`+ Uncolored(source),`
`333`	`336`	`e.what()`
`334`	`337`	`);`
`335`	`338`	`return false;`
`336`	`339`	`}`
`337`	`340`	`}`
`338`	`341`
`339`	`342`	`bool handleJSONLogMessage(const std::string & msg,`
`340`		`- const Activity & act, std::map<ActivityId, Activity> & activities, bool trusted)`
	`343`	`+ const Activity & act, std::map<ActivityId, Activity> & activities, std::string_view source, bool trusted)`
`341`	`344`	`{`
`342`		`- auto json = parseJSONMessage(msg);`
	`345`	`+ auto json = parseJSONMessage(msg, source);`
`343`	`346`	`if (!json) return false;`
`344`	`347`
`345`		`- return handleJSONLogMessage(*json, act, activities, trusted);`
	`348`	`+ return handleJSONLogMessage(*json, act, activities, source, trusted);`
`346`	`349`	`}`
`347`	`350`
`348`	`351`	`Activity::~Activity()`