Warn against use of toString ./path as a derivation attribute

This never really worked, since the resulting string depends on the
location of the source tree (which is only predictable when using
flakes) and is not in the context of the derivation (so it's
inaccessible when sandboxing is enabled, and not reproducible when
sandboxing is disabled).

But we don't want to change evaluation results, so warn against it and
simulate the pre-lazy-trees behaviour by replacing virtual paths with
the store paths that *would* exist if we copied the source tree to the
store.

src/libexpr/eval.hh

@@ -368,6 +368,12 @@ public:
     /* Decode a path encoded by `encodePath()`. */
     SourcePath decodePath(std::string_view s, PosIdx pos = noPos);
 
+    /* Rewrite virtual paths to store paths without actually
+       materializing those store paths. This is a backward
+       compatibility hack to make buggy derivation attributes like
+       `tostring ./bla` produce the same evaluation result. */
+    std::string rewriteVirtualPaths(std::string_view s, PosIdx pos);
+
     /* Replace all virtual paths (i.e. `/nix/store/lazylazy...`) in a
        string by a pretty-printed rendition of the corresponding input
        accessor (e.g. `«github:NixOS/nix/<rev>»`). */

src/libexpr/paths.cc

@@ -1,5 +1,6 @@
 #include "eval.hh"
 #include "util.hh"
+#include "store-api.hh"
 
 namespace nix {
 
@@ -64,16 +65,16 @@ std::string EvalState::prettyPrintPaths(std::string_view s)
 {
     std::string res;
 
-    size_t pos = 0;
+    size_t p = 0;
 
     while (true) {
-        auto m = s.find(virtualPathMarker, pos);
+        auto m = s.find(virtualPathMarker, p);
         if (m == s.npos) {
-            res.append(s.substr(pos));
+            res.append(s.substr(p));
             return res;
         }
 
-        res.append(s.substr(pos, m - pos));
+        res.append(s.substr(p, m - p));
 
         auto end = s.find_first_of(" \n\r\t'\"’:", m);
         if (end == s.npos)
@@ -83,11 +84,53 @@ std::string EvalState::prettyPrintPaths(std::string_view s)
             auto path = decodePath(s.substr(m, end - m), noPos);
             res.append(path.to_string());
         } catch (...) {
-            throw;
-            res.append(s.substr(pos, end - m));
+            res.append(s.substr(m, end - m));
         }
 
-        pos = end;
+        p = end;
+    }
+}
+
+std::string EvalState::rewriteVirtualPaths(std::string_view s, PosIdx pos)
+{
+    std::string res;
+
+    size_t p = 0;
+
+    while (true) {
+        auto m = s.find("lazylazy0000000000000000", p); // FIXME
+        if (m == s.npos) {
+            res.append(s.substr(p));
+            return res;
+        }
+
+        res.append(s.substr(p, m - p));
+
+        auto end = m + StorePath::HashLen;
+
+        if (end > s.size()) {
+            res.append(s.substr(m));
+            return res;
+        }
+
+        try {
+            size_t number = std::stoi(std::string(s.substr(m + 24, 8)), nullptr, 10); // FIXME
+
+            auto accessor = inputAccessors.find(number);
+            assert(accessor != inputAccessors.end()); // FIXME
+
+            warn(
+                "derivation at %s has an attribute that refers to source tree '%s' without context; this does not work correctly",
+                positions[pos], accessor->second->showPath(CanonPath::root));
+
+            // FIXME: cache this.
+            res.append(store->computeStorePath("source", *accessor->second, CanonPath::root).first.hashPart());
+        } catch (...) {
+            ignoreException();
+            res.append(s.substr(m, end - m));
+        }
+
+        p = end;
     }
 }
 

src/libexpr/primops.cc

@@ -1347,6 +1347,14 @@ static void derivationStrictInternal(
     /* Rewrite virtual paths (from encodePath()) to real store paths. */
     drv.applyRewrites(rewrites);
 
+    /* For backward compatibility, rewrite virtual paths without
+       context (e.g. passing `toString ./foo`) to store paths that
+       don't exist. This is a bug in user code (since those strings
+       don't have a context, so aren't accessible from a sandbox) but
+       we don't want to change evaluation results.  */
+    for (auto & [name, value] : drv.env)
+        value = state.rewriteVirtualPaths(value, pos);
+
     /* Do we have all required attributes? */
     if (drv.builder == "")
         state.error<EvalError>("required attribute 'builder' missing")
@@ -2471,13 +2479,13 @@ static void prim_path(EvalState & state, const PosIdx pos, Value * * args, Value
             expectedHash = newHashAllowEmpty(state.forceStringNoCtx(*attr.value, attr.pos, "while evaluating the `sha256` attribute passed to builtins.path"), HashAlgorithm::SHA256);
         else
             state.error<EvalError>(
-                "unsupported argument '%1%' to 'addPath'",
+                "unsupported argument '%1%' to 'builtins.path'",
                 state.symbols[attr.name]
             ).atPos(attr.pos).debugThrow();
     }
     if (!path)
         state.error<EvalError>(
-            "missing required 'path' attribute in the first argument to builtins.path"
+            "missing required 'path' attribute in the first argument to 'builtins.path'"
         ).atPos(pos).debugThrow();
     if (name.empty())
         name = path->baseName();

tests/functional/flakes/lazy-trees.sh

@@ -7,15 +7,32 @@ mkdir -p $flake1Dir
 cat > $flake1Dir/flake.nix <<EOF
 {
   outputs = { self }: {
-    everything = ./.;
+    everything = builtins.path { path = ./.; name = "source"; };
     foo = ./foo;
     trace = builtins.trace "path \${toString ./foo}" 123;
     throw = builtins.throw "path \${toString ./foo}" 123;
     abort = builtins.abort "path \${toString ./foo}" 123;
+
+    drv1 = with import ./config.nix; mkDerivation {
+      name = "bla";
+      buildCommand = ''
+        mkdir \$out
+        ln -s \${self.foo} \$out/foo
+      '';
+    };
+
+    drv2 = with import ./config.nix; mkDerivation {
+      name = "bla";
+      buildCommand = ''
+        mkdir \$out
+        ln -s \${toString self.foo} \$out/foo
+      '';
+    };
   };
 }
 EOF
 
+cp ../config.nix $flake1Dir/
 echo foo > $flake1Dir/foo
 
 # Add an uncopyable file to test laziness.
@@ -31,3 +48,24 @@ nix build --json --out-link $TEST_ROOT/result $flake1Dir#foo
 nix eval $flake1Dir#trace 2>&1 | grep "trace: path $flake1Dir/foo"
 expectStderr 1 nix eval $flake1Dir#throw 2>&1 | grep "error: path $flake1Dir/foo"
 expectStderr 1 nix eval $flake1Dir#abort 2>&1 | grep "error:.*path $flake1Dir/foo"
+
+nix build --out-link $TEST_ROOT/result $flake1Dir#drv1
+[[ $(cat $TEST_ROOT/result/foo) = foo ]]
+[[ $(realpath $TEST_ROOT/result/foo) =~ $NIX_STORE_DIR/.*-foo$ ]]
+
+# Check for warnings about passing `toString ./path` to a derivation.
+nix build --out-link $TEST_ROOT/result $flake1Dir#drv2 2>&1 | grep "warning: derivation.*has an attribute that refers to source tree"
+[[ $(readlink $TEST_ROOT/result/foo) =~ $NIX_STORE_DIR/lazylazy.*-source/foo$ ]]
+
+# If the source tree can be hashed, the virtual path will be rewritten
+# to the path that would exist if the source tree were copied to the
+# Nix store.
+rm $flake1Dir/fifo
+nix build --out-link $TEST_ROOT/result $flake1Dir#drv2
+
+# But we don't *actually* copy it.
+(! realpath $TEST_ROOT/result/foo)
+
+# Force the path to exist.
+path=$(nix eval --raw $flake1Dir#everything)
+realpath $TEST_ROOT/result/foo