eval cache: guard behind an eval setting whether or not to cache errors

When I run `nix build` on a broken attribute in a flake twice, I get the
dreaded

    error: cached failure of attribute 'snens'

This also happens when you re-run the previous command with
`--show-trace`. While trying to fix that, Linus and I discovered that
the eval cache was broken for use on local flakes (i.e. `nix build .#`)
was broken which got fixed in the commit before.

We decided to use an eval setting for that because `forceError` seems
to have been added somewhat arbitrarily as argument to the functions of
AttrCursor where this was needed at the time it was developed. The effect
was that it was hard for us to understand when an invocation can force
errors and when it doesn't. For instance, `AttrCursor::isDerivation()`
never forces re-evaluation of errors, so if `maybeGetAttr("type")`
is the first thing that breaks eval, you'd always run into a cached
failure.

For now, this is `true` by default because I think the current behavior
with a properly working eval cache is pretty painful while hacking on
(or with) a flake. Potential future improvements are

* Setting `evalSettings.forceErrorsInEvalCache` to `false` for
  commands like `nix search`.

* Allowing the eval cache to cache the full failure (right now,
  `failed_t` is empty). That way, you could get errors when re-running
  `nix build` without having to re-evaluate everything, perhaps even
  with traced.

  That's the solution Linus and I would prefer.

Co-authored-by: Linus Heckemann <git@sphalerite.org>

src/libexpr/eval-cache.cc

@@ -3,6 +3,7 @@
 #include "sqlite.hh"
 #include "eval.hh"
 #include "eval-inline.hh"
+#include "eval-settings.hh"
 #include "store-api.hh"
 
 namespace nix::eval_cache {
@@ -470,7 +471,7 @@ Suggestions AttrCursor::getSuggestionsForAttr(Symbol name)
     return Suggestions::bestMatches(strAttrNames, root->state.symbols[name]);
 }
 
-std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(Symbol name, bool forceErrors)
+std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(Symbol name)
 {
     if (root->db) {
         if (!cachedValue)
@@ -488,7 +489,7 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(Symbol name, bool forceErro
                     if (std::get_if<missing_t>(&attr->second))
                         return nullptr;
                     else if (std::get_if<failed_t>(&attr->second)) {
-                        if (forceErrors)
+                        if (evalSettings.forceErrorsInEvalCache)
                             debug("reevaluating failed cached attribute '%s'", getAttrPathStr(name));
                         else
                             throw CachedEvalError(root->state, "cached failure of attribute '%s'", getAttrPathStr(name));
@@ -537,9 +538,9 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(std::string_view name)
     return maybeGetAttr(root->state.symbols.create(name));
 }
 
-ref<AttrCursor> AttrCursor::getAttr(Symbol name, bool forceErrors)
+ref<AttrCursor> AttrCursor::getAttr(Symbol name)
 {
-    auto p = maybeGetAttr(name, forceErrors);
+    auto p = maybeGetAttr(name);
     if (!p)
         throw Error("attribute '%s' does not exist", getAttrPathStr(name));
     return ref(p);
@@ -550,11 +551,11 @@ ref<AttrCursor> AttrCursor::getAttr(std::string_view name)
     return getAttr(root->state.symbols.create(name));
 }
 
-OrSuggestions<ref<AttrCursor>> AttrCursor::findAlongAttrPath(const std::vector<Symbol> & attrPath, bool force)
+OrSuggestions<ref<AttrCursor>> AttrCursor::findAlongAttrPath(const std::vector<Symbol> & attrPath)
 {
     auto res = shared_from_this();
     for (auto & attr : attrPath) {
-        auto child = res->maybeGetAttr(attr, force);
+        auto child = res->maybeGetAttr(attr);
         if (!child) {
             auto suggestions = res->getSuggestionsForAttr(attr);
             return OrSuggestions<ref<AttrCursor>>::failed(suggestions);
@@ -691,7 +692,7 @@ std::vector<std::string> AttrCursor::getListOfStrings()
         }
     }
 
-    debug("evaluating uncached attribute '%s'", getAttrPathStr());
+    debug("[AttrCursor::getListOfStrings] evaluating uncached attribute '%s'", getAttrPathStr());
 
     auto & v = getValue();
     root->state.forceValue(v, noPos);
@@ -751,7 +752,7 @@ bool AttrCursor::isDerivation()
 
 StorePath AttrCursor::forceDerivation()
 {
-    auto aDrvPath = getAttr(root->state.sDrvPath, true);
+    auto aDrvPath = getAttr(root->state.sDrvPath);
     auto drvPath = root->state.store->parseStorePath(aDrvPath->getString());
     if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) {
         /* The eval cache contains 'drvPath', but the actual path has

src/libexpr/eval-cache.hh

@@ -102,19 +102,19 @@ public:
 
     Suggestions getSuggestionsForAttr(Symbol name);
 
-    std::shared_ptr<AttrCursor> maybeGetAttr(Symbol name, bool forceErrors = false);
+    std::shared_ptr<AttrCursor> maybeGetAttr(Symbol name);
 
     std::shared_ptr<AttrCursor> maybeGetAttr(std::string_view name);
 
-    ref<AttrCursor> getAttr(Symbol name, bool forceErrors = false);
+    ref<AttrCursor> getAttr(Symbol name);
 
     ref<AttrCursor> getAttr(std::string_view name);
 
     /**
      * Get an attribute along a chain of attrsets. Note that this does
      * not auto-call functors or functions.
      */
-    OrSuggestions<ref<AttrCursor>> findAlongAttrPath(const std::vector<Symbol> & attrPath, bool force = false);
+    OrSuggestions<ref<AttrCursor>> findAlongAttrPath(const std::vector<Symbol> & attrPath);
 
     std::string getString();
 

src/libexpr/eval-settings.hh

@@ -128,6 +128,9 @@ struct EvalSettings : Config
     Setting<bool> useEvalCache{this, true, "eval-cache",
         "Whether to use the flake evaluation cache."};
 
+    Setting<bool> forceErrorsInEvalCache{this, true, "force-errors-in-eval-cache",
+        "Whether to force reevaluation of cached failures."};
+
     Setting<bool> ignoreExceptionsDuringTry{this, false, "ignore-try",
         R"(
           If set to true, ignore exceptions inside 'tryEval' calls when evaluating nix expressions in

tests/functional/flakes/eval-cache.sh

@@ -30,11 +30,15 @@ nix flake lock "$flake1Dir"
 # FIXME `packages.$system.foo` requires three invocations of `nix build`,
 # for `foo = throw` only it'll never be cached.
 expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'
-expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: cached failure of attribute '"'foo.bar'"
+expect 1 nix build "$flake1Dir#foo.bar" --no-force-errors-in-eval-cache 2>&1 | \
+    grepQuiet 'error: cached failure of attribute '"'foo.bar'"
+expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'
 
 git -C "$flake1Dir" add flake.lock
 git -C "$flake1Dir" commit -m "Init"
 
 # Cache is invalidated with a new git revision
 expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'
-expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: cached failure of attribute '"'foo.bar'"
+expect 1 nix build "$flake1Dir#foo.bar" --no-force-errors-in-eval-cache 2>&1 | \
+    grepQuiet 'error: cached failure of attribute '"'foo.bar'"
+expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'