From b26562c629604b76c58b789ee520581cc5549433 Mon Sep 17 00:00:00 2001 From: dramforever Date: Mon, 23 Jan 2023 23:06:57 +0800 Subject: [PATCH 1/3] InstallableFlake: Handle missing attr in getCursors Handle the case where none of getActualAttrPaths() actually exists, in which case instead of returning an empty vector. This fixes the case where the user misspells the attribute name in nix search. Instead of getting no search results, now it shows an error with suggestions. Also remove InstallableFlake::getCursor() override since it's now equivalent to the base class version. --- src/libcmd/installables.cc | 44 +++++++++++--------------------------- src/libcmd/installables.hh | 4 ---- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 5090ea6d2..f8632a535 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -696,46 +696,28 @@ InstallableFlake::getCursors(EvalState & state) std::vector> res; - for (auto & attrPath : getActualAttrPaths()) { - auto attr = root->findAlongAttrPath(parseAttrPath(state, attrPath)); - if (attr) res.push_back(ref(*attr)); - } - - return res; -} - -ref InstallableFlake::getCursor(EvalState & state) -{ - auto lockedFlake = getLockedFlake(); - - auto cache = openEvalCache(state, lockedFlake); - auto root = cache->getRoot(); - Suggestions suggestions; - auto attrPaths = getActualAttrPaths(); for (auto & attrPath : attrPaths) { debug("trying flake output attribute '%s'", attrPath); - auto attrOrSuggestions = root->findAlongAttrPath( - parseAttrPath(state, attrPath), - true - ); - - if (!attrOrSuggestions) { - suggestions += attrOrSuggestions.getSuggestions(); - continue; + auto attr = root->findAlongAttrPath(parseAttrPath(state, attrPath)); + if (attr) { + res.push_back(ref(*attr)); + } else { + suggestions += attr.getSuggestions(); } - - return *attrOrSuggestions; } - throw Error( - suggestions, - "flake '%s' does not provide attribute %s", - flakeRef, - showAttrPaths(attrPaths)); + if (res.size() == 0) + throw Error( + suggestions, + "flake '%s' does not provide attribute %s", + flakeRef, + showAttrPaths(attrPaths)); + + return res; } std::shared_ptr InstallableFlake::getLockedFlake() const diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 3d12639b0..d489315df 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -198,10 +198,6 @@ struct InstallableFlake : InstallableValue std::vector> getCursors(EvalState & state) override; - /* Get a cursor to the first attrpath in getActualAttrPaths() that - exists, or throw an exception with suggestions if none exists. */ - ref getCursor(EvalState & state) override; - std::shared_ptr getLockedFlake() const; FlakeRef nixpkgsFlakeRef() const override; From 377d5eb38815d8290ab444c81c7826f46ba79844 Mon Sep 17 00:00:00 2001 From: dramforever Date: Mon, 30 Jan 2023 23:56:27 +0800 Subject: [PATCH 2/3] Installable::getCursors: Cleanup - Clarify doc comments, Installables::getCursors returns non-empty vector - Use vector::at in Installable::getCursor instead of checking for empty vector and throwing an exception with error message. --- src/libcmd/installables.cc | 7 +++---- src/libcmd/installables.hh | 8 ++++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index f8632a535..24f458f1a 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -379,10 +379,9 @@ Installable::getCursors(EvalState & state) ref Installable::getCursor(EvalState & state) { - auto cursors = getCursors(state); - if (cursors.empty()) - throw Error("cannot find flake attribute '%s'", what()); - return cursors[0]; + /* Although getCursors should return at least one element, in case it doesn't, + bound check to avoid an undefined behavior for vector[0] */ + return getCursors(state).at(0); } static StorePath getDeriver( diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index d489315df..da6a3addd 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -103,9 +103,13 @@ struct Installable return {}; } + /* Get a cursor to each value this Installable could refer to. However + if none exists, throw exception instead of returning empty vector. */ virtual std::vector> getCursors(EvalState & state); + /* Get the first and most preferred cursor this Installable could refer + to, or throw an exception if none exists. */ virtual ref getCursor(EvalState & state); @@ -193,8 +197,8 @@ struct InstallableFlake : InstallableValue std::pair toValue(EvalState & state) override; - /* Get a cursor to every attrpath in getActualAttrPaths() that - exists. */ + /* Get a cursor to every attrpath in getActualAttrPaths() + that exists. However if none exists, throw an exception. */ std::vector> getCursors(EvalState & state) override; From 6b779e4b07c42e5780feda5daf6dedc6c49e50d7 Mon Sep 17 00:00:00 2001 From: dramforever Date: Mon, 23 Jan 2023 23:07:34 +0800 Subject: [PATCH 3/3] Fix extra "." in CmdSearch::getDefaultFlakeAttrPaths No other getDefaultFlakeAttrPaths implementation has this trailing dot, and the dot can show up in error messages like: error: flake '...' does not provide attribute 'packages.x86_64-linux.', ... --- src/nix/search.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/search.cc b/src/nix/search.cc index d2a31607d..4fa1e7837 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -56,8 +56,8 @@ struct CmdSearch : InstallableCommand, MixJSON Strings getDefaultFlakeAttrPaths() override { return { - "packages." + settings.thisSystem.get() + ".", - "legacyPackages." + settings.thisSystem.get() + "." + "packages." + settings.thisSystem.get(), + "legacyPackages." + settings.thisSystem.get() }; }