From 5f723e96e620dbda93e090354611e12806f14997 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 11 May 2025 22:50:32 +0200 Subject: [PATCH] libstore: flatten {,Single}{Built,Derived}Path only dynamic derivations could produce a non-opaque drvPath. since dynamic derivations are no longer supported we can have drvPath be opaque at all times, simplifying downstream code significantly and making quite a few methods unnecessary. discardOutputPath was only called on drvPath members anyway and thus reduces to a copy, other operations at the very least are no longer recursive. some vestige of dynamic derivations remains in DerivedPathMap though (for now). Change-Id: Ifb4ad53a3c67800be5a62540068c8279d4ae0046 --- lix/legacy/nix-build.cc | 10 ++---- lix/libcmd/built-path.cc | 36 ++----------------- lix/libcmd/built-path.hh | 10 ++---- lix/libcmd/installable-derived-path.cc | 3 +- lix/libcmd/installables.cc | 34 ++---------------- lix/libexpr/eval-cache.cc | 2 +- lix/libexpr/eval.cc | 17 +++------ lix/libexpr/primops.cc | 2 +- lix/libexpr/value/context.cc | 11 ++---- lix/libstore/build/derivation-goal.cc | 10 ++---- lix/libstore/build/worker.cc | 5 +-- lix/libstore/derivations.cc | 19 +++------- lix/libstore/derived-path.cc | 34 +++++------------- lix/libstore/derived-path.hh | 24 ++++--------- lix/libstore/downstream-placeholder.cc | 27 +------------- lix/libstore/downstream-placeholder.hh | 16 +-------- lix/libstore/misc.cc | 16 +++------ lix/libstore/path-with-outputs.cc | 31 +++++++--------- lix/libstore/remote-store.cc | 2 +- lix/nix/app.cc | 2 +- lix/nix/log.cc | 2 +- tests/unit/libexpr/value/context.cc | 2 +- .../libstore-support/tests/derived-path.cc | 4 +-- tests/unit/libstore/derived-path.cc | 2 +- tests/unit/libstore/downstream-placeholder.cc | 20 ----------- 25 files changed, 67 insertions(+), 274 deletions(-) diff --git a/lix/legacy/nix-build.cc b/lix/legacy/nix-build.cc index fd09d8945..0f5fdde6a 100644 --- a/lix/legacy/nix-build.cc +++ b/lix/legacy/nix-build.cc @@ -368,18 +368,14 @@ static void main_nix_build(AsyncIoRoot & aio, std::string programName, Strings a } } - std::function, const DerivedPathMap::ChildNode &)> accumDerivedPath; - - accumDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { + auto accumDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { if (!inputNode.value.empty()) pathsToBuild.push_back(DerivedPath::Built { .drvPath = inputDrv, .outputs = OutputsSpec::Names { inputNode.value }, }); - for (const auto & [outputName, childNode] : inputNode.childMap) - accumDerivedPath( - make_ref(SingleDerivedPath::Built { inputDrv, outputName }), - childNode); + // only dynamic derivations have a non-empty childMap + assert(inputNode.childMap.empty()); }; // Build or fetch all dependencies of the derivation. diff --git a/lix/libcmd/built-path.cc b/lix/libcmd/built-path.cc index 677e4af32..7612dce8a 100644 --- a/lix/libcmd/built-path.cc +++ b/lix/libcmd/built-path.cc @@ -34,16 +34,6 @@ CMP(SingleBuiltPath, BuiltPathBuilt, outputs) #undef CMP #undef CMP_ONE -StorePath SingleBuiltPath::outPath() const -{ - return std::visit( - overloaded{ - [](const SingleBuiltPath::Opaque & p) { return p.path; }, - [](const SingleBuiltPath::Built & b) { return b.output.second; }, - }, raw() - ); -} - StorePathSet BuiltPath::outPaths() const { return std::visit( @@ -59,28 +49,6 @@ StorePathSet BuiltPath::outPaths() const ); } -SingleDerivedPath::Built SingleBuiltPath::Built::discardOutputPath() const -{ - return SingleDerivedPath::Built { - .drvPath = make_ref(drvPath->discardOutputPath()), - .output = output.first, - }; -} - -SingleDerivedPath SingleBuiltPath::discardOutputPath() const -{ - return std::visit( - overloaded{ - [](const SingleBuiltPath::Opaque & p) -> SingleDerivedPath { - return p; - }, - [](const SingleBuiltPath::Built & b) -> SingleDerivedPath { - return b.discardOutputPath(); - }, - }, raw() - ); -} - kj::Promise> BuiltPath::Built::toJSON(const Store & store) const try { JSON res; @@ -141,7 +109,7 @@ try { [&](const BuiltPath::Built & p) -> kj::Promise> { try { auto drvHashes = TRY_AWAIT( - staticOutputHashes(store, TRY_AWAIT(store.readDerivation(p.drvPath->outPath()))) + staticOutputHashes(store, TRY_AWAIT(store.readDerivation(p.drvPath->path))) ); for (auto& [outputName, outputPath] : p.outputs) { if (experimentalFeatureSettings.isEnabled( @@ -150,7 +118,7 @@ try { if (!drvOutput) throw Error( "the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)", - store.printStorePath(p.drvPath->outPath()), outputName); + store.printStorePath(p.drvPath->path), outputName); auto thisRealisation = TRY_AWAIT(store.queryRealisation( DrvOutput{*drvOutput, outputName})); assert(thisRealisation); // We’ve built it, so we must diff --git a/lix/libcmd/built-path.hh b/lix/libcmd/built-path.hh index 7652269dd..8e2ace092 100644 --- a/lix/libcmd/built-path.hh +++ b/lix/libcmd/built-path.hh @@ -10,11 +10,9 @@ namespace nix { struct SingleBuiltPath; struct SingleBuiltPathBuilt { - ref drvPath; + ref drvPath; std::pair output; - SingleDerivedPathBuilt discardOutputPath() const; - std::string to_string(const Store & store) const; static SingleBuiltPathBuilt parse(const Store & store, std::string_view, std::string_view); kj::Promise> toJSON(const Store & store) const; @@ -40,10 +38,6 @@ struct SingleBuiltPath : built_path::detail::SingleBuiltPathRaw { return static_cast(*this); } - StorePath outPath() const; - - SingleDerivedPath discardOutputPath() const; - static SingleBuiltPath parse(const Store & store, std::string_view); kj::Promise> toJSON(const Store & store) const; }; @@ -59,7 +53,7 @@ static inline ref staticDrv(StorePath drvPath) * See 'BuiltPath' for more an explanation. */ struct BuiltPathBuilt { - ref drvPath; + ref drvPath; std::map outputs; std::string to_string(const Store & store) const; diff --git a/lix/libcmd/installable-derived-path.cc b/lix/libcmd/installable-derived-path.cc index b2aca0971..c0477cac0 100644 --- a/lix/libcmd/installable-derived-path.cc +++ b/lix/libcmd/installable-derived-path.cc @@ -48,8 +48,7 @@ InstallableDerivedPath InstallableDerivedPath::parse( }, // If the user did use ^, we just do exactly what is written. [&](const ExtendedOutputsSpec::Explicit & outputSpec) -> DerivedPath { - auto drv = make_ref(SingleDerivedPath::parse(*store, prefix)); - drvRequireExperiment(*drv); + auto drv = make_ref(DerivedPathOpaque::parse(*store, prefix)); return DerivedPath::Built { .drvPath = std::move(drv), .outputs = outputSpec, diff --git a/lix/libcmd/installables.cc b/lix/libcmd/installables.cc index 30ebcf050..1087789ed 100644 --- a/lix/libcmd/installables.cc +++ b/lix/libcmd/installables.cc @@ -524,36 +524,6 @@ ref SourceExprCommand::parseInstallable( return installables.front(); } -static kj::Promise> getBuiltPath(ref evalStore, ref store, const SingleDerivedPath & b) -try { - auto handlers = overloaded{ - [&](const SingleDerivedPath::Opaque & bo) -> kj::Promise> { - return {SingleBuiltPath::Opaque { bo.path }}; - }, - // NOLINTNEXTLINE(cppcoreguidelines-avoid-capturing-lambda-coroutines) - [&](const SingleDerivedPath::Built & bfd) -> kj::Promise> { - try { - auto drvPath = TRY_AWAIT(getBuiltPath(evalStore, store, *bfd.drvPath)); - // Resolving this instead of `bfd` will yield the same result, but avoid duplicative work. - SingleDerivedPath::Built truncatedBfd { - .drvPath = makeConstantStorePathRef(drvPath.outPath()), - .output = bfd.output, - }; - auto outputPath = TRY_AWAIT(resolveDerivedPath(*store, truncatedBfd, &*evalStore)); - co_return SingleBuiltPath::Built { - .drvPath = make_ref(std::move(drvPath)), - .output = { bfd.output, outputPath }, - }; - } catch (...) { - co_return result::current_exception(); - } - }, - }; - co_return TRY_AWAIT(std::visit(handlers, b.raw())); -} catch (...) { - co_return result::current_exception(); -} - std::vector Installable::build( EvalState & state, ref evalStore, @@ -642,7 +612,7 @@ std::vector, BuiltPathWithResult>> Installable::build state.aio.blockOn(resolveDerivedPath(*store, bfd, &*evalStore)); res.push_back({aux.installable, { .path = BuiltPath::Built { - .drvPath = make_ref(state.aio.blockOn(getBuiltPath(evalStore, store, *bfd.drvPath))), + .drvPath = bfd.drvPath, .outputs = outputs, }, .info = aux.info}}); @@ -674,7 +644,7 @@ std::vector, BuiltPathWithResult>> Installable::build outputs.emplace(outputName, realisation.outPath); res.push_back({aux.installable, { .path = BuiltPath::Built { - .drvPath = make_ref(state.aio.blockOn(getBuiltPath(evalStore, store, *bfd.drvPath))), + .drvPath = bfd.drvPath, .outputs = outputs, }, .info = aux.info, diff --git a/lix/libexpr/eval-cache.cc b/lix/libexpr/eval-cache.cc index 2fb950b77..58f36f2b5 100644 --- a/lix/libexpr/eval-cache.cc +++ b/lix/libexpr/eval-cache.cc @@ -580,7 +580,7 @@ string_t AttrCursor::getStringWithContext(EvalState & state) return d.drvPath; }, [&](const NixStringContextElem::Built & b) -> const StorePath & { - return b.drvPath->getBaseStorePath(); + return b.drvPath->path; }, [&](const NixStringContextElem::Opaque & o) -> const StorePath & { return o.path; diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index 51b19c8a5..232370850 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -922,18 +922,11 @@ std::string EvalState::mkSingleDerivedPathStringRaw( return ctx.store->printStorePath(o.path); }, [&](const SingleDerivedPath::Built & b) { - auto optStaticOutputPath = std::visit(overloaded { - [&](const SingleDerivedPath::Opaque & o) { - auto drv = aio.blockOn(ctx.store->readDerivation(o.path)); - auto i = drv.outputs.find(b.output); - if (i == drv.outputs.end()) - throw Error("derivation '%s' does not have output '%s'", b.drvPath->to_string(*ctx.store), b.output); - return i->second.path(*ctx.store, drv.name, b.output); - }, - [&](const SingleDerivedPath::Built & o) -> std::optional { - return std::nullopt; - }, - }, b.drvPath->raw()); + auto drv = aio.blockOn(ctx.store->readDerivation(b.drvPath->path)); + auto i = drv.outputs.find(b.output); + if (i == drv.outputs.end()) + throw Error("derivation '%s' does not have output '%s'", b.drvPath->to_string(*ctx.store), b.output); + auto optStaticOutputPath = i->second.path(*ctx.store, drv.name, b.output); return mkOutputStringRaw(b, optStaticOutputPath); } }, p.raw()); diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index f539c7c3a..60fc339a3 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -58,7 +58,7 @@ StringMap EvalState::realiseContext(const NixStringContext & context) .drvPath = b.drvPath, .outputs = OutputsSpec::Names { b.output }, }); - return ensureValid(b.drvPath->getBaseStorePath()); + return ensureValid(b.drvPath->path); }, [&](const NixStringContextElem::Opaque & o) { auto ctxS = ctx.store->printStorePath(o.path); diff --git a/lix/libexpr/value/context.cc b/lix/libexpr/value/context.cc index 479cc1720..97398e948 100644 --- a/lix/libexpr/value/context.cc +++ b/lix/libexpr/value/context.cc @@ -26,7 +26,7 @@ NixStringContextElem NixStringContextElem::parse(std::string_view s0) std::string output { s.substr(0, index) }; // Advance string to parse after the '!' s = s.substr(index + 1); - auto drv = make_ref(SingleDerivedPath::Opaque{StorePath{s}}); + auto drv = make_ref(SingleDerivedPath::Opaque{StorePath{s}}); return SingleDerivedPath::Built{ .drvPath = std::move(drv), .output = std::move(output), @@ -59,14 +59,7 @@ std::string NixStringContextElem::to_string() const res += '!'; res += b.output; res += '!'; - std::visit(overloaded { - [&](const SingleDerivedPath::Opaque & o2) { - res += o2.path.to_string(); - }, - [&](const SingleDerivedPath::Built & o) { - assert(false && "dynamic derivations shouldn't exist in string context any more"); - }, - }, b.drvPath->raw()); + res += b.drvPath->path.to_string(); }, [&](const NixStringContextElem::Opaque & o) { res += o.path.to_string(); diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index 69bc523c9..3f54556fa 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -373,9 +373,7 @@ try { /* The inputs must be built before we can build this goal. */ inputDrvOutputs.clear(); if (useDerivation) { - std::function, const DerivedPathMap::ChildNode &)> addWaiteeDerivedPath; - - addWaiteeDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { + auto addWaiteeDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { if (!inputNode.value.empty()) dependencies.add(worker.goalFactory().makeGoal( DerivedPath::Built { @@ -383,10 +381,8 @@ try { .outputs = inputNode.value, }, buildMode == bmRepair ? bmRepair : bmNormal)); - for (const auto & [outputName, childNode] : inputNode.childMap) - addWaiteeDerivedPath( - make_ref(SingleDerivedPath::Built { inputDrv, outputName }), - childNode); + // only dynamic derivations have a non-empty childMap + assert(inputNode.childMap.empty()); }; for (const auto & [inputDrvPath, inputNode] : dynamic_cast(drv.get())->inputDrvs.map) { diff --git a/lix/libstore/build/worker.cc b/lix/libstore/build/worker.cc index 2523d2802..2e308ae46 100644 --- a/lix/libstore/build/worker.cc +++ b/lix/libstore/build/worker.cc @@ -201,10 +201,7 @@ std::pair>> Worker::makeGoal(const { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> std::pair>> { - if (auto bop = std::get_if(&*bfd.drvPath)) - return makeDerivationGoal(bop->path, bfd.outputs, buildMode); - else - throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented."); + return makeDerivationGoal(bfd.drvPath->path, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> std::pair>> { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); diff --git a/lix/libstore/derivations.cc b/lix/libstore/derivations.cc index a7fccfc41..86a9de9b3 100644 --- a/lix/libstore/derivations.cc +++ b/lix/libstore/derivations.cc @@ -931,7 +931,6 @@ try { static bool tryResolveInput( Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, - const DownstreamPlaceholder * placeholderOpt, const StorePath & inputDrv, const DerivedPathMap::ChildNode & inputNode, const std::map, StorePath> & inputDrvOutputs) { @@ -946,9 +945,7 @@ static bool tryResolveInput( }; auto getPlaceholder = [&](const std::string & outputName) { - return placeholderOpt - ? DownstreamPlaceholder::unknownDerivation(*placeholderOpt, outputName) - : DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName); + return DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName); }; for (auto & outputName : inputNode.value) { @@ -963,16 +960,8 @@ static bool tryResolveInput( inputSrcs.insert(std::move(actualPath)); } - for (auto & [outputName, childNode] : inputNode.childMap) { - auto actualPathOpt = getOutput(outputName); - if (!actualPathOpt) return false; - auto actualPath = *actualPathOpt; - auto nextPlaceholder = getPlaceholder(outputName); - if (!tryResolveInput(store, inputSrcs, inputRewrites, - &nextPlaceholder, actualPath, childNode, - inputDrvOutputs)) - return false; - } + // only dynamic drvs can have non-empty childMaps + assert(inputNode.childMap.empty()); return true; } @@ -987,7 +976,7 @@ try { for (auto & [inputDrv, inputNode] : inputDrvs.map) if (!tryResolveInput(store, resolved.inputSrcs, inputRewrites, - nullptr, inputDrv, inputNode, inputDrvOutputs)) + inputDrv, inputNode, inputDrvOutputs)) co_return std::nullopt; TRY_AWAIT(rewriteDerivation(store, resolved, inputRewrites)); diff --git a/lix/libstore/derived-path.cc b/lix/libstore/derived-path.cc index 66dc664df..576392b93 100644 --- a/lix/libstore/derived-path.cc +++ b/lix/libstore/derived-path.cc @@ -121,7 +121,7 @@ std::string DerivedPath::Built::to_string(const Store & store) const std::string DerivedPath::Built::to_string_legacy(const Store & store) const { - return drvPath->to_string_legacy(store) + return drvPath->to_string(store) + "!" + outputs.to_string(); } @@ -162,26 +162,11 @@ DerivedPath::Opaque DerivedPath::Opaque::parse(const Store & store, std::string_ return {store.parseStorePath(s)}; } -void drvRequireExperiment( - const SingleDerivedPath & drv, - const ExperimentalFeatureSettings & xpSettings) -{ - std::visit(overloaded { - [&](const SingleDerivedPath::Opaque &) { - // plain drv path; no experimental features required. - }, - [&](const SingleDerivedPath::Built &) { - xpSettings.require(Xp::DynamicDerivations); - }, - }, drv.raw()); -} - SingleDerivedPath::Built SingleDerivedPath::Built::parse( - const Store & store, ref drv, + const Store & store, ref drv, OutputNameView output, const ExperimentalFeatureSettings & xpSettings) { - drvRequireExperiment(*drv, xpSettings); return { .drvPath = drv, .output = std::string { output }, @@ -189,11 +174,10 @@ SingleDerivedPath::Built SingleDerivedPath::Built::parse( } DerivedPath::Built DerivedPath::Built::parse( - const Store & store, ref drv, + const Store & store, ref drv, OutputNameView outputsS, const ExperimentalFeatureSettings & xpSettings) { - drvRequireExperiment(*drv, xpSettings); return { .drvPath = drv, .outputs = OutputsSpec::parse(outputsS), @@ -210,7 +194,7 @@ static DerivedPathT parseDerivedPath( return DerivedPathT::Opaque::parse(store, s); } else { auto path = DerivedPathT::Built::parse(store, - make_ref(DerivedPathT::Opaque::parse( + make_ref(DerivedPathT::Opaque::parse( store, s.substr(0, n))), s.substr(n + 1), @@ -275,22 +259,22 @@ DerivedPath DerivedPath::fromSingle(const SingleDerivedPath & req) const StorePath & SingleDerivedPath::Built::getBaseStorePath() const { - return drvPath->getBaseStorePath(); + return drvPath->path; } const StorePath & DerivedPath::Built::getBaseStorePath() const { - return drvPath->getBaseStorePath(); + return drvPath->path; } template static inline const StorePath & getBaseStorePath_(const DP & derivedPath) { return std::visit(overloaded { - [&](const typename DP::Built & bfd) -> auto & { - return bfd.drvPath->getBaseStorePath(); + [&](const typename DP::Built & bfd) -> const StorePath & { + return bfd.drvPath->path; }, - [&](const typename DP::Opaque & bo) -> auto & { + [&](const typename DP::Opaque & bo) -> const StorePath & { return bo.path; }, }, derivedPath.raw()); diff --git a/lix/libstore/derived-path.hh b/lix/libstore/derived-path.hh index a4cf26f1a..dd6b8c130 100644 --- a/lix/libstore/derived-path.hh +++ b/lix/libstore/derived-path.hh @@ -43,7 +43,7 @@ struct SingleDerivedPath; * path of the given output name. */ struct SingleDerivedPathBuilt { - ref drvPath; + ref drvPath; OutputName output; /** @@ -72,7 +72,7 @@ struct SingleDerivedPathBuilt { * @param xpSettings Stop-gap to avoid globals during unit tests. */ static SingleDerivedPathBuilt parse( - const Store & store, ref drvPath, + const Store & store, ref drvPath, OutputNameView outputs, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); kj::Promise> toJSON(Store & store) const; @@ -150,9 +150,9 @@ struct SingleDerivedPath : derived_path::detail::SingleDerivedPathRaw { kj::Promise> toJSON(Store & store) const; }; -static inline ref makeConstantStorePathRef(StorePath drvPath) +static inline ref makeConstantStorePathRef(StorePath drvPath) { - return make_ref(SingleDerivedPath::Opaque { drvPath }); + return make_ref(SingleDerivedPath::Opaque { drvPath }); } /** @@ -168,7 +168,7 @@ static inline ref makeConstantStorePathRef(StorePath drvPath) * output name. */ struct DerivedPathBuilt { - ref drvPath; + ref drvPath; OutputsSpec outputs; /** @@ -197,7 +197,7 @@ struct DerivedPathBuilt { * @param xpSettings Stop-gap to avoid globals during unit tests. */ static DerivedPathBuilt parse( - const Store & store, ref, + const Store & store, ref, std::string_view, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); kj::Promise> toJSON(Store & store) const; @@ -282,16 +282,4 @@ struct DerivedPath : derived_path::detail::DerivedPathRaw { typedef std::vector DerivedPaths; -/** - * Used by various parser functions to require experimental features as - * needed. - * - * Somewhat unfortunate this cannot just be an implementation detail for - * this module. - * - * @param xpSettings Stop-gap to avoid globals during unit tests. - */ -void drvRequireExperiment( - const SingleDerivedPath & drv, - const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); } diff --git a/lix/libstore/downstream-placeholder.cc b/lix/libstore/downstream-placeholder.cc index d725c967b..82ff2f1b0 100644 --- a/lix/libstore/downstream-placeholder.cc +++ b/lix/libstore/downstream-placeholder.cc @@ -23,36 +23,11 @@ DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput( }; } -DownstreamPlaceholder DownstreamPlaceholder::unknownDerivation( - const DownstreamPlaceholder & placeholder, - OutputNameView outputName, - const ExperimentalFeatureSettings & xpSettings) -{ - xpSettings.require(Xp::DynamicDerivations); - auto compressed = compressHash(placeholder.hash, 20); - auto clearText = "nix-computed-output:" - + compressed.to_string(Base::Base32, false) - + ":" + std::string { outputName }; - return DownstreamPlaceholder { - hashString(HashType::SHA256, clearText) - }; -} - DownstreamPlaceholder DownstreamPlaceholder::fromSingleDerivedPathBuilt( const SingleDerivedPath::Built & b, const ExperimentalFeatureSettings & xpSettings) { - return std::visit(overloaded { - [&](const SingleDerivedPath::Opaque & o) { - return DownstreamPlaceholder::unknownCaOutput(o.path, b.output, xpSettings); - }, - [&](const SingleDerivedPath::Built & b2) { - return DownstreamPlaceholder::unknownDerivation( - DownstreamPlaceholder::fromSingleDerivedPathBuilt(b2, xpSettings), - b.output, - xpSettings); - }, - }, b.drvPath->raw()); + return DownstreamPlaceholder::unknownCaOutput(b.drvPath->path, b.output, xpSettings); } } diff --git a/lix/libstore/downstream-placeholder.hh b/lix/libstore/downstream-placeholder.hh index c53864f3d..8b6ee7a44 100644 --- a/lix/libstore/downstream-placeholder.hh +++ b/lix/libstore/downstream-placeholder.hh @@ -61,24 +61,10 @@ public: OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); - /** - * Create a placehold for the output of an unknown derivation. - * - * The derivation is not yet known because it is a dynamic - * derivaiton --- it is itself an output of another derivation --- - * and we just have (another) placeholder for it. - * - * @param xpSettings Stop-gap to avoid globals during unit tests. - */ - static DownstreamPlaceholder unknownDerivation( - const DownstreamPlaceholder & drvPlaceholder, - OutputNameView outputName, - const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); - /** * Convenience constructor that handles both cases (unknown * content-addressed output and unknown derivation), delegating as - * needed to `unknownCaOutput` and `unknownDerivation`. + * needed to `unknownCaOutput`. * * Recursively builds up a placeholder from a * `SingleDerivedPath::Built.drvPath` chain. diff --git a/lix/libstore/misc.cc b/lix/libstore/misc.cc index d39bc0582..9c9a74343 100644 --- a/lix/libstore/misc.cc +++ b/lix/libstore/misc.cc @@ -150,17 +150,15 @@ struct QueryMissingContext kj::Promise> queryMissing(const std::vector & targets); - void enqueueDerivedPaths(ref inputDrv, const DerivedPathMap::ChildNode & inputNode) + void enqueueDerivedPaths(ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { if (!inputNode.value.empty()) { pool.enqueueWithAio([this, path{DerivedPath::Built{inputDrv, inputNode.value}}]( AsyncIoRoot & aio ) { doPath(aio, path); }); } - for (const auto & [outputName, childNode] : inputNode.childMap) - enqueueDerivedPaths( - make_ref(SingleDerivedPath::Built { inputDrv, outputName }), - childNode); + // only dynamic derivations have a non-empty childMap + assert(inputNode.childMap.empty()); } void mustBuildDrv(const StorePath & drvPath, const Derivation & drv) @@ -234,13 +232,7 @@ struct QueryMissingContext void doPathBuilt(AsyncIoRoot & aio, const DerivedPath::Built & bfd) { - auto drvPathP = std::get_if(&*bfd.drvPath); - if (!drvPathP) { - // TODO make work in this case. - warn("Ignoring dynamic derivation %s while querying missing paths; not yet implemented", bfd.drvPath->to_string(store)); - return; - } - auto & drvPath = drvPathP->path; + auto & drvPath = bfd.drvPath->path; if (!aio.blockOn(store.isValidPath(drvPath))) { // FIXME: we could try to substitute the derivation. diff --git a/lix/libstore/path-with-outputs.cc b/lix/libstore/path-with-outputs.cc index 8b6f5eacb..e97ddb60e 100644 --- a/lix/libstore/path-with-outputs.cc +++ b/lix/libstore/path-with-outputs.cc @@ -50,25 +50,18 @@ StorePathWithOutputs::ParseResult StorePathWithOutputs::tryFromDerivedPath(const return StorePathWithOutputs { bo.path }; }, [&](const DerivedPath::Built & bfd) -> StorePathWithOutputs::ParseResult { - return std::visit(overloaded { - [&](const SingleDerivedPath::Opaque & bo) -> StorePathWithOutputs::ParseResult { - return StorePathWithOutputs { - .path = bo.path, - // Use legacy encoding of wildcard as empty set - .outputs = std::visit(overloaded { - [&](const OutputsSpec::All &) -> StringSet { - return {}; - }, - [&](const OutputsSpec::Names & outputs) { - return static_cast(outputs); - }, - }, bfd.outputs.raw), - }; - }, - [&](const SingleDerivedPath::Built &) -> StorePathWithOutputs::ParseResult { - return std::monostate {}; - }, - }, bfd.drvPath->raw()); + return StorePathWithOutputs { + .path = bfd.drvPath->path, + // Use legacy encoding of wildcard as empty set + .outputs = std::visit(overloaded { + [&](const OutputsSpec::All &) -> StringSet { + return {}; + }, + [&](const OutputsSpec::Names & outputs) { + return static_cast(outputs); + }, + }, bfd.outputs.raw), + }; }, }, p.raw()); } diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index 99133325a..1d20b2150 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -667,7 +667,7 @@ try { // Do nothing, path is hopefully there already }, [&](const DerivedPath::Built & bp) { - drvPaths2.insert(bp.drvPath->getBaseStorePath()); + drvPaths2.insert(bp.drvPath->path); }, }, i.raw()); } diff --git a/lix/nix/app.cc b/lix/nix/app.cc index 5bff0bba9..9ca4a37ba 100644 --- a/lix/nix/app.cc +++ b/lix/nix/app.cc @@ -27,7 +27,7 @@ StringPairs resolveRewrites( res.emplace( DownstreamPlaceholder::fromSingleDerivedPathBuilt( SingleDerivedPath::Built { - .drvPath = make_ref(drvDep->drvPath->discardOutputPath()), + .drvPath = makeConstantStorePathRef(drvDep->drvPath->path), .output = outputName, }).render(), store.printStorePath(outputPath) diff --git a/lix/nix/log.cc b/lix/nix/log.cc index 575e03261..d420fa90e 100644 --- a/lix/nix/log.cc +++ b/lix/nix/log.cc @@ -36,7 +36,7 @@ struct CmdLog : InstallableCommand // For compat with CLI today, TODO revisit auto oneUp = std::visit(overloaded { [&](const DerivedPath::Opaque & bo) { - return make_ref(bo); + return make_ref(bo); }, [&](const DerivedPath::Built & bfd) { return bfd.drvPath; diff --git a/tests/unit/libexpr/value/context.cc b/tests/unit/libexpr/value/context.cc index 1e7857aa8..9c775d6fc 100644 --- a/tests/unit/libexpr/value/context.cc +++ b/tests/unit/libexpr/value/context.cc @@ -78,7 +78,7 @@ TEST(NixStringContextElemTest, built_opaque) { auto * p = std::get_if(&elem.raw); ASSERT_TRUE(p); ASSERT_EQ(p->output, "foo"); - ASSERT_EQ(*p->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque { + ASSERT_EQ(*p->drvPath, (SingleDerivedPath::Opaque { .path = StorePath { built.substr(5) }, })); ASSERT_EQ(elem.to_string(), built); diff --git a/tests/unit/libstore-support/tests/derived-path.cc b/tests/unit/libstore-support/tests/derived-path.cc index b8506fb8f..c026711bb 100644 --- a/tests/unit/libstore-support/tests/derived-path.cc +++ b/tests/unit/libstore-support/tests/derived-path.cc @@ -20,7 +20,7 @@ Gen Arbitrary::arbitrary() Gen Arbitrary::arbitrary() { return gen::just(SingleDerivedPath::Built { - .drvPath = make_ref(*gen::arbitrary()), + .drvPath = make_ref(*gen::arbitrary()), .output = (*gen::arbitrary()).name, }); } @@ -28,7 +28,7 @@ Gen Arbitrary::arbitrary() Gen Arbitrary::arbitrary() { return gen::just(DerivedPath::Built { - .drvPath = make_ref(*gen::arbitrary()), + .drvPath = make_ref(*gen::arbitrary()), .outputs = *gen::arbitrary(), }); } diff --git a/tests/unit/libstore/derived-path.cc b/tests/unit/libstore/derived-path.cc index 3c9219bdc..49aca7c67 100644 --- a/tests/unit/libstore/derived-path.cc +++ b/tests/unit/libstore/derived-path.cc @@ -38,7 +38,7 @@ TEST_F(DerivedPathTest, built_opaque) { auto * p = std::get_if(&elem); ASSERT_TRUE(p); ASSERT_EQ(p->outputs, ((OutputsSpec) OutputsSpec::Names { "foo", "bar" })); - ASSERT_EQ(*p->drvPath, ((SingleDerivedPath) SingleDerivedPath::Opaque { + ASSERT_EQ(*p->drvPath, (SingleDerivedPath::Opaque { .path = store->parseStorePath(built.substr(0, 49)), })); ASSERT_EQ(elem.to_string(*store), built); diff --git a/tests/unit/libstore/downstream-placeholder.cc b/tests/unit/libstore/downstream-placeholder.cc index 8de615dd7..d67932f35 100644 --- a/tests/unit/libstore/downstream-placeholder.cc +++ b/tests/unit/libstore/downstream-placeholder.cc @@ -20,24 +20,4 @@ TEST(DownstreamPlaceholder, unknownCaOutput) { "/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz"); } -TEST(DownstreamPlaceholder, unknownDerivation) { - /** - * Same reason as above - */ - ExperimentalFeatureSettings mockXpSettings; - mockXpSettings.experimentalFeatures.override( - ExperimentalFeatures{} | Xp::DynamicDerivations | Xp::CaDerivations - ); - - ASSERT_EQ( - DownstreamPlaceholder::unknownDerivation( - DownstreamPlaceholder::unknownCaOutput( - StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" }, - "out", - mockXpSettings), - "out", - mockXpSettings).render(), - "/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w"); -} - }