From 095009488af5c1596f79807049272689287d4efa Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Tue, 30 Apr 2024 15:18:34 +0200
Subject: [PATCH] Fix fileset compatibility

It was getting confused between the virtual path representation
(/nix/store/<virtual>-source) on the one hand, and the path type
representation on the other hand. In particular, we need to make sure
that `dirOf <flake-root>` returns /nix/store rather than /.
---
 src/libexpr/eval.cc               | 15 ++++++++++++---
 src/libexpr/primops.cc            | 12 +++++++++---
 tests/functional/flakes/flakes.sh |  2 +-
 3 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index f24b82998ab..8e9e0c56083 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -260,7 +260,12 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
     } else {
         Value nameValue;
         name.expr->eval(state, env, nameValue);
-        state.forceStringNoCtx(nameValue, name.expr->getPos(), "while evaluating an attribute name");
+        // FIXME: should use forceStringNoCtx(). However, that
+        // requires us to make builtins.substring more precise about
+        // propagating contexts. E.g. `builtins.substring 44 (-1)
+        // "${./src}"` should not have a context (at least not a
+        // `InputAccessor` context).
+        state.forceString(nameValue, name.expr->getPos(), "while evaluating an attribute name");
         return state.symbols.create(nameValue.string_view());
     }
 }
@@ -2453,8 +2458,12 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext
     }
 
     /* Handle path values directly, without coercing to a string. */
-    if (v.type() == nPath)
-        return v.path();
+    if (v.type() == nPath) {
+        auto path = v.path();
+        return path.accessor == rootFS
+            ? decodePath(path.path.abs())
+            : path;
+    }
 
     /* Similarly, handle __toString where the result may be a path
        value. */
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 77038bc62b6..687fd85c242 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -1648,6 +1648,7 @@ static std::string_view legacyBaseNameOf(std::string_view path)
 static void prim_baseNameOf(EvalState & state, const PosIdx pos, Value * * args, Value & v)
 {
     NixStringContext context;
+    // FIXME: handle roots of source trees (should return "<hash>-source").
     v.mkString(legacyBaseNameOf(*state.coerceToString(pos, *args[0], context,
             "while evaluating the first argument passed to builtins.baseNameOf",
             false, false)), context);
@@ -1672,14 +1673,19 @@ static RegisterPrimOp primop_baseNameOf({
 });
 
 /* Return the directory of the given path, i.e., everything before the
-   last slash.  Return either a path or a string depending on the type
-   of the argument. */
+   last slash. Return either a path or a string depending on the type
+   of the argument. For backwards compatibility, the parent of a tree
+   other than rootFS is the store directory. */
 static void prim_dirOf(EvalState & state, const PosIdx pos, Value * * args, Value & v)
 {
     state.forceValue(*args[0], pos);
     if (args[0]->type() == nPath) {
         auto path = args[0]->path();
-        v.mkPath(path.path.isRoot() ? path : path.parent());
+        v.mkPath(path.path.isRoot()
+            ? path.accessor != state.rootFS
+            ? SourcePath{state.rootFS, CanonPath(state.store->storeDir)}
+            : SourcePath{state.rootFS}
+            : path.parent());
     } else {
         NixStringContext context;
         auto path = state.coerceToString(pos, *args[0], context,
diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh
index 708d0bb84a7..650241106ee 100644
--- a/tests/functional/flakes/flakes.sh
+++ b/tests/functional/flakes/flakes.sh
@@ -233,7 +233,7 @@ nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"$flake1Dir\").pack
 nix build -o "$TEST_ROOT/result" --expr "(builtins.getFlake \"git+file://$flake1Dir?rev=$hash2\").packages.$system.default"
 
 # Regression test for dirOf on the root of the flake.
-[[ $(nix eval --json flake1#parent) = '"/"' ]]
+[[ $(nix eval --json flake1#parent) = \""$NIX_STORE_DIR"\" ]]
 
 # Building a flake with an unlocked dependency should fail in pure mode.
 (! nix build -o "$TEST_ROOT/result" flake2#bar --no-registries)