From 4b99c09f5ccd385d2bf0c82a8c9a4ae1658abbe8 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Fri, 1 May 2020 14:32:06 -0600 Subject: [PATCH] convert some errors --- src/libstore/builtins/buildenv.cc | 12 +++++++-- src/libstore/download.cc | 7 ++++- src/libstore/gc.cc | 13 ++++++--- src/libstore/local-store.cc | 44 +++++++++++++++++++++++-------- src/libstore/optimise-store.cc | 18 ++++++++++--- src/libstore/remote-store.cc | 9 +++++++ src/libstore/sqlite.cc | 5 +++- 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