From 5abaf361a41facb2baeee05aa58b9b5e3aa08f45 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:06:13 +0200 Subject: [PATCH 01/10] docker: reduce duplicates, use `coreutils-full` --- docker.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker.nix b/docker.nix index e3a0635d0..8262d1b70 100644 --- a/docker.nix +++ b/docker.nix @@ -290,7 +290,7 @@ let echo "${channelURL} ${channelName}" > $out${userHome}/.nix-channels mkdir -p $out/bin $out/usr/bin - ln -s ${pkgs.coreutils}/bin/env $out/usr/bin/env + ln -s ${pkgs.coreutils-full}/bin/env $out/usr/bin/env ln -s ${pkgs.bashInteractive}/bin/bash $out/bin/sh '' From 5862f38d00b3d84ee9272f90983a52ba1b9c71ee Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:07:20 +0200 Subject: [PATCH 02/10] docker: use `callPackage`, parametrise the image build --- docker.nix | 90 +++++++++++++++++++++++--------------- flake.nix | 3 +- tests/nixos/nix-docker.nix | 10 +---- 3 files changed, 58 insertions(+), 45 deletions(-) diff --git a/docker.nix b/docker.nix index 8262d1b70..060dcd8f0 100644 --- a/docker.nix +++ b/docker.nix @@ -1,6 +1,10 @@ { - pkgs ? import { }, - lib ? pkgs.lib, + # Core dependencies + pkgs, + lib, + runCommand, + buildPackages, + # Image configuration name ? "nix", tag ? "latest", bundleNixpkgs ? true, @@ -14,36 +18,52 @@ gid ? 0, uname ? "root", gname ? "root", + # Default Packages + nix, + bashInteractive, + coreutils-full, + gnutar, + gzip, + gnugrep, + which, + curl, + less, + wget, + man, + cacert, + findutils, + iana-etc, + git, + openssh, + # Other dependencies + shadow, }: let - defaultPkgs = - with pkgs; - [ - nix - bashInteractive - coreutils-full - gnutar - gzip - gnugrep - which - curl - less - wget - man - cacert.out - findutils - iana-etc - git - openssh - ] - ++ extraPkgs; + defaultPkgs = [ + nix + bashInteractive + coreutils-full + gnutar + gzip + gnugrep + which + curl + less + wget + man + cacert.out + findutils + iana-etc + git + openssh + ] ++ extraPkgs; users = { root = { uid = 0; - shell = "${pkgs.bashInteractive}/bin/bash"; + shell = lib.getExe bashInteractive; home = "/root"; gid = 0; groups = [ "root" ]; @@ -52,7 +72,7 @@ let nobody = { uid = 65534; - shell = "${pkgs.shadow}/bin/nologin"; + shell = lib.getExe' shadow "nologin"; home = "/var/empty"; gid = 65534; groups = [ "nobody" ]; @@ -63,7 +83,7 @@ let // lib.optionalAttrs (uid != 0) { "${uname}" = { uid = uid; - shell = "${pkgs.bashInteractive}/bin/bash"; + shell = lib.getExe bashInteractive; home = "/home/${uname}"; gid = gid; groups = [ "${gname}" ]; @@ -170,7 +190,7 @@ let baseSystem = let nixpkgs = pkgs.path; - channel = pkgs.runCommand "channel-nixos" { inherit bundleNixpkgs; } '' + channel = runCommand "channel-nixos" { inherit bundleNixpkgs; } '' mkdir $out if [ "$bundleNixpkgs" ]; then ln -s ${ @@ -182,11 +202,11 @@ let echo "[]" > $out/manifest.nix fi ''; - rootEnv = pkgs.buildPackages.buildEnv { + rootEnv = buildPackages.buildEnv { name = "root-profile-env"; paths = defaultPkgs; }; - manifest = pkgs.buildPackages.runCommand "manifest.nix" { } '' + manifest = buildPackages.runCommand "manifest.nix" { } '' cat > $out < $out${userHome}/.nix-channels mkdir -p $out/bin $out/usr/bin - ln -s ${pkgs.coreutils-full}/bin/env $out/usr/bin/env - ln -s ${pkgs.bashInteractive}/bin/bash $out/bin/sh + ln -s ${lib.getExe' coreutils-full "env"} $out/usr/bin/env + ln -s ${lib.getExe bashInteractive} $out/bin/sh '' + (lib.optionalString (flake-registry-path != null) '' @@ -300,7 +320,7 @@ let globalFlakeRegistryPath="$nixCacheDir/flake-registry.json" ln -s ${flake-registry-path} $out$globalFlakeRegistryPath mkdir -p $out/nix/var/nix/gcroots/auto - rootName=$(${pkgs.nix}/bin/nix --extra-experimental-features nix-command hash file --type sha1 --base32 <(echo -n $globalFlakeRegistryPath)) + rootName=$(${lib.getExe' nix "nix"} --extra-experimental-features nix-command hash file --type sha1 --base32 <(echo -n $globalFlakeRegistryPath)) ln -s $globalFlakeRegistryPath $out/nix/var/nix/gcroots/auto/$rootName '') ); @@ -332,7 +352,7 @@ pkgs.dockerTools.buildLayeredImageWithNixDb { ''; config = { - Cmd = [ "${userHome}/.nix-profile/bin/bash" ]; + Cmd = [ (lib.getExe bashInteractive) ]; User = "${toString uid}:${toString gid}"; Env = [ "USER=${uname}" diff --git a/flake.nix b/flake.nix index 7d7c4d4c2..69bd2a21a 100644 --- a/flake.nix +++ b/flake.nix @@ -404,8 +404,7 @@ dockerImage = let pkgs = nixpkgsFor.${system}.native; - image = import ./docker.nix { - inherit pkgs; + image = pkgs.callPackage ./docker.nix { tag = pkgs.nix.version; }; in diff --git a/tests/nixos/nix-docker.nix b/tests/nixos/nix-docker.nix index c58a00cdd..f1c218585 100644 --- a/tests/nixos/nix-docker.nix +++ b/tests/nixos/nix-docker.nix @@ -1,21 +1,15 @@ # Test the container built by ../../docker.nix. { - lib, config, - nixpkgs, - hostPkgs, ... }: let pkgs = config.nodes.machine.nixpkgs.pkgs; - nixImage = import ../../docker.nix { - inherit (config.nodes.machine.nixpkgs) pkgs; - }; - nixUserImage = import ../../docker.nix { - inherit (config.nodes.machine.nixpkgs) pkgs; + nixImage = pkgs.callPackage ../../docker.nix { }; + nixUserImage = pkgs.callPackage ../../docker.nix { name = "nix-user"; uid = 1000; gid = 1000; From 6eb4ee68556af97ab88f3f909f3158e57429f6af Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Thu, 12 Jun 2025 19:18:07 +0200 Subject: [PATCH 03/10] docker: replace `git` with `gitMinimal` --- docker.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker.nix b/docker.nix index 060dcd8f0..e97ed1752 100644 --- a/docker.nix +++ b/docker.nix @@ -33,7 +33,7 @@ cacert, findutils, iana-etc, - git, + gitMinimal, openssh, # Other dependencies shadow, @@ -54,7 +54,7 @@ let cacert.out findutils iana-etc - git + gitMinimal openssh ] ++ extraPkgs; From 6587e7bcff6bfc7718cea96e64823fc2742d3d6e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:42:50 +0000 Subject: [PATCH 04/10] libexpr: Add and use `lambda` getter --- src/libcmd/repl.cc | 2 +- src/libexpr-tests/primops.cc | 4 ++-- src/libexpr/eval-profiler.cc | 2 +- src/libexpr/eval.cc | 22 +++++++++++----------- src/libexpr/include/nix/expr/value.hh | 3 +++ src/libexpr/primops.cc | 4 ++-- src/libexpr/print.cc | 8 ++++---- src/libexpr/value-to-xml.cc | 12 ++++++------ src/libflake/flake.cc | 4 ++-- src/nix-build/nix-build.cc | 4 ++-- src/nix/flake.cc | 4 ++-- 11 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index f01f12860..8170bd579 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -484,7 +484,7 @@ ProcessLineResult NixRepl::processLine(std::string line) auto path = state->coerceToPath(noPos, v, context, "while evaluating the filename to edit"); return {path, 0}; } else if (v.isLambda()) { - auto pos = state->positions[v.payload.lambda.fun->pos]; + auto pos = state->positions[v.lambda().fun->pos]; if (auto path = std::get_if(&pos.origin)) return {*path, pos.line}; else diff --git a/src/libexpr-tests/primops.cc b/src/libexpr-tests/primops.cc index 2f864f2c2..6c301f157 100644 --- a/src/libexpr-tests/primops.cc +++ b/src/libexpr-tests/primops.cc @@ -667,8 +667,8 @@ namespace nix { auto v = eval("derivation"); ASSERT_EQ(v.type(), nFunction); ASSERT_TRUE(v.isLambda()); - ASSERT_NE(v.payload.lambda.fun, nullptr); - ASSERT_TRUE(v.payload.lambda.fun->hasFormals()); + ASSERT_NE(v.lambda().fun, nullptr); + ASSERT_TRUE(v.lambda().fun->hasFormals()); } TEST_F(PrimOpTest, currentTime) { diff --git a/src/libexpr/eval-profiler.cc b/src/libexpr/eval-profiler.cc index 7053e4ec7..b65bc3a4d 100644 --- a/src/libexpr/eval-profiler.cc +++ b/src/libexpr/eval-profiler.cc @@ -204,7 +204,7 @@ FrameInfo SampleStack::getFrameInfoFromValueAndPos(const Value & v, std::spanpos; - case tLambda: return payload.lambda.fun->pos; + case tLambda: return lambda().fun->pos; case tApp: return payload.app.left->determinePos(pos); default: return pos; } @@ -610,7 +610,7 @@ std::optional EvalState::getDoc(Value & v) }; } if (v.isLambda()) { - auto exprLambda = v.payload.lambda.fun; + auto exprLambda = v.lambda().fun; std::ostringstream s; std::string name; @@ -1567,13 +1567,13 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, if (vCur.isLambda()) { - ExprLambda & lambda(*vCur.payload.lambda.fun); + ExprLambda & lambda(*vCur.lambda().fun); auto size = (!lambda.arg ? 0 : 1) + (lambda.hasFormals() ? lambda.formals->formals.size() : 0); Env & env2(allocEnv(size)); - env2.up = vCur.payload.lambda.env; + env2.up = vCur.lambda().env; Displacement displ = 0; @@ -1603,7 +1603,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") - .withFrame(*fun.payload.lambda.env, lambda) + .withFrame(*fun.lambda().env, lambda) .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); @@ -1630,7 +1630,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, .atPos(lambda.pos) .withTrace(pos, "from call site") .withSuggestions(suggestions) - .withFrame(*fun.payload.lambda.env, lambda) + .withFrame(*fun.lambda().env, lambda) .debugThrow(); } unreachable(); @@ -1825,14 +1825,14 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res } } - if (!fun.isLambda() || !fun.payload.lambda.fun->hasFormals()) { + if (!fun.isLambda() || !fun.lambda().fun->hasFormals()) { res = fun; return; } - auto attrs = buildBindings(std::max(static_cast(fun.payload.lambda.fun->formals->formals.size()), args.size())); + auto attrs = buildBindings(std::max(static_cast(fun.lambda().fun->formals->formals.size()), args.size())); - if (fun.payload.lambda.fun->formals->ellipsis) { + if (fun.lambda().fun->formals->ellipsis) { // If the formals have an ellipsis (eg the function accepts extra args) pass // all available automatic arguments (which includes arguments specified on // the command line via --arg/--argstr) @@ -1840,7 +1840,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res attrs.insert(v); } else { // Otherwise, only pass the arguments that the function accepts - for (auto & i : fun.payload.lambda.fun->formals->formals) { + for (auto & i : fun.lambda().fun->formals->formals) { auto j = args.get(i.name); if (j) { attrs.insert(*j); @@ -1850,7 +1850,7 @@ Nix attempted to evaluate a function as a top level expression; in this case it must have its arguments supplied either by default values, or passed explicitly with '--arg' or '--argstr'. See https://nixos.org/manual/nix/stable/language/constructs.html#functions.)", symbols[i.name]) - .atPos(i.pos).withFrame(*fun.payload.lambda.env, *fun.payload.lambda.fun).debugThrow(); + .atPos(i.pos).withFrame(*fun.lambda().env, *fun.lambda().fun).debugThrow(); } } } diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index febe36f80..26ec5ff38 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -482,6 +482,9 @@ public: NixFloat fpoint() const { return payload.fpoint; } + + Lambda lambda() const + { return payload.lambda; } }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 48bc272f9..a24695bcd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3138,12 +3138,12 @@ static void prim_functionArgs(EvalState & state, const PosIdx pos, Value * * arg if (!args[0]->isLambda()) state.error("'functionArgs' requires a function").atPos(pos).debugThrow(); - if (!args[0]->payload.lambda.fun->hasFormals()) { + if (!args[0]->lambda().fun->hasFormals()) { v.mkAttrs(&state.emptyBindings); return; } - const auto &formals = args[0]->payload.lambda.fun->formals->formals; + const auto &formals = args[0]->lambda().fun->formals->formals; auto attrs = state.buildBindings(formals.size()); for (auto & i : formals) attrs.insert(i.name, state.getBool(i.def), i.pos); diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 06bae9c5c..f0e0eed2d 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -453,13 +453,13 @@ private: if (v.isLambda()) { output << "lambda"; - if (v.payload.lambda.fun) { - if (v.payload.lambda.fun->name) { - output << " " << state.symbols[v.payload.lambda.fun->name]; + if (v.lambda().fun) { + if (v.lambda().fun->name) { + output << " " << state.symbols[v.lambda().fun->name]; } std::ostringstream s; - s << state.positions[v.payload.lambda.fun->pos]; + s << state.positions[v.lambda().fun->pos]; output << " @ " << filterANSIEscapes(toView(s)); } } else if (v.isPrimOp()) { diff --git a/src/libexpr/value-to-xml.cc b/src/libexpr/value-to-xml.cc index e26fff71b..54ff06f9e 100644 --- a/src/libexpr/value-to-xml.cc +++ b/src/libexpr/value-to-xml.cc @@ -126,18 +126,18 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, break; } XMLAttrs xmlAttrs; - if (location) posToXML(state, xmlAttrs, state.positions[v.payload.lambda.fun->pos]); + if (location) posToXML(state, xmlAttrs, state.positions[v.lambda().fun->pos]); XMLOpenElement _(doc, "function", xmlAttrs); - if (v.payload.lambda.fun->hasFormals()) { + if (v.lambda().fun->hasFormals()) { XMLAttrs attrs; - if (v.payload.lambda.fun->arg) attrs["name"] = state.symbols[v.payload.lambda.fun->arg]; - if (v.payload.lambda.fun->formals->ellipsis) attrs["ellipsis"] = "1"; + if (v.lambda().fun->arg) attrs["name"] = state.symbols[v.lambda().fun->arg]; + if (v.lambda().fun->formals->ellipsis) attrs["ellipsis"] = "1"; XMLOpenElement _(doc, "attrspat", attrs); - for (auto & i : v.payload.lambda.fun->formals->lexicographicOrder(state.symbols)) + for (auto & i : v.lambda().fun->formals->lexicographicOrder(state.symbols)) doc.writeEmptyElement("attr", singletonAttrs("name", state.symbols[i.name])); } else - doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.payload.lambda.fun->arg])); + doc.writeEmptyElement("varpat", singletonAttrs("name", state.symbols[v.lambda().fun->arg])); break; } diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 24252d710..41141d41d 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -252,8 +252,8 @@ static Flake readFlake( if (auto outputs = vInfo.attrs()->get(sOutputs)) { expectType(state, nFunction, *outputs->value, outputs->pos); - if (outputs->value->isLambda() && outputs->value->payload.lambda.fun->hasFormals()) { - for (auto & formal : outputs->value->payload.lambda.fun->formals->formals) { + if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals()) { + for (auto & formal : outputs->value->lambda().fun->formals->formals) { if (formal.name != state.sSelf) flake.inputs.emplace(state.symbols[formal.name], FlakeInput { .ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name])) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 80ebf6bfa..e302aca67 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -387,8 +387,8 @@ static void main_nix_build(int argc, char * * argv) return false; } bool add = false; - if (v.type() == nFunction && v.payload.lambda.fun->hasFormals()) { - for (auto & i : v.payload.lambda.fun->formals->formals) { + if (v.type() == nFunction && v.lambda().fun->hasFormals()) { + for (auto & i : v.lambda().fun->formals->formals) { if (state->symbols[i.name] == "inNixShell") { add = true; break; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 95cf85663..13f7363fc 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -492,8 +492,8 @@ struct CmdFlakeCheck : FlakeCommand if (!v.isLambda()) { throw Error("overlay is not a function, but %s instead", showType(v)); } - if (v.payload.lambda.fun->hasFormals() - || !argHasName(v.payload.lambda.fun->arg, "final")) + if (v.lambda().fun->hasFormals() + || !argHasName(v.lambda().fun->arg, "final")) throw Error("overlay does not take an argument named 'final'"); // FIXME: if we have a 'nixpkgs' input, use it to // evaluate the overlay. From 441fa86e82da65a71e01a5c45e3459287ab4d01e Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:48:42 +0000 Subject: [PATCH 05/10] libexpr: Add and use `thunk` getter --- src/libexpr/eval.cc | 10 +++++----- src/libexpr/include/nix/expr/eval-inline.hh | 4 ++-- src/libexpr/include/nix/expr/value.hh | 5 ++++- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 51036a223..badb271f2 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -160,10 +160,10 @@ bool Value::isTrivial() const internalType != tApp && internalType != tPrimOpApp && (internalType != tThunk - || (dynamic_cast(payload.thunk.expr) - && ((ExprAttrs *) payload.thunk.expr)->dynamicAttrs.empty()) - || dynamic_cast(payload.thunk.expr) - || dynamic_cast(payload.thunk.expr)); + || (dynamic_cast(thunk().expr) + && ((ExprAttrs *) thunk().expr)->dynamicAttrs.empty()) + || dynamic_cast(thunk().expr) + || dynamic_cast(thunk().expr)); } @@ -2163,7 +2163,7 @@ void EvalState::forceValueDeep(Value & v) try { // If the value is a thunk, we're evaling. Otherwise no trace necessary. auto dts = debugRepl && i.value->isThunk() - ? makeDebugTraceStacker(*this, *i.value->payload.thunk.expr, *i.value->payload.thunk.env, i.pos, + ? makeDebugTraceStacker(*this, *i.value->thunk().expr, *i.value->thunk().env, i.pos, "while evaluating the attribute '%1%'", symbols[i.name]) : nullptr; diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 6e5759c0b..97bf71a6b 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -89,9 +89,9 @@ Env & EvalState::allocEnv(size_t size) void EvalState::forceValue(Value & v, const PosIdx pos) { if (v.isThunk()) { - Env * env = v.payload.thunk.env; + Env * env = v.thunk().env; assert(env || v.isBlackhole()); - Expr * expr = v.payload.thunk.expr; + Expr * expr = v.thunk().expr; try { v.mkBlackhole(); //checkInterrupt(); diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 26ec5ff38..29f8ac379 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -485,6 +485,9 @@ public: Lambda lambda() const { return payload.lambda; } + + ClosureThunk thunk() const + { return payload.thunk; } }; @@ -492,7 +495,7 @@ extern ExprBlackHole eBlackHole; bool Value::isBlackhole() const { - return internalType == tThunk && payload.thunk.expr == (Expr*) &eBlackHole; + return internalType == tThunk && thunk().expr == (Expr*) &eBlackHole; } void Value::mkBlackhole() From f07a9f863e16bb71ec330abf6c2ec5d5bff68788 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:51:44 +0000 Subject: [PATCH 06/10] libexpr: Add and use `primOpApp` getter --- src/libexpr/eval.cc | 10 +++++----- src/libexpr/include/nix/expr/value.hh | 3 +++ 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index badb271f2..f9bff7b98 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -525,9 +525,9 @@ std::ostream & operator<<(std::ostream & output, const PrimOp & primOp) const PrimOp * Value::primOpAppPrimOp() const { - Value * left = payload.primOpApp.left; + Value * left = primOpApp().left; while (left && !left->isPrimOp()) { - left = left->payload.primOpApp.left; + left = left->primOpApp().left; } if (!left) @@ -1702,7 +1702,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, Value * primOp = &vCur; while (primOp->isPrimOpApp()) { argsDone++; - primOp = primOp->payload.primOpApp.left; + primOp = primOp->primOpApp().left; } assert(primOp->isPrimOp()); auto arity = primOp->primOp()->arity; @@ -1718,8 +1718,8 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, Value * vArgs[maxPrimOpArity]; auto n = argsDone; - for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->payload.primOpApp.left) - vArgs[--n] = arg->payload.primOpApp.right; + for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp().left) + vArgs[--n] = arg->primOpApp().right; for (size_t i = 0; i < argsLeft; ++i) vArgs[argsDone + i] = args[i]; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 29f8ac379..797e31191 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -488,6 +488,9 @@ public: ClosureThunk thunk() const { return payload.thunk; } + + FunctionApplicationThunk primOpApp() const + { return payload.primOpApp; } }; From c041d71406f706d971ee0ad53d555450de7ad03d Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:53:44 +0000 Subject: [PATCH 07/10] libexpr: Add and use `app` getter --- src/libexpr/eval.cc | 2 +- src/libexpr/include/nix/expr/eval-inline.hh | 2 +- src/libexpr/include/nix/expr/value.hh | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f9bff7b98..37018007f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -148,7 +148,7 @@ PosIdx Value::determinePos(const PosIdx pos) const switch (internalType) { case tAttrs: return attrs()->pos; case tLambda: return lambda().fun->pos; - case tApp: return payload.app.left->determinePos(pos); + case tApp: return app().left->determinePos(pos); default: return pos; } #pragma GCC diagnostic pop diff --git a/src/libexpr/include/nix/expr/eval-inline.hh b/src/libexpr/include/nix/expr/eval-inline.hh index 97bf71a6b..7d13d7cc7 100644 --- a/src/libexpr/include/nix/expr/eval-inline.hh +++ b/src/libexpr/include/nix/expr/eval-inline.hh @@ -106,7 +106,7 @@ void EvalState::forceValue(Value & v, const PosIdx pos) } } else if (v.isApp()) - callFunction(*v.payload.app.left, *v.payload.app.right, v, pos); + callFunction(*v.app().left, *v.app().right, v, pos); } diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 797e31191..d25511c45 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -491,6 +491,9 @@ public: FunctionApplicationThunk primOpApp() const { return payload.primOpApp; } + + FunctionApplicationThunk app() const + { return payload.app; } }; From e4df1891237b58050e7b10380f7f62551ade2783 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 19:57:46 +0000 Subject: [PATCH 08/10] libexpr: Add and use `pathStr` getter --- src/libexpr-c/nix_api_value.cc | 2 +- src/libexpr/eval.cc | 6 +++--- src/libexpr/include/nix/expr/value.hh | 5 ++++- src/libexpr/primops.cc | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/libexpr-c/nix_api_value.cc b/src/libexpr-c/nix_api_value.cc index 298d94845..8afe35a4b 100644 --- a/src/libexpr-c/nix_api_value.cc +++ b/src/libexpr-c/nix_api_value.cc @@ -252,7 +252,7 @@ const char * nix_get_path_string(nix_c_context * context, const nix_value * valu // We could use v.path().to_string().c_str(), but I'm concerned this // crashes. Looks like .path() allocates a CanonPath with a copy of the // string, then it gets the underlying data from that. - return v.payload.path.path; + return v.pathStr(); } NIXC_CATCH_ERRS_NULL } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 37018007f..66ac2e9fd 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2368,7 +2368,7 @@ BackedStringView EvalState::coerceToString( !canonicalizePath && !copyToStore ? // FIXME: hack to preserve path literals that end in a // slash, as in /foo/${x}. - v.payload.path.path + v.pathStr() : copyToStore ? store->printStorePath(copyPathToStore(context, v.path())) : std::string(v.path().path.abs()); @@ -2643,7 +2643,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st ValuePrinter(*this, v2, errorPrintOptions)) .debugThrow(); } - if (strcmp(v1.payload.path.path, v2.payload.path.path) != 0) { + if (strcmp(v1.pathStr(), v2.pathStr()) != 0) { error( "path '%s' is not equal to path '%s'", ValuePrinter(*this, v1, errorPrintOptions), @@ -2811,7 +2811,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v return // FIXME: compare accessors by their fingerprint. v1.payload.path.accessor == v2.payload.path.accessor - && strcmp(v1.payload.path.path, v2.payload.path.path) == 0; + && strcmp(v1.pathStr(), v2.pathStr()) == 0; case nNull: return true; diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index d25511c45..fc9ce1b39 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -445,7 +445,7 @@ public: assert(internalType == tPath); return SourcePath( ref(payload.path.accessor->shared_from_this()), - CanonPath(CanonPath::unchecked_t(), payload.path.path)); + CanonPath(CanonPath::unchecked_t(), pathStr())); } std::string_view string_view() const @@ -494,6 +494,9 @@ public: FunctionApplicationThunk app() const { return payload.app; } + + const char * pathStr() const + { return payload.path.path; } }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a24695bcd..60f44ca62 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -651,7 +651,7 @@ struct CompareValues // Note: we don't take the accessor into account // since it's not obvious how to compare them in a // reproducible way. - return strcmp(v1->payload.path.path, v2->payload.path.path) < 0; + return strcmp(v1->pathStr(), v2->pathStr()) < 0; case nList: // Lexicographic comparison for (size_t i = 0;; i++) { From bc6b52aff012592a9794df979c0617c85f46c2c3 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 20:01:38 +0000 Subject: [PATCH 09/10] libexpr: Add and use `pathAccessor` getter --- src/libexpr/eval.cc | 4 ++-- src/libexpr/include/nix/expr/value.hh | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 66ac2e9fd..ae422a3d4 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2636,7 +2636,7 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st return; case nPath: - if (v1.payload.path.accessor != v2.payload.path.accessor) { + if (v1.pathAccessor() != v2.pathAccessor()) { error( "path '%s' is not equal to path '%s' because their accessors are different", ValuePrinter(*this, v1, errorPrintOptions), @@ -2810,7 +2810,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v case nPath: return // FIXME: compare accessors by their fingerprint. - v1.payload.path.accessor == v2.payload.path.accessor + v1.pathAccessor() == v2.pathAccessor() && strcmp(v1.pathStr(), v2.pathStr()) == 0; case nNull: diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index fc9ce1b39..80bee59e9 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -444,7 +444,7 @@ public: { assert(internalType == tPath); return SourcePath( - ref(payload.path.accessor->shared_from_this()), + ref(pathAccessor()->shared_from_this()), CanonPath(CanonPath::unchecked_t(), pathStr())); } @@ -497,6 +497,9 @@ public: const char * pathStr() const { return payload.path.path; } + + SourceAccessor * pathAccessor() const + { return payload.path.accessor; } }; From 7b46eb9958e7681958020b68d9ea30a52be7cf09 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Thu, 12 Jun 2025 22:29:05 +0000 Subject: [PATCH 10/10] libexpr: Remove non-const overload of `listElems` This overload isn't actually necessary anywhere and doesn't make much sense. The pointers to `Value`s are themselves const, but the `Value`s are mutable. A non-const member function implies that the object itself can be modified but this doesn't make much sense considering the return type: `Value * const * `, which is a pointer to a constant array of pointers to mutable values. --- src/libexpr/include/nix/expr/value.hh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libexpr/include/nix/expr/value.hh b/src/libexpr/include/nix/expr/value.hh index 80bee59e9..fcc118c7e 100644 --- a/src/libexpr/include/nix/expr/value.hh +++ b/src/libexpr/include/nix/expr/value.hh @@ -410,11 +410,6 @@ public: return internalType == tList1 || internalType == tList2 || internalType == tListN; } - Value * const * listElems() - { - return internalType == tList1 || internalType == tList2 ? payload.smallList : payload.bigList.elems; - } - std::span listItems() const { assert(isList());