From 5d7cee32e6ddb5087b999a8f9ff95e36786c6892 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra <edolstra@gmail.com> Date: Thu, 20 Feb 2025 01:09:49 +0100 Subject: [PATCH] Mount flake input source accessors on top of storeFS This makes paths in error messages behave similar to lazy-trees, e.g. instead of store paths like error: attribute 'foobar' missing at /nix/store/ddzfiipzqlrh3gnprmqbadnsnrxsmc9i-source/machine/configuration.nix:209:7: 208| 209| pkgs.foobar | ^ 210| ]; you now get error: attribute 'foobar' missing at /home/eelco/Misc/eelco-configurations/machine/configuration.nix:209:7: 208| 209| pkgs.foobar | ^ 210| ]; 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 | 4 +-- src/libexpr/include/nix/expr/eval.hh | 3 +- src/libexpr/primops/fetchMercurial.cc | 2 +- src/libexpr/primops/fetchTree.cc | 5 ++- src/libfetchers/fetchers.cc | 32 ++++++++----------- src/libfetchers/filtering-source-accessor.cc | 7 +++- src/libfetchers/git.cc | 1 + .../include/nix/fetchers/fetchers.hh | 2 +- .../nix/fetchers/filtering-source-accessor.hh | 2 ++ src/libflake/flake/flake.cc | 3 ++ src/libutil/include/nix/util/meson.build | 1 + .../nix/util/mounted-source-accessor.hh | 14 ++++++++ .../include/nix/util/source-accessor.hh | 4 +-- src/libutil/mounted-source-accessor.cc | 16 +++++++--- src/nix/env.cc | 1 + src/nix/flake.cc | 2 +- tests/functional/flakes/source-paths.sh | 19 +++++++++++ tests/functional/restricted.sh | 6 ++-- 18 files changed, 87 insertions(+), 37 deletions(-) create mode 100644 src/libutil/include/nix/util/mounted-source-accessor.hh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 9d60676f5..9e686aa75 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -15,11 +15,11 @@ #include "nix/expr/print.hh" #include "nix/fetchers/filtering-source-accessor.hh" #include "nix/util/memory-source-accessor.hh" +#include "nix/util/mounted-source-accessor.hh" #include "nix/expr/gc-small-vector.hh" #include "nix/util/url.hh" #include "nix/fetchers/fetch-to-store.hh" #include "nix/fetchers/tarball.hh" - #include "parser-tab.hh" #include <algorithm> @@ -286,7 +286,7 @@ EvalState::EvalState( auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy)); if (settings.pureEval || store->storeDir != realStoreDir) { accessor = settings.pureEval - ? storeFS + ? storeFS.cast<SourceAccessor>() : makeUnionSourceAccessor({accessor, storeFS}); } diff --git a/src/libexpr/include/nix/expr/eval.hh b/src/libexpr/include/nix/expr/eval.hh index 61da225fc..9623c2a9c 100644 --- a/src/libexpr/include/nix/expr/eval.hh +++ b/src/libexpr/include/nix/expr/eval.hh @@ -40,6 +40,7 @@ class StorePath; struct SingleDerivedPath; enum RepairFlag : bool; struct MemorySourceAccessor; +struct MountedSourceAccessor; namespace eval_cache { class EvalCache; } @@ -268,7 +269,7 @@ public: /** * The accessor corresponding to `store`. */ - const ref<SourceAccessor> storeFS; + const ref<MountedSourceAccessor> storeFS; /** * The accessor for the root filesystem. diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index 189bd1f73..843beb4d1 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -64,7 +64,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value * * a if (rev) attrs.insert_or_assign("rev", rev->gitRev()); auto input = fetchers::Input::fromAttrs(state.fetchSettings, std::move(attrs)); - auto [storePath, input2] = input.fetchToStore(state.store); + auto [storePath, accessor, input2] = input.fetchToStore(state.store); auto attrs2 = state.buildBindings(8); state.mkStorePathString(storePath, attrs2.alloc(state.sOutPath)); diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 0be9f4bdc..4d7de1f76 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -10,6 +10,7 @@ #include "nix/util/url.hh" #include "nix/expr/value-to-json.hh" #include "nix/fetchers/fetch-to-store.hh" +#include "nix/util/mounted-source-accessor.hh" #include <nlohmann/json.hpp> @@ -204,10 +205,12 @@ static void fetchTree( throw Error("input '%s' is not allowed to use the '__final' attribute", input.to_string()); } - auto [storePath, input2] = input.fetchToStore(state.store); + auto [storePath, accessor, input2] = input.fetchToStore(state.store); state.allowPath(storePath); + state.storeFS->mount(CanonPath(state.store->printStorePath(storePath)), accessor); + emitTreeAttrs(state, storePath, input2, v, params.emptyRevFallback, false); } diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 3ae45dcf8..9693f1773 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -187,34 +187,30 @@ bool Input::contains(const Input & other) const } // FIXME: remove -std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const +std::tuple<StorePath, ref<SourceAccessor>, Input> Input::fetchToStore(ref<Store> store) const { if (!scheme) throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs())); - auto [storePath, input] = [&]() -> std::pair<StorePath, Input> { - try { - auto [accessor, result] = getAccessorUnchecked(store); + try { + auto [accessor, result] = getAccessorUnchecked(store); - auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, result.getName()); + auto storePath = nix::fetchToStore(*store, SourcePath(accessor), FetchMode::Copy, result.getName()); - auto narHash = store->queryPathInfo(storePath)->narHash; - result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true)); + auto narHash = store->queryPathInfo(storePath)->narHash; + result.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true)); - result.attrs.insert_or_assign("__final", Explicit<bool>(true)); + result.attrs.insert_or_assign("__final", Explicit<bool>(true)); - assert(result.isFinal()); + assert(result.isFinal()); - checkLocks(*this, result); + checkLocks(*this, result); - return {storePath, result}; - } catch (Error & e) { - e.addTrace({}, "while fetching the input '%s'", to_string()); - throw; - } - }(); - - return {std::move(storePath), input}; + return {std::move(storePath), accessor, result}; + } catch (Error & e) { + e.addTrace({}, "while fetching the input '%s'", to_string()); + throw; + } } void Input::checkLocks(Input specified, Input & result) diff --git a/src/libfetchers/filtering-source-accessor.cc b/src/libfetchers/filtering-source-accessor.cc index 72a3fb4eb..97f230c7e 100644 --- a/src/libfetchers/filtering-source-accessor.cc +++ b/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) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 4a916929e..8bd5a35d8 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -15,6 +15,7 @@ #include "nix/fetchers/fetch-settings.hh" #include "nix/util/json-utils.hh" #include "nix/util/archive.hh" +#include "nix/util/mounted-source-accessor.hh" #include <regex> #include <string.h> diff --git a/src/libfetchers/include/nix/fetchers/fetchers.hh b/src/libfetchers/include/nix/fetchers/fetchers.hh index 3288ecc5e..c2ae647af 100644 --- a/src/libfetchers/include/nix/fetchers/fetchers.hh +++ b/src/libfetchers/include/nix/fetchers/fetchers.hh @@ -121,7 +121,7 @@ public: * Fetch the entire input into the Nix store, returning the * location in the Nix store and the locked input. */ - std::pair<StorePath, Input> fetchToStore(ref<Store> store) const; + std::tuple<StorePath, ref<SourceAccessor>, Input> fetchToStore(ref<Store> store) const; /** * Check the locking attributes in `result` against diff --git a/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh b/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh index 2b59f03ca..1a90fe9ef 100644 --- a/src/libfetchers/include/nix/fetchers/filtering-source-accessor.hh +++ b/src/libfetchers/include/nix/fetchers/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; diff --git a/src/libflake/flake/flake.cc b/src/libflake/flake/flake.cc index 8856a03dd..dc8844302 100644 --- a/src/libflake/flake/flake.cc +++ b/src/libflake/flake/flake.cc @@ -14,6 +14,7 @@ #include "nix/store/local-fs-store.hh" #include "nix/fetchers/fetch-to-store.hh" #include "nix/util/memory-source-accessor.hh" +#include "nix/util/mounted-source-accessor.hh" #include <nlohmann/json.hpp> @@ -93,6 +94,8 @@ static StorePath copyInputToStore( state.allowPath(storePath); + 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)); diff --git a/src/libutil/include/nix/util/meson.build b/src/libutil/include/nix/util/meson.build index e30b8dacd..329d40612 100644 --- a/src/libutil/include/nix/util/meson.build +++ b/src/libutil/include/nix/util/meson.build @@ -43,6 +43,7 @@ headers = files( 'logging.hh', 'lru-cache.hh', 'memory-source-accessor.hh', + 'mounted-source-accessor.hh', 'muxable-pipe.hh', 'os-string.hh', 'pool.hh', diff --git a/src/libutil/include/nix/util/mounted-source-accessor.hh b/src/libutil/include/nix/util/mounted-source-accessor.hh new file mode 100644 index 000000000..4e75edfaf --- /dev/null +++ b/src/libutil/include/nix/util/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); + +} diff --git a/src/libutil/include/nix/util/source-accessor.hh b/src/libutil/include/nix/util/source-accessor.hh index 5ef660150..f5ec04646 100644 --- a/src/libutil/include/nix/util/source-accessor.hh +++ b/src/libutil/include/nix/util/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. diff --git a/src/libutil/mounted-source-accessor.cc b/src/libutil/mounted-source-accessor.cc index b7de2afbf..15da826d2 100644 --- a/src/libutil/mounted-source-accessor.cc +++ b/src/libutil/mounted-source-accessor.cc @@ -1,12 +1,12 @@ -#include "nix/util/source-accessor.hh" +#include "nix/util/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)); } } diff --git a/src/nix/env.cc b/src/nix/env.cc index 277bd0fdd..a0b0e976b 100644 --- a/src/nix/env.cc +++ b/src/nix/env.cc @@ -6,6 +6,7 @@ #include "run.hh" #include "nix/util/strings.hh" #include "nix/util/executable-path.hh" +#include "nix/util/mounted-source-accessor.hh" using namespace nix; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 3d174dc53..56014099b 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -1095,7 +1095,7 @@ struct CmdFlakeArchive : FlakeCommand, MixJSON, MixDryRun storePath = dryRun ? (*inputNode)->lockedRef.input.computeStorePath(*store) - : (*inputNode)->lockedRef.input.fetchToStore(store).first; + : std::get<0>((*inputNode)->lockedRef.input.fetchToStore(store)); sources.insert(*storePath); } if (json) { diff --git a/tests/functional/flakes/source-paths.sh b/tests/functional/flakes/source-paths.sh index 4709bf2fc..6cd0917aa 100644 --- a/tests/functional/flakes/source-paths.sh +++ b/tests/functional/flakes/source-paths.sh @@ -12,6 +12,8 @@ cat > "$repo/flake.nix" <<EOF { outputs = { ... }: { x = 1; + y = assert false; 1; + z = builtins.readFile ./foo; }; } EOF @@ -21,3 +23,20 @@ expectStderr 1 nix eval "$repo#x" | grepQuiet "error: Path 'flake.nix' in the re git -C "$repo" add flake.nix [[ $(nix eval "$repo#x") = 1 ]] + +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 123 > "$repo/foo" + +expectStderr 1 nix eval "$repo#z" | grepQuiet "error: Path 'foo' in the repository \"$repo\" is not tracked by Git." + +git -C "$repo" add "$repo/foo" + +[[ $(nix eval --raw "$repo#z") = 123 ]] diff --git a/tests/functional/restricted.sh b/tests/functional/restricted.sh index 00ee4ddc8..bc42ec891 100755 --- a/tests/functional/restricted.sh +++ b/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"; }' ]]