From e1de1fe0d82d8ba702947dcad3b678cbb9ce9333 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 23 Jul 2020 19:02:57 +0000 Subject: [PATCH 1/9] Make `Buildable` a `std::variant` I think this better captures the intent of what's going on: we either have an opaque store path, or a drv path with some outputs. Having this structure will also help us support CA derivations: we'll have to allow the outpath paths to be optional, so the structure we gain now makes up for the structure we loose then. --- src/libstore/content-address.cc | 4 -- src/libstore/store-api.cc | 4 -- src/libutil/util.hh | 5 ++ src/nix/build.cc | 29 +++++++---- src/nix/command.cc | 17 ++++-- src/nix/installables.cc | 91 ++++++++++++++++----------------- src/nix/installables.hh | 14 +++-- src/nix/log.cc | 13 +++-- 8 files changed, 98 insertions(+), 79 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 6cb69d0a9..f45f77d5c 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -24,10 +24,6 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) + hash.to_string(Base32, true); } -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - std::string renderContentAddress(ContentAddress ca) { return std::visit(overloaded { [](TextHash th) { diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index ab21b0bd5..0f6f3b98f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -865,10 +865,6 @@ void ValidPathInfo::sign(const Store & store, const SecretKey & secretKey) sigs.insert(secretKey.signDetached(fingerprint(store))); } -// FIXME Put this somewhere? -template struct overloaded : Ts... { using Ts::operator()...; }; -template overloaded(Ts...) -> overloaded; - bool ValidPathInfo::isContentAddressed(const Store & store) const { if (! ca) return false; diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 42130f6dc..630303a5d 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -601,4 +601,9 @@ constexpr auto enumerate(T && iterable) } +// C++17 std::visit boilerplate +template struct overloaded : Ts... { using Ts::operator()...; }; +template overloaded(Ts...) -> overloaded; + + } diff --git a/src/nix/build.cc b/src/nix/build.cc index 0f7e0e123..927301314 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -57,17 +57,24 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixProfile if (dryRun) return; - if (outLink != "") { - for (size_t i = 0; i < buildables.size(); ++i) { - for (auto & output : buildables[i].outputs) - if (auto store2 = store.dynamic_pointer_cast()) { - std::string symlink = outLink; - if (i) symlink += fmt("-%d", i); - if (output.first != "out") symlink += fmt("-%s", output.first); - store2->addPermRoot(output.second, absPath(symlink), true); - } - } - } + if (outLink != "") + if (auto store2 = store.dynamic_pointer_cast()) + for (size_t i = 0; i < buildables.size(); ++i) + std::visit(overloaded { + [&](BuildableOpaque bo) { + std::string symlink = outLink; + if (i) symlink += fmt("-%d", i); + store2->addPermRoot(bo.path, absPath(symlink), true); + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) { + std::string symlink = outLink; + if (i) symlink += fmt("-%d", i); + if (output.first != "out") symlink += fmt("-%s", output.first); + store2->addPermRoot(output.second, absPath(symlink), true); + } + }, + }, buildables[i]); updateProfile(buildables); } diff --git a/src/nix/command.cc b/src/nix/command.cc index af36dda89..04715b43a 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -131,11 +131,18 @@ void MixProfile::updateProfile(const Buildables & buildables) std::optional result; for (auto & buildable : buildables) { - for (auto & output : buildable.outputs) { - if (result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); - result = output.second; - } + std::visit(overloaded { + [&](BuildableOpaque bo) { + result = bo.path; + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) { + if (result) + throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); + result = output.second; + } + }, + }, buildable); } if (!result) diff --git a/src/nix/installables.cc b/src/nix/installables.cc index a13e5a3df..a051950cd 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -307,16 +307,15 @@ struct InstallableStorePath : Installable for (auto & [name, output] : store->readDerivation(storePath).outputs) outputs.emplace(name, output.path); return { - Buildable { + BuildableFromDrv { .drvPath = storePath, .outputs = std::move(outputs) } }; } else { return { - Buildable { - .drvPath = {}, - .outputs = {{"out", storePath}} + BuildableOpaque { + .path = storePath, } }; } @@ -332,33 +331,20 @@ Buildables InstallableValue::toBuildables() { Buildables res; - StorePathSet drvPaths; + std::map drvsToOutputs; + // Group by derivation, helps with .all in particular for (auto & drv : toDerivations()) { - Buildable b{.drvPath = drv.drvPath}; - drvPaths.insert(drv.drvPath); - auto outputName = drv.outputName; if (outputName == "") - throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(*b.drvPath)); - - b.outputs.emplace(outputName, drv.outPath); - - res.push_back(std::move(b)); + throw Error("derivation '%s' lacks an 'outputName' attribute", state->store->printStorePath(drv.drvPath)); + drvsToOutputs[drv.drvPath].insert_or_assign(outputName, drv.outPath); } - // Hack to recognize .all: if all drvs have the same drvPath, - // merge the buildables. - if (drvPaths.size() == 1) { - Buildable b{.drvPath = *drvPaths.begin()}; - for (auto & b2 : res) - for (auto & output : b2.outputs) - b.outputs.insert_or_assign(output.first, output.second); - Buildables bs; - bs.push_back(std::move(b)); - return bs; - } else - return res; + for (auto & i : drvsToOutputs) + res.push_back(BuildableFromDrv { i.first, i.second }); + + return res; } struct InstallableAttrPath : InstallableValue @@ -653,14 +639,17 @@ Buildables build(ref store, Realise mode, for (auto & i : installables) { for (auto & b : i->toBuildables()) { - if (b.drvPath) { - StringSet outputNames; - for (auto & output : b.outputs) - outputNames.insert(output.first); - pathsToBuild.push_back({*b.drvPath, outputNames}); - } else - for (auto & output : b.outputs) - pathsToBuild.push_back({output.second}); + std::visit(overloaded { + [&](BuildableOpaque bo) { + pathsToBuild.push_back({bo.path}); + }, + [&](BuildableFromDrv bfd) { + StringSet outputNames; + for (auto & output : bfd.outputs) + outputNames.insert(output.first); + pathsToBuild.push_back({bfd.drvPath, outputNames}); + }, + }, b); buildables.push_back(std::move(b)); } } @@ -681,16 +670,23 @@ StorePathSet toStorePaths(ref store, if (operateOn == OperateOn::Output) { for (auto & b : build(store, mode, installables)) - for (auto & output : b.outputs) - outPaths.insert(output.second); + std::visit(overloaded { + [&](BuildableOpaque bo) { + outPaths.insert(bo.path); + }, + [&](BuildableFromDrv bfd) { + for (auto & output : bfd.outputs) + outPaths.insert(output.second); + }, + }, b); } else { if (mode == Realise::Nothing) settings.readOnlyMode = true; for (auto & i : installables) for (auto & b : i->toBuildables()) - if (b.drvPath) - outPaths.insert(*b.drvPath); + if (auto bfd = std::get_if(&b)) + outPaths.insert(bfd->drvPath); } return outPaths; @@ -714,20 +710,21 @@ StorePathSet toDerivations(ref store, StorePathSet drvPaths; for (auto & i : installables) - for (auto & b : i->toBuildables()) { - if (!b.drvPath) { - if (!useDeriver) - throw Error("argument '%s' did not evaluate to a derivation", i->what()); - for (auto & output : b.outputs) { - auto derivers = store->queryValidDerivers(output.second); + for (auto & b : i->toBuildables()) + std::visit(overloaded { + [&](BuildableOpaque bo) { + if (!useDeriver) + throw Error("argument '%s' did not evaluate to a derivation", i->what()); + auto derivers = store->queryValidDerivers(bo.path); if (derivers.empty()) throw Error("'%s' does not have a known deriver", i->what()); // FIXME: use all derivers? drvPaths.insert(*derivers.begin()); - } - } else - drvPaths.insert(*b.drvPath); - } + }, + [&](BuildableFromDrv bfd) { + drvPaths.insert(bfd.drvPath); + }, + }, b); return drvPaths; } diff --git a/src/nix/installables.hh b/src/nix/installables.hh index eb34365d4..9edff3331 100644 --- a/src/nix/installables.hh +++ b/src/nix/installables.hh @@ -14,12 +14,20 @@ struct SourceExprCommand; namespace eval_cache { class EvalCache; class AttrCursor; } -struct Buildable -{ - std::optional drvPath; +struct BuildableOpaque { + StorePath path; +}; + +struct BuildableFromDrv { + StorePath drvPath; std::map outputs; }; +typedef std::variant< + BuildableOpaque, + BuildableFromDrv +> Buildable; + typedef std::vector Buildables; struct App diff --git a/src/nix/log.cc b/src/nix/log.cc index 7e10d373a..33380dcf5 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -45,11 +45,14 @@ struct CmdLog : InstallableCommand RunPager pager; for (auto & sub : subs) { - auto log = b.drvPath ? sub->getBuildLog(*b.drvPath) : nullptr; - for (auto & output : b.outputs) { - if (log) break; - log = sub->getBuildLog(output.second); - } + auto log = std::visit(overloaded { + [&](BuildableOpaque bo) { + return sub->getBuildLog(bo.path); + }, + [&](BuildableFromDrv bfd) { + return sub->getBuildLog(bfd.drvPath); + }, + }, b); if (!log) continue; stopProgressBar(); printInfo("got build log for '%s' from '%s'", installable->what(), sub->getUri()); From a9bbfaa8515de7107457fda2a7aef87aa198788e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 5 Aug 2020 16:27:15 +0000 Subject: [PATCH 2/9] Fix --profile with multiple opaque paths --- src/nix/command.cc | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/nix/command.cc b/src/nix/command.cc index 04715b43a..da32819da 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -128,27 +128,25 @@ void MixProfile::updateProfile(const Buildables & buildables) { if (!profile) return; - std::optional result; + std::vector result; for (auto & buildable : buildables) { std::visit(overloaded { [&](BuildableOpaque bo) { - result = bo.path; + result.push_back(bo.path); }, [&](BuildableFromDrv bfd) { for (auto & output : bfd.outputs) { - if (result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are multiple"); - result = output.second; + result.push_back(output.second); } }, }, buildable); } - if (!result) - throw Error("'--profile' requires that the arguments produce a single store path, but there are none"); + if (result.size() != 1) + throw Error("'--profile' requires that the arguments produce a single store path, but there are %d", result.size()); - updateProfile(*result); + updateProfile(result[0]); } MixDefaultProfile::MixDefaultProfile() From f1a47a96b6ab55d0b0017df5b94b3da69c65bf21 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 10:58:00 -0600 Subject: [PATCH 3/9] error messages for issue 2238 --- src/build-remote/build-remote.cc | 33 +++++++++++++++++++++++++++++++- src/libstore/build.cc | 13 +++++++++++-- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 3579d8fff..2414a47c1 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -103,7 +103,7 @@ static int _main(int argc, char * * argv) drvPath = store->parseStorePath(readString(source)); auto requiredFeatures = readStrings>(source); - auto canBuildLocally = amWilling + auto canBuildLocally = amWilling && ( neededSystem == settings.thisSystem || settings.extraPlatforms.get().count(neededSystem) > 0) && allSupportedLocally(requiredFeatures); @@ -170,7 +170,38 @@ static int _main(int argc, char * * argv) if (rightType && !canBuildLocally) std::cerr << "# postpone\n"; else + { + // build the hint template. + string hintstring = "required (system, features): (%s, %s)"; + hintstring += "\n%s available machines:"; + hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)"; + + for (unsigned int i = 0; i < machines.size(); ++i) { + hintstring += "\n(%s, %s, %s, %s)"; + } + + // add the template values. + auto hint = hintformat(hintstring); + hint + % neededSystem + % concatStringsSep(", ", requiredFeatures) + % machines.size(); + + for (auto & m : machines) { + hint % concatStringsSep>(", ", m.systemTypes) + % m.maxJobs + % concatStringsSep(", ", m.supportedFeatures) + % concatStringsSep(", ", m.mandatoryFeatures); + } + + logError({ + .name = "Remote build", + .description = "Failed to find a machine for remote build!", + .hint = hint + }); + std::cerr << "# decline\n"; + } break; } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 94c398b2f..76baa1a6e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4876,8 +4876,17 @@ void Worker::run(const Goals & _topGoals) waitForInput(); else { if (awake.empty() && 0 == settings.maxBuildJobs) - throw Error("unable to start any build; either increase '--max-jobs' " - "or enable remote builds"); + { + if (getMachines().empty()) + throw Error("unable to start any build; either increase '--max-jobs' " + "or enable remote builds." + "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + else + throw Error("unable to start any build; remote machines may not have " + "all required system features." + "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + + } assert(!awake.empty()); } } From e4eae078a52fa4209bb5a46d21dbed09dd9c54ec Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 11:21:36 -0600 Subject: [PATCH 4/9] add derivation path to hint --- src/build-remote/build-remote.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 2414a47c1..723e1463e 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -172,7 +172,7 @@ static int _main(int argc, char * * argv) else { // build the hint template. - string hintstring = "required (system, features): (%s, %s)"; + string hintstring = "derivation: %s\nrequired (system, features): (%s, %s)"; hintstring += "\n%s available machines:"; hintstring += "\n(systems, maxjobs, supportedFeatures, mandatoryFeatures)"; @@ -183,6 +183,7 @@ static int _main(int argc, char * * argv) // add the template values. auto hint = hintformat(hintstring); hint + % drvPath->to_string() % neededSystem % concatStringsSep(", ", requiredFeatures) % machines.size(); From 31f1af0cabb65e3c5f8f4539500c6b236c387780 Mon Sep 17 00:00:00 2001 From: Ben Burdette Date: Wed, 5 Aug 2020 11:26:06 -0600 Subject: [PATCH 5/9] don't crash if there's no drvPath --- src/build-remote/build-remote.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 723e1463e..5247cefa6 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -181,9 +181,15 @@ static int _main(int argc, char * * argv) } // add the template values. + string drvstr; + if (drvPath.has_value()) + drvstr = drvPath->to_string(); + else + drvstr = ""; + auto hint = hintformat(hintstring); hint - % drvPath->to_string() + % drvstr % neededSystem % concatStringsSep(", ", requiredFeatures) % machines.size(); From 59067f0f587f75908c4f990bcbab29340c7f7652 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 6 Aug 2020 11:40:41 +0200 Subject: [PATCH 6/9] repl.cc: Check for HAVE_BOEHMGC Fixes #3906. --- src/nix/repl.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/nix/repl.cc b/src/nix/repl.cc index fb9050d0d..c3c9e54a8 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -33,12 +33,17 @@ extern "C" { #include "command.hh" #include "finally.hh" +#if HAVE_BOEHMGC #define GC_INCLUDE_NEW #include +#endif namespace nix { -struct NixRepl : gc +struct NixRepl + #if HAVE_BOEHMGC + : gc + #endif { string curDir; std::unique_ptr state; From 2ffc0589507de63bea5555ba993ac195154b4517 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 7 Aug 2020 14:13:24 +0200 Subject: [PATCH 7/9] Make --no-eval-cache a global setting --- src/libexpr/eval.hh | 3 +++ src/nix/app.cc | 2 +- src/nix/flake.cc | 11 ++--------- src/nix/installables.cc | 22 +++++++++------------- src/nix/installables.hh | 9 ++++----- src/nix/search.cc | 2 +- 6 files changed, 20 insertions(+), 29 deletions(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 0382298b3..5855b4ef2 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -375,6 +375,9 @@ struct EvalSettings : Config Setting traceFunctionCalls{this, false, "trace-function-calls", "Emit log messages for each function entry and exit at the 'vomit' log level (-vvvv)."}; + + Setting useEvalCache{this, true, "eval-cache", + "Whether to use the flake evaluation cache."}; }; extern EvalSettings evalSettings; diff --git a/src/nix/app.cc b/src/nix/app.cc index 3935297cf..80acbf658 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -8,7 +8,7 @@ namespace nix { App Installable::toApp(EvalState & state) { - auto [cursor, attrPath] = getCursor(state, true); + auto [cursor, attrPath] = getCursor(state); auto type = cursor->getAttr("type")->getString(); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 80d8654bc..653f8db1b 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -572,7 +572,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand Strings{templateName == "" ? "defaultTemplate" : templateName}, Strings(attrsPathPrefixes), lockFlags); - auto [cursor, attrPath] = installable.getCursor(*evalState, true); + auto [cursor, attrPath] = installable.getCursor(*evalState); auto templateDir = cursor->getAttr("path")->getString(); @@ -782,7 +782,6 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun struct CmdFlakeShow : FlakeCommand { bool showLegacy = false; - bool useEvalCache = true; CmdFlakeShow() { @@ -791,12 +790,6 @@ struct CmdFlakeShow : FlakeCommand .description = "show the contents of the 'legacyPackages' output", .handler = {&showLegacy, true} }); - - addFlag({ - .longName = "no-eval-cache", - .description = "do not use the flake evaluation cache", - .handler = {[&]() { useEvalCache = false; }} - }); } std::string description() override @@ -934,7 +927,7 @@ struct CmdFlakeShow : FlakeCommand } }; - auto cache = openEvalCache(*state, flake, useEvalCache); + auto cache = openEvalCache(*state, flake); visit(*cache->getRoot(), {}, fmt(ANSI_BOLD "%s" ANSI_NORMAL, flake->flake.lockedRef), ""); } diff --git a/src/nix/installables.cc b/src/nix/installables.cc index 06e61380b..ea152709f 100644 --- a/src/nix/installables.cc +++ b/src/nix/installables.cc @@ -183,8 +183,7 @@ void completeFlakeRefWithFragment( auto flakeRef = parseFlakeRef(flakeRefS, absPath(".")); auto evalCache = openEvalCache(*evalState, - std::make_shared(lockFlake(*evalState, flakeRef, lockFlags)), - true); + std::make_shared(lockFlake(*evalState, flakeRef, lockFlags))); auto root = evalCache->getRoot(); @@ -273,7 +272,7 @@ Buildable Installable::toBuildable() } std::vector, std::string>> -Installable::getCursors(EvalState & state, bool useEvalCache) +Installable::getCursors(EvalState & state) { auto evalCache = std::make_shared(std::nullopt, state, @@ -282,9 +281,9 @@ Installable::getCursors(EvalState & state, bool useEvalCache) } std::pair, std::string> -Installable::getCursor(EvalState & state, bool useEvalCache) +Installable::getCursor(EvalState & state) { - auto cursors = getCursors(state, useEvalCache); + auto cursors = getCursors(state); if (cursors.empty()) throw Error("cannot find flake attribute '%s'", what()); return cursors[0]; @@ -420,12 +419,11 @@ Value * InstallableFlake::getFlakeOutputs(EvalState & state, const flake::Locked ref openEvalCache( EvalState & state, - std::shared_ptr lockedFlake, - bool useEvalCache) + std::shared_ptr lockedFlake) { auto fingerprint = lockedFlake->getFingerprint(); return make_ref( - useEvalCache && evalSettings.pureEval + evalSettings.useEvalCache && evalSettings.pureEval ? std::optional { std::cref(fingerprint) } : std::nullopt, state, @@ -460,10 +458,9 @@ static std::string showAttrPaths(const std::vector & paths) std::tuple InstallableFlake::toDerivation() { - auto lockedFlake = getLockedFlake(); - auto cache = openEvalCache(*state, lockedFlake, true); + auto cache = openEvalCache(*state, lockedFlake); auto root = cache->getRoot(); for (auto & attrPath : getActualAttrPaths()) { @@ -517,11 +514,10 @@ std::pair InstallableFlake::toValue(EvalState & state) } std::vector, std::string>> -InstallableFlake::getCursors(EvalState & state, bool useEvalCache) +InstallableFlake::getCursors(EvalState & state) { auto evalCache = openEvalCache(state, - std::make_shared(lockFlake(state, flakeRef, lockFlags)), - useEvalCache); + std::make_shared(lockFlake(state, flakeRef, lockFlags))); auto root = evalCache->getRoot(); diff --git a/src/nix/installables.hh b/src/nix/installables.hh index 9edff3331..26e87ee3a 100644 --- a/src/nix/installables.hh +++ b/src/nix/installables.hh @@ -62,10 +62,10 @@ struct Installable } virtual std::vector, std::string>> - getCursors(EvalState & state, bool useEvalCache); + getCursors(EvalState & state); std::pair, std::string> - getCursor(EvalState & state, bool useEvalCache); + getCursor(EvalState & state); virtual FlakeRef nixpkgsFlakeRef() const { @@ -118,7 +118,7 @@ struct InstallableFlake : InstallableValue std::pair toValue(EvalState & state) override; std::vector, std::string>> - getCursors(EvalState & state, bool useEvalCache) override; + getCursors(EvalState & state) override; std::shared_ptr getLockedFlake() const; @@ -127,7 +127,6 @@ struct InstallableFlake : InstallableValue ref openEvalCache( EvalState & state, - std::shared_ptr lockedFlake, - bool useEvalCache); + std::shared_ptr lockedFlake); } diff --git a/src/nix/search.cc b/src/nix/search.cc index 65a1e1818..430979274 100644 --- a/src/nix/search.cc +++ b/src/nix/search.cc @@ -177,7 +177,7 @@ struct CmdSearch : InstallableCommand, MixJSON } }; - for (auto & [cursor, prefix] : installable->getCursors(*state, true)) + for (auto & [cursor, prefix] : installable->getCursors(*state)) visit(*cursor, parseAttrPath(*state, prefix)); if (!json && !results) From 3c75ddc16b054a7dabf0710ee0a4323b4371effd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 7 Aug 2020 14:46:00 +0200 Subject: [PATCH 8/9] nix build (and others): Force re-evaluation of cached errors Fixes #3872. This is a bit hacky. Ideally we would automatically re-evaluate the failed attribute iff we need to print the error message (so in commands like 'nix search' we wouldn't re-evaluate because we're suppressing errors). --- src/libexpr/eval-cache.cc | 17 ++++++++++------- src/libexpr/eval-cache.hh | 6 ++++-- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index deb32484f..46177a0a4 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -405,7 +405,7 @@ Value & AttrCursor::forceValue() return v; } -std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) +std::shared_ptr AttrCursor::maybeGetAttr(Symbol name, bool forceErrors) { if (root->db) { if (!cachedValue) @@ -422,9 +422,12 @@ std::shared_ptr AttrCursor::maybeGetAttr(Symbol name) if (attr) { if (std::get_if(&attr->second)) return nullptr; - else if (std::get_if(&attr->second)) - throw EvalError("cached failure of attribute '%s'", getAttrPathStr(name)); - else + else if (std::get_if(&attr->second)) { + if (forceErrors) + debug("reevaluating failed cached attribute '%s'"); + else + throw CachedEvalError("cached failure of attribute '%s'", getAttrPathStr(name)); + } else return std::make_shared(root, std::make_pair(shared_from_this(), name), nullptr, std::move(attr)); } @@ -469,9 +472,9 @@ std::shared_ptr AttrCursor::maybeGetAttr(std::string_view name) return maybeGetAttr(root->state.symbols.create(name)); } -std::shared_ptr AttrCursor::getAttr(Symbol name) +std::shared_ptr AttrCursor::getAttr(Symbol name, bool forceErrors) { - auto p = maybeGetAttr(name); + auto p = maybeGetAttr(name, forceErrors); if (!p) throw Error("attribute '%s' does not exist", getAttrPathStr(name)); return p; @@ -600,7 +603,7 @@ bool AttrCursor::isDerivation() StorePath AttrCursor::forceDerivation() { - auto aDrvPath = getAttr(root->state.sDrvPath); + auto aDrvPath = getAttr(root->state.sDrvPath, true); 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 diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index afee85fa9..8ffffc0ed 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -9,6 +9,8 @@ namespace nix::eval_cache { +MakeError(CachedEvalError, EvalError); + class AttrDb; class AttrCursor; @@ -92,11 +94,11 @@ public: std::string getAttrPathStr(Symbol name) const; - std::shared_ptr maybeGetAttr(Symbol name); + std::shared_ptr maybeGetAttr(Symbol name, bool forceErrors = false); std::shared_ptr maybeGetAttr(std::string_view name); - std::shared_ptr getAttr(Symbol name); + std::shared_ptr getAttr(Symbol name, bool forceErrors = false); std::shared_ptr getAttr(std::string_view name); From 47644e49ca252441fcc3ae57f5a01e7fc579ba8c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 7 Aug 2020 17:05:14 +0000 Subject: [PATCH 9/9] Specialize `std::optional` so this is backwards compatible While I am cautious to break parametricity, I think it's OK in this cases---we're not about to try to do some crazy polymorphic protocol anytime soon. --- src/libstore/remote-store.cc | 14 ++++++++++++++ src/libstore/worker-protocol.hh | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 96e0aabba..005415666 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -70,6 +70,20 @@ void write(const Store & store, Sink & out, const StorePath & storePath) out << store.printStorePath(storePath); } + +template<> +std::optional read(const Store & store, Source & from, Phantom> _) +{ + auto s = readString(from); + return s == "" ? std::optional {} : store.parseStorePath(s); +} + +template<> +void write(const Store & store, Sink & out, const std::optional & storePathOpt) +{ + out << (storePathOpt ? store.printStorePath(*storePathOpt) : ""); +} + } diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index c50995d7c..b3576fbeb 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -90,6 +90,13 @@ std::optional read(const Store & store, Source & from, Phantom void write(const Store & store, Sink & out, const std::optional & optVal); +/* Specialization which uses and empty string for the empty case, taking + advantage of the fact StorePaths always serialize to a non-empty string. + This is done primarily for backwards compatability, so that StorePath <= + std::optional, where <= is the compatability partial order. + */ +template<> +void write(const Store & store, Sink & out, const std::optional & optVal); template std::map read(const Store & store, Source & from, Phantom> _)