Split OutputsSpec and ExtendedOutputsSpec, use the former more

`DerivedPath::Built` and `DerivationGoal` were previously using a regular set with the convention that the empty set means all outputs. But it is easy to forget about this rule when processing those sets. Using `OutputSpec` forces us to get it right.
2024-11-22 14:52:55 +00:00 · 2023-01-11 18:57:18 -05:00 · 2023-01-11 18:57:18 -05:00 · ce2f91d356
commit ce2f91d356
parent a7c0cff07f
23 changed files with 377 additions and 193 deletions
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@ -488,18 +488,23 @@ struct InstallableAttrPath : InstallableValue
            if (!drvPath)
                throw Error("'%s' is not a derivation", what());

-            std::set<std::string> outputsToInstall;
+            auto derivedPath = byDrvPath.emplace(*drvPath, DerivedPath::Built {
+                .drvPath = *drvPath,
+                // Not normally legal, but we will merge right below
+                .outputs = OutputsSpec::Names { },
+            }).first;

-            if (auto outputNames = std::get_if<OutputNames>(&extendedOutputsSpec))
-                outputsToInstall = *outputNames;
-            else
-                for (auto & output : drvInfo.queryOutputs(false, std::get_if<DefaultOutputs>(&extendedOutputsSpec)))
-                    outputsToInstall.insert(output.first);
-
-            auto derivedPath = byDrvPath.emplace(*drvPath, DerivedPath::Built { .drvPath = *drvPath }).first;
-
-            for (auto & output : outputsToInstall)
-                derivedPath->second.outputs.insert(output);
+            derivedPath->second.outputs.merge(std::visit(overloaded {
+                [&](const ExtendedOutputsSpec::Default & d) -> OutputsSpec {
+                    std::set<std::string> outputsToInstall;
+                    for (auto & output : drvInfo.queryOutputs(false, true))
+                        outputsToInstall.insert(output.first);
+                    return OutputsSpec::Names { std::move(outputsToInstall) };
+                },
+                [&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
+                    return e;
+                },
+            }, extendedOutputsSpec.raw()));
        }

        DerivedPathsWithInfo res;
@ -639,41 +644,40 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()

    auto drvPath = attr->forceDerivation();

-    std::set<std::string> outputsToInstall;
    std::optional<NixInt> priority;

-    if (auto aOutputSpecified = attr->maybeGetAttr(state->sOutputSpecified)) {
-        if (aOutputSpecified->getBool()) {
-            if (auto aOutputName = attr->maybeGetAttr("outputName"))
-                outputsToInstall = { aOutputName->getString() };
-        }
-    }
-
-    else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
-        if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall"))
-            for (auto & s : aOutputsToInstall->getListOfStrings())
-                outputsToInstall.insert(s);
+    if (attr->maybeGetAttr(state->sOutputSpecified)) {
+    } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
        if (auto aPriority = aMeta->maybeGetAttr("priority"))
            priority = aPriority->getInt();
    }

