From d4da9cd3b0e4f7e962ad8fef1521422b86a8b8c9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 13 Dec 2025 12:40:06 -0500 Subject: [PATCH 01/10] `Store::queryRealisation` always return nullptr without xp feature This is a more comprehensive fix than the one done in that this replaces ee5860f54283615a9029f34c446f144a2a01b0e9, because it affects all store types. --- src/libstore-tests/dummy-store.cc | 4 ++++ src/libstore/local-store.cc | 4 ---- src/libstore/store-api.cc | 6 ++++++ 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/libstore-tests/dummy-store.cc b/src/libstore-tests/dummy-store.cc index bc165ad7f58..4972fc424fd 100644 --- a/src/libstore-tests/dummy-store.cc +++ b/src/libstore-tests/dummy-store.cc @@ -31,6 +31,8 @@ TEST(DummyStore, realisation_read) { initLibStore(/*loadConfig=*/false); + nix::experimentalFeatureSettings.set("extra-experimental-features", "ca-derivations"); + auto store = [] { auto cfg = make_ref(StoreReference::Params{}); cfg->readOnly = false; @@ -53,6 +55,8 @@ TEST(DummyStore, realisation_read) ASSERT_TRUE(value2); EXPECT_EQ(*value2, value); + + nix::experimentalFeatureSettings.set("extra-experimental-features", ""); } /* ---------------------------------------------------------------------------- diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 0d06c747146..ece197def61 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1568,10 +1568,6 @@ void LocalStore::queryRealisationUncached( const DrvOutput & id, Callback> callback) noexcept { try { - if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { - callback(nullptr); - return; - } auto maybeRealisation = retrySQLite>( [&]() { return queryRealisation_(*_state->lock(), id); }); if (maybeRealisation) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 83446a1b343..baddfb4ab20 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -656,6 +656,12 @@ void Store::queryPathInfo(const StorePath & storePath, Callback> callback) noexcept { + if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { + /* then we should not be checking any experimental realisations + data structures. */ + callback(nullptr); + return; + } try { if (diskCache) { From 9aa33147c19997b71090608744dfe3d1f532aac1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 13 Dec 2025 15:33:00 -0500 Subject: [PATCH 02/10] Remove `BuildTrace` <-> `ValidPaths` foreign key We don't want to only have build traces for objects we have in the store, anymore. --- src/libstore/build/derivation-building-goal.cc | 2 +- src/libstore/ca-specific-schema.sql | 9 ++++----- src/libstore/local-store.cc | 14 ++++++-------- 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 51667557077..ed24a304caf 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1272,7 +1272,7 @@ DerivationBuildingGoal::checkPathValidity(std::map & if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { .path = real->outPath, - .status = PathStatus::Valid, + .status = worker.store.isValidPath(real->outPath) ? PathStatus::Valid : PathStatus::Absent, }; } else if (info.known && info.known->isValid()) { // We know the output because it's a static output of the diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql index 5daeef8c853..931d7ab7d95 100644 --- a/src/libstore/ca-specific-schema.sql +++ b/src/libstore/ca-specific-schema.sql @@ -11,13 +11,12 @@ -- of having many abandoned tables lying around. Closer to the end of -- the experiment, we'll provide guidance on how to clean this up. -create table if not exists BuildTraceV2 ( +create table if not exists BuildTraceV3 ( id integer primary key autoincrement not null, drvPath text not null, outputName text not null, -- symbolic output id, usually "out" - outputPath integer not null, - signatures text, -- space-separated list - foreign key (outputPath) references ValidPaths(id) on delete cascade + outputPath text not null, + signatures text -- space-separated list ); -create index if not exists IndexBuildTraceV2 on BuildTraceV2(drvPath, outputName); +create index if not exists IndexBuildTraceV3 on BuildTraceV3(drvPath, outputName); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ece197def61..95f36160e28 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -360,14 +360,14 @@ LocalStore::LocalStore(ref config) state->stmts->RegisterRealisedOutput.create( state->db, R"( - insert into BuildTraceV2 (drvPath, outputName, outputPath, signatures) - values (?, ?, (select id from ValidPaths where path = ?), ?) + insert into BuildTraceV3 (drvPath, outputName, outputPath, signatures) + values (?, ?, ?, ?) ; )"); state->stmts->UpdateRealisedOutput.create( state->db, R"( - update BuildTraceV2 + update BuildTraceV3 set signatures = ? where drvPath = ? and @@ -377,16 +377,14 @@ LocalStore::LocalStore(ref config) state->stmts->QueryRealisedOutput.create( state->db, R"( - select BuildTraceV2.id, Output.path, BuildTraceV2.signatures from BuildTraceV2 - inner join ValidPaths as Output on Output.id = BuildTraceV2.outputPath + select id, outputPath, signatures from BuildTraceV3 where drvPath = ? and outputName = ? ; )"); state->stmts->QueryAllRealisedOutputs.create( state->db, R"( - select outputName, Output.path from BuildTraceV2 - inner join ValidPaths as Output on Output.id = BuildTraceV2.outputPath + select outputName, outputPath from BuildTraceV3 where drvPath = ? ; )"); @@ -604,7 +602,7 @@ void LocalStore::upgradeDBSchema(State & state) if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) doUpgrade( - "20251016-ca-derivations", + "20251017-ca-derivations", #include "ca-specific-schema.sql.gen.hh" ); } From 9c3eabb87ecd2f0372dbe49be7a83349780fad80 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 13 Dec 2025 14:20:51 -0500 Subject: [PATCH 03/10] Get rid of deep realisations entirely Fix #11896 --- src/libcmd/built-path.cc | 22 ++- src/libcmd/repl.cc | 3 +- .../issue-11928/store-after.json | 30 ++++- .../issue-11928/substituter.json | 25 +++- src/libstore-tests/worker-substitution.cc | 43 +++--- .../build/derivation-building-goal.cc | 3 +- src/libstore/build/derivation-goal.cc | 15 +-- src/libstore/derivations.cc | 2 +- src/libstore/include/nix/store/local-store.hh | 3 + src/libstore/include/nix/store/meson.build | 1 + .../include/nix/store/outputs-query.hh | 76 +++++++++++ src/libstore/include/nix/store/store-api.hh | 15 +++ src/libstore/local-store.cc | 21 +++ src/libstore/meson.build | 1 + src/libstore/misc.cc | 19 +-- src/libstore/outputs-query.cc | 125 ++++++++++++++++++ src/libstore/store-api.cc | 45 +++++-- src/nix/copy.cc | 2 +- src/nix/develop.cc | 3 +- src/nix/nix-build/nix-build.cc | 11 +- src/nix/nix-env/nix-env.cc | 3 +- src/nix/nix-store/nix-store.cc | 3 +- tests/functional/ca/build-cache.sh | 2 +- tests/functional/ca/substitute.sh | 23 ---- 24 files changed, 388 insertions(+), 108 deletions(-) create mode 100644 src/libstore/include/nix/store/outputs-query.hh create mode 100644 src/libstore/outputs-query.cc diff --git a/src/libcmd/built-path.cc b/src/libcmd/built-path.cc index f60e4499c3e..fae65fca2b0 100644 --- a/src/libcmd/built-path.cc +++ b/src/libcmd/built-path.cc @@ -1,6 +1,7 @@ #include "nix/cmd/built-path.hh" #include "nix/store/derivations.hh" #include "nix/store/store-api.hh" +#include "nix/store/outputs-query.hh" #include "nix/util/comparator.hh" #include @@ -109,18 +110,15 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const [&](const BuiltPath::Opaque & p) { res.insert(p.path); }, [&](const BuiltPath::Built & p) { for (auto & [outputName, outputPath] : p.outputs) { - if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { - DrvOutput key{ - .drvPath = p.drvPath->outPath(), - .outputName = outputName, - }; - auto thisRealisation = store.queryRealisation(key); - // We’ve built it, so we must have the realisation. - assert(thisRealisation); - res.insert(Realisation{*thisRealisation, key}); - } else { - res.insert(outputPath); - } + /* Use a custom callback to collect realisations as they're queried. */ + deepQueryPartialDerivationOutput( + store, p.drvPath->outPath(), outputName, nullptr, [&](const DrvOutput & drvOutput) { + auto realisation = store.queryRealisation(drvOutput); + if (realisation) + res.insert(Realisation{*realisation, drvOutput}); + return realisation; + }); + res.insert(outputPath); } }, }, diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 0324d98f040..35538d531ed 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -16,6 +16,7 @@ #include "nix/cmd/get-build-log.hh" #include "nix/expr/get-drvs.hh" #include "nix/store/derivations.hh" +#include "nix/store/outputs-query.hh" #include "nix/store/globals.hh" #include "nix/flake/flake.hh" #include "nix/flake/lockfile.hh" @@ -548,7 +549,7 @@ ProcessLineResult NixRepl::processLine(std::string line) }); auto drv = state->store->readDerivation(drvPath); logger->cout("\nThis derivation produced the following outputs:"); - for (auto & [outputName, outputPath] : state->store->queryDerivationOutputMap(drvPath)) { + for (auto & [outputName, outputPath] : deepQueryDerivationOutputMap(*state->store, drvPath)) { auto localStore = state->store.dynamic_pointer_cast(); if (localStore && command == ":bl") { std::string symlink = "repl-result-" + outputName; diff --git a/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json index c4d3901236c..fc469c1de11 100644 --- a/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json +++ b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json @@ -1,10 +1,16 @@ { "buildTrace": { - "11yvkl84ashq63ilwc2mi4va41z2disw-root-drv.drv": { + "am42vl3rp8w29f5hyp9ljckxll50icpl-root-drv.drv": { "out": { "outPath": "px7apdw6ydm9ynjy5g0bpdcylw3xz2kj-root-drv-out", "signatures": [] } + }, + "vy7j6m6p5y0327fhk3zxn12hbpzkh6lp-dep-drv.drv": { + "out": { + "outPath": "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out", + "signatures": [] + } } }, "config": { @@ -32,6 +38,28 @@ "ultimate": false, "version": 2 } + }, + "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out": { + "contents": { + "contents": "I am the dependency output", + "executable": false, + "type": "regular" + }, + "info": { + "ca": { + "hash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", + "method": "nar" + }, + "deriver": null, + "narHash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", + "narSize": 144, + "references": [], + "registrationTime": null, + "signatures": [], + "storeDir": "/nix/store", + "ultimate": false, + "version": 2 + } } }, "derivations": { diff --git a/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json b/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json index ecd22821469..f2007d3075d 100644 --- a/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json +++ b/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json @@ -1,6 +1,6 @@ { "buildTrace": { - "11yvkl84ashq63ilwc2mi4va41z2disw-root-drv.drv": { + "am42vl3rp8w29f5hyp9ljckxll50icpl-root-drv.drv": { "out": { "outPath": "px7apdw6ydm9ynjy5g0bpdcylw3xz2kj-root-drv-out", "signatures": [] @@ -62,5 +62,26 @@ } } }, - "derivations": {} + "derivations": { + "am42vl3rp8w29f5hyp9ljckxll50icpl-root-drv.drv": { + "args": [], + "builder": "", + "env": {}, + "inputs": { + "drvs": {}, + "srcs": [ + "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out" + ] + }, + "name": "root-drv", + "outputs": { + "out": { + "hashAlgo": "sha256", + "method": "nar" + } + }, + "system": "", + "version": 4 + } + } } diff --git a/src/libstore-tests/worker-substitution.cc b/src/libstore-tests/worker-substitution.cc index 9cc7b56dcb8..7be88aaba74 100644 --- a/src/libstore-tests/worker-substitution.cc +++ b/src/libstore-tests/worker-substitution.cc @@ -339,6 +339,19 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) // Write the root derivation to the destination store auto rootDrvPath = dummyStore->writeDerivation(rootDrv); + // Resolve the root derivation using a callback that returns the dep output path + auto resolvedRootDrv = rootDrv.tryResolve( + *substituter, + [&](ref drvPath, const std::string & outputName) -> std::optional { + EXPECT_EQ(*drvPath, SingleDerivedPath::Opaque{depDrvPath}); + EXPECT_EQ(outputName, "out"); + return depOutputPath; + }); + ASSERT_TRUE(resolvedRootDrv); + + // Write the resolved derivation to the substituter + auto resolvedRootDrvPath = substituter->writeDerivation(Derivation{*resolvedRootDrv}); + // Snapshot the destination store before checkpointJson("issue-11928/store-before", dummyStore); @@ -363,11 +376,12 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) // The DrvOutputs for both derivations DrvOutput depDrvOutput{depDrvPath, "out"}; - DrvOutput rootDrvOutput{rootDrvPath, "out"}; + DrvOutput resolvedRootDrvOutput{resolvedRootDrvPath, "out"}; - // Add the realisation for the root derivation to the substituter + // Add the realisation for the *resolved* root derivation to the substituter + // (not the original root derivation - that would be an illegal "deep" realisation) substituter->buildTrace.insert_or_assign( - rootDrvPath, + resolvedRootDrvPath, std::map{ { "out", @@ -378,12 +392,12 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) }); // Snapshot the substituter - // Note: it has realisations for both drvs, but only the root's output store object + // Note: it has the dep realisation, resolved root drv, resolved root realisation, and root output checkpointJson("issue-11928/substituter", substituter); // The realisations should not exist in the destination store yet ASSERT_FALSE(dummyStore->queryRealisation(depDrvOutput)); - ASSERT_FALSE(dummyStore->queryRealisation(rootDrvOutput)); + ASSERT_FALSE(dummyStore->queryRealisation(resolvedRootDrvOutput)); // Create a worker with our custom substituter Worker worker{*dummyStore, *dummyStore}; @@ -407,22 +421,19 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) // The root output path should now exist in the destination store ASSERT_TRUE(dummyStore->isValidPath(rootOutputPath)); - // The root realisation should now exist in the destination store - auto rootRealisation = dummyStore->queryRealisation(rootDrvOutput); + // The resolved root realisation should now exist in the destination store + auto rootRealisation = dummyStore->queryRealisation(resolvedRootDrvOutput); ASSERT_TRUE(rootRealisation); ASSERT_EQ(rootRealisation->outPath, rootOutputPath); - // #11928: The dependency's REALISATION should be fetched, because - // it is needed to resolve the underlying derivation. Currently the - // realisation is not fetched (bug). Once fixed: Change - // depRealisation ASSERT_FALSE to ASSERT_TRUE and uncomment the - // ASSERT_EQ + // The dependency's REALISATION should have been fetched auto depRealisation = dummyStore->queryRealisation(depDrvOutput); - ASSERT_FALSE(depRealisation); - // ASSERT_EQ(depRealisation->outPath, depOutputPath); + ASSERT_TRUE(depRealisation); + ASSERT_EQ(depRealisation->outPath, depOutputPath); - // The dependency's OUTPUT is correctly not fetched (not referenced by root output) - ASSERT_FALSE(dummyStore->isValidPath(depOutputPath)); + // TODO #11928: The dependency's OUTPUT should NOT be fetched (not referenced + // by root output). Once #11928 is fixed, change ASSERT_TRUE to ASSERT_FALSE. + ASSERT_TRUE(dummyStore->isValidPath(depOutputPath)); // Verify the goal succeeded ASSERT_EQ(upcast_goal(goal)->exitCode, Goal::ecSuccess); diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index ed24a304caf..a118144b179 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -13,6 +13,7 @@ #include "nix/store/common-protocol.hh" #include "nix/store/common-protocol-impl.hh" #include "nix/store/local-store.hh" // TODO remove, along with remaining downcasts +#include "nix/store/outputs-query.hh" #include "nix/store/globals.hh" #include @@ -142,7 +143,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) auto outMap = [&] { for (auto * drvStore : {&worker.evalStore, &worker.store}) if (drvStore->isValidPath(depDrvPath)) - return worker.store.queryDerivationOutputMap(depDrvPath, drvStore); + return deepQueryDerivationOutputMap(worker.store, depDrvPath, drvStore); assert(false); }(); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 4a726e29503..e824dfdc9ca 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -13,6 +13,7 @@ #include "nix/util/compression.hh" #include "nix/store/common-protocol.hh" #include "nix/store/common-protocol-impl.hh" // Don't remove is actually needed +#include "nix/store/outputs-query.hh" #include "nix/store/globals.hh" #include @@ -209,18 +210,6 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) wantedOutput); }(); - if (!drv->type().isImpure()) { - Realisation newRealisation{ - realisation, - { - .drvPath = drvPath, - .outputName = wantedOutput, - }}; - newRealisation.signatures.clear(); - worker.store.signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - } - auto status = success.status; if (status == BuildResult::Success::AlreadyValid) status = BuildResult::Success::ResolvesToAlreadyValid; @@ -299,7 +288,7 @@ Goal::Co DerivationGoal::repairClosure() auto outputs = [&] { for (auto * drvStore : {&worker.evalStore, &worker.store}) if (drvStore->isValidPath(drvPath)) - return worker.store.queryDerivationOutputMap(drvPath, drvStore); + return deepQueryDerivationOutputMap(worker.store, drvPath, drvStore); OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 63cbd2d36af..21a6b4aa13e 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -1159,7 +1159,7 @@ std::optional Derivation::tryResolve(Store & store, Store * eva } static bool tryResolveInput( - Store & store, + const StoreDirConfig & store, StorePathSet & inputSrcs, StringMap & inputRewrites, const DownstreamPlaceholder * placeholderOpt, diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 411ff6a3cf5..40ec438ef97 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -274,6 +274,9 @@ public: std::map> queryStaticPartialDerivationOutputMap(const StorePath & path) override; + std::optional + queryStaticPartialDerivationOutput(const StorePath & path, const std::string & outputName) override; + std::optional queryPathFromHashPart(const std::string & hashPart) override; bool pathInfoIsUntrusted(const ValidPathInfo &) override; diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index 93be5921abc..7db3d2c07e8 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -61,6 +61,7 @@ headers = [ config_pub_h ] + files( 'names.hh', 'nar-info-disk-cache.hh', 'nar-info.hh', + 'outputs-query.hh', 'outputs-spec.hh', 'parsed-derivations.hh', 'path-info.hh', diff --git a/src/libstore/include/nix/store/outputs-query.hh b/src/libstore/include/nix/store/outputs-query.hh new file mode 100644 index 00000000000..04dacd78f4b --- /dev/null +++ b/src/libstore/include/nix/store/outputs-query.hh @@ -0,0 +1,76 @@ +#pragma once +///@file + +#include "nix/store/store-api.hh" + +#include + +namespace nix { + +/** + * Callback type for querying realisations. The callback should return + * the realisation for the given DrvOutput, or nullptr if not found. + */ +using QueryRealisationFun = std::function(const DrvOutput &)>; + +/** + * For internal use only. + */ +void queryPartialDerivationOutputMapCA( + Store & store, + const StorePath & drvPath, + const BasicDerivation & drv, + std::map> & outputs, + QueryRealisationFun queryRealisation = {}); + +struct DeepDerivationOutputResult +{ + /** + * The output path, if known. + */ + std::optional outPath; + + /** + * The resolved derivation path. For non-CA derivations or + * derivations that don't need resolution, this equals the + * original drvPath. + */ + StorePath resolvedDrvPath; +}; + +/** + * Like Store::queryStaticPartialDerivationOutput, but resolves the + * derivation first if needed. Returns both the output path and + * the resolved derivation path. + * + * @param queryRealisation Optional callback for querying realisations. + * If not provided, uses store.queryRealisation(). + */ +DeepDerivationOutputResult deepQueryPartialDerivationOutput( + Store & store, + const StorePath & drvPath, + const std::string & outputName, + Store * evalStore = nullptr, + QueryRealisationFun queryRealisation = {}); + +/** + * Like Store::queryStaticPartialDerivationOutputMap, but resolves the + * derivation first if needed for CA derivation output lookup. + * + * @param queryRealisation Optional callback for querying realisations. + * If not provided, uses store.queryRealisation(). + */ +std::map> deepQueryPartialDerivationOutputMap( + Store & store, const StorePath & drvPath, Store * evalStore = nullptr, QueryRealisationFun queryRealisation = {}); + +/** + * Like deepQueryPartialDerivationOutputMap, but throws if any output + * path is not known. + * + * @param queryRealisation Optional callback for querying realisations. + * If not provided, uses store.queryRealisation(). + */ +OutputPathMap deepQueryDerivationOutputMap( + Store & store, const StorePath & drvPath, Store * evalStore = nullptr, QueryRealisationFun queryRealisation = {}); + +} // namespace nix diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 890f7c075ee..4214bcd38bc 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -478,6 +478,15 @@ public: virtual std::map> queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr); + /** + * Like the above, but takes the derivation directly. + * + * @note For CA derivations, the derivation should already be + * resolved, or output path lookup will not work. + */ + std::map> + queryPartialDerivationOutputMap(const Derivation & drv, Store * evalStore = nullptr); + /** * Like `queryPartialDerivationOutputMap` but only considers * statically known output paths (i.e. those that can be gotten from @@ -489,6 +498,12 @@ public: virtual std::map> queryStaticPartialDerivationOutputMap(const StorePath & path); + /** + * Like the above, but for a single output. + */ + virtual std::optional + queryStaticPartialDerivationOutput(const StorePath & path, const std::string & outputName); + /** * Query the mapping outputName=>outputPath for the given derivation. * Assume every output has a mapping and throw an exception otherwise. diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 95f36160e28..9f47da5b20f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -875,6 +875,27 @@ LocalStore::queryStaticPartialDerivationOutputMap(const StorePath & path) }); } +std::optional +LocalStore::queryStaticPartialDerivationOutput(const StorePath & path, const std::string & outputName) +{ + auto outputs = queryStaticPartialDerivationOutputMap(path); + auto it = outputs.find(outputName); + if (it == outputs.end()) { + /* Only throw if CA derivations is disabled, because then the + SQL table is complete. + + With CA derivations enabled, derivations without static + outputs exist, this absence of a row in this table does not + mean the derivation doesn't have an output necessarily, just + that that it doesn't have an output with a known output path. + */ + if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) + throw Error("derivation '%s' does not have an output named '%s'", printStorePath(path), outputName); + return std::nullopt; + } + return it->second; +} + std::optional LocalStore::queryPathFromHashPart(const std::string & hashPart) { if (hashPart.size() != StorePath::HashLen) diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 4d9b22f56f9..d6cfb8c7f36 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -324,6 +324,7 @@ sources = files( 'nar-info-disk-cache.cc', 'nar-info.cc', 'optimise-store.cc', + 'outputs-query.cc', 'outputs-spec.cc', 'parsed-derivations.cc', 'path-info.cc', diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index f8f94070e48..0deee9ae853 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -1,4 +1,5 @@ #include "nix/store/derivations.hh" +#include "nix/store/outputs-query.hh" #include "nix/store/parsed-derivations.hh" #include "nix/store/derivation-options.hh" #include "nix/store/globals.hh" @@ -339,7 +340,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, { auto drvPath = resolveDerivedPath(store, *bfd.drvPath, evalStore_); - auto outputsOpt_ = store.queryPartialDerivationOutputMap(drvPath, evalStore_); + auto outputsOpt_ = deepQueryPartialDerivationOutputMap(store, drvPath, evalStore_); auto outputsOpt = std::visit( overloaded{ @@ -376,23 +377,15 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, StorePath resolveDerivedPath(Store & store, const SingleDerivedPath & req, Store * evalStore_) { - auto & evalStore = evalStore_ ? *evalStore_ : store; - return std::visit( overloaded{ [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, [&](const SingleDerivedPath::Built & bfd) { auto drvPath = resolveDerivedPath(store, *bfd.drvPath, evalStore_); - auto outputPaths = evalStore.queryPartialDerivationOutputMap(drvPath, evalStore_); - if (outputPaths.count(bfd.output) == 0) - throw Error( - "derivation '%s' does not have an output named '%s'", - store.printStorePath(drvPath), - bfd.output); - auto & optPath = outputPaths.at(bfd.output); - if (!optPath) + auto result = deepQueryPartialDerivationOutput(store, drvPath, bfd.output, evalStore_); + if (!result.outPath) throw MissingRealisation(store, *bfd.drvPath, drvPath, bfd.output); - return *optPath; + return *result.outPath; }, }, req.raw()); @@ -401,7 +394,7 @@ StorePath resolveDerivedPath(Store & store, const SingleDerivedPath & req, Store OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd) { auto drvPath = resolveDerivedPath(store, *bfd.drvPath); - auto outputMap = store.queryDerivationOutputMap(drvPath); + auto outputMap = deepQueryDerivationOutputMap(store, drvPath); auto outputsLeft = std::visit( overloaded{ [&](const OutputsSpec::All &) { return StringSet{}; }, diff --git a/src/libstore/outputs-query.cc b/src/libstore/outputs-query.cc new file mode 100644 index 00000000000..083f1c9ca21 --- /dev/null +++ b/src/libstore/outputs-query.cc @@ -0,0 +1,125 @@ +#include "nix/store/outputs-query.hh" +#include "nix/store/derivations.hh" +#include "nix/store/realisation.hh" + +namespace nix { + +/** + * Resolve a derivation and compute its store path. + * + * @param queryRealisation must already be initialized (not empty) + */ +static std::pair resolveDerivation( + Store & store, const StorePath & drvPath, Store * evalStore_, QueryRealisationFun & queryRealisation) +{ + auto & evalStore = evalStore_ ? *evalStore_ : store; + + Derivation drv = evalStore.readInvalidDerivation(drvPath); + if (drv.shouldResolve()) { + /* Without a custom queryRealisation, we could just use + drv.tryResolve(store, &evalStore). But we need to use the + callback variant to ensure all realisation queries go + through queryRealisation. */ + auto resolvedDrv = drv.tryResolve( + store, + [&](ref depDrvPath, const std::string & depOutputName) + -> std::optional { + auto * opaque = std::get_if(&depDrvPath->raw()); + if (!opaque) + return std::nullopt; + return deepQueryPartialDerivationOutput( + store, opaque->path, depOutputName, evalStore_, queryRealisation) + .outPath; + }); + if (resolvedDrv) + drv = Derivation{*resolvedDrv}; + } + + auto resolvedDrvPath = computeStorePath(store, drv); + return {std::move(drv), std::move(resolvedDrvPath)}; +} + +void queryPartialDerivationOutputMapCA( + Store & store, + const StorePath & drvPath, + const BasicDerivation & drv, + std::map> & outputs, + QueryRealisationFun queryRealisation) +{ + if (!queryRealisation) + queryRealisation = [&store](const DrvOutput & o) { return store.queryRealisation(o); }; + + for (auto & [outputName, _] : drv.outputs) { + auto realisation = queryRealisation(DrvOutput{drvPath, outputName}); + if (realisation) { + outputs.insert_or_assign(outputName, realisation->outPath); + } else { + outputs.insert({outputName, std::nullopt}); + } + } +} + +std::map> deepQueryPartialDerivationOutputMap( + Store & store, const StorePath & drvPath, Store * evalStore_, QueryRealisationFun queryRealisation) +{ + auto & evalStore = evalStore_ ? *evalStore_ : store; + + if (!queryRealisation) + queryRealisation = [&store](const DrvOutput & o) { return store.queryRealisation(o); }; + + auto outputs = evalStore.queryStaticPartialDerivationOutputMap(drvPath); + + if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) + return outputs; + + auto [drv, resolvedDrvPath] = resolveDerivation(store, drvPath, evalStore_, queryRealisation); + queryPartialDerivationOutputMapCA(store, resolvedDrvPath, drv, outputs, queryRealisation); + + return outputs; +} + +OutputPathMap deepQueryDerivationOutputMap( + Store & store, const StorePath & drvPath, Store * evalStore, QueryRealisationFun queryRealisation) +{ + auto resp = deepQueryPartialDerivationOutputMap(store, drvPath, evalStore, std::move(queryRealisation)); + OutputPathMap result; + for (auto & [outName, optOutPath] : resp) { + if (!optOutPath) + throw MissingRealisation(store, drvPath, outName); + result.insert_or_assign(outName, *optOutPath); + } + return result; +} + +DeepDerivationOutputResult deepQueryPartialDerivationOutput( + Store & store, + const StorePath & drvPath, + const std::string & outputName, + Store * evalStore_, + QueryRealisationFun queryRealisation) +{ + auto & evalStore = evalStore_ ? *evalStore_ : store; + + if (!queryRealisation) + queryRealisation = [&store](const DrvOutput & o) { return store.queryRealisation(o); }; + + auto staticResult = evalStore.queryStaticPartialDerivationOutput(drvPath, outputName); + if (staticResult || !experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) + return { + .outPath = staticResult, + .resolvedDrvPath = drvPath, + }; + + auto [drv, resolvedDrvPath] = resolveDerivation(store, drvPath, evalStore_, queryRealisation); + + if (drv.outputs.count(outputName) == 0) + throw Error("derivation '%s' does not have an output named '%s'", store.printStorePath(drvPath), outputName); + + auto realisation = queryRealisation(DrvOutput{resolvedDrvPath, outputName}); + return { + .outPath = realisation ? std::optional{realisation->outPath} : std::nullopt, + .resolvedDrvPath = resolvedDrvPath, + }; +} + +} // namespace nix diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index baddfb4ab20..b1f7cdf6321 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -7,6 +7,7 @@ #include "nix/store/derivations.hh" #include "nix/store/store-api.hh" #include "nix/store/store-open.hh" +#include "nix/store/outputs-query.hh" #include "nix/util/util.hh" #include "nix/store/nar-info-disk-cache.hh" #include "nix/util/thread-pool.hh" @@ -369,6 +370,17 @@ std::map> Store::queryStaticPartialDerivat return outputs; } +std::optional +Store::queryStaticPartialDerivationOutput(const StorePath & path, const std::string & outputName) +{ + auto drv = readInvalidDerivation(path); + auto outputs = drv.outputsAndOptPaths(*this); + auto it = outputs.find(outputName); + if (it == outputs.end()) + throw Error("derivation '%s' does not have an output named '%s'", printStorePath(path), outputName); + return it->second.second; +} + std::map> Store::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore_) { @@ -380,18 +392,25 @@ Store::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore return outputs; auto drv = evalStore.readInvalidDerivation(path); - for (auto & [outputName, _] : drv.outputs) { - auto realisation = queryRealisation(DrvOutput{path, outputName}); - if (realisation) { - outputs.insert_or_assign(outputName, realisation->outPath); - } else { - // queryStaticPartialDerivationOutputMap is not guaranteed - // to return std::nullopt for outputs which are not - // statically known. - outputs.insert({outputName, std::nullopt}); - } + queryPartialDerivationOutputMapCA(*this, path, drv, outputs); + + return outputs; +} + +std::map> +Store::queryPartialDerivationOutputMap(const Derivation & drv, Store * evalStore_) +{ + std::map> outputs; + for (auto & [outputName, output] : drv.outputsAndOptPaths(*this)) { + outputs.emplace(outputName, output.second); } + if (!experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) + return outputs; + + auto drvPath = nix::computeStorePath(*this, drv); + queryPartialDerivationOutputMapCA(*this, drvPath, drv, outputs); + return outputs; } @@ -409,7 +428,7 @@ OutputPathMap Store::queryDerivationOutputMap(const StorePath & path, Store * ev StorePathSet Store::queryDerivationOutputs(const StorePath & path) { - auto outputMap = this->queryDerivationOutputMap(path); + auto outputMap = nix::deepQueryDerivationOutputMap(*this, path); StorePathSet outputPaths; for (auto & i : outputMap) { outputPaths.emplace(std::move(i.second)); @@ -1096,9 +1115,7 @@ void copyClosure( StorePathSet closure0; for (auto & path : paths) { - if (auto * opaquePath = std::get_if(&path.raw)) { - closure0.insert(opaquePath->path); - } + closure0.insert(path.path()); } StorePathSet closure1; diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 86306e7fdb8..841218a0de5 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -61,7 +61,7 @@ struct CmdCopy : virtual CopyCommand, virtual BuiltPathsCommand, MixProfile, Mix stuffToCopy.insert(theseRealisations.begin(), theseRealisations.end()); } - copyPaths(*srcStore, *dstStore, stuffToCopy, NoRepair, checkSigs, substitute); + copyClosure(*srcStore, *dstStore, stuffToCopy, NoRepair, checkSigs, substitute); updateProfile(*dstStore, rootPaths); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 09fd62b7efa..d9ebeda951b 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -7,6 +7,7 @@ #include "nix/store/store-api.hh" #include "nix/store/globals.hh" #include "nix/store/outputs-spec.hh" +#include "nix/store/outputs-query.hh" #include "nix/store/derivations.hh" #ifndef _WIN32 // TODO re-enable on Windows @@ -304,7 +305,7 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore // `get-env.sh` will write its JSON output to an arbitrary output // path, so return the first non-empty output path. - for (auto & [_0, optPath] : evalStore->queryPartialDerivationOutputMap(shellDrvPath)) { + for (auto & [_0, optPath] : deepQueryPartialDerivationOutputMap(*evalStore, shellDrvPath)) { assert(optPath); auto accessor = evalStore->requireStoreObjectAccessor(*optPath); if (auto st = accessor->maybeLstat(CanonPath::root); st && st->fileSize.value_or(0)) diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index 13c38f9298a..9bf1df178fc 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -17,6 +17,7 @@ #include "nix/store/globals.hh" #include "nix/store/realisation.hh" #include "nix/store/derivations.hh" +#include "nix/store/outputs-query.hh" #include "nix/main/shared.hh" #include "nix/store/path-with-outputs.hh" #include "nix/expr/eval.hh" @@ -527,7 +528,7 @@ static void main_nix_build(int argc, char ** argv) return; if (shellDrv) { - auto shellDrvOutputs = store->queryPartialDerivationOutputMap(shellDrv.value(), &*evalStore); + auto shellDrvOutputs = deepQueryPartialDerivationOutputMap(*store, shellDrv.value(), &*evalStore); shell = store->printStorePath(shellDrvOutputs.at("out").value()) + "/bin/bash"; } @@ -720,11 +721,9 @@ static void main_nix_build(int argc, char ** argv) if (counter) drvPrefix += fmt("-%d", counter + 1); - auto builtOutputs = store->queryPartialDerivationOutputMap(drvPath, &*evalStore); - - auto maybeOutputPath = builtOutputs.at(outputName); - assert(maybeOutputPath); - auto outputPath = *maybeOutputPath; + auto result = deepQueryPartialDerivationOutput(*store, drvPath, outputName, &*evalStore); + assert(result.outPath); + auto outputPath = *result.outPath; if (auto store2 = store.dynamic_pointer_cast()) { std::string symlink = drvPrefix; diff --git a/src/nix/nix-env/nix-env.cc b/src/nix/nix-env/nix-env.cc index 18bb2c74a03..67e6c35f793 100644 --- a/src/nix/nix-env/nix-env.cc +++ b/src/nix/nix-env/nix-env.cc @@ -2,6 +2,7 @@ #include "nix/expr/attr-path.hh" #include "nix/cmd/common-eval-args.hh" #include "nix/store/derivations.hh" +#include "nix/store/outputs-query.hh" #include "nix/expr/eval.hh" #include "nix/expr/get-drvs.hh" #include "nix/store/globals.hh" @@ -457,7 +458,7 @@ static void queryInstSources( if (path.isDerivation()) { elem.setDrvPath(path); - auto outputs = state.store->queryDerivationOutputMap(path); + auto outputs = deepQueryDerivationOutputMap(*state.store, path); elem.setOutPath(outputs.at("out")); if (name.size() >= drvExtension.size() && std::string(name, name.size() - drvExtension.size()) == drvExtension) diff --git a/src/nix/nix-store/nix-store.cc b/src/nix/nix-store/nix-store.cc index 45fb8e58177..0888ea86c7e 100644 --- a/src/nix/nix-store/nix-store.cc +++ b/src/nix/nix-store/nix-store.cc @@ -1,5 +1,6 @@ #include "nix/util/archive.hh" #include "nix/store/derivations.hh" +#include "nix/store/outputs-query.hh" #include "dotgraph.hh" #include "nix/store/globals.hh" #include "nix/store/store-open.hh" @@ -78,7 +79,7 @@ static PathSet realisePath(StorePathWithOutputs path, bool build = true) if (path.path.isDerivation()) { if (build) store->buildPaths({path.toDerivedPath()}); - auto outputPaths = store->queryDerivationOutputMap(path.path); + auto outputPaths = deepQueryDerivationOutputMap(*store, path.path); Derivation drv = store->derivationFromPath(path.path); rootNr++; diff --git a/tests/functional/ca/build-cache.sh b/tests/functional/ca/build-cache.sh index 5cc71823ee6..2ec1637c8f4 100644 --- a/tests/functional/ca/build-cache.sh +++ b/tests/functional/ca/build-cache.sh @@ -36,7 +36,7 @@ testRemoteCacheFor () { copyAttr "$derivationPath" 1 clearStore # Check nothing gets built. - buildAttr "$derivationPath" 1 --option substituters "file://$cacheDir" --no-require-sigs |& grepQuietInverse " will be built:" + buildAttr "$derivationPath" 1 --option substituters "file://$cacheDir" --no-require-sigs -j0 } testRemoteCache () { diff --git a/tests/functional/ca/substitute.sh b/tests/functional/ca/substitute.sh index 022bea5cfce..533df8b412f 100644 --- a/tests/functional/ca/substitute.sh +++ b/tests/functional/ca/substitute.sh @@ -38,29 +38,6 @@ else pushToStore="../push-to-store-old.sh" fi -# Same thing, but -# 1. With non-ca derivations -# 2. Erasing the realisations on the remote store -# -# Even in that case, realising the derivations should still produce the right -# realisations on the local store -# -# Regression test for #4725 -clearStore -nix build --file ../simple.nix -L --no-link --post-build-hook "$pushToStore" -clearStore -rm -r "$REMOTE_STORE_DIR/build-trace-v2" -nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 -# There's no easy way to check whether a realisation is present on the local -# store − short of manually querying the db, but the build environment doesn't -# have the sqlite binary − so we instead push things again, and check that the -# realisations have correctly been pushed to the remote store -nix copy --to "$REMOTE_STORE" --file ../simple.nix -if [[ -z "$(ls "$REMOTE_STORE_DIR/build-trace-v2")" ]]; then - echo "Realisations not rebuilt" - exit 1 -fi - # Test the local realisation disk cache buildDrvs --post-build-hook "$pushToStore" clearStore From 0cb5d86faa99cc79181f8f93dcd9d0f764c91dde Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 14:06:35 -0500 Subject: [PATCH 04/10] `DerivationBuildingGoal`: Unify input substitution and closure We don't need to treat `inputSrcs` and `inputDrvs` separately as much. --- .../build/derivation-building-goal.cc | 75 ++++++++++--------- 1 file changed, 40 insertions(+), 35 deletions(-) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index a118144b179..d35530e2123 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -91,6 +91,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) copyClosure(worker.evalStore, worker.store, inputSrcs); } + StorePathSet inputs = drv->inputSrcs; + + /* Substitute any input sources that aren't valid. */ for (auto & i : drv->inputSrcs) { if (worker.store.isValidPath(i)) continue; @@ -102,6 +105,41 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) waitees.insert(upcast_goal(worker.makePathSubstitutionGoal(i))); } + /* If we get this far, we know no dynamic drvs inputs */ + + /* Build any input derivation outputs that aren't valid. */ + for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) { + for (auto & outputName : depNode.value) { + auto outMap = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(depDrvPath)) + return deepQueryDerivationOutputMap(worker.store, depDrvPath, drvStore); + assert(false); + }(); + + auto outMapPath = outMap.find(outputName); + if (outMapPath == outMap.end()) { + throw Error( + "derivation '%s' requires non-existent output '%s' from input derivation '%s'", + worker.store.printStorePath(drvPath), + outputName, + worker.store.printStorePath(depDrvPath)); + } + + auto outputPath = outMapPath->second; + inputs.insert(outputPath); + + if (!worker.store.isValidPath(outputPath)) { + waitees.insert(worker.makeGoal( + DerivedPath::Built{ + .drvPath = makeConstantStorePathRef(depDrvPath), + .outputs = OutputsSpec::Names{outputName}, + }, + buildMode)); + } + } + } + co_await await(std::move(waitees)); trace("all inputs realised"); @@ -120,8 +158,6 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) /* Gather information necessary for computing the closure and/or running the build hook. */ - /* Determine the full set of input paths. */ - if (storeDerivation) { assert(drv->inputDrvs.map.empty()); /* Store the resolved derivation, as part of the record of @@ -129,40 +165,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) worker.store.writeDerivation(*drv); } + /* Determine the full set of input paths. */ StorePathSet inputPaths; - - { - /* If we get this far, we know no dynamic drvs inputs */ - - for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) { - for (auto & outputName : depNode.value) { - /* Don't need to worry about `inputGoals`, because - impure derivations are always resolved above. Can - just use DB. This case only happens in the (older) - input addressed and fixed output derivation cases. */ - auto outMap = [&] { - for (auto * drvStore : {&worker.evalStore, &worker.store}) - if (drvStore->isValidPath(depDrvPath)) - return deepQueryDerivationOutputMap(worker.store, depDrvPath, drvStore); - assert(false); - }(); - - auto outMapPath = outMap.find(outputName); - if (outMapPath == outMap.end()) { - throw Error( - "derivation '%s' requires non-existent output '%s' from input derivation '%s'", - worker.store.printStorePath(drvPath), - outputName, - worker.store.printStorePath(depDrvPath)); - } - - worker.store.computeFSClosure(outMapPath->second, inputPaths); - } - } - } - - /* Second, the input sources. */ - worker.store.computeFSClosure(drv->inputSrcs, inputPaths); + worker.store.computeFSClosure(inputs, inputPaths); debug("added input paths %s", worker.store.showPaths(inputPaths)); From 4caa8de0d69e37249742cae13b34c6f7385ea286 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 15 Feb 2025 13:34:38 -0500 Subject: [PATCH 05/10] Add goal to support recursive build trace queries Progress on #11928 --- .../build/build-trace-trampoline-goal.cc | 199 ++++++++++++++++++ src/libstore/build/derivation-goal.cc | 7 +- .../build/drv-output-substitution-goal.cc | 4 +- src/libstore/build/goal.cc | 2 + src/libstore/build/worker.cc | 26 +++ src/libstore/derived-path-map.cc | 2 + .../build/build-trace-trampoline-goal.hh | 50 +++++ .../build/drv-output-substitution-goal.hh | 12 +- .../include/nix/store/build/worker.hh | 15 +- src/libstore/include/nix/store/meson.build | 1 + src/libstore/meson.build | 1 + 11 files changed, 307 insertions(+), 12 deletions(-) create mode 100644 src/libstore/build/build-trace-trampoline-goal.cc create mode 100644 src/libstore/include/nix/store/build/build-trace-trampoline-goal.hh diff --git a/src/libstore/build/build-trace-trampoline-goal.cc b/src/libstore/build/build-trace-trampoline-goal.cc new file mode 100644 index 00000000000..ba50e620d67 --- /dev/null +++ b/src/libstore/build/build-trace-trampoline-goal.cc @@ -0,0 +1,199 @@ +#include "nix/store/build/build-trace-trampoline-goal.hh" +#include "nix/store/build/drv-output-substitution-goal.hh" +#include "nix/store/build/derivation-resolution-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" +#include "nix/util/util.hh" + +namespace nix { + +BuildTraceTrampolineGoal::BuildTraceTrampolineGoal(const SingleDerivedPath::Built & id, Worker & worker) + : Goal{worker, init()} + , id{id} +{ + name = fmt("resolving build trace for '%s'", id.to_string(worker.store)); + trace("created"); +} + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit( + overloaded{ + [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, + [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); }, + }, + req.raw()); +} + +std::string BuildTraceTrampolineGoal::key() +{ + return "bt$" + std::string(pathPartOfReq(*id.drvPath).name()) + "$" + id.to_string(worker.store); +} + +Goal::Co BuildTraceTrampolineGoal::init() +{ + trace("init"); + + DrvOutput id2{ + .drvPath = StorePath::dummy, + .outputName = id.output, + }; + + // No `std::visit` with coroutines :( + if (const auto * path = std::get_if(&*id.drvPath)) { + // At least we know the drv path statically, can proceed + id2.drvPath = path->path; + } else if (const auto * outputDeriving = std::get_if(&*id.drvPath)) { + // Dynamic derivation case, need to resolve that first. + trace("need to resolve dynamic derivation first"); + + auto g = worker.makeBuildTraceTrampolineGoal({ + outputDeriving->drvPath, + outputDeriving->output, + }); + + co_await await(Goals{upcast_goal(g)}); + + if (nrFailed > 0) { + co_return amDone(nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed); + } + + id2.drvPath = g->outputInfo->outPath; + } + + trace("have concrete drv path"); + + /* If the realisation already exists, we're done */ + if ((outputInfo = worker.store.queryRealisation(id2))) { + co_return amDone(ecSuccess); + } + + /** + * Firstly, whether we know the status, secondly, what it is + */ + std::optional drvIsResolved; + + std::optional drvOpt; + + if (worker.evalStore.isValidPath(id2.drvPath)) { + drvOpt = worker.evalStore.readDerivation(id2.drvPath); + } else if (worker.store.isValidPath(id2.drvPath)) { + drvOpt = worker.store.readDerivation(id2.drvPath); + } + + /* If we have the derivation, and the derivation has statically-known output paths */ + if (drvOpt) { + auto & drv = *drvOpt; + auto os = drv.outputsAndOptPaths(worker.store); + /* Mark what we now know */ + drvIsResolved = {drv.inputDrvs.map.empty()}; + if (auto * p = get(os, id2.outputName)) { + if (auto & outPath = p->second) { + outputInfo = std::make_shared(*outPath); + co_return amDone(ecSuccess); + } else { + /* Otherwise, not failure, just looking up build trace below. */ + } + } else { + co_return amDone(ecFailed); + } + } + + bool substituterFailed = false; + + if (!drvIsResolved || *drvIsResolved) { + /* Since derivation might be resolved --- isn't known to be + not-resolved, it might have entries. So, let's try querying + the substituters. */ + auto g = worker.makeDrvOutputSubstitutionGoal(id2); + + co_await await(Goals{upcast_goal(g)}); + + if (g->exitCode == ecSuccess) { + outputInfo = g->outputInfo; + co_return amDone(ecSuccess); + } + + if (g->exitCode == ecFailed) { + substituterFailed = true; + } + + /* If ecNoSubstituters or ecFailed, fall through to try resolution */ + } + + if (drvIsResolved && *drvIsResolved) { + /* Derivation is already resolved, no point trying to resolve it */ + co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + } + + /* Derivation might not be resolved, let's try doing that */ + trace("trying resolving derivation in build-trace goal"); + + if (!drvOpt) { + /* Derivation not available locally, can't try resolution. + Let the caller fall back to building. */ + co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + } + + auto g = worker.makeDerivationResolutionGoal(id2.drvPath, *drvOpt, bmNormal); + + co_await await(Goals{g}); + + if (nrFailed > 0) { + /* None left. Terminate this goal and let someone else deal + with it. */ + debug( + "derivation output '%s' is required, but there is no substituter that can provide it", + id2.render(worker.store)); + + if (substituterFailed) { + worker.failedSubstitutions++; + worker.updateProgress(); + } + + /* Hack: don't indicate failure if there were no substituters. + In that case the calling derivation should just do a + build. */ + co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + } + + /* This should be set if the goal succeeded */ + assert(g->resolvedDrv); + + if (g->resolvedDrv->first != id2.drvPath) { + /* Try again with the resolved derivation. Since we know it's + resolved, we can go straight to DrvOutputSubstitutionGoal. */ + DrvOutput convergentId{g->resolvedDrv->first, id2.outputName}; + + auto bt2 = worker.makeDrvOutputSubstitutionGoal(convergentId); + + /* No longer need 'g' */ + g = nullptr; + + co_await await(Goals{upcast_goal(bt2)}); + + /* Set the build trace value as our own. Note the signature will not + match our key since we're the unresolved derivation, but that's + fine. We're not writing it to the DB; that's `bt2`'s job. */ + if (bt2->outputInfo) + outputInfo = bt2->outputInfo; + + co_return amDone(bt2->exitCode); + } else { + /* None left. Terminate this goal and let someone else deal + with it. */ + debug("build trace is not known for '%s', derivation is already resolved", id2.to_string()); + + if (substituterFailed) { + worker.failedSubstitutions++; + worker.updateProgress(); + } + + /* Hack: don't indicate failure if there were no substituters. + In that case the calling derivation should just do a + build. */ + co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters); + } +} + +} // namespace nix diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index e824dfdc9ca..7349e7bb11d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -99,18 +99,17 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) if (!checkResult) { DrvOutput id{drvPath, wantedOutput}; auto g = worker.makeDrvOutputSubstitutionGoal(id); - waitees.insert(g); + waitees.insert(upcast_goal(g)); co_await await(std::move(waitees)); if (nrFailed == 0) { + assert(g->outputInfo); waitees.insert(upcast_goal(worker.makePathSubstitutionGoal(g->outputInfo->outPath))); co_await await(std::move(waitees)); trace("output path substituted"); - if (nrFailed == 0) - worker.store.registerDrvOutput({*g->outputInfo, id}); - else + if (nrFailed > 0) debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); } } else { diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 5a5fe06d5e8..73db52ace21 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -18,7 +18,7 @@ Goal::Co DrvOutputSubstitutionGoal::init() { trace("init"); - /* If the derivation already exists, we’re done */ + /* If the derivation already exists, we're done */ if ((outputInfo = worker.store.queryRealisation(id))) { co_return amDone(ecSuccess); } @@ -87,6 +87,8 @@ Goal::Co DrvOutputSubstitutionGoal::init() if (!outputInfo) continue; + worker.store.registerDrvOutput({*outputInfo, id}); + trace("finished"); co_return amDone(ecSuccess); } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index d64b869e9e1..2f4a0c474ec 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -212,6 +212,8 @@ Goal::Done Goal::amDone(ExitCode result) if (auto * failure = buildResult.tryGetFailure()) { if (!preserveFailure && !waiters.empty()) logError(failure->info()); + else + logErrorInfo(lvlDebug, failure->info()); } } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 1069b68a193..6950e5c7cf7 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -4,6 +4,7 @@ #include "nix/store/build/worker.hh" #include "nix/store/build/substitution-goal.hh" #include "nix/store/build/drv-output-substitution-goal.hh" +#include "nix/store/build/build-trace-trampoline-goal.hh" #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-resolution-goal.hh" #include "nix/store/build/derivation-building-goal.hh" @@ -110,6 +111,11 @@ std::shared_ptr Worker::makeDrvOutputSubstitutionGoal return initGoalIfNeeded(drvOutputSubstitutionGoals[id], id, *this); } +std::shared_ptr Worker::makeBuildTraceTrampolineGoal(const SingleDerivedPath::Built & id) +{ + return initGoalIfNeeded(buildTraceTrampolineGoals.ensureSlot(id).value, id, *this); +} + GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit( @@ -166,6 +172,14 @@ removeGoal(std::shared_ptr goal, typename DerivedPathMap +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>::ChildNode & node) +{ + bool valueKeep = removeGoal(goal, node.value); + bool childMapKeep = removeGoal(goal, node.childMap); + return valueKeep || childMapKeep; +} + void Worker::removeGoal(GoalPtr goal) { if (auto drvGoal = std::dynamic_pointer_cast(goal)) @@ -180,6 +194,8 @@ void Worker::removeGoal(GoalPtr goal) nix::removeGoal(subGoal, substitutionGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(subGoal, drvOutputSubstitutionGoals); + else if (auto subGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(subGoal, buildTraceTrampolineGoals.map); else assert(false); @@ -546,9 +562,19 @@ GoalPtr upcast_goal(std::shared_ptr subGoal) return subGoal; } +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + GoalPtr upcast_goal(std::shared_ptr subGoal) { return subGoal; } +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + } // namespace nix diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index bcbdc85bd63..838bc01d50b 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -53,6 +53,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations +#include "nix/store/build/build-trace-trampoline-goal.hh" #include "nix/store/build/derivation-trampoline-goal.hh" namespace nix { @@ -71,6 +72,7 @@ std::strong_ordering DerivedPathMap::ChildNode::operator <=> ( template struct DerivedPathMap::ChildNode; template struct DerivedPathMap; +template struct DerivedPathMap>; template struct DerivedPathMap>>; }; // namespace nix diff --git a/src/libstore/include/nix/store/build/build-trace-trampoline-goal.hh b/src/libstore/include/nix/store/build/build-trace-trampoline-goal.hh new file mode 100644 index 00000000000..89c2cda2448 --- /dev/null +++ b/src/libstore/include/nix/store/build/build-trace-trampoline-goal.hh @@ -0,0 +1,50 @@ +#pragma once +///@file + +#include "nix/store/store-api.hh" +#include "nix/store/build/goal.hh" +#include "nix/store/realisation.hh" + +namespace nix { + +class Worker; +class DrvOutputSubstitutionGoal; + +/** + * This is the "outermost" goal type relating to build trace lookups. + * + * It handles nested `SingleDerivedPath::Built` (dynamic derivations) by + * recursively resolving the path before delegating to `DrvOutputSubstitutionGoal`. + * + * This is analogous to `DerivationTrampolineGoal` which handles nested paths + * for derivation building. + */ +class BuildTraceTrampolineGoal : public Goal +{ + /** + * The output deriving path we're trying to resolve. + * This can be nested (dynamic derivations). + */ + SingleDerivedPath::Built id; + +public: + BuildTraceTrampolineGoal(const SingleDerivedPath::Built & id, Worker & worker); + + /** + * The realisation corresponding to the given output id. + * Will be filled once we can get it. + */ + std::shared_ptr outputInfo; + + std::string key() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + Co init(); +}; + +} // namespace nix diff --git a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh index 5f36bbf06d8..191d2e91612 100644 --- a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh +++ b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh @@ -5,8 +5,8 @@ #include #include "nix/store/store-api.hh" -#include "nix/store/build/goal.hh" #include "nix/store/realisation.hh" +#include "nix/store/build/goal.hh" #include "nix/util/muxable-pipe.hh" namespace nix { @@ -14,11 +14,11 @@ namespace nix { class Worker; /** - * Fetch a `Realisation` (drv ⨯ output name -> output path) from a - * substituter. + * Try to obtain build trace key-value pairs for a concrete derivation output. * - * If the output store object itself should also be substituted, that is - * the responsibility of the caller to do so. + * This goal takes a concrete `DrvOutput` (StorePath + output name) and queries + * substituters for the realisation. For nested/dynamic derivations, use + * `BuildTraceTrampolineGoal` which resolves the path first. * * @todo rename this `BuidlTraceEntryGoal`, which will make sense * especially once `Realisation` is renamed to `BuildTraceEntry`. @@ -27,7 +27,7 @@ class DrvOutputSubstitutionGoal : public Goal { /** - * The drv output we're trying to substitute + * The concrete derivation output we're trying to find. */ DrvOutput id; diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index c65387de4f9..3ae8f5f19d9 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -23,6 +23,7 @@ struct DerivationResolutionGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; +class BuildTraceTrampolineGoal; /** * Workaround for not being able to declare a something like @@ -38,7 +39,9 @@ class DrvOutputSubstitutionGoal; */ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; @@ -119,6 +122,7 @@ private: std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; + DerivedPathMap> buildTraceTrampolineGoals; /** * Goals waiting for busy paths to be unlocked. @@ -232,12 +236,21 @@ public: const StorePath & drvPath, const Derivation & drv, BuildMode buildMode, bool storeDerivation); /** - * @ref PathSubstitutionGoal "substitution goal" + * @ref PathSubstitutionGoal "path substitution goal" */ std::shared_ptr makePathSubstitutionGoal( const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); + + /** + * @ref DrvOutputSubstitutionGoal "derivation output substitution goal" + */ std::shared_ptr makeDrvOutputSubstitutionGoal(const DrvOutput & id); + /** + * @ref BuildTraceTrampolineGoal "build trace trampoline goal" + */ + std::shared_ptr makeBuildTraceTrampolineGoal(const SingleDerivedPath::Built & id); + /** * Make a goal corresponding to the `DerivedPath`. * diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index 7db3d2c07e8..1067b31783b 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -14,6 +14,7 @@ headers = [ config_pub_h ] + files( 'binary-cache-store.hh', 'build-result.hh', 'build/build-log.hh', + 'build/build-trace-trampoline-goal.hh', 'build/derivation-builder.hh', 'build/derivation-building-goal.hh', 'build/derivation-building-misc.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index d6cfb8c7f36..7decd05138d 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -278,6 +278,7 @@ sources = files( 'binary-cache-store.cc', 'build-result.cc', 'build/build-log.cc', + 'build/build-trace-trampoline-goal.cc', 'build/derivation-builder.cc', 'build/derivation-building-goal.cc', 'build/derivation-check.cc', From edfbba9fe57f4b243b7715fa2681e5445d8ff3e0 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 14:23:55 -0500 Subject: [PATCH 06/10] Introduce `DerivedOutputGoal` --- .../build/derivation-resolution-goal.cc | 41 +++++------- src/libstore/build/derived-output-goal.cc | 64 +++++++++++++++++++ src/libstore/build/worker.cc | 14 ++++ src/libstore/derived-path-map.cc | 2 + .../nix/store/build/derived-output-goal.hh | 46 +++++++++++++ .../include/nix/store/build/worker.hh | 8 +++ src/libstore/include/nix/store/meson.build | 1 + src/libstore/meson.build | 1 + 8 files changed, 151 insertions(+), 26 deletions(-) create mode 100644 src/libstore/build/derived-output-goal.cc create mode 100644 src/libstore/include/nix/store/build/derived-output-goal.hh diff --git a/src/libstore/build/derivation-resolution-goal.cc b/src/libstore/build/derivation-resolution-goal.cc index 81c698e1856..652c96c44b3 100644 --- a/src/libstore/build/derivation-resolution-goal.cc +++ b/src/libstore/build/derivation-resolution-goal.cc @@ -1,4 +1,5 @@ #include "nix/store/build/derivation-resolution-goal.hh" +#include "nix/store/build/derived-output-goal.hh" #include "nix/store/build/worker.hh" #include "nix/util/util.hh" @@ -38,7 +39,11 @@ Goal::Co DerivationResolutionGoal::resolveDerivation() { Goals waitees; - std::map, GoalPtr, value_comparison> inputGoals; + /** + * Map from output deriving path to the DerivedOutputGoal + * that will get its realisation (either from build trace lookup or by building). + */ + std::map> inputGoals; { std::function, const DerivedPathMap::ChildNode &)> @@ -46,15 +51,11 @@ Goal::Co DerivationResolutionGoal::resolveDerivation() addWaiteeDerivedPath = [&](ref inputDrv, const DerivedPathMap::ChildNode & inputNode) { - if (!inputNode.value.empty()) { - auto g = worker.makeGoal( - DerivedPath::Built{ - .drvPath = inputDrv, - .outputs = inputNode.value, - }, - buildMode == bmRepair ? bmRepair : bmNormal); - inputGoals.insert_or_assign(inputDrv, g); - waitees.insert(std::move(g)); + for (const auto & outputName : inputNode.value) { + SingleDerivedPath::Built id{inputDrv, outputName}; + auto g = worker.makeDerivedOutputGoal(id, buildMode == bmRepair ? bmRepair : bmNormal); + inputGoals.insert_or_assign(std::move(id), g); + waitees.insert(upcast_goal(g)); } for (const auto & [outputName, childNode] : inputNode.childMap) addWaiteeDerivedPath( @@ -115,24 +116,12 @@ Goal::Co DerivationResolutionGoal::resolveDerivation() stub goal aliasing that resolved derivation goal. */ std::optional attempt = fullDrv.tryResolve( worker.store, - [&](ref drvPath, const std::string & outputName) -> std::optional { - auto mEntry = get(inputGoals, drvPath); - if (!mEntry) + [&](ref inputDrv, const std::string & outputName) -> std::optional { + auto mGoal = get(inputGoals, SingleDerivedPath::Built{inputDrv, outputName}); + if (!mGoal) return std::nullopt; - auto & buildResult = (*mEntry)->buildResult; - return std::visit( - overloaded{ - [](const BuildResult::Failure &) -> std::optional { return std::nullopt; }, - [&](const BuildResult::Success & success) -> std::optional { - auto i = get(success.builtOutputs, outputName); - if (!i) - return std::nullopt; - - return i->outPath; - }, - }, - buildResult.inner); + return (*mGoal)->outputPath; }); if (!attempt) { /* TODO (impure derivations-induced tech debt) (see below): diff --git a/src/libstore/build/derived-output-goal.cc b/src/libstore/build/derived-output-goal.cc new file mode 100644 index 00000000000..6b59175e24b --- /dev/null +++ b/src/libstore/build/derived-output-goal.cc @@ -0,0 +1,64 @@ +#include "nix/store/build/derived-output-goal.hh" +#include "nix/store/build/build-trace-trampoline-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/util/util.hh" + +namespace nix { + +DerivedOutputGoal::DerivedOutputGoal(const SingleDerivedPath::Built & id, Worker & worker, BuildMode buildMode) + : Goal{worker, init()} + , id{id} + , buildMode{buildMode} +{ + name = fmt("getting derived output '%s'", id.to_string(worker.store)); + trace("created"); +} + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit( + overloaded{ + [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, + [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); }, + }, + req.raw()); +} + +std::string DerivedOutputGoal::key() +{ + return "do$" + std::string(pathPartOfReq(*id.drvPath).name()) + "$" + id.to_string(worker.store); +} + +Goal::Co DerivedOutputGoal::init() +{ + trace("init"); + + // ASSUME WE CANNOT FETCH REALISTION + + /* No realisation found. Fall back to building via DerivationGoal. + We use makeGoal which will create a DerivationTrampolineGoal, + which handles getting the derivation and building it. */ + auto buildGoal = worker.makeGoal( + DerivedPath::Built{ + .drvPath = id.drvPath, + .outputs = OutputsSpec::Names{id.output}, + }, + buildMode); + + buildGoal->preserveFailure = true; + + co_await await(Goals{buildGoal}); + + trace("build goal finished"); + + /* Extract the output path from the build result. */ + if (auto * successP = buildGoal->buildResult.tryGetSuccess()) { + if (auto * realisation = get(successP->builtOutputs, id.output)) { + outputPath = realisation->outPath; + } + } + + co_return amDone(buildGoal->exitCode); +} + +} // namespace nix diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 6950e5c7cf7..255f6ffa4e1 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -5,6 +5,7 @@ #include "nix/store/build/substitution-goal.hh" #include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/build-trace-trampoline-goal.hh" +#include "nix/store/build/derived-output-goal.hh" #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-resolution-goal.hh" #include "nix/store/build/derivation-building-goal.hh" @@ -116,6 +117,12 @@ std::shared_ptr Worker::makeBuildTraceTrampolineGoal(c return initGoalIfNeeded(buildTraceTrampolineGoals.ensureSlot(id).value, id, *this); } +std::shared_ptr +Worker::makeDerivedOutputGoal(const SingleDerivedPath::Built & id, BuildMode buildMode) +{ + return initGoalIfNeeded(derivedOutputGoals.ensureSlot(id).value, id, *this, buildMode); +} + GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit( @@ -196,6 +203,8 @@ void Worker::removeGoal(GoalPtr goal) nix::removeGoal(subGoal, drvOutputSubstitutionGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(subGoal, buildTraceTrampolineGoals.map); + else if (auto subGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(subGoal, derivedOutputGoals.map); else assert(false); @@ -577,4 +586,9 @@ GoalPtr upcast_goal(std::shared_ptr subGoal) return subGoal; } +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + } // namespace nix diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index 838bc01d50b..b436169004d 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -54,6 +54,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations #include "nix/store/build/build-trace-trampoline-goal.hh" +#include "nix/store/build/derived-output-goal.hh" #include "nix/store/build/derivation-trampoline-goal.hh" namespace nix { @@ -73,6 +74,7 @@ template struct DerivedPathMap::ChildNode; template struct DerivedPathMap; template struct DerivedPathMap>; +template struct DerivedPathMap>; template struct DerivedPathMap>>; }; // namespace nix diff --git a/src/libstore/include/nix/store/build/derived-output-goal.hh b/src/libstore/include/nix/store/build/derived-output-goal.hh new file mode 100644 index 00000000000..e6bde8c14d3 --- /dev/null +++ b/src/libstore/include/nix/store/build/derived-output-goal.hh @@ -0,0 +1,46 @@ +#pragma once +///@file + +#include "nix/store/store-api.hh" +#include "nix/store/build/goal.hh" + +namespace nix { + +class Worker; + +/** + * A goal for getting the output path of a single derived output. + * + * This is used by `DerivationResolutionGoal` to get input derivation + * output paths for resolving CA derivations. + */ +class DerivedOutputGoal : public Goal +{ + /** + * The output deriving path we're trying to realise. + * This can be nested (dynamic derivations). + */ + SingleDerivedPath::Built id; + + BuildMode buildMode; + +public: + DerivedOutputGoal(const SingleDerivedPath::Built & id, Worker & worker, BuildMode buildMode); + + /** + * The output path. Will be set once the goal succeeds. + */ + std::optional outputPath; + + std::string key() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + Co init(); +}; + +} // namespace nix diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index 3ae8f5f19d9..d8df817ec27 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -24,6 +24,7 @@ struct DerivationBuildingGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; class BuildTraceTrampolineGoal; +class DerivedOutputGoal; /** * Workaround for not being able to declare a something like @@ -42,6 +43,7 @@ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; @@ -123,6 +125,7 @@ private: std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; DerivedPathMap> buildTraceTrampolineGoals; + DerivedPathMap> derivedOutputGoals; /** * Goals waiting for busy paths to be unlocked. @@ -251,6 +254,11 @@ public: */ std::shared_ptr makeBuildTraceTrampolineGoal(const SingleDerivedPath::Built & id); + /** + * @ref DerivedOutputGoal "derived output goal" + */ + std::shared_ptr makeDerivedOutputGoal(const SingleDerivedPath::Built & id, BuildMode buildMode); + /** * Make a goal corresponding to the `DerivedPath`. * diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index 1067b31783b..74e65686b24 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -22,6 +22,7 @@ headers = [ config_pub_h ] + files( 'build/derivation-goal.hh', 'build/derivation-resolution-goal.hh', 'build/derivation-trampoline-goal.hh', + 'build/derived-output-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 7decd05138d..32a65342e81 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -286,6 +286,7 @@ sources = files( 'build/derivation-goal.cc', 'build/derivation-resolution-goal.cc', 'build/derivation-trampoline-goal.cc', + 'build/derived-output-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc', From a3e6f5f49c0c181a8f02847298d4cf7480907fef Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 14:02:00 -0500 Subject: [PATCH 07/10] WIP Fix #11928 --- .../issue-11928/store-after.json | 22 --------- src/libstore-tests/worker-substitution.cc | 6 +-- src/libstore/build/derived-output-goal.cc | 47 ++++++++++++++++++- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json index fc469c1de11..5f717a4c9a2 100644 --- a/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json +++ b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json @@ -38,28 +38,6 @@ "ultimate": false, "version": 2 } - }, - "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out": { - "contents": { - "contents": "I am the dependency output", - "executable": false, - "type": "regular" - }, - "info": { - "ca": { - "hash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", - "method": "nar" - }, - "deriver": null, - "narHash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", - "narSize": 144, - "references": [], - "registrationTime": null, - "signatures": [], - "storeDir": "/nix/store", - "ultimate": false, - "version": 2 - } } }, "derivations": { diff --git a/src/libstore-tests/worker-substitution.cc b/src/libstore-tests/worker-substitution.cc index 7be88aaba74..0820fea70d6 100644 --- a/src/libstore-tests/worker-substitution.cc +++ b/src/libstore-tests/worker-substitution.cc @@ -431,9 +431,9 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) ASSERT_TRUE(depRealisation); ASSERT_EQ(depRealisation->outPath, depOutputPath); - // TODO #11928: The dependency's OUTPUT should NOT be fetched (not referenced - // by root output). Once #11928 is fixed, change ASSERT_TRUE to ASSERT_FALSE. - ASSERT_TRUE(dummyStore->isValidPath(depOutputPath)); + // The dependency's OUTPUT should NOT be fetched (not referenced by + // root output). + ASSERT_FALSE(dummyStore->isValidPath(depOutputPath)); // Verify the goal succeeded ASSERT_EQ(upcast_goal(goal)->exitCode, Goal::ecSuccess); diff --git a/src/libstore/build/derived-output-goal.cc b/src/libstore/build/derived-output-goal.cc index 6b59175e24b..461ff17f203 100644 --- a/src/libstore/build/derived-output-goal.cc +++ b/src/libstore/build/derived-output-goal.cc @@ -33,7 +33,52 @@ Goal::Co DerivedOutputGoal::init() { trace("init"); - // ASSUME WE CANNOT FETCH REALISTION + /* Check if this is an input-addressed derivation with a statically-known + output path. If so, skip build trace lookup and build directly. */ + bool usesBuildTrace = true; + if (auto * opaque = std::get_if(&*id.drvPath)) { + for (auto * drvStore : {&worker.evalStore, &worker.store}) { + if (drvStore->isValidPath(opaque->path)) { + auto drv = drvStore->readDerivation(opaque->path); + auto outputs = drv.outputsAndOptPaths(worker.store); + if (auto * output = get(outputs, id.output)) { + if (output->second) { + /* Output path is statically known (input-addressed). + Check if it already exists. */ + if (worker.store.isValidPath(*output->second)) { + trace("output already exists"); + outputPath = *output->second; + co_return amDone(ecSuccess); + } + /* Input-addressed, but output doesn't exist. Build it. */ + trace("input-addressed derivation, skipping build trace"); + usesBuildTrace = false; + } + } + break; + } + } + } + + if (usesBuildTrace) { + /* Try to look up the realisation via BuildTraceTrampolineGoal. */ + auto btGoal = worker.makeBuildTraceTrampolineGoal(id); + + co_await await(Goals{upcast_goal(btGoal)}); + + if (btGoal->outputInfo) { + /* Found a realisation! We're done. */ + trace("found realisation via build trace lookup"); + outputPath = btGoal->outputInfo->outPath; + co_return amDone(ecSuccess); + } + + trace("no realisation found, falling back to building"); + } + + /* Reset counters since we're starting a fresh build attempt. */ + nrFailed = 0; + nrNoSubstituters = 0; /* No realisation found. Fall back to building via DerivationGoal. We use makeGoal which will create a DerivationTrampolineGoal, From 05fe04dac5c0233844af908f8016bd2ebed4b7d1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 19:33:04 -0500 Subject: [PATCH 08/10] `nix realisation delete` --- src/libstore/include/nix/store/gc-store.hh | 5 +++ src/libstore/include/nix/store/local-store.hh | 2 + .../include/nix/store/remote-store.hh | 2 + src/libstore/local-store.cc | 19 ++++++++ src/libstore/remote-store.cc | 5 +++ src/libstore/restricted-store.cc | 5 +++ src/nix/realisation.cc | 45 +++++++++++++++++++ src/nix/realisation/delete.md | 14 ++++++ tests/functional/build-delete.sh | 1 + 9 files changed, 98 insertions(+) create mode 100644 src/nix/realisation/delete.md diff --git a/src/libstore/include/nix/store/gc-store.hh b/src/libstore/include/nix/store/gc-store.hh index d5617d6d5c4..101e37edda6 100644 --- a/src/libstore/include/nix/store/gc-store.hh +++ b/src/libstore/include/nix/store/gc-store.hh @@ -117,6 +117,11 @@ struct GcStore : public virtual Store * Perform a garbage collection. */ virtual void collectGarbage(const GCOptions & options, GCResults & results) = 0; + + /** + * Delete a build trace entry (realisation) from the store's database. + */ + virtual void deleteBuildTrace(const DrvOutput & id) = 0; }; } // namespace nix diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 40ec438ef97..ce1d7c5a913 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -336,6 +336,8 @@ public: void collectGarbage(const GCOptions & options, GCResults & results) override; + void deleteBuildTrace(const DrvOutput & id) override; + /** * Called by `collectGarbage` to trace in reverse. * diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index 0df2b5bab66..539239c91dd 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -123,6 +123,8 @@ struct RemoteStore : public virtual Store, public virtual GcStore, public virtua void collectGarbage(const GCOptions & options, GCResults & results) override; + void deleteBuildTrace(const DrvOutput & id) override; + void optimiseStore() override; bool verifyStore(bool checkContents, RepairFlag repair) override; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9f47da5b20f..92f93a4d712 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -106,6 +106,7 @@ struct LocalStore::State::Stmts SQLiteStmt AddDerivationOutput; SQLiteStmt RegisterRealisedOutput; SQLiteStmt UpdateRealisedOutput; + SQLiteStmt DeleteRealisedOutput; SQLiteStmt QueryValidDerivers; SQLiteStmt QueryDerivationOutputs; SQLiteStmt QueryRealisedOutput; @@ -374,6 +375,15 @@ LocalStore::LocalStore(ref config) outputName = ? ; )"); + state->stmts->DeleteRealisedOutput.create( + state->db, + R"( + delete from BuildTraceV3 + where + drvPath = ? and + outputName = ? + ; + )"); state->stmts->QueryRealisedOutput.create( state->db, R"( @@ -668,6 +678,15 @@ void LocalStore::registerDrvOutput(const Realisation & info) }); } +void LocalStore::deleteBuildTrace(const DrvOutput & id) +{ + experimentalFeatureSettings.require(Xp::CaDerivations); + retrySQLite([&]() { + auto state(_state->lock()); + state->stmts->DeleteRealisedOutput.use()(id.drvPath.to_string())(id.outputName).exec(); + }); +} + void LocalStore::cacheDrvOutputMapping( State & state, const uint64_t deriver, const std::string & outputName, const StorePath & output) { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 2e16c999699..8a4001a763d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -699,6 +699,11 @@ void RemoteStore::collectGarbage(const GCOptions & options, GCResults & results) pathInfoCache->lock()->clear(); } +void RemoteStore::deleteBuildTrace(const DrvOutput & id) +{ + unsupported("deleteBuildTrace"); +} + void RemoteStore::optimiseStore() { auto conn(getConnection()); diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index 91162015449..efd89583a38 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -134,6 +134,11 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor void collectGarbage(const GCOptions & options, GCResults & results) override {} + void deleteBuildTrace(const DrvOutput & id) override + { + unsupported("deleteBuildTrace"); + } + void addSignatures(const StorePath & storePath, const std::set & sigs) override { unsupported("addSignatures"); diff --git a/src/nix/realisation.cc b/src/nix/realisation.cc index 8dd608d23b0..b806c947a45 100644 --- a/src/nix/realisation.cc +++ b/src/nix/realisation.cc @@ -1,5 +1,7 @@ #include "nix/cmd/command.hh" #include "nix/main/common-args.hh" +#include "nix/store/gc-store.hh" +#include "nix/store/store-cast.hh" #include @@ -78,3 +80,46 @@ struct CmdRealisationInfo : BuiltPathsCommand, MixJSON }; static auto rCmdRealisationInfo = registerCommand2({"realisation", "info"}); + +struct CmdRealisationDelete : virtual StoreCommand +{ + std::vector ids; + + CmdRealisationDelete() + { + expectArgs({ + .label = "id", + .handler = {&ids}, + }); + } + + std::string description() override + { + return "delete realisations from the store"; + } + + std::string doc() override + { + return +#include "realisation/delete.md" + ; + } + + Category category() override + { + return catSecondary; + } + + void run(ref store) override + { + experimentalFeatureSettings.require(Xp::CaDerivations); + auto & gcStore = require(*store); + + for (auto & id : ids) { + auto drvOutput = DrvOutput::parse(*store, id); + gcStore.deleteBuildTrace(drvOutput); + } + } +}; + +static auto rCmdRealisationDelete = registerCommand2({"realisation", "delete"}); diff --git a/src/nix/realisation/delete.md b/src/nix/realisation/delete.md new file mode 100644 index 00000000000..56b8421f5ed --- /dev/null +++ b/src/nix/realisation/delete.md @@ -0,0 +1,14 @@ +R"MdBoundary( +# Description + +Delete realisations from the store. + +# Examples + +Delete a realisation by its ID: + +```console +$ nix realisation delete sha256:3d382378a00588e064ee30be96dd0fa7e7df7cf3fbcace85a0e7b7dada1eef25!out +``` + +)MdBoundary" diff --git a/tests/functional/build-delete.sh b/tests/functional/build-delete.sh index 66b14fd1438..7068a75f918 100755 --- a/tests/functional/build-delete.sh +++ b/tests/functional/build-delete.sh @@ -46,6 +46,7 @@ issue_6572_dependent_outputs() { if [[ -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then # Resolved derivations interferre with the deletion nix-store --delete "${NIX_STORE_DIR}"/*.drv + #nix realisation delete "$(jq -r <"$TEST_ROOT"/a.json .[0].outputs.second)" fi nix-store --delete "$(jq -r <"$TEST_ROOT"/a.json .[0].outputs.second)" p=$(nix build -f multiple-outputs.nix use-a --no-link --print-out-paths) From eae2d4ba94f50e4c8a9cadd5f76308b76afb8f39 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 19:39:43 -0500 Subject: [PATCH 09/10] Only resolve when we thinnk we can later substitute --- src/libstore/build/derived-output-goal.cc | 24 +++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/libstore/build/derived-output-goal.cc b/src/libstore/build/derived-output-goal.cc index 461ff17f203..27de33a672e 100644 --- a/src/libstore/build/derived-output-goal.cc +++ b/src/libstore/build/derived-output-goal.cc @@ -67,13 +67,29 @@ Goal::Co DerivedOutputGoal::init() co_await await(Goals{upcast_goal(btGoal)}); if (btGoal->outputInfo) { - /* Found a realisation! We're done. */ + /* Found a realisation! Check if the output is available. */ trace("found realisation via build trace lookup"); outputPath = btGoal->outputInfo->outPath; - co_return amDone(ecSuccess); - } - trace("no realisation found, falling back to building"); + /* Check if the output path exists locally or in any substitutor. */ + if (worker.store.isValidPath(*outputPath)) { + trace("realisation found, and output is known to exist in default store"); + co_return amDone(ecSuccess); + } + + for (auto & sub : worker.getSubstituters()) { + if (sub->isValidPath(*outputPath)) { + trace( + fmt("realisation found, and output is known to exist in substitutor '%s'", + sub->config.getHumanReadableURI())); + co_return amDone(ecSuccess); + } + } + + trace("realisation found but output not available, falling back to building"); + } else { + trace("no realisation found, falling back to building"); + } } /* Reset counters since we're starting a fresh build attempt. */ From 84c36c21a2fb01e5bbc90d94e280f5999791bd09 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 18 Dec 2025 19:51:25 -0500 Subject: [PATCH 10/10] Fix impure dreivations --- src/libstore/build/derived-output-goal.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/libstore/build/derived-output-goal.cc b/src/libstore/build/derived-output-goal.cc index 27de33a672e..64cb9f0ba26 100644 --- a/src/libstore/build/derived-output-goal.cc +++ b/src/libstore/build/derived-output-goal.cc @@ -1,6 +1,7 @@ #include "nix/store/build/derived-output-goal.hh" #include "nix/store/build/build-trace-trampoline-goal.hh" #include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" #include "nix/util/util.hh" namespace nix { @@ -53,6 +54,20 @@ Goal::Co DerivedOutputGoal::init() /* Input-addressed, but output doesn't exist. Build it. */ trace("input-addressed derivation, skipping build trace"); usesBuildTrace = false; + } else if (drv.type().isImpure()) { + /* Impure derivation without static output path. + Check local realisation database directly. */ + auto outputId = DrvOutput{opaque->path, id.output}; + if (auto realisation = worker.store.queryRealisation(outputId)) { + if (worker.store.isValidPath(realisation->outPath)) { + trace("impure derivation already built"); + outputPath = realisation->outPath; + co_return amDone(ecSuccess); + } + } + /* No existing realisation found, or output missing. Build it. */ + trace("impure derivation, skipping build trace"); + usesBuildTrace = false; } } break;