Merge pull request #7744 from obsidiansystems/split-installable-store-path

Factor out `InstallableStorePath` to its own file, dedup

src/libcmd/installable-derived-path.cc

@@ -0,0 +1,70 @@
+#include "installable-derived-path.hh"
+#include "derivations.hh"
+
+namespace nix {
+
+std::string InstallableDerivedPath::what() const
+{
+    return derivedPath.to_string(*store);
+}
+
+DerivedPathsWithInfo InstallableDerivedPath::toDerivedPaths()
+{
+    return {{.path = derivedPath, .info = {} }};
+}
+
+std::optional<StorePath> InstallableDerivedPath::getStorePath()
+{
+    return std::visit(overloaded {
+        [&](const DerivedPath::Built & bfd) {
+            return bfd.drvPath;
+        },
+        [&](const DerivedPath::Opaque & bo) {
+            return bo.path;
+        },
+    }, derivedPath.raw());
+}
+
+InstallableDerivedPath InstallableDerivedPath::parse(
+    ref<Store> store,
+    std::string_view prefix,
+    ExtendedOutputsSpec extendedOutputsSpec)
+{
+    auto derivedPath = std::visit(overloaded {
+        // If the user did not use ^, we treat the output more liberally.
+        [&](const ExtendedOutputsSpec::Default &) -> DerivedPath {
+            // First, we accept a symlink chain or an actual store path.
+            auto storePath = store->followLinksToStorePath(prefix);
+            // Second, we see if the store path ends in `.drv` to decide what sort
+            // of derived path they want.
+            //
+            // This handling predates the `^` syntax. The `^*` in
+            // `/nix/store/hash-foo.drv^*` unambiguously means "do the
+            // `DerivedPath::Built` case", so plain `/nix/store/hash-foo.drv` could
+            // also unambiguously mean "do the DerivedPath::Opaque` case".
+            //
+            // Issue #7261 tracks reconsidering this `.drv` dispatching.
+            return storePath.isDerivation()
+                ? (DerivedPath) DerivedPath::Built {
+                    .drvPath = std::move(storePath),
+                    .outputs = OutputsSpec::All {},
+                }
+                : (DerivedPath) DerivedPath::Opaque {
+                    .path = std::move(storePath),
+                };
+        },
+        // If the user did use ^, we just do exactly what is written.
+        [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> DerivedPath {
+            return DerivedPath::Built {
+                .drvPath = store->parseStorePath(prefix),
+                .outputs = outputSpec,
+            };
+        },
+    }, extendedOutputsSpec.raw());
+    return InstallableDerivedPath {
+        store,
+        std::move(derivedPath),
+    };
+}
+
+}

src/libcmd/installable-derived-path.hh

@@ -0,0 +1,28 @@
+#pragma once
+
+#include "installables.hh"
+
+namespace nix {
+
+struct InstallableDerivedPath : Installable
+{
+    ref<Store> store;
+    DerivedPath derivedPath;
+
+    InstallableDerivedPath(ref<Store> store, DerivedPath && derivedPath)
+        : store(store), derivedPath(std::move(derivedPath))
+    { }
+
+    std::string what() const override;
+
+    DerivedPathsWithInfo toDerivedPaths() override;
+
+    std::optional<StorePath> getStorePath() override;
+
+    static InstallableDerivedPath parse(
+        ref<Store> store,
+        std::string_view prefix,
+        ExtendedOutputsSpec extendedOutputsSpec);
+};
+
+}

src/libcmd/installables.cc

@@ -1,5 +1,6 @@
 #include "globals.hh"
 #include "installables.hh"
+#include "installable-derived-path.hh"
 #include "outputs-spec.hh"
 #include "util.hh"
 #include "command.hh"
@@ -389,38 +390,6 @@ static StorePath getDeriver(
     return *derivers.begin();
 }
 
-struct InstallableStorePath : Installable
-{
-    ref<Store> store;
-    DerivedPath req;
-
-    InstallableStorePath(ref<Store> store, DerivedPath && req)
-        : store(store), req(std::move(req))
-    { }
-
-    std::string what() const override
-    {
-        return req.to_string(*store);
-    }
-
-    DerivedPathsWithInfo toDerivedPaths() override
-    {
-        return {{.path = req, .info = {} }};
-    }
-
-    std::optional<StorePath> getStorePath() override
-    {
-        return std::visit(overloaded {
-            [&](const DerivedPath::Built & bfd) {
-                return bfd.drvPath;
-            },
-            [&](const DerivedPath::Opaque & bo) {
-                return bo.path;
-            },
-        }, req.raw());
-    }
-};
-
 struct InstallableAttrPath : InstallableValue
 {
     SourceExprCommand & cmd;
@@ -785,41 +754,10 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
             auto prefix = std::move(prefix_);
             auto extendedOutputsSpec = std::move(extendedOutputsSpec_);
 
-            auto found = prefix.find('/');
+            if (prefix.find('/') != std::string::npos) {
-            if (found != std::string::npos) {
                 try {
-                    auto derivedPath = std::visit(overloaded {
+                    result.push_back(std::make_shared<InstallableDerivedPath>(
-                        // If the user did not use ^, we treat the output more liberally.
+                        InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec)));
-                        [&](const ExtendedOutputsSpec::Default &) -> DerivedPath {
-                            // First, we accept a symlink chain or an actual store path.
-                            auto storePath = store->followLinksToStorePath(prefix);
-                            // Second, we see if the store path ends in `.drv` to decide what sort
-                            // of derived path they want.
-                            //
-                            // This handling predates the `^` syntax. The `^*` in
-                            // `/nix/store/hash-foo.drv^*` unambiguously means "do the
-                            // `DerivedPath::Built` case", so plain `/nix/store/hash-foo.drv` could
-                            // also unambiguously mean "do the DerivedPath::Opaque` case".
-                            //
-                            // Issue #7261 tracks reconsidering this `.drv` dispatching.
-                            return storePath.isDerivation()
-                                ? (DerivedPath) DerivedPath::Built {
-                                    .drvPath = std::move(storePath),
-                                    .outputs = OutputsSpec::All {},
-                                }
-                                : (DerivedPath) DerivedPath::Opaque {
-                                    .path = std::move(storePath),
-                                };
-                        },
-                        // If the user did use ^, we just do exactly what is written.
-                        [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> DerivedPath {
-                            return DerivedPath::Built {
-                                .drvPath = store->parseStorePath(prefix),
-                                .outputs = outputSpec,
-                            };
-                        },
-                    }, extendedOutputsSpec.raw());
-                    result.push_back(std::make_shared<InstallableStorePath>(store, std::move(derivedPath)));
                     continue;
                 } catch (BadStorePath &) {
                 } catch (...) {

src/nix/app.cc

@@ -1,4 +1,5 @@
 #include "installables.hh"
+#include "installable-derived-path.hh"
 #include "store-api.hh"
 #include "eval-inline.hh"
 #include "eval-cache.hh"
@@ -8,30 +9,6 @@
 
 namespace nix {
 
-struct InstallableDerivedPath : Installable
-{
-    ref<Store> store;
-    const DerivedPath derivedPath;
-
-    InstallableDerivedPath(ref<Store> store, const DerivedPath & derivedPath)
-        : store(store)
-        , derivedPath(derivedPath)
-    {
-    }
-
-    std::string what() const override { return derivedPath.to_string(*store); }
-
-    DerivedPathsWithInfo toDerivedPaths() override
-    {
-        return {{derivedPath}};
-    }
-
-    std::optional<StorePath> getStorePath() override
-    {
-        return std::nullopt;
-    }
-};
-
 /**
  * Return the rewrites that are needed to resolve a string whose context is
  * included in `dependencies`.
@@ -146,7 +123,7 @@ App UnresolvedApp::resolve(ref<Store> evalStore, ref<Store> store)
 
     for (auto & ctxElt : unresolved.context)
         installableContext.push_back(
-            std::make_shared<InstallableDerivedPath>(store, ctxElt));
+            std::make_shared<InstallableDerivedPath>(store, DerivedPath { ctxElt }));
 
     auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext);
     res.program = resolveString(*store, unresolved.program, builtContext);