Parse string context elements properly

Prior to this change, we had a bunch of ad-hoc string manipulation code
scattered around. This made it hard to figure out what data model for
string contexts is.

Now, we still store string contexts most of the time as encoded strings
--- I was wary of the performance implications of changing that --- but
whenever we parse them we do so only through the
`NixStringContextElem::parse` method, which handles all cases. This
creates a data type that is very similar to `DerivedPath` but:

 - Represents the funky `=<drvpath>` case as properly distinct from the
   others.

 - Only encodes a single output, no wildcards and no set, for the
   "built" case.

(I would like to deprecate `=<path>`, after which we are in spitting
distance of `DerivedPath` and could maybe get away with fewer types, but
that is another topic for another day.)

1 files changed, 51 insertions, 39 deletions

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 9ef91cbc5..a433e99f0 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -43,16 +43,32 @@ StringMap EvalState::realiseContext(const PathSet & context)
     std::vector<DerivedPath::Built> drvs;
     StringMap res;
 
-    for (auto & i : context) {
-        auto [ctx, outputName] = decodeContext(*store, i);
-        auto ctxS = store->printStorePath(ctx);
-        if (!store->isValidPath(ctx))
-            debugThrowLastTrace(InvalidPathError(store->printStorePath(ctx)));
-        if (!outputName.empty() && ctx.isDerivation()) {
-            drvs.push_back({ctx, {outputName}});
-        } else {
-            res.insert_or_assign(ctxS, ctxS);
-        }
+    for (auto & c_ : context) {
+        auto ensureValid = [&](const StorePath & p) {
+            if (!store->isValidPath(p))
+                debugThrowLastTrace(InvalidPathError(store->printStorePath(p)));
+        };
+        auto c = NixStringContextElem::parse(*store, c_);
+        std::visit(overloaded {
+            [&](const NixStringContextElem::Built & b) {
+                drvs.push_back(DerivedPath::Built {
+                    .drvPath = b.drvPath,
+                    .outputs = std::set { b.output },
+                });
+                ensureValid(b.drvPath);
+            },
+            [&](const NixStringContextElem::Opaque & o) {
+                auto ctxS = store->printStorePath(o.path);
+                res.insert_or_assign(ctxS, ctxS);
+                ensureValid(o.path);
+            },
+            [&](const NixStringContextElem::DrvDeep & d) {
+                /* Treat same as Opaque */
+                auto ctxS = store->printStorePath(d.drvPath);
+                res.insert_or_assign(ctxS, ctxS);
+                ensureValid(d.drvPath);
+            },
+        }, c.raw());
     }
 
     if (drvs.empty()) return {};
@@ -1179,35 +1195,31 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value * *
     /* Everything in the context of the strings in the derivation
        attributes should be added as dependencies of the resulting
        derivation. */
-    for (auto & path : context) {
-
-        /* Paths marked with `=' denote that the path of a derivation
-           is explicitly passed to the builder.  Since that allows the
-           builder to gain access to every path in the dependency
-           graph of the derivation (including all outputs), all paths
-           in the graph must be added to this derivation's list of
-           inputs to ensure that they are available when the builder
-           runs. */
-        if (path.at(0) == '=') {
-            /* !!! This doesn't work if readOnlyMode is set. */
-            StorePathSet refs;
-            state.store->computeFSClosure(state.store->parseStorePath(std::string_view(path).substr(1)), refs);
-            for (auto & j : refs) {
-                drv.inputSrcs.insert(j);
-                if (j.isDerivation())
-                    drv.inputDrvs[j] = state.store->readDerivation(j).outputNames();
-            }
-        }
-
-        /* Handle derivation outputs of the form ‘!<name>!<path>’. */
-        else if (path.at(0) == '!') {
-            auto ctx = decodeContext(*state.store, path);
-            drv.inputDrvs[ctx.first].insert(ctx.second);
-        }
-
-        /* Otherwise it's a source file. */
-        else
-            drv.inputSrcs.insert(state.store->parseStorePath(path));
+    for (auto & c_ : context) {
+        auto c = NixStringContextElem::parse(*state.store, c_);
+        std::visit(overloaded {
+            /* Since this allows the builder to gain access to every
+               path in the dependency graph of the derivation (including
+               all outputs), all paths in the graph must be added to
+               this derivation's list of inputs to ensure that they are
+               available when the builder runs. */
+            [&](const NixStringContextElem::DrvDeep & d) {
+                /* !!! This doesn't work if readOnlyMode is set. */
+                StorePathSet refs;
+                state.store->computeFSClosure(d.drvPath, refs);
+                for (auto & j : refs) {
+                    drv.inputSrcs.insert(j);
+                    if (j.isDerivation())
+                        drv.inputDrvs[j] = state.store->readDerivation(j).outputNames();
+                }
+            },
+            [&](const NixStringContextElem::Built & b) {
+                drv.inputDrvs[b.drvPath].insert(b.output);
+            },
+            [&](const NixStringContextElem::Opaque & o) {
+                drv.inputSrcs.insert(o.path);
+            },
+        }, c.raw());
     }
 
     /* Do we have all required attributes? */