From 45fa297e4052acef962d9d124241e7abd02f58af Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 3 Feb 2023 11:21:47 -0500 Subject: [PATCH] Factor out `InstallableStorePath` to its own file, dedup `nix app` had something called `InstallableDerivedPath` which is actually the same thing. We go with the later's name because it has become more correct. I originally did this change (more hurriedly) as part of #6225 --- a mini store-only Nix and a full Nix need to share this code. In the first RFC meeting for https://github.com/NixOS/rfcs/pull/134 we discussed how some splitting of the massive `installables.cc` could begin prior, as that is a good thing anyways. (@edolstra's words, not mine!) This would be one such PR. --- src/libcmd/installable-derived-path.cc | 70 ++++++++++++++++++++++++++ src/libcmd/installable-derived-path.hh | 28 +++++++++++ src/libcmd/installables.cc | 70 ++------------------------ src/nix/app.cc | 27 +--------- 4 files changed, 104 insertions(+), 91 deletions(-) create mode 100644 src/libcmd/installable-derived-path.cc create mode 100644 src/libcmd/installable-derived-path.hh diff --git a/src/libcmd/installable-derived-path.cc b/src/libcmd/installable-derived-path.cc new file mode 100644 index 000000000..a9921b901 --- /dev/null +++ b/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 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, + 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), + }; +} + +} diff --git a/src/libcmd/installable-derived-path.hh b/src/libcmd/installable-derived-path.hh new file mode 100644 index 000000000..042878b91 --- /dev/null +++ b/src/libcmd/installable-derived-path.hh @@ -0,0 +1,28 @@ +#pragma once + +#include "installables.hh" + +namespace nix { + +struct InstallableDerivedPath : Installable +{ + ref store; + DerivedPath derivedPath; + + InstallableDerivedPath(ref store, DerivedPath && derivedPath) + : store(store), derivedPath(std::move(derivedPath)) + { } + + std::string what() const override; + + DerivedPathsWithInfo toDerivedPaths() override; + + std::optional getStorePath() override; + + static InstallableDerivedPath parse( + ref store, + std::string_view prefix, + ExtendedOutputsSpec extendedOutputsSpec); +}; + +} diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 24f458f1a..5a75ed7af 100644 --- a/src/libcmd/installables.cc +++ b/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" @@ -396,38 +397,6 @@ static StorePath getDeriver( return *derivers.begin(); } -struct InstallableStorePath : Installable -{ - ref store; - DerivedPath req; - - InstallableStorePath(ref 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 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; @@ -792,41 +761,10 @@ std::vector> SourceExprCommand::parseInstallables( auto prefix = std::move(prefix_); auto extendedOutputsSpec = std::move(extendedOutputsSpec_); - auto found = prefix.find('/'); - if (found != std::string::npos) { + if (prefix.find('/') != std::string::npos) { try { - 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()); - result.push_back(std::make_shared(store, std::move(derivedPath))); + result.push_back(std::make_shared( + InstallableDerivedPath::parse(store, prefix, extendedOutputsSpec))); continue; } catch (BadStorePath &) { } catch (...) { diff --git a/src/nix/app.cc b/src/nix/app.cc index 08cd0ccd4..5cd65136f 100644 --- a/src/nix/app.cc +++ b/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; - const DerivedPath derivedPath; - - InstallableDerivedPath(ref 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 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 evalStore, ref store) for (auto & ctxElt : unresolved.context) installableContext.push_back( - std::make_shared(store, ctxElt)); + std::make_shared(store, DerivedPath { ctxElt })); auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext); res.program = resolveString(*store, unresolved.program, builtContext);