From 9aa2ecd78cc865f69a9e978ae55ab0f3eda73c46 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 19 May 2020 04:51:45 +0100 Subject: [PATCH] refactor(3p/nix/nix-daemon): Remove activities from protocol Removes the activity transfer that was previously nulled out from the daemon protocol completely. This might actually break Nix completely, I haven't tried yet, but that's fine because this will be replaced with gRPC. --- third_party/nix/src/libstore/remote-store.cc | 58 +------------- third_party/nix/src/nix-daemon/nix-daemon.cc | 81 +++++++------------- 2 files changed, 27 insertions(+), 112 deletions(-) diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc index d041c452a..ce2fc3888 100644 --- a/third_party/nix/src/libstore/remote-store.cc +++ b/third_party/nix/src/libstore/remote-store.cc @@ -632,31 +632,6 @@ RemoteStore::Connection::~Connection() { } } -// TODO(tazjin): these logger fields used to be passed to the JSON -// logger but I don't care about them, whatever sends them should -// also be fixed. -static void ignoreFields(Source& from) { - size_t size = readInt(from); - - // This ignores the fields simply by reading the data into nowhere. - for (size_t n = 0; n < size; n++) { - auto type_tag = readInt(from); - - switch (type_tag) { - case 0: // previously: 0 ~ Logger::Field::tInt - readNum(from); - break; - - case 1: // previously: 1 ~ Logger::Field::tString - readString(from); - break; - - default: - throw Error("got unsupported field type %x from Nix daemon", type_tag); - } - } -} - std::exception_ptr RemoteStore::Connection::processStderr(Sink* sink, Source* source) { to.flush(); @@ -689,38 +664,7 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink* sink, } else if (msg == STDERR_START_ACTIVITY) { - // Various fields need to be ignored in this case, as the - // activity stuff is being removed. - readNum(from); // used to be ActivityId - const auto verbosity = static_cast(readInt(from)); - readInt(from); // activity type - const auto msg = readString(from); - ignoreFields(from); - readNum(from); // ActivityId of "parent" - - switch (verbosity) { - case compat::kError: - LOG(ERROR) << msg; - break; - case compat::kWarn: - LOG(WARNING) << msg; - break; - case compat::kInfo: - LOG(INFO) << msg; - break; - default: - DLOG(INFO) << msg; - } - } - - else if (msg == STDERR_STOP_ACTIVITY) { - readNum(from); // used to be ActivityId - } - - else if (msg == STDERR_RESULT) { - readNum(from); // ActivityId - readInt(from); // ResultType - ignoreFields(from); + LOG(INFO) << readString(from); } else if (msg == STDERR_LAST) { diff --git a/third_party/nix/src/nix-daemon/nix-daemon.cc b/third_party/nix/src/nix-daemon/nix-daemon.cc index 799a4aadd..fdb86eaf9 100644 --- a/third_party/nix/src/nix-daemon/nix-daemon.cc +++ b/third_party/nix/src/nix-daemon/nix-daemon.cc @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -54,24 +55,10 @@ static ssize_t splice(int fd_in, void* off_in, int fd_out, void* off_out, static FdSource from(STDIN_FILENO); static FdSink to(STDOUT_FILENO); -Sink& operator<<(Sink& sink, const Logger::Fields& fields) { - sink << fields.size(); - for (auto& f : fields) { - sink << f.type; - if (f.type == Logger::Field::tInt) - sink << f.i; - else if (f.type == Logger::Field::tString) - sink << f.s; - else - abort(); - } - return sink; -} - /* Logger that forwards log messages to the client, *if* we're in a state where the protocol allows it (i.e., when canSendStderr is true). */ -struct TunnelLogger : public Logger { +struct TunnelLogger { struct State { bool canSendStderr = false; std::vector pendingMsgs; @@ -101,9 +88,7 @@ struct TunnelLogger : public Logger { state->pendingMsgs.push_back(s); } - void log(Verbosity lvl, const FormatOrString& fs) override { - if (lvl > verbosity) return; - + void log(const FormatOrString& fs) { StringSink buf; buf << STDERR_NEXT << (fs.s + "\n"); enqueueMsg(*buf.s); @@ -138,30 +123,16 @@ struct TunnelLogger : public Logger { } } - void startActivity(ActivityId act, Verbosity lvl, ActivityType type, - const std::string& s, const Fields& fields, - ActivityId parent) override { + void startActivity(const std::string& s) { if (GET_PROTOCOL_MINOR(clientVersion) < 20) { - if (!s.empty()) log(lvl, s + "..."); + if (!s.empty()) { + LOG(INFO) << s; + } return; } StringSink buf; - buf << STDERR_START_ACTIVITY << act << lvl << type << s << fields << parent; - enqueueMsg(*buf.s); - } - - void stopActivity(ActivityId act) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; - StringSink buf; - buf << STDERR_STOP_ACTIVITY << act; - enqueueMsg(*buf.s); - } - - void result(ActivityId act, ResultType type, const Fields& fields) override { - if (GET_PROTOCOL_MINOR(clientVersion) < 20) return; - StringSink buf; - buf << STDERR_RESULT << act << type << fields; + buf << STDERR_START_ACTIVITY << s; enqueueMsg(*buf.s); } }; @@ -492,11 +463,11 @@ static void performOp(TunnelLogger* logger, ref store, bool trusted, settings.keepFailed = readInt(from); settings.keepGoing = readInt(from); settings.tryFallback = readInt(from); - verbosity = (Verbosity)readInt(from); + readInt(from); // obsolete verbosity settings.maxBuildJobs.assign(readInt(from)); settings.maxSilentTime = readInt(from); readInt(from); // obsolete useBuildHook - settings.verboseBuild = lvlError == (Verbosity)readInt(from); + settings.verboseBuild = 0 == readInt(from); readInt(from); // obsolete logType readInt(from); // obsolete printBuildTrace settings.buildCores = readInt(from); @@ -528,7 +499,7 @@ static void performOp(TunnelLogger* logger, ref store, bool trusted, if (trusted.count(s)) subs.push_back(s); else - warn("ignoring untrusted substituter '%s'", s); + LOG(WARNING) << "ignoring untrusted substituter '" << s << "'"; res = subs; return true; }; @@ -545,12 +516,11 @@ static void performOp(TunnelLogger* logger, ref store, bool trusted, else if (setSubstituters(settings.extraSubstituters)) ; else - warn( - "ignoring the user-specified setting '%s', because it is a " - "restricted setting and you are not a trusted user", - name); + LOG(WARNING) << "ignoring the user-specified setting '" << name + << "', because it is a " + << "restricted setting and you are not a trusted user"; } catch (UsageError& e) { - warn(e.what()); + LOG(WARNING) << e.what(); } } @@ -724,14 +694,13 @@ static void processConnection(bool trusted, const std::string& userName, if (clientVersion < 0x10a) throw Error("the Nix client version is too old"); auto tunnelLogger = new TunnelLogger(clientVersion); - auto prevLogger = nix::logger; - logger = tunnelLogger; + // logger = tunnelLogger; unsigned int opCount = 0; Finally finally([&]() { _isInterrupted = false; - prevLogger->log(lvlDebug, fmt("%d operations", opCount)); + DLOG(INFO) << opCount << " operations"; }); if (GET_PROTOCOL_MINOR(clientVersion) >= 14 && readInt(from)) @@ -972,15 +941,16 @@ static void daemonLoop(char** argv) { if (matchUser(user, group, trustedUsers)) trusted = true; if ((!trusted && !matchUser(user, group, allowedUsers)) || - group == settings.buildUsersGroup) + group == settings.buildUsersGroup) { throw Error( format("user '%1%' is not allowed to connect to the Nix daemon") % user); + } - printInfo(format((string) "accepted connection from pid %1%, user %2%" + - (trusted ? " (trusted)" : "")) % - (peer.pidKnown ? std::to_string(peer.pid) : "") % - (peer.uidKnown ? user : "")); + LOG(INFO) << "accepted connection from pid " + << (peer.pidKnown ? std::to_string(peer.pid) : "") + << ", user " << (peer.uidKnown ? user : "") + << (trusted ? " (trusted)" : ""); /* Fork a child to handle the connection. */ ProcessOptions options; @@ -993,8 +963,9 @@ static void daemonLoop(char** argv) { fdSocket = -1; /* Background the daemon. */ - if (setsid() == -1) + if (setsid() == -1) { throw SysError(format("creating a new session")); + } /* Restore normal handling of SIGCHLD. */ setSigChldAction(false); @@ -1017,7 +988,7 @@ static void daemonLoop(char** argv) { } catch (Interrupted& e) { return; } catch (Error& e) { - printError(format("error processing connection: %1%") % e.msg()); + LOG(ERROR) << "error processing connection: " << e.msg(); } } }