From 2b02ce0e481b653bcc5b403ef5d9e3670a88e8e5 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 28 Sep 2021 22:12:41 +0200 Subject: [PATCH] libexpr: throw a more helpful eval-error if a builtin is not available due to a missing feature-flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I found it somewhat confusing to have an error like error: attribute 'getFlake' missing if the required experimental-feature (`flakes`) is not enabled. Instead, I'd expect Nix to throw an error just like it's the case when using e.g. `nix flake` without `flakes` being enabled. With this change, the error looks like this: $ nix-instantiate -E 'builtins.getFlake "nixpkgs"' error: Cannot call 'builtins.getFlake' because experimental Nix feature 'flakes' is disabled. You can enable it via '--extra-experimental-features flakes'. at «string»:1:1: 1| builtins.getFlake "nixpkgs" | ^ I didn't use `settings.requireExperimentalFeature` here on purpose because this doesn't contain a position. Also, it doesn't seem as if we need to catch the error and check for the missing feature here since this already happens at evaluation time. --- src/libexpr/eval.cc | 17 +++++++++++++++++ src/libexpr/eval.hh | 6 ++++++ src/libexpr/flake/flake.cc | 4 +++- src/libexpr/primops.cc | 19 ++++++++----------- src/libexpr/primops.hh | 4 +--- 5 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index bc41a2cd9..800839a8d 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -465,6 +465,23 @@ EvalState::~EvalState() } +void EvalState::requireExperimentalFeatureOnEvaluation( + const std::string & feature, + const std::string_view fName, + const Pos & pos) +{ + if (!settings.isExperimentalFeatureEnabled(feature)) { + throw EvalError({ + .msg = hintfmt( + "Cannot call '%2%' because experimental Nix feature '%1%' is disabled. You can enable it via '--extra-experimental-features %1%'.", + feature, + fName + ), + .errPos = pos + }); + } +} + Path EvalState::checkSourcePath(const Path & path_) { if (!allowedPaths) return path_; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index b29feb134..9df6150c6 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -140,6 +140,12 @@ public: std::shared_ptr buildStore = nullptr); ~EvalState(); + void requireExperimentalFeatureOnEvaluation( + const std::string & feature, + const std::string_view fName, + const Pos & pos + ); + void addToSearchPath(const string & s); SearchPath getSearchPath() { return searchPath; } diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 492b47115..1a1fa6938 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -688,6 +688,8 @@ void callFlake(EvalState & state, static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v) { + state.requireExperimentalFeatureOnEvaluation("flakes", "builtins.getFlake", pos); + auto flakeRefS = state.forceStringNoCtx(*args[0], pos); auto flakeRef = parseFlakeRef(flakeRefS, {}, true); if (evalSettings.pureEval && !flakeRef.input.isImmutable()) @@ -703,7 +705,7 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va v); } -static RegisterPrimOp r2("__getFlake", 1, prim_getFlake, "flakes"); +static RegisterPrimOp r2("__getFlake", 1, prim_getFlake); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index c98a30177..ead8c1ded 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3606,15 +3606,13 @@ static RegisterPrimOp primop_splitVersion({ RegisterPrimOp::PrimOps * RegisterPrimOp::primOps; -RegisterPrimOp::RegisterPrimOp(std::string name, size_t arity, PrimOpFun fun, - std::optional requiredFeature) +RegisterPrimOp::RegisterPrimOp(std::string name, size_t arity, PrimOpFun fun) { if (!primOps) primOps = new PrimOps; primOps->push_back({ .name = name, .args = {}, .arity = arity, - .requiredFeature = std::move(requiredFeature), .fun = fun }); } @@ -3688,14 +3686,13 @@ void EvalState::createBaseEnv() if (RegisterPrimOp::primOps) for (auto & primOp : *RegisterPrimOp::primOps) - if (!primOp.requiredFeature || settings.isExperimentalFeatureEnabled(*primOp.requiredFeature)) - addPrimOp({ - .fun = primOp.fun, - .arity = std::max(primOp.args.size(), primOp.arity), - .name = symbols.create(primOp.name), - .args = std::move(primOp.args), - .doc = primOp.doc, - }); + addPrimOp({ + .fun = primOp.fun, + .arity = std::max(primOp.args.size(), primOp.arity), + .name = symbols.create(primOp.name), + .args = std::move(primOp.args), + .doc = primOp.doc, + }); /* Add a wrapper around the derivation primop that computes the `drvPath' and `outPath' attributes lazily. */ diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh index 9d42d6539..5b16e075f 100644 --- a/src/libexpr/primops.hh +++ b/src/libexpr/primops.hh @@ -15,7 +15,6 @@ struct RegisterPrimOp std::vector args; size_t arity = 0; const char * doc; - std::optional requiredFeature; PrimOpFun fun; }; @@ -28,8 +27,7 @@ struct RegisterPrimOp RegisterPrimOp( std::string name, size_t arity, - PrimOpFun fun, - std::optional requiredFeature = {}); + PrimOpFun fun); RegisterPrimOp(Info && info); };