From 7103c6da705c8a18fef9e8f8d404e8d0ab5082ff Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 19 Apr 2023 14:48:53 -0400 Subject: [PATCH] Remove references from fixed output derivation ab syntax In other words, use a plain `ContentAddress` not `ContentAddressWithReferences` for `DerivationOutput::CAFixed`. Supporting fixed output derivations with (fixed) references would be a cool feature, but it is out of scope at this moment. --- src/libexpr/primops.cc | 6 ++--- src/libstore/build/derivation-goal.cc | 6 +++-- src/libstore/content-address.cc | 10 ++++----- src/libstore/content-address.hh | 4 ++-- src/libstore/derivations.cc | 13 +++++------ src/libstore/derivations.hh | 6 +++-- src/libstore/misc.cc | 28 ++++++++++------------- src/libstore/store-api.hh | 2 +- src/libstore/tests/derivation.cc | 32 +++++++++++++++++---------- 9 files changed, 55 insertions(+), 52 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index fc397db33..4f93f3d98 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1303,11 +1303,9 @@ drvName, Bindings * attrs, Value & v) auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); DerivationOutput::CAFixed dof { - .ca = ContentAddressWithReferences::fromParts( + .ca = ContentAddress::fromParts( std::move(method), - std::move(h), - // FIXME non-trivial fixed refs set - {}), + std::move(h)), }; drv.env["out"] = state.store->printStorePath(dof.path(*state.store, drvName, "out")); diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a4bb94b0e..af153ebfb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -274,11 +274,13 @@ void DerivationGoal::haveDerivation() ) ) ); - else + else { + auto * cap = getDerivationCA(*drv); addWaitee(upcast_goal(worker.makePathSubstitutionGoal( status.known->path, buildMode == bmRepair ? Repair : NoRepair, - getDerivationCA(*drv)))); + cap ? std::optional { *cap } : std::nullopt))); + } } if (waitees.empty()) /* to prevent hang (no wake-up event) */ diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 8a65059e3..167f7af6c 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -196,6 +196,11 @@ const Hash & ContentAddress::getHash() const }, raw); } +std::string ContentAddress::printMethodAlgo() const { + return getMethod().renderPrefix() + + printHashType(getHash().type); +} + bool StoreReferences::empty() const { return !self && others.empty(); @@ -271,9 +276,4 @@ Hash ContentAddressWithReferences::getHash() const }, raw); } -std::string ContentAddressWithReferences::printMethodAlgo() const { - return getMethod().renderPrefix() - + printHashType(getHash().type); -} - } diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index eb01e9ce4..b25e6d49d 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -177,6 +177,8 @@ struct ContentAddress ContentAddressMethod getMethod() const; const Hash & getHash() const; + + std::string printMethodAlgo() const; }; std::string renderContentAddress(std::optional ca); @@ -286,8 +288,6 @@ struct ContentAddressWithReferences ContentAddressMethod getMethod() const; Hash getHash() const; - - std::string printMethodAlgo() const; }; } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 564c12f9e..d56dc727b 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -38,7 +38,7 @@ StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view { return store.makeFixedOutputPathFromCA( outputPathName(drvName, outputName), - ca); + ContentAddressWithReferences::withoutRefs(ca)); } @@ -230,11 +230,9 @@ static DerivationOutput parseDerivationOutput(const Store & store, validatePath(pathS); auto hash = Hash::parseNonSRIUnprefixed(hashS, hashType); return DerivationOutput::CAFixed { - .ca = ContentAddressWithReferences::fromParts( + .ca = ContentAddress::fromParts( std::move(method), - std::move(hash), - // FIXME non-trivial fixed refs set - {}), + std::move(hash)), }; } else { experimentalFeatureSettings.require(Xp::CaDerivations); @@ -1020,10 +1018,9 @@ DerivationOutput DerivationOutput::fromJSON( else if (keys == (std::set { "path", "hashAlgo", "hash" })) { auto [method, hashType] = methodAlgo(); auto dof = DerivationOutput::CAFixed { - .ca = ContentAddressWithReferences::fromParts( + .ca = ContentAddress::fromParts( std::move(method), - Hash::parseNonSRIUnprefixed((std::string) json["hash"], hashType), - {}), + Hash::parseNonSRIUnprefixed((std::string) json["hash"], hashType)), }; if (dof.path(store, drvName, outputName) != store.parseStorePath((std::string) json["path"])) throw Error("Path doesn't match derivation output"); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 666dbff41..1e2143f31 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -36,9 +36,11 @@ struct DerivationOutputInputAddressed struct DerivationOutputCAFixed { /** - * hash and refs used for expected hash computation + * Method and hash used for expected hash computation. + * + * References are not allowed by fiat. */ - ContentAddressWithReferences ca; /* hash and refs used for validating output */ + ContentAddress ca; /** * Return the \ref StorePath "store path" corresponding to this output diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 39bdfec6e..00f629a85 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -83,26 +83,16 @@ void Store::computeFSClosure(const StorePath & startPath, } -std::optional getDerivationCA(const BasicDerivation & drv) +const ContentAddress * getDerivationCA(const BasicDerivation & drv) { auto out = drv.outputs.find("out"); if (out == drv.outputs.end()) - return std::nullopt; + return nullptr; if (auto dof = std::get_if(&out->second)) { - return std::visit(overloaded { - [&](const TextInfo & ti) -> std::optional { - if (!ti.references.empty()) - return std::nullopt; - return ti.hash; - }, - [&](const FixedOutputInfo & fi) -> std::optional { - if (!fi.references.empty()) - return std::nullopt; - return fi.hash; - }, - }, dof->ca.raw); + + return &dof->ca; } - return std::nullopt; + return nullptr; } void Store::queryMissing(const std::vector & targets, @@ -152,7 +142,13 @@ void Store::queryMissing(const std::vector & targets, if (drvState_->lock()->done) return; SubstitutablePathInfos infos; - querySubstitutablePathInfos({{outPath, getDerivationCA(*drv)}}, infos); + auto * cap = getDerivationCA(*drv); + querySubstitutablePathInfos({ + { + outPath, + cap ? std::optional { *cap } : std::nullopt, + }, + }, infos); if (infos.empty()) { drvState_->lock()->done = true; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index c910d1c96..bad610014 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -1022,7 +1022,7 @@ std::optional decodeValidPathInfo( */ std::pair splitUriAndParams(const std::string & uri); -std::optional getDerivationCA(const BasicDerivation & drv); +const ContentAddress * getDerivationCA(const BasicDerivation & drv); std::map drvOutputReferences( Store & store, diff --git a/src/libstore/tests/derivation.cc b/src/libstore/tests/derivation.cc index 85e451b29..6328ad370 100644 --- a/src/libstore/tests/derivation.cc +++ b/src/libstore/tests/derivation.cc @@ -74,19 +74,30 @@ TEST_JSON(DerivationTest, inputAddressed, }), "drv-name", "output-name") -TEST_JSON(DerivationTest, caFixed, +TEST_JSON(DerivationTest, caFixedFlat, + R"({ + "hashAlgo": "sha256", + "hash": "894517c9163c896ec31a2adbd33c0681fd5f45b2c0ef08a64c92a03fb97f390f", + "path": "/nix/store/rhcg9h16sqvlbpsa6dqm57sbr2al6nzg-drv-name-output-name" + })", + (DerivationOutput::CAFixed { + .ca = FixedOutputHash { + .method = FileIngestionMethod::Flat, + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), + }, + }), + "drv-name", "output-name") + +TEST_JSON(DerivationTest, caFixedNAR, R"({ "hashAlgo": "r:sha256", "hash": "894517c9163c896ec31a2adbd33c0681fd5f45b2c0ef08a64c92a03fb97f390f", "path": "/nix/store/c015dhfh5l0lp6wxyvdn7bmwhbbr6hr9-drv-name-output-name" })", (DerivationOutput::CAFixed { - .ca = FixedOutputInfo { - .hash = { - .method = FileIngestionMethod::Recursive, - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - .references = {}, + .ca = FixedOutputHash { + .method = FileIngestionMethod::Recursive, + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), "drv-name", "output-name") @@ -98,11 +109,8 @@ TEST_JSON(DynDerivationTest, caFixedText, "path": "/nix/store/6s1zwabh956jvhv4w9xcdb5jiyanyxg1-drv-name-output-name" })", (DerivationOutput::CAFixed { - .ca = TextInfo { - .hash = { - .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), - }, - .references = {}, + .ca = TextHash { + .hash = Hash::parseAnyPrefixed("sha256-iUUXyRY8iW7DGirb0zwGgf1fRbLA7wimTJKgP7l/OQ8="), }, }), "drv-name", "output-name")