diff --git a/src/libcmd/built-path.cc b/src/libcmd/built-path.cc index fc7f18493..4d76dd6da 100644 --- a/src/libcmd/built-path.cc +++ b/src/libcmd/built-path.cc @@ -117,11 +117,10 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const "the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)", store.printStorePath(p.drvPath->outPath()), outputName); - DrvOutput key{*drvOutput, outputName}; - auto thisRealisation = store.queryRealisation(key); + auto thisRealisation = store.queryRealisation(DrvOutput{*drvOutput, outputName}); assert(thisRealisation); // We’ve built it, so we must // have the realisation - res.insert(Realisation{*thisRealisation, std::move(key)}); + res.insert(*thisRealisation); } else { res.insert(outputPath); } diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index 38d06336b..a308b731d 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -669,7 +669,7 @@ ProcessLineResult NixRepl::processLine(std::string line) ss << "No documentation found.\n\n"; } - auto markdown = toView(ss); + auto markdown = ss.view(); logger->cout(trim(renderMarkdownToTerminal(markdown))); } else diff --git a/src/libexpr/eval-profiler.cc b/src/libexpr/eval-profiler.cc index ba92faf18..e9dc1e021 100644 --- a/src/libexpr/eval-profiler.cc +++ b/src/libexpr/eval-profiler.cc @@ -324,7 +324,7 @@ void SampleStack::saveProfile() std::visit([&](auto && info) { info.symbolize(state, os, posCache); }, pos); } os << " " << count; - writeLine(profileFd.get(), std::move(os).str()); + writeLine(profileFd.get(), os.str()); /* Clear ostringstream. */ os.str(""); os.clear(); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 20ebe026a..db17f103b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -591,7 +591,7 @@ std::optional EvalState::getDoc(Value & v) .name = name, .arity = 0, // FIXME: figure out how deep by syntax only? It's not semantically useful though... .args = {}, - .doc = makeImmutableString(toView(s)), // NOTE: memory leak when compiled without GC + .doc = makeImmutableString(s.view()), // NOTE: memory leak when compiled without GC }; } if (isFunctor(v)) { @@ -1341,7 +1341,7 @@ void ExprVar::eval(EvalState & state, Env & env, Value & v) v = *v2; } -static std::string showAttrPath(EvalState & state, Env & env, const AttrPath & attrPath) +static std::string showAttrPath(EvalState & state, Env & env, std::span attrPath) { std::ostringstream out; bool first = true; @@ -1377,10 +1377,10 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) env, getPos(), "while evaluating the attribute '%1%'", - showAttrPath(state, env, attrPath)) + showAttrPath(state, env, getAttrPath())) : nullptr; - for (auto & i : attrPath) { + for (auto & i : getAttrPath()) { state.nrLookups++; const Attr * j; auto name = getName(i, state, env); @@ -1418,7 +1418,7 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) auto origin = std::get_if(&pos2r.origin); if (!(origin && *origin == state.derivationInternal)) state.addErrorTrace( - e, pos2, "while evaluating the attribute '%1%'", showAttrPath(state, env, attrPath)); + e, pos2, "while evaluating the attribute '%1%'", showAttrPath(state, env, getAttrPath())); } throw; } @@ -1429,13 +1429,13 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) Symbol ExprSelect::evalExceptFinalSelect(EvalState & state, Env & env, Value & attrs) { Value vTmp; - Symbol name = getName(attrPath[attrPath.size() - 1], state, env); + Symbol name = getName(attrPathStart[nAttrPath - 1], state, env); - if (attrPath.size() == 1) { + if (nAttrPath == 1) { e->eval(state, env, vTmp); } else { ExprSelect init(*this); - init.attrPath.pop_back(); + init.nAttrPath--; init.eval(state, env, vTmp); } attrs = vTmp; @@ -1811,7 +1811,7 @@ void ExprAssert::eval(EvalState & state, Env & env, Value & v) if (!state.evalBool(env, cond, pos, "in the condition of the assert statement")) { std::ostringstream out; cond->show(state.symbols, out); - auto exprStr = toView(out); + auto exprStr = out.view(); if (auto eq = dynamic_cast(cond)) { try { diff --git a/src/libexpr/include/nix/expr/nixexpr.hh b/src/libexpr/include/nix/expr/nixexpr.hh index 2af6039cd..863a1369d 100644 --- a/src/libexpr/include/nix/expr/nixexpr.hh +++ b/src/libexpr/include/nix/expr/nixexpr.hh @@ -2,8 +2,10 @@ ///@file #include +#include #include #include +#include #include "nix/expr/gc-small-vector.hh" #include "nix/expr/value.hh" @@ -79,9 +81,11 @@ struct AttrName : expr(e) {}; }; +static_assert(std::is_trivially_copy_constructible_v); + typedef std::vector AttrPath; -std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath); +std::string showAttrPath(const SymbolTable & symbols, std::span attrPath); using UpdateQueue = SmallTemporaryValueVector; @@ -288,20 +292,33 @@ struct ExprInheritFrom : ExprVar struct ExprSelect : Expr { PosIdx pos; + uint32_t nAttrPath; Expr *e, *def; - AttrPath attrPath; - ExprSelect(const PosIdx & pos, Expr * e, AttrPath attrPath, Expr * def) + AttrName * attrPathStart; + + ExprSelect( + std::pmr::polymorphic_allocator & alloc, + const PosIdx & pos, + Expr * e, + std::span attrPath, + Expr * def) : pos(pos) + , nAttrPath(attrPath.size()) , e(e) , def(def) - , attrPath(std::move(attrPath)) {}; + , attrPathStart(alloc.allocate_object(nAttrPath)) + { + std::ranges::copy(attrPath, attrPathStart); + }; - ExprSelect(const PosIdx & pos, Expr * e, Symbol name) + ExprSelect(std::pmr::polymorphic_allocator & alloc, const PosIdx & pos, Expr * e, Symbol name) : pos(pos) + , nAttrPath(1) , e(e) , def(0) + , attrPathStart((alloc.allocate_object())) { - attrPath.push_back(AttrName(name)); + *attrPathStart = AttrName(name); }; PosIdx getPos() const override @@ -309,6 +326,11 @@ struct ExprSelect : Expr return pos; } + std::span getAttrPath() const + { + return {attrPathStart, nAttrPath}; + } + /** * Evaluate the `a.b.c` part of `a.b.c.d`. This exists mostly for the purpose of :doc in the repl. * @@ -326,10 +348,14 @@ struct ExprSelect : Expr struct ExprOpHasAttr : Expr { Expr * e; - AttrPath attrPath; - ExprOpHasAttr(Expr * e, AttrPath attrPath) + std::span attrPath; + + ExprOpHasAttr(std::pmr::polymorphic_allocator & alloc, Expr * e, std::vector attrPath) : e(e) - , attrPath(std::move(attrPath)) {}; + , attrPath({alloc.allocate_object(attrPath.size()), attrPath.size()}) + { + std::ranges::copy(attrPath, this->attrPath.begin()); + }; PosIdx getPos() const override { diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 014b85f20..5b9d17d49 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -57,7 +57,7 @@ void ExprSelect::show(const SymbolTable & symbols, std::ostream & str) const { str << "("; e->show(symbols, str); - str << ")." << showAttrPath(symbols, attrPath); + str << ")." << showAttrPath(symbols, getAttrPath()); if (def) { str << " or ("; def->show(symbols, str); @@ -261,7 +261,7 @@ void ExprPos::show(const SymbolTable & symbols, std::ostream & str) const str << "__curPos"; } -std::string showAttrPath(const SymbolTable & symbols, const AttrPath & attrPath) +std::string showAttrPath(const SymbolTable & symbols, std::span attrPath) { std::ostringstream out; bool first = true; @@ -362,7 +362,7 @@ void ExprSelect::bindVars(EvalState & es, const std::shared_ptr e->bindVars(es, env); if (def) def->bindVars(es, env); - for (auto & i : attrPath) + for (auto & i : getAttrPath()) if (!i.symbol) i.expr->bindVars(es, env); } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index bc1eb056e..9186fcf4b 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -261,7 +261,7 @@ expr_op | expr_op OR expr_op { $$ = new ExprOpOr(state->at(@2), $1, $3); } | expr_op IMPL expr_op { $$ = new ExprOpImpl(state->at(@2), $1, $3); } | expr_op UPDATE expr_op { $$ = new ExprOpUpdate(state->at(@2), $1, $3); } - | expr_op '?' attrpath { $$ = new ExprOpHasAttr($1, std::move(*$3)); delete $3; } + | expr_op '?' attrpath { $$ = new ExprOpHasAttr(state->alloc, $1, std::move(*$3)); delete $3; } | expr_op '+' expr_op { $$ = new ExprConcatStrings(state->at(@2), false, new std::vector >({{state->at(@1), $1}, {state->at(@3), $3}})); } | expr_op '-' expr_op { $$ = new ExprCall(state->at(@2), new ExprVar(state->s.sub), {$1, $3}); } @@ -282,9 +282,9 @@ expr_app expr_select : expr_simple '.' attrpath - { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), nullptr); delete $3; } + { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), nullptr); delete $3; } | expr_simple '.' attrpath OR_KW expr_select - { $$ = new ExprSelect(CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); } + { $$ = new ExprSelect(state->alloc, CUR_POS, $1, std::move(*$3), $5); delete $3; $5->warnIfCursedOr(state->symbols, state->positions); } | /* Backwards compatibility: because Nixpkgs has a function named ‘or’, allow stuff like ‘map or [...]’. This production is problematic (see https://github.com/NixOS/nix/issues/11118) and will be refactored in the @@ -343,7 +343,7 @@ expr_simple /* Let expressions `let {..., body = ...}' are just desugared into `(rec {..., body = ...}).body'. */ | LET '{' binds '}' - { $3->recursive = true; $3->pos = CUR_POS; $$ = new ExprSelect(noPos, $3, state->s.body); } + { $3->recursive = true; $3->pos = CUR_POS; $$ = new ExprSelect(state->alloc, noPos, $3, state->s.body); } | REC '{' binds '}' { $3->recursive = true; $3->pos = CUR_POS; $$ = $3; } | '{' binds1 '}' @@ -447,7 +447,7 @@ binds1 $accum->attrs.emplace( i.symbol, ExprAttrs::AttrDef( - new ExprSelect(iPos, from, i.symbol), + new ExprSelect(state->alloc, iPos, from, i.symbol), iPos, ExprAttrs::AttrDef::Kind::InheritedFrom)); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index a8ac8d159..86cb00131 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2412,7 +2412,7 @@ static void prim_toXML(EvalState & state, const PosIdx pos, Value ** args, Value std::ostringstream out; NixStringContext context; printValueAsXML(state, true, false, *args[0], out, context, pos); - v.mkString(toView(out), context); + v.mkString(out.view(), context); } static RegisterPrimOp primop_toXML({ @@ -2520,7 +2520,7 @@ static void prim_toJSON(EvalState & state, const PosIdx pos, Value ** args, Valu std::ostringstream out; NixStringContext context; printValueAsJSON(state, true, *args[0], pos, out, context); - v.mkString(toView(out), context); + v.mkString(out.view(), context); } static RegisterPrimOp primop_toJSON({ diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index e673e55a0..ee2ca375a 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -561,14 +561,22 @@ static void fetch( .hash = *expectedHash, .references = {}}); - if (state.store->isValidPath(expectedPath)) { + // Try to get the path from the local store or substituters + try { + state.store->ensurePath(expectedPath); + debug("using substituted/cached path '%s' for '%s'", state.store->printStorePath(expectedPath), *url); state.allowAndSetStorePathString(expectedPath, v); return; + } catch (Error & e) { + debug( + "substitution of '%s' failed, will try to download: %s", + state.store->printStorePath(expectedPath), + e.what()); + // Fall through to download } } - // TODO: fetching may fail, yet the path may be substitutable. - // https://github.com/NixOS/nix/issues/4313 + // Download the file/tarball if substitution failed or no hash was provided auto storePath = unpack ? fetchToStore( state.fetchSettings, *state.store, diff --git a/src/libexpr/primops/fromTOML.cc b/src/libexpr/primops/fromTOML.cc index 3ab594905..d2f91a75b 100644 --- a/src/libexpr/primops/fromTOML.cc +++ b/src/libexpr/primops/fromTOML.cc @@ -139,7 +139,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Va attrs.alloc("_type").mkStringNoCopy("timestamp"); std::ostringstream s; s << t; - auto str = toView(s); + auto str = s.view(); forceNoNullByte(str); attrs.alloc("value").mkString(str); v.mkAttrs(attrs); diff --git a/src/libexpr/print.cc b/src/libexpr/print.cc index 071addc1a..4776be033 100644 --- a/src/libexpr/print.cc +++ b/src/libexpr/print.cc @@ -461,7 +461,7 @@ private: std::ostringstream s; s << state.positions[v.lambda().fun->pos]; - output << " @ " << filterANSIEscapes(toView(s)); + output << " @ " << filterANSIEscapes(s.view()); } } else if (v.isPrimOp()) { if (v.primOp()) diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index c00f5d86b..edec8460d 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -183,7 +183,7 @@ public: std::ostringstream oss; showErrorInfo(oss, ei, loggerSettings.showTrace.get()); - log(*state, ei.level, toView(oss)); + log(*state, ei.level, oss.view()); } void log(State & state, Verbosity lvl, std::string_view s) diff --git a/src/libstore-tests/common-protocol.cc b/src/libstore-tests/common-protocol.cc index 2c001957b..35fca165d 100644 --- a/src/libstore-tests/common-protocol.cc +++ b/src/libstore-tests/common-protocol.cc @@ -112,34 +112,32 @@ CHARACTERIZATION_TEST( "realisation", (std::tuple{ Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + .id = + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, }, Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - .dependentRealisations = + .id = + { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, + .dependentRealisations = + { { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", }, + StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + }, }, })) diff --git a/src/libstore-tests/dummy-store.cc b/src/libstore-tests/dummy-store.cc index 3dd8137a3..b841d7890 100644 --- a/src/libstore-tests/dummy-store.cc +++ b/src/libstore-tests/dummy-store.cc @@ -1,6 +1,6 @@ #include -#include "nix/store/dummy-store-impl.hh" +#include "nix/store/dummy-store.hh" #include "nix/store/globals.hh" #include "nix/store/realisation.hh" @@ -13,7 +13,7 @@ TEST(DummyStore, realisation_read) auto store = [] { auto cfg = make_ref(StoreReference::Params{}); cfg->readOnly = false; - return cfg->openDummyStore(); + return cfg->openStore(); }(); auto drvHash = Hash::parseExplicitFormatUnprefixed( @@ -22,17 +22,6 @@ TEST(DummyStore, realisation_read) auto outputName = "foo"; EXPECT_EQ(store->queryRealisation({drvHash, outputName}), nullptr); - - UnkeyedRealisation value{ - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, - }; - - store->buildTrace.insert({drvHash, {{outputName, make_ref(value)}}}); - - auto value2 = store->queryRealisation({drvHash, outputName}); - - ASSERT_TRUE(value2); - EXPECT_EQ(*value2, value); } } // namespace nix diff --git a/src/libstore-tests/realisation.cc b/src/libstore-tests/realisation.cc index d16049bc5..a5a5bee50 100644 --- a/src/libstore-tests/realisation.cc +++ b/src/libstore-tests/realisation.cc @@ -49,16 +49,16 @@ INSTANTIATE_TEST_SUITE_P( RealisationJsonTest, ([] { Realisation simple{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, - }, - { - .drvHash = Hash::parseExplicitFormatUnprefixed( - "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", - HashAlgorithm::SHA256, - HashFormat::Base16), - .outputName = "foo", - }, + + .id = + { + .drvHash = Hash::parseExplicitFormatUnprefixed( + "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", + HashAlgorithm::SHA256, + HashFormat::Base16), + .outputName = "foo", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, }; return ::testing::Values( std::pair{ diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index 10aa21e9d..a63201164 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -95,34 +95,32 @@ VERSIONED_CHARACTERIZATION_TEST( defaultVersion, (std::tuple{ Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + .id = + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, }, Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - .dependentRealisations = + .id = + { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, + .dependentRealisations = + { { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", }, + StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + }, }, })) @@ -198,27 +196,25 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, }, { "bar", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index c4afde3bd..489151c8c 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -148,34 +148,32 @@ VERSIONED_CHARACTERIZATION_TEST( defaultVersion, (std::tuple{ Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + .id = + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, }, Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - .dependentRealisations = + .id = + { + .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .outputName = "baz", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, + .dependentRealisations = + { { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + DrvOutput{ + .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "quux", }, + StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + }, }, })) @@ -216,25 +214,25 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, }, { "bar", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, @@ -269,27 +267,25 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, }, { "bar", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, @@ -328,27 +324,25 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "foo", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "foo", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }, }, { "bar", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, - }, - DrvOutput{ - .drvHash = - Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "bar", - }, + .id = + DrvOutput{ + .drvHash = + Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .outputName = "bar", + }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 3705f3d4d..badfb4b14 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -502,15 +502,10 @@ StorePath BinaryCacheStore::addToStore( ->path; } -std::string BinaryCacheStore::makeRealisationPath(const DrvOutput & id) -{ - return realisationsPrefix + "/" + id.to_string() + ".doi"; -} - void BinaryCacheStore::queryRealisationUncached( - const DrvOutput & id, Callback> callback) noexcept + const DrvOutput & id, Callback> callback) noexcept { - auto outputInfoFilePath = makeRealisationPath(id); + auto outputInfoFilePath = realisationsPrefix + "/" + id.to_string() + ".doi"; auto callbackPtr = std::make_shared(std::move(callback)); @@ -520,12 +515,11 @@ void BinaryCacheStore::queryRealisationUncached( if (!data) return (*callbackPtr)({}); - std::shared_ptr realisation; + std::shared_ptr realisation; try { - realisation = std::make_shared(nlohmann::json::parse(*data)); + realisation = std::make_shared(nlohmann::json::parse(*data)); } catch (Error & e) { - e.addTrace( - {}, "while parsing file '%s' as a realisation for key '%s'", outputInfoFilePath, id.to_string()); + e.addTrace({}, "while parsing file '%s' as a realisation", outputInfoFilePath); throw; } return (*callbackPtr)(std::move(realisation)); @@ -541,7 +535,8 @@ void BinaryCacheStore::registerDrvOutput(const Realisation & info) { if (diskCache) diskCache->upsertRealisation(config.getReference().render(/*FIXME withParams=*/false), info); - upsertFile(makeRealisationPath(info.id), static_cast(info).dump(), "application/json"); + auto filePath = realisationsPrefix + "/" + info.id.to_string() + ".doi"; + upsertFile(filePath, static_cast(info).dump(), "application/json"); } ref BinaryCacheStore::getRemoteFSAccessor(bool requireValidPath) diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index c39fd8c1c..001816ca0 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1,5 +1,6 @@ #include "nix/store/build/derivation-building-goal.hh" #include "nix/store/build/derivation-env-desugar.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -26,8 +27,8 @@ namespace nix { DerivationBuildingGoal::DerivationBuildingGoal( - const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode, bool storeDerivation) - : Goal(worker, gaveUpOnSubstitution(storeDerivation)) + const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode) + : Goal(worker, gaveUpOnSubstitution()) , drvPath(drvPath) , buildMode(buildMode) { @@ -124,10 +125,50 @@ static void runPostBuildHook( /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ -Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) +Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() { Goals waitees; + std::map, GoalPtr, value_comparison> inputGoals; + + { + std::function, const DerivedPathMap::ChildNode &)> + addWaiteeDerivedPath; + + 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, childNode] : inputNode.childMap) + addWaiteeDerivedPath( + make_ref(SingleDerivedPath::Built{inputDrv, outputName}), childNode); + }; + + for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) { + /* Ensure that pure, non-fixed-output derivations don't + depend on impure derivations. */ + if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure() + && !drv->type().isFixed()) { + auto inputDrv = worker.evalStore.readDerivation(inputDrvPath); + if (inputDrv.type().isImpure()) + throw Error( + "pure derivation '%s' depends on impure derivation '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(inputDrvPath)); + } + + addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode); + } + } + /* Copy the input sources from the eval store to the build store. @@ -172,17 +213,177 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution(bool storeDerivation) /* Determine the full set of input paths. */ - if (storeDerivation) { - assert(drv->inputDrvs.map.empty()); - /* Store the resolved derivation, as part of the record of - what we're actually building */ - writeDerivation(worker.store, *drv); - } - + /* First, the input derivations. */ { + auto & fullDrv = *drv; + + auto drvType = fullDrv.type(); + bool resolveDrv = + std::visit( + overloaded{ + [&](const DerivationType::InputAddressed & ia) { + /* must resolve if deferred. */ + return ia.deferred; + }, + [&](const DerivationType::ContentAddressed & ca) { + return !fullDrv.inputDrvs.map.empty() + && (ca.fixed + /* Can optionally resolve if fixed, which is good + for avoiding unnecessary rebuilds. */ + ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations) + /* Must resolve if floating and there are any inputs + drvs. */ + : true); + }, + [&](const DerivationType::Impure &) { return true; }}, + drvType.raw) + /* no inputs are outputs of dynamic derivations */ + || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) { + return !pair.second.childMap.empty(); + }); + + if (resolveDrv && !fullDrv.inputDrvs.map.empty()) { + experimentalFeatureSettings.require(Xp::CaDerivations); + + /* We are be able to resolve this derivation based on the + now-known results of dependencies. If so, we become a + 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) + 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); + }); + if (!attempt) { + /* TODO (impure derivations-induced tech debt) (see below): + The above attempt should have found it, but because we manage + inputDrvOutputs statefully, sometimes it gets out of sync with + the real source of truth (store). So we query the store + directly if there's a problem. */ + attempt = fullDrv.tryResolve(worker.store, &worker.evalStore); + } + assert(attempt); + Derivation drvResolved{std::move(*attempt)}; + + auto pathResolved = writeDerivation(worker.store, drvResolved); + + auto msg = + fmt("resolved derivation: '%s' -> '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved)); + act = std::make_unique( + *logger, + lvlInfo, + actBuildWaiting, + msg, + Logger::Fields{ + worker.store.printStorePath(drvPath), + worker.store.printStorePath(pathResolved), + }); + + /* TODO https://github.com/NixOS/nix/issues/13247 we should + let the calling goal do this, so it has a change to pass + just the output(s) it cares about. */ + auto resolvedDrvGoal = + worker.makeDerivationTrampolineGoal(pathResolved, OutputsSpec::All{}, drvResolved, buildMode); + { + Goals waitees{resolvedDrvGoal}; + co_await await(std::move(waitees)); + } + + trace("resolved derivation finished"); + + auto resolvedResult = resolvedDrvGoal->buildResult; + + // No `std::visit` for coroutines yet + if (auto * successP = resolvedResult.tryGetSuccess()) { + auto & success = *successP; + SingleDrvOutputs builtOutputs; + + auto outputHashes = staticOutputHashes(worker.evalStore, *drv); + auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); + + StorePathSet outputPaths; + + for (auto & outputName : drvResolved.outputNames()) { + auto outputHash = get(outputHashes, outputName); + auto resolvedHash = get(resolvedHashes, outputName); + if ((!outputHash) || (!resolvedHash)) + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)", + worker.store.printStorePath(drvPath), + outputName); + + auto realisation = [&] { + auto take1 = get(success.builtOutputs, outputName); + if (take1) + return *take1; + + /* The above `get` should work. But stateful tracking of + outputs in resolvedResult, this can get out of sync with the + store, which is our actual source of truth. For now we just + check the store directly if it fails. */ + auto take2 = worker.evalStore.queryRealisation(DrvOutput{*resolvedHash, outputName}); + if (take2) + return *take2; + + throw Error( + "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", + worker.store.printStorePath(pathResolved), + outputName); + }(); + + if (!drv->type().isImpure()) { + auto newRealisation = realisation; + newRealisation.id = DrvOutput{*outputHash, outputName}; + newRealisation.signatures.clear(); + if (!drv->type().isFixed()) { + auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store; + newRealisation.dependentRealisations = + drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); + } + worker.store.signRealisation(newRealisation); + worker.store.registerDrvOutput(newRealisation); + } + outputPaths.insert(realisation.outPath); + builtOutputs.emplace(outputName, realisation); + } + + runPostBuildHook(worker.store, *logger, drvPath, outputPaths); + + auto status = success.status; + if (status == BuildResult::Success::AlreadyValid) + status = BuildResult::Success::ResolvesToAlreadyValid; + + co_return doneSuccess(success.status, std::move(builtOutputs)); + } else if (resolvedResult.tryGetFailure()) { + co_return doneFailure({ + BuildResult::Failure::DependencyFailed, + "build of resolved derivation '%s' failed", + worker.store.printStorePath(pathResolved), + }); + } else + assert(false); + } + /* If we get this far, we know no dynamic drvs inputs */ - for (auto & [depDrvPath, depNode] : drv->inputDrvs.map) { + for (auto & [depDrvPath, depNode] : fullDrv.inputDrvs.map) { for (auto & outputName : depNode.value) { /* Don't need to worry about `inputGoals`, because impure derivations are always resolved above. Can @@ -1092,22 +1293,13 @@ DerivationBuildingGoal::checkPathValidity(std::map & // without the `ca-derivations` experimental flag). worker.store.registerDrvOutput( Realisation{ - { - .outPath = info.known->path, - }, drvOutput, + info.known->path, }); } } if (info.known && info.known->isValid()) - validOutputs.emplace( - i.first, - Realisation{ - { - .outPath = info.known->path, - }, - drvOutput, - }); + validOutputs.emplace(i.first, Realisation{drvOutput, info.known->path}); } bool allValid = true; diff --git a/src/libstore/build/derivation-check.cc b/src/libstore/build/derivation-check.cc index db3ec7c3d..181221ba5 100644 --- a/src/libstore/build/derivation-check.cc +++ b/src/libstore/build/derivation-check.cc @@ -18,7 +18,11 @@ void checkOutputs( for (auto & output : outputs) outputsByPath.emplace(store.printStorePath(output.second.path), output.second); - for (auto & [outputName, info] : outputs) { + for (auto & pair : outputs) { + // We can't use auto destructuring here because + // clang-tidy seems to complain about it. + const std::string & outputName = pair.first; + const auto & info = pair.second; auto * outputSpec = get(drvOutputs, outputName); assert(outputSpec); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 81f4e6654..2e57c1708 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1,6 +1,5 @@ #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-building-goal.hh" -#include "nix/store/build/derivation-resolution-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -30,9 +29,8 @@ DerivationGoal::DerivationGoal( const Derivation & drv, const OutputName & wantedOutput, Worker & worker, - BuildMode buildMode, - bool storeDerivation) - : Goal(worker, haveDerivation(storeDerivation)) + BuildMode buildMode) + : Goal(worker, haveDerivation()) , drvPath(drvPath) , wantedOutput(wantedOutput) , outputHash{[&] { @@ -66,7 +64,7 @@ std::string DerivationGoal::key() }.to_string(worker.store); } -Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) +Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); @@ -148,104 +146,9 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) worker.store.printStorePath(drvPath)); } - auto resolutionGoal = worker.makeDerivationResolutionGoal(drvPath, *drv, buildMode); - { - Goals waitees{resolutionGoal}; - co_await await(std::move(waitees)); - } - if (nrFailed != 0) { - co_return doneFailure({BuildResult::Failure::DependencyFailed, "resolution failed"}); - } - - if (resolutionGoal->resolvedDrv) { - auto & [pathResolved, drvResolved] = *resolutionGoal->resolvedDrv; - - auto resolvedDrvGoal = - worker.makeDerivationGoal(pathResolved, drvResolved, wantedOutput, buildMode, /*storeDerivation=*/true); - { - Goals waitees{resolvedDrvGoal}; - co_await await(std::move(waitees)); - } - - trace("resolved derivation finished"); - - auto resolvedResult = resolvedDrvGoal->buildResult; - - // No `std::visit` for coroutines yet - if (auto * successP = resolvedResult.tryGetSuccess()) { - auto & success = *successP; - auto outputHashes = staticOutputHashes(worker.evalStore, *drv); - auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); - - StorePathSet outputPaths; - - auto outputHash = get(outputHashes, wantedOutput); - auto resolvedHash = get(resolvedHashes, wantedOutput); - if ((!outputHash) || (!resolvedHash)) - throw Error( - "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)", - worker.store.printStorePath(drvPath), - wantedOutput); - - auto realisation = [&] { - auto take1 = get(success.builtOutputs, wantedOutput); - if (take1) - return static_cast(*take1); - - /* The above `get` should work. But stateful tracking of - outputs in resolvedResult, this can get out of sync with the - store, which is our actual source of truth. For now we just - check the store directly if it fails. */ - auto take2 = worker.evalStore.queryRealisation( - DrvOutput{ - .drvHash = *resolvedHash, - .outputName = wantedOutput, - }); - if (take2) - return *take2; - - throw Error( - "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", - worker.store.printStorePath(pathResolved), - wantedOutput); - }(); - - if (!drv->type().isImpure()) { - Realisation newRealisation{ - realisation, - { - .drvHash = *outputHash, - .outputName = wantedOutput, - }}; - newRealisation.signatures.clear(); - if (!drv->type().isFixed()) { - auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store; - newRealisation.dependentRealisations = - drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); - } - worker.store.signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - } - outputPaths.insert(realisation.outPath); - - auto status = success.status; - if (status == BuildResult::Success::AlreadyValid) - status = BuildResult::Success::ResolvesToAlreadyValid; - - co_return doneSuccess(status, std::move(realisation)); - } else if (resolvedResult.tryGetFailure()) { - co_return doneFailure({ - BuildResult::Failure::DependencyFailed, - "build of resolved derivation '%s' failed", - worker.store.printStorePath(pathResolved), - }); - } else - assert(false); - } - /* Give up on substitution for the output we want, actually build this derivation */ - auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode, storeDerivation); + auto g = worker.makeDerivationBuildingGoal(drvPath, *drv, buildMode); /* We will finish with it ourselves, as if we were the derivational goal. */ g->preserveException = true; @@ -266,16 +169,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) /* In checking mode, the builder will not register any outputs. So we want to make sure the ones that we wanted to check are properly there. */ - success.builtOutputs = {{ - wantedOutput, - { - assertPathValidity(), - { - .drvHash = outputHash, - .outputName = wantedOutput, - }, - }, - }}; + success.builtOutputs = {{wantedOutput, assertPathValidity()}}; } else { /* Otherwise the builder will give us info for out output, but also for other outputs. Filter down to just our output so as @@ -378,9 +272,10 @@ Goal::Co DerivationGoal::repairClosure() bmRepair)); } + bool haveWaitees = !waitees.empty(); co_await await(std::move(waitees)); - if (!waitees.empty()) { + if (haveWaitees) { trace("closure repaired"); if (nrFailed > 0) throw Error( @@ -390,20 +285,18 @@ Goal::Co DerivationGoal::repairClosure() co_return doneSuccess(BuildResult::Success::AlreadyValid, assertPathValidity()); } -std::optional> DerivationGoal::checkPathValidity() +std::optional> DerivationGoal::checkPathValidity() { if (drv->type().isImpure()) return std::nullopt; auto drvOutput = DrvOutput{outputHash, wantedOutput}; - std::optional mRealisation; + std::optional mRealisation; if (auto * mOutput = get(drv->outputs, wantedOutput)) { if (auto mPath = mOutput->path(worker.store, drv->name, wantedOutput)) { - mRealisation = UnkeyedRealisation{ - .outPath = std::move(*mPath), - }; + mRealisation = Realisation{drvOutput, std::move(*mPath)}; } } else { throw Error( @@ -431,14 +324,7 @@ std::optional> DerivationGoal::checkPa // derivation, and the output path is valid, but we don't have // its realisation stored (probably because it has been built // without the `ca-derivations` experimental flag). - worker.store.registerDrvOutput( - Realisation{ - *mRealisation, - { - .drvHash = outputHash, - .outputName = wantedOutput, - }, - }); + worker.store.registerDrvOutput(*mRealisation); } return {{*mRealisation, status}}; @@ -446,7 +332,7 @@ std::optional> DerivationGoal::checkPa return std::nullopt; } -UnkeyedRealisation DerivationGoal::assertPathValidity() +Realisation DerivationGoal::assertPathValidity() { auto checkResult = checkPathValidity(); if (!(checkResult && checkResult->second == PathStatus::Valid)) @@ -454,20 +340,11 @@ UnkeyedRealisation DerivationGoal::assertPathValidity() return checkResult->first; } -Goal::Done DerivationGoal::doneSuccess(BuildResult::Success::Status status, UnkeyedRealisation builtOutput) +Goal::Done DerivationGoal::doneSuccess(BuildResult::Success::Status status, Realisation builtOutput) { buildResult.inner = BuildResult::Success{ .status = status, - .builtOutputs = {{ - wantedOutput, - { - std::move(builtOutput), - DrvOutput{ - .drvHash = outputHash, - .outputName = wantedOutput, - }, - }, - }}, + .builtOutputs = {{wantedOutput, std::move(builtOutput)}}, }; mcExpectedBuilds.reset(); diff --git a/src/libstore/build/derivation-resolution-goal.cc b/src/libstore/build/derivation-resolution-goal.cc deleted file mode 100644 index 584169ef3..000000000 --- a/src/libstore/build/derivation-resolution-goal.cc +++ /dev/null @@ -1,210 +0,0 @@ -#include "nix/store/build/derivation-resolution-goal.hh" -#include "nix/store/build/derivation-env-desugar.hh" -#include "nix/store/build/worker.hh" -#include "nix/util/util.hh" -#include "nix/store/common-protocol.hh" -#include "nix/store/globals.hh" - -#include -#include -#include - -#include - -namespace nix { - -DerivationResolutionGoal::DerivationResolutionGoal( - const StorePath & drvPath, const Derivation & drv_, Worker & worker, BuildMode buildMode) - : Goal(worker, resolveDerivation()) - , drvPath(drvPath) -{ - drv = std::make_unique(drv_); - - name = fmt("building of '%s' from in-memory derivation", worker.store.printStorePath(drvPath)); - trace("created"); - - /* Prevent the .chroot directory from being - garbage-collected. (See isActiveTempFile() in gc.cc.) */ - worker.store.addTempRoot(this->drvPath); -} - -void DerivationResolutionGoal::timedOut(Error && ex) {} - -std::string DerivationResolutionGoal::key() -{ - /* Ensure that derivations get built in order of their name, - i.e. a derivation named "aardvark" always comes before - "baboon". And substitution goals always happen before - derivation goals (due to "bd$"). */ - return "rd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); -} - -/** - * Used for `inputGoals` local variable below - */ -struct value_comparison -{ - template - bool operator()(const ref & lhs, const ref & rhs) const - { - return *lhs < *rhs; - } -}; - -/* At least one of the output paths could not be - produced using a substitute. So we have to build instead. */ -Goal::Co DerivationResolutionGoal::resolveDerivation() -{ - Goals waitees; - - std::map, GoalPtr, value_comparison> inputGoals; - - { - std::function, const DerivedPathMap::ChildNode &)> - addWaiteeDerivedPath; - - 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, childNode] : inputNode.childMap) - addWaiteeDerivedPath( - make_ref(SingleDerivedPath::Built{inputDrv, outputName}), childNode); - }; - - for (const auto & [inputDrvPath, inputNode] : drv->inputDrvs.map) { - /* Ensure that pure, non-fixed-output derivations don't - depend on impure derivations. */ - if (experimentalFeatureSettings.isEnabled(Xp::ImpureDerivations) && !drv->type().isImpure() - && !drv->type().isFixed()) { - auto inputDrv = worker.evalStore.readDerivation(inputDrvPath); - if (inputDrv.type().isImpure()) - throw Error( - "pure derivation '%s' depends on impure derivation '%s'", - worker.store.printStorePath(drvPath), - worker.store.printStorePath(inputDrvPath)); - } - - addWaiteeDerivedPath(makeConstantStorePathRef(inputDrvPath), inputNode); - } - } - - co_await await(std::move(waitees)); - - trace("all inputs realised"); - - if (nrFailed != 0) { - auto msg = - fmt("Cannot build '%s'.\n" - "Reason: " ANSI_RED "%d %s failed" ANSI_NORMAL ".", - Magenta(worker.store.printStorePath(drvPath)), - nrFailed, - nrFailed == 1 ? "dependency" : "dependencies"); - msg += showKnownOutputs(worker.store, *drv); - co_return amDone(ecFailed, {BuildError(BuildResult::Failure::DependencyFailed, msg)}); - } - - /* Gather information necessary for computing the closure and/or - running the build hook. */ - - /* Determine the full set of input paths. */ - - /* First, the input derivations. */ - { - auto & fullDrv = *drv; - - auto drvType = fullDrv.type(); - bool resolveDrv = - std::visit( - overloaded{ - [&](const DerivationType::InputAddressed & ia) { - /* must resolve if deferred. */ - return ia.deferred; - }, - [&](const DerivationType::ContentAddressed & ca) { - return !fullDrv.inputDrvs.map.empty() - && (ca.fixed - /* Can optionally resolve if fixed, which is good - for avoiding unnecessary rebuilds. */ - ? experimentalFeatureSettings.isEnabled(Xp::CaDerivations) - /* Must resolve if floating and there are any inputs - drvs. */ - : true); - }, - [&](const DerivationType::Impure &) { return true; }}, - drvType.raw) - /* no inputs are outputs of dynamic derivations */ - || std::ranges::any_of(fullDrv.inputDrvs.map.begin(), fullDrv.inputDrvs.map.end(), [](auto & pair) { - return !pair.second.childMap.empty(); - }); - - if (resolveDrv && !fullDrv.inputDrvs.map.empty()) { - experimentalFeatureSettings.require(Xp::CaDerivations); - - /* We are be able to resolve this derivation based on the - now-known results of dependencies. If so, we become a - 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) - 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); - }); - if (!attempt) { - /* TODO (impure derivations-induced tech debt) (see below): - The above attempt should have found it, but because we manage - inputDrvOutputs statefully, sometimes it gets out of sync with - the real source of truth (store). So we query the store - directly if there's a problem. */ - attempt = fullDrv.tryResolve(worker.store, &worker.evalStore); - } - assert(attempt); - - auto pathResolved = writeDerivation(worker.store, *attempt, NoRepair, /*readOnly =*/true); - - auto msg = - fmt("resolved derivation: '%s' -> '%s'", - worker.store.printStorePath(drvPath), - worker.store.printStorePath(pathResolved)); - act = std::make_unique( - *logger, - lvlInfo, - actBuildWaiting, - msg, - Logger::Fields{ - worker.store.printStorePath(drvPath), - worker.store.printStorePath(pathResolved), - }); - - resolvedDrv = - std::make_unique>(std::move(pathResolved), *std::move(attempt)); - } - } - - co_return amDone(ecSuccess, std::nullopt); -} - -} // namespace nix diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index a969b905b..b6ace4784 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -43,10 +43,10 @@ Goal::Co DrvOutputSubstitutionGoal::init() outPipe->createAsyncPipe(worker.ioport.get()); #endif - auto promise = std::make_shared>>(); + auto promise = std::make_shared>>(); sub->queryRealisation( - id, {[outPipe(outPipe), promise(promise)](std::future> res) { + id, {[outPipe(outPipe), promise(promise)](std::future> res) { try { Finally updateStats([&]() { outPipe->writeSide.close(); }); promise->set_value(res.get()); @@ -75,7 +75,7 @@ Goal::Co DrvOutputSubstitutionGoal::init() * The realisation corresponding to the given output id. * Will be filled once we can get it. */ - std::shared_ptr outputInfo; + std::shared_ptr outputInfo; try { outputInfo = promise->get_future().get(); @@ -132,7 +132,7 @@ Goal::Co DrvOutputSubstitutionGoal::init() } Goal::Co DrvOutputSubstitutionGoal::realisationFetched( - Goals waitees, std::shared_ptr outputInfo, nix::ref sub) + Goals waitees, std::shared_ptr outputInfo, nix::ref sub) { waitees.insert(worker.makePathSubstitutionGoal(outputInfo->outPath)); @@ -145,7 +145,7 @@ Goal::Co DrvOutputSubstitutionGoal::realisationFetched( co_return amDone(nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed); } - worker.store.registerDrvOutput({*outputInfo, id}); + worker.store.registerDrvOutput(*outputInfo); trace("finished"); co_return amDone(ecSuccess); diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 53175a8c4..3e6e0bef0 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -4,7 +4,6 @@ #include "nix/store/build/substitution-goal.hh" #include "nix/store/build/drv-output-substitution-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" #include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows @@ -76,26 +75,15 @@ std::shared_ptr Worker::makeDerivationTrampolineGoal( } std::shared_ptr Worker::makeDerivationGoal( - const StorePath & drvPath, - const Derivation & drv, - const OutputName & wantedOutput, - BuildMode buildMode, - bool storeDerivation) + const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode) { - return initGoalIfNeeded( - derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode, storeDerivation); + return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode); } -std::shared_ptr -Worker::makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) +std::shared_ptr +Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) { - return initGoalIfNeeded(derivationResolutionGoals[drvPath], drvPath, drv, *this, buildMode); -} - -std::shared_ptr Worker::makeDerivationBuildingGoal( - const StorePath & drvPath, const Derivation & drv, BuildMode buildMode, bool storeDerivation) -{ - return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode, storeDerivation); + return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode); } std::shared_ptr @@ -170,8 +158,6 @@ void Worker::removeGoal(GoalPtr goal) nix::removeGoal(drvGoal, derivationTrampolineGoals.map); else if (auto drvGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvGoal, derivationGoals); - else if (auto drvResolutionGoal = std::dynamic_pointer_cast(goal)) - nix::removeGoal(drvResolutionGoal, derivationResolutionGoals); else if (auto drvBuildingGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvBuildingGoal, derivationBuildingGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 2898f113f..1fc568e87 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -102,7 +102,7 @@ struct TunnelLogger : public Logger showErrorInfo(oss, ei, false); StringSink buf; - buf << STDERR_NEXT << toView(oss); + buf << STDERR_NEXT << oss.view(); enqueueMsg(buf.s); } @@ -964,7 +964,7 @@ static void performOp( if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { auto outputId = DrvOutput::parse(readString(conn.from)); auto outputPath = StorePath(readString(conn.from)); - store->registerDrvOutput(Realisation{{.outPath = outputPath}, outputId}); + store->registerDrvOutput(Realisation{.id = outputId, .outPath = outputPath}); } else { auto realisation = WorkerProto::Serialise::read(*store, rconn); store->registerDrvOutput(realisation); @@ -986,7 +986,7 @@ static void performOp( } else { std::set realisations; if (info) - realisations.insert({*info, outputId}); + realisations.insert(*info); WorkerProto::write(*store, wconn, realisations); } break; diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 509b7a0b1..1eb51fe3e 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -3,7 +3,6 @@ #include "nix/util/callback.hh" #include "nix/util/memory-source-accessor.hh" #include "nix/store/dummy-store-impl.hh" -#include "nix/store/realisation.hh" #include @@ -252,10 +251,7 @@ struct DummyStoreImpl : DummyStore void registerDrvOutput(const Realisation & output) override { - auto ref = make_ref(output); - buildTrace.insert_or_visit({output.id.drvHash, {{output.id.outputName, ref}}}, [&](auto & kv) { - kv.second.insert_or_assign(output.id.outputName, make_ref(output)); - }); + unsupported("registerDrvOutput"); } void narFromPath(const StorePath & path, Sink & sink) override @@ -270,19 +266,10 @@ struct DummyStoreImpl : DummyStore throw Error("path '%s' is not valid", printStorePath(path)); } - void queryRealisationUncached( - const DrvOutput & drvOutput, Callback> callback) noexcept override + void + queryRealisationUncached(const DrvOutput &, Callback> callback) noexcept override { - bool visited = false; - buildTrace.cvisit(drvOutput.drvHash, [&](const auto & kv) { - if (auto it = kv.second.find(drvOutput.outputName); it != kv.second.end()) { - visited = true; - callback(it->second.get_ptr()); - } - }); - - if (!visited) - callback(nullptr); + callback(nullptr); } std::shared_ptr getFSAccessor(const StorePath & path, bool requireValidPath) override diff --git a/src/libstore/include/nix/store/binary-cache-store.hh b/src/libstore/include/nix/store/binary-cache-store.hh index 3f4de2bd4..c316b1199 100644 --- a/src/libstore/include/nix/store/binary-cache-store.hh +++ b/src/libstore/include/nix/store/binary-cache-store.hh @@ -80,22 +80,13 @@ private: protected: - /** - * The prefix under which realisation infos will be stored - */ - constexpr const static std::string realisationsPrefix = "realisations"; + // The prefix under which realisation infos will be stored + const std::string realisationsPrefix = "realisations"; - constexpr const static std::string cacheInfoFile = "nix-cache-info"; + const std::string cacheInfoFile = "nix-cache-info"; BinaryCacheStore(Config &); - /** - * Compute the path to the given realisation - * - * It's `${realisationsPrefix}/${drvOutput}.doi`. - */ - std::string makeRealisationPath(const DrvOutput & id); - public: virtual bool fileExists(const std::string & path) = 0; @@ -184,7 +175,7 @@ public: void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept override; + const DrvOutput &, Callback> callback) noexcept override; void narFromPath(const StorePath & path, Sink & sink) override; diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index ab063ff3f..edb496024 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -29,21 +29,8 @@ typedef enum { rpAccept, rpDecline, rpPostpone } HookReply; */ struct DerivationBuildingGoal : public Goal { - /** - * @param storeDerivation Whether to store the derivation in - * `worker.store`. This is useful for newly-resolved derivations. In this - * case, the derivation was not created a priori, e.g. purely (or close - * enough) from evaluation of the Nix language, but also depends on the - * exact content produced by upstream builds. It is strongly advised to - * have a permanent record of such a resolved derivation in order to - * faithfully reconstruct the build history. - */ DerivationBuildingGoal( - const StorePath & drvPath, - const Derivation & drv, - Worker & worker, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationBuildingGoal(); private: @@ -113,7 +100,7 @@ private: /** * The states. */ - Co gaveUpOnSubstitution(bool storeDerivation); + Co gaveUpOnSubstitution(); Co tryToBuild(); /** @@ -168,7 +155,7 @@ private: JobCategory jobCategory() const override { - return JobCategory::Administration; + return JobCategory::Build; }; }; diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index c31645fff..e05bf1c0b 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -40,16 +40,12 @@ struct DerivationGoal : public Goal */ OutputName wantedOutput; - /** - * @param storeDerivation See `DerivationBuildingGoal`. This is just passed along. - */ DerivationGoal( const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, Worker & worker, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + BuildMode buildMode = bmNormal); ~DerivationGoal() = default; void timedOut(Error && ex) override @@ -84,7 +80,7 @@ private: /** * The states. */ - Co haveDerivation(bool storeDerivation); + Co haveDerivation(); /** * Return `std::nullopt` if the output is unknown, e.g. un unbuilt @@ -93,17 +89,17 @@ private: * of the wanted output, and a `PathStatus` with the * current status of that output. */ - std::optional> checkPathValidity(); + std::optional> checkPathValidity(); /** * Aborts if any output is not valid or corrupt, and otherwise * returns a 'Realisation' for the wanted output. */ - UnkeyedRealisation assertPathValidity(); + Realisation assertPathValidity(); Co repairClosure(); - Done doneSuccess(BuildResult::Success::Status status, UnkeyedRealisation builtOutput); + Done doneSuccess(BuildResult::Success::Status status, Realisation builtOutput); Done doneFailure(BuildError ex); }; diff --git a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh b/src/libstore/include/nix/store/build/derivation-resolution-goal.hh deleted file mode 100644 index ebaab4f06..000000000 --- a/src/libstore/include/nix/store/build/derivation-resolution-goal.hh +++ /dev/null @@ -1,82 +0,0 @@ -#pragma once -///@file - -#include "nix/store/derivations.hh" -#include "nix/store/derivation-options.hh" -#include "nix/store/build/derivation-building-misc.hh" -#include "nix/store/store-api.hh" -#include "nix/store/build/goal.hh" - -namespace nix { - -struct BuilderFailureError; - -/** - * A goal for resolving a derivation. Resolving a derivation (@see - * `Derivation::tryResolve`) simplifies its inputs, replacing - * `inputDrvs` with `inputSrcs. - * - * Conceptually, we resolve all derivations. For input-addressed - * derivations (that don't transtively depend on content-addressed - * derivations), however, we don't actually use the resolved derivation, - * because the output paths would appear invalid (if we tried to verify - * them), since they are computed from the original, unresolved inputs. - * - * That said, if we ever made the new flavor of input-addressing as described - * in issue #9259, then the input-addressing would be based on the resolved - * inputs, and we like the CA case *would* use the output of this goal. - * - * (The point of this discussion is not to randomly stuff information on - * a yet-unimplemented feature (issue #9259) in the codebase, but - * rather, to illustrate that there is no inherent tension between - * explicit derivation resolution and input-addressing in general. That - * tension only exists with the type of input-addressing we've - * historically used.) - */ -struct DerivationResolutionGoal : public Goal -{ - DerivationResolutionGoal( - const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal); - - /** - * If the derivation needed to be resolved, this is resulting - * resolved derivations and its path. - */ - std::unique_ptr> resolvedDrv; - - void timedOut(Error && ex) override; - -private: - - /** - * The path of the derivation. - */ - StorePath drvPath; - - /** - * The derivation stored at drvPath. - */ - std::unique_ptr drv; - - /** - * The remainder is state held during the build. - */ - - BuildMode buildMode; - - std::unique_ptr act; - - std::string key() override; - - /** - * The states. - */ - Co resolveDerivation(); - - JobCategory jobCategory() const override - { - return JobCategory::Administration; - }; -}; - -} // 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 1a5a4ea26..b42336427 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 @@ -39,8 +39,7 @@ public: GoalState state; Co init(); - Co - realisationFetched(Goals waitees, std::shared_ptr outputInfo, nix::ref sub); + Co realisationFetched(Goals waitees, std::shared_ptr outputInfo, nix::ref sub); void timedOut(Error && ex) override { diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index 9767590ac..a6de780c1 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -16,7 +16,6 @@ namespace nix { /* Forward definition. */ struct DerivationTrampolineGoal; struct DerivationGoal; -struct DerivationResolutionGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; @@ -112,7 +111,6 @@ private: DerivedPathMap>> derivationTrampolineGoals; std::map>> derivationGoals; - std::map> derivationResolutionGoals; std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -223,23 +221,13 @@ public: const StorePath & drvPath, const Derivation & drv, const OutputName & wantedOutput, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + BuildMode buildMode = bmNormal); /** - * @ref DerivationResolutionGoal "derivation resolution goal" + * @ref DerivationBuildingGoal "derivation goal" */ - std::shared_ptr - makeDerivationResolutionGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal); - - /** - * @ref DerivationBuildingGoal "derivation building goal" - */ - std::shared_ptr makeDerivationBuildingGoal( - const StorePath & drvPath, - const Derivation & drv, - BuildMode buildMode = bmNormal, - bool storeDerivation = false); + std::shared_ptr + makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode = bmNormal); /** * @ref PathSubstitutionGoal "substitution goal" diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 4c9f54e98..e05bb94ff 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/src/libstore/include/nix/store/dummy-store-impl.hh @@ -30,18 +30,6 @@ struct DummyStore : virtual Store */ boost::concurrent_flat_map contents; - /** - * The build trace maps the pair of a content-addressing (fixed or - * floating) derivations an one of its output to a - * (content-addressed) store object. - * - * It is [curried](https://en.wikipedia.org/wiki/Currying), so we - * instead having a single output with a `DrvOutput` key, we have an - * outer map for the derivation, and inner maps for the outputs of a - * given derivation. - */ - boost::concurrent_flat_map>> buildTrace; - DummyStore(ref config) : Store{*config} , config(config) diff --git a/src/libstore/include/nix/store/dummy-store.hh b/src/libstore/include/nix/store/dummy-store.hh index d371c4e51..95c09078c 100644 --- a/src/libstore/include/nix/store/dummy-store.hh +++ b/src/libstore/include/nix/store/dummy-store.hh @@ -3,8 +3,6 @@ #include "nix/store/store-api.hh" -#include - namespace nix { struct DummyStore; diff --git a/src/libstore/include/nix/store/legacy-ssh-store.hh b/src/libstore/include/nix/store/legacy-ssh-store.hh index 994918f90..c91f88a84 100644 --- a/src/libstore/include/nix/store/legacy-ssh-store.hh +++ b/src/libstore/include/nix/store/legacy-ssh-store.hh @@ -208,8 +208,8 @@ public: */ std::optional isTrustedClient() override; - void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept override + void + queryRealisationUncached(const DrvOutput &, Callback> callback) noexcept override // TODO: Implement { unsupported("queryRealisation"); diff --git a/src/libstore/include/nix/store/local-overlay-store.hh b/src/libstore/include/nix/store/local-overlay-store.hh index 1d69d3417..b89d0a1a0 100644 --- a/src/libstore/include/nix/store/local-overlay-store.hh +++ b/src/libstore/include/nix/store/local-overlay-store.hh @@ -173,7 +173,7 @@ private: * Check lower store if upper DB does not have. */ void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept override; + const DrvOutput &, Callback> callback) noexcept override; /** * Call `remountIfNecessary` after collecting garbage normally. diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index ab255fba8..b871aaee2 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -385,10 +385,10 @@ public: void cacheDrvOutputMapping( State & state, const uint64_t deriver, const std::string & outputName, const StorePath & output); - std::optional queryRealisation_(State & state, const DrvOutput & id); - std::optional> queryRealisationCore_(State & state, const DrvOutput & id); + std::optional queryRealisation_(State & state, const DrvOutput & id); + std::optional> queryRealisationCore_(State & state, const DrvOutput & id); void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept override; + const DrvOutput &, Callback> callback) noexcept override; std::optional getVersion() override; diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index 1f04e357a..c9e4c36dd 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -18,7 +18,6 @@ headers = [ config_pub_h ] + files( 'build/derivation-building-misc.hh', 'build/derivation-env-desugar.hh', 'build/derivation-goal.hh', - 'build/derivation-resolution-goal.hh', 'build/derivation-trampoline-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index c7e0a4483..3424a39c9 100644 --- a/src/libstore/include/nix/store/realisation.hh +++ b/src/libstore/include/nix/store/realisation.hh @@ -46,12 +46,12 @@ struct DrvOutput static DrvOutput parse(const std::string &); - bool operator==(const DrvOutput &) const = default; - auto operator<=>(const DrvOutput &) const = default; + GENERATE_CMP(DrvOutput, me->drvHash, me->outputName); }; -struct UnkeyedRealisation +struct Realisation { + DrvOutput id; StorePath outPath; StringSet signatures; @@ -64,35 +64,22 @@ struct UnkeyedRealisation */ std::map dependentRealisations; - std::string fingerprint(const DrvOutput & key) const; + std::string fingerprint() const; + void sign(const Signer &); + bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; + size_t checkSignatures(const PublicKeys & publicKeys) const; - void sign(const DrvOutput & key, const Signer &); + static std::set closure(Store &, const std::set &); + static void closure(Store &, const std::set &, std::set & res); - bool checkSignature(const DrvOutput & key, const PublicKeys & publicKeys, const std::string & sig) const; + bool isCompatibleWith(const Realisation & other) const; - size_t checkSignatures(const DrvOutput & key, const PublicKeys & publicKeys) const; - - const StorePath & getPath() const + StorePath getPath() const { return outPath; } - // TODO sketchy that it avoids signatures - GENERATE_CMP(UnkeyedRealisation, me->outPath); -}; - -struct Realisation : UnkeyedRealisation -{ - DrvOutput id; - - bool isCompatibleWith(const UnkeyedRealisation & other) const; - - static std::set closure(Store &, const std::set &); - - static void closure(Store &, const std::set &, std::set & res); - - bool operator==(const Realisation &) const = default; - auto operator<=>(const Realisation &) const = default; + GENERATE_CMP(Realisation, me->id, me->outPath); }; /** @@ -116,13 +103,12 @@ struct OpaquePath { StorePath path; - const StorePath & getPath() const + StorePath getPath() const { return path; } - bool operator==(const OpaquePath &) const = default; - auto operator<=>(const OpaquePath &) const = default; + GENERATE_CMP(OpaquePath, me->path); }; /** @@ -130,7 +116,7 @@ struct OpaquePath */ struct RealisedPath { - /** + /* * A path is either the result of the realisation of a derivation or * an opaque blob that has been directly added to the store */ @@ -152,14 +138,13 @@ struct RealisedPath /** * Get the raw store path associated to this */ - const StorePath & path() const; + StorePath path() const; void closure(Store & store, Set & ret) const; static void closure(Store & store, const Set & startPaths, Set & ret); Set closure(Store & store) const; - bool operator==(const RealisedPath &) const = default; - auto operator<=>(const RealisedPath &) const = default; + GENERATE_CMP(RealisedPath, me->raw); }; class MissingRealisation : public Error @@ -182,5 +167,4 @@ public: } // namespace nix -JSON_IMPL(nix::UnkeyedRealisation) JSON_IMPL(nix::Realisation) diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index b152e054b..1aaf29d37 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -102,7 +102,7 @@ struct RemoteStore : public virtual Store, public virtual GcStore, public virtua void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept override; + const DrvOutput &, Callback> callback) noexcept override; void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index c9fd00513..1131ec975 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -31,7 +31,6 @@ MakeError(SubstituterDisabled, Error); MakeError(InvalidStoreReference, Error); -struct UnkeyedRealisation; struct Realisation; struct RealisedPath; struct DrvOutput; @@ -399,12 +398,12 @@ public: /** * Query the information about a realisation. */ - std::shared_ptr queryRealisation(const DrvOutput &); + std::shared_ptr queryRealisation(const DrvOutput &); /** * Asynchronous version of queryRealisation(). */ - void queryRealisation(const DrvOutput &, Callback> callback) noexcept; + void queryRealisation(const DrvOutput &, Callback> callback) noexcept; /** * Check whether the given valid path info is sufficiently attested, by @@ -431,8 +430,8 @@ protected: virtual void queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept = 0; - virtual void queryRealisationUncached( - const DrvOutput &, Callback> callback) noexcept = 0; + virtual void + queryRealisationUncached(const DrvOutput &, Callback> callback) noexcept = 0; public: diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index f23feb8fb..2b000b3db 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -77,7 +77,7 @@ void LocalOverlayStore::registerDrvOutput(const Realisation & info) // First do queryRealisation on lower layer to populate DB auto res = lowerStore->queryRealisation(info.id); if (res) - LocalStore::registerDrvOutput({*res, info.id}); + LocalStore::registerDrvOutput(*res); LocalStore::registerDrvOutput(info); } @@ -108,12 +108,12 @@ void LocalOverlayStore::queryPathInfoUncached( } void LocalOverlayStore::queryRealisationUncached( - const DrvOutput & drvOutput, Callback> callback) noexcept + const DrvOutput & drvOutput, Callback> callback) noexcept { auto callbackPtr = std::make_shared(std::move(callback)); LocalStore::queryRealisationUncached( - drvOutput, {[this, drvOutput, callbackPtr](std::future> fut) { + drvOutput, {[this, drvOutput, callbackPtr](std::future> fut) { try { auto info = fut.get(); if (info) @@ -123,7 +123,7 @@ void LocalOverlayStore::queryRealisationUncached( } // If we don't have it, check lower store lowerStore->queryRealisation( - drvOutput, {[callbackPtr](std::future> fut) { + drvOutput, {[callbackPtr](std::future> fut) { try { (*callbackPtr)(fut.get()); } catch (...) { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 6425819c5..ebc987ee0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1036,7 +1036,7 @@ bool LocalStore::pathInfoIsUntrusted(const ValidPathInfo & info) bool LocalStore::realisationIsUntrusted(const Realisation & realisation) { - return config->requireSigs && !realisation.checkSignatures(realisation.id, getPublicKeys()); + return config->requireSigs && !realisation.checkSignatures(getPublicKeys()); } void LocalStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) @@ -1586,7 +1586,7 @@ void LocalStore::addSignatures(const StorePath & storePath, const StringSet & si }); } -std::optional> +std::optional> LocalStore::queryRealisationCore_(LocalStore::State & state, const DrvOutput & id) { auto useQueryRealisedOutput(state.stmts->QueryRealisedOutput.use()(id.strHash())(id.outputName)); @@ -1598,13 +1598,14 @@ LocalStore::queryRealisationCore_(LocalStore::State & state, const DrvOutput & i return { {realisationDbId, - UnkeyedRealisation{ + Realisation{ + .id = id, .outPath = outputPath, .signatures = signatures, }}}; } -std::optional LocalStore::queryRealisation_(LocalStore::State & state, const DrvOutput & id) +std::optional LocalStore::queryRealisation_(LocalStore::State & state, const DrvOutput & id) { auto maybeCore = queryRealisationCore_(state, id); if (!maybeCore) @@ -1630,13 +1631,13 @@ std::optional LocalStore::queryRealisation_(LocalStore } void LocalStore::queryRealisationUncached( - const DrvOutput & id, Callback> callback) noexcept + const DrvOutput & id, Callback> callback) noexcept { try { - auto maybeRealisation = retrySQLite>( - [&]() { return queryRealisation_(*_state->lock(), id); }); + auto maybeRealisation = + retrySQLite>([&]() { return queryRealisation_(*_state->lock(), id); }); if (maybeRealisation) - callback(std::make_shared(maybeRealisation.value())); + callback(std::make_shared(maybeRealisation.value())); else callback(nullptr); diff --git a/src/libstore/meson.build b/src/libstore/meson.build index e220e65cd..a3502c2e0 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -302,7 +302,6 @@ sources = files( 'build/derivation-check.cc', 'build/derivation-env-desugar.cc', 'build/derivation-goal.cc', - 'build/derivation-resolution-goal.cc', 'build/derivation-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index a31d149c2..7efaa4f86 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -360,12 +360,11 @@ drvOutputReferences(Store & store, const Derivation & drv, const StorePath & out if (!outputHash) throw Error( "output '%s' of derivation '%s' isn't realised", outputName, store.printStorePath(inputDrv)); - DrvOutput key{*outputHash, outputName}; - auto thisRealisation = store.queryRealisation(key); + auto thisRealisation = store.queryRealisation(DrvOutput{*outputHash, outputName}); if (!thisRealisation) throw Error( "output '%s' of derivation '%s' isn’t built", outputName, store.printStorePath(inputDrv)); - inputRealisations.insert({*thisRealisation, std::move(key)}); + inputRealisations.insert(*thisRealisation); } } if (!inputNode.value.empty()) { diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index e08d5ee8a..febd67bd2 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -39,7 +39,7 @@ void Realisation::closure(Store & store, const std::set & startOutp std::set res; for (auto & [currentDep, _] : current.dependentRealisations) { if (auto currentRealisation = store.queryRealisation(currentDep)) - res.insert({*currentRealisation, currentDep}); + res.insert(*currentRealisation); else throw Error("Unrealised derivation '%s'", currentDep.to_string()); } @@ -61,25 +61,24 @@ void Realisation::closure(Store & store, const std::set & startOutp }); } -std::string UnkeyedRealisation::fingerprint(const DrvOutput & key) const +std::string Realisation::fingerprint() const { - nlohmann::json serialized = Realisation{*this, key}; + nlohmann::json serialized = *this; serialized.erase("signatures"); return serialized.dump(); } -void UnkeyedRealisation::sign(const DrvOutput & key, const Signer & signer) +void Realisation::sign(const Signer & signer) { - signatures.insert(signer.signDetached(fingerprint(key))); + signatures.insert(signer.signDetached(fingerprint())); } -bool UnkeyedRealisation::checkSignature( - const DrvOutput & key, const PublicKeys & publicKeys, const std::string & sig) const +bool Realisation::checkSignature(const PublicKeys & publicKeys, const std::string & sig) const { - return verifyDetached(fingerprint(key), sig, publicKeys); + return verifyDetached(fingerprint(), sig, publicKeys); } -size_t UnkeyedRealisation::checkSignatures(const DrvOutput & key, const PublicKeys & publicKeys) const +size_t Realisation::checkSignatures(const PublicKeys & publicKeys) const { // FIXME: Maybe we should return `maxSigs` if the realisation corresponds to // an input-addressed one − because in that case the drv is enough to check @@ -87,18 +86,19 @@ size_t UnkeyedRealisation::checkSignatures(const DrvOutput & key, const PublicKe size_t good = 0; for (auto & sig : signatures) - if (checkSignature(key, publicKeys, sig)) + if (checkSignature(publicKeys, sig)) good++; return good; } -const StorePath & RealisedPath::path() const +StorePath RealisedPath::path() const { - return std::visit([](auto && arg) -> auto & { return arg.getPath(); }, raw); + return std::visit([](auto && arg) { return arg.getPath(); }, raw); } -bool Realisation::isCompatibleWith(const UnkeyedRealisation & other) const +bool Realisation::isCompatibleWith(const Realisation & other) const { + assert(id == other.id); if (outPath == other.outPath) { if (dependentRealisations.empty() != other.dependentRealisations.empty()) { warn( @@ -144,7 +144,7 @@ namespace nlohmann { using namespace nix; -UnkeyedRealisation adl_serializer::from_json(const json & json0) +Realisation adl_serializer::from_json(const json & json0) { auto json = getObject(json0); @@ -157,39 +157,25 @@ UnkeyedRealisation adl_serializer::from_json(const json & js for (auto & [jsonDepId, jsonDepOutPath] : getObject(*jsonDependencies)) dependentRealisations.insert({DrvOutput::parse(jsonDepId), jsonDepOutPath}); - return UnkeyedRealisation{ + return Realisation{ + .id = DrvOutput::parse(valueAt(json, "id")), .outPath = valueAt(json, "outPath"), .signatures = signatures, .dependentRealisations = dependentRealisations, }; } -void adl_serializer::to_json(json & json, const UnkeyedRealisation & r) +void adl_serializer::to_json(json & json, const Realisation & r) { auto jsonDependentRealisations = nlohmann::json::object(); for (auto & [depId, depOutPath] : r.dependentRealisations) jsonDependentRealisations.emplace(depId.to_string(), depOutPath); json = { + {"id", r.id.to_string()}, {"outPath", r.outPath}, {"signatures", r.signatures}, {"dependentRealisations", jsonDependentRealisations}, }; } -Realisation adl_serializer::from_json(const json & json0) -{ - auto json = getObject(json0); - - return Realisation{ - static_cast(json0), - DrvOutput::parse(valueAt(json, "id")), - }; -} - -void adl_serializer::to_json(json & json, const Realisation & r) -{ - json = static_cast(r); - json["id"] = r.id.to_string(); -} - } // namespace nlohmann diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8dd5bc064..a6994f844 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -501,7 +501,7 @@ void RemoteStore::registerDrvOutput(const Realisation & info) } void RemoteStore::queryRealisationUncached( - const DrvOutput & id, Callback> callback) noexcept + const DrvOutput & id, Callback> callback) noexcept { try { auto conn(getConnection()); @@ -515,21 +515,21 @@ void RemoteStore::queryRealisationUncached( conn->to << id.to_string(); conn.processStderr(); - auto real = [&]() -> std::shared_ptr { + auto real = [&]() -> std::shared_ptr { if (GET_PROTOCOL_MINOR(conn->protoVersion) < 31) { auto outPaths = WorkerProto::Serialise>::read(*this, *conn); if (outPaths.empty()) return nullptr; - return std::make_shared(UnkeyedRealisation{.outPath = *outPaths.begin()}); + return std::make_shared(Realisation{.id = id, .outPath = *outPaths.begin()}); } else { auto realisations = WorkerProto::Serialise>::read(*this, *conn); if (realisations.empty()) return nullptr; - return std::make_shared(*realisations.begin()); + return std::make_shared(*realisations.begin()); } }(); - callback(std::shared_ptr(real)); + callback(std::shared_ptr(real)); } catch (...) { return callback.rethrow(); } @@ -626,15 +626,13 @@ std::vector RemoteStore::buildPathsWithResults( auto realisation = queryRealisation(outputId); if (!realisation) throw MissingRealisation(outputId); - success.builtOutputs.emplace(output, Realisation{*realisation, outputId}); + success.builtOutputs.emplace(output, *realisation); } else { success.builtOutputs.emplace( output, Realisation{ - UnkeyedRealisation{ - .outPath = outputPath, - }, - outputId, + .id = outputId, + .outPath = outputPath, }); } } diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index 5270f7d10..a1cb41606 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -107,7 +107,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached( - const DrvOutput & id, Callback> callback) noexcept override; + const DrvOutput & id, Callback> callback) noexcept override; void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override; @@ -244,7 +244,7 @@ void RestrictedStore::registerDrvOutput(const Realisation & info) } void RestrictedStore::queryRealisationUncached( - const DrvOutput & id, Callback> callback) noexcept + const DrvOutput & id, Callback> callback) noexcept // XXX: This should probably be allowed if the realisation corresponds to // an allowed derivation { diff --git a/src/libstore/ssh.cc b/src/libstore/ssh.cc index 0f1dba1e9..1a9908366 100644 --- a/src/libstore/ssh.cc +++ b/src/libstore/ssh.cc @@ -78,7 +78,7 @@ SSHMaster::SSHMaster( oss << authority.host; return std::move(oss).str(); }()) - , fakeSSH(authority.host == "localhost") + , fakeSSH(authority.to_string() == "localhost") , keyFile(keyFile) , sshPublicHostKey(parsePublicHostKey(authority.host, sshPublicHostKey)) , useMaster(useMaster && !fakeSSH) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index df00dc179..4ce6b15fa 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -598,8 +598,7 @@ void Store::queryPathInfo(const StorePath & storePath, Callback> callback) noexcept +void Store::queryRealisation(const DrvOutput & id, Callback> callback) noexcept { try { @@ -625,20 +624,20 @@ void Store::queryRealisation( auto callbackPtr = std::make_shared(std::move(callback)); - queryRealisationUncached(id, {[this, id, callbackPtr](std::future> fut) { + queryRealisationUncached(id, {[this, id, callbackPtr](std::future> fut) { try { auto info = fut.get(); if (diskCache) { if (info) diskCache->upsertRealisation( - config.getReference().render(/*FIXME withParams=*/false), {*info, id}); + config.getReference().render(/*FIXME withParams=*/false), *info); else diskCache->upsertAbsentRealisation( config.getReference().render(/*FIXME withParams=*/false), id); } - (*callbackPtr)(std::shared_ptr(info)); + (*callbackPtr)(std::shared_ptr(info)); } catch (...) { callbackPtr->rethrow(); @@ -646,9 +645,9 @@ void Store::queryRealisation( }}); } -std::shared_ptr Store::queryRealisation(const DrvOutput & id) +std::shared_ptr Store::queryRealisation(const DrvOutput & id) { - using RealPtr = std::shared_ptr; + using RealPtr = std::shared_ptr; std::promise promise; queryRealisation(id, {[&](std::future result) { @@ -911,12 +910,11 @@ std::map copyPaths( std::set toplevelRealisations; for (auto & path : paths) { storePaths.insert(path.path()); - if (auto * realisation = std::get_if(&path.raw)) { + if (auto realisation = std::get_if(&path.raw)) { experimentalFeatureSettings.require(Xp::CaDerivations); toplevelRealisations.insert(*realisation); } } - auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); try { @@ -933,7 +931,7 @@ std::map copyPaths( "dependency of '%s' but isn't registered", drvOutput.to_string(), current.id.to_string()); - children.insert({*currentChild, drvOutput}); + children.insert(*currentChild); } return children; }, @@ -1201,7 +1199,7 @@ void Store::signRealisation(Realisation & realisation) for (auto & secretKeyFile : secretKeyFiles.get()) { SecretKey secretKey(readFile(secretKeyFile)); LocalSigner signer(std::move(secretKey)); - realisation.sign(realisation.id, signer); + realisation.sign(signer); } } diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index e2bcb1b84..5bdd843bd 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1841,12 +1841,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() for (auto & [outputName, newInfo] : infos) { auto oldinfo = get(initialOutputs, outputName); assert(oldinfo); - auto thisRealisation = Realisation{ - { - .outPath = newInfo.path, - }, - DrvOutput{oldinfo->outputHash, outputName}, - }; + auto thisRealisation = Realisation{.id = DrvOutput{oldinfo->outputHash, outputName}, .outPath = newInfo.path}; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().isImpure()) { store.signRealisation(thisRealisation); store.registerDrvOutput(thisRealisation); diff --git a/src/libutil/include/nix/util/hash.hh b/src/libutil/include/nix/util/hash.hh index 0b16b423c..571b6acca 100644 --- a/src/libutil/include/nix/util/hash.hh +++ b/src/libutil/include/nix/util/hash.hh @@ -222,22 +222,3 @@ public: }; } // namespace nix - -template<> -struct std::hash -{ - std::size_t operator()(const nix::Hash & hash) const noexcept - { - assert(hash.hashSize > sizeof(size_t)); - return *reinterpret_cast(&hash.hash); - } -}; - -namespace nix { - -inline std::size_t hash_value(const Hash & hash) -{ - return std::hash{}(hash); -} - -} // namespace nix diff --git a/src/libutil/include/nix/util/strings.hh b/src/libutil/include/nix/util/strings.hh index b4ef66bfe..ba37ce79f 100644 --- a/src/libutil/include/nix/util/strings.hh +++ b/src/libutil/include/nix/util/strings.hh @@ -12,11 +12,6 @@ namespace nix { -/* - * workaround for unavailable view() method (C++20) of std::ostringstream under MacOS with clang-16 - */ -std::string_view toView(const std::ostringstream & os); - /** * String tokenizer. * diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 997110617..e2f28f553 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -121,7 +121,7 @@ public: std::ostringstream oss; showErrorInfo(oss, ei, loggerSettings.showTrace.get()); - log(ei.level, toView(oss)); + log(ei.level, oss.view()); } void startActivity( diff --git a/src/libutil/strings.cc b/src/libutil/strings.cc index a95390089..a87567cef 100644 --- a/src/libutil/strings.cc +++ b/src/libutil/strings.cc @@ -8,23 +8,6 @@ namespace nix { -struct view_stringbuf : public std::stringbuf -{ - inline std::string_view toView() - { - auto begin = pbase(); - return {begin, begin + pubseekoff(0, std::ios_base::cur, std::ios_base::out)}; - } -}; - -__attribute__((no_sanitize("undefined"))) std::string_view toView(const std::ostringstream & os) -{ - /* Downcasting like this is very much undefined behavior, so we disable - UBSAN for this function. */ - auto buf = static_cast(os.rdbuf()); - return buf->toView(); -} - template std::list tokenizeString(std::string_view s, std::string_view separators); template StringSet tokenizeString(std::string_view s, std::string_view separators); template std::vector tokenizeString(std::string_view s, std::string_view separators); diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index c04943eab..e1efb40eb 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -100,7 +100,7 @@ struct CmdConfigCheck : StoreCommand ss << "Multiple versions of nix found in PATH:\n"; for (auto & dir : dirs) ss << " " << dir << "\n"; - return checkFail(toView(ss)); + return checkFail(ss.view()); } return checkPass("PATH contains only one nix version."); @@ -143,7 +143,7 @@ struct CmdConfigCheck : StoreCommand for (auto & dir : dirs) ss << " " << dir << "\n"; ss << "\n"; - return checkFail(toView(ss)); + return checkFail(ss.view()); } return checkPass("All profiles are gcroots."); @@ -162,7 +162,7 @@ struct CmdConfigCheck : StoreCommand << "sync with the daemon.\n\n" << "Client protocol: " << formatProtocol(clientProto) << "\n" << "Store protocol: " << formatProtocol(storeProto) << "\n\n"; - return checkFail(toView(ss)); + return checkFail(ss.view()); } return checkPass("Client protocol matches store protocol."); diff --git a/src/nix/nix-build/nix-build.cc b/src/nix/nix-build/nix-build.cc index d3902f2a6..eef97aa19 100644 --- a/src/nix/nix-build/nix-build.cc +++ b/src/nix/nix-build/nix-build.cc @@ -285,10 +285,10 @@ static void main_nix_build(int argc, char ** argv) execArgs, interpreter, escapeShellArgAlways(script), - toView(joined)); + joined.view()); } else { envCommand = - fmt("exec %1% %2% %3% %4%", execArgs, interpreter, escapeShellArgAlways(script), toView(joined)); + fmt("exec %1% %2% %3% %4%", execArgs, interpreter, escapeShellArgAlways(script), joined.view()); } } diff --git a/src/nix/nix-env/user-env.cc b/src/nix/nix-env/user-env.cc index fbdcb14f8..81e2c4f80 100644 --- a/src/nix/nix-env/user-env.cc +++ b/src/nix/nix-env/user-env.cc @@ -108,7 +108,7 @@ bool createUserEnv( auto manifestFile = ({ std::ostringstream str; printAmbiguous(manifest, state.symbols, str, nullptr, std::numeric_limits::max()); - StringSource source{toView(str)}; + StringSource source{str.view()}; state.store->addToStoreFromDump( source, "env-manifest.nix", diff --git a/tests/functional/build.sh b/tests/functional/build.sh index c9a39438d..0a19ff7da 100755 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -178,8 +178,7 @@ test "$(<<<"$out" grep -cE '^error:')" = 4 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$? test "$status" = 1 -# Precise number of errors depends on daemon version / goal refactorings -(( "$(<<<"$out" grep -cE '^error:')" >= 2 )) +test "$(<<<"$out" grep -cE '^error:')" = 2 if isDaemonNewer "2.29pre"; then <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" @@ -187,13 +186,11 @@ if isDaemonNewer "2.29pre"; then else <<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build" fi -# Either x2 or x3 could have failed, x4 depends on both symmetrically -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x[23]\\.drv'" +<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'" out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$? test "$status" = 1 -# Precise number of errors depends on daemon version / goal refactorings -(( "$(<<<"$out" grep -cE '^error:')" >= 3 )) +test "$(<<<"$out" grep -cE '^error:')" = 3 if isDaemonNewer "2.29pre"; then <<<"$out" grepQuiet -E "error: Cannot build '.*-x4\\.drv'" <<<"$out" grepQuiet -E "Reason: 2 dependencies failed." diff --git a/tests/functional/ca/issue-13247.sh b/tests/functional/ca/issue-13247.sh index 705919513..686d90ced 100755 --- a/tests/functional/ca/issue-13247.sh +++ b/tests/functional/ca/issue-13247.sh @@ -65,4 +65,7 @@ buildViaSubstitute use-a-prime-more-outputs^first # Should only fetch the output we asked for [[ -d "$(jq -r <"$TEST_ROOT"/a.json '.[0].outputs.out')" ]] [[ -f "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.first')" ]] -[[ ! -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]] + +# Output should *not* be here, this is the bug +[[ -e "$(jq -r <"$TEST_ROOT"/a.json '.[2].outputs.second')" ]] +skipTest "bug is not yet fixed" diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix index 5a1e08528..edfa4124f 100644 --- a/tests/nixos/default.nix +++ b/tests/nixos/default.nix @@ -207,5 +207,7 @@ in fetchurl = runNixOSTest ./fetchurl.nix; + fetchersSubstitute = runNixOSTest ./fetchers-substitute.nix; + chrootStore = runNixOSTest ./chroot-store.nix; } diff --git a/tests/nixos/fetchers-substitute.nix b/tests/nixos/fetchers-substitute.nix new file mode 100644 index 000000000..453982677 --- /dev/null +++ b/tests/nixos/fetchers-substitute.nix @@ -0,0 +1,176 @@ +{ + name = "fetchers-substitute"; + + nodes.substituter = + { pkgs, ... }: + { + virtualisation.writableStore = true; + + nix.settings.extra-experimental-features = [ + "nix-command" + "fetch-tree" + ]; + + networking.firewall.allowedTCPPorts = [ 5000 ]; + + services.nix-serve = { + enable = true; + secretKeyFile = + let + key = pkgs.writeTextFile { + name = "secret-key"; + text = '' + substituter:SerxxAca5NEsYY0DwVo+subokk+OoHcD9m6JwuctzHgSQVfGHe6nCc+NReDjV3QdFYPMGix4FMg0+K/TM1B3aA== + ''; + }; + in + "${key}"; + }; + }; + + nodes.importer = + { lib, ... }: + { + virtualisation.writableStore = true; + + nix.settings = { + extra-experimental-features = [ + "nix-command" + "fetch-tree" + ]; + substituters = lib.mkForce [ "http://substituter:5000" ]; + trusted-public-keys = lib.mkForce [ "substituter:EkFXxh3upwnPjUXg41d0HRWDzBoseBTINPiv0zNQd2g=" ]; + }; + }; + + testScript = + { nodes }: # python + '' + import json + + start_all() + + substituter.wait_for_unit("multi-user.target") + + ########################################## + # Test 1: builtins.fetchurl with substitution + ########################################## + + missing_file = "/only-on-substituter.txt" + + substituter.succeed(f"echo 'this should only exist on the substituter' > {missing_file}") + + file_hash = substituter.succeed(f"nix hash file {missing_file}").strip() + + file_store_path_json = substituter.succeed(f""" + nix-instantiate --eval --json --read-write-mode --expr ' + builtins.fetchurl {{ + url = "file://{missing_file}"; + sha256 = "{file_hash}"; + }} + ' + """) + + file_store_path = json.loads(file_store_path_json) + + substituter.succeed(f"nix store sign --key-file ${nodes.substituter.services.nix-serve.secretKeyFile} {file_store_path}") + + importer.wait_for_unit("multi-user.target") + + print("Testing fetchurl with substitution...") + importer.succeed(f""" + nix-instantiate -vvvvv --eval --json --read-write-mode --expr ' + builtins.fetchurl {{ + url = "file://{missing_file}"; + sha256 = "{file_hash}"; + }} + ' + """) + print("✓ fetchurl substitution works!") + + ########################################## + # Test 2: builtins.fetchTarball with substitution + ########################################## + + missing_tarball = "/only-on-substituter.tar.gz" + + # Create a directory with some content + substituter.succeed(""" + mkdir -p /tmp/test-tarball + echo 'Hello from tarball!' > /tmp/test-tarball/hello.txt + echo 'Another file' > /tmp/test-tarball/file2.txt + """) + + # Create a tarball + substituter.succeed(f"tar czf {missing_tarball} -C /tmp test-tarball") + + # For fetchTarball, we need to first fetch it without hash to get the store path, + # then compute the NAR hash of that path + tarball_store_path_json = substituter.succeed(f""" + nix-instantiate --eval --json --read-write-mode --expr ' + builtins.fetchTarball {{ + url = "file://{missing_tarball}"; + }} + ' + """) + + tarball_store_path = json.loads(tarball_store_path_json) + + # Get the NAR hash of the unpacked tarball in SRI format + path_info_json = substituter.succeed(f"nix path-info --json {tarball_store_path}").strip() + path_info_dict = json.loads(path_info_json) + # nix path-info returns a dict with store paths as keys + tarball_hash_sri = path_info_dict[tarball_store_path]["narHash"] + print(f"Tarball NAR hash (SRI): {tarball_hash_sri}") + + # Also get the old format hash for fetchTarball (which uses sha256 parameter) + tarball_hash = substituter.succeed(f"nix-store --query --hash {tarball_store_path}").strip() + + # Sign the tarball's store path + substituter.succeed(f"nix store sign --recursive --key-file ${nodes.substituter.services.nix-serve.secretKeyFile} {tarball_store_path}") + + # Now try to fetch the same tarball on the importer + # The file doesn't exist locally, so it should be substituted + print("Testing fetchTarball with substitution...") + result = importer.succeed(f""" + nix-instantiate -vvvvv --eval --json --read-write-mode --expr ' + builtins.fetchTarball {{ + url = "file://{missing_tarball}"; + sha256 = "{tarball_hash}"; + }} + ' + """) + + result_path = json.loads(result) + print(f"✓ fetchTarball substitution works! Result: {result_path}") + + # Verify the content is correct + # fetchTarball strips the top-level directory if there's only one + content = importer.succeed(f"cat {result_path}/hello.txt").strip() + assert content == "Hello from tarball!", f"Content mismatch: {content}" + print("✓ fetchTarball content verified!") + + ########################################## + # Test 3: Verify fetchTree does NOT substitute (preserves metadata) + ########################################## + + print("Testing that fetchTree without __final does NOT use substitution...") + + # fetchTree with just narHash (not __final) should try to download, which will fail + # since the file doesn't exist on the importer + exit_code = importer.fail(f""" + nix-instantiate --eval --json --read-write-mode --expr ' + builtins.fetchTree {{ + type = "tarball"; + url = "file:///only-on-substituter.tar.gz"; + narHash = "{tarball_hash_sri}"; + }} + ' 2>&1 + """) + + # Should fail with "does not exist" since it tries to download instead of substituting + assert "does not exist" in exit_code or "Couldn't open file" in exit_code, f"Expected download failure, got: {exit_code}" + print("✓ fetchTree correctly does NOT substitute non-final inputs!") + print(" (This preserves metadata like lastModified from the actual fetch)") + ''; +}