-    if (outputsToInstall.empty() || std::get_if<AllOutputs>(&extendedOutputsSpec)) {
-        outputsToInstall.clear();
-        if (auto aOutputs = attr->maybeGetAttr(state->sOutputs))
-            for (auto & s : aOutputs->getListOfStrings())
-                outputsToInstall.insert(s);
-    }
-
-    if (outputsToInstall.empty())
-        outputsToInstall.insert("out");
-
-    if (auto outputNames = std::get_if<OutputNames>(&extendedOutputsSpec))
-        outputsToInstall = *outputNames;
-
    return {{
        .path = DerivedPath::Built {
            .drvPath = std::move(drvPath),
-            .outputs = std::move(outputsToInstall),
+            .outputs = std::visit(overloaded {
+                [&](const ExtendedOutputsSpec::Default & d) -> OutputsSpec {
+                    std::set<std::string> outputsToInstall;
+                    if (auto aOutputSpecified = attr->maybeGetAttr(state->sOutputSpecified)) {
+                        if (aOutputSpecified->getBool()) {
+                            if (auto aOutputName = attr->maybeGetAttr("outputName"))
+                                outputsToInstall = { aOutputName->getString() };
+                        }
+                    } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
+                        if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall"))
+                            for (auto & s : aOutputsToInstall->getListOfStrings())
+                                outputsToInstall.insert(s);
+                    }
+
+                    if (outputsToInstall.empty())
+                        outputsToInstall.insert("out");
+
+                    return OutputsSpec::Names { std::move(outputsToInstall) };
+                },
+                [&](const ExtendedOutputsSpec::Explicit & e) -> OutputsSpec {
+                    return e;
+                },
+            }, extendedOutputsSpec.raw()),
        },
        .info = {
            .priority = priority,
@ -801,7 +805,7 @@ std::vector<std::shared_ptr<Installable>> SourceExprCommand::parseInstallables(
            result.push_back(
                std::make_shared<InstallableAttrPath>(
                    state, *this, vFile,
-                    prefix == "." ? "" : prefix,
+                    prefix == "." ? "" : std::string { prefix },
                    extendedOutputsSpec));
        }

@ -918,32 +922,7 @@ std::vector<std::pair<std::shared_ptr<Installable>, BuiltPathWithResult>> Instal
            for (auto & aux : backmap[path]) {
                std::visit(overloaded {
                    [&](const DerivedPath::Built & bfd) {
-                        OutputPathMap outputs;
-                        auto drv = evalStore->readDerivation(bfd.drvPath);
-                        auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive
-                        auto drvOutputs = drv.outputsAndOptPaths(*store);
-                        for (auto & output : bfd.outputs) {
-                            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);
-                            }
-                        }
+                        auto outputs = resolveDerivedPath(*store, bfd, &*evalStore);
                        res.push_back({aux.installable, {
                            .path = BuiltPath::Built { bfd.drvPath, outputs },
                            .info = aux.info}});
--- a/src/libexpr/flake/flakeref.cc
+++ b/src/libexpr/flake/flakeref.cc
@ -245,7 +245,7 @@ std::tuple<FlakeRef, std::string, ExtendedOutputsSpec> parseFlakeRefWithFragment
    bool isFlake)
 {
    auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse(url);
-    auto [flakeRef, fragment] = parseFlakeRefWithFragment(prefix, baseDir, allowMissing, isFlake);
+    auto [flakeRef, fragment] = parseFlakeRefWithFragment(std::string { prefix }, baseDir, allowMissing, isFlake);
    return {std::move(flakeRef), fragment, extendedOutputsSpec};
 }

--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@ -53,7 +53,7 @@ StringMap EvalState::realiseContext(const PathSet & context)
            [&](const NixStringContextElem::Built & b) {
                drvs.push_back(DerivedPath::Built {
                    .drvPath = b.drvPath,
-                    .outputs = std::set { b.output },
+                    .outputs = OutputsSpec::Names { b.output },
                });
                ensureValid(b.drvPath);
            },
@ -84,16 +84,12 @@ StringMap EvalState::realiseContext(const PathSet & context)
    store->buildPaths(buildReqs);

    /* Get all the output paths corresponding to the placeholders we had */
-    for (auto & [drvPath, outputs] : drvs) {
-        const auto outputPaths = store->queryDerivationOutputMap(drvPath);
-        for (auto & outputName : outputs) {
-            auto outputPath = get(outputPaths, outputName);
-            if (!outputPath)
-                debugThrowLastTrace(Error("derivation '%s' does not have an output named '%s'",
-                    store->printStorePath(drvPath), outputName));
+    for (auto & drv : drvs) {
+        auto outputs = resolveDerivedPath(*store, drv);
+        for (auto & [outputName, outputPath] : outputs) {
            res.insert_or_assign(
-                downstreamPlaceholder(*store, drvPath, outputName),
-                store->printStorePath(*outputPath)
+                downstreamPlaceholder(*store, drv.drvPath, outputName),
+                store->printStorePath(outputPath)
            );
        }
    }
--- 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,11 @@ 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();
+    bool newOutputs = wantedOutputs.merge(outputs);
+    if (newOutputs)
        needRestart = true;
-    } else
-        for (auto & i : outputs)
-            if (wantedOutputs.insert(i).second)
-                needRestart = true;
 }


@ -390,7 +383,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 +415,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 +984,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 names;
+            },
+        }, wantedOutputs.raw());

        for (auto & wantedOutput : realWantedOutputs) {
            auto initialOutput = get(initialOutputs, wantedOutput);
@ -1322,7 +1320,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 names;
+        },
+    }, wantedOutputs.raw());
    DrvOutputs validOutputs;

    for (auto & i : queryPartialDerivationOutputMap()) {
@ -1331,7 +1336,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 +1374,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())
--- 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();
--- a/src/libstore/build/entry-points.cc
+++ b/src/libstore/build/entry-points.cc
@ -130,7 +130,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));
--- 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);
    }

