Merge pull request #11445 from eclairevoyant/backport-10564-to-2.18

[backport 2.18-maintenance] AttrCursor: Remove forceErrors

src/libexpr/eval-cache.cc

@@ -6,6 +6,25 @@
 
 namespace nix::eval_cache {
 
+CachedEvalError::CachedEvalError(ref<EvalState> state, ref<AttrCursor> cursor, Symbol attr)
+    : EvalError("cached failure of attribute '%s'", cursor->getAttrPathStr(attr))
+    , state(state), cursor(cursor), attr(attr)
+{ }
+
+void CachedEvalError::force()
+{
+    auto & v = cursor->forceValue();
+
+    if (v.type() == nAttrs) {
+        auto a = v.attrs->get(this->attr);
+
+        state->forceValue(*a->value, a->pos);
+    }
+
+    // Shouldn't happen.
+    throw EvalError("evaluation of cached failed attribute '%s' unexpectedly succeeded", cursor->getAttrPathStr(attr));
+}
+
 static const char * schema = R"sql(
 create table if not exists Attributes (
     parent      integer not null,
@@ -469,7 +488,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)
@@ -486,12 +505,9 @@ std::shared_ptr<AttrCursor> AttrCursor::maybeGetAttr(Symbol name, bool forceErro
                 if (attr) {
                     if (std::get_if<missing_t>(&attr->second))
                         return nullptr;
-                    else if (std::get_if<failed_t>(&attr->second)) {
+                    else if (std::get_if<failed_t>(&attr->second))
-                        if (forceErrors)
+                        throw CachedEvalError(ref(root->state.shared_from_this()), ref(shared_from_this()), name);
-                            debug("reevaluating failed cached attribute '%s'", getAttrPathStr(name));
+                    else
-                        else
-                            throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(name));
-                    } else
                         return std::make_shared<AttrCursor>(root,
                             std::make_pair(shared_from_this(), name), nullptr, std::move(attr));
                 }
@@ -536,9 +552,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);
@@ -549,11 +565,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);
@@ -750,7 +766,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

@@ -10,14 +10,29 @@
 
 namespace nix::eval_cache {
 
-MakeError(CachedEvalError, EvalError);
-
 struct AttrDb;
 class AttrCursor;
 
+struct CachedEvalError : EvalError
+{
+    const ref<EvalState> state;
+    const ref<AttrCursor> cursor;
+    const Symbol attr;
+
+    CachedEvalError(ref<EvalState>, ref<AttrCursor> cursor, Symbol attr);
+
+    /**
+     * Evaluate this attribute, which should result in a regular
+     * `EvalError` exception being thrown.
+     */
+    [[noreturn]]
+    void force();
+};
+
 class EvalCache : public std::enable_shared_from_this<EvalCache>
 {
     friend class AttrCursor;
+    friend class CachedEvalError;
 
     std::shared_ptr<AttrDb> db;
     EvalState & state;
@@ -73,6 +88,7 @@ typedef std::variant<
 class AttrCursor : public std::enable_shared_from_this<AttrCursor>
 {
     friend class EvalCache;
+    friend class CachedEvalError;
 
     ref<EvalCache> root;
     typedef std::optional<std::pair<std::shared_ptr<AttrCursor>, Symbol>> Parent;
@@ -102,11 +118,11 @@ 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);
 
@@ -114,7 +130,7 @@ public:
      * 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/nix/flake.cc

@@ -1110,7 +1110,7 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
                 // If we don't recognize it, it's probably content
                 return true;
             } catch (EvalError & e) {
-                // Some attrs may contain errors, eg. legacyPackages of
+                // Some attrs may contain errors, e.g. legacyPackages of
                 // nixpkgs. We still want to recurse into it, instead of
                 // skipping it at all.
                 return true;

src/nix/main.cc

@@ -12,6 +12,7 @@
 #include "finally.hh"
 #include "loggers.hh"
 #include "markdown.hh"
+#include "eval-cache.hh"
 
 #include <sys/types.h>
 #include <sys/socket.h>
@@ -480,7 +481,15 @@ void mainWrapped(int argc, char * * argv)
     if (args.command->second->forceImpureByDefault() && !evalSettings.pureEval.overridden) {
         evalSettings.pureEval = false;
     }
-    args.command->second->run();
+
+    try {
+        args.command->second->run();
+    } catch (eval_cache::CachedEvalError & e) {
+        /* Evaluate the original attribute that resulted in this
+           cached error so that we can show the original error to the
+           user. */
+        e.force();
+    }
 }
 
 }

tests/functional/flakes/eval-cache.sh

@@ -0,0 +1,22 @@
+source ./common.sh
+
+requireGit
+
+flake1Dir="$TEST_ROOT/eval-cache-flake"
+
+createGitRepo "$flake1Dir" ""
+
+cat >"$flake1Dir/flake.nix" <<EOF
+{
+  description = "Fnord";
+  outputs = { self }: {
+    foo.bar = throw "breaks";
+  };
+}
+EOF
+
+git -C "$flake1Dir" add flake.nix
+git -C "$flake1Dir" commit -m "Init"
+
+expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'
+expect 1 nix build "$flake1Dir#foo.bar" 2>&1 | grepQuiet 'error: breaks'

tests/functional/local.mk

@@ -14,6 +14,7 @@ nix_tests = \
   flakes/absolute-paths.sh \
   flakes/build-paths.sh \
   flakes/flake-in-submodule.sh \
+  flakes/eval-cache.sh \
   gc.sh \
   nix-collect-garbage-d.sh \
   remote-store.sh \