From c5bd564c69ec1fb4f2ffc1091653d99ebc87e284 Mon Sep 17 00:00:00 2001
From: Benjamin Hipple <bhipple@protonmail.com>
Date: Sun, 6 Oct 2019 16:54:56 -0400
Subject: [PATCH] nix doctor: add more logging output to checks

When running nix doctor on a healthy system, it just prints the store URI and
nothing else. This makes it unclear whether the system is in a good state and
what check(s) it actually ran, since some of the checks are optional depending
on the store type.

This commit updates nix doctor to print an colored log message for every check
that it does, and explicitly state whether that check was a PASS or FAIL to make
it clear to the user whether the system passed its checkup with the doctor.

Fixes #3084
---
 src/nix/doctor.cc | 67 +++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 26 deletions(-)

diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc
index 7b5444619..795236c5f 100644
--- a/src/nix/doctor.cc
+++ b/src/nix/doctor.cc
@@ -1,11 +1,17 @@
+#include <sstream>
+
 #include "command.hh"
+#include "logging.hh"
 #include "serve-protocol.hh"
 #include "shared.hh"
 #include "store-api.hh"
+#include "util.hh"
 #include "worker-protocol.hh"
 
 using namespace nix;
 
+namespace {
+
 std::string formatProtocol(unsigned int proto)
 {
     if (proto) {
@@ -16,6 +22,18 @@ std::string formatProtocol(unsigned int proto)
     return "unknown";
 }
 
+bool checkPass(const std::string & msg) {
+    logger->log(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg);
+    return true;
+}
+
+bool checkFail(const std::string & msg) {
+    logger->log(ANSI_RED "[FAIL] " ANSI_NORMAL + msg);
+    return false;
+}
+
+}
+
 struct CmdDoctor : StoreCommand
 {
     bool success = true;
@@ -27,13 +45,12 @@ struct CmdDoctor : StoreCommand
 
     std::string description() override
     {
-        return "check your system for potential problems";
+        return "check your system for potential problems and print a PASS or FAIL for each check.";
     }
 
     void run(ref<Store> store) override
     {
-        std::cout << "Store uri: " << store->getUri() << std::endl;
-        std::cout << std::endl;
+        logger->log("Running checks against store uri: " + store->getUri());
 
         auto type = getStoreType();
 
@@ -56,15 +73,14 @@ struct CmdDoctor : StoreCommand
                 dirs.insert(dirOf(canonPath(dir + "/nix-env", true)));
 
         if (dirs.size() != 1) {
-            std::cout << "Warning: multiple versions of nix found in PATH." << std::endl;
-            std::cout << std::endl;
+            std::stringstream ss;
+            ss << "Multiple versions of nix found in PATH:\n";
             for (auto & dir : dirs)
-                std::cout << "  " << dir << std::endl;
-            std::cout << std::endl;
-            return false;
+                ss << "  " << dir << "\n";
+            return checkFail(ss.str());
         }
 
-        return true;
+        return checkPass("PATH contains only one nix version.");
     }
 
     bool checkProfileRoots(ref<Store> store)
@@ -87,17 +103,17 @@ struct CmdDoctor : StoreCommand
         }
 
         if (!dirs.empty()) {
-            std::cout << "Warning: found profiles outside of " << settings.nixStateDir << "/profiles." << std::endl;
-            std::cout << "The generation this profile points to might not have a gcroot and could be" << std::endl;
-            std::cout << "garbage collected, resulting in broken symlinks." << std::endl;
-            std::cout << std::endl;
+            std::stringstream ss;
+            ss << "Found profiles outside of " << settings.nixStateDir << "/profiles.\n"
+               << "The generation this profile points to might not have a gcroot and could be\n"
+               << "garbage collected, resulting in broken symlinks.\n\n";
             for (auto & dir : dirs)
-                std::cout << "  " << dir << std::endl;
-            std::cout << std::endl;
-            return false;
+                ss << "  " << dir << "\n";
+            ss << "\n";
+            return checkFail(ss.str());
         }
 
-        return true;
+        return checkPass("All profiles are gcroots.");
     }
 
     bool checkStoreProtocol(unsigned int storeProto)
@@ -107,17 +123,16 @@ struct CmdDoctor : StoreCommand
             : PROTOCOL_VERSION;
 
         if (clientProto != storeProto) {
-            std::cout << "Warning: protocol version of this client does not match the store." << std::endl;
-            std::cout << "While this is not necessarily a problem it's recommended to keep the client in" << std::endl;
-            std::cout << "sync with the daemon." << std::endl;
-            std::cout << std::endl;
-            std::cout << "Client protocol: " << formatProtocol(clientProto) << std::endl;
-            std::cout << "Store protocol: " << formatProtocol(storeProto) << std::endl;
-            std::cout << std::endl;
-            return false;
+            std::stringstream ss;
+            ss << "Warning: protocol version of this client does not match the store.\n"
+               << "While this is not necessarily a problem it's recommended to keep the client in\n"
+               << "sync with the daemon.\n\n"
+               << "Client protocol: " << formatProtocol(clientProto) << "\n"
+               << "Store protocol: " << formatProtocol(storeProto) << "\n\n";
+            return checkFail(ss.str());
         }
 
-        return true;
+        return checkPass("Client protocol matches store protocol.");
     }
 };