From 4a5d960952ac1d87690e5282ed60d09f1b2a5841 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 13 Oct 2025 18:35:19 -0400 Subject: [PATCH] Remove dependent realisations This progress on #11896. It introduces some issues temporarily which will be fixed when #11928 is fixed. The SQL tables are left in place because there is no point inducing a migration now, when we will be immediately landing more changes after this that also require schema changes. They will simply be ignored by in this commit, and so all data will be preserved. --- .../protocols/json/build-trace-entry.md | 16 ++-- doc/manual/source/protocols/json/meson.build | 2 +- .../json/schema/build-result-v1.yaml | 2 +- ...ld-trace-entry-v1 => build-trace-entry-v2} | 0 ...ntry-v1.yaml => build-trace-entry-v2.yaml} | 31 +++---- .../protocols/json/schema/store-v1.yaml | 2 +- src/json-schema-checks/meson.build | 4 +- .../include/nix/store/tests/protocol.hh | 47 ++++++---- src/libstore-tests/common-protocol.cc | 50 +++++------ .../issue-11928/store-after.json | 33 +------ .../issue-11928/substituter.json | 4 +- src/libstore-tests/realisation.cc | 81 ++++++++--------- src/libstore-tests/serve-protocol.cc | 12 +-- src/libstore-tests/worker-protocol.cc | 12 +-- src/libstore-tests/worker-substitution.cc | 17 ++-- src/libstore/build/derivation-goal.cc | 5 -- .../build/drv-output-substitution-goal.cc | 24 ------ src/libstore/include/nix/store/realisation.hh | 16 ---- src/libstore/include/nix/store/store-api.hh | 3 - src/libstore/local-store.cc | 51 ----------- src/libstore/misc.cc | 59 ------------- src/libstore/realisation.cc | 86 +------------------ src/libstore/restricted-store.cc | 2 +- src/libstore/store-api.cc | 40 ++++----- .../ca/duplicate-realisation-in-closure.sh | 5 ++ tests/functional/ca/substitute.sh | 5 +- 26 files changed, 157 insertions(+), 452 deletions(-) rename doc/manual/source/protocols/json/schema/{build-trace-entry-v1 => build-trace-entry-v2} (100%) rename doc/manual/source/protocols/json/schema/{build-trace-entry-v1.yaml => build-trace-entry-v2.yaml} (74%) diff --git a/doc/manual/source/protocols/json/build-trace-entry.md b/doc/manual/source/protocols/json/build-trace-entry.md index 8050a2840..9eea93712 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 e32cf0640..c7c48c4ed 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 31f59a44d..55e55d3e5 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 a85738b50..4340f8238 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 e0c6f8fed..ebe61d9cb 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 20acfe411..a1b525bc5 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 0f774df0e..f65f12c53 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 fa676eb7f..6bd7beb44 100644 --- a/src/libstore-tests/common-protocol.cc +++ b/src/libstore-tests/common-protocol.cc @@ -47,24 +47,30 @@ public: } }; -#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/data/worker-substitution/issue-11928/store-after.json b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json index 0d2cd439c..617984a27 100644 --- a/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json +++ b/src/libstore-tests/data/worker-substitution/issue-11928/store-after.json @@ -1,18 +1,9 @@ { "buildTrace": { "8vEkprm3vQ3BE6JLB8XKfU+AdAwEFOMI/skzyj3pr5I=": { - "out": { - "dependentRealisations": { - "sha256:82746e2bec1f6d7a913f380ee4cca205e6d7ad5d742b3bfeb6464212e3e6ee96!out": "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out" - }, - "outPath": "px7apdw6ydm9ynjy5g0bpdcylw3xz2kj-root-drv-out", - "signatures": [] - } - }, - "gnRuK+wfbXqRPzgO5MyiBebXrV10Kzv+tkZCEuPm7pY=": { "out": { "dependentRealisations": {}, - "outPath": "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out", + "outPath": "px7apdw6ydm9ynjy5g0bpdcylw3xz2kj-root-drv-out", "signatures": [] } } @@ -42,28 +33,6 @@ "ultimate": false, "version": 2 } - }, - "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out": { - "contents": { - "contents": "I am the dependency output", - "executable": false, - "type": "regular" - }, - "info": { - "ca": { - "hash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", - "method": "nar" - }, - "deriver": null, - "narHash": "sha256-HK2LBzSTtwuRjc44PH3Ac1JHHPKmfnAgNxz6I5mVgL8=", - "narSize": 144, - "references": [], - "registrationTime": null, - "signatures": [], - "storeDir": "/nix/store", - "ultimate": false, - "version": 2 - } } }, "derivations": { diff --git a/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json b/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json index c3530b8d5..a63d6243f 100644 --- a/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json +++ b/src/libstore-tests/data/worker-substitution/issue-11928/substituter.json @@ -2,9 +2,7 @@ "buildTrace": { "8vEkprm3vQ3BE6JLB8XKfU+AdAwEFOMI/skzyj3pr5I=": { "out": { - "dependentRealisations": { - "sha256:82746e2bec1f6d7a913f380ee4cca205e6d7ad5d742b3bfeb6464212e3e6ee96!out": "w0yjpwh59kpbyc7hz9jgmi44r9br908i-dep-drv-out" - }, + "dependentRealisations": {}, "outPath": "px7apdw6ydm9ynjy5g0bpdcylw3xz2kj-root-drv-out", "signatures": [] } diff --git a/src/libstore-tests/realisation.cc b/src/libstore-tests/realisation.cc index d16049bc5..66f4707e9 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 258dbf049..be82ca347 100644 --- a/src/libstore-tests/serve-protocol.cc +++ b/src/libstore-tests/serve-protocol.cc @@ -118,7 +118,7 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) -VERSIONED_CHARACTERIZATION_TEST( +VERSIONED_READ_CHARACTERIZATION_TEST( ServeProtoTest, realisation_with_deps, "realisation-with-deps", @@ -128,16 +128,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 7416d7323..74e8b6b65 100644 --- a/src/libstore-tests/worker-protocol.cc +++ b/src/libstore-tests/worker-protocol.cc @@ -171,7 +171,7 @@ VERSIONED_CHARACTERIZATION_TEST( }, })) -VERSIONED_CHARACTERIZATION_TEST( +VERSIONED_READ_CHARACTERIZATION_TEST( WorkerProtoTest, realisation_with_deps, "realisation-with-deps", @@ -181,16 +181,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-substitution.cc b/src/libstore-tests/worker-substitution.cc index 016f6207b..316165849 100644 --- a/src/libstore-tests/worker-substitution.cc +++ b/src/libstore-tests/worker-substitution.cc @@ -382,7 +382,6 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) DrvOutput rootDrvOutput{rootDrvHash, "out"}; // Add the realisation for the root derivation to the substituter - // Include the dependency realisation in dependentRealisations substituter->buildTrace.insert_or_assign( rootDrvHash, std::map{ @@ -390,7 +389,6 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) "out", UnkeyedRealisation{ .outPath = rootOutputPath, - .dependentRealisations = {{depDrvOutput, depOutputPath}}, }, }, }); @@ -430,14 +428,17 @@ TEST_F(WorkerSubstitutionTest, floatingDerivationOutputWithDepDrv) ASSERT_TRUE(rootRealisation); ASSERT_EQ(rootRealisation->outPath, rootOutputPath); - // The dependency's REALISATION should have been fetched + // #11928: The dependency's REALISATION should be fetched, because + // it is needed to resolve the underlying derivation. Currently the + // realisation is not fetched (bug). Once fixed: Change + // depRealisation ASSERT_FALSE to ASSERT_TRUE and uncomment the + // ASSERT_EQ auto depRealisation = dummyStore->queryRealisation(depDrvOutput); - ASSERT_TRUE(depRealisation); - ASSERT_EQ(depRealisation->outPath, depOutputPath); + ASSERT_FALSE(depRealisation); + // ASSERT_EQ(depRealisation->outPath, depOutputPath); - // TODO #11928: The dependency's OUTPUT should NOT be fetched (not referenced - // by root output). Once #11928 is fixed, change ASSERT_TRUE to ASSERT_FALSE. - ASSERT_TRUE(dummyStore->isValidPath(depOutputPath)); + // The dependency's OUTPUT is correctly not fetched (not referenced by root output) + ASSERT_FALSE(dummyStore->isValidPath(depOutputPath)); // Verify the goal succeeded ASSERT_EQ(upcast_goal(goal)->exitCode, Goal::ecSuccess); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b36685a24..13ffe51f3 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -210,11 +210,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 171aa900a..59a08b8a6 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -93,32 +93,8 @@ 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)); diff --git a/src/libstore/include/nix/store/realisation.hh b/src/libstore/include/nix/store/realisation.hh index af0e4aefd..3691c14fb 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 db107fc0c..3261a553e 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 b625b6c1b..c5fd85fff 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) @@ -390,21 +388,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 +637,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 +1570,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 f9a339a00..79f9a1f6f 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 4aeb05874..2075b39e6 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 ef8aaa380..b4d696a53 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 dc5be065d..660370d1b 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 4a5e8c042..032fb6164 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 9728470f0..2f6ebcef5 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