From c51d554c933b5fe294da41fcdf5afe0d4f33f586 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 30 Mar 2023 17:12:49 -0400 Subject: [PATCH] Use "raw pattern" for content address types We weren't because this ancient PR predated it! This is actually a new version of the pattern which addresses some issues identified in #7479. --- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/build/substitution-goal.cc | 2 +- src/libstore/content-address.cc | 47 +++++---- src/libstore/content-address.hh | 110 +++++++++++++------- src/libstore/daemon.cc | 10 +- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 8 +- src/libstore/nar-info-disk-cache.cc | 2 +- src/libstore/nar-info.cc | 2 +- src/libstore/path-info.cc | 6 +- src/libstore/remote-store.cc | 8 +- src/libstore/store-api.cc | 2 +- src/nix-store/nix-store.cc | 2 +- src/nix/prefetch.cc | 2 +- 14 files changed, 118 insertions(+), 87 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index caa15ab04..4fb7aa9d8 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2508,7 +2508,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() /* Check wanted hash */ const Hash & wanted = dof.hash.hash; assert(newInfo0.ca); - auto got = getContentAddressHash(*newInfo0.ca); + auto got = newInfo0.ca->getHash(); if (wanted != got) { /* Throw an error after registering the path as valid. */ diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 87fed495c..190fb455a 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -97,7 +97,7 @@ void PathSubstitutionGoal::tryNext() if (ca) { subPath = sub->makeFixedOutputPathFromCA( std::string { storePath.name() }, - caWithoutRefs(*ca)); + ContentAddressWithReferences::withoutRefs(*ca)); if (sub->storeDir == worker.store.storeDir) assert(subPath == storePath); } else if (sub->storeDir != worker.store.storeDir) { diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 64daea0d4..055b216db 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -9,7 +9,6 @@ std::string FixedOutputHash::printMethodAlgo() const return makeFileIngestionPrefix(method) + printHashType(hash.type); } - std::string makeFileIngestionPrefix(FileIngestionMethod m) { switch (m) { @@ -22,35 +21,35 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m) } } -std::string renderContentAddress(ContentAddress ca) +std::string ContentAddress::render() const { return std::visit(overloaded { - [](TextHash & th) { + [](const TextHash & th) { return "text:" + th.hash.to_string(Base32, true); }, - [](FixedOutputHash & fsh) { + [](const FixedOutputHash & fsh) { return "fixed:" + makeFileIngestionPrefix(fsh.method) + fsh.hash.to_string(Base32, true); } - }, ca); + }, raw); } -std::string renderContentAddressMethod(ContentAddressMethod cam) +std::string ContentAddressMethod::render() const { return std::visit(overloaded { - [](TextHashMethod & th) { + [](const TextHashMethod & th) { return std::string{"text:"} + printHashType(htSHA256); }, - [](FixedOutputHashMethod & fshm) { + [](const FixedOutputHashMethod & fshm) { return "fixed:" + makeFileIngestionPrefix(fshm.fileIngestionMethod) + printHashType(fshm.hashType); } - }, cam); + }, raw); } -/* - Parses content address strings up to the hash. +/** + * Parses content address strings up to the hash. */ static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest) { @@ -94,7 +93,7 @@ static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & r throw UsageError("content address prefix '%s' is unrecognized. Recogonized prefixes are 'text' or 'fixed'", prefix); } -ContentAddress parseContentAddress(std::string_view rawCa) { +ContentAddress ContentAddress::parse(std::string_view rawCa) { auto rest = rawCa; ContentAddressMethod caMethod = parseContentAddressMethodPrefix(rest); @@ -112,10 +111,10 @@ ContentAddress parseContentAddress(std::string_view rawCa) { .hash = Hash::parseNonSRIUnprefixed(rest, std::move(fohMethod.hashType)), }); }, - }, caMethod); + }, caMethod.raw); } -ContentAddressMethod parseContentAddressMethod(std::string_view caMethod) +ContentAddressMethod ContentAddressMethod::parse(std::string_view caMethod) { std::string asPrefix = std::string{caMethod} + ":"; // parseContentAddressMethodPrefix takes its argument by reference @@ -123,26 +122,28 @@ ContentAddressMethod parseContentAddressMethod(std::string_view caMethod) return parseContentAddressMethodPrefix(asPrefixView); } -std::optional parseContentAddressOpt(std::string_view rawCaOpt) +std::optional ContentAddress::parseOpt(std::string_view rawCaOpt) { - return rawCaOpt == "" ? std::optional() : parseContentAddress(rawCaOpt); + return rawCaOpt == "" + ? std::nullopt + : std::optional { ContentAddress::parse(rawCaOpt) }; }; std::string renderContentAddress(std::optional ca) { - return ca ? renderContentAddress(*ca) : ""; + return ca ? ca->render() : ""; } -Hash getContentAddressHash(const ContentAddress & ca) +const Hash & ContentAddress::getHash() const { return std::visit(overloaded { - [](const TextHash & th) { + [](const TextHash & th) -> auto & { return th.hash; }, - [](const FixedOutputHash & fsh) { + [](const FixedOutputHash & fsh) -> auto & { return fsh.hash; }, - }, ca); + }, raw); } bool StoreReferences::empty() const @@ -155,7 +156,7 @@ size_t StoreReferences::size() const return (self ? 1 : 0) + others.size(); } -ContentAddressWithReferences caWithoutRefs(const ContentAddress & ca) { +ContentAddressWithReferences ContentAddressWithReferences::withoutRefs(const ContentAddress & ca) { return std::visit(overloaded { [&](const TextHash & h) -> ContentAddressWithReferences { return TextInfo { @@ -169,7 +170,7 @@ ContentAddressWithReferences caWithoutRefs(const ContentAddress & ca) { .references = {}, }; }, - }, ca); + }, ca.raw); } } diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index d74d1ff4b..d1dd1256c 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -39,17 +39,16 @@ enum struct FileIngestionMethod : uint8_t { Recursive = true }; -struct FixedOutputHashMethod { - FileIngestionMethod fileIngestionMethod; - HashType hashType; -}; - /** * Compute the prefix to the hash algorithm which indicates how the * files were ingested. */ std::string makeFileIngestionPrefix(FileIngestionMethod m); +struct FixedOutputHashMethod { + FileIngestionMethod fileIngestionMethod; + HashType hashType; +}; /** * An enumeration of all the ways we can serialize file system objects. @@ -59,14 +58,25 @@ std::string makeFileIngestionPrefix(FileIngestionMethod m); * with info on references, and we have `ContentAddressWithReferences`, * as defined further below. */ -typedef std::variant< - TextHashMethod, - FixedOutputHashMethod -> ContentAddressMethod; +struct ContentAddressMethod +{ + typedef std::variant< + TextHashMethod, + FixedOutputHashMethod + > Raw; -ContentAddressMethod parseContentAddressMethod(std::string_view rawCaMethod); + Raw raw; + + /* The moral equivalent of `using Raw::Raw;` */ + ContentAddressMethod(auto &&... arg) + : raw(std::forward(arg)...) + { } + + static ContentAddressMethod parse(std::string_view rawCaMethod); + + std::string render() const; +}; -std::string renderContentAddressMethod(ContentAddressMethod caMethod); /* * Mini content address @@ -115,25 +125,41 @@ struct FixedOutputHash { * - ‘fixed:::’: For paths computed by * Store::makeFixedOutputPath() / Store::addToStore(). */ -typedef std::variant< - TextHash, - FixedOutputHash -> ContentAddress; +struct ContentAddress +{ + typedef std::variant< + TextHash, + FixedOutputHash + > Raw; -/** - * Compute the content-addressability assertion (ValidPathInfo::ca) for - * paths created by Store::makeFixedOutputPath() / Store::addToStore(). - */ -std::string renderContentAddress(ContentAddress ca); + Raw raw; + + /* The moral equivalent of `using Raw::Raw;` */ + ContentAddress(auto &&... arg) + : raw(std::forward(arg)...) + { } + + /** + * Compute the content-addressability assertion (ValidPathInfo::ca) for + * paths created by Store::makeFixedOutputPath() / Store::addToStore(). + */ + std::string render() const; + + static ContentAddress parse(std::string_view rawCa); + + static std::optional parseOpt(std::string_view rawCaOpt); + + const Hash & getHash() const; +}; std::string renderContentAddress(std::optional ca); -ContentAddress parseContentAddress(std::string_view rawCa); - -std::optional parseContentAddressOpt(std::string_view rawCaOpt); - -Hash getContentAddressHash(const ContentAddress & ca); +/* + * Full content address + * + * See the schema for store paths in store-api.cc + */ /** * A set of references to other store objects. @@ -167,12 +193,6 @@ struct StoreReferences { GENERATE_CMP(StoreReferences, me->self, me->others); }; -/* - * Full content address - * - * See the schema for store paths in store-api.cc - */ - // This matches the additional info that we need for makeTextPath struct TextInfo { TextHash hash; @@ -200,15 +220,25 @@ struct FixedOutputInfo { * * A ContentAddress without a Hash. */ -typedef std::variant< - TextInfo, - FixedOutputInfo -> ContentAddressWithReferences; +struct ContentAddressWithReferences +{ + typedef std::variant< + TextInfo, + FixedOutputInfo + > Raw; -/** - * Create a ContentAddressWithReferences from a mere ContentAddress, by - * assuming no references in all cases. - */ -ContentAddressWithReferences caWithoutRefs(const ContentAddress &); + Raw raw; + + /* The moral equivalent of `using Raw::Raw;` */ + ContentAddressWithReferences(auto &&... arg) + : raw(std::forward(arg)...) + { } + + /** + * Create a ContentAddressWithReferences from a mere ContentAddress, by + * assuming no references in all cases. + */ + static ContentAddressWithReferences withoutRefs(const ContentAddress &); +}; } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 656ad4587..0169eef1a 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -401,21 +401,21 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); auto pathInfo = [&]() { // NB: FramedSource must be out of scope before logger->stopWork(); - ContentAddressMethod contentAddressMethod = parseContentAddressMethod(camStr); + ContentAddressMethod contentAddressMethod = ContentAddressMethod::parse(camStr); FramedSource source(from); // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store. return std::visit(overloaded { - [&](TextHashMethod &) { + [&](const TextHashMethod &) { // We could stream this by changing Store std::string contents = source.drain(); auto path = store->addTextToStore(name, contents, refs, repair); return store->queryPathInfo(path); }, - [&](FixedOutputHashMethod & fohm) { + [&](const FixedOutputHashMethod & fohm) { auto path = store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair, refs); return store->queryPathInfo(path); }, - }, contentAddressMethod); + }, contentAddressMethod.raw); }(); logger->stopWork(); @@ -880,7 +880,7 @@ static void performOp(TunnelLogger * logger, ref store, info.references = worker_proto::read(*store, from, Phantom {}); from >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(from); - info.ca = parseContentAddressOpt(readString(from)); + info.ca = ContentAddress::parseOpt(readString(from)); from >> repair >> dontCheckSigs; if (!trusted && dontCheckSigs) dontCheckSigs = false; diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 98322b045..a1c38d180 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -156,7 +156,7 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor throw Error("NAR hash is now mandatory"); info->narHash = Hash::parseAnyPrefixed(s); } - info->ca = parseContentAddressOpt(readString(conn->from)); + info->ca = ContentAddress::parseOpt(readString(conn->from)); info->sigs = readStrings(conn->from); auto s = readString(conn->from); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e1c7e387a..b49d5462b 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -944,7 +944,7 @@ std::shared_ptr LocalStore::queryPathInfoInternal(State & s if (s) info->sigs = tokenizeString(s, " "); s = (const char *) sqlite3_column_text(state.stmts->QueryPathInfo, 7); - if (s) info->ca = parseContentAddressOpt(s); + if (s) info->ca = ContentAddress::parseOpt(s); /* Get the references. */ auto useQueryReferences(state.stmts->QueryReferences.use()(info->id)); @@ -1150,7 +1150,7 @@ void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, Subst if (path.second) { subPath = makeFixedOutputPathFromCA( path.first.name(), - caWithoutRefs(*path.second)); + ContentAddressWithReferences::withoutRefs(*path.second)); if (sub->storeDir == storeDir) assert(subPath == path.first); if (subPath != path.first) @@ -1329,7 +1329,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, printStorePath(info.path), info.narSize, hashResult.second); if (info.ca) { - if (auto foHash = std::get_if(&*info.ca)) { + if (auto foHash = std::get_if(&info.ca->raw)) { auto actualFoHash = hashCAPath( foHash->method, foHash->hash.type, @@ -1342,7 +1342,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, actualFoHash.hash.to_string(Base32, true)); } } - if (auto textHash = std::get_if(&*info.ca)) { + if (auto textHash = std::get_if(&info.ca->raw)) { auto actualTextHash = hashString(htSHA256, readFile(realPath)); if (textHash->hash != actualTextHash) { throw Error("ca hash mismatch importing path '%s';\n specified: %s\n got: %s", diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 2645f468b..c7176d30f 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -273,7 +273,7 @@ public: narInfo->deriver = StorePath(queryNAR.getStr(9)); for (auto & sig : tokenizeString(queryNAR.getStr(10), " ")) narInfo->sigs.insert(sig); - narInfo->ca = parseContentAddressOpt(queryNAR.getStr(11)); + narInfo->ca = ContentAddress::parseOpt(queryNAR.getStr(11)); return {oValid, narInfo}; }); diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index 071d8355e..274cd861c 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -74,7 +74,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & else if (name == "CA") { if (ca) throw corrupt(); // FIXME: allow blank ca or require skipping field? - ca = parseContentAddressOpt(value); + ca = ContentAddress::parseOpt(value); } pos = eol + 1; diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index 76cab63e0..e60d7abe0 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -49,7 +49,7 @@ std::optional ValidPathInfo::contentAddressWithRef }, }; }, - }, *ca); + }, ca->raw); } bool ValidPathInfo::isContentAddressed(const Store & store) const @@ -116,7 +116,7 @@ ValidPathInfo::ValidPathInfo( this->references.insert(path); this->ca = std::move((FixedOutputHash &&) foi); }, - }, std::move(ca)); + }, std::move(ca).raw); } @@ -136,7 +136,7 @@ ValidPathInfo ValidPathInfo::read(Source & source, const Store & store, unsigned if (format >= 16) { source >> info.ultimate; info.sigs = readStrings(source); - info.ca = parseContentAddressOpt(readString(source)); + info.ca = ContentAddress::parseOpt(readString(source)); } return info; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d24d83117..ac98e76d2 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -44,7 +44,7 @@ void write(const Store & store, Sink & out, const StorePath & storePath) ContentAddress read(const Store & store, Source & from, Phantom _) { - return parseContentAddress(readString(from)); + return ContentAddress::parse(readString(from)); } void write(const Store & store, Sink & out, const ContentAddress & ca) @@ -134,7 +134,7 @@ void write(const Store & store, Sink & out, const std::optional & sto std::optional read(const Store & store, Source & from, Phantom> _) { - return parseContentAddressOpt(readString(from)); + return ContentAddress::parseOpt(readString(from)); } void write(const Store & store, Sink & out, const std::optional & caOpt) @@ -545,7 +545,7 @@ ref RemoteStore::addCAToStore( conn->to << wopAddToStore << name - << renderContentAddressMethod(caMethod); + << caMethod.render(); worker_proto::write(*this, conn->to, references); conn->to << repair; @@ -603,7 +603,7 @@ ref RemoteStore::addCAToStore( } } - }, caMethod); + }, caMethod.raw); auto path = parseStorePath(readString(conn->from)); // Release our connection to prevent a deadlock in queryPathInfo(). conn_.reset(); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index fed38e2dd..78b0d907e 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -221,7 +221,7 @@ StorePath Store::makeFixedOutputPathFromCA(std::string_view name, const ContentA [&](const FixedOutputInfo & foi) { return makeFixedOutputPath(name, foi); } - }, ca); + }, ca.raw); } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 26febb6e3..3d2dc49fd 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -970,7 +970,7 @@ static void opServe(Strings opFlags, Strings opArgs) info.references = worker_proto::read(*store, in, Phantom {}); in >> info.registrationTime >> info.narSize >> info.ultimate; info.sigs = readStrings(in); - info.ca = parseContentAddressOpt(readString(in)); + info.ca = ContentAddress::parseOpt(readString(in)); if (info.narSize == 0) throw Error("narInfo is too old and missing the narSize field"); diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index b06b8a320..56e7bbb6e 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -124,7 +124,7 @@ std::tuple prefetchFile( auto info = store->addToStoreSlow(*name, tmpFile, ingestionMethod, hashType, expectedHash); storePath = info.path; assert(info.ca); - hash = getContentAddressHash(*info.ca); + hash = info.ca->getHash(); } return {storePath.value(), hash.value()};