From c1f75860d6ed402a17ca9c57b770ed952a42e3bb Mon Sep 17 00:00:00 2001 From: piegames Date: Wed, 15 Apr 2026 17:46:45 +0200 Subject: [PATCH] builtins.flakeRefToString: Force the arguments Without this, the primop chokes on any thunks on attributes passed in the attrset. It even is the reason why the test contained `builtins.seq` to work around this. Supposedly, this might have been an intentional restriction and changing this might break things in ways I cannot forsee due to not knowing much about Flakes, however the status quo is equally broken: - The error message looks like an internal error and not like some explicitly forbidden invariant violation. - Seemingly simple syntax literals like "-1" compile to "__sub 0 1" and thus create a thunk which then fails, which is utterly confusing ("why does 1 work but not -1?") - This is a stark violation of the principle of least surprise. - Thunking relies on maybeThunk and thunk inlining optimizations, thus not forcing thunks turns operational details of the evaluator into language-observable behavior. That's bad. I am changing this now regardless of the risk of breakage, because the bytecode evaluator will have different thunk inlining optimizations and thus inevitably cause mismatches in behavior anyways. Change-Id: Ifc45c4d2900e40822383670b28e4e50ab8af317a --- lix/libexpr/flake/flake.cc | 1 + .../eval-fail-negative-integer.err.exp | 15 +++++---------- .../in-negative-integer.nix | 7 +++++-- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lix/libexpr/flake/flake.cc b/lix/libexpr/flake/flake.cc index 94ba34433..9e3711a48 100644 --- a/lix/libexpr/flake/flake.cc +++ b/lix/libexpr/flake/flake.cc @@ -1015,6 +1015,7 @@ Value prim_flakeRefToString(EvalState & state, Value ** args) "while evaluating the argument passed to builtins.flakeRefToString"); fetchers::Attrs attrs; for (const auto & attr : *args[0]->attrs()) { + state.forceValue(attr.value, noPos); auto t = attr.value.type(); if (t == nInt) { auto intValue = attr.value.integer().value; diff --git a/tests/functional2/lang/builtins.flakeRefToString/eval-fail-negative-integer.err.exp b/tests/functional2/lang/builtins.flakeRefToString/eval-fail-negative-integer.err.exp index fdb4a591a..5e8db03ae 100644 --- a/tests/functional2/lang/builtins.flakeRefToString/eval-fail-negative-integer.err.exp +++ b/tests/functional2/lang/builtins.flakeRefToString/eval-fail-negative-integer.err.exp @@ -1,14 +1,9 @@ error: - … while calling the 'seq' builtin - at /pwd/in.nix:1:16: - 1| let n = -1; in builtins.seq n (builtins.flakeRefToString { - | ^ - 2| type = "github"; - … while calling the 'flakeRefToString' builtin - at /pwd/in.nix:1:32: - 1| let n = -1; in builtins.seq n (builtins.flakeRefToString { - | ^ - 2| type = "github"; + at /pwd/in.nix:4:1: + 3| in + 4| builtins.flakeRefToString { + | ^ + 5| type = "github"; error: negative value given for flake ref attr repo: -1 diff --git a/tests/functional2/lang/builtins.flakeRefToString/in-negative-integer.nix b/tests/functional2/lang/builtins.flakeRefToString/in-negative-integer.nix index e0208eb25..790f138ca 100644 --- a/tests/functional2/lang/builtins.flakeRefToString/in-negative-integer.nix +++ b/tests/functional2/lang/builtins.flakeRefToString/in-negative-integer.nix @@ -1,7 +1,10 @@ -let n = -1; in builtins.seq n (builtins.flakeRefToString { +let + n = -1; +in +builtins.flakeRefToString { type = "github"; owner = "NixOS"; repo = n; ref = "23.05"; dir = "lib"; -}) +}