From 8b438fccb4fce1e8c06136ff9f9bae324911c193 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 31 Mar 2025 15:03:57 +0200 Subject: [PATCH 1/2] Throw CachedEvalError if a cached value exists but has type "failed" Otherwise you get unhelpful errors like error: 'apps' is not an attribute set Fixes #12762. --- src/libexpr/eval-cache.cc | 29 +++++++++++++++-------------- src/libexpr/eval-cache.hh | 8 ++++++++ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index ea3319f99..0042eebe0 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -418,6 +418,14 @@ Value & AttrCursor::getValue() return **_value; } +void AttrCursor::fetchCachedValue() +{ + if (!cachedValue) + cachedValue = root->db->getAttr(getKey()); + if (cachedValue && std::get_if(&cachedValue->second) && parent) + throw CachedEvalError(ref(parent->first), parent->second); +} + std::vector AttrCursor::getAttrPath() const { if (parent) { @@ -494,8 +502,7 @@ Suggestions AttrCursor::getSuggestionsForAttr(Symbol name) std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue) { if (auto attrs = std::get_if>(&cachedValue->second)) { @@ -585,8 +592,7 @@ OrSuggestions> AttrCursor::findAlongAttrPath(const std::vectordb) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto s = std::get_if(&cachedValue->second)) { debug("using cached string attribute '%s'", getAttrPathStr()); @@ -607,8 +613,7 @@ std::string AttrCursor::getString() string_t AttrCursor::getStringWithContext() { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto s = std::get_if(&cachedValue->second)) { bool valid = true; @@ -654,8 +659,7 @@ string_t AttrCursor::getStringWithContext() bool AttrCursor::getBool() { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto b = std::get_if(&cachedValue->second)) { debug("using cached Boolean attribute '%s'", getAttrPathStr()); @@ -676,8 +680,7 @@ bool AttrCursor::getBool() NixInt AttrCursor::getInt() { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto i = std::get_if(&cachedValue->second)) { debug("using cached integer attribute '%s'", getAttrPathStr()); @@ -698,8 +701,7 @@ NixInt AttrCursor::getInt() std::vector AttrCursor::getListOfStrings() { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto l = std::get_if>(&cachedValue->second)) { debug("using cached list of strings attribute '%s'", getAttrPathStr()); @@ -731,8 +733,7 @@ std::vector AttrCursor::getListOfStrings() std::vector AttrCursor::getAttrs() { if (root->db) { - if (!cachedValue) - cachedValue = root->db->getAttr(getKey()); + fetchCachedValue(); if (cachedValue && !std::get_if(&cachedValue->second)) { if (auto attrs = std::get_if>(&cachedValue->second)) { debug("using cached attrset attribute '%s'", getAttrPathStr()); diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index b1911e3a4..d37303b31 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -99,6 +99,14 @@ class AttrCursor : public std::enable_shared_from_this Value & getValue(); + /** + * If `cachedValue` is unset, try to initialize it from the + * database. It is not an error if it does not exist. Throw a + * `CachedEvalError` exception if it does exist but has type + * `AttrType::Failed`. + */ + void fetchCachedValue(); + public: AttrCursor( From 5a357459497c5111207fba63af21e5cdd6a945c0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 31 Mar 2025 15:14:10 +0200 Subject: [PATCH 2/2] AttrCursor::Parent: shared_ptr -> ref --- src/libexpr/eval-cache.cc | 8 ++++---- src/libexpr/eval-cache.hh | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 0042eebe0..44e46311e 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -423,7 +423,7 @@ void AttrCursor::fetchCachedValue() if (!cachedValue) cachedValue = root->db->getAttr(getKey()); if (cachedValue && std::get_if(&cachedValue->second) && parent) - throw CachedEvalError(ref(parent->first), parent->second); + throw CachedEvalError(parent->first, parent->second); } std::vector AttrCursor::getAttrPath() const @@ -508,7 +508,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) if (auto attrs = std::get_if>(&cachedValue->second)) { for (auto & attr : *attrs) if (attr == name) - return std::make_shared(root, std::make_pair(shared_from_this(), attr)); + return std::make_shared(root, std::make_pair(ref(shared_from_this()), attr)); return nullptr; } else if (std::get_if(&cachedValue->second)) { auto attr = root->db->getAttr({cachedValue->first, name}); @@ -519,7 +519,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) throw CachedEvalError(ref(shared_from_this()), name); else return std::make_shared(root, - std::make_pair(shared_from_this(), name), nullptr, std::move(attr)); + std::make_pair(ref(shared_from_this()), name), nullptr, std::move(attr)); } // Incomplete attrset, so need to fall thru and // evaluate to see whether 'name' exists @@ -554,7 +554,7 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) } return make_ref( - root, std::make_pair(shared_from_this(), name), attr->value, std::move(cachedValue2)); + root, std::make_pair(ref(shared_from_this()), name), attr->value, std::move(cachedValue2)); } std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name) diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index d37303b31..51b7ee365 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -90,7 +90,7 @@ class AttrCursor : public std::enable_shared_from_this friend struct CachedEvalError; ref root; - typedef std::optional, Symbol>> Parent; + using Parent = std::optional, Symbol>>; Parent parent; RootValue _value; std::optional> cachedValue;