From ab6f0b9641ad6e9cef72f73d23e31138a97a225b Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Sun, 3 May 2020 08:01:25 -0600 Subject: [PATCH] convert some printError calls to logError --- src/build-remote/build-remote.cc | 8 +- src/libexpr/parser.y | 15 ++- src/libmain/shared.cc | 10 +- src/libstore/build.cc | 97 ++++++++++++++----- src/libstore/pathlocks.cc | 3 +- src/libstore/s3-binary-cache-store.cc | 4 +- src/libstore/store-api.cc | 2 +- src/libutil/affinity.cc | 4 +- src/libutil/error.hh | 6 ++ src/libutil/logging.hh | 1 + src/libutil/util.cc | 18 ++-- src/nix-build/nix-build.cc | 2 + src/nix-daemon/nix-daemon.cc | 3 +- src/nix-env/nix-env.cc | 23 ++++- src/nix-env/user-env.cc | 2 +- src/nix-store/nix-store.cc | 15 ++- src/nix/upgrade-nix.cc | 8 +- src/nix/verify.cc | 20 +++- .../resolve-system-dependencies.cc | 24 ++++- 19 files changed, 195 insertions(+), 70 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 00340b787..0d6859596 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -200,9 +200,13 @@ static int _main(int argc, char * * argv) } catch (std::exception & e) { auto msg = chomp(drainFD(5, false)); - printError("cannot build on '%s': %s%s", + logError( + ErrorInfo { + .name = "remote build", + .hint = hintfmt("cannot build on '%s': %s%s", bestMachine->storeUri, e.what(), - (msg.empty() ? "" : ": " + msg)); + (msg.empty() ? "" : ": " + msg)) + }); bestMachine->enabled = false; continue; } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 3767532d5..3ed9a7a4f 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -689,8 +689,12 @@ std::pair EvalState::resolveSearchPathElem(const SearchPathEl request.unpack = true; res = { true, getDownloader()->downloadCached(store, request).path }; } catch (DownloadError & e) { - // TODO: change to warn()? - printError("warning: Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second); + logWarning( + ErrorInfo { + .name = "Download Error", + .hint = hintfmt("warning: Nix search path entry '%1%' cannot be downloaded, ignoring", elem.second) + }); + res = { false, "" }; } } else { @@ -698,8 +702,11 @@ std::pair EvalState::resolveSearchPathElem(const SearchPathEl if (pathExists(path)) res = { true, path }; else { - // TODO: change to warn()? - printError("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second); + logWarning( + ErrorInfo { + .name = "Search path not found", + .hint = hintfmt("warning: Nix search path entry '%1%' does not exist, ignoring", elem.second) + }); res = { false, "" }; } } diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 8551eb048..5773d90cc 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -300,21 +300,21 @@ int handleExceptions(const string & programName, std::function fun) } catch (UsageError & e) { // TODO: switch to logError // logError(e.info()); - printError( + _printError( format("%1%\nTry '%2% --help' for more information.") % e.what() % programName); return 1; } catch (BaseError & e) { // logError(e.info()); - printError("%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); + _printError("%1%%2%", (settings.showTrace ? e.prefix() : ""), e.msg()); if (e.prefix() != "" && !settings.showTrace) - printError("(use '--show-trace' to show detailed location information)"); + _printError("(use '--show-trace' to show detailed location information)"); return e.status; } catch (std::bad_alloc & e) { - printError(error + "out of memory"); + _printError(error + "out of memory"); return 1; } catch (std::exception & e) { - printError(error + e.what()); + _printError(error + e.what()); return 1; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 2301adb16..bacbd5808 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -493,7 +493,9 @@ void handleDiffHook( if (diffRes.second != "") printError(chomp(diffRes.second)); } catch (Error & error) { - printError("diff hook execution failed: %s", error.what()); + // logError(error.info()) + // TODO append message onto errorinfo... + _printError("diff hook execution failed: %s", error.what()); } } } @@ -1144,7 +1146,11 @@ void DerivationGoal::loadDerivation() trace("loading derivation"); if (nrFailed != 0) { - printError("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath)); + logError( + ErrorInfo { + .name = "missing derivation during build", + .hint = hintfmt("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath)) + }); done(BuildResult::MiscFailure); return; } @@ -1295,8 +1301,14 @@ void DerivationGoal::repairClosure() /* Check each path (slow!). */ for (auto & i : outputClosure) { if (worker.pathContentsGood(i)) continue; - printError("found corrupted or missing path '%s' in the output closure of '%s'", - worker.store.printStorePath(i), worker.store.printStorePath(drvPath)); + logError( + ErrorInfo { + .name = "Corrupt path in closure", + .hint = hintfmt( + "found corrupted or missing path '%s' in the output closure of '%s'", + worker.store.printStorePath(i), worker.store.printStorePath(drvPath)) + }); + auto drvPath2 = outputsToDrv.find(i); if (drvPath2 == outputsToDrv.end()) addWaitee(worker.makeSubstitutionGoal(i, Repair)); @@ -1330,8 +1342,13 @@ void DerivationGoal::inputsRealised() if (nrFailed != 0) { if (!useDerivation) throw Error("some dependencies of '%s' are missing", worker.store.printStorePath(drvPath)); - printError("cannot build derivation '%s': %s dependencies couldn't be built", - worker.store.printStorePath(drvPath), nrFailed); + logError( + ErrorInfo { + .name = "Dependencies could not be built", + .hint = hintfmt( + "cannot build derivation '%s': %s dependencies couldn't be built", + worker.store.printStorePath(drvPath), nrFailed) + }); done(BuildResult::DependencyFailed); return; } @@ -1489,7 +1506,7 @@ void DerivationGoal::tryToBuild() startBuilder(); } catch (BuildError & e) { - printError(e.msg()); + logError(e.info()); outputLocks.unlock(); buildUser.reset(); worker.permanentFailure = true; @@ -1709,7 +1726,7 @@ void DerivationGoal::buildDone() outputLocks.unlock(); } catch (BuildError & e) { - printError(e.msg()); + logError(e.info()); outputLocks.unlock(); @@ -1788,8 +1805,13 @@ HookReply DerivationGoal::tryBuildHook() } catch (SysError & e) { if (e.errNo == EPIPE) { - printError("build hook died unexpectedly: %s", - chomp(drainFD(worker.hook->fromHook.readSide.get()))); + logError( + ErrorInfo { + .name = "Build hook died", + .hint = hintfmt( + "build hook died unexpectedly: %s", + chomp(drainFD(worker.hook->fromHook.readSide.get()))) + }); worker.hook = 0; return rpDecline; } else @@ -3783,10 +3805,10 @@ void DerivationGoal::registerOutputs() result.isNonDeterministic = true; Path prev = worker.store.printStorePath(i->second.path) + checkSuffix; bool prevExists = keepPreviousRound && pathExists(prev); - auto msg = prevExists - ? fmt("output '%s' of '%s' differs from '%s' from previous round", + hintformat hint = prevExists + ? hintfmt("output '%s' of '%s' differs from '%s' from previous round", worker.store.printStorePath(i->second.path), worker.store.printStorePath(drvPath), prev) - : fmt("output '%s' of '%s' differs from previous round", + : hintfmt("output '%s' of '%s' differs from previous round", worker.store.printStorePath(i->second.path), worker.store.printStorePath(drvPath)); handleDiffHook( @@ -3796,9 +3818,15 @@ void DerivationGoal::registerOutputs() worker.store.printStorePath(drvPath), tmpDir); if (settings.enforceDeterminism) - throw NotDeterministic(msg); + throw NotDeterministic(hint); + + logError( + ErrorInfo { + .name = "Output determinism error", + .hint = hint + }); + - printError(msg); curRound = nrRounds; // we know enough, bail out early } } @@ -4060,9 +4088,13 @@ void DerivationGoal::handleChildOutput(int fd, const string & data) { logSize += data.size(); if (settings.maxLogSize && logSize > settings.maxLogSize) { - printError( - format("%1% killed after writing more than %2% bytes of log output") - % getName() % settings.maxLogSize); + logError( + ErrorInfo { + .name = "Max log size exceeded", + .hint = hintfmt( + "%1% killed after writing more than %2% bytes of log output", + getName(), settings.maxLogSize) + }); killChild(); done(BuildResult::LogLimitExceeded); return; @@ -4352,7 +4384,7 @@ void SubstitutionGoal::tryNext() throw; } catch (Error & e) { if (settings.tryFallback) { - printError(e.what()); + logError(e.info()); tryNext(); return; } @@ -4864,9 +4896,13 @@ void Worker::waitForInput() j->respectTimeouts && after - j->lastOutput >= std::chrono::seconds(settings.maxSilentTime)) { - printError( - format("%1% timed out after %2% seconds of silence") - % goal->getName() % settings.maxSilentTime); + logError( + ErrorInfo { + .name = "Silent build timeout", + .hint = hintfmt( + "%1% timed out after %2% seconds of silence", + goal->getName(), settings.maxSilentTime) + }); goal->timedOut(); } @@ -4875,9 +4911,13 @@ void Worker::waitForInput() j->respectTimeouts && after - j->timeStarted >= std::chrono::seconds(settings.buildTimeout)) { - printError( - format("%1% timed out after %2% seconds") - % goal->getName() % settings.buildTimeout); + logError( + ErrorInfo { + .name = "Build timeout", + .hint = hintfmt( + "%1% timed out after %2% seconds", + goal->getName(), settings.buildTimeout) + }); goal->timedOut(); } } @@ -4939,7 +4979,12 @@ bool Worker::pathContentsGood(const StorePath & path) res = info->narHash == nullHash || info->narHash == current.first; } pathContentsGoodCache.insert_or_assign(path.clone(), res); - if (!res) printError("path '%s' is corrupted or missing!", store.printStorePath(path)); + if (!res) + logError( + ErrorInfo { + .name = "Corrupted path", + .hint = hintfmt("path '%s' is corrupted or missing!", store.printStorePath(path)) + }); return res; } diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc index 52d430ffd..926f4ea1e 100644 --- a/src/libstore/pathlocks.cc +++ b/src/libstore/pathlocks.cc @@ -160,7 +160,8 @@ void PathLocks::unlock() if (close(i.first) == -1) printError( - format("error (ignored): cannot close lock file on '%1%'") % i.second); + "error (ignored): cannot close lock file on '%1%'", + i.second); debug(format("lock released on '%1%'") % i.second); } diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 0326821f6..3175eb69b 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -111,7 +111,9 @@ class RetryStrategy : public Aws::Client::DefaultRetryStrategy auto retry = Aws::Client::DefaultRetryStrategy::ShouldRetry(error, attemptedRetries); if (retry) printError("AWS error '%s' (%s), will retry in %d ms", - error.GetExceptionName(), error.GetMessage(), CalculateDelayBeforeNextRetry(error, attemptedRetries)); + error.GetExceptionName(), + error.GetMessage(), + CalculateDelayBeforeNextRetry(error, attemptedRetries)); return retry; } }; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index a04d5013e..542e5c552 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -961,7 +961,7 @@ std::list> getDefaultSubstituters() try { stores.push_back(openStore(uri)); } catch (Error & e) { - printError("warning: %s", e.what()); + logWarning(e.info()); } }; diff --git a/src/libutil/affinity.cc b/src/libutil/affinity.cc index 98f8287ad..e3cf33b58 100644 --- a/src/libutil/affinity.cc +++ b/src/libutil/affinity.cc @@ -25,7 +25,7 @@ void setAffinityTo(int cpu) CPU_ZERO(&newAffinity); CPU_SET(cpu, &newAffinity); if (sched_setaffinity(0, sizeof(cpu_set_t), &newAffinity) == -1) - printError(format("failed to lock thread to CPU %1%") % cpu); + printError("failed to lock thread to CPU %1%", cpu); #endif } @@ -47,7 +47,7 @@ void restoreAffinity() #if __linux__ if (!didSaveAffinity) return; if (sched_setaffinity(0, sizeof(cpu_set_t), &savedAffinity) == -1) - printError("failed to restore affinity %1%"); + _printError("failed to restore affinity"); #endif } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 48e6311bd..86cff5609 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -117,6 +117,12 @@ public: } { } + BaseError(hintformat hint) + : err { .level = lvlError, + .hint = hint + } + { } + BaseError(ErrorInfo e) : err(e) { } diff --git a/src/libutil/logging.hh b/src/libutil/logging.hh index 0c4980b83..5f03cdf41 100644 --- a/src/libutil/logging.hh +++ b/src/libutil/logging.hh @@ -149,6 +149,7 @@ extern Verbosity verbosity; /* suppress msgs > this */ } \ } while (0) +#define _printError(args...) printMsg(lvlError, args) #define printError(args...) printMsg(lvlError, args) #define printInfo(args...) printMsg(lvlInfo, args) #define printTalkative(args...) printMsg(lvlTalkative, args) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 747a9e991..68dc1b738 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1417,15 +1417,15 @@ string base64Decode(const string & s) } -void callFailure(const std::function & failure, std::exception_ptr exc) -{ - try { - failure(exc); - } catch (std::exception & e) { - printError("uncaught exception: %s", e.what()); - abort(); - } -} +// void callFailure(const std::function & failure, std::exception_ptr exc) +// { +// try { +// failure(exc); +// } catch (std::exception & e) { +// printError("uncaught exception: %s", e.what()); +// abort(); +// } +// } static Sync> windowSize{{0, 0}}; diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 0a058a31b..401c8d340 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -368,6 +368,8 @@ static void _main(int argc, char * * argv) shell = drv->queryOutPath() + "/bin/bash"; } catch (Error & e) { + // TODO: append error msg + logError(e.info()); printError("warning: %s; will use bash from your environment", e.what()); shell = "bash"; } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 5f78ab464..3cd2c3a6b 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -247,7 +247,8 @@ static void daemonLoop(char * * argv) } catch (Interrupted & e) { return; } catch (Error & e) { - printError("error processing connection: %1%", e.msg()); + // TODO append error message + _printError("error processing connection: %1%", e.msg()); } } } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index dde8875f1..76008d00c 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -123,7 +123,11 @@ static void getAllExprs(EvalState & state, if (hasSuffix(attrName, ".nix")) attrName = string(attrName, 0, attrName.size() - 4); if (!attrs.insert(attrName).second) { - printError(format("warning: name collision in input Nix expressions, skipping '%1%'") % path2); + logError( + ErrorInfo { + .name = "Name collision", + .hint = hintfmt("warning: name collision in input Nix expressions, skipping '%1%'", path2) + }); continue; } /* Load the expression on demand. */ @@ -860,7 +864,12 @@ static void queryJSON(Globals & globals, vector & elems) auto placeholder = metaObj.placeholder(j); Value * v = i.queryMeta(j); if (!v) { - printError("derivation '%s' has invalid meta attribute '%s'", i.queryName(), j); + logError( + ErrorInfo { + .name = "Invalid meta attribute", + .hint = hintfmt("derivation '%s' has invalid meta attribute '%s'", + i.queryName(), j) + }); placeholder.write(nullptr); } else { PathSet context; @@ -1110,8 +1119,14 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) XMLAttrs attrs2; attrs2["name"] = j; Value * v = i.queryMeta(j); - if (!v) - printError("derivation '%s' has invalid meta attribute '%s'", i.queryName(), j); + if (!v) + logError( + ErrorInfo { + .name = "Invalid meta attribute", + .hint = hintfmt( + "derivation '%s' has invalid meta attribute '%s'", + i.queryName(), j) + }); else { if (v->type == tString) { attrs2["type"] = "string"; diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index f852916d8..2484b9759 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -146,7 +146,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, Path lockTokenCur = optimisticLockProfile(profile); if (lockToken != lockTokenCur) { - printError(format("profile '%1%' changed while we were busy; restarting") % profile); + printError("profile '%1%' changed while we were busy; restarting", profile); return false; } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 1e8a88a7b..2e2276b4a 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -726,9 +726,15 @@ static void opVerifyPath(Strings opFlags, Strings opArgs) store->narFromPath(path, sink); auto current = sink.finish(); if (current.first != info->narHash) { - printError( - "path '%s' was modified! expected hash '%s', got '%s'", - store->printStorePath(path), info->narHash.to_string(), current.first.to_string()); + logError( + ErrorInfo { + .name = "Hash match error", + .hint = hintfmt( + "path '%s' was modified! expected hash '%s', got '%s'", + store->printStorePath(path), + info->narHash.to_string(), + current.first.to_string()) + }); status = 1; } } @@ -835,7 +841,8 @@ static void opServe(Strings opFlags, Strings opArgs) for (auto & p : willSubstitute) subs.emplace_back(p.clone()); store->buildPaths(subs); } catch (Error & e) { - printError("warning: %1%", e.msg()); + // logWarning(e.info()) TODO: + _printError("warning: %1%", e.msg()); } } diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc index c05c29517..8f4e529bc 100644 --- a/src/nix/upgrade-nix.cc +++ b/src/nix/upgrade-nix.cc @@ -64,7 +64,13 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand if (dryRun) { stopProgressBar(); - printError("would upgrade to version %s", version); + // TODO change to info? + logWarning( + ErrorInfo { + .name = "Version update", + .hint = hintfmt("would upgrade to version %s", version) + }); + // printError("would upgrade to version %s", version); return; } diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 6e043dc2d..f53217239 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -97,9 +97,15 @@ struct CmdVerify : StorePathsCommand if (hash.first != info->narHash) { corrupted++; act2.result(resCorruptedPath, store->printStorePath(info->path)); - printError( - "path '%s' was modified! expected hash '%s', got '%s'", - store->printStorePath(info->path), info->narHash.to_string(), hash.first.to_string()); + logError( + ErrorInfo { + .name = "Hash error - path modified", + .hint = hintfmt( + "path '%s' was modified! expected hash '%s', got '%s'", + store->printStorePath(info->path), + info->narHash.to_string(), + hash.first.to_string()) + }); } } @@ -148,7 +154,13 @@ struct CmdVerify : StorePathsCommand if (!good) { untrusted++; act2.result(resUntrustedPath, store->printStorePath(info->path)); - printError("path '%s' is untrusted", store->printStorePath(info->path)); + logError( + ErrorInfo { + .name = "Untrusted path", + .hint = hintfmt("path '%s' is untrusted", + store->printStorePath(info->path)) + }); + } } diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc index 8f0c99c84..dcea72529 100644 --- a/src/resolve-system-dependencies/resolve-system-dependencies.cc +++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc @@ -39,12 +39,20 @@ std::set runResolver(const Path & filename) throw SysError("statting '%s'", filename); if (!S_ISREG(st.st_mode)) { - printError("file '%s' is not a regular file", filename); + logError( + ErrorInfo { + .name = "Regular MACH file", + .hint = hintfmt("file '%s' is not a regular file", filename) + }); return {}; } if (st.st_size < sizeof(mach_header_64)) { - printError("file '%s' is too short for a MACH binary", filename); + logError( + ErrorInfo { + .name = "File too short", + .hint = hintfmt("file '%s' is too short for a MACH binary", filename) + }); return {}; } @@ -66,13 +74,21 @@ std::set runResolver(const Path & filename) } } if (mach64_offset == 0) { - printError(format("Could not find any mach64 blobs in file '%1%', continuing...") % filename); + logError( + ErrorInfo { + .name = "No mach64 blobs", + .hint = hintfmt("Could not find any mach64 blobs in file '%1%', continuing...", filename) + }); return {}; } } else if (magic == MH_MAGIC_64 || magic == MH_CIGAM_64) { mach64_offset = 0; } else { - printError(format("Object file has unknown magic number '%1%', skipping it...") % magic); + logError( + ErrorInfo { + .name = "Magic number", + .hint = hintfmt("Object file has unknown magic number '%1%', skipping it...", magic) + }); return {}; }