From 87dcd090470ed6e56a2744cbe1490d2cf235d5c0 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 23 Jun 2023 12:31:09 -0400
Subject: [PATCH] Clean up `resolveSearchPathElem`

We should use `std::optional<std::string>` not `std::pair<bool,
std::string>` for an optional string.
---
 src/libexpr/eval.cc  | 14 +++++++-------
 src/libexpr/eval.hh  |  8 ++++++--
 src/libexpr/parser.y | 45 +++++++++++++++++++++++---------------------
 3 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 706a19024..97a264085 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -571,22 +571,22 @@ EvalState::EvalState(
         allowedPaths = PathSet();
 
         for (auto & i : searchPath) {
-            auto r = resolveSearchPathElem(i);
-            if (!r.first) continue;
+            auto r = resolveSearchPathElem(i.path);
+            if (!r) continue;
 
-            auto path = r.second;
+            auto path = *std::move(r);
 
-            if (store->isInStore(r.second)) {
+            if (store->isInStore(path)) {
                 try {
                     StorePathSet closure;
-                    store->computeFSClosure(store->toStorePath(r.second).first, closure);
+                    store->computeFSClosure(store->toStorePath(path).first, closure);
                     for (auto & path : closure)
                         allowPath(path);
                 } catch (InvalidPath &) {
-                    allowPath(r.second);
+                    allowPath(path);
                 }
             } else
-                allowPath(r.second);
+                allowPath(path);
         }
     }
 
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index e3676c1b7..e1a540a7f 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -317,7 +317,7 @@ private:
 
     SearchPath searchPath;
 
-    std::map<std::string, std::pair<bool, std::string>> searchPathResolved;
+    std::map<std::string, std::optional<std::string>> searchPathResolved;
 
     /**
      * Cache used by checkSourcePath().
@@ -434,9 +434,13 @@ public:
     SourcePath findFile(SearchPath & searchPath, const std::string_view path, const PosIdx pos = noPos);
 
     /**
+     * Try to resolve a search path value (not the optinal key part)
+     *
      * If the specified search path element is a URI, download it.
+     *
+     * If it is not found, return `std::nullopt`
      */
-    std::pair<bool, std::string> resolveSearchPathElem(const SearchPathElem & elem);
+    std::optional<std::string> resolveSearchPathElem(const std::string & value);
 
     /**
      * Evaluate an expression to normal form
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 0d0004f9f..f839e804c 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -772,9 +772,9 @@ SourcePath EvalState::findFile(SearchPath & searchPath, const std::string_view p
                 continue;
             suffix = path.size() == s ? "" : concatStrings("/", path.substr(s));
         }
-        auto r = resolveSearchPathElem(i);
-        if (!r.first) continue;
-        Path res = r.second + suffix;
+        auto r = resolveSearchPathElem(i.path);
+        if (!r) continue;
+        Path res = *r + suffix;
         if (pathExists(res)) return CanonPath(canonPath(res));
     }
 
@@ -791,49 +791,52 @@ SourcePath EvalState::findFile(SearchPath & searchPath, const std::string_view p
 }
 
 
-std::pair<bool, std::string> EvalState::resolveSearchPathElem(const SearchPathElem & elem)
+std::optional<std::string> EvalState::resolveSearchPathElem(const std::string & value)
 {
-    auto i = searchPathResolved.find(elem.path);
+    auto i = searchPathResolved.find(value);
     if (i != searchPathResolved.end()) return i->second;
 
-    std::pair<bool, std::string> res;
+    std::optional<std::string> res;
 
-    if (EvalSettings::isPseudoUrl(elem.path)) {
+    if (EvalSettings::isPseudoUrl(value)) {
         try {
             auto storePath = fetchers::downloadTarball(
-                store, EvalSettings::resolvePseudoUrl(elem.path), "source", false).tree.storePath;
-            res = { true, store->toRealPath(storePath) };
+                store, EvalSettings::resolvePseudoUrl(value), "source", false).tree.storePath;
+            res = { store->toRealPath(storePath) };
         } catch (FileTransferError & e) {
             logWarning({
-                .msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", elem.path)
+                .msg = hintfmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value)
             });
-            res = { false, "" };
+            res = std::nullopt;
         }
     }
 
-    else if (hasPrefix(elem.path, "flake:")) {
+    else if (hasPrefix(value, "flake:")) {
         experimentalFeatureSettings.require(Xp::Flakes);
-        auto flakeRef = parseFlakeRef(elem.path.substr(6), {}, true, false);
-        debug("fetching flake search path element '%s''", elem.path);
+        auto flakeRef = parseFlakeRef(value.substr(6), {}, true, false);
+        debug("fetching flake search path element '%s''", value);
         auto storePath = flakeRef.resolve(store).fetchTree(store).first.storePath;
-        res = { true, store->toRealPath(storePath) };
+        res = { store->toRealPath(storePath) };
     }
 
     else {
-        auto path = absPath(elem.path);
+        auto path = absPath(value);
         if (pathExists(path))
-            res = { true, path };
+            res = { path };
         else {
             logWarning({
-                .msg = hintfmt("Nix search path entry '%1%' does not exist, ignoring", elem.path)
+                .msg = hintfmt("Nix search path entry '%1%' does not exist, ignoring", value)
             });
-            res = { false, "" };
+            res = std::nullopt;
         }
     }
 
-    debug("resolved search path element '%s' to '%s'", elem.path, res.second);
+    if (res)
+        debug("resolved search path element '%s' to '%s'", value, *res);
+    else
+        debug("failed to resolve search path element '%s'", value);
 
-    searchPathResolved[elem.path] = res;
+    searchPathResolved[value] = res;
     return res;
 }