diff --git a/doc/manual/source/protocols/json/build-trace-entry.md b/doc/manual/source/protocols/json/build-trace-entry.md index 8050a2840bf..9eea9371260 100644 --- a/doc/manual/source/protocols/json/build-trace-entry.md +++ b/doc/manual/source/protocols/json/build-trace-entry.md @@ -1,27 +1,21 @@ -{{#include build-trace-entry-v1-fixed.md}} +{{#include build-trace-entry-v2-fixed.md}} ## Examples ### Simple build trace entry ```json -{{#include schema/build-trace-entry-v1/simple.json}} -``` - -### Build trace entry with dependencies - -```json -{{#include schema/build-trace-entry-v1/with-dependent-realisations.json}} +{{#include schema/build-trace-entry-v2/simple.json}} ``` ### Build trace entry with signature ```json -{{#include schema/build-trace-entry-v1/with-signature.json}} +{{#include schema/build-trace-entry-v2/with-signature.json}} ``` \ No newline at end of file +[JSON Schema for Build Trace Entry v1](schema/build-trace-entry-v2.json) +--> diff --git a/doc/manual/source/protocols/json/meson.build b/doc/manual/source/protocols/json/meson.build index ab9d76d3e9b..06af4e40c82 100644 --- a/doc/manual/source/protocols/json/meson.build +++ b/doc/manual/source/protocols/json/meson.build @@ -17,7 +17,7 @@ schemas = [ 'derivation-v4', 'derivation-options-v1', 'deriving-path-v1', - 'build-trace-entry-v1', + 'build-trace-entry-v2', 'build-result-v1', 'store-v1', ] diff --git a/doc/manual/source/protocols/json/schema/build-result-v1.yaml b/doc/manual/source/protocols/json/schema/build-result-v1.yaml index 31f59a44dda..55e55d3e5cb 100644 --- a/doc/manual/source/protocols/json/schema/build-result-v1.yaml +++ b/doc/manual/source/protocols/json/schema/build-result-v1.yaml @@ -83,7 +83,7 @@ properties: description: | A mapping from output names to their build trace entries. additionalProperties: - "$ref": "build-trace-entry-v1.yaml" + "$ref": "build-trace-entry-v2.yaml" failure: type: object diff --git a/doc/manual/source/protocols/json/schema/build-trace-entry-v1 b/doc/manual/source/protocols/json/schema/build-trace-entry-v2 similarity index 100% rename from doc/manual/source/protocols/json/schema/build-trace-entry-v1 rename to doc/manual/source/protocols/json/schema/build-trace-entry-v2 diff --git a/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml b/doc/manual/source/protocols/json/schema/build-trace-entry-v2.yaml similarity index 74% rename from doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml rename to doc/manual/source/protocols/json/schema/build-trace-entry-v2.yaml index a85738b50b9..4340f82388c 100644 --- a/doc/manual/source/protocols/json/schema/build-trace-entry-v1.yaml +++ b/doc/manual/source/protocols/json/schema/build-trace-entry-v2.yaml @@ -1,5 +1,5 @@ "$schema": "http://json-schema.org/draft-04/schema" -"$id": "https://nix.dev/manual/nix/latest/protocols/json/schema/build-trace-entry-v1.json" +"$id": "https://nix.dev/manual/nix/latest/protocols/json/schema/build-trace-entry-v2.json" title: Build Trace Entry description: | A record of a successful build outcome for a specific derivation output. @@ -11,10 +11,17 @@ description: | > This JSON format is currently > [**experimental**](@docroot@/development/experimental-features.md#xp-feature-ca-derivations) > and subject to change. + + Verision history: + + - Version 1: Original format + + - Version 2: Remove `dependentRealisations` + +type: object required: - id - outPath - - dependentRealisations - signatures allOf: - "$ref": "#/$defs/key" @@ -22,9 +29,11 @@ allOf: properties: id: {} outPath: {} - dependentRealisations: {} signatures: {} -additionalProperties: false +additionalProperties: + dependentRealisations: + description: deprecated field + type: object "$defs": key: @@ -60,7 +69,6 @@ additionalProperties: false type: object required: - outPath - - dependentRealisations - signatures properties: outPath: @@ -69,19 +77,6 @@ additionalProperties: false description: | The path to the store object that resulted from building this derivation for the given output name. - dependentRealisations: - type: object - title: Underlying Base Build Trace - description: | - This is for [*derived*](@docroot@/store/build-trace.md#derived) build trace entries to ensure coherence. - - Keys are derivation output IDs (same format as the main `id` field). - Values are the store paths that those dependencies resolved to. - - As described in the linked section on derived build trace traces, derived build trace entries must be kept in addition and not instead of the underlying base build entries. - This is the set of base build trace entries that this derived build trace is derived from. - (The set is also a map since this miniature base build trace must be coherent, mapping each key to a single value.) - patternProperties: "^sha256:[0-9a-f]{64}![a-zA-Z_][a-zA-Z0-9_-]*$": "$ref": "store-path-v1.yaml" diff --git a/doc/manual/source/protocols/json/schema/store-v1.yaml b/doc/manual/source/protocols/json/schema/store-v1.yaml index e0c6f8fed6c..ebe61d9cb22 100644 --- a/doc/manual/source/protocols/json/schema/store-v1.yaml +++ b/doc/manual/source/protocols/json/schema/store-v1.yaml @@ -70,7 +70,7 @@ properties: "^[A-Za-z0-9+/]{43}=$": type: object additionalProperties: - "$ref": "./build-trace-entry-v1.yaml#/$defs/value" + "$ref": "./build-trace-entry-v2.yaml#/$defs/value" additionalProperties: false "$defs": diff --git a/src/json-schema-checks/meson.build b/src/json-schema-checks/meson.build index 42c9132bd81..0be4c1372cb 100644 --- a/src/json-schema-checks/meson.build +++ b/src/json-schema-checks/meson.build @@ -62,9 +62,11 @@ schemas = [ }, { 'stem' : 'build-trace-entry', - 'schema' : schema_dir / 'build-trace-entry-v1.yaml', + 'schema' : schema_dir / 'build-trace-entry-v2.yaml', 'files' : [ 'simple.json', + # The field is no longer supported, but we want to show that we + # ignore it during parsing. 'with-dependent-realisations.json', 'with-signature.json', ], diff --git a/src/libstore-test-support/include/nix/store/tests/protocol.hh b/src/libstore-test-support/include/nix/store/tests/protocol.hh index 0f774df0ec0..f65f12c533d 100644 --- a/src/libstore-test-support/include/nix/store/tests/protocol.hh +++ b/src/libstore-test-support/include/nix/store/tests/protocol.hh @@ -88,25 +88,38 @@ public: } }; -#define VERSIONED_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - TEST_F(FIXTURE, NAME##_read) \ - { \ - readProtoTest(STEM, VERSION, VALUE); \ - } \ - TEST_F(FIXTURE, NAME##_write) \ - { \ - writeProtoTest(STEM, VERSION, VALUE); \ +#define VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + TEST_F(FIXTURE, NAME##_read) \ + { \ + readProtoTest(STEM, VERSION, VALUE); \ } -#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ - VERSIONED_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ - TEST_F(FIXTURE, NAME##_json_read) \ - { \ - readJsonTest(STEM, VALUE); \ - } \ - TEST_F(FIXTURE, NAME##_json_write) \ - { \ - writeJsonTest(STEM, VALUE); \ +#define VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + TEST_F(FIXTURE, NAME##_write) \ + { \ + writeProtoTest(STEM, VERSION, VALUE); \ } +#define VERSIONED_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) + +#define VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + TEST_F(FIXTURE, NAME##_json_read) \ + { \ + readJsonTest(STEM, VALUE); \ + } + +#define VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST_NO_JSON(FIXTURE, NAME, STEM, VERSION, VALUE) \ + TEST_F(FIXTURE, NAME##_json_write) \ + { \ + writeJsonTest(STEM, VALUE); \ + } + +#define VERSIONED_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_READ_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) \ + VERSIONED_WRITE_CHARACTERIZATION_TEST(FIXTURE, NAME, STEM, VERSION, VALUE) + } // namespace nix diff --git a/src/libstore-tests/common-protocol.cc b/src/libstore-tests/common-protocol.cc index fa676eb7f4e..6bd7beb4455 100644 --- a/src/libstore-tests/common-protocol.cc +++ b/src/libstore-tests/common-protocol.cc @@ -47,24 +47,30 @@ class CommonProtoTest : public ProtoTest } }; -#define CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ - TEST_F(CommonProtoTest, NAME##_read) \ - { \ - readProtoTest(STEM, VALUE); \ - } \ - TEST_F(CommonProtoTest, NAME##_write) \ - { \ - writeProtoTest(STEM, VALUE); \ - } \ - TEST_F(CommonProtoTest, NAME##_json_read) \ - { \ - readJsonTest(STEM, VALUE); \ - } \ - TEST_F(CommonProtoTest, NAME##_json_write) \ - { \ - writeJsonTest(STEM, VALUE); \ +#define READ_CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ + TEST_F(CommonProtoTest, NAME##_read) \ + { \ + readProtoTest(STEM, VALUE); \ + } \ + TEST_F(CommonProtoTest, NAME##_json_read) \ + { \ + readJsonTest(STEM, VALUE); \ } +#define WRITE_CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ + TEST_F(CommonProtoTest, NAME##_write) \ + { \ + writeProtoTest(STEM, VALUE); \ + } \ + TEST_F(CommonProtoTest, NAME##_json_write) \ + { \ + writeJsonTest(STEM, VALUE); \ + } + +#define CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ + READ_CHARACTERIZATION_TEST(NAME, STEM, VALUE) \ + WRITE_CHARACTERIZATION_TEST(NAME, STEM, VALUE) + CHARACTERIZATION_TEST( string, "string", @@ -141,7 +147,7 @@ CHARACTERIZATION_TEST( }, })) -CHARACTERIZATION_TEST( +READ_CHARACTERIZATION_TEST( realisation_with_deps, "realisation-with-deps", (std::tuple{ @@ -149,16 +155,6 @@ CHARACTERIZATION_TEST( { .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, .signatures = {"asdf", "qwer"}, - .dependentRealisations = - { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - }, }, { .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), diff --git a/src/libstore-tests/realisation.cc b/src/libstore-tests/realisation.cc index d16049bc5b0..66f4707e9ac 100644 --- a/src/libstore-tests/realisation.cc +++ b/src/libstore-tests/realisation.cc @@ -44,54 +44,45 @@ 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, - ([] { - Realisation simple{ - { - .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, - }, - { - .drvHash = Hash::parseExplicitFormatUnprefixed( - "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", - HashAlgorithm::SHA256, - HashFormat::Base16), - .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; - }()}, - std::pair{ - "with-dependent-realisations", - [&] { - auto r = simple; - r.dependentRealisations = {{ - { - .drvHash = Hash::parseExplicitFormatUnprefixed( - "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad", - HashAlgorithm::SHA256, - HashFormat::Base16), - .outputName = "foo", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv"}, - }}; - return r; - }(), - }); - } + ::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); +} } // namespace nix diff --git a/src/libstore-tests/serve-protocol.cc b/src/libstore-tests/serve-protocol.cc index e04d89e3ddb..d138330cf8d 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -116,7 +116,7 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) -VERSIONED_CHARACTERIZATION_TEST( +VERSIONED_READ_CHARACTERIZATION_TEST( ServeProtoTest, realisation_with_deps, "realisation-with-deps", @@ -126,16 +126,6 @@ VERSIONED_CHARACTERIZATION_TEST( { .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, .signatures = {"asdf", "qwer"}, - .dependentRealisations = - { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - }, }, { .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), diff --git a/src/libstore-tests/worker-protocol.cc b/src/libstore-tests/worker-protocol.cc index 1e0ede81ccd..04cada02979 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -169,7 +169,7 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) -VERSIONED_CHARACTERIZATION_TEST( +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, realisation_with_deps, "realisation-with-deps", @@ -179,16 +179,6 @@ VERSIONED_CHARACTERIZATION_TEST( { .outPath = StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, .signatures = {"asdf", "qwer"}, - .dependentRealisations = - { - { - DrvOutput{ - .drvHash = Hash::parseSRI("sha256-b4afnqKCO9oWXgYHb9DeQ2berSwOjS27rSd9TxXDc/U="), - .outputName = "quux", - }, - StorePath{"g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo"}, - }, - }, }, { .drvHash = Hash::parseSRI("sha256-FePFYIlMuycIXPZbWi7LGEiMmZSX9FMbaQenWBzm1Sc="), diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b36685a242c..bdbca5f00ae 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1,4 +1,5 @@ #include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/derivation-building-goal.hh" #include "nix/store/build/derivation-resolution-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows @@ -100,9 +101,24 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) through substitutes. If that doesn't work, we'll build them. */ if (settings.useSubstitutes && drvOptions.substitutesAllowed()) { - if (!checkResult) - waitees.insert(upcast_goal(worker.makeDrvOutputSubstitutionGoal(DrvOutput{outputHash, wantedOutput}))); - else { + if (!checkResult) { + DrvOutput id{outputHash, wantedOutput}; + auto g = worker.makeDrvOutputSubstitutionGoal(id); + waitees.insert(g); + co_await await(std::move(waitees)); + + if (nrFailed == 0) { + waitees.insert(upcast_goal(worker.makePathSubstitutionGoal(g->outputInfo->outPath))); + co_await await(std::move(waitees)); + + trace("output path substituted"); + + if (nrFailed == 0) + worker.store.registerDrvOutput({*g->outputInfo, id}); + else + debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); + } + } else { auto * cap = getDerivationCA(*drv); waitees.insert(upcast_goal(worker.makePathSubstitutionGoal( checkResult->first.outPath, @@ -210,11 +226,6 @@ Goal::Co DerivationGoal::haveDerivation(bool storeDerivation) .outputName = wantedOutput, }}; newRealisation.signatures.clear(); - if (!drv->type().isFixed()) { - auto & drvStore = worker.evalStore.isValidPath(drvPath) ? worker.evalStore : worker.store; - newRealisation.dependentRealisations = - drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore); - } worker.store.signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 8d0a307beda..998412d1e7b 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -21,7 +21,7 @@ Goal::Co DrvOutputSubstitutionGoal::init() trace("init"); /* If the derivation already exists, we’re done */ - if (worker.store.queryRealisation(id)) { + if ((outputInfo = worker.store.queryRealisation(id))) { co_return amDone(ecSuccess); } @@ -70,12 +70,6 @@ Goal::Co DrvOutputSubstitutionGoal::init() worker.childTerminated(this); - /* - * The realisation corresponding to the given output id. - * Will be filled once we can get it. - */ - std::shared_ptr outputInfo; - try { outputInfo = promise->get_future().get(); } catch (std::exception & e) { @@ -86,45 +80,6 @@ Goal::Co DrvOutputSubstitutionGoal::init() if (!outputInfo) continue; - bool failed = false; - - Goals waitees; - - for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { - if (depId != id) { - if (auto localOutputInfo = worker.store.queryRealisation(depId); - localOutputInfo && localOutputInfo->outPath != depPath) { - warn( - "substituter '%s' has an incompatible realisation for '%s', ignoring.\n" - "Local: %s\n" - "Remote: %s", - sub->config.getHumanReadableURI(), - depId.to_string(), - worker.store.printStorePath(localOutputInfo->outPath), - worker.store.printStorePath(depPath)); - failed = true; - break; - } - waitees.insert(worker.makeDrvOutputSubstitutionGoal(depId)); - } - } - - if (failed) - continue; - - waitees.insert(worker.makePathSubstitutionGoal(outputInfo->outPath)); - - co_await await(std::move(waitees)); - - trace("output path substituted"); - - if (nrFailed > 0) { - debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); - co_return amDone(nrNoSubstituters > 0 ? ecNoSubstituters : ecFailed); - } - - worker.store.registerDrvOutput({*outputInfo, id}); - trace("finished"); co_return amDone(ecSuccess); } diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql index c5e4e389799..d563b33d879 100644 --- a/src/libstore/ca-specific-schema.sql +++ b/src/libstore/ca-specific-schema.sql @@ -12,30 +12,3 @@ create table if not exists Realisations ( ); create index if not exists IndexRealisations on Realisations(drvPath, outputName); - --- We can end-up in a weird edge-case where a path depends on itself because --- it’s an output of a CA derivation, that happens to be the same as one of its --- dependencies. --- In that case we have a dependency loop (path -> realisation1 -> realisation2 --- -> path) that we need to break by removing the dependencies between the --- realisations -create trigger if not exists DeleteSelfRefsViaRealisations before delete on ValidPaths - begin - delete from RealisationsRefs where realisationReference in ( - select id from Realisations where outputPath = old.id - ); - end; - -create table if not exists RealisationsRefs ( - referrer integer not null, - realisationReference integer, - foreign key (referrer) references Realisations(id) on delete cascade, - foreign key (realisationReference) references Realisations(id) on delete restrict -); --- used by deletion trigger -create index if not exists IndexRealisationsRefsRealisationReference on RealisationsRefs(realisationReference); - --- used by QueryRealisationReferences -create index if not exists IndexRealisationsRefs on RealisationsRefs(referrer); --- used by cascade deletion when ValidPaths is deleted -create index if not exists IndexRealisationsRefsOnOutputPath on Realisations(outputPath); diff --git a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh index 6310e0d2ccc..88f4d6e55e6 100644 --- a/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh +++ b/src/libstore/include/nix/store/build/drv-output-substitution-goal.hh @@ -14,11 +14,11 @@ namespace nix { class Worker; /** - * Substitution of a derivation output. - * This is done in three steps: - * 1. Fetch the output info from a substituter - * 2. Substitute the corresponding output path - * 3. Register the output info + * Fetch a `Realisation` (drv ⨯ output name -> output path) from a + * substituter. + * + * If the output store object itself should also be substituted, that is + * the responsibility of the caller to do so. */ class DrvOutputSubstitutionGoal : public Goal { @@ -31,6 +31,12 @@ class DrvOutputSubstitutionGoal : public Goal public: DrvOutputSubstitutionGoal(const DrvOutput & id, Worker & worker); + /** + * The realisation corresponding to the given output id. + * Will be filled once we can get it. + */ + std::shared_ptr outputInfo; + Co init(); void timedOut(Error && ex) override diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index af0e4aefd8a..3691c14fb79 100644 --- a/src/libstore/include/nix/store/realisation.hh +++ b/src/libstore/include/nix/store/realisation.hh @@ -56,14 +56,6 @@ struct UnkeyedRealisation StringSet signatures; - /** - * The realisations that are required for the current one to be valid. - * - * When importing this realisation, the store will first check that all its - * dependencies exist, and map to the correct output path - */ - std::map dependentRealisations; - std::string fingerprint(const DrvOutput & key) const; void sign(const DrvOutput & key, const Signer &); @@ -87,10 +79,6 @@ struct Realisation : UnkeyedRealisation bool isCompatibleWith(const UnkeyedRealisation & other) const; - static std::set closure(Store &, const std::set &); - - static void closure(Store &, const std::set &, std::set & res); - bool operator==(const Realisation &) const = default; auto operator<=>(const Realisation &) const = default; }; @@ -154,10 +142,6 @@ struct RealisedPath */ const StorePath & path() const &; - void closure(Store & store, Set & ret) const; - static void closure(Store & store, const Set & startPaths, Set & ret); - Set closure(Store & store) const; - bool operator==(const RealisedPath &) const = default; auto operator<=>(const RealisedPath &) const = default; }; diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index db107fc0ce7..3261a553e21 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -1007,9 +1007,6 @@ decodeValidPathInfo(const Store & store, std::istream & str, std::optional -drvOutputReferences(Store & store, const Derivation & drv, const StorePath & outputPath, Store * evalStore = nullptr); - template<> struct json_avoids_null : std::true_type {}; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 51392f014f0..1382f577eba 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -390,21 +390,6 @@ LocalStore::LocalStore(ref config) where drvPath = ? ; )"); - state->stmts->QueryRealisationReferences.create( - state->db, - R"( - select drvPath, outputName from Realisations - join RealisationsRefs on realisationReference = Realisations.id - where referrer = ?; - )"); - state->stmts->AddRealisationReference.create( - state->db, - R"( - insert or replace into RealisationsRefs (referrer, realisationReference) - values ( - (select id from Realisations where drvPath = ? and outputName = ?), - (select id from Realisations where drvPath = ? and outputName = ?)); - )"); } } @@ -654,25 +639,6 @@ void LocalStore::registerDrvOutput(const Realisation & info) concatStringsSep(" ", info.signatures)) .exec(); } - for (auto & [outputId, depPath] : info.dependentRealisations) { - auto localRealisation = queryRealisationCore_(*state, outputId); - if (!localRealisation) - throw Error( - "unable to register the derivation '%s' as it " - "depends on the non existent '%s'", - info.id.to_string(), - outputId.to_string()); - if (localRealisation->second.outPath != depPath) - throw Error( - "unable to register the derivation '%s' as it " - "depends on a realisation of '%s' that doesn’t" - "match what we have locally", - info.id.to_string(), - outputId.to_string()); - state->stmts->AddRealisationReference - .use()(info.id.strHash())(info.id.outputName)(outputId.strHash())(outputId.outputName) - .exec(); - } }); } @@ -1606,21 +1572,6 @@ std::optional LocalStore::queryRealisation_(LocalStore return std::nullopt; auto [realisationDbId, res] = *maybeCore; - std::map dependentRealisations; - auto useRealisationRefs(state.stmts->QueryRealisationReferences.use()(realisationDbId)); - while (useRealisationRefs.next()) { - auto depId = DrvOutput{ - Hash::parseAnyPrefixed(useRealisationRefs.getStr(0)), - useRealisationRefs.getStr(1), - }; - auto dependentRealisation = queryRealisationCore_(state, depId); - assert(dependentRealisation); // Enforced by the db schema - auto outputPath = dependentRealisation->second.outPath; - dependentRealisations.insert({depId, outputPath}); - } - - res.dependentRealisations = dependentRealisations; - return {res}; } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index f9a339a0057..79f9a1f6fee 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -334,65 +334,6 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) result); } -std::map -drvOutputReferences(const std::set & inputRealisations, const StorePathSet & pathReferences) -{ - std::map res; - - for (const auto & input : inputRealisations) { - if (pathReferences.count(input.outPath)) { - res.insert({input.id, input.outPath}); - } - } - - return res; -} - -std::map -drvOutputReferences(Store & store, const Derivation & drv, const StorePath & outputPath, Store * evalStore_) -{ - auto & evalStore = evalStore_ ? *evalStore_ : store; - - std::set inputRealisations; - - auto accumRealisations = [&](this auto & self, - const StorePath & inputDrv, - const DerivedPathMap::ChildNode & inputNode) -> void { - if (!inputNode.value.empty()) { - auto outputHashes = staticOutputHashes(evalStore, evalStore.readDerivation(inputDrv)); - for (const auto & outputName : inputNode.value) { - auto outputHash = get(outputHashes, outputName); - if (!outputHash) - throw Error( - "output '%s' of derivation '%s' isn't realised", outputName, store.printStorePath(inputDrv)); - DrvOutput key{*outputHash, outputName}; - auto thisRealisation = store.queryRealisation(key); - if (!thisRealisation) - throw Error( - "output '%s' of derivation '%s' isn’t built", outputName, store.printStorePath(inputDrv)); - inputRealisations.insert({*thisRealisation, std::move(key)}); - } - } - if (!inputNode.value.empty()) { - auto d = makeConstantStorePathRef(inputDrv); - for (const auto & [outputName, childNode] : inputNode.childMap) { - SingleDerivedPath next = SingleDerivedPath::Built{d, outputName}; - self( - // TODO deep resolutions for dynamic derivations, issue #8947, would go here. - resolveDerivedPath(store, next, evalStore_), - childNode); - } - } - }; - - for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) - accumRealisations(inputDrv, inputNode); - - auto info = store.queryPathInfo(outputPath); - - return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); -} - OutputPathMap resolveDerivedPath(Store & store, const DerivedPath::Built & bfd, Store * evalStore_) { auto drvPath = resolveDerivedPath(store, *bfd.drvPath, evalStore_); diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 4aeb05874fb..2075b39e6fb 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -1,6 +1,5 @@ #include "nix/store/realisation.hh" #include "nix/store/store-api.hh" -#include "nix/util/closure.hh" #include "nix/util/signature/local-keys.hh" #include "nix/util/json-utils.hh" #include @@ -26,41 +25,6 @@ std::string DrvOutput::to_string() const return strHash() + "!" + outputName; } -std::set Realisation::closure(Store & store, const std::set & startOutputs) -{ - std::set res; - Realisation::closure(store, startOutputs, res); - return res; -} - -void Realisation::closure(Store & store, const std::set & startOutputs, std::set & res) -{ - auto getDeps = [&](const Realisation & current) -> std::set { - std::set res; - for (auto & [currentDep, _] : current.dependentRealisations) { - if (auto currentRealisation = store.queryRealisation(currentDep)) - res.insert({*currentRealisation, currentDep}); - else - throw Error("Unrealised derivation '%s'", currentDep.to_string()); - } - return res; - }; - - computeClosure( - startOutputs, - res, - [&](const Realisation & current, std::function> &)> processEdges) { - std::promise> promise; - try { - auto res = getDeps(current); - promise.set_value(res); - } catch (...) { - promise.set_exception(std::current_exception()); - } - return processEdges(promise); - }); -} - std::string UnkeyedRealisation::fingerprint(const DrvOutput & key) const { nlohmann::json serialized = Realisation{*this, key}; @@ -99,43 +63,7 @@ const StorePath & RealisedPath::path() const & bool Realisation::isCompatibleWith(const UnkeyedRealisation & other) const { - if (outPath == other.outPath) { - if (dependentRealisations.empty() != other.dependentRealisations.empty()) { - warn( - "Encountered a realisation for '%s' with an empty set of " - "dependencies. This is likely an artifact from an older Nix. " - "I’ll try to fix the realisation if I can", - id.to_string()); - return true; - } else if (dependentRealisations == other.dependentRealisations) { - return true; - } - } - return false; -} - -void RealisedPath::closure(Store & store, const RealisedPath::Set & startPaths, RealisedPath::Set & ret) -{ - // FIXME: This only builds the store-path closure, not the real realisation - // closure - StorePathSet initialStorePaths, pathsClosure; - for (auto & path : startPaths) - initialStorePaths.insert(path.path()); - store.computeFSClosure(initialStorePaths, pathsClosure); - ret.insert(startPaths.begin(), startPaths.end()); - ret.insert(pathsClosure.begin(), pathsClosure.end()); -} - -void RealisedPath::closure(Store & store, RealisedPath::Set & ret) const -{ - RealisedPath::closure(store, {*this}, ret); -} - -RealisedPath::Set RealisedPath::closure(Store & store) const -{ - RealisedPath::Set ret; - closure(store, ret); - return ret; + return outPath == other.outPath; } } // namespace nix @@ -162,27 +90,19 @@ UnkeyedRealisation adl_serializer::from_json(const json & js if (auto signaturesOpt = optionalValueAt(json, "signatures")) signatures = *signaturesOpt; - std::map dependentRealisations; - if (auto jsonDependencies = optionalValueAt(json, "dependentRealisations")) - for (auto & [jsonDepId, jsonDepOutPath] : getObject(*jsonDependencies)) - dependentRealisations.insert({DrvOutput::parse(jsonDepId), jsonDepOutPath}); - return UnkeyedRealisation{ .outPath = valueAt(json, "outPath"), .signatures = signatures, - .dependentRealisations = dependentRealisations, }; } void adl_serializer::to_json(json & json, const UnkeyedRealisation & r) { - auto jsonDependentRealisations = nlohmann::json::object(); - for (auto & [depId, depOutPath] : r.dependentRealisations) - jsonDependentRealisations.emplace(depId.to_string(), depOutPath); json = { {"outPath", r.outPath}, {"signatures", r.signatures}, - {"dependentRealisations", jsonDependentRealisations}, + // back-compat + {"dependentRealisations", json::object()}, }; } diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index ef8aaa3801d..b4d696a536b 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -292,7 +292,7 @@ std::vector RestrictedStore::buildPathsWithResults( next->computeFSClosure(newPaths, closure); for (auto & path : closure) goal.addDependency(path); - for (auto & real : Realisation::closure(*next, newRealisations)) + for (auto & real : newRealisations) goal.addedDrvOutputs.insert(real.id); return results; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 921507add6c..f23f0ebfd31 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -915,36 +915,21 @@ std::map copyPaths( SubstituteFlag substitute) { StorePathSet storePaths; - std::set toplevelRealisations; + std::vector realisations; for (auto & path : paths) { storePaths.insert(path.path()); if (auto * realisation = std::get_if(&path.raw)) { experimentalFeatureSettings.require(Xp::CaDerivations); - toplevelRealisations.insert(*realisation); + realisations.push_back(realisation); } } auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); try { - // Copy the realisation closure - processGraph( - Realisation::closure(srcStore, toplevelRealisations), - [&](const Realisation & current) -> std::set { - std::set children; - for (const auto & [drvOutput, _] : current.dependentRealisations) { - auto currentChild = srcStore.queryRealisation(drvOutput); - if (!currentChild) - throw Error( - "incomplete realisation closure: '%s' is a " - "dependency of '%s' but isn't registered", - drvOutput.to_string(), - current.id.to_string()); - children.insert({*currentChild, drvOutput}); - } - return children; - }, - [&](const Realisation & current) -> void { dstStore.registerDrvOutput(current, checkSigs); }); + // Copy the realisations. TODO batch this + for (const auto * realisation : realisations) + dstStore.registerDrvOutput(*realisation, checkSigs); } catch (MissingExperimentalFeature & e) { // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want @@ -1055,8 +1040,19 @@ void copyClosure( if (&srcStore == &dstStore) return; - RealisedPath::Set closure; - RealisedPath::closure(srcStore, paths, closure); + StorePathSet closure0; + for (auto & path : paths) { + if (auto * opaquePath = std::get_if(&path.raw)) { + closure0.insert(opaquePath->path); + } + } + + StorePathSet closure1; + srcStore.computeFSClosure(closure0, closure1); + + RealisedPath::Set closure = paths; + for (auto && path : closure1) + closure.insert({std::move(path)}); copyPaths(srcStore, dstStore, closure, repair, checkSigs, substitute); } diff --git a/tests/functional/ca/duplicate-realisation-in-closure.sh b/tests/functional/ca/duplicate-realisation-in-closure.sh index 4a5e8c042b7..032fb6164d1 100644 --- a/tests/functional/ca/duplicate-realisation-in-closure.sh +++ b/tests/functional/ca/duplicate-realisation-in-closure.sh @@ -25,4 +25,9 @@ nix build -f nondeterministic.nix dep2 --no-link # If everything goes right, we should rebuild dep2 rather than fetch it from # the cache (because that would mean duplicating `current-time` in the closure), # and have `dep1 == dep2`. + +# FIXME: Force the use of small-step resolutions only to fix this in a +# better way (#11896, #11928). +skipTest "temporarily broken because dependent realisations are removed" + nix build --substituters "$REMOTE_STORE" -f nondeterministic.nix toplevel --no-require-sigs --no-link diff --git a/tests/functional/ca/substitute.sh b/tests/functional/ca/substitute.sh index 9728470f0b8..2f6ebcef5c1 100644 --- a/tests/functional/ca/substitute.sh +++ b/tests/functional/ca/substitute.sh @@ -22,7 +22,10 @@ nix copy --to "$REMOTE_STORE" --file ./content-addressed.nix # Restart the build on an empty store, ensuring that we don't build clearStore -buildDrvs --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 transitivelyDependentCA +# FIXME: `dependentCA` should not need to be explicitly mentioned in +# this. Force the use of small-step resolutions only to allow not +# mentioning it explicitly again. (#11896, #11928). +buildDrvs --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 transitivelyDependentCA dependentCA # Check that the thing we’ve just substituted has its realisation stored nix realisation info --file ./content-addressed.nix transitivelyDependentCA # Check that its dependencies have it too