From da64f026dd7b12d72ffbc15752e8b95707fa1f9f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 10 Jan 2023 11:27:19 -0500 Subject: [PATCH] Make clear that `StorePathWithOutputs` is a deprecated type - Add a comment - Put `OutputsSpec` in a different header (First part of #6815) - Make a few stray uses of it in new code use `DerivedPath` instead. --- src/libcmd/installables.hh | 4 +- src/libexpr/flake/flakeref.hh | 2 +- src/libmain/shared.hh | 1 - src/libstore/outputs-spec.cc | 61 +++++++++++++++++++ src/libstore/outputs-spec.hh | 32 ++++++++++ src/libstore/path-with-outputs.cc | 54 ---------------- src/libstore/path-with-outputs.hh | 30 +++------ .../{path-with-outputs.cc => outputs-spec.cc} | 2 +- src/nix/app.cc | 23 +++++-- src/nix/develop.cc | 2 +- src/nix/flake.cc | 2 +- 11 files changed, 124 insertions(+), 89 deletions(-) create mode 100644 src/libstore/outputs-spec.cc create mode 100644 src/libstore/outputs-spec.hh rename src/libstore/tests/{path-with-outputs.cc => outputs-spec.cc} (97%) diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index 250cf7e83..9b92cc4be 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -2,7 +2,7 @@ #include "util.hh" #include "path.hh" -#include "path-with-outputs.hh" +#include "outputs-spec.hh" #include "derived-path.hh" #include "eval.hh" #include "store-api.hh" @@ -20,7 +20,7 @@ namespace eval_cache { class EvalCache; class AttrCursor; } struct App { - std::vector context; + std::vector context; Path program; // FIXME: add args, sandbox settings, metadata, ... }; diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh index a36d852a8..4ec79fb73 100644 --- a/src/libexpr/flake/flakeref.hh +++ b/src/libexpr/flake/flakeref.hh @@ -3,7 +3,7 @@ #include "types.hh" #include "hash.hh" #include "fetchers.hh" -#include "path-with-outputs.hh" +#include "outputs-spec.hh" #include diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index 3c37fd627..1715374a6 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -39,7 +39,6 @@ void printVersion(const std::string & programName); void printGCWarning(); class Store; -struct StorePathWithOutputs; void printMissing( ref store, diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc new file mode 100644 index 000000000..76779d193 --- /dev/null +++ b/src/libstore/outputs-spec.cc @@ -0,0 +1,61 @@ +#include "outputs-spec.hh" +#include "nlohmann/json.hpp" + +#include + +namespace nix { + +std::pair parseOutputsSpec(const std::string & s) +{ + static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))"); + + std::smatch match; + if (!std::regex_match(s, match, regex)) + return {s, DefaultOutputs()}; + + if (match[3].matched) + return {match[1], AllOutputs()}; + + return {match[1], tokenizeString(match[4].str(), ",")}; +} + +std::string printOutputsSpec(const OutputsSpec & outputsSpec) +{ + if (std::get_if(&outputsSpec)) + return ""; + + if (std::get_if(&outputsSpec)) + return "^*"; + + if (auto outputNames = std::get_if(&outputsSpec)) + return "^" + concatStringsSep(",", *outputNames); + + assert(false); +} + +void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec) +{ + if (std::get_if(&outputsSpec)) + json = nullptr; + + else if (std::get_if(&outputsSpec)) + json = std::vector({"*"}); + + else if (auto outputNames = std::get_if(&outputsSpec)) + json = *outputNames; +} + +void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec) +{ + if (json.is_null()) + outputsSpec = DefaultOutputs(); + else { + auto names = json.get(); + if (names == OutputNames({"*"})) + outputsSpec = AllOutputs(); + else + outputsSpec = names; + } +} + +} diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh new file mode 100644 index 000000000..e2cf1d12b --- /dev/null +++ b/src/libstore/outputs-spec.hh @@ -0,0 +1,32 @@ +#pragma once + +#include + +#include "util.hh" + +#include "nlohmann/json_fwd.hpp" + +namespace nix { + +typedef std::set OutputNames; + +struct AllOutputs { + bool operator < (const AllOutputs & _) const { return false; } +}; + +struct DefaultOutputs { + bool operator < (const DefaultOutputs & _) const { return false; } +}; + +typedef std::variant OutputsSpec; + +/* Parse a string of the form 'prefix^output1,...outputN' or + 'prefix^*', returning the prefix and the outputs spec. */ +std::pair parseOutputsSpec(const std::string & s); + +std::string printOutputsSpec(const OutputsSpec & outputsSpec); + +void to_json(nlohmann::json &, const OutputsSpec &); +void from_json(const nlohmann::json &, OutputsSpec &); + +} diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index d6d67ea05..17230ffc4 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -1,6 +1,5 @@ #include "path-with-outputs.hh" #include "store-api.hh" -#include "nlohmann/json.hpp" #include @@ -71,57 +70,4 @@ StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std: return StorePathWithOutputs { store.followLinksToStorePath(path), std::move(outputs) }; } -std::pair parseOutputsSpec(const std::string & s) -{ - static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))"); - - std::smatch match; - if (!std::regex_match(s, match, regex)) - return {s, DefaultOutputs()}; - - if (match[3].matched) - return {match[1], AllOutputs()}; - - return {match[1], tokenizeString(match[4].str(), ",")}; -} - -std::string printOutputsSpec(const OutputsSpec & outputsSpec) -{ - if (std::get_if(&outputsSpec)) - return ""; - - if (std::get_if(&outputsSpec)) - return "^*"; - - if (auto outputNames = std::get_if(&outputsSpec)) - return "^" + concatStringsSep(",", *outputNames); - - assert(false); -} - -void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec) -{ - if (std::get_if(&outputsSpec)) - json = nullptr; - - else if (std::get_if(&outputsSpec)) - json = std::vector({"*"}); - - else if (auto outputNames = std::get_if(&outputsSpec)) - json = *outputNames; -} - -void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec) -{ - if (json.is_null()) - outputsSpec = DefaultOutputs(); - else { - auto names = json.get(); - if (names == OutputNames({"*"})) - outputsSpec = AllOutputs(); - else - outputsSpec = names; - } -} - } diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index 0cb5eb223..ed55cc333 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -1,13 +1,18 @@ #pragma once -#include - #include "path.hh" #include "derived-path.hh" #include "nlohmann/json_fwd.hpp" namespace nix { +/* This is a deprecated old type just for use by the old CLI, and older + versions of the RPC protocols. In new code don't use it; you want + `DerivedPath` instead. + + `DerivedPath` is better because it handles more cases, and does so more + explicitly without devious punning tricks. +*/ struct StorePathWithOutputs { StorePath path; @@ -33,25 +38,4 @@ StorePathWithOutputs parsePathWithOutputs(const Store & store, std::string_view StorePathWithOutputs followLinksToStorePathWithOutputs(const Store & store, std::string_view pathWithOutputs); -typedef std::set OutputNames; - -struct AllOutputs { - bool operator < (const AllOutputs & _) const { return false; } -}; - -struct DefaultOutputs { - bool operator < (const DefaultOutputs & _) const { return false; } -}; - -typedef std::variant OutputsSpec; - -/* Parse a string of the form 'prefix^output1,...outputN' or - 'prefix^*', returning the prefix and the outputs spec. */ -std::pair parseOutputsSpec(const std::string & s); - -std::string printOutputsSpec(const OutputsSpec & outputsSpec); - -void to_json(nlohmann::json &, const OutputsSpec &); -void from_json(const nlohmann::json &, OutputsSpec &); - } diff --git a/src/libstore/tests/path-with-outputs.cc b/src/libstore/tests/outputs-spec.cc similarity index 97% rename from src/libstore/tests/path-with-outputs.cc rename to src/libstore/tests/outputs-spec.cc index 350ea7ffd..d781a930e 100644 --- a/src/libstore/tests/path-with-outputs.cc +++ b/src/libstore/tests/outputs-spec.cc @@ -1,4 +1,4 @@ -#include "path-with-outputs.hh" +#include "outputs-spec.hh" #include diff --git a/src/nix/app.cc b/src/nix/app.cc index a8d7e115b..fb149042c 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -79,9 +79,19 @@ UnresolvedApp Installable::toApp(EvalState & state) if (type == "app") { auto [program, context] = cursor->getAttr("program")->getStringWithContext(); - std::vector context2; - for (auto & [path, name] : context) - context2.push_back({path, {name}}); + std::vector context2; + for (auto & [path, name] : context) { + context2.push_back(name != "" || path.isDerivation() + ? (DerivedPath) DerivedPath::Built { + .drvPath = path, + .outputs = name != "" + ? StringSet { name } + : StringSet { }, + } + : (DerivedPath) DerivedPath::Opaque { + .path = path, + }); + } return UnresolvedApp{App { .context = std::move(context2), @@ -105,7 +115,10 @@ UnresolvedApp Installable::toApp(EvalState & state) : DrvName(name).name; auto program = outPath + "/bin/" + mainProgram; return UnresolvedApp { App { - .context = { { drvPath, {outputName} } }, + .context = { DerivedPath::Built { + .drvPath = drvPath, + .outputs = {outputName}, + } }, .program = program, }}; } @@ -123,7 +136,7 @@ App UnresolvedApp::resolve(ref evalStore, ref store) for (auto & ctxElt : unresolved.context) installableContext.push_back( - std::make_shared(store, ctxElt.toDerivedPath())); + std::make_shared(store, ctxElt)); auto builtContext = Installable::build(evalStore, store, Realise::Outputs, installableContext); res.program = resolveString(*store, unresolved.program, builtContext); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 1d90d1dac..6aa675386 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -3,7 +3,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" -#include "path-with-outputs.hh" +#include "outputs-spec.hh" #include "derivations.hh" #include "progress-bar.hh" #include "run.hh" diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 9b4cdf35a..bb020d51e 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -7,7 +7,7 @@ #include "get-drvs.hh" #include "store-api.hh" #include "derivations.hh" -#include "path-with-outputs.hh" +#include "outputs-spec.hh" #include "attr-path.hh" #include "fetchers.hh" #include "registry.hh"