From accfd8aa9d786831e6e08e92162891e99e11c08a Mon Sep 17 00:00:00 2001 From: Qyriad Date: Sun, 7 Jul 2024 05:47:41 -0600 Subject: [PATCH] libexpr: stop lying about DrvInfo's constness DrvInfo's query methods all use mutable fields to cache, but like. that's basically the entire interface for DrvInfo. Not only that, but these formerly-const-marked functions can even throw due to eval errors! Changing this only required removing some `const` markers in nix-env, and changing a single inline `queryInstalled()` call to be an lvalue instead. Change-Id: I796807118f3b35b0e93668b5e28210d9e521b2ae --- src/libexpr/get-drvs.cc | 12 ++++++------ src/libexpr/get-drvs.hh | 22 +++++++++++----------- src/nix-env/nix-env.cc | 21 ++++++++++++--------- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 8c8e142b8..b199cd09e 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -45,7 +45,7 @@ DrvInfo::DrvInfo(EvalState & state, ref store, const std::string & drvPat } -std::string DrvInfo::queryName() const +std::string DrvInfo::queryName() { if (name == "" && attrs) { auto i = attrs->find(state->sName); @@ -56,7 +56,7 @@ std::string DrvInfo::queryName() const } -std::string DrvInfo::querySystem() const +std::string DrvInfo::querySystem() { if (system == "" && attrs) { auto i = attrs->find(state->sSystem); @@ -66,7 +66,7 @@ std::string DrvInfo::querySystem() const } -std::optional DrvInfo::queryDrvPath() const +std::optional DrvInfo::queryDrvPath() { if (!drvPath && attrs) { Bindings::iterator i = attrs->find(state->sDrvPath); @@ -80,7 +80,7 @@ std::optional DrvInfo::queryDrvPath() const } -StorePath DrvInfo::requireDrvPath() const +StorePath DrvInfo::requireDrvPath() { if (auto drvPath = queryDrvPath()) return *drvPath; @@ -88,7 +88,7 @@ StorePath DrvInfo::requireDrvPath() const } -StorePath DrvInfo::queryOutPath() const +StorePath DrvInfo::queryOutPath() { if (!outPath && attrs) { Bindings::iterator i = attrs->find(state->sOutPath); @@ -164,7 +164,7 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall } -std::string DrvInfo::queryOutputName() const +std::string DrvInfo::queryOutputName() { if (outputName == "" && attrs) { Bindings::iterator i = attrs->find(state->sOutputName); diff --git a/src/libexpr/get-drvs.hh b/src/libexpr/get-drvs.hh index 584d64ac1..d620eca63 100644 --- a/src/libexpr/get-drvs.hh +++ b/src/libexpr/get-drvs.hh @@ -19,11 +19,11 @@ public: private: EvalState * state; - mutable std::string name; - mutable std::string system; - mutable std::optional> drvPath; - mutable std::optional outPath; - mutable std::string outputName; + std::string name; + std::string system; + std::optional> drvPath; + std::optional outPath; + std::string outputName; Outputs outputs; /** @@ -47,12 +47,12 @@ public: DrvInfo(EvalState & state, std::string attrPath, Bindings * attrs); DrvInfo(EvalState & state, ref store, const std::string & drvPathWithOutputs); - std::string queryName() const; - std::string querySystem() const; - std::optional queryDrvPath() const; - StorePath requireDrvPath() const; - StorePath queryOutPath() const; - std::string queryOutputName() const; + std::string queryName(); + std::string querySystem(); + std::optional queryDrvPath(); + StorePath requireDrvPath(); + StorePath queryOutPath(); + std::string queryOutputName(); /** * Return the unordered map of output names to (optional) output paths. * The "outputs to install" are determined by `meta.outputsToInstall`. diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 283d3a3a7..fdac7e69a 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -236,10 +236,10 @@ static void checkSelectorUse(DrvNames & selectors) namespace { -std::set searchByPrefix(const DrvInfos & allElems, std::string_view prefix) { +std::set searchByPrefix(DrvInfos & allElems, std::string_view prefix) { constexpr std::size_t maxResults = 3; std::set result; - for (const auto & drvInfo : allElems) { + for (auto & drvInfo : allElems) { const auto drvName = DrvName { drvInfo.queryName() }; if (drvName.name.starts_with(prefix)) { result.emplace(drvName.name); @@ -319,7 +319,7 @@ std::vector pickNewestOnly(EvalState & state, std::vector matches) } // end namespace -static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems, +static DrvInfos filterBySelector(EvalState & state, DrvInfos & allElems, const Strings & args, bool newestOnly) { DrvNames selectors = drvNamesFromArgs(args); @@ -331,7 +331,7 @@ static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems, for (auto & selector : selectors) { std::vector matches; - for (const auto & [index, drvInfo] : enumerate(allElems)) { + for (auto && [index, drvInfo] : enumerate(allElems)) { const auto drvName = DrvName { drvInfo.queryName() }; if (selector.matches(drvName)) { ++selector.hits; @@ -457,9 +457,12 @@ static void queryInstSources(EvalState & state, user environment. These are then filtered as in the `srcNixExprDrvs' case. */ case srcProfile: { - elems = filterBySelector(state, - queryInstalled(state, instSource.profile), - args, newestOnly); + auto installed = queryInstalled(state, instSource.profile); + elems = filterBySelector( + state, + installed, + args, newestOnly + ); break; } @@ -838,7 +841,7 @@ static bool cmpChars(char a, char b) } -static bool cmpElemByName(const DrvInfo & a, const DrvInfo & b) +static bool cmpElemByName(DrvInfo & a, DrvInfo & b) { auto a_name = a.queryName(); auto b_name = b.queryName(); @@ -891,7 +894,7 @@ void printTable(Table & table) typedef enum { cvLess, cvEqual, cvGreater, cvUnavail } VersionDiff; static VersionDiff compareVersionAgainstSet( - const DrvInfo & elem, const DrvInfos & elems, std::string & version) + DrvInfo & elem, DrvInfos & elems, std::string & version) { DrvName name(elem.queryName());