From f50ab9bad268040ff8914adc4ea5790cbddc0ffb Mon Sep 17 00:00:00 2001
From: Josh Heinrichs <josh.heinrichs@shopify.com>
Date: Wed, 12 Aug 2026 18:36:05 -0600
Subject: [PATCH] tecnix: intern builtin argument names at call time, not
 statically

Adding gitDir, resolver, rev, checkoutPath, and targets to
StaticEvalSymbols assigned them the lowest symbol ids, ahead of every
symbol interned while evaluating user code. Bindings iterate in
symbol-id order, so pre-interning common attr names like 'targets' and
'rev' observably reordered attribute iteration for every attrset
containing them -- for example, which error a deepSeq (or nix-unit's
forceValueDeep) surfaces first out of an attrset with several throwing
attributes.

These lookups run once per tecnixTargets/tecnixTargetNames call, right
before git subprocesses, SQLite reads, and target evaluation, so the
static ids bought nothing measurable. Intern the names at call time
instead, like upstream's fetcher builtins do, which restores
upstream-identical iteration order.
---
 src/libexpr/include/nix/expr/eval.hh | 12 +++---------
 src/libexpr/primops/tecnix.cc        | 10 +++++-----
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh
index 7cacd6684..68d907d78 100644
--- a/src/libexpr/include/nix/expr/eval.hh
+++ b/src/libexpr/include/nix/expr/eval.hh
@@ -243,10 +243,9 @@ struct StaticEvalSymbols
 {
     Symbol with, outPath, drvPath, type, meta, name, value, system, overrides, outputs, outputName, ignoreNulls, file,
         line, column, functor, toString, right, wrong, structuredAttrs, json, allowedReferences, allowedRequisites,
-        disallowedReferences, disallowedRequisites, maxSize, maxClosureSize, builder, args, gitDir, resolver, rev,
-        checkoutPath, targets, contentAddressed, impure, outputHash, outputHashAlgo, outputHashMode,
-        recurseForDerivations, description, self, epsilon, startSet, operator_, key, path, prefix, outputSpecified,
-        __meta;
+        disallowedReferences, disallowedRequisites, maxSize, maxClosureSize, builder, args, contentAddressed, impure,
+        outputHash, outputHashAlgo, outputHashMode, recurseForDerivations, description, self, epsilon, startSet,
+        operator_, key, path, prefix, outputSpecified, __meta;
 
     Expr::AstSymbols exprSymbols;
 
@@ -284,11 +283,6 @@ struct StaticEvalSymbols
             .maxClosureSize = alloc.create("maxClosureSize"),
             .builder = alloc.create("builder"),
             .args = alloc.create("args"),
-            .gitDir = alloc.create("gitDir"),
-            .resolver = alloc.create("resolver"),
-            .rev = alloc.create("rev"),
-            .checkoutPath = alloc.create("checkoutPath"),
-            .targets = alloc.create("targets"),
             .contentAddressed = alloc.create("__contentAddressed"),
             .impure = alloc.create("__impure"),
             .outputHash = alloc.create("outputHash"),
diff --git a/src/libexpr/primops/tecnix.cc b/src/libexpr/primops/tecnix.cc
index 8f1065c22..534c0158b 100644
--- a/src/libexpr/primops/tecnix.cc
+++ b/src/libexpr/primops/tecnix.cc
@@ -129,7 +129,7 @@ static std::string
 resolveRev(EvalState & state, const PosIdx pos, const Bindings & attrs, const std::string & checkoutPath)
 {
     // Check for explicit rev attr
-    auto revAttr = attrs.get(state.s.rev);
+    auto revAttr = attrs.get(state.symbols.create("rev"));
     if (revAttr) {
         auto sha = state.forceStringNoCtx(*revAttr->value, pos, "while evaluating the 'rev' argument");
         if (!sha.empty())
@@ -181,19 +181,19 @@ static const Bindings & forceTecnixBuiltinAttrs(EvalState & state, const PosIdx
 
 static void parseTecnixRepoArgs(EvalState & state, const PosIdx pos, const Bindings & attrs, TecnixArgs & result)
 {
-    auto gitDirAttr = attrs.get(state.s.gitDir);
+    auto gitDirAttr = attrs.get(state.symbols.create("gitDir"));
     if (!gitDirAttr)
         state.error<EvalError>("'gitDir' attribute required").atPos(pos).debugThrow();
     result.gitDir =
         std::string(state.forceStringNoCtx(*gitDirAttr->value, pos, "while evaluating the 'gitDir' argument"));
 
-    auto resolverAttr = attrs.get(state.s.resolver);
+    auto resolverAttr = attrs.get(state.symbols.create("resolver"));
     if (!resolverAttr)
         state.error<EvalError>("'resolver' attribute required").atPos(pos).debugThrow();
     result.resolver =
         std::string(state.forceStringNoCtx(*resolverAttr->value, pos, "while evaluating the 'resolver' argument"));
 
-    auto checkoutPathAttr = attrs.get(state.s.checkoutPath);
+    auto checkoutPathAttr = attrs.get(state.symbols.create("checkoutPath"));
     if (checkoutPathAttr)
         result.checkoutPath = std::string(
             state.forceStringNoCtx(*checkoutPathAttr->value, pos, "while evaluating the 'checkoutPath' argument"));
@@ -273,7 +273,7 @@ parseTecnixResolverArgsValue(EvalState & state, const PosIdx pos, const Bindings
 
 static std::vector<std::string> parseTecnixTargets(EvalState & state, const PosIdx pos, const Bindings & attrs)
 {
-    auto targetsAttr = attrs.get(state.s.targets);
+    auto targetsAttr = attrs.get(state.symbols.create("targets"));
     if (!targetsAttr)
         state.error<EvalError>("'targets' attribute required").atPos(pos).debugThrow();