Merge pull request #11913 from DeterminateSystems/source-path-in-look…

…up-path resolveLookupPathPath(): Return a SourcePath instead of a string
NixOS · Nov 19, 2024 · ce6b5de · ce6b5de
2 parents 8637f3d + af07f33
commit ce6b5de
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 24 deletions.
diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc
@@ -29,13 +29,13 @@ EvalSettings evalSettings {
     {
         {
             "flake",
-            [](ref<Store> store, std::string_view rest) {
+            [](EvalState & state, std::string_view rest) {
                 experimentalFeatureSettings.require(Xp::Flakes);
                 // FIXME `parseFlakeRef` should take a `std::string_view`.
                 auto flakeRef = parseFlakeRef(fetchSettings, std::string { rest }, {}, true, false);
                 debug("fetching flake search path element '%s''", rest);
-                auto storePath = flakeRef.resolve(store).fetchTree(store).first;
-                return store->toRealPath(storePath);
+                auto storePath = flakeRef.resolve(state.store).fetchTree(state.store).first;
+                return state.rootPath(state.store->toRealPath(storePath));
             },
         },
     },

diff --git a/src/libexpr/eval-settings.hh b/src/libexpr/eval-settings.hh
@@ -3,10 +3,11 @@
 
 #include "config.hh"
 #include "ref.hh"
+#include "source-path.hh"
 
 namespace nix {
 
-class Store;
+class EvalState;
 
 struct EvalSettings : Config
 {
@@ -18,11 +19,8 @@ struct EvalSettings : Config
      *
      * The return value is (a) whether the entry was valid, and, if so,
      * what does it map to.
-     *
-     * @todo Return (`std::optional` of) `SourceAccssor` or something
-     * more structured instead of mere `std::string`?
      */
-    using LookupPathHook = std::optional<std::string>(ref<Store> store, std::string_view);
+    using LookupPathHook = std::optional<SourcePath>(EvalState & state, std::string_view);
 
     /**
      * Map from "scheme" to a `LookupPathHook`.

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
@@ -3025,8 +3025,8 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_
         if (!rOpt) continue;
         auto r = *rOpt;
 
-        Path res = suffix == "" ? r : concatStrings(r, "/", suffix);
-        if (pathExists(res)) return rootPath(CanonPath(canonPath(res)));
+        auto res = (r / CanonPath(suffix)).resolveSymlinks();
+        if (res.pathExists()) return res;
     }
 
     if (hasPrefix(path, "nix/"))
@@ -3041,13 +3041,13 @@ SourcePath EvalState::findFile(const LookupPath & lookupPath, const std::string_
 }
 
 
-std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl)
+std::optional<SourcePath> EvalState::resolveLookupPathPath(const LookupPath::Path & value0, bool initAccessControl)
 {
     auto & value = value0.s;
     auto i = lookupPathResolved.find(value);
     if (i != lookupPathResolved.end()) return i->second;
 
-    auto finish = [&](std::string res) {
+    auto finish = [&](SourcePath res) {
         debug("resolved search path element '%s' to '%s'", value, res);
         lookupPathResolved.emplace(value, res);
         return res;
@@ -3060,7 +3060,7 @@ std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Pa
                 fetchSettings,
                 EvalSettings::resolvePseudoUrl(value));
             auto storePath = fetchToStore(*store, SourcePath(accessor), FetchMode::Copy);
-            return finish(store->toRealPath(storePath));
+            return finish(rootPath(store->toRealPath(storePath)));
         } catch (Error & e) {
             logWarning({
                 .msg = HintFmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value)
@@ -3072,29 +3072,29 @@ std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Pa
         auto scheme = value.substr(0, colPos);
         auto rest = value.substr(colPos + 1);
         if (auto * hook = get(settings.lookupPathHooks, scheme)) {
-            auto res = (*hook)(store, rest);
+            auto res = (*hook)(*this, rest);
             if (res)
                 return finish(std::move(*res));
         }
     }
 
     {
-        auto path = absPath(value);
+        auto path = rootPath(value);
 
         /* Allow access to paths in the search path. */
         if (initAccessControl) {
-            allowPath(path);
-            if (store->isInStore(path)) {
+            allowPath(path.path.abs());
+            if (store->isInStore(path.path.abs())) {
                 try {
                     StorePathSet closure;
-                    store->computeFSClosure(store->toStorePath(path).first, closure);
+                    store->computeFSClosure(store->toStorePath(path.path.abs()).first, closure);
                     for (auto & p : closure)
                         allowPath(p);
                 } catch (InvalidPath &) { }
             }
         }
 
-        if (pathExists(path))
+        if (path.pathExists())
             return finish(std::move(path));
         else {
             logWarning({
@@ -3105,7 +3105,6 @@ std::optional<std::string> EvalState::resolveLookupPathPath(const LookupPath::Pa
 
     debug("failed to resolve search path element '%s'", value);
     return std::nullopt;
-
 }
 
 

diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
@@ -347,7 +347,7 @@ private:
 
     LookupPath lookupPath;
 
-    std::map<std::string, std::optional<std::string>> lookupPathResolved;
+    std::map<std::string, std::optional<SourcePath>> lookupPathResolved;
 
     /**
      * Cache used by prim_match().
@@ -452,9 +452,9 @@ public:
      *
      * If the specified search path element is a URI, download it.
      *
-     * If it is not found, return `std::nullopt`
+     * If it is not found, return `std::nullopt`.
      */
-    std::optional<std::string> resolveLookupPathPath(
+    std::optional<SourcePath> resolveLookupPathPath(
         const LookupPath::Path & elem,
         bool initAccessControl = false);
 

diff --git 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 "was not found in the Nix search path"
 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}")