convert some errors

7 files changed, 86 insertions, 22 deletions

diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc
index 000769094..64085fcc9 100644
--- a/src/libstore/builtins/buildenv.cc
+++ b/src/libstore/builtins/buildenv.cc
@@ -22,7 +22,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir,
         srcFiles = readDirectory(srcDir);
     } catch (SysError & e) {
         if (e.errNo == ENOTDIR) {
-            printError("warning: not including '%s' in the user environment because it's not a directory", srcDir);
+            logWarning(
+                ErrorInfo { 
+                    .name = "Create Links - Directory",
+                    .hint = hintfmt("not including '%s' in the user environment because it's not a directory", srcDir)
+            });
             return;
         }
         throw;
@@ -41,7 +45,11 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir,
                 throw SysError("getting status of '%1%'", srcFile);
         } catch (SysError & e) {
             if (e.errNo == ENOENT || e.errNo == ENOTDIR) {
-                printError("warning: skipping dangling symlink '%s'", dstFile);
+                logWarning(
+                    ErrorInfo { 
+                        .name = "Create Links - Skipping Symlink",
+                        .hint = hintfmt("skipping dangling symlink '%s'", dstFile)
+                });
                 continue;
             }
             throw;
diff --git a/src/libstore/download.cc b/src/libstore/download.cc
index 7d0cb449b..60c6a80f3 100644
--- a/src/libstore/download.cc
+++ b/src/libstore/download.cc
@@ -600,7 +600,12 @@ struct CurlDownloader : public Downloader
             workerThreadMain();
         } catch (nix::Interrupted & e) {
         } catch (std::exception & e) {
-            printError("unexpected error in download thread: %s", e.what());
+            logError(
+                ErrorInfo { 
+                    .name = "download",
+                    .hint = hintfmt("unexpected error in download thread: %s", 
+                                    e.what())
+                });
         }
 
         {
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 27bd3f3e9..4f6f78a99 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -41,7 +41,8 @@ AutoCloseFD LocalStore::openGCLock(LockType lockType)
         throw SysError("opening global GC lock '%1%'", fnGCLock);
 
     if (!lockFile(fdGCLock.get(), lockType, false)) {
-        printError(format("waiting for the big garbage collector lock..."));
+        // TODO: info?
+        printError("waiting for the big garbage collector lock...");
         lockFile(fdGCLock.get(), lockType, true);
     }
 
@@ -129,10 +130,14 @@ Path LocalFSStore::addPermRoot(const StorePath & storePath,
     if (settings.checkRootReachability) {
         auto roots = findRoots(false);
         if (roots[storePath.clone()].count(gcRoot) == 0)
-            printError(
-                "warning: '%1%' is not in a directory where the garbage collector looks for roots; "
+
+            logWarning(
+                ErrorInfo { 
+                    .name = "GC Root",
+                    .hint = hintfmt("warning: '%1%' is not in a directory where the garbage collector looks for roots; "
                 "therefore, '%2%' might be removed by the garbage collector",
-                gcRoot, printStorePath(storePath));
+                gcRoot, printStorePath(storePath))
+            });
     }
 
     /* Grab the global GC root, causing us to block while a GC is in
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index db5a3b53c..1f16a22b1 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -87,8 +87,13 @@ LocalStore::LocalStore(const Params & params)
 
         struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str());
         if (!gr)
-            printError(format("warning: the group '%1%' specified in 'build-users-group' does not exist")
-                % settings.buildUsersGroup);
+            logError(
+                ErrorInfo { 
+                    .name = "'build-users-group' not found",
+                    .hint = hintfmt(
+                        "warning: the group '%1%' specified in 'build-users-group' does not exist",
+                        settings.buildUsersGroup)
+            });
         else {
             struct stat st;
             if (stat(realStoreDir.c_str(), &st))
@@ -876,7 +881,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths,
             } catch (SubstituterDisabled &) {
             } catch (Error & e) {
                 if (settings.tryFallback)
-                    printError(e.what());
+                    logError(e.info());
                 else
                     throw;
             }
@@ -1237,9 +1242,13 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
             Path linkPath = linksDir + "/" + link.name;
             string hash = hashPath(htSHA256, linkPath).first.to_string(Base32, false);
             if (hash != link.name) {
-                printError(
+                logError(
+                    ErrorInfo { 
+                        .name = "Invalid Hash",
+                        .hint = hintfmt(
                     "link '%s' was modified! expected hash '%s', got '%s'",
-                    linkPath, link.name, hash);
+                    linkPath, link.name, hash)
+                });
                 if (repair) {
                     if (unlink(linkPath.c_str()) == 0)
                         printError("removed link '%s'", linkPath);
@@ -1272,8 +1281,12 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
                 auto current = hashSink->finish();
 
                 if (info->narHash != nullHash && info->narHash != current.first) {
-                    printError("path '%s' was modified! expected hash '%s', got '%s'",
-                        printStorePath(i), info->narHash.to_string(), current.first.to_string());
+                    logError(
+                        ErrorInfo { 
+                            .name = "Invalid Hash - Path Modified",
+                            .hint = hintfmt("path '%s' was modified! expected hash '%s', got '%s'",
+                            printStorePath(i), info->narHash.to_string(), current.first.to_string())
+                    });
                     if (repair) repairPath(i); else errors = true;
                 } else {
 
@@ -1304,7 +1317,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
                 /* It's possible that the path got GC'ed, so ignore
                    errors on invalid paths. */
                 if (isValidPath(i))
-                    printError("error: %s", e.msg());
+                    logError(e.info());
                 else
                     warn(e.msg());
                 errors = true;
@@ -1324,7 +1337,11 @@ void LocalStore::verifyPath(const Path & pathS, const StringSet & store,
     if (!done.insert(pathS).second) return;
 
     if (!isStorePath(pathS)) {
-        printError("path '%s' is not in the Nix store", pathS);
+        logError(
+            ErrorInfo { 
+                .name = "Nix path not found",
+                .hint = hintfmt("path '%s' is not in the Nix store", pathS)
+        });
         return;
     }
 
@@ -1347,12 +1364,17 @@ void LocalStore::verifyPath(const Path & pathS, const StringSet & store,
             auto state(_state.lock());
             invalidatePath(*state, path);
         } else {
-            printError("path '%s' disappeared, but it still has valid referrers!", pathS);
+            // TODO log as warning if repair successful??
+            logError(
+                ErrorInfo { 
+                    .name = "Missing path with referrers",
+                    .hint = hintfmt("path '%s' disappeared, but it still has valid referrers!", pathS)
+            });
             if (repair)
                 try {
                     repairPath(path);
                 } catch (Error & e) {
-                    warn(e.msg());
+                    logWarning(e.info());
                     errors = true;
                 }
             else errors = true;
diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc
index f9cf2bb6b..9f6112183 100644
--- a/src/libstore/optimise-store.cc
+++ b/src/libstore/optimise-store.cc
@@ -130,7 +130,11 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
        NixOS (example: $fontconfig/var/cache being modified).  Skip
        those files.  FIXME: check the modification time. */
     if (S_ISREG(st.st_mode) && (st.st_mode & S_IWUSR)) {
-        printError("skipping suspicious writable file '%1%'", path);
+        logWarning(
+            ErrorInfo { 
+                .name = "Suspicious File",
+                .hint = hintfmt("skipping suspicious writable file '%1%'", path)
+        });
         return;
     }
 
@@ -194,7 +198,11 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     }
 
     if (st.st_size != stLink.st_size) {
-        printError("removing corrupted link '%1%'", linkPath);
+        logWarning(
+            ErrorInfo { 
+                .name = "Corrupted Link",
+                .hint = hintfmt("removing corrupted link '%1%'", linkPath)
+        });
         unlink(linkPath.c_str());
         goto retry;
     }
@@ -229,7 +237,11 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
     /* Atomically replace the old file with the new hard link. */
     if (rename(tempLink.c_str(), path.c_str()) == -1) {
         if (unlink(tempLink.c_str()) == -1)
-            printError("unable to unlink '%1%'", tempLink);
+            logError(
+                ErrorInfo { 
+                    .name = "Unlink error",
+                    .hint = hintfmt("unable to unlink '%1%'", tempLink)
+            });
         if (errno == EMLINK) {
             /* Some filesystems generate too many links on the rename,
                rather than on the original link.  (Probably it
diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
index cc336e460..7c50c1065 100644
--- a/src/libstore/remote-store.cc
+++ b/src/libstore/remote-store.cc
@@ -780,6 +780,15 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source *
         }
 
         else if (msg == STDERR_NEXT)
+            // TODO: is this really an ErrorInfo error?  Seems like we're forwarding the 
+            // stderr output of the remote to current stderr/log 
+            // ErrorInfo gets lost in this scenario.
+            // An alternative might be a logger on the remote that forwards ErrorInfo and etc.
+            // logError(
+            //     ErrorInfo { 
+            //         // .name = "Remote Store" TODO reasonable name.
+            //         .hint = hintfmt(chomp(readString(from)))
+            // });
             printError(chomp(readString(from)));
 
         else if (msg == STDERR_START_ACTIVITY) {
diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc
index a72cd5d88..082c54005 100644
--- a/src/libstore/sqlite.cc
+++ b/src/libstore/sqlite.cc
@@ -204,7 +204,10 @@ void handleSQLiteBusy(SQLiteBusy & e)
 
     if (now > lastWarned + 10) {
         lastWarned = now;
-        printError("warning: %s", e.what());
+        logWarning(
+            ErrorInfo { .name = "sqlite busy",
+                        .hint = hintfmt(e.what())
+            });
     }
 
     /* Sleep for a while since retrying the transaction right away