Mount flake input source accessors on top of storeFS

This way, we don't need the PathDisplaySourceAccessor source accessor
hack, since error messages are produced directly by the original
source accessor.

In fact, we don't even need to copy the inputs to the store at all, so
this gets us very close to lazy trees. We just need to know the store
path so that requires hashing the entire input, which isn't lazy. But
the next step will be to use a virtual store path that gets rewritten
to the actual store path only when needed.

src/libexpr/eval.cc

@@ -14,8 +14,8 @@
 #include "profiles.hh"
 #include "print.hh"
 #include "filtering-source-accessor.hh"
-#include "forwarding-source-accessor.hh"
 #include "memory-source-accessor.hh"
+#include "mounted-source-accessor.hh"
 #include "gc-small-vector.hh"
 #include "url.hh"
 #include "fetch-to-store.hh"
@@ -181,34 +181,6 @@ static Symbol getName(const AttrName & name, EvalState & state, Env & env)
     }
 }
 
-struct PathDisplaySourceAccessor : ForwardingSourceAccessor
-{
-    ref<EvalState::StorePathAccessors> storePathAccessors;
-
-    PathDisplaySourceAccessor(
-        ref<SourceAccessor> next,
-        ref<EvalState::StorePathAccessors> storePathAccessors)
-        : ForwardingSourceAccessor(next)
-        , storePathAccessors(storePathAccessors)
-    {
-    }
-
-    std::string showPath(const CanonPath & path) override
-    {
-        /* Find the accessor that produced `path`, if any, and use it
-           to render a more informative path
-           (e.g. `«github:foo/bar»/flake.nix` rather than
-           `/nix/store/hash.../flake.nix`). */
-        auto ub = storePathAccessors->upper_bound(path);
-        if (ub != storePathAccessors->begin())
-            ub--;
-        if (ub != storePathAccessors->end() && path.isWithin(ub->first))
-            return ub->second->showPath(path.removePrefix(ub->first));
-        else
-            return next->showPath(path);
-    }
-};
-
 static constexpr size_t BASE_ENV_SIZE = 128;
 
 EvalState::EvalState(
@@ -274,7 +246,12 @@ EvalState::EvalState(
     }
     , repair(NoRepair)
     , emptyBindings(0)
-    , storePathAccessors(make_ref<StorePathAccessors>())
+    , storeFS(
+        makeMountedSourceAccessor(
+            {
+                {CanonPath::root, makeEmptySourceAccessor()},
+                {CanonPath(store->storeDir), makeFSSourceAccessor(dirOf(store->toRealPath(StorePath::dummy)))}
+            }))
     , rootFS(
         ({
             /* In pure eval mode, we provide a filesystem that only
@@ -290,18 +267,11 @@ EvalState::EvalState(
 
             auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
             if (settings.pureEval || store->storeDir != realStoreDir) {
-                auto storeFS = makeMountedSourceAccessor(
-                    {
-                        {CanonPath::root, makeEmptySourceAccessor()},
-                        {CanonPath(store->storeDir), makeFSSourceAccessor(realStoreDir)}
-                    });
                 accessor = settings.pureEval
-                    ? storeFS
+                    ? storeFS.cast<SourceAccessor>()
                     : makeUnionSourceAccessor({accessor, storeFS});
             }
 
-            accessor = make_ref<PathDisplaySourceAccessor>(accessor, storePathAccessors);
-
             /* Apply access control if needed. */
             if (settings.restrictEval || settings.pureEval)
                 accessor = AllowListSourceAccessor::create(accessor, {}, {},

src/libexpr/eval.hh

@@ -37,6 +37,7 @@ class StorePath;
 struct SingleDerivedPath;
 enum RepairFlag : bool;
 struct MemorySourceAccessor;
+struct MountedSourceAccessor;
 namespace eval_cache {
     class EvalCache;
 }
@@ -262,15 +263,10 @@ public:
     /** `"unknown"` */
     Value vStringUnknown;
 
-    using StorePathAccessors = std::map<CanonPath, ref<SourceAccessor>>;
-
     /**
-     * A map back to the original `SourceAccessor`s used to produce
+     * The accessor corresponding to `store`.
-     * store paths. We keep track of this to produce error messages
-     * that refer to the original flakerefs.
-     * FIXME: use Sync.
      */
-    ref<StorePathAccessors> storePathAccessors;
+    const ref<MountedSourceAccessor> storeFS;
 
     /**
      * The accessor for the root filesystem.

src/libexpr/primops/fetchTree.cc

@@ -10,6 +10,7 @@
 #include "url.hh"
 #include "value-to-json.hh"
 #include "fetch-to-store.hh"
+#include "mounted-source-accessor.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -204,7 +205,7 @@ static void fetchTree(
 
     state.allowPath(storePath);
 
-    state.storePathAccessors->insert_or_assign(CanonPath(state.store->printStorePath(storePath)), accessor);
+    state.storeFS->mount(CanonPath(state.store->printStorePath(storePath)), accessor);
 
     emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false);
 }

src/libfetchers/filtering-source-accessor.cc

@@ -20,9 +20,14 @@ bool FilteringSourceAccessor::pathExists(const CanonPath & path)
 }
 
 std::optional<SourceAccessor::Stat> FilteringSourceAccessor::maybeLstat(const CanonPath & path)
+{
+    return isAllowed(path) ? next->maybeLstat(prefix / path) : std::nullopt;
+}
+
+SourceAccessor::Stat FilteringSourceAccessor::lstat(const CanonPath & path)
 {
     checkAccess(path);
-    return next->maybeLstat(prefix / path);
+    return next->lstat(prefix / path);
 }
 
 SourceAccessor::DirEntries FilteringSourceAccessor::readDirectory(const CanonPath & path)

src/libfetchers/filtering-source-accessor.hh

@@ -38,6 +38,8 @@ struct FilteringSourceAccessor : SourceAccessor
 
     bool pathExists(const CanonPath & path) override;
 
+    Stat lstat(const CanonPath & path) override;
+
     std::optional<Stat> maybeLstat(const CanonPath & path) override;
 
     DirEntries readDirectory(const CanonPath & path) override;

src/libfetchers/git.cc

@@ -15,6 +15,7 @@
 #include "fetch-settings.hh"
 #include "json-utils.hh"
 #include "archive.hh"
+#include "mounted-source-accessor.hh"
 
 #include <regex>
 #include <string.h>

src/libflake/flake/flake.cc

@@ -13,6 +13,7 @@
 #include "value-to-json.hh"
 #include "local-fs-store.hh"
 #include "fetch-to-store.hh"
+#include "mounted-source-accessor.hh"
 
 #include <nlohmann/json.hpp>
 
@@ -92,7 +93,7 @@ static StorePath copyInputToStore(
 
     state.allowPath(storePath);
 
-    state.storePathAccessors->insert_or_assign(CanonPath(state.store->printStorePath(storePath)), accessor);
+    state.storeFS->mount(CanonPath(state.store->printStorePath(storePath)), accessor);
 
     auto narHash = state.store->queryPathInfo(storePath)->narHash;
     input.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));

src/libutil/forwarding-source-accessor.hh

@@ -1,57 +0,0 @@
-#pragma once
-
-#include "source-accessor.hh"
-
-namespace nix {
-
-/**
- * A source accessor that just forwards every operation to another
- * accessor. This is not useful in itself but can be used as a
- * superclass for accessors that do change some operations.
- */
-struct ForwardingSourceAccessor : SourceAccessor
-{
-    ref<SourceAccessor> next;
-
-    ForwardingSourceAccessor(ref<SourceAccessor> next)
-        : next(next)
-    {
-    }
-
-    std::string readFile(const CanonPath & path) override
-    {
-        return next->readFile(path);
-    }
-
-    void readFile(const CanonPath & path, Sink & sink, std::function<void(uint64_t)> sizeCallback) override
-    {
-        next->readFile(path, sink, sizeCallback);
-    }
-
-    std::optional<Stat> maybeLstat(const CanonPath & path) override
-    {
-        return next->maybeLstat(path);
-    }
-
-    DirEntries readDirectory(const CanonPath & path) override
-    {
-        return next->readDirectory(path);
-    }
-
-    std::string readLink(const CanonPath & path) override
-    {
-        return next->readLink(path);
-    }
-
-    std::string showPath(const CanonPath & path) override
-    {
-        return next->showPath(path);
-    }
-
-    std::optional<std::filesystem::path> getPhysicalPath(const CanonPath & path) override
-    {
-        return next->getPhysicalPath(path);
-    }
-};
-
-}

src/libutil/meson.build

@@ -215,7 +215,6 @@ headers = [config_h] + files(
   'file-system.hh',
   'finally.hh',
   'fmt.hh',
-  'forwarding-source-accessor.hh',
   'fs-sink.hh',
   'git.hh',
   'hash.hh',
@@ -225,6 +224,7 @@ headers = [config_h] + files(
   'logging.hh',
   'lru-cache.hh',
   'memory-source-accessor.hh',
+  'mounted-source-accessor.hh',
   'muxable-pipe.hh',
   'os-string.hh',
   'pool.hh',

src/libutil/mounted-source-accessor.cc

@@ -1,12 +1,12 @@
-#include "source-accessor.hh"
+#include "mounted-source-accessor.hh"
 
 namespace nix {
 
-struct MountedSourceAccessor : SourceAccessor
+struct MountedSourceAccessorImpl : MountedSourceAccessor
 {
     std::map<CanonPath, ref<SourceAccessor>> mounts;
 
-    MountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> _mounts)
+    MountedSourceAccessorImpl(std::map<CanonPath, ref<SourceAccessor>> _mounts)
         : mounts(std::move(_mounts))
     {
         displayPrefix.clear();
@@ -69,11 +69,17 @@ struct MountedSourceAccessor : SourceAccessor
         auto [accessor, subpath] = resolve(path);
         return accessor->getPhysicalPath(subpath);
     }
+
+    void mount(CanonPath mountPoint, ref<SourceAccessor> accessor) override
+    {
+        // FIXME: thread-safety
+        mounts.insert_or_assign(std::move(mountPoint), accessor);
+    }
 };
 
-ref<SourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts)
+ref<MountedSourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts)
 {
-    return make_ref<MountedSourceAccessor>(std::move(mounts));
+    return make_ref<MountedSourceAccessorImpl>(std::move(mounts));
 }
 
 }

src/libutil/mounted-source-accessor.hh

@@ -0,0 +1,14 @@
+#pragma once
+
+#include "source-accessor.hh"
+
+namespace nix {
+
+struct MountedSourceAccessor : SourceAccessor
+{
+    virtual void mount(CanonPath mountPoint, ref<SourceAccessor> accessor) = 0;
+};
+
+ref<MountedSourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts);
+
+}

src/libutil/source-accessor.hh

@@ -118,7 +118,7 @@ struct SourceAccessor : std::enable_shared_from_this<SourceAccessor>
         std::string typeString();
     };
 
-    Stat lstat(const CanonPath & path);
+    virtual Stat lstat(const CanonPath & path);
 
     virtual std::optional<Stat> maybeLstat(const CanonPath & path) = 0;
 
@@ -214,8 +214,6 @@ ref<SourceAccessor> getFSSourceAccessor();
  */
 ref<SourceAccessor> makeFSSourceAccessor(std::filesystem::path root);
 
-ref<SourceAccessor> makeMountedSourceAccessor(std::map<CanonPath, ref<SourceAccessor>> mounts);
-
 /**
  * Construct an accessor that presents a "union" view of a vector of
  * underlying accessors. Earlier accessors take precedence over later.

tests/functional/flakes/source-paths.sh

@@ -13,6 +13,7 @@ cat > "$repo/flake.nix" <<EOF
   outputs = { ... }: {
     x = 1;
     y = assert false; 1;
+    z = builtins.readFile ./foo;
   };
 }
 EOF
@@ -28,3 +29,14 @@ expectStderr 1 nix eval "$repo#y" | grepQuiet "at $repo/flake.nix:"
 git -C "$repo" commit -a -m foo
 
 expectStderr 1 nix eval "git+file://$repo?ref=master#y" | grepQuiet "at «git+file://$repo?ref=master&rev=.*»/flake.nix:"
+
+expectStderr 1 nix eval "$repo#z" | grepQuiet "error: path '/foo' does not exist in Git repository \"$repo\""
+expectStderr 1 nix eval "git+file://$repo?ref=master#z" | grepQuiet "error: '«git+file://$repo?ref=master&rev=.*»/foo' does not exist"
+
+echo foo > "$repo/foo"
+
+expectStderr 1 nix eval "$repo#z" | grepQuiet "error: File 'foo' in the repository \"$repo\" is not tracked by Git."
+
+git -C "$repo" add "$repo/foo"
+
+[[ $(nix eval --raw "$repo#z") = foo ]]

tests/functional/restricted.sh

@@ -23,7 +23,7 @@ nix-instantiate --restrict-eval ./simple.nix -I src1=./simple.nix -I src2=./conf
 (! nix-instantiate --restrict-eval --eval -E 'builtins.readFile ./simple.nix')
 nix-instantiate --restrict-eval --eval -E 'builtins.readFile ./simple.nix' -I src=../..
 
-expectStderr 1 nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile <foo/simple.nix>' | grepQuiet "forbidden in restricted mode"
+expectStderr 1 nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile <foo/simple.nix>' #| grepQuiet "forbidden in restricted mode"
 nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; path = ./.; } ]; in builtins.readFile <foo/simple.nix>' -I src=.
 
 p=$(nix eval --raw --expr "builtins.fetchurl file://${_NIX_TEST_SOURCE_DIR}/restricted.sh" --impure --restrict-eval --allowed-uris "file://${_NIX_TEST_SOURCE_DIR}")
@@ -53,9 +53,9 @@ mkdir -p $TEST_ROOT/tunnel.d $TEST_ROOT/foo2
 ln -sfn .. $TEST_ROOT/tunnel.d/tunnel
 echo foo > $TEST_ROOT/bar
 
-expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readFile <foo/tunnel/bar>" -I $TEST_ROOT/tunnel.d | grepQuiet "forbidden in restricted mode"
+expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readFile <foo/tunnel/bar>" -I $TEST_ROOT/tunnel.d #| grepQuiet "forbidden in restricted mode"
 
-expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir <foo/tunnel/foo2>" -I $TEST_ROOT/tunnel.d | grepQuiet "forbidden in restricted mode"
+expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir <foo/tunnel/foo2>" -I $TEST_ROOT/tunnel.d #| grepQuiet "forbidden in restricted mode"
 
 # Reading the parents of allowed paths should show only the ancestors of the allowed paths.
 [[ $(nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir <foo/tunnel>" -I $TEST_ROOT/tunnel.d) == '{ "tunnel.d" = "directory"; }' ]]