diff options
author | Robert Hensing <roberth@users.noreply.github.com> | 2023-01-13 16:03:12 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-13 16:03:12 +0100 |
commit | d21f54958ebc3400ca41e0458338e6a90bd47fbe (patch) | |
tree | a4ea90b2cc1be9209e0707e1179e1a8064b1ab74 /src/libstore | |
parent | bdeb6de889219cb9d1ba94b4adc75b0d8000e1b2 (diff) | |
parent | d8512653d480acf69aae820f8b9d4b674dd6fc2f (diff) |
Merge pull request #6815 from obsidiansystems/better-wanted-outputs
`OutputSpec` for `DerivationGoal` and `DerivedPath`, today's `OutputSpec` -> `ExtendedOutputSpec`
Diffstat (limited to 'src/libstore')
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 48 | ||||
-rw-r--r-- | src/libstore/build/derivation-goal.hh | 9 | ||||
-rw-r--r-- | src/libstore/build/entry-points.cc | 10 | ||||
-rw-r--r-- | src/libstore/build/local-derivation-goal.cc | 2 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 6 | ||||
-rw-r--r-- | src/libstore/build/worker.hh | 6 | ||||
-rw-r--r-- | src/libstore/derivations.cc | 6 | ||||
-rw-r--r-- | src/libstore/derivations.hh | 2 | ||||
-rw-r--r-- | src/libstore/derived-path.cc | 25 | ||||
-rw-r--r-- | src/libstore/derived-path.hh | 3 | ||||
-rw-r--r-- | src/libstore/legacy-ssh-store.cc | 7 | ||||
-rw-r--r-- | src/libstore/misc.cc | 45 | ||||
-rw-r--r-- | src/libstore/outputs-spec.cc | 186 | ||||
-rw-r--r-- | src/libstore/outputs-spec.hh | 93 | ||||
-rw-r--r-- | src/libstore/path-with-outputs.cc | 23 | ||||
-rw-r--r-- | src/libstore/path-with-outputs.hh | 1 | ||||
-rw-r--r-- | src/libstore/path.hh | 1 | ||||
-rw-r--r-- | src/libstore/remote-store.cc | 18 | ||||
-rw-r--r-- | src/libstore/store-api.hh | 10 | ||||
-rw-r--r-- | src/libstore/tests/outputs-spec.cc | 201 |
20 files changed, 552 insertions, 150 deletions
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 5e86b5269..2021d0023 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -63,7 +63,7 @@ namespace nix { DerivationGoal::DerivationGoal(const StorePath & drvPath, - const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(true) , drvPath(drvPath) @@ -82,7 +82,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(false) , drvPath(drvPath) @@ -142,18 +142,12 @@ void DerivationGoal::work() (this->*state)(); } -void DerivationGoal::addWantedOutputs(const StringSet & outputs) +void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) { - /* If we already want all outputs, there is nothing to do. */ - if (wantedOutputs.empty()) return; - - if (outputs.empty()) { - wantedOutputs.clear(); + auto newWanted = wantedOutputs.union_(outputs); + if (!newWanted.isSubsetOf(wantedOutputs)) needRestart = true; - } else - for (auto & i : outputs) - if (wantedOutputs.insert(i).second) - needRestart = true; + wantedOutputs = newWanted; } @@ -390,7 +384,7 @@ void DerivationGoal::repairClosure() auto outputs = queryDerivationOutputMap(); StorePathSet outputClosure; for (auto & i : outputs) { - if (!wantOutput(i.first, wantedOutputs)) continue; + if (!wantedOutputs.contains(i.first)) continue; worker.store.computeFSClosure(i.second, outputClosure); } @@ -422,7 +416,7 @@ void DerivationGoal::repairClosure() if (drvPath2 == outputsToDrv.end()) addWaitee(upcast_goal(worker.makePathSubstitutionGoal(i, Repair))); else - addWaitee(worker.makeDerivationGoal(drvPath2->second, StringSet(), bmRepair)); + addWaitee(worker.makeDerivationGoal(drvPath2->second, OutputsSpec::All(), bmRepair)); } if (waitees.empty()) { @@ -991,10 +985,15 @@ void DerivationGoal::resolvedFinished() StorePathSet outputPaths; - // `wantedOutputs` might be empty, which means “all the outputs” - auto realWantedOutputs = wantedOutputs; - if (realWantedOutputs.empty()) - realWantedOutputs = resolvedDrv.outputNames(); + // `wantedOutputs` might merely indicate “all the outputs” + auto realWantedOutputs = std::visit(overloaded { + [&](const OutputsSpec::All &) { + return resolvedDrv.outputNames(); + }, + [&](const OutputsSpec::Names & names) { + return static_cast<std::set<std::string>>(names); + }, + }, wantedOutputs.raw()); for (auto & wantedOutput : realWantedOutputs) { auto initialOutput = get(initialOutputs, wantedOutput); @@ -1322,7 +1321,14 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() if (!drv->type().isPure()) return { false, {} }; bool checkHash = buildMode == bmRepair; - auto wantedOutputsLeft = wantedOutputs; + auto wantedOutputsLeft = std::visit(overloaded { + [&](const OutputsSpec::All &) { + return StringSet {}; + }, + [&](const OutputsSpec::Names & names) { + return static_cast<StringSet>(names); + }, + }, wantedOutputs.raw()); DrvOutputs validOutputs; for (auto & i : queryPartialDerivationOutputMap()) { @@ -1331,7 +1337,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() // this is an invalid output, gets catched with (!wantedOutputsLeft.empty()) continue; auto & info = *initialOutput; - info.wanted = wantOutput(i.first, wantedOutputs); + info.wanted = wantedOutputs.contains(i.first); if (info.wanted) wantedOutputsLeft.erase(i.first); if (i.second) { @@ -1369,7 +1375,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity() validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); } - // If we requested all the outputs via the empty set, we are always fine. + // If we requested all the outputs, we are always fine. // If we requested specific elements, the loop above removes all the valid // ones, so any that are left must be invalid. if (!wantedOutputsLeft.empty()) diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index d33e04cbc..707e38b4b 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -2,6 +2,7 @@ #include "parsed-derivations.hh" #include "lock.hh" +#include "outputs-spec.hh" #include "store-api.hh" #include "pathlocks.hh" #include "goal.hh" @@ -55,7 +56,7 @@ struct DerivationGoal : public Goal /* The specific outputs that we need to build. Empty means all of them. */ - StringSet wantedOutputs; + OutputsSpec wantedOutputs; /* Mapping from input derivations + output names to actual store paths. This is filled in by waiteeDone() as each dependency @@ -128,10 +129,10 @@ struct DerivationGoal : public Goal std::string machineName; DerivationGoal(const StorePath & drvPath, - const StringSet & wantedOutputs, Worker & worker, + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const StringSet & wantedOutputs, Worker & worker, + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode = bmNormal); virtual ~DerivationGoal(); @@ -142,7 +143,7 @@ struct DerivationGoal : public Goal void work() override; /* Add wanted outputs to an already existing derivation goal. */ - void addWantedOutputs(const StringSet & outputs); + void addWantedOutputs(const OutputsSpec & outputs); /* The states. */ void getDerivation(); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index e1b80165e..2925fe3ca 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -80,7 +80,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { Worker worker(*this, *this); - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); + auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode); try { worker.run(Goals{goal}); @@ -89,7 +89,10 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat return BuildResult { .status = BuildResult::MiscFailure, .errorMsg = e.msg(), - .path = DerivedPath::Built { .drvPath = drvPath }, + .path = DerivedPath::Built { + .drvPath = drvPath, + .outputs = OutputsSpec::All { }, + }, }; }; } @@ -130,7 +133,8 @@ void LocalStore::repairPath(const StorePath & path) auto info = queryPathInfo(path); if (info->deriver && isValidPath(*info->deriver)) { goals.clear(); - goals.insert(worker.makeDerivationGoal(*info->deriver, StringSet(), bmRepair)); + // FIXME: Should just build the specific output we need. + goals.insert(worker.makeDerivationGoal(*info->deriver, OutputsSpec::All { }, bmRepair)); worker.run(goals); } else throw Error(worker.exitStatus(), "cannot repair path '%s'", printStorePath(path)); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 752f9a4f3..9ab9b17fe 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2735,7 +2735,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - if (wantOutput(outputName, wantedOutputs)) + if (wantedOutputs.contains(outputName)) builtOutputs.emplace(thisRealisation.id, thisRealisation); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index b192fbc77..b94fb8416 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -42,7 +42,7 @@ Worker::~Worker() std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon( const StorePath & drvPath, - const StringSet & wantedOutputs, + const OutputsSpec & wantedOutputs, std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal) { std::weak_ptr<DerivationGoal> & goal_weak = derivationGoals[drvPath]; @@ -59,7 +59,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon( std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath, - const StringSet & wantedOutputs, BuildMode buildMode) + const OutputsSpec & wantedOutputs, BuildMode buildMode) { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { return !dynamic_cast<LocalStore *>(&store) @@ -70,7 +70,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, const StringSet & wantedOutputs, BuildMode buildMode) + const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode) { return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { return !dynamic_cast<LocalStore *>(&store) diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index a1e036a96..6d68d3cf1 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -140,15 +140,15 @@ public: /* derivation goal */ private: std::shared_ptr<DerivationGoal> makeDerivationGoalCommon( - const StorePath & drvPath, const StringSet & wantedOutputs, + const StorePath & drvPath, const OutputsSpec & wantedOutputs, std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal); public: std::shared_ptr<DerivationGoal> makeDerivationGoal( const StorePath & drvPath, - const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr<DerivationGoal> makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, - const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); /* substitution goal */ std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 42a53912e..cf18724ef 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -688,12 +688,6 @@ std::map<std::string, Hash> staticOutputHashes(Store & store, const Derivation & } -bool wantOutput(const std::string & output, const std::set<std::string> & wanted) -{ - return wanted.empty() || wanted.find(output) != wanted.end(); -} - - static DerivationOutput readDerivationOutput(Source & in, const Store & store) { const auto pathS = readString(in); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index f3cd87fb1..7ee3ded6a 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -294,8 +294,6 @@ typedef std::map<StorePath, DrvHash> DrvHashes; // FIXME: global, though at least thread-safe. extern Sync<DrvHashes> drvHashes; -bool wantOutput(const std::string & output, const std::set<std::string> & wanted); - struct Source; struct Sink; diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 3fa5ae4f7..e0d86a42f 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -19,11 +19,11 @@ nlohmann::json DerivedPath::Built::toJSON(ref<Store> store) const { res["drvPath"] = store->printStorePath(drvPath); // Fallback for the input-addressed derivation case: We expect to always be // able to print the output paths, so let’s do it - const auto knownOutputs = store->queryPartialDerivationOutputMap(drvPath); - for (const auto & output : outputs) { - auto knownOutput = get(knownOutputs, output); - if (knownOutput && *knownOutput) - res["outputs"][output] = store->printStorePath(**knownOutput); + const auto outputMap = store->queryPartialDerivationOutputMap(drvPath); + for (const auto & [output, outputPathOpt] : outputMap) { + if (!outputs.contains(output)) continue; + if (outputPathOpt) + res["outputs"][output] = store->printStorePath(*outputPathOpt); else res["outputs"][output] = nullptr; } @@ -63,7 +63,7 @@ std::string DerivedPath::Built::to_string(const Store & store) const { return store.printStorePath(drvPath) + "!" - + (outputs.empty() ? std::string { "*" } : concatStringsSep(",", outputs)); + + outputs.to_string(); } std::string DerivedPath::to_string(const Store & store) const @@ -81,15 +81,10 @@ DerivedPath::Opaque DerivedPath::Opaque::parse(const Store & store, std::string_ DerivedPath::Built DerivedPath::Built::parse(const Store & store, std::string_view drvS, std::string_view outputsS) { - auto drvPath = store.parseStorePath(drvS); - std::set<std::string> outputs; - if (outputsS != "*") { - outputs = tokenizeString<std::set<std::string>>(outputsS, ","); - if (outputs.empty()) - throw Error( - "Explicit list of wanted outputs '%s' must not be empty. Consider using '*' as a wildcard meaning all outputs if no output in particular is wanted.", outputsS); - } - return {drvPath, outputs}; + return { + .drvPath = store.parseStorePath(drvS), + .outputs = OutputsSpec::parse(outputsS), + }; } DerivedPath DerivedPath::parse(const Store & store, std::string_view s) diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index 706e5dcb4..4edff7467 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -3,6 +3,7 @@ #include "util.hh" #include "path.hh" #include "realisation.hh" +#include "outputs-spec.hh" #include <optional> @@ -44,7 +45,7 @@ struct DerivedPathOpaque { */ struct DerivedPathBuilt { StorePath drvPath; - std::set<std::string> outputs; + OutputsSpec outputs; std::string to_string(const Store & store) const; static DerivedPathBuilt parse(const Store & store, std::string_view, std::string_view); diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 4d398b21d..e1a4e13a3 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -279,7 +279,12 @@ public: conn->to.flush(); - BuildResult status { .path = DerivedPath::Built { .drvPath = drvPath } }; + BuildResult status { + .path = DerivedPath::Built { + .drvPath = drvPath, + .outputs = OutputsSpec::All { }, + }, + }; status.status = (BuildResult::Status) readInt(conn->from); conn->from >> status.errorMsg; diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index fb985c97b..5758c3d93 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -185,7 +185,7 @@ void Store::queryMissing(const std::vector<DerivedPath> & targets, knownOutputPaths = false; break; } - if (wantOutput(outputName, bfd.outputs) && !isValidPath(*pathOpt)) + if (bfd.outputs.contains(outputName) && !isValidPath(*pathOpt)) invalid.insert(*pathOpt); } if (knownOutputPaths && invalid.empty()) return; @@ -301,4 +301,47 @@ std::map<DrvOutput, StorePath> drvOutputReferences( return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); } +OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalStore_) +{ + auto & evalStore = evalStore_ ? *evalStore_ : store; + + OutputPathMap outputs; + auto drv = evalStore.readDerivation(bfd.drvPath); + auto outputHashes = staticOutputHashes(store, drv); + auto drvOutputs = drv.outputsAndOptPaths(store); + auto outputNames = std::visit(overloaded { + [&](const OutputsSpec::All &) { + StringSet names; + for (auto & [outputName, _] : drv.outputs) + names.insert(outputName); + return names; + }, + [&](const OutputsSpec::Names & names) { + return static_cast<std::set<std::string>>(names); + }, + }, bfd.outputs); + for (auto & output : outputNames) { + auto outputHash = get(outputHashes, output); + if (!outputHash) + throw Error( + "the derivation '%s' doesn't have an output named '%s'", + store.printStorePath(bfd.drvPath), output); + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + DrvOutput outputId { *outputHash, output }; + auto realisation = store.queryRealisation(outputId); + if (!realisation) + throw MissingRealisation(outputId); + outputs.insert_or_assign(output, realisation->outPath); + } else { + // If ca-derivations isn't enabled, assume that + // the output path is statically known. + auto drvOutput = get(drvOutputs, output); + assert(drvOutput); + assert(drvOutput->second); + outputs.insert_or_assign(output, *drvOutput->second); + } + } + return outputs; +} + } diff --git a/src/libstore/outputs-spec.cc b/src/libstore/outputs-spec.cc index 76779d193..d0f39a854 100644 --- a/src/libstore/outputs-spec.cc +++ b/src/libstore/outputs-spec.cc @@ -1,3 +1,4 @@ +#include "util.hh" #include "outputs-spec.hh" #include "nlohmann/json.hpp" @@ -5,57 +6,184 @@ namespace nix { -std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s) +bool OutputsSpec::contains(const std::string & outputName) const { - static std::regex regex(R"((.*)\^((\*)|([a-z]+(,[a-z]+)*)))"); + return std::visit(overloaded { + [&](const OutputsSpec::All &) { + return true; + }, + [&](const OutputsSpec::Names & outputNames) { + return outputNames.count(outputName) > 0; + }, + }, raw()); +} + + +std::optional<OutputsSpec> OutputsSpec::parseOpt(std::string_view s) +{ + static std::regex regex(R"((\*)|([a-z]+(,[a-z]+)*))"); std::smatch match; - if (!std::regex_match(s, match, regex)) - return {s, DefaultOutputs()}; + std::string s2 { s }; // until some improves std::regex + if (!std::regex_match(s2, match, regex)) + return std::nullopt; + + if (match[1].matched) + return { OutputsSpec::All {} }; + + if (match[2].matched) + return OutputsSpec::Names { tokenizeString<StringSet>(match[2].str(), ",") }; + + assert(false); +} - if (match[3].matched) - return {match[1], AllOutputs()}; - return {match[1], tokenizeString<OutputNames>(match[4].str(), ",")}; +OutputsSpec OutputsSpec::parse(std::string_view s) +{ + std::optional spec = parseOpt(s); + if (!spec) + throw Error("Invalid outputs specifier: '%s'", s); + return *spec; } -std::string printOutputsSpec(const OutputsSpec & outputsSpec) + +std::optional<std::pair<std::string_view, ExtendedOutputsSpec>> ExtendedOutputsSpec::parseOpt(std::string_view s) { - if (std::get_if<DefaultOutputs>(&outputsSpec)) - return ""; + auto found = s.rfind('^'); - if (std::get_if<AllOutputs>(&outputsSpec)) - return "^*"; + if (found == std::string::npos) + return std::pair { s, ExtendedOutputsSpec::Default {} }; - if (auto outputNames = std::get_if<OutputNames>(&outputsSpec)) - return "^" + concatStringsSep(",", *outputNames); + auto specOpt = OutputsSpec::parseOpt(s.substr(found + 1)); + if (!specOpt) + return std::nullopt; + return std::pair { s.substr(0, found), ExtendedOutputsSpec::Explicit { *std::move(specOpt) } }; +} - assert(false); + +std::pair<std::string_view, ExtendedOutputsSpec> ExtendedOutputsSpec::parse(std::string_view s) +{ + std::optional spec = parseOpt(s); + if (!spec) + throw Error("Invalid extended outputs specifier: '%s'", s); + return *spec; +} + + +std::string OutputsSpec::to_string() const +{ + return std::visit(overloaded { + [&](const OutputsSpec::All &) -> std::string { + return "*"; + }, + [&](const OutputsSpec::Names & outputNames) -> std::string { + return concatStringsSep(",", outputNames); + }, + }, raw()); } -void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec) + +std::string ExtendedOutputsSpec::to_string() const { - if (std::get_if<DefaultOutputs>(&outputsSpec)) - json = nullptr; + return std::visit(overloaded { + [&](const ExtendedOutputsSpec::Default &) -> std::string { + return ""; + }, + [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> std::string { + return "^" + outputSpec.to_string(); + }, + }, raw()); +} - else if (std::get_if<AllOutputs>(&outputsSpec)) - json = std::vector<std::string>({"*"}); - else if (auto outputNames = std::get_if<OutputNames>(&outputsSpec)) - json = *outputNames; +OutputsSpec OutputsSpec::union_(const OutputsSpec & that) const +{ + return std::visit(overloaded { + [&](const OutputsSpec::All &) -> OutputsSpec { + return OutputsSpec::All { }; + }, + [&](const OutputsSpec::Names & theseNames) -> OutputsSpec { + return std::visit(overloaded { + [&](const OutputsSpec::All &) -> OutputsSpec { + return OutputsSpec::All {}; + }, + [&](const OutputsSpec::Names & thoseNames) -> OutputsSpec { + OutputsSpec::Names ret = theseNames; + ret.insert(thoseNames.begin(), thoseNames.end()); + return ret; + }, + }, that.raw()); + }, + }, raw()); } -void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec) + +bool OutputsSpec::isSubsetOf(const OutputsSpec & that) const { + return std::visit(overloaded { + [&](const OutputsSpec::All &) { + return true; + }, + [&](const OutputsSpec::Names & thoseNames) { + return std::visit(overloaded { + [&](const OutputsSpec::All &) { + return false; + }, + [&](const OutputsSpec::Names & theseNames) { + bool ret = true; + for (auto & o : theseNames) + if (thoseNames.count(o) == 0) + ret = false; + return ret; + }, + }, raw()); + }, + }, that.raw()); +} + +} + +namespace nlohmann { + +using namespace nix; + +OutputsSpec adl_serializer<OutputsSpec>::from_json(const json & json) { + auto names = json.get<StringSet>(); + if (names == StringSet({"*"})) + return OutputsSpec::All {}; + else + return OutputsSpec::Names { std::move(names) }; +} + +void adl_serializer<OutputsSpec>::to_json(json & json, OutputsSpec t) { + std::visit(overloaded { + [&](const OutputsSpec::All &) { + json = std::vector<std::string>({"*"}); + }, + [&](const OutputsSpec::Names & names) { + json = names; + }, + }, t); +} + + +ExtendedOutputsSpec adl_serializer<ExtendedOutputsSpec>::from_json(const json & json) { if (json.is_null()) - outputsSpec = DefaultOutputs(); + return ExtendedOutputsSpec::Default {}; else { - auto names = json.get<OutputNames>(); - if (names == OutputNames({"*"})) - outputsSpec = AllOutputs(); - else - outputsSpec = names; + return ExtendedOutputsSpec::Explicit { json.get<OutputsSpec>() }; } } +void adl_serializer<ExtendedOutputsSpec>::to_json(json & json, ExtendedOutputsSpec t) { + std::visit(overloaded { + [&](const ExtendedOutputsSpec::Default &) { + json = nullptr; + }, + [&](const ExtendedOutputsSpec::Explicit & e) { + adl_serializer<OutputsSpec>::to_json(json, e); + }, + }, t); +} + } diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh index e2cf1d12b..46bc35ebc 100644 --- a/src/libstore/outputs-spec.hh +++ b/src/libstore/outputs-spec.hh @@ -1,32 +1,95 @@ #pragma once +#include <cassert> +#include <optional> +#include <set> #include <variant> -#include "util.hh" - -#include "nlohmann/json_fwd.hpp" +#include "json-impls.hh" namespace nix { -typedef std::set<std::string> OutputNames; +struct OutputNames : std::set<std::string> { + using std::set<std::string>::set; + + /* These need to be "inherited manually" */ + + OutputNames(const std::set<std::string> & s) + : std::set<std::string>(s) + { assert(!empty()); } -struct AllOutputs { - bool operator < (const AllOutputs & _) const { return false; } + OutputNames(std::set<std::string> && s) + : std::set<std::string>(s) + { assert(!empty()); } + + /* This set should always be non-empty, so we delete this + constructor in order make creating empty ones by mistake harder. + */ + OutputNames() = delete; }; -struct DefaultOutputs { - bool operator < (const DefaultOutputs & _) const { return false; } +struct AllOutputs : std::monostate { }; + +typedef std::variant<AllOutputs, OutputNames> _OutputsSpecRaw; + +struct OutputsSpec : _OutputsSpecRaw { + using Raw = _OutputsSpecRaw; + using Raw::Raw; + + /* Force choosing a variant */ + OutputsSpec() = delete; + + using Names = OutputNames; + using All = AllOutputs; + + inline const Raw & raw() const { + return static_cast<const Raw &>(*this); + } + + inline Raw & raw() { + return static_cast<Raw &>(*this); + } + + bool contains(const std::string & output) const; + + /* Create a new OutputsSpec which is the union of this and that. */ + OutputsSpec union_(const OutputsSpec & that) const; + + /* Whether this OutputsSpec is a subset of that. */ + bool isSubsetOf(const OutputsSpec & outputs) const; + + /* Parse a string of the form 'output1,...outputN' or + '*', returning the outputs spec. */ + static OutputsSpec parse(std::string_view s); + static std::optional<OutputsSpec> parseOpt(std::string_view s); + + std::string to_string() const; }; -typedef std::variant<DefaultOutputs, AllOutputs, OutputNames> OutputsSpec; +struct DefaultOutputs : std::monostate { }; -/* Parse a string of the form 'prefix^output1,...outputN' or - 'prefix^*', returning the prefix and the outputs spec. */ -std::pair<std::string, OutputsSpec> parseOutputsSpec(const std::string & s); +typedef std::variant<DefaultOutputs, OutputsSpec> _ExtendedOutputsSpecRaw; -std::string printOutputsSpec(const OutputsSpec & outputsSpec); +struct ExtendedOutputsSpec : _ExtendedOutputsSpecRaw { + using Raw = _ExtendedOutputsSpecRaw; + using Raw::Raw; -void to_json(nlohmann::json &, const OutputsSpec &); -void from_json(const nlohmann::json &, OutputsSpec &); + using Default = DefaultOutputs; + using Explicit = OutputsSpec; + + inline const Raw & raw() const { + return static_cast<const Raw &>(*this); + } + + /* Parse a string of the form 'prefix^output1,...outputN' or + 'prefix^*', returning the prefix and the extended outputs spec. */ + static std::pair<std::string_view, ExtendedOutputsSpec> parse(std::string_view s); + static std::optional<std::pair<std::string_view, ExtendedOutputsSpec>> parseOpt(std::string_view s); + + std::string to_string() const; +}; } + +JSON_IMPL(OutputsSpec) +JSON_IMPL(ExtendedOutputsSpec) diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index eae5553c5..869b490ad 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -15,10 +15,14 @@ std::string StorePathWithOutputs::to_string(const Store & store) const DerivedPath StorePathWithOutputs::toDerivedPath() const { - if (!outputs.empty() || path.isDerivation()) - return DerivedPath::Built { path, outputs }; - else + if (!outputs.empty()) { + return DerivedPath::Built { path, OutputsSpec::Names { outputs } }; + } else if (path.isDerivation()) { + assert(outputs.empty()); + return DerivedPath::Built { path, OutputsSpec::All { } }; + } else { return DerivedPath::Opaque { path }; + } } @@ -41,7 +45,18 @@ std::variant<StorePathWithOutputs, StorePath> StorePathWithOutputs::tryFromDeriv return StorePathWithOutputs { bo.path }; }, [&](const DerivedPath::Built & bfd) -> std::variant<StorePathWithOutputs, StorePath> { - return StorePathWithOutputs { bfd.drvPath, bfd.outputs }; + return StorePathWithOutputs { + .path = bfd.drvPath, + // Use legacy encoding of wildcard as empty set + .outputs = std::visit(overloaded { + [&](const OutputsSpec::All &) -> StringSet { + return {}; + }, + [&](const OutputsSpec::Names & outputs) { + return static_cast<StringSet>(outputs); + }, + }, bfd.outputs.raw()), + }; }, }, p.raw()); } diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index ed55cc333..5d25656a5 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -2,7 +2,6 @@ #include "path.hh" #include "derived-path.hh" -#include "nlohmann/json_fwd.hpp" namespace nix { diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 77fd0f8dc..0694b4c18 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -64,7 +64,6 @@ public: typedef std::set<StorePath> StorePathSet; typedef std::vector<StorePath> StorePaths; -typedef std::map<std::string, StorePath> OutputPathMap; typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index ccf7d7e8b..ff57a77ca 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -867,8 +867,8 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults( OutputPathMap outputs; auto drv = evalStore->readDerivation(bfd.drvPath); const auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive - const auto drvOutputs = drv.outputsAndOptPaths(*this); - for (auto & output : bfd.outputs) { + auto built = resolveDerivedPath(*this, bfd, &*evalStore); + for (auto & [output, outputPath] : built) { auto outputHash = get(outputHashes, output); if (!outputHash) throw Error( @@ -882,16 +882,11 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults( throw MissingRealisation(outputId); res.builtOutputs.emplace(realisation->id, *realisation); } else { - // If ca-derivations isn't enabled, assume that - // the output path is statically known. - const auto drvOutput = get(drvOutputs, output); - assert(drvOutput); - assert(drvOutput->second); res.builtOutputs.emplace( outputId, Realisation { .id = outputId, - .outPath = *drvOutput->second, + .outPath = outputPath, }); } } @@ -915,7 +910,12 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD writeDerivation(conn->to, *this, drv); conn->to << buildMode; conn.processStderr(); - BuildResult res { .path = DerivedPath::Built { .drvPath = drvPath } }; + BuildResult res { + .path = DerivedPath::Built { + .drvPath = drvPath, + .outputs = OutputsSpec::All { }, + }, + }; res.status = (BuildResult::Status) readInt(conn->from); conn->from >> res.errorMsg; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) { diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a1c499249..9eab4b4e5 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -71,6 +71,9 @@ class NarInfoDiskCache; class Store; +typedef std::map<std::string, StorePath> OutputPathMap; + + enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true }; enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true }; enum AllowInvalidFlag : bool { DisallowInvalid = false, AllowInvalid = true }; @@ -120,6 +123,8 @@ public: typedef std::map<std::string, std::string> Params; + + protected: struct PathInfoCacheValue { @@ -726,6 +731,11 @@ void copyClosure( void removeTempRoots(); +/* Resolve the derived path completely, failing if any derivation output + is unknown. */ +OutputPathMap resolveDerivedPath(Store &, const DerivedPath::Built &, Store * evalStore = nullptr); + + /* Return a Store object to access the Nix store denoted by ‘uri’ (slight misnomer...). Supported values are: diff --git a/src/libstore/tests/outputs-spec.cc b/src/libstore/tests/outputs-spec.cc index d781a930e..c9c2cafd0 100644 --- a/src/libstore/tests/outputs-spec.cc +++ b/src/libstore/tests/outputs-spec.cc @@ -1,46 +1,187 @@ #include "outputs-spec.hh" +#include <nlohmann/json.hpp> #include <gtest/gtest.h> namespace nix { -TEST(parseOutputsSpec, basic) -{ - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get_if<DefaultOutputs>(&outputsSpec)); - } +#ifndef NDEBUG +TEST(OutputsSpec, no_empty_names) { + ASSERT_DEATH(OutputsSpec::Names { std::set<std::string> { } }, ""); +} +#endif - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo^*"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get_if<AllOutputs>(&outputsSpec)); +#define TEST_DONT_PARSE(NAME, STR) \ + TEST(OutputsSpec, bad_ ## NAME) { \ + std::optional OutputsSpecOpt = \ + OutputsSpec::parseOpt(STR); \ + ASSERT_FALSE(OutputsSpecOpt); \ } - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo^out"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get<OutputNames>(outputsSpec) == OutputNames({"out"})); - } +TEST_DONT_PARSE(empty, "") +TEST_DONT_PARSE(garbage, "&*()") +TEST_DONT_PARSE(double_star, "**") +TEST_DONT_PARSE(star_first, "*,foo") +TEST_DONT_PARSE(star_second, "foo,*") - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo^out,bin"); - ASSERT_EQ(prefix, "foo"); - ASSERT_TRUE(std::get<OutputNames>(outputsSpec) == OutputNames({"out", "bin"})); - } +#undef TEST_DONT_PARSE - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo^bar^out,bin"); - ASSERT_EQ(prefix, "foo^bar"); - ASSERT_TRUE(std::get<OutputNames>(outputsSpec) == OutputNames({"out", "bin"})); - } +TEST(OutputsSpec, all) { + std::string_view str = "*"; + OutputsSpec expected = OutputsSpec::All { }; + ASSERT_EQ(OutputsSpec::parse(str), expected); + ASSERT_EQ(expected.to_string(), str); +} + +TEST(OutputsSpec, names_out) { + std::string_view str = "out"; + OutputsSpec expected = OutputsSpec::Names { "out" }; + ASSERT_EQ(OutputsSpec::parse(str), expected); + ASSERT_EQ(expected.to_string(), str); +} + +TEST(OutputsSpec, names_out_bin) { + OutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(OutputsSpec::parse("out,bin"), expected); + // N.B. This normalization is OK. + ASSERT_EQ(expected.to_string(), "bin,out"); +} + +#define TEST_SUBSET(X, THIS, THAT) \ + X((OutputsSpec { THIS }).isSubsetOf(THAT)); + +TEST(OutputsSpec, subsets_all_all) { + TEST_SUBSET(ASSERT_TRUE, OutputsSpec::All { }, OutputsSpec::All { }); +} - { - auto [prefix, outputsSpec] = parseOutputsSpec("foo^&*()"); - ASSERT_EQ(prefix, "foo^&*()"); - ASSERT_TRUE(std::get_if<DefaultOutputs>(&outputsSpec)); +TEST(OutputsSpec, subsets_names_all) { + TEST_SUBSET(ASSERT_TRUE, OutputsSpec::Names { "a" }, OutputsSpec::All { }); +} + +TEST(OutputsSpec, subsets_names_names_eq) { + TEST_SUBSET(ASSERT_TRUE, OutputsSpec::Names { "a" }, OutputsSpec::Names { "a" }); +} + +TEST(OutputsSpec, subsets_names_names_noneq) { + TEST_SUBSET(ASSERT_TRUE, OutputsSpec::Names { "a" }, (OutputsSpec::Names { "a", "b" })); +} + +TEST(OutputsSpec, not_subsets_all_names) { + TEST_SUBSET(ASSERT_FALSE, OutputsSpec::All { }, OutputsSpec::Names { "a" }); +} + +TEST(OutputsSpec, not_subsets_names_names) { + TEST_SUBSET(ASSERT_FALSE, (OutputsSpec::Names { "a", "b" }), (OutputsSpec::Names { "a" })); +} + +#undef TEST_SUBSET + +#define TEST_UNION(RES, THIS, THAT) \ + ASSERT_EQ(OutputsSpec { RES }, (OutputsSpec { THIS }).union_(THAT)); + +TEST(OutputsSpec, union_all_all) { + TEST_UNION(OutputsSpec::All { }, OutputsSpec::All { }, OutputsSpec::All { }); +} + +TEST(OutputsSpec, union_all_names) { + TEST_UNION(OutputsSpec::All { }, OutputsSpec::All { }, OutputsSpec::Names { "a" }); +} + +TEST(OutputsSpec, union_names_all) { + TEST_UNION(OutputsSpec::All { }, OutputsSpec::Names { "a" }, OutputsSpec::All { }); +} + +TEST(OutputsSpec, union_names_names) { + TEST_UNION((OutputsSpec::Names { "a", "b" }), OutputsSpec::Names { "a" }, OutputsSpec::Names { "b" }); +} + +#undef TEST_UNION + +#define TEST_DONT_PARSE(NAME, STR) \ + TEST(ExtendedOutputsSpec, bad_ ## NAME) { \ + std::optional extendedOutputsSpecOpt = \ + ExtendedOutputsSpec::parseOpt(STR); \ + ASSERT_FALSE(extendedOutputsSpecOpt); \ } + +TEST_DONT_PARSE(carot_empty, "^") +TEST_DONT_PARSE(prefix_carot_empty, "foo^") +TEST_DONT_PARSE(garbage, "^&*()") +TEST_DONT_PARSE(double_star, "^**") +TEST_DONT_PARSE(star_first, "^*,foo") +TEST_DONT_PARSE(star_second, "^foo,*") + +#undef TEST_DONT_PARSE + +TEST(ExtendedOutputsSpec, defeault) { + std::string_view str = "foo"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = ExtendedOutputsSpec::Default { }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); +} + +TEST(ExtendedOutputsSpec, all) { + std::string_view str = "foo^*"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::All { }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); } +TEST(ExtendedOutputsSpec, out) { + std::string_view str = "foo^out"; + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(str); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), str); +} + +TEST(ExtendedOutputsSpec, out_bin) { + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out,bin"); + ASSERT_EQ(prefix, "foo"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), "foo^bin,out"); +} + +TEST(ExtendedOutputsSpec, many_carrot) { + auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^bar^out,bin"); + ASSERT_EQ(prefix, "foo^bar"); + ExtendedOutputsSpec expected = OutputsSpec::Names { "out", "bin" }; + ASSERT_EQ(extendedOutputsSpec, expected); + ASSERT_EQ(std::string { prefix } + expected.to_string(), "foo^bar^bin,out"); +} + + +#define TEST_JSON(TYPE, NAME, STR, VAL) \ + \ + TEST(TYPE, NAME ## _to_json) { \ + using nlohmann::literals::operator "" _json; \ + ASSERT_EQ( \ + STR ## _json, \ + ((nlohmann::json) TYPE { VAL })); \ + } \ + \ + TEST(TYPE, NAME ## _from_json) { \ + using nlohmann::literals::operator "" _json; \ + ASSERT_EQ( \ + TYPE { VAL }, \ + (STR ## _json).get<TYPE>()); \ + } + +TEST_JSON(OutputsSpec, all, R"(["*"])", OutputsSpec::All { }) +TEST_JSON(OutputsSpec, name, R"(["a"])", OutputsSpec::Names { "a" }) +TEST_JSON(OutputsSpec, names, R"(["a","b"])", (OutputsSpec::Names { "a", "b" })) + +TEST_JSON(ExtendedOutputsSpec, def, R"(null)", ExtendedOutputsSpec::Default { }) +TEST_JSON(ExtendedOutputsSpec, all, R"(["*"])", ExtendedOutputsSpec::Explicit { OutputsSpec::All { } }) +TEST_JSON(ExtendedOutputsSpec, name, R"(["a"])", ExtendedOutputsSpec::Explicit { OutputsSpec::Names { "a" } }) +TEST_JSON(ExtendedOutputsSpec, names, R"(["a","b"])", (ExtendedOutputsSpec::Explicit { OutputsSpec::Names { "a", "b" } })) + +#undef TEST_JSON + } |