--- 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)
--- 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);
--- 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);
--- 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;

--- 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)
--- 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);
--- 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 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;
+}
+
 }
--- a/src/libstore/outputs-spec.cc
+++ b/src/libstore/outputs-spec.cc
@ -6,57 +6,153 @@

 namespace nix {

-std::pair<std::string, ExtendedOutputsSpec> ExtendedOutputsSpec::parse(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[3].matched)
-        return {match[1], AllOutputs()};
+    if (match[1].matched)
+        return { OutputsSpec::All {} };

-    return {match[1], tokenizeString<OutputNames>(match[4].str(), ",")};
+    if (match[2].matched)
+        return { tokenizeString<OutputsSpec::Names>(match[2].str(), ",") };
+
+    assert(false);
 }

+
+OutputsSpec OutputsSpec::parse(std::string_view s)
+{
+    std::optional spec = OutputsSpec::parseOpt(s);
+    if (!spec)
+        throw Error("Invalid outputs specifier: '%s'", s);
+    return *spec;
+}
+
+
+std::pair<std::string_view, ExtendedOutputsSpec> ExtendedOutputsSpec::parse(std::string_view s)
+{
+    auto found = s.rfind('^');
+
+    if (found == std::string::npos)
+        return { s, ExtendedOutputsSpec::Default {} };
+
+    auto spec = OutputsSpec::parse(s.substr(found + 1));
+    return { s.substr(0, found), ExtendedOutputsSpec::Explicit { 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());
+}
+
+
 std::string ExtendedOutputsSpec::to_string() const
 {
    return std::visit(overloaded {
        [&](const ExtendedOutputsSpec::Default &) -> std::string {
            return "";
        },
-        [&](const ExtendedOutputsSpec::All &) -> std::string {
-            return "*";
-        },
-        [&](const ExtendedOutputsSpec::Names & outputNames) -> std::string {
-            return "^" + concatStringsSep(",", outputNames);
+        [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> std::string {
+            return "^" + outputSpec.to_string();
        },
    }, raw());
 }

+
+bool OutputsSpec::merge(const OutputsSpec & that)
+{
+    return std::visit(overloaded {
+        [&](OutputsSpec::All &) {
+            /* If we already refer to all outputs, there is nothing to do. */
+            return false;
+        },
+        [&](OutputsSpec::Names & theseNames) {
+            return std::visit(overloaded {
+                [&](const OutputsSpec::All &) {
+                    *this = OutputsSpec::All {};
+                    return true;
+                },
+                [&](const OutputsSpec::Names & thoseNames) {
+                    bool ret = false;
+                    for (auto & i : thoseNames)
+                        if (theseNames.insert(i).second)
+                            ret = true;
+                    return ret;
+                },
+            }, that.raw());
+        },
+    }, raw());
+}
+
+
+void to_json(nlohmann::json & json, const OutputsSpec & outputsSpec)
+{
+    std::visit(overloaded {
+        [&](const OutputsSpec::All &) {
+            json = std::vector<std::string>({"*"});
+        },
+        [&](const OutputsSpec::Names & names) {
+            json = names;
+        },
+    }, outputsSpec);
+}
+
+
 void to_json(nlohmann::json & json, const ExtendedOutputsSpec & extendedOutputsSpec)
 {
-    if (std::get_if<DefaultOutputs>(&extendedOutputsSpec))
-        json = nullptr;
-
-    else if (std::get_if<AllOutputs>(&extendedOutputsSpec))
-        json = std::vector<std::string>({"*"});
-
-    else if (auto outputNames = std::get_if<OutputNames>(&extendedOutputsSpec))
-        json = *outputNames;
+    std::visit(overloaded {
+        [&](const ExtendedOutputsSpec::Default &) {
+            json = nullptr;
+        },
+        [&](const ExtendedOutputsSpec::Explicit & e) {
+            to_json(json, e);
+        },
+    }, extendedOutputsSpec);
 }

+
+void from_json(const nlohmann::json & json, OutputsSpec & outputsSpec)
+{
+    auto names = json.get<OutputNames>();
+    if (names == OutputNames({"*"}))
+        outputsSpec = OutputsSpec::All {};
+    else
+        outputsSpec = names;
+}
+
+
 void from_json(const nlohmann::json & json, ExtendedOutputsSpec & extendedOutputsSpec)
 {
    if (json.is_null())
-        extendedOutputsSpec = DefaultOutputs();
+        extendedOutputsSpec = ExtendedOutputsSpec::Default {};
    else {
-        auto names = json.get<OutputNames>();
-        if (names == OutputNames({"*"}))
-            extendedOutputsSpec = AllOutputs();
-        else
-            extendedOutputsSpec = names;
+        extendedOutputsSpec = ExtendedOutputsSpec::Explicit { json.get<OutputsSpec>() };
    }
 }

--- a/src/libstore/outputs-spec.hh
+++ b/src/libstore/outputs-spec.hh
@ -1,5 +1,6 @@
 #pragma once

+#include <optional>
 #include <set>
 #include <variant>

@ -13,31 +14,66 @@ struct AllOutputs {
    bool operator < (const AllOutputs & _) const { return false; }
 };

+typedef std::variant<AllOutputs, OutputNames> _OutputsSpecRaw;
+
+struct OutputsSpec : _OutputsSpecRaw {
+    using Raw = _OutputsSpecRaw;
+    using Raw::Raw;
+
+    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;
+
+    /* Modify the receiver outputs spec so it is the union of it's old value
+       and the argument. Return whether the output spec needed to be modified
+       --- if it didn't it was already "large enough". */
+    bool merge(const OutputsSpec & outputs);
+
+    /* 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;
+};
+
 struct DefaultOutputs {
    bool operator < (const DefaultOutputs & _) const { return false; }
 };

-typedef std::variant<DefaultOutputs, AllOutputs, OutputNames> _ExtendedOutputsSpecRaw;
+typedef std::variant<DefaultOutputs, OutputsSpec> _ExtendedOutputsSpecRaw;

 struct ExtendedOutputsSpec : _ExtendedOutputsSpecRaw {
    using Raw = _ExtendedOutputsSpecRaw;
    using Raw::Raw;

-    using Names = OutputNames;
-    using All = AllOutputs;
    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 outputs spec. */
-    static std::pair<std::string, ExtendedOutputsSpec> parse(std::string s);
+       'prefix^*', returning the prefix and the extended outputs spec. */
+    static std::pair<std::string_view, ExtendedOutputsSpec> parse(std::string_view s);

    std::string to_string() const;
 };

+
+void to_json(nlohmann::json &, const OutputsSpec &);
+void from_json(const nlohmann::json &, OutputsSpec &);
+
 void to_json(nlohmann::json &, const ExtendedOutputsSpec &);
 void from_json(const nlohmann::json &, ExtendedOutputsSpec &);

--- 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 outputs;
+                    },
+                }, bfd.outputs.raw()),
+            };
        },
    }, p.raw());
 }
--- 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;

--- 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,
                                    });
                            }
                        }
--- 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 {
@ -719,6 +724,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:

--- a/src/libstore/tests/outputs-spec.cc
+++ b/src/libstore/tests/outputs-spec.cc
@ -4,42 +4,62 @@

 namespace nix {

+TEST(OutputsSpec_parse, basic)
+{
+    {
+        auto outputsSpec = OutputsSpec::parse("*");
+        ASSERT_TRUE(std::get_if<OutputsSpec::All>(&outputsSpec));
+    }
+
+    {
+        auto outputsSpec = OutputsSpec::parse("out");
+        ASSERT_TRUE(std::get<OutputsSpec::Names>(outputsSpec) == OutputsSpec::Names({"out"}));
+    }
+
+    {
+        auto outputsSpec = OutputsSpec::parse("out,bin");
+        ASSERT_TRUE(std::get<OutputsSpec::Names>(outputsSpec) == OutputsSpec::Names({"out", "bin"}));
+    }
+
+    {
+        std::optional outputsSpecOpt = OutputsSpec::parseOpt("&*()");
+        ASSERT_FALSE(outputsSpecOpt);
+    }
+}
+
+
 TEST(ExtendedOutputsSpec_parse, basic)
 {
    {
        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo");
        ASSERT_EQ(prefix, "foo");
-        ASSERT_TRUE(std::get_if<DefaultOutputs>(&extendedOutputsSpec));
+        ASSERT_TRUE(std::get_if<ExtendedOutputsSpec::Default>(&extendedOutputsSpec));
    }

    {
        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^*");
        ASSERT_EQ(prefix, "foo");
-        ASSERT_TRUE(std::get_if<AllOutputs>(&extendedOutputsSpec));
+        auto * explicit_p = std::get_if<ExtendedOutputsSpec::Explicit>(&extendedOutputsSpec);
+        ASSERT_TRUE(explicit_p);
+        ASSERT_TRUE(std::get_if<OutputsSpec::All>(explicit_p));
    }

    {
        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out");
        ASSERT_EQ(prefix, "foo");
-        ASSERT_TRUE(std::get<OutputNames>(extendedOutputsSpec) == OutputNames({"out"}));
+        ASSERT_TRUE(std::get<OutputsSpec::Names>(std::get<ExtendedOutputsSpec::Explicit>(extendedOutputsSpec)) == OutputsSpec::Names({"out"}));
    }

    {
        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^out,bin");
        ASSERT_EQ(prefix, "foo");
-        ASSERT_TRUE(std::get<OutputNames>(extendedOutputsSpec) == OutputNames({"out", "bin"}));
+        ASSERT_TRUE(std::get<OutputsSpec::Names>(std::get<ExtendedOutputsSpec::Explicit>(extendedOutputsSpec)) == OutputsSpec::Names({"out", "bin"}));
    }

    {
        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^bar^out,bin");
        ASSERT_EQ(prefix, "foo^bar");
-        ASSERT_TRUE(std::get<OutputNames>(extendedOutputsSpec) == OutputNames({"out", "bin"}));
-    }
-
-    {
-        auto [prefix, extendedOutputsSpec] = ExtendedOutputsSpec::parse("foo^&*()");
-        ASSERT_EQ(prefix, "foo^&*()");
-        ASSERT_TRUE(std::get_if<DefaultOutputs>(&extendedOutputsSpec));
+        ASSERT_TRUE(std::get<OutputsSpec::Names>(std::get<ExtendedOutputsSpec::Explicit>(extendedOutputsSpec)) == OutputsSpec::Names({"out", "bin"}));
    }
 }

--- a/src/nix-build/nix-build.cc
+++ b/src/nix-build/nix-build.cc
@ -397,7 +397,7 @@ static void main_nix_build(int argc, char * * argv)
                auto bashDrv = drv->requireDrvPath();
                pathsToBuild.push_back(DerivedPath::Built {
                    .drvPath = bashDrv,
-                    .outputs = {"out"},
+                    .outputs = OutputsSpec::Names {"out"},
                });
                pathsToCopy.insert(bashDrv);
                shellDrv = bashDrv;
@ -591,7 +591,7 @@ static void main_nix_build(int argc, char * * argv)
            if (outputName == "")
                throw Error("derivation '%s' lacks an 'outputName' attribute", store->printStorePath(drvPath));

-            pathsToBuild.push_back(DerivedPath::Built{drvPath, {outputName}});
+            pathsToBuild.push_back(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}});
            pathsToBuildOrdered.push_back({drvPath, {outputName}});
            drvsToCopy.insert(drvPath);

--- a/src/nix/app.cc
+++ b/src/nix/app.cc
@ -86,13 +86,13 @@ UnresolvedApp Installable::toApp(EvalState & state)
                    /* We want all outputs of the drv */
                    return DerivedPath::Built {
                        .drvPath = d.drvPath,
-                        .outputs = {},
+                        .outputs = OutputsSpec::All {},
                    };
                },
                [&](const NixStringContextElem::Built & b) -> DerivedPath {
                    return DerivedPath::Built {
                        .drvPath = b.drvPath,
-                        .outputs = { b.output },
+                        .outputs = OutputsSpec::Names { b.output },
                    };
                },
                [&](const NixStringContextElem::Opaque & o) -> DerivedPath {
@ -127,7 +127,7 @@ UnresolvedApp Installable::toApp(EvalState & state)
        return UnresolvedApp { App {
            .context = { DerivedPath::Built {
                .drvPath = drvPath,
-                .outputs = {outputName},
+                .outputs = OutputsSpec::Names { outputName },
            } },
            .program = program,
        }};