Merge pull request #297 from DeterminateSystems/eelcodolstra/fh-1214-make-fetchtree-final

fetchTree: Implicitly set __final = true when a narHash is supplied

Author: edolstra

src/libexpr/primops/fetchTree.cc

@@ -77,7 +77,6 @@ struct FetchTreeParams
     bool emptyRevFallback = false;
     bool allowNameArgument = false;
     bool isFetchGit = false;
-    bool isFinal = false;
 };
 
 static void fetchTree(
@@ -202,12 +201,8 @@ static void fetchTree(
 
     state.checkURI(input.toURLString());
 
-    if (params.isFinal) {
+    if (input.getNarHash())
         input.attrs.insert_or_assign("__final", Explicit<bool>(true));
-    } else {
-        if (input.isFinal())
-            throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string());
-    }
 
     auto cachedInput =
         state.inputCache->getAccessor(state.fetchSettings, *state.store, input, fetchers::UseRegistries::No);
@@ -348,18 +343,6 @@ static RegisterPrimOp primop_fetchTree({
     .fun = prim_fetchTree,
 });
 
-void prim_fetchFinalTree(EvalState & state, const PosIdx pos, Value ** args, Value & v)
-{
-    fetchTree(state, pos, args, v, {.isFinal = true});
-}
-
-static RegisterPrimOp primop_fetchFinalTree({
-    .name = "fetchFinalTree",
-    .args = {"input"},
-    .fun = prim_fetchFinalTree,
-    .internal = true,
-});
-
 static void fetch(
     EvalState & state,
     const PosIdx pos,

src/libflake/call-flake.nix

@@ -10,9 +10,6 @@ lockFileStr:
 # unlocked trees.
 overrides:
 
-# This is `prim_fetchFinalTree`.
-fetchTreeFinal:
-
 let
   inherit (builtins) mapAttrs;
 
@@ -62,7 +59,7 @@ let
         else
           # FIXME: remove obsolete node.info.
           # Note: lock file entries are always final.
-          fetchTreeFinal (node.info or { } // removeAttrs node.locked [ "dir" ]);
+          builtins.fetchTree (node.info or { } // removeAttrs node.locked [ "dir" ]);
 
       subdir = overrides.${key}.dir or node.locked.dir or "";
 

src/libflake/flake.cc

@@ -1008,10 +1008,7 @@ void callFlake(EvalState & state, const LockedFlake & lockedFlake, Value & vRes)
     auto vLocks = state.allocValue();
     vLocks->mkString(lockFileStr, state.mem);
 
-    auto vFetchFinalTree = get(state.internalPrimOps, "fetchFinalTree");
-    assert(vFetchFinalTree);
-
-    Value * args[] = {vLocks, &vOverrides, *vFetchFinalTree};
+    Value * args[] = {vLocks, &vOverrides};
     state.callFunction(*vCallFlake, args, vRes, noPos);
 }
 

src/libflake/include/nix/flake/flake.hh

@@ -246,11 +246,4 @@ void emitTreeAttrs(
     bool emptyRevFallback = false,
     bool forceDirty = false);
 
-/**
- * An internal builtin similar to `fetchTree`, except that it
- * always treats the input as final (i.e. no attributes can be
- * added/removed/changed).
- */
-void prim_fetchFinalTree(EvalState & state, const PosIdx pos, Value ** args, Value & v);
-
 } // namespace nix

tests/functional/tarball.sh

@@ -38,6 +38,9 @@ test_tarball() {
 
     [[ $(nix eval --impure --expr "(fetchTree file://$tarball).lastModified") = 1000000000 ]]
 
+    # fetchTree with a narHash is implicitly final, so it doesn't return attributes like lastModified.
+    [[ $(nix eval --impure --expr "(fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; }) ? lastModified") = false ]]
+
     nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2
     nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" 2>&1 | grep 'true'
 

tests/nixos/fetchers-substitute.nix

@@ -150,28 +150,5 @@
       content = importer.succeed(f"cat {result_path}/hello.txt").strip()
       assert content == "Hello from tarball!", f"Content mismatch: {content}"
       print("✓ fetchTarball content verified!")
-
-      ##########################################
-      # Test 3: Verify fetchTree does NOT substitute (preserves metadata)
-      ##########################################
-
-      print("Testing that fetchTree without __final does NOT use substitution...")
-
-      # fetchTree with just narHash (not __final) should try to download, which will fail
-      # since the file doesn't exist on the importer
-      exit_code = importer.fail(f"""
-        nix-instantiate --eval --json --read-write-mode --expr '
-          builtins.fetchTree {{
-            type = "tarball";
-            url = "file:///only-on-substituter.tar.gz";
-            narHash = "{tarball_hash_sri}";
-          }}
-        ' 2>&1
-      """)
-
-      # Should fail with "does not exist" since it tries to download instead of substituting
-      assert "does not exist" in exit_code or "Couldn't open file" in exit_code, f"Expected download failure, got: {exit_code}"
-      print("✓ fetchTree correctly does NOT substitute non-final inputs!")
-      print("  (This preserves metadata like lastModified from the actual fetch)")
     '';
 }

tests/nixos/github-flakes.nix

@@ -266,7 +266,7 @@ in
 
       # Fetching with an incorrect NAR hash should fail.
       out = client.fail(f"nix eval --no-trust-tarballs-from-git-forges --raw --expr '(fetchTree \"github:fancy-enterprise/private-flake/{info['revision']}?narHash=sha256-HsrRFZYg69qaVe/wDyWBYLeS6ca7ACEJg2Z%2BGpEFw4A%3D\").narHash' 2>&1")
-      assert "NAR hash mismatch in input" in out, "NAR hash check did not fail with the expected error"
+      assert "mismatch in field 'narHash'" in out, "NAR hash check did not fail with the expected error"
 
       # Fetching without a narHash should succeed if trust-github is set and fail otherwise.
       client.succeed(f"nix eval --raw --expr 'builtins.fetchTree github:github:fancy-enterprise/private-flake/{info['revision']}'")