From b41ce0d263ef4e50181ebb45b3f5dd708e77193c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 12 Feb 2025 23:54:24 -0500 Subject: [PATCH] Revert "Use the hash modulo in the derivation outputs" Fix #11897 As described in the issue, this makes for a simpler and much more intuitive notion of a realisation key. This is better for pedagogy, and interoperability between more tools. The way the issue was written was that we would switch to only having shallow realisations first, and then do this. But going to only shallow realisations is more complex change, and it turns out we weren't even testing for the benefits that derivation hashes (modulo FODs) provided in the deep realisation case, so I now just want to do this first. Doing this gets the binary cache data structures in order, which will unblock the Hydra fixed-output-derivation tracking work. I don't want to delay that work while I figure out the changes needed for shallow-realisations only. This reverts commit bab1cda0e6c30e25460b5a9c809589d3948f35df. --- src/libcmd/built-path.cc | 18 +- src/libexpr/primops.cc | 23 +- src/libstore-tests/common-protocol.cc | 55 ----- .../data/realisation/simple.json | 12 +- .../data/realisation/with-signature.json | 16 +- .../data/serve-protocol/build-result-2.8.bin | Bin 0 -> 360 bytes .../data/serve-protocol/drv-output-2.8.bin | Bin 0 -> 160 bytes .../data/serve-protocol/realisation-2.8.bin | Bin 0 -> 176 bytes .../unkeyed-realisation-2.8.bin | Bin 0 -> 96 bytes .../worker-protocol/build-result-1.39.bin | Bin 0 -> 424 bytes .../data/worker-protocol/drv-output-1.39.bin | Bin 0 -> 160 bytes .../data/worker-protocol/realisation-1.39.bin | Bin 0 -> 176 bytes .../unkeyed-realisation-1.39.bin | Bin 0 -> 96 bytes src/libstore-tests/dummy-store.cc | 11 +- src/libstore-tests/realisation.cc | 65 ++--- src/libstore-tests/serve-protocol.cc | 140 ++++++----- src/libstore-tests/worker-protocol.cc | 173 +++++++------ src/libstore/binary-cache-store.cc | 12 +- .../build/derivation-building-goal.cc | 7 +- src/libstore/build/derivation-goal.cc | 47 +--- .../build/drv-output-substitution-goal.cc | 7 +- src/libstore/common-protocol.cc | 28 --- src/libstore/daemon.cc | 26 +- src/libstore/derivations.cc | 231 +++++++++++------- src/libstore/dummy-store.cc | 4 +- .../include/nix/store/binary-cache-store.hh | 9 +- .../store/build/derivation-building-misc.hh | 1 - .../nix/store/build/derivation-goal.hh | 2 - .../include/nix/store/common-protocol-impl.hh | 5 +- .../include/nix/store/common-protocol.hh | 7 +- src/libstore/include/nix/store/derivations.hh | 61 +++-- .../include/nix/store/dummy-store-impl.hh | 2 +- .../store/length-prefixed-protocol-helper.hh | 20 +- src/libstore/include/nix/store/realisation.hh | 74 +++--- .../include/nix/store/serve-protocol-impl.hh | 6 +- .../include/nix/store/serve-protocol.hh | 15 +- .../include/nix/store/worker-protocol-impl.hh | 6 +- .../include/nix/store/worker-protocol.hh | 17 +- src/libstore/local-binary-cache-store.cc | 5 +- src/libstore/local-store.cc | 8 +- src/libstore/misc.cc | 9 +- src/libstore/nar-info-disk-cache.cc | 49 ++-- src/libstore/realisation.cc | 85 ++++--- src/libstore/remote-store.cc | 44 +--- src/libstore/restricted-store.cc | 15 +- src/libstore/serve-protocol.cc | 109 ++++++++- src/libstore/store-api.cc | 7 +- src/libstore/unix/build/derivation-builder.cc | 5 +- src/libstore/worker-protocol.cc | 144 ++++++++++- src/nix/build-remote/build-remote.cc | 6 +- src/nix/develop.cc | 10 +- src/perl/lib/Nix/Store.xs | 7 +- tests/functional/ca/substitute.sh | 6 +- 53 files changed, 909 insertions(+), 700 deletions(-) create mode 100644 src/libstore-tests/data/serve-protocol/build-result-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/drv-output-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/realisation-2.8.bin create mode 100644 src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin create mode 100644 src/libstore-tests/data/worker-protocol/build-result-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/drv-output-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/realisation-1.39.bin create mode 100644 src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin diff --git a/src/libcmd/built-path.cc b/src/libcmd/built-path.cc index fc7f18493..f60e4499c 100644 --- a/src/libcmd/built-path.cc +++ b/src/libcmd/built-path.cc @@ -108,20 +108,16 @@ RealisedPath::Set BuiltPath::toRealisedPaths(Store & store) const overloaded{ [&](const BuiltPath::Opaque & p) { res.insert(p.path); }, [&](const BuiltPath::Built & p) { - auto drvHashes = staticOutputHashes(store, store.readDerivation(p.drvPath->outPath())); for (auto & [outputName, outputPath] : p.outputs) { if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { - auto drvOutput = get(drvHashes, outputName); - if (!drvOutput) - throw Error( - "the derivation '%s' has unrealised output '%s' (derived-path.cc/toRealisedPaths)", - store.printStorePath(p.drvPath->outPath()), - outputName); - DrvOutput key{*drvOutput, outputName}; + DrvOutput key{ + .drvPath = p.drvPath->outPath(), + .outputName = outputName, + }; auto thisRealisation = store.queryRealisation(key); - assert(thisRealisation); // We’ve built it, so we must - // have the realisation - res.insert(Realisation{*thisRealisation, std::move(key)}); + // We’ve built it, so we must have the realisation. + assert(thisRealisation); + res.insert(Realisation{*thisRealisation, key}); } else { res.insert(outputPath); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d1aae64fa..fb458d0fc 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1718,28 +1718,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName drv.outputs.insert_or_assign(i, DerivationOutput::Deferred{}); } - auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); - switch (hashModulo.kind) { - case DrvHash::Kind::Regular: - for (auto & i : outputs) { - auto h = get(hashModulo.hashes, i); - if (!h) - state.error("derivation produced no hash for output '%s'", i).atPos(v).debugThrow(); - auto outPath = state.store->makeOutputPath(i, *h, drvName); - drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign( - i, - DerivationOutput::InputAddressed{ - .path = std::move(outPath), - }); - } - break; - ; - case DrvHash::Kind::Deferred: - for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, DerivationOutput::Deferred{}); - } - } + resolveInputAddressed(*state.store, drv); } /* Write the resulting term into the Nix store directory. */ diff --git a/src/libstore-tests/common-protocol.cc b/src/libstore-tests/common-protocol.cc index 6bd7beb44..71f42517f 100644 --- a/src/libstore-tests/common-protocol.cc +++ b/src/libstore-tests/common-protocol.cc @@ -108,61 +108,6 @@ CHARACTERIZATION_TEST( }, })) -CHARACTERIZATION_TEST( - drvOutput, - "drv-output", - (std::tuple{ - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - })) - -CHARACTERIZATION_TEST( - realisation, - "realisation", - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, - })) - -READ_CHARACTERIZATION_TEST( - realisation_with_deps, - "realisation-with-deps", - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, - })) - CHARACTERIZATION_TEST( vector, "vector", diff --git a/src/libstore-tests/data/realisation/simple.json b/src/libstore-tests/data/realisation/simple.json index 2ccb1e721..1e4760b56 100644 --- a/src/libstore-tests/data/realisation/simple.json +++ b/src/libstore-tests/data/realisation/simple.json @@ -1,6 +1,10 @@ { - "dependentRealisations": {}, - "id": "sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad!foo", - "outPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv", - "signatures": [] + "key": { + "drvPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + "outputName": "foo" + }, + "value": { + "outPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo", + "signatures": [] + } } diff --git a/src/libstore-tests/data/realisation/with-signature.json b/src/libstore-tests/data/realisation/with-signature.json index a28848cb0..4a73ed3ef 100644 --- a/src/libstore-tests/data/realisation/with-signature.json +++ b/src/libstore-tests/data/realisation/with-signature.json @@ -1,8 +1,12 @@ { - "dependentRealisations": {}, - "id": "sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad!foo", - "outPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv", - "signatures": [ - "asdfasdfasdf" - ] + "key": { + "drvPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv", + "outputName": "foo" + }, + "value": { + "outPath": "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo", + "signatures": [ + "asdfasdfasdf" + ] + } } diff --git a/src/libstore-tests/data/serve-protocol/build-result-2.8.bin b/src/libstore-tests/data/serve-protocol/build-result-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..fa072599578a351e0f289c1c6bbdc5c1ff90107c GIT binary patch literal 360 zcmZQ&fBjf;Y*hsmSsV}eS+ z%uh-z0*mN_Nd^Y}yvz#y;*$KLRQ+_ra`TKz<3e1tE=(^-E6lvK{Cp6XfgJN-dO-#N E0Qb5q`2YX_ literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/serve-protocol/drv-output-2.8.bin b/src/libstore-tests/data/serve-protocol/drv-output-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..5be0b15a34567aed217e56dbf2543f2f60a200be GIT binary patch literal 160 zcmXqJfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7JG>N>LeDBQuy}U`R@=0<$PJj|FTB M14ChHX$6Q00QuS{z5oCK literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/serve-protocol/realisation-2.8.bin b/src/libstore-tests/data/serve-protocol/realisation-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..5295ee34400a58e06c919eaf4b0c70b95b713fc9 GIT binary patch literal 176 zcmXqJfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7JG>N>LeDBQuy}U`R@=0<-kNBm)D9 Z<}olq^|3(d#Nw1R5EI5PEKe;0@c}{FC(r-@ literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin b/src/libstore-tests/data/serve-protocol/unkeyed-realisation-2.8.bin new file mode 100644 index 0000000000000000000000000000000000000000..10f4ebcb2eb1622e1c56b462f36a7db1e863b345 GIT binary patch literal 96 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7EtQ6GR&W3zSYQPDukXVf@1K)FKcc E0Dd79Jpcdz literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/build-result-1.39.bin b/src/libstore-tests/data/worker-protocol/build-result-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..11bec3e6e3b7a82f7b2d792d687a807644ff9d05 GIT binary patch literal 424 zcmZQ&fBO)Le|4^xkB4qQI< ztSm?kobS(|0^&1)2nGfQn0t~Ei@+lKV3L7BKQFUFzqlm7C{;h*u-rVO(zp;8tqao& U(h4&#Ek7T`Wgy2qm|lN>LeDBQuy}U`R@=0<$PJj|FTB M14ChHX$6Q00QuS{z5oCK literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/realisation-1.39.bin b/src/libstore-tests/data/worker-protocol/realisation-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..5295ee34400a58e06c919eaf4b0c70b95b713fc9 GIT binary patch literal 176 zcmXqJfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7JG>N>LeDBQuy}U`R@=0<-kNBm)D9 Z<}olq^|3(d#Nw1R5EI5PEKe;0@c}{FC(r-@ literal 0 HcmV?d00001 diff --git a/src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin b/src/libstore-tests/data/worker-protocol/unkeyed-realisation-1.39.bin new file mode 100644 index 0000000000000000000000000000000000000000..10f4ebcb2eb1622e1c56b462f36a7db1e863b345 GIT binary patch literal 96 zcmdOAfB^lx%nJSDlKi4n{dB`}^NdR4LR_?NT7EtQ6GR&W3zSYQPDukXVf@1K)FKcc E0Dd79Jpcdz literal 0 HcmV?d00001 diff --git a/src/libstore-tests/dummy-store.cc b/src/libstore-tests/dummy-store.cc index 36a0c3019..f53fc03b4 100644 --- a/src/libstore-tests/dummy-store.cc +++ b/src/libstore-tests/dummy-store.cc @@ -37,20 +37,19 @@ TEST(DummyStore, realisation_read) return cfg->openDummyStore(); }(); - auto drvHash = Hash::parseExplicitFormatUnprefixed( - "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", HashAlgorithm::SHA256, HashFormat::Base16); + StorePath drvPath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv"}; auto outputName = "foo"; - EXPECT_EQ(store->queryRealisation({drvHash, outputName}), nullptr); + EXPECT_EQ(store->queryRealisation({drvPath, outputName}), nullptr); UnkeyedRealisation value{ - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, }; - store->buildTrace.insert({drvHash, {{outputName, make_ref(value)}}}); + store->buildTrace.insert({drvPath, {{outputName, make_ref(value)}}}); - auto value2 = store->queryRealisation({drvHash, outputName}); + auto value2 = store->queryRealisation({drvPath, outputName}); ASSERT_TRUE(value2); EXPECT_EQ(*value2, value); diff --git a/src/libstore-tests/realisation.cc b/src/libstore-tests/realisation.cc index 66f4707e9..3087bd1b5 100644 --- a/src/libstore-tests/realisation.cc +++ b/src/libstore-tests/realisation.cc @@ -44,45 +44,30 @@ TEST_P(RealisationJsonTest, to_json) writeJsonTest(name, value); } -Realisation simple{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, - }, - { - .drvHash = Hash::parseExplicitFormatUnprefixed( - "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", - HashAlgorithm::SHA256, - HashFormat::Base16), - .outputName = "foo", - }, -}; - -INSTANTIATE_TEST_SUITE_P( - RealisationJSON, - RealisationJsonTest, - ::testing::Values( - std::pair{ - "simple", - simple, - }, - std::pair{ - "with-signature", - [&] { - auto r = simple; - // FIXME actually sign properly - r.signatures = {"asdfasdfasdf"}; - return r; - }(), - })); - -/** - * We no longer have a notion of "dependent realisations", but we still - * want to parse old realisation files. So make this just be a read test - * (no write direction), accordingly. - */ -TEST_F(RealisationTest, dependent_realisations_from_json) -{ - readJsonTest("with-dependent-realisations", simple); -} +INSTANTIATE_TEST_SUITE_P(RealisationJSON, RealisationJsonTest, ([] { + Realisation simple{ + { + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + }, + { + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar.drv"}, + .outputName = "foo", + }, + }; + return ::testing::Values( + std::pair{ + "simple", + simple, + }, + std::pair{ + "with-signature", + [&] { + auto r = simple; + // FIXME actually sign properly + r.signatures = {"asdfasdfasdf"}; + return r; + }(), + }); + }())); } // namespace nix diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index afc35d03c..1bffc08ef 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -73,16 +73,16 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - drvOutput, - "drv-output", - defaultVersion, + drvOutput_2_8, + "drv-output-2.8", + 2 << 8 | 8, (std::tuple{ { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, .outputName = "baz", }, DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, .outputName = "quux", }, })) @@ -91,46 +91,27 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - realisation, - "realisation", - defaultVersion, - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, + unkeyedRealisation_2_8, + "unkeyed-realisation-2.8", + 2 << 8 | 8, + (UnkeyedRealisation{ + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, })) -VERSIONED_READ_CHARACTERIZATION_TEST( +VERSIONED_CHARACTERIZATION_TEST( ServeProtoTest, - realisation_with_deps, - "realisation-with-deps", - defaultVersion, - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + realisation_2_8, + "realisation-2.8", + 2 << 8 | 8, + (Realisation{ + UnkeyedRealisation{ + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, + }, + { + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, + .outputName = "baz", }, })) @@ -180,7 +161,10 @@ VERSIONED_CHARACTERIZATION_TEST(ServeProtoTest, buildResult_2_3, "build-result-2 t; })) -VERSIONED_CHARACTERIZATION_TEST( +/* We now do a lossy read which does not allow us to faithfully right + back, since we changed the data type. We still however want to test + that this read works, and so for that we have a one-way test. */ +VERSIONED_READ_CHARACTERIZATION_TEST( ServeProtoTest, buildResult_2_6, "build-result-2.6", 2 << 8 | 6, ({ using namespace std::literals::chrono_literals; std::tuple t{ @@ -206,27 +190,65 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - 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", - }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, + }, + }, + }, + }}, + .timesBuilt = 1, + .startTime = 30, + .stopTime = 50, +#if 0 + // These fields are not yet serialized. + // FIXME Include in next version of protocol or document + // why they are skipped. + .cpuUser = std::chrono::milliseconds(500s), + .cpuSystem = std::chrono::milliseconds(604s), +#endif + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + ServeProtoTest, buildResult_2_8, "build-result-2.8", 2 << 8 | 8, ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + BuildResult{.inner{BuildResult::Failure{ + .status = BuildResult::Failure::OutputRejected, + .errorMsg = "no idea why", + }}}, + BuildResult{ + .inner{BuildResult::Failure{ + .status = BuildResult::Failure::NotDeterministic, + .errorMsg = "no idea why", + .isNonDeterministic = true, + }}, + .timesBuilt = 3, + .startTime = 30, + .stopTime = 50, + }, + BuildResult{ + .inner{BuildResult::Success{ + .status = BuildResult::Success::Built, + .builtOutputs = + { + { + "foo", + { + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + }, + }, + { + "bar", + { + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index a1f68e231..853568940 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -129,61 +129,42 @@ VERSIONED_CHARACTERIZATION_TEST( VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, drvOutput, - "drv-output", - defaultVersion, + "drv-output-1.39", + 1 << 8 | 39, (std::tuple{ { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, .outputName = "baz", }, DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, .outputName = "quux", }, })) VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - realisation, - "realisation", - defaultVersion, - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, - }, + unkeyedRealisation_1_39, + "unkeyed-realisation-1.39", + 1 << 8 | 39, + (UnkeyedRealisation{ + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, })) -VERSIONED_READ_CHARACTERIZATION_TEST( +VERSIONED_CHARACTERIZATION_TEST( WorkerProtoTest, - realisation_with_deps, - "realisation-with-deps", - defaultVersion, - (std::tuple{ - Realisation{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - .signatures = {"asdf", "qwer"}, - }, - { - .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), - .outputName = "baz", - }, + realisation_1_39, + "realisation-1.39", + 1 << 8 | 39, + (Realisation{ + UnkeyedRealisation{ + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + .signatures = {"asdf", "qwer"}, + }, + { + .drvPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, + .outputName = "baz", }, })) @@ -205,7 +186,10 @@ VERSIONED_CHARACTERIZATION_TEST(WorkerProtoTest, buildResult_1_27, "build-result t; })) -VERSIONED_CHARACTERIZATION_TEST( +/* We now do a lossy read which does not allow us to faithfully right + back, since we changed the data type. We still however want to test + that this read works, and so for that we have a one-way test. */ +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_28, "build-result-1.28", 1 << 8 | 28, ({ using namespace std::literals::chrono_literals; std::tuple t{ @@ -224,25 +208,13 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - 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", - }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, @@ -251,7 +223,8 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +// See above note +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_29, "build-result-1.29", 1 << 8 | 29, ({ using namespace std::literals::chrono_literals; std::tuple t{ @@ -277,27 +250,13 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - 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", - }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, @@ -310,7 +269,8 @@ VERSIONED_CHARACTERIZATION_TEST( t; })) -VERSIONED_CHARACTERIZATION_TEST( +// See above note +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, buildResult_1_37, "build-result-1.37", 1 << 8 | 37, ({ using namespace std::literals::chrono_literals; std::tuple t{ @@ -336,27 +296,60 @@ VERSIONED_CHARACTERIZATION_TEST( { "foo", { - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - 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", - }, + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, + }, + }, + }, + }}, + .timesBuilt = 1, + .startTime = 30, + .stopTime = 50, + .cpuUser = std::chrono::microseconds(500s), + .cpuSystem = std::chrono::microseconds(604s), + }, + }; + t; + })) + +VERSIONED_CHARACTERIZATION_TEST( + WorkerProtoTest, buildResult_1_39, "build-result-1.39", 1 << 8 | 39, ({ + using namespace std::literals::chrono_literals; + std::tuple t{ + BuildResult{.inner{BuildResult::Failure{ + .status = BuildResult::Failure::OutputRejected, + .errorMsg = "no idea why", + }}}, + BuildResult{ + .inner{BuildResult::Failure{ + .status = BuildResult::Failure::NotDeterministic, + .errorMsg = "no idea why", + .isNonDeterministic = true, + }}, + .timesBuilt = 3, + .startTime = 30, + .stopTime = 50, + }, + BuildResult{ + .inner{BuildResult::Success{ + .status = BuildResult::Success::Built, + .builtOutputs = + { + { + "foo", + { + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, + }, + }, + { + "bar", + { + .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-bar"}, }, }, }, diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 274e47271..6a213da57 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -514,7 +514,7 @@ StorePath BinaryCacheStore::addToStore( std::string BinaryCacheStore::makeRealisationPath(const DrvOutput & id) { - return realisationsPrefix + "/" + id.to_string() + ".doi"; + return realisationsPrefix + "/" + id.drvPath.to_string() + "/" + id.outputName + ".doi"; } void BinaryCacheStore::queryRealisationUncached( @@ -535,7 +535,10 @@ void BinaryCacheStore::queryRealisationUncached( 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()); + {}, + "while parsing file '%s' as a build trace value for key '%s'", + outputInfoFilePath, + id.to_string()); throw; } return (*callbackPtr)(std::move(realisation)); @@ -551,7 +554,10 @@ 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"); + upsertFile( + makeRealisationPath(info.id), + static_cast(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 c72130142..8e06b2e08 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -212,9 +212,8 @@ Goal::Co DerivationBuildingGoal::tryToBuild() given this information by the downstream goal, that cannot happen anymore if the downstream goal only cares about one output, but we care about all outputs. */ - auto outputHashes = staticOutputHashes(worker.evalStore, *drv); - for (auto & [outputName, outputHash] : outputHashes) { - InitialOutput v{.outputHash = outputHash}; + for (auto & [outputName, _] : drv->outputs) { + InitialOutput v; /* TODO we might want to also allow randomizing the paths for regular CA derivations, e.g. for sake of checking @@ -1106,7 +1105,7 @@ DerivationBuildingGoal::checkPathValidity(std::map & : PathStatus::Corrupt, }; } - auto drvOutput = DrvOutput{info.outputHash, i.first}; + auto drvOutput = DrvOutput{drvPath, i.first}; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d8e9ff5e4..d27485abe 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -36,12 +36,6 @@ DerivationGoal::DerivationGoal( , drvPath(drvPath) , wantedOutput(wantedOutput) , drv{std::make_unique(drv)} - , outputHash{[&] { - auto outputHashes = staticOutputHashes(worker.evalStore, drv); - if (auto * mOutputHash = get(outputHashes, wantedOutput)) - return *mOutputHash; - throw Error("derivation '%s' does not have output '%s'", worker.store.printStorePath(drvPath), wantedOutput); - }()} , buildMode(buildMode) { @@ -100,7 +94,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) them. */ if (settings.useSubstitutes && drvOptions.substitutesAllowed()) { if (!checkResult) - waitees.insert(upcast_goal(worker.makeDrvOutputSubstitutionGoal(DrvOutput{outputHash, wantedOutput}))); + waitees.insert(upcast_goal(worker.makeDrvOutputSubstitutionGoal(DrvOutput{drvPath, wantedOutput}))); else { auto * cap = getDerivationCA(*drv); waitees.insert(upcast_goal(worker.makePathSubstitutionGoal( @@ -167,12 +161,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) // 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); - - auto outputHash = get(outputHashes, wantedOutput); - auto resolvedHash = get(resolvedHashes, wantedOutput); - if ((!outputHash) || (!resolvedHash)) + if (!drv->outputs.contains(wantedOutput)) throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolve)", worker.store.printStorePath(drvPath), @@ -181,7 +170,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) auto realisation = [&] { auto take1 = get(success.builtOutputs, wantedOutput); if (take1) - return static_cast(*take1); + return *take1; /* The above `get` should work. But stateful tracking of outputs in resolvedResult, this can get out of sync with the @@ -189,7 +178,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) check the store directly if it fails. */ auto take2 = worker.evalStore.queryRealisation( DrvOutput{ - .drvHash = *resolvedHash, + .drvPath = pathResolved, .outputName = wantedOutput, }); if (take2) @@ -205,7 +194,7 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) Realisation newRealisation{ realisation, { - .drvHash = *outputHash, + .drvPath = drvPath, .outputName = wantedOutput, }}; newRealisation.signatures.clear(); @@ -251,16 +240,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 @@ -385,7 +365,7 @@ std::optional> DerivationGoal::checkPa if (drv->type().isImpure()) return std::nullopt; - auto drvOutput = DrvOutput{outputHash, wantedOutput}; + auto drvOutput = DrvOutput{drvPath, wantedOutput}; std::optional mRealisation; @@ -425,7 +405,7 @@ std::optional> DerivationGoal::checkPa Realisation{ *mRealisation, { - .drvHash = outputHash, + .drvPath = drvPath, .outputName = wantedOutput, }, }); @@ -448,16 +428,7 @@ Goal::Done DerivationGoal::doneSuccess(BuildResult::Success::Status status, Unke { 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/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 58f3de2b7..34a263edd 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -12,7 +12,7 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(const DrvOutput & id, Worke : Goal(worker, init()) , id(id) { - name = fmt("substitution of '%s'", id.to_string()); + name = fmt("substitution of '%s'", id.render(worker.store)); trace("created"); } @@ -107,7 +107,8 @@ Goal::Co DrvOutputSubstitutionGoal::init() /* None left. Terminate this goal and let someone else deal with it. */ - debug("derivation output '%s' is required, but there is no substituter that can provide it", id.to_string()); + debug( + "derivation output '%s' is required, but there is no substituter that can provide it", id.render(worker.store)); if (substituterFailed) { worker.failedSubstitutions++; @@ -122,7 +123,7 @@ Goal::Co DrvOutputSubstitutionGoal::init() std::string DrvOutputSubstitutionGoal::key() { - return "a$" + std::string(id.to_string()); + return "a$" + std::string(id.render(worker.store)); } void DrvOutputSubstitutionGoal::handleEOF(Descriptor fd) diff --git a/src/libstore/common-protocol.cc b/src/libstore/common-protocol.cc index b069c9498..6b819fd6e 100644 --- a/src/libstore/common-protocol.cc +++ b/src/libstore/common-protocol.cc @@ -46,34 +46,6 @@ void CommonProto::Serialise::write( conn.to << renderContentAddress(ca); } -Realisation CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) -{ - std::string rawInput = readString(conn.from); - try { - return nlohmann::json::parse(rawInput); - } catch (Error & e) { - e.addTrace({}, "while parsing a realisation object in the remote protocol"); - throw; - } -} - -void CommonProto::Serialise::write( - const StoreDirConfig & store, CommonProto::WriteConn conn, const Realisation & realisation) -{ - conn.to << static_cast(realisation).dump(); -} - -DrvOutput CommonProto::Serialise::read(const StoreDirConfig & store, CommonProto::ReadConn conn) -{ - return DrvOutput::parse(readString(conn.from)); -} - -void CommonProto::Serialise::write( - const StoreDirConfig & store, CommonProto::WriteConn conn, const DrvOutput & drvOutput) -{ - conn.to << drvOutput.to_string(); -} - std::optional CommonProto::Serialise>::read(const StoreDirConfig & store, CommonProto::ReadConn conn) { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 937946134..0b494d0d7 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -963,33 +963,31 @@ static void performOp( case WorkerProto::Op::RegisterDrvOutput: { logger->startWork(); - 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}); - } else { - auto realisation = WorkerProto::Serialise::read(*store, rconn); - store->registerDrvOutput(realisation); - } + // TODO move to WorkerProto::Serialise and friends + // if (GET_PROTOCOL_MINOR(conn.protoVersion) < 39) { + // throw Error("old-style build traces no longer supported"); + //} + auto realisation = WorkerProto::Serialise::read(*store, rconn); + store->registerDrvOutput(realisation); logger->stopWork(); break; } case WorkerProto::Op::QueryRealisation: { logger->startWork(); - auto outputId = DrvOutput::parse(readString(conn.from)); - auto info = store->queryRealisation(outputId); + auto outputId = WorkerProto::Serialise::read(*store, rconn); + std::optional info = *store->queryRealisation(outputId); logger->stopWork(); if (GET_PROTOCOL_MINOR(conn.protoVersion) < 31) { std::set outPaths; if (info) outPaths.insert(info->outPath); WorkerProto::write(*store, wconn, outPaths); + } else if (GET_PROTOCOL_MINOR(conn.protoVersion) < 39) { + // No longer support this format + WorkerProto::write(*store, wconn, StringSet{}); } else { - std::set realisations; - if (info) - realisations.insert({*info, outputId}); - WorkerProto::write(*store, wconn, realisations); + WorkerProto::write(*store, wconn, info); } break; } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index e6ac08fd9..b9ec4d64b 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -866,13 +866,14 @@ DrvHashes drvHashes; /* Look up the derivation by value and memoize the `hashDerivationModulo` call. */ -static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPath) +static DrvHashModulo pathDerivationModulo(Store & store, const StorePath & drvPath) { - std::optional hash; + std::optional hash; if (drvHashes.cvisit(drvPath, [&hash](const auto & kv) { hash.emplace(kv.second); })) { return *hash; } auto h = hashDerivationModulo(store, store.readInvalidDerivation(drvPath), false); + // Cache it drvHashes.insert_or_assign(drvPath, h); return h; @@ -895,12 +896,10 @@ static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPa don't leak the provenance of fixed outputs, reducing pointless cache misses as the build itself won't know this. */ -DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { - auto type = drv.type(); - /* Return a fixed hash for fixed-output derivations. */ - if (type.isFixed()) { + if (drv.type().isFixed()) { std::map outputHashes; for (const auto & i : drv.outputs) { auto & dof = std::get(i.second.raw); @@ -910,54 +909,66 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return DrvHash{ - .hashes = outputHashes, - .kind = DrvHash::Kind::Regular, - }; + return outputHashes; } - auto kind = std::visit( - overloaded{ - [](const DerivationType::InputAddressed & ia) { - /* This might be a "pesimistically" deferred output, so we don't - "taint" the kind yet. */ - return DrvHash::Kind::Regular; - }, - [](const DerivationType::ContentAddressed & ca) { - return ca.fixed ? DrvHash::Kind::Regular : DrvHash::Kind::Deferred; - }, - [](const DerivationType::Impure &) -> DrvHash::Kind { return DrvHash::Kind::Deferred; }}, - drv.type().raw); + if (std::visit( + overloaded{ + [](const DerivationType::InputAddressed & ia) { + /* This might be a "pesimistically" deferred output, so we don't + "taint" the kind yet. */ + return false; + }, + [](const DerivationType::ContentAddressed & ca) { + // Already covered + assert(!ca.fixed); + return true; + }, + [](const DerivationType::Impure &) { return true; }}, + drv.type().raw)) { + return DrvHashModulo::DeferredDrv{}; + } + /* For other derivations, replace the inputs paths with recursive + calls to this function. */ DerivedPathMap::ChildNode::Map inputs2; for (auto & [drvPath, node] : drv.inputDrvs.map) { + /* Need to build and resolve dynamic derivations first */ + if (!node.childMap.empty()) { + return DrvHashModulo::DeferredDrv{}; + } + const auto & res = pathDerivationModulo(store, drvPath); - if (res.kind == DrvHash::Kind::Deferred) - kind = DrvHash::Kind::Deferred; - for (auto & outputName : node.value) { - const auto h = get(res.hashes, outputName); - if (!h) - throw Error("no hash for output '%s' of derivation '%s'", outputName, drv.name); - inputs2[h->to_string(HashFormat::Base16, false)].value.insert(outputName); + if (std::visit( + overloaded{ + [&](const DrvHashModulo::DeferredDrv &) { return true; }, + // Regular non-CA derivation, replace derivation + [&](const DrvHashModulo::DrvHash & drvHash) { + inputs2.insert_or_assign(drvHash.to_string(HashFormat::Base16, false), node); + return false; + }, + // CA derivation's output hashes + [&](const DrvHashModulo::CaOutputHashes & outputHashes) { + for (auto & outputName : node.value) { + /* Put each one in with a single "out" output.. */ + const auto h = get(outputHashes, outputName); + if (!h) + throw Error("no hash for output '%s' of derivation '%s'", outputName, drv.name); + inputs2.insert_or_assign( + h->to_string(HashFormat::Base16, false), + DerivedPathMap::ChildNode{ + .value = {"out"}, + }); + } + return false; + }, + }, + res.raw)) { + return DrvHashModulo::DeferredDrv{}; } } - auto hash = hashString(HashAlgorithm::SHA256, drv.unparse(store, maskOutputs, &inputs2)); - - std::map outputHashes; - for (const auto & [outputName, _] : drv.outputs) { - outputHashes.insert_or_assign(outputName, hash); - } - - return DrvHash{ - .hashes = outputHashes, - .kind = kind, - }; -} - -std::map staticOutputHashes(Store & store, const Derivation & drv) -{ - return hashDerivationModulo(store, drv, true).hashes; + return hashString(HashAlgorithm::SHA256, drv.unparse(store, maskOutputs, &inputs2)); } static DerivationOutput readDerivationOutput(Source & in, const StoreDirConfig & store) @@ -1107,22 +1118,39 @@ void BasicDerivation::applyRewrites(const StringMap & rewrites) } } -static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) +void resolveInputAddressed(Store & store, Derivation & drv) { - drv.applyRewrites(rewrites); + std::optional hashModulo_; + + auto hashModulo = [&]() -> const auto & { + if (!hashModulo_) { + // somewhat expensive so we do lazily + hashModulo_ = hashDerivationModulo(store, drv, true); + } + return *hashModulo_; + }; - auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.raw)) { - auto h = get(hashModulo.hashes, outputName); - if (!h) - throw Error( - "derivation '%s' output '%s' has no hash (derivations.cc/rewriteDerivation)", drv.name, outputName); - auto outPath = store.makeOutputPath(outputName, *h, drv.name); - drv.env[outputName] = store.printStorePath(outPath); - output = DerivationOutput::InputAddressed{ - .path = std::move(outPath), - }; + std::visit( + overloaded{ + [&](const DrvHashModulo::DrvHash & drvHash) { + auto outPath = store.makeOutputPath(outputName, drvHash, drv.name); + drv.env.insert_or_assign(outputName, store.printStorePath(outPath)); + output = DerivationOutput::InputAddressed{ + .path = std::move(outPath), + }; + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + deferred (waiting to be input-addressed). */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + // Nothing to do, already deferred + }, + }, + hashModulo().raw); } } } @@ -1205,9 +1233,13 @@ std::optional Derivation::tryResolve( queryResolutionChain)) return std::nullopt; - rewriteDerivation(store, resolved, inputRewrites); + resolved.applyRewrites(inputRewrites); - return resolved; + Derivation resolved2{std::move(resolved)}; + + resolveInputAddressed(store, resolved2); + + return resolved2; } void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const @@ -1235,46 +1267,79 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const // combinations that are currently prohibited. type(); - std::optional hashesModulo; - for (auto & i : outputs) { + std::optional hashModulo_; + + auto hashModulo = [&]() -> const auto & { + if (!hashModulo_) { + // somewhat expensive so we do lazily + hashModulo_ = hashDerivationModulo(store, *this, true); + } + return *hashModulo_; + }; + + for (auto & [outputName, output] : outputs) { std::visit( overloaded{ [&](const DerivationOutput::InputAddressed & doia) { - if (!hashesModulo) { - // somewhat expensive so we do lazily - hashesModulo = hashDerivationModulo(store, *this, true); - } - auto currentOutputHash = get(hashesModulo->hashes, i.first); - if (!currentOutputHash) - throw Error( - "derivation '%s' has unexpected output '%s' (local-store / hashesModulo) named '%s'", - store.printStorePath(drvPath), - store.printStorePath(doia.path), - i.first); - StorePath recomputed = store.makeOutputPath(i.first, *currentOutputHash, drvName); - if (doia.path != recomputed) - throw Error( - "derivation '%s' has incorrect output '%s', should be '%s'", - store.printStorePath(drvPath), - store.printStorePath(doia.path), - store.printStorePath(recomputed)); - envHasRightPath(doia.path, i.first); + std::visit( + overloaded{ + [&](const DrvHashModulo::DrvHash & drvHash) { + StorePath recomputed = store.makeOutputPath(outputName, drvHash, drvName); + if (doia.path != recomputed) + throw Error( + "derivation '%s' has incorrect output '%s', should be '%s'", + store.printStorePath(drvPath), + store.printStorePath(doia.path), + store.printStorePath(recomputed)); + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + input-addressed. */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + throw Error( + "derivation '%s' has output '%s', but derivation is not yet ready to be input-addressed", + store.printStorePath(drvPath), + store.printStorePath(doia.path)); + }, + }, + hashModulo().raw); + envHasRightPath(doia.path, outputName); }, [&](const DerivationOutput::CAFixed & dof) { - auto path = dof.path(store, drvName, i.first); - envHasRightPath(path, i.first); + auto path = dof.path(store, drvName, outputName); + envHasRightPath(path, outputName); }, [&](const DerivationOutput::CAFloating &) { /* Nothing to check */ }, [&](const DerivationOutput::Deferred &) { /* Nothing to check */ + std::visit( + overloaded{ + [&](const DrvHashModulo::DrvHash & drvHash) { + throw Error( + "derivation '%s' has deferred output '%s', yet is ready to be input-addressed", + store.printStorePath(drvPath), + outputName); + }, + [&](const DrvHashModulo::CaOutputHashes &) { + /* Shouldn't happen as the original output is + input-addressed. */ + assert(false); + }, + [&](const DrvHashModulo::DeferredDrv &) { + /* Nothing to check */ + }, + }, + hashModulo().raw); }, [&](const DerivationOutput::Impure &) { /* Nothing to check */ }, }, - i.second.raw); + output.raw); } } diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 218a4e2ed..2a4c21b5f 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -332,7 +332,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) { + buildTrace.insert_or_visit({output.id.drvPath, {{output.id.outputName, ref}}}, [&](auto & kv) { kv.second.insert_or_assign(output.id.outputName, make_ref(output)); }); } @@ -341,7 +341,7 @@ struct DummyStoreImpl : DummyStore const DrvOutput & drvOutput, Callback> callback) noexcept override { bool visited = false; - buildTrace.cvisit(drvOutput.drvHash, [&](const auto & kv) { + buildTrace.cvisit(drvOutput.drvPath, [&](const auto & kv) { if (auto it = kv.second.find(drvOutput.outputName); it != kv.second.end()) { visited = true; callback(it->second.get_ptr()); diff --git a/src/libstore/include/nix/store/binary-cache-store.hh b/src/libstore/include/nix/store/binary-cache-store.hh index e64dc3eae..8623d5f4e 100644 --- a/src/libstore/include/nix/store/binary-cache-store.hh +++ b/src/libstore/include/nix/store/binary-cache-store.hh @@ -82,8 +82,13 @@ protected: /** * The prefix under which realisation infos will be stored + * + * @note The previous (still experimental, though) hash-keyed + * realisations were under "realisations". "build trace" is a better + * name anyways (issue #11895), and this serves as some light + * versioning. */ - constexpr const static std::string realisationsPrefix = "realisations"; + constexpr const static std::string realisationsPrefix = "build-trace"; constexpr const static std::string cacheInfoFile = "nix-cache-info"; @@ -92,7 +97,7 @@ protected: /** * Compute the path to the given realisation * - * It's `${realisationsPrefix}/${drvOutput}.doi`. + * It's `${realisationsPrefix}/${drvPath}/${outputName}`. */ std::string makeRealisationPath(const DrvOutput & id); diff --git a/src/libstore/include/nix/store/build/derivation-building-misc.hh b/src/libstore/include/nix/store/build/derivation-building-misc.hh index 2b68fa178..8d6892839 100644 --- a/src/libstore/include/nix/store/build/derivation-building-misc.hh +++ b/src/libstore/include/nix/store/build/derivation-building-misc.hh @@ -45,7 +45,6 @@ struct InitialOutputStatus struct InitialOutput { - Hash outputHash; std::optional known; }; diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 0fe610987..c732fd441 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -71,8 +71,6 @@ private: */ std::unique_ptr drv; - const Hash outputHash; - const BuildMode buildMode; /** diff --git a/src/libstore/include/nix/store/common-protocol-impl.hh b/src/libstore/include/nix/store/common-protocol-impl.hh index cb1020a3c..d0eebe0bc 100644 --- a/src/libstore/include/nix/store/common-protocol-impl.hh +++ b/src/libstore/include/nix/store/common-protocol-impl.hh @@ -26,12 +26,13 @@ namespace nix { LengthPrefixedProtoHelper::write(store, conn, t); \ } -#define COMMA_ , COMMON_USE_LENGTH_PREFIX_SERIALISER(template, std::vector) +#define COMMA_ , COMMON_USE_LENGTH_PREFIX_SERIALISER(template, std::set) COMMON_USE_LENGTH_PREFIX_SERIALISER(template, std::tuple) -COMMON_USE_LENGTH_PREFIX_SERIALISER(template, std::map) +COMMON_USE_LENGTH_PREFIX_SERIALISER( + template, std::map) #undef COMMA_ /* protocol-specific templates */ diff --git a/src/libstore/include/nix/store/common-protocol.hh b/src/libstore/include/nix/store/common-protocol.hh index c1d22fa6c..45bec3ff2 100644 --- a/src/libstore/include/nix/store/common-protocol.hh +++ b/src/libstore/include/nix/store/common-protocol.hh @@ -12,7 +12,6 @@ struct Source; class StorePath; struct ContentAddress; struct DrvOutput; -struct Realisation; /** * Shared serializers between the worker protocol, serve protocol, and a @@ -70,8 +69,6 @@ template<> DECLARE_COMMON_SERIALISER(ContentAddress); template<> DECLARE_COMMON_SERIALISER(DrvOutput); -template<> -DECLARE_COMMON_SERIALISER(Realisation); #define COMMA_ , template @@ -81,8 +78,8 @@ DECLARE_COMMON_SERIALISER(std::set); template DECLARE_COMMON_SERIALISER(std::tuple); -template -DECLARE_COMMON_SERIALISER(std::map); +template +DECLARE_COMMON_SERIALISER(std::map); #undef COMMA_ /** diff --git a/src/libstore/include/nix/store/derivations.hh b/src/libstore/include/nix/store/derivations.hh index 259314d3f..045ba0eab 100644 --- a/src/libstore/include/nix/store/derivations.hh +++ b/src/libstore/include/nix/store/derivations.hh @@ -427,34 +427,39 @@ std::string outputPathName(std::string_view drvName, OutputNameView outputName); * derivations (fixed-output or not) will have a different hash for each * output. */ -struct DrvHash +struct DrvHashModulo { /** - * Map from output names to hashes + * Single hash for the derivation + * + * This is for an input-addressed derivation that doesn't + * transitively depend on any floating-CA derivations. */ - std::map hashes; - - enum struct Kind : bool { - /** - * Statically determined derivations. - * This hash will be directly used to compute the output paths - */ - Regular, - - /** - * Floating-output derivations (and their reverse dependencies). - */ - Deferred, - }; + using DrvHash = Hash; /** - * The kind of derivation this is, simplified for just "derivation hash - * modulo" purposes. + * Known CA drv's output hashes, for fixed-output derivations whose + * output hashes are always known since they are fixed up-front. */ - Kind kind; -}; + using CaOutputHashes = std::map; -void operator|=(DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; + /** + * This derivation doesn't yet have known output hashes. + * + * Either because itself is floating CA, or it (transtively) depends + * on a floating CA derivation. + */ + using DeferredDrv = std::monostate; + + using Raw = std::variant; + + Raw raw; + + bool operator==(const DrvHashModulo &) const = default; + // auto operator <=> (const DrvHashModulo &) const = default; + + MAKE_WRAPPER_CONSTRUCTOR(DrvHashModulo); +}; /** * Returns hashes with the details of fixed-output subderivations @@ -480,15 +485,17 @@ void operator|=(DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; * ATerm, after subderivations have been likewise expunged from that * derivation. */ -DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); /** - * Return a map associating each output to a hash that uniquely identifies its - * derivation (modulo the self-references). + * If a derivation is input addressed and doesn't yet have its input + * addressed (is deferred) try using `hashDerivationModulo`. * - * \todo What is the Hash in this map? + * Does nothing if not deferred input-addressed, or + * `hashDerivationModulo` indicates it is missing inputs' output paths + * and is not yet ready (and must stay deferred). */ -std::map staticOutputHashes(Store & store, const Derivation & drv); +void resolveInputAddressed(Store & store, Derivation & drv); struct DrvHashFct { @@ -503,7 +510,7 @@ struct DrvHashFct /** * Memoisation of hashDerivationModulo(). */ -typedef boost::concurrent_flat_map DrvHashes; +typedef boost::concurrent_flat_map DrvHashes; // FIXME: global, though at least thread-safe. extern DrvHashes drvHashes; diff --git a/src/libstore/include/nix/store/dummy-store-impl.hh b/src/libstore/include/nix/store/dummy-store-impl.hh index 71869cb7e..42fa32d61 100644 --- a/src/libstore/include/nix/store/dummy-store-impl.hh +++ b/src/libstore/include/nix/store/dummy-store-impl.hh @@ -49,7 +49,7 @@ struct DummyStore : virtual Store * outer map for the derivation, and inner maps for the outputs of a * given derivation. */ - boost::concurrent_flat_map>> buildTrace; + boost::concurrent_flat_map>> buildTrace; DummyStore(ref config) : Store{*config} diff --git a/src/libstore/include/nix/store/length-prefixed-protocol-helper.hh b/src/libstore/include/nix/store/length-prefixed-protocol-helper.hh index 035019340..e1a80e8dc 100644 --- a/src/libstore/include/nix/store/length-prefixed-protocol-helper.hh +++ b/src/libstore/include/nix/store/length-prefixed-protocol-helper.hh @@ -56,14 +56,14 @@ LENGTH_PREFIXED_PROTO_HELPER(Inner, std::vector); #define COMMA_ , template LENGTH_PREFIXED_PROTO_HELPER(Inner, std::set); -#undef COMMA_ template LENGTH_PREFIXED_PROTO_HELPER(Inner, std::tuple); -template -#define LENGTH_PREFIXED_PROTO_HELPER_X std::map +template +#define LENGTH_PREFIXED_PROTO_HELPER_X std::map LENGTH_PREFIXED_PROTO_HELPER(Inner, LENGTH_PREFIXED_PROTO_HELPER_X); +#undef COMMA_ template std::vector @@ -109,11 +109,11 @@ void LengthPrefixedProtoHelper>::write( } } -template -std::map -LengthPrefixedProtoHelper>::read(const StoreDirConfig & store, typename Inner::ReadConn conn) +template +std::map LengthPrefixedProtoHelper>::read( + const StoreDirConfig & store, typename Inner::ReadConn conn) { - std::map resMap; + std::map resMap; auto size = readNum(conn.from); while (size--) { auto k = S::read(store, conn); @@ -123,9 +123,9 @@ LengthPrefixedProtoHelper>::read(const StoreDirConfig & st return resMap; } -template -void LengthPrefixedProtoHelper>::write( - const StoreDirConfig & store, typename Inner::WriteConn conn, const std::map & resMap) +template +void LengthPrefixedProtoHelper>::write( + const StoreDirConfig & store, typename Inner::WriteConn conn, const std::map & resMap) { conn.to << resMap.size(); for (auto & i : resMap) { diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index 3691c14fb..e5ac6d73d 100644 --- a/src/libstore/include/nix/store/realisation.hh +++ b/src/libstore/include/nix/store/realisation.hh @@ -18,33 +18,40 @@ struct OutputsSpec; /** * A general `Realisation` key. * - * This is similar to a `DerivedPath::Opaque`, but the derivation is - * identified by its "hash modulo" instead of by its store path. + * This is similar to a `DerivedPath::Built`, except it is only a single + * step: `drvPath` is a `StorePath` rather than a `DerivedPath`. */ struct DrvOutput { /** - * The hash modulo of the derivation. - * - * Computed from the derivation itself for most types of - * derivations, but computed from the (fixed) content address of the - * output for fixed-output derivations. + * The store path to the derivation */ - Hash drvHash; + StorePath drvPath; /** * The name of the output. */ OutputName outputName; + /** + * Skips the store dir on the `drvPath` + */ std::string to_string() const; - std::string strHash() const - { - return drvHash.to_string(HashFormat::Base16, true); - } + /** + * Skips the store dir on the `drvPath` + */ + static DrvOutput from_string(std::string_view); - static DrvOutput parse(const std::string &); + /** + * Includes the store dir on `drvPath` + */ + std::string render(const StoreDirConfig & store) const; + + /** + * Includes the store dir on `drvPath` + */ + static DrvOutput parse(const StoreDirConfig & store, std::string_view); bool operator==(const DrvOutput &) const = default; auto operator<=>(const DrvOutput &) const = default; @@ -64,6 +71,16 @@ struct UnkeyedRealisation size_t checkSignatures(const DrvOutput & key, const PublicKeys & publicKeys) const; + /** + * Just check the `outPath`. Signatures don't matter for this. + * Callers must ensure that the corresponding key is the same for + * most use-cases. + */ + bool isCompatibleWith(const UnkeyedRealisation & other) const + { + return outPath == other.outPath; + } + const StorePath & getPath() const { return outPath; @@ -77,8 +94,6 @@ struct Realisation : UnkeyedRealisation { DrvOutput id; - bool isCompatibleWith(const UnkeyedRealisation & other) const; - bool operator==(const Realisation &) const = default; auto operator<=>(const Realisation &) const = default; }; @@ -89,16 +104,7 @@ struct Realisation : UnkeyedRealisation * Since these are the outputs of a single derivation, we know the * output names are unique so we can use them as the map key. */ -typedef std::map SingleDrvOutputs; - -/** - * Collection type for multiple derivations' outputs' `Realisation`s. - * - * `DrvOutput` is used because in general the derivations are not all - * the same, so we need to identify firstly which derivation, and - * secondly which output of that derivation. - */ -typedef std::map DrvOutputs; +typedef std::map SingleDrvOutputs; struct OpaquePath { @@ -149,19 +155,17 @@ struct RealisedPath class MissingRealisation : public Error { public: - MissingRealisation(DrvOutput & outputId) - : MissingRealisation(outputId.outputName, outputId.strHash()) + MissingRealisation(const StoreDirConfig & store, DrvOutput & outputId) + : MissingRealisation(store, outputId.drvPath, outputId.outputName) { } - MissingRealisation(std::string_view drv, OutputName outputName) - : Error( - "cannot operate on output '%s' of the " - "unbuilt derivation '%s'", - outputName, - drv) - { - } + MissingRealisation(const StoreDirConfig & store, const StorePath & drvPath, const OutputName & outputName); + MissingRealisation( + const StoreDirConfig & store, + const SingleDerivedPath & drvPath, + const StorePath & drvPathResolved, + const OutputName & outputName); }; } // namespace nix diff --git a/src/libstore/include/nix/store/serve-protocol-impl.hh b/src/libstore/include/nix/store/serve-protocol-impl.hh index a9617165a..24fc3c9ab 100644 --- a/src/libstore/include/nix/store/serve-protocol-impl.hh +++ b/src/libstore/include/nix/store/serve-protocol-impl.hh @@ -34,8 +34,10 @@ SERVE_USE_LENGTH_PREFIX_SERIALISER(template, std::tuple) #define SERVE_USE_LENGTH_PREFIX_SERIALISER_COMMA , SERVE_USE_LENGTH_PREFIX_SERIALISER( - template, - std::map) + template + , + std::map) /** * Use `CommonProto` where possible. diff --git a/src/libstore/include/nix/store/serve-protocol.hh b/src/libstore/include/nix/store/serve-protocol.hh index 974bf42d5..1b548b517 100644 --- a/src/libstore/include/nix/store/serve-protocol.hh +++ b/src/libstore/include/nix/store/serve-protocol.hh @@ -8,7 +8,7 @@ namespace nix { #define SERVE_MAGIC_1 0x390c9deb #define SERVE_MAGIC_2 0x5452eecb -#define SERVE_PROTOCOL_VERSION (2 << 8 | 7) +#define SERVE_PROTOCOL_VERSION (2 << 8 | 8) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -18,6 +18,9 @@ struct Source; // items being serialised struct BuildResult; struct UnkeyedValidPathInfo; +struct DrvOutput; +struct UnkeyedRealisation; +struct Realisation; /** * The "serve protocol", used by ssh:// stores. @@ -178,6 +181,12 @@ inline std::ostream & operator<<(std::ostream & s, ServeProto::Command op) template<> DECLARE_SERVE_SERIALISER(BuildResult); template<> +DECLARE_SERVE_SERIALISER(DrvOutput); +template<> +DECLARE_SERVE_SERIALISER(UnkeyedRealisation); +template<> +DECLARE_SERVE_SERIALISER(Realisation); +template<> DECLARE_SERVE_SERIALISER(UnkeyedValidPathInfo); template<> DECLARE_SERVE_SERIALISER(ServeProto::BuildOptions); @@ -190,8 +199,8 @@ DECLARE_SERVE_SERIALISER(std::set); template DECLARE_SERVE_SERIALISER(std::tuple); -template -DECLARE_SERVE_SERIALISER(std::map); +template +DECLARE_SERVE_SERIALISER(std::map); #undef COMMA_ } // namespace nix diff --git a/src/libstore/include/nix/store/worker-protocol-impl.hh b/src/libstore/include/nix/store/worker-protocol-impl.hh index 26f6b9d44..605663d2e 100644 --- a/src/libstore/include/nix/store/worker-protocol-impl.hh +++ b/src/libstore/include/nix/store/worker-protocol-impl.hh @@ -34,8 +34,10 @@ WORKER_USE_LENGTH_PREFIX_SERIALISER(template, std::tuple) #define WORKER_USE_LENGTH_PREFIX_SERIALISER_COMMA , WORKER_USE_LENGTH_PREFIX_SERIALISER( - template, - std::map) + template + , + std::map) /** * Use `CommonProto` where possible. diff --git a/src/libstore/include/nix/store/worker-protocol.hh b/src/libstore/include/nix/store/worker-protocol.hh index 6ae5fdcbc..4374ba7a6 100644 --- a/src/libstore/include/nix/store/worker-protocol.hh +++ b/src/libstore/include/nix/store/worker-protocol.hh @@ -12,7 +12,7 @@ namespace nix { /* Note: you generally shouldn't change the protocol version. Define a new `WorkerProto::Feature` instead. */ -#define PROTOCOL_VERSION (1 << 8 | 38) +#define PROTOCOL_VERSION (1 << 8 | 39) #define MINIMUM_PROTOCOL_VERSION (1 << 8 | 18) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) @@ -35,6 +35,9 @@ struct BuildResult; struct KeyedBuildResult; struct ValidPathInfo; struct UnkeyedValidPathInfo; +struct DrvOutput; +struct UnkeyedRealisation; +struct Realisation; enum BuildMode : uint8_t; enum TrustedFlag : bool; @@ -261,6 +264,14 @@ DECLARE_WORKER_SERIALISER(ValidPathInfo); template<> DECLARE_WORKER_SERIALISER(UnkeyedValidPathInfo); template<> +DECLARE_WORKER_SERIALISER(DrvOutput); +template<> +DECLARE_WORKER_SERIALISER(UnkeyedRealisation); +template<> +DECLARE_WORKER_SERIALISER(Realisation); +template<> +DECLARE_WORKER_SERIALISER(std::optional); +template<> DECLARE_WORKER_SERIALISER(BuildMode); template<> DECLARE_WORKER_SERIALISER(std::optional); @@ -277,8 +288,8 @@ DECLARE_WORKER_SERIALISER(std::set); template DECLARE_WORKER_SERIALISER(std::tuple); -template -DECLARE_WORKER_SERIALISER(std::map); +template +DECLARE_WORKER_SERIALISER(std::map); #undef COMMA_ } // namespace nix diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 63730a01b..641a397b3 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -56,9 +56,10 @@ protected: void upsertFile( const std::string & path, RestartableSource & source, const std::string & mimeType, uint64_t sizeHint) override { - auto path2 = config->binaryCacheDir + "/" + path; + auto path2 = std::filesystem::path{config->binaryCacheDir} / path; static std::atomic counter{0}; - Path tmp = fmt("%s.tmp.%d.%d", path2, getpid(), ++counter); + createDirs(path2.parent_path()); + auto tmp = path2 + fmt(".tmp.%d.%d", getpid(), ++counter); AutoDelete del(tmp, false); writeFile(tmp, source); std::filesystem::rename(tmp, path2); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 98f2d70af..cae2dc4a0 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -110,8 +110,6 @@ struct LocalStore::State::Stmts SQLiteStmt QueryAllRealisedOutputs; SQLiteStmt QueryPathFromHashPart; SQLiteStmt QueryValidPaths; - SQLiteStmt QueryRealisationReferences; - SQLiteStmt AddRealisationReference; }; LocalStore::LocalStore(ref config) @@ -621,7 +619,7 @@ void LocalStore::registerDrvOutput(const Realisation & info) auto combinedSignatures = oldR->signatures; combinedSignatures.insert(info.signatures.begin(), info.signatures.end()); state->stmts->UpdateRealisedOutput - .use()(concatStringsSep(" ", combinedSignatures))(info.id.strHash())(info.id.outputName) + .use()(concatStringsSep(" ", combinedSignatures))(info.id.drvPath.to_string())(info.id.outputName) .exec(); } else { throw Error( @@ -635,7 +633,7 @@ void LocalStore::registerDrvOutput(const Realisation & info) } } else { state->stmts->RegisterRealisedOutput - .use()(info.id.strHash())(info.id.outputName)(printStorePath(info.outPath))( + .use()(info.id.drvPath.to_string())(info.id.outputName)(printStorePath(info.outPath))( concatStringsSep(" ", info.signatures)) .exec(); } @@ -1553,7 +1551,7 @@ void LocalStore::addSignatures(const StorePath & storePath, const StringSet & si std::optional> LocalStore::queryRealisationCore_(LocalStore::State & state, const DrvOutput & id) { - auto useQueryRealisedOutput(state.stmts->QueryRealisedOutput.use()(id.strHash())(id.outputName)); + auto useQueryRealisedOutput(state.stmts->QueryRealisedOutput.use()(id.drvPath.to_string())(id.outputName)); if (!useQueryRealisedOutput.next()) return std::nullopt; auto realisationDbId = useQueryRealisedOutput.getInt(0); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 6829563c2..b46d900b7 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -240,16 +240,15 @@ MissingPaths Store::queryMissing(const std::vector & targets) // If there are unknown output paths, attempt to find if the // paths are known to substituters through a realisation. - auto outputHashes = staticOutputHashes(*this, *drv); knownOutputPaths = true; - for (auto [outputName, hash] : outputHashes) { + for (auto & [outputName, _] : drv->outputs) { if (!bfd.outputs.contains(outputName)) continue; bool found = false; for (auto & sub : getDefaultSubstituters()) { - auto realisation = sub->queryRealisation({hash, outputName}); + auto realisation = sub->queryRealisation({drvPath, outputName}); if (!realisation) continue; found = true; @@ -362,7 +361,7 @@ OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, OutputPathMap outputs; for (auto & [outputName, outputPathOpt] : outputsOpt) { if (!outputPathOpt) - throw MissingRealisation(bfd.drvPath->to_string(store), outputName); + throw MissingRealisation(store, *bfd.drvPath, drvPath, outputName); auto & outputPath = *outputPathOpt; outputs.insert_or_assign(outputName, outputPath); } @@ -386,7 +385,7 @@ StorePath resolveDerivedPath(Store & store, const SingleDerivedPath & req, Store bfd.output); auto & optPath = outputPaths.at(bfd.output); if (!optPath) - throw MissingRealisation(bfd.drvPath->to_string(store), bfd.output); + throw MissingRealisation(store, *bfd.drvPath, drvPath, bfd.output); return *optPath; }, }, diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 11608a667..116957e55 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -44,10 +44,16 @@ create table if not exists NARs ( create table if not exists Realisations ( cache integer not null, - outputId text not null, - content blob, -- Json serialisation of the realisation, or null if the realisation is absent + + drvPath text not null, + outputName text not null, + + -- The following are null if the realisation is absent + outputPath text, + sigs text, + timestamp integer not null, - primary key (cache, outputId), + primary key (cache, drvPath, outputName), foreign key (cache) references BinaryCaches(id) on delete cascade ); @@ -121,24 +127,24 @@ public: state->insertRealisation.create( state->db, R"( - insert or replace into Realisations(cache, outputId, content, timestamp) - values (?, ?, ?, ?) + insert or replace into Realisations(cache, drvPath, outputName, outputPath, sigs, timestamp) + values (?, ?, ?, ?, ?, ?) )"); state->insertMissingRealisation.create( state->db, R"( - insert or replace into Realisations(cache, outputId, timestamp) - values (?, ?, ?) + insert or replace into Realisations(cache, drvPath, outputName, timestamp) + values (?, ?, ?, ?) )"); state->queryRealisation.create( state->db, R"( - select content from Realisations - where cache = ? and outputId = ? and - ((content is null and timestamp > ?) or - (content is not null and timestamp > ?)) + select outputPath, sigs from Realisations + where cache = ? and drvPath = ? and outputName = ? and + ((outputPath is null and timestamp > ?) or + (outputPath is not null and timestamp > ?)) )"); /* Periodically purge expired entries from the database. */ @@ -295,22 +301,27 @@ public: auto now = time(0); - auto queryRealisation(state->queryRealisation.use()(cache.id)(id.to_string())( + auto queryRealisation(state->queryRealisation.use()(cache.id)(id.drvPath.to_string())(id.outputName)( now - settings.ttlNegativeNarInfoCache)(now - settings.ttlPositiveNarInfoCache)); if (!queryRealisation.next()) - return {oUnknown, 0}; + return {oUnknown, nullptr}; if (queryRealisation.isNull(0)) - return {oInvalid, 0}; + return {oInvalid, nullptr}; try { return { oValid, - std::make_shared(nlohmann::json::parse(queryRealisation.getStr(0))), + std::make_shared( + UnkeyedRealisation{ + .outPath = StorePath{queryRealisation.getStr(0)}, + .signatures = nlohmann::json::parse(queryRealisation.getStr(1)), + }, + id), }; } catch (Error & e) { - e.addTrace({}, "while parsing the local disk cache"); + e.addTrace({}, "reading build trace key-value from the local disk cache"); throw; } }); @@ -355,7 +366,9 @@ public: auto & cache(getCache(*state, uri)); state->insertRealisation - .use()(cache.id)(realisation.id.to_string())(static_cast(realisation).dump())(time(0)) + .use()(cache.id)(realisation.id.drvPath.to_string())(realisation.id.outputName)( + realisation.outPath.to_string())(static_cast(realisation.signatures).dump())( + time(0)) .exec(); }); } @@ -366,7 +379,7 @@ public: auto state(_state.lock()); auto & cache(getCache(*state, uri)); - state->insertMissingRealisation.use()(cache.id)(id.to_string())(time(0)).exec(); + state->insertMissingRealisation.use()(cache.id)(id.drvPath.to_string())(id.outputName)(time(0)).exec(); }); } }; diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 2075b39e6..aaad8f662 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -8,28 +8,34 @@ namespace nix { MakeError(InvalidDerivationOutputId, Error); -DrvOutput DrvOutput::parse(const std::string & strRep) +DrvOutput DrvOutput::parse(const StoreDirConfig & store, std::string_view s) { - size_t n = strRep.find("!"); - if (n == strRep.npos) - throw InvalidDerivationOutputId("Invalid derivation output id %s", strRep); - + size_t n = s.rfind('^'); + if (n == s.npos) + throw InvalidDerivationOutputId("Invalid derivation output id '%s': missing '^'", s); return DrvOutput{ - .drvHash = Hash::parseAnyPrefixed(strRep.substr(0, n)), - .outputName = strRep.substr(n + 1), + .drvPath = store.parseStorePath(s.substr(0, n)), + .outputName = OutputName{s.substr(n + 1)}, }; } +std::string DrvOutput::render(const StoreDirConfig & store) const +{ + return std::string(store.printStorePath(drvPath)) + "^" + outputName; +} + std::string DrvOutput::to_string() const { - return strHash() + "!" + outputName; + return std::string(drvPath.to_string()) + "^" + outputName; } std::string UnkeyedRealisation::fingerprint(const DrvOutput & key) const { - nlohmann::json serialized = Realisation{*this, key}; - serialized.erase("signatures"); - return serialized.dump(); + auto serialised = static_cast(Realisation{*this, key}); + auto value = serialised.find("value"); + assert(value != serialised.end()); + value->erase("signatures"); + return serialised.dump(); } void UnkeyedRealisation::sign(const DrvOutput & key, const Signer & signer) @@ -61,9 +67,20 @@ const StorePath & RealisedPath::path() const & return std::visit([](auto & arg) -> auto & { return arg.getPath(); }, raw); } -bool Realisation::isCompatibleWith(const UnkeyedRealisation & other) const +MissingRealisation::MissingRealisation( + const StoreDirConfig & store, const StorePath & drvPath, const OutputName & outputName) + : Error("cannot operate on output '%s' of the unbuilt derivation '%s'", outputName, store.printStorePath(drvPath)) { - return outPath == other.outPath; +} + +MissingRealisation::MissingRealisation( + const StoreDirConfig & store, + const SingleDerivedPath & drvPath, + const StorePath & drvPathResolved, + const OutputName & outputName) + : MissingRealisation{store, drvPathResolved, outputName} +{ + addTrace({}, "looking up realisation for derivation '%s'", drvPath.to_string(store)); } } // namespace nix @@ -74,24 +91,32 @@ using namespace nix; DrvOutput adl_serializer::from_json(const json & json) { - return DrvOutput::parse(getString(json)); + auto obj = getObject(json); + + return { + .drvPath = valueAt(obj, "drvPath"), + .outputName = getString(valueAt(obj, "outputName")), + }; } void adl_serializer::to_json(json & json, const DrvOutput & drvOutput) { - json = drvOutput.to_string(); + json = { + {"drvPath", drvOutput.drvPath}, + {"outputName", drvOutput.outputName}, + }; } -UnkeyedRealisation adl_serializer::from_json(const json & json0) +UnkeyedRealisation adl_serializer::from_json(const json & json) { - auto json = getObject(json0); + auto obj = getObject(json); StringSet signatures; - if (auto signaturesOpt = optionalValueAt(json, "signatures")) - signatures = *signaturesOpt; + if (auto * signaturesJson = get(obj, "signatures")) + signatures = getStringSet(*signaturesJson); - return UnkeyedRealisation{ - .outPath = valueAt(json, "outPath"), + return { + .outPath = valueAt(obj, "outPath"), .signatures = signatures, }; } @@ -101,25 +126,25 @@ void adl_serializer::to_json(json & json, const UnkeyedReali json = { {"outPath", r.outPath}, {"signatures", r.signatures}, - // back-compat - {"dependentRealisations", json::object()}, }; } -Realisation adl_serializer::from_json(const json & json0) +Realisation adl_serializer::from_json(const json & json) { - auto json = getObject(json0); + auto obj = getObject(json); - return Realisation{ - static_cast(json0), - valueAt(json, "id"), + return { + static_cast(valueAt(obj, "value")), + static_cast(valueAt(obj, "key")), }; } void adl_serializer::to_json(json & json, const Realisation & r) { - json = static_cast(r); - json["id"] = r.id; + json = { + {"key", r.id}, + {"value", static_cast(r)}, + }; } } // namespace nlohmann diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 949a51f18..6a8e48a6d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -507,7 +507,7 @@ void RemoteStore::queryRealisationUncached( try { auto conn(getConnection()); - if (GET_PROTOCOL_MINOR(conn->protoVersion) < 27) { + if (GET_PROTOCOL_MINOR(conn->protoVersion) < 39) { warn("the daemon is too old to support content-addressing derivations, please upgrade it to 2.4"); return callback(nullptr); } @@ -516,21 +516,12 @@ void RemoteStore::queryRealisationUncached( conn->to << id.to_string(); conn.processStderr(); - 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()}); - } else { - auto realisations = WorkerProto::Serialise>::read(*this, *conn); - if (realisations.empty()) - return nullptr; - return std::make_shared(*realisations.begin()); - } - }(); - - callback(std::shared_ptr(real)); + callback([&]() -> std::shared_ptr { + auto realisation = WorkerProto::Serialise>::read(*this, *conn); + if (!realisation) + return nullptr; + return std::make_shared(*realisation); + }()); } catch (...) { return callback.rethrow(); } @@ -612,30 +603,19 @@ std::vector RemoteStore::buildPathsWithResults( OutputPathMap outputs; auto drvPath = resolveDerivedPath(*evalStore, *bfd.drvPath); - auto drv = evalStore->readDerivation(drvPath); - const auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive auto built = resolveDerivedPath(*this, bfd, &*evalStore); for (auto & [output, outputPath] : built) { - auto outputHash = get(outputHashes, output); - if (!outputHash) - throw Error( - "the derivation '%s' doesn't have an output named '%s'", - printStorePath(drvPath), - output); - auto outputId = DrvOutput{*outputHash, output}; + auto outputId = DrvOutput{drvPath, output}; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) { auto realisation = queryRealisation(outputId); if (!realisation) - throw MissingRealisation(outputId); - success.builtOutputs.emplace(output, Realisation{*realisation, outputId}); + throw MissingRealisation(*this, outputId); + success.builtOutputs.emplace(output, *realisation); } else { success.builtOutputs.emplace( output, - Realisation{ - UnkeyedRealisation{ - .outPath = outputPath, - }, - outputId, + UnkeyedRealisation{ + .outPath = outputPath, }); } } diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index b4d696a53..99650870e 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -281,9 +281,18 @@ std::vector RestrictedStore::buildPathsWithResults( for (auto & result : results) { if (auto * successP = result.tryGetSuccess()) { - for (auto & [outputName, output] : successP->builtOutputs) { - newPaths.insert(output.outPath); - newRealisations.insert(output); + if (auto * pathBuilt = std::get_if(&result.path)) { + // TODO ugly extra IO + auto drvPath = resolveDerivedPath(*next, *pathBuilt->drvPath); + for (auto & [outputName, output] : successP->builtOutputs) { + newPaths.insert(output.outPath); + newRealisations.insert( + {output, + { + .drvPath = drvPath, + .outputName = outputName, + }}); + } } } } diff --git a/src/libstore/serve-protocol.cc b/src/libstore/serve-protocol.cc index 51b575fcd..d5f8a9ea9 100644 --- a/src/libstore/serve-protocol.cc +++ b/src/libstore/serve-protocol.cc @@ -6,6 +6,7 @@ #include "nix/store/serve-protocol-impl.hh" #include "nix/util/archive.hh" #include "nix/store/path-info.hh" +#include "nix/util/json-utils.hh" #include @@ -24,10 +25,19 @@ BuildResult ServeProto::Serialise::read(const StoreDirConfig & stor if (GET_PROTOCOL_MINOR(conn.version) >= 3) conn.from >> status.timesBuilt >> failure.isNonDeterministic >> status.startTime >> status.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { - auto builtOutputs = ServeProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) - success.builtOutputs.insert_or_assign(std::move(output.outputName), std::move(realisation)); + + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { + success.builtOutputs = ServeProto::Serialise>::read(store, conn); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + for (auto & [output, realisation] : ServeProto::Serialise::read(store, conn)) { + size_t n = output.find("!"); + if (n == output.npos) + throw Error("Invalid derivation output id %s", output); + success.builtOutputs.insert_or_assign( + output.substr(n + 1), + UnkeyedRealisation{ + StorePath{getString(valueAt(getObject(nlohmann::json::parse(realisation)), "outPath"))}}); + } } if (BuildResult::Success::statusIs(rawStatus)) { @@ -51,15 +61,18 @@ void ServeProto::Serialise::write( default value for the fields that don't exist in that case. */ auto common = [&](std::string_view errorMsg, bool isNonDeterministic, const auto & builtOutputs) { conn.to << errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 3) conn.to << res.timesBuilt << isNonDeterministic << res.startTime << res.stopTime; - if (GET_PROTOCOL_MINOR(conn.version) >= 6) { - DrvOutputs builtOutputsFullKey; - for (auto & [output, realisation] : builtOutputs) - builtOutputsFullKey.insert_or_assign(realisation.id, realisation); - ServeProto::write(store, conn, builtOutputsFullKey); + + if (GET_PROTOCOL_MINOR(conn.version) >= 8) { + ServeProto::write(store, conn, builtOutputs); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 6) { + // We no longer support these types of realisations + ServeProto::write(store, conn, StringMap{}); } }; + std::visit( overloaded{ [&](const BuildResult::Failure & failure) { @@ -144,4 +157,82 @@ void ServeProto::Serialise::write( } } +UnkeyedRealisation ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error( + "daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + + auto outPath = ServeProto::Serialise::read(store, conn); + auto signatures = ServeProto::Serialise::read(store, conn); + + return UnkeyedRealisation{ + .outPath = std::move(outPath), + .signatures = std::move(signatures), + }; +} + +void ServeProto::Serialise::write( + const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error( + "daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + ServeProto::write(store, conn, info.outPath); + ServeProto::write(store, conn, info.signatures); +} + +DrvOutput ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error( + "daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + + auto drvPath = ServeProto::Serialise::read(store, conn); + auto outputName = ServeProto::Serialise::read(store, conn); + + return DrvOutput{ + .drvPath = std::move(drvPath), + .outputName = std::move(outputName), + }; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const DrvOutput & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 8) { + throw Error( + "daemon protocol %d.%d is too old (< 2.8) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + ServeProto::write(store, conn, info.drvPath); + ServeProto::write(store, conn, info.outputName); +} + +Realisation ServeProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + auto id = ServeProto::Serialise::read(store, conn); + auto unkeyed = ServeProto::Serialise::read(store, conn); + + return Realisation{ + std::move(unkeyed), + std::move(id), + }; +} + +void ServeProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const Realisation & info) +{ + ServeProto::write(store, conn, info.id); + ServeProto::write(store, conn, static_cast(info)); +} + } // namespace nix diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index c70af9301..71642431f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -367,9 +367,8 @@ Store::queryPartialDerivationOutputMap(const StorePath & path, Store * evalStore return outputs; auto drv = evalStore.readInvalidDerivation(path); - auto drvHashes = staticOutputHashes(*this, drv); - for (auto & [outputName, hash] : drvHashes) { - auto realisation = queryRealisation(DrvOutput{hash, outputName}); + for (auto & [outputName, _] : drv.outputs) { + auto realisation = queryRealisation(DrvOutput{path, outputName}); if (realisation) { outputs.insert_or_assign(outputName, realisation->outPath); } else { @@ -389,7 +388,7 @@ OutputPathMap Store::queryDerivationOutputMap(const StorePath & path, Store * ev OutputPathMap result; for (auto & [outName, optOutPath] : resp) { if (!optOutPath) - throw MissingRealisation(printStorePath(path), outName); + throw MissingRealisation(*this, path, outName); result.insert_or_assign(outName, *optOutPath); } return result; diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index c2ef730dc..35b0063db 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1870,7 +1870,10 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() { .outPath = newInfo.path, }, - DrvOutput{oldinfo->outputHash, outputName}, + DrvOutput{ + .drvPath = drvPath, + .outputName = outputName, + }, }; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().isImpure()) { store.signRealisation(thisRealisation); diff --git a/src/libstore/worker-protocol.cc b/src/libstore/worker-protocol.cc index a17d2c028..c6719974b 100644 --- a/src/libstore/worker-protocol.cc +++ b/src/libstore/worker-protocol.cc @@ -6,6 +6,7 @@ #include "nix/store/worker-protocol-impl.hh" #include "nix/util/archive.hh" #include "nix/store/path-info.hh" +#include "nix/util/json-utils.hh" #include #include @@ -132,7 +133,7 @@ void WorkerProto::Serialise::write( throw Error( "trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", store.printStorePath(drvPath), - GET_PROTOCOL_MAJOR(conn.version), + GET_PROTOCOL_MAJOR(conn.version) >> 8, GET_PROTOCOL_MINOR(conn.version)); }, [&](std::monostate) { @@ -174,14 +175,24 @@ BuildResult WorkerProto::Serialise::read(const StoreDirConfig & sto if (GET_PROTOCOL_MINOR(conn.version) >= 29) { conn.from >> res.timesBuilt >> failure.isNonDeterministic >> res.startTime >> res.stopTime; } + if (GET_PROTOCOL_MINOR(conn.version) >= 37) { res.cpuUser = WorkerProto::Serialise>::read(store, conn); res.cpuSystem = WorkerProto::Serialise>::read(store, conn); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { - auto builtOutputs = WorkerProto::Serialise::read(store, conn); - for (auto && [output, realisation] : builtOutputs) - success.builtOutputs.insert_or_assign(std::move(output.outputName), std::move(realisation)); + + if (GET_PROTOCOL_MINOR(conn.version) >= 39) { + success.builtOutputs = WorkerProto::Serialise>::read(store, conn); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + for (auto && [output, realisation] : WorkerProto::Serialise::read(store, conn)) { + size_t n = output.find("!"); + if (n == output.npos) + throw Error("Invalid derivation output id %s", output); + success.builtOutputs.insert_or_assign( + output.substr(n + 1), + UnkeyedRealisation{ + StorePath{getString(valueAt(getObject(nlohmann::json::parse(realisation)), "outPath"))}}); + } } if (BuildResult::Success::statusIs(rawStatus)) { @@ -205,20 +216,24 @@ void WorkerProto::Serialise::write( default value for the fields that don't exist in that case. */ auto common = [&](std::string_view errorMsg, bool isNonDeterministic, const auto & builtOutputs) { conn.to << errorMsg; + if (GET_PROTOCOL_MINOR(conn.version) >= 29) { conn.to << res.timesBuilt << isNonDeterministic << res.startTime << res.stopTime; } + if (GET_PROTOCOL_MINOR(conn.version) >= 37) { WorkerProto::write(store, conn, res.cpuUser); WorkerProto::write(store, conn, res.cpuSystem); } - if (GET_PROTOCOL_MINOR(conn.version) >= 28) { - DrvOutputs builtOutputsFullKey; - for (auto & [output, realisation] : builtOutputs) - builtOutputsFullKey.insert_or_assign(realisation.id, realisation); - WorkerProto::write(store, conn, builtOutputsFullKey); + + if (GET_PROTOCOL_MINOR(conn.version) >= 39) { + WorkerProto::write(store, conn, builtOutputs); + } else if (GET_PROTOCOL_MINOR(conn.version) >= 28) { + // Don't support those types of realisations anymore. + WorkerProto::write(store, conn, StringMap{}); } }; + std::visit( overloaded{ [&](const BuildResult::Failure & failure) { @@ -309,4 +324,113 @@ void WorkerProto::Serialise::write( } } +UnkeyedRealisation WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error( + "daemon protocol %d.%d is too old (< 1.39) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + + auto outPath = WorkerProto::Serialise::read(store, conn); + auto signatures = WorkerProto::Serialise::read(store, conn); + + return UnkeyedRealisation{ + .outPath = std::move(outPath), + .signatures = std::move(signatures), + }; +} + +void WorkerProto::Serialise::write( + const StoreDirConfig & store, WriteConn conn, const UnkeyedRealisation & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error( + "daemon protocol %d.%d is too old (< 1.39) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + WorkerProto::write(store, conn, info.outPath); + WorkerProto::write(store, conn, info.signatures); +} + +std::optional +WorkerProto::Serialise>::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + // Hack to improve compat + (void) WorkerProto::Serialise::read(store, conn); + return std::nullopt; + } else { + auto temp = readNum(conn.from); + switch (temp) { + case 0: + return std::nullopt; + case 1: + return WorkerProto::Serialise::read(store, conn); + default: + throw Error("Invalid optional build trace from remote"); + } + } +} + +void WorkerProto::Serialise>::write( + const StoreDirConfig & store, WriteConn conn, const std::optional & info) +{ + if (!info) { + conn.to << uint8_t{0}; + } else { + conn.to << uint8_t{1}; + WorkerProto::write(store, conn, *info); + } +} + +DrvOutput WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error( + "daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + + auto drvPath = WorkerProto::Serialise::read(store, conn); + auto outputName = WorkerProto::Serialise::read(store, conn); + + return DrvOutput{ + .drvPath = std::move(drvPath), + .outputName = std::move(outputName), + }; +} + +void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const DrvOutput & info) +{ + if (GET_PROTOCOL_MINOR(conn.version) < 39) { + throw Error( + "daemon protocol %d.%d is too old (< 1.29) to understand build trace", + GET_PROTOCOL_MAJOR(conn.version) >> 8, + GET_PROTOCOL_MINOR(conn.version)); + } + WorkerProto::write(store, conn, info.drvPath); + WorkerProto::write(store, conn, info.outputName); +} + +Realisation WorkerProto::Serialise::read(const StoreDirConfig & store, ReadConn conn) +{ + auto id = WorkerProto::Serialise::read(store, conn); + auto unkeyed = WorkerProto::Serialise::read(store, conn); + + return Realisation{ + std::move(unkeyed), + std::move(id), + }; +} + +void WorkerProto::Serialise::write(const StoreDirConfig & store, WriteConn conn, const Realisation & info) +{ + WorkerProto::write(store, conn, info.id); + WorkerProto::write(store, conn, static_cast(info)); +} + } // namespace nix diff --git a/src/nix/build-remote/build-remote.cc b/src/nix/build-remote/build-remote.cc index ffb77ddf1..1e52daadc 100644 --- a/src/nix/build-remote/build-remote.cc +++ b/src/nix/build-remote/build-remote.cc @@ -346,13 +346,11 @@ static int main_build_remote(int argc, char ** argv) optResult = std::move(res[0]); } - auto outputHashes = staticOutputHashes(*store, drv); std::set missingRealisations; StorePathSet missingPaths; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) { for (auto & outputName : wantedOutputs) { - auto thisOutputHash = outputHashes.at(outputName); - auto thisOutputId = DrvOutput{thisOutputHash, outputName}; + auto thisOutputId = DrvOutput{*drvPath, outputName}; if (!store->queryRealisation(thisOutputId)) { debug("missing output %s", outputName); assert(optResult); @@ -362,7 +360,7 @@ static int main_build_remote(int argc, char ** argv) auto i = success.builtOutputs.find(outputName); assert(i != success.builtOutputs.end()); auto & newRealisation = i->second; - missingRealisations.insert(newRealisation); + missingRealisations.insert({newRealisation, thisOutputId}); missingPaths.insert(newRealisation.outPath); } } diff --git a/src/nix/develop.cc b/src/nix/develop.cc index d23dce10b..0f894b9de 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -279,16 +279,8 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore output.second = DerivationOutput::Deferred{}; drv.env[output.first] = ""; } - auto hashesModulo = hashDerivationModulo(*evalStore, drv, true); - for (auto & output : drv.outputs) { - Hash h = hashesModulo.hashes.at(output.first); - auto outPath = store->makeOutputPath(output.first, h, drv.name); - output.second = DerivationOutput::InputAddressed{ - .path = outPath, - }; - drv.env[output.first] = store->printStorePath(outPath); - } + resolveInputAddressed(*evalStore, drv); } auto shellDrvPath = writeDerivation(*evalStore, drv); diff --git a/src/perl/lib/Nix/Store.xs b/src/perl/lib/Nix/Store.xs index 93e9f0f95..6d5dcb400 100644 --- a/src/perl/lib/Nix/Store.xs +++ b/src/perl/lib/Nix/Store.xs @@ -163,10 +163,13 @@ StoreWrapper::queryPathInfo(char * path, int base32) } SV * -StoreWrapper::queryRawRealisation(char * outputId) +StoreWrapper::queryRawRealisation(char * drvPath, char * outputName) PPCODE: try { - auto realisation = THIS->store->queryRealisation(DrvOutput::parse(outputId)); + auto realisation = THIS->store->queryRealisation(DrvOutput{ + .drvPath = THIS->store->parseStorePath(drvPath), + .outputName = outputName, + }); if (realisation) XPUSHs(sv_2mortal(newSVpv(static_cast(*realisation).dump().c_str(), 0))); else diff --git a/tests/functional/ca/substitute.sh b/tests/functional/ca/substitute.sh index 2f6ebcef5..c4cbf2c96 100644 --- a/tests/functional/ca/substitute.sh +++ b/tests/functional/ca/substitute.sh @@ -49,14 +49,14 @@ fi clearStore nix build --file ../simple.nix -L --no-link --post-build-hook "$pushToStore" clearStore -rm -r "$REMOTE_STORE_DIR/realisations" +rm -r "$REMOTE_STORE_DIR/build-trace" nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 # There's no easy way to check whether a realisation is present on the local # store − short of manually querying the db, but the build environment doesn't # have the sqlite binary − so we instead push things again, and check that the # realisations have correctly been pushed to the remote store nix copy --to "$REMOTE_STORE" --file ../simple.nix -if [[ -z "$(ls "$REMOTE_STORE_DIR/realisations")" ]]; then +if [[ -z "$(ls "$REMOTE_STORE_DIR/build-trace")" ]]; then echo "Realisations not rebuilt" exit 1 fi @@ -71,5 +71,5 @@ buildDrvs --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 # Try rebuilding, but remove the realisations from the remote cache to force # using the cachecache clearStore -rm "$REMOTE_STORE_DIR"/realisations/* +rm -r "$REMOTE_STORE_DIR"/build-trace/* buildDrvs --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0