From 26b254b146c9094472fe20975a8468406f6fb23b Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 8 Nov 2023 21:11:48 -0500 Subject: [PATCH] Remove now-redundant text-hashing store methods `addTextToStore` and `computeStorePathFromDump` are now redundant. Co-authored-by: Robert Hensing --- perl/lib/Nix/Store.xs | 12 +++- src/libexpr/primops.cc | 10 ++- src/libstore/binary-cache-store.cc | 71 +++++++++++---------- src/libstore/binary-cache-store.hh | 6 -- src/libstore/build/local-derivation-goal.cc | 11 ---- src/libstore/content-address.hh | 8 +-- src/libstore/daemon.cc | 5 +- src/libstore/derivations.cc | 10 ++- src/libstore/dummy-store.cc | 7 -- src/libstore/legacy-ssh-store.cc | 7 -- src/libstore/local-store.cc | 52 --------------- src/libstore/local-store.hh | 6 -- src/libstore/remote-store.cc | 10 --- src/libstore/remote-store.hh | 6 -- src/libstore/store-api.cc | 34 +++------- src/libstore/store-api.hh | 33 ---------- src/nix-env/user-env.cc | 13 ++-- src/nix/develop.cc | 6 +- 18 files changed, 93 insertions(+), 214 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index f89ac4077d0..c696fe58843 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -12,6 +12,7 @@ #include "globals.hh" #include "store-api.hh" #include "crypto.hh" +#include "posix-source-accessor.hh" #include #include @@ -204,7 +205,10 @@ void importPaths(int fd, int dontCheckSigs) SV * hashPath(char * algo, int base32, char * path) PPCODE: try { - Hash h = hashPath(parseHashType(algo), path).first; + PosixSourceAccessor accessor; + Hash h = hashPath( + accessor, CanonPath::fromCwd(path), + FileIngestionMethod::Recursive, parseHashType(algo)).first; auto s = h.to_string(base32 ? HashFormat::Base32 : HashFormat::Base16, false); XPUSHs(sv_2mortal(newSVpv(s.c_str(), 0))); } catch (Error & e) { @@ -280,7 +284,11 @@ SV * addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; - auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, method, parseHashType(algo)); + PosixSourceAccessor accessor; + auto path = store()->addToStore( + std::string(baseNameOf(srcPath)), + accessor, CanonPath::fromCwd(srcPath), + method, parseHashType(algo)); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d10b18fbe28..19927ad582c 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2090,8 +2090,14 @@ static void prim_toFile(EvalState & state, const PosIdx pos, Value * * args, Val } auto storePath = settings.readOnlyMode - ? state.store->computeStorePathForText(name, contents, refs) - : state.store->addTextToStore(name, contents, refs, state.repair); + ? state.store->makeFixedOutputPathFromCA(name, TextInfo { + .hash = hashString(htSHA256, contents), + .references = std::move(refs), + }) + : ({ + StringSource s { contents }; + state.store->addToStoreFromDump(s, name, TextIngestionMethod {}, htSHA256, refs, state.repair); + }); /* Note: we don't need to add `context' to the context of the result, since `storePath' itself has references to the paths diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 114c7b07844..f963cba07e0 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -12,6 +12,7 @@ #include "thread-pool.hh" #include "callback.hh" #include "signals.hh" +#include "archive.hh" #include #include @@ -308,15 +309,47 @@ StorePath BinaryCacheStore::addToStoreFromDump( const StorePathSet & references, RepairFlag repair) { - if (method != FileIngestionMethod::Recursive || hashAlgo != htSHA256) - unsupported("addToStoreFromDump"); - return addToStoreCommon(dump, repair, CheckSigs, [&](HashResult nar) { + std::optional caHash; + std::string nar; + + if (auto * dump2p = dynamic_cast(&dump)) { + auto & dump2 = *dump2p; + // Hack, this gives us a "replayable" source so we can compute + // multiple hashes more easily. + caHash = hashString(htSHA256, dump2.s); + switch (method.getFileIngestionMethod()) { + case FileIngestionMethod::Recursive: + // The dump is already NAR in this case, just use it. + nar = dump2.s; + break; + case FileIngestionMethod::Flat: + // The dump is Flat, so we need to convert it to NAR with a + // single file. + StringSink s; + dumpString(dump2.s, s); + nar = std::move(s.s); + break; + } + } else { + // Otherwise, we have to do th same hashing as NAR so our single + // hash will suffice for both purposes. + if (method != FileIngestionMethod::Recursive || hashAlgo != htSHA256) + unsupported("addToStoreFromDump"); + } + StringSource narDump { nar }; + + // Use `narDump` if we wrote to `nar`. + Source & narDump2 = nar.size() > 0 + ? static_cast(narDump) + : dump; + + return addToStoreCommon(narDump2, repair, CheckSigs, [&](HashResult nar) { ValidPathInfo info { *this, name, ContentAddressWithReferences::fromParts( method, - nar.first, + caHash ? *caHash : nar.first, { .others = references, // caller is not capable of creating a self-reference, because this is content-addressed without modulus @@ -440,36 +473,6 @@ StorePath BinaryCacheStore::addToStore( })->path; } -StorePath BinaryCacheStore::addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) -{ - auto textHash = hashString(htSHA256, s); - auto path = makeTextPath(name, TextInfo { { textHash }, references }); - - if (!repair && isValidPath(path)) - return path; - - StringSink sink; - dumpString(s, sink); - StringSource source(sink.s); - return addToStoreCommon(source, repair, CheckSigs, [&](HashResult nar) { - ValidPathInfo info { - *this, - std::string { name }, - TextInfo { - .hash = textHash, - .references = references, - }, - nar.first, - }; - info.narSize = nar.second; - return info; - })->path; -} - void BinaryCacheStore::queryRealisationUncached(const DrvOutput & id, Callback> callback) noexcept { diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 1d3b934aba4..4f2ba57da2a 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -141,12 +141,6 @@ public: PathFilter & filter, RepairFlag repair) override; - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) override; - void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached(const DrvOutput &, diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8d56951b8c6..ff187eca5ea 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1307,17 +1307,6 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual In goal.addDependency(info.path); } - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair = NoRepair) override - { - auto path = next->addTextToStore(name, s, references, repair); - goal.addDependency(path); - return path; - } - StorePath addToStoreFromDump( Source & dump, std::string_view name, diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index f04da03a738..45b96da4cb5 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -109,11 +109,11 @@ struct ContentAddressMethod * serialisation methods (flat file vs NAR). Thus, ‘ca’ has one of the * following forms: * - * - ‘text:sha256:’: For paths - * computed by Store::makeTextPath() / Store::addTextToStore(). + * - `TextIngestionMethod`: + * ‘text:sha256:’ * - * - ‘fixed:::’: For paths computed by - * Store::makeFixedOutputPath() / Store::addToStore(). + * - `FixedIngestionMethod`: + * ‘fixed:::’ */ struct ContentAddress { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 459c4df69c2..3db82c1c02d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -483,7 +483,10 @@ static void performOp(TunnelLogger * logger, ref store, std::string s = readString(from); auto refs = WorkerProto::Serialise::read(*store, rconn); logger->startWork(); - auto path = store->addTextToStore(suffix, s, refs, NoRepair); + auto path = ({ + StringSource source { s }; + store->addToStoreFromDump(source, suffix, TextIngestionMethod {}, htSHA256, refs, NoRepair); + }); logger->stopWork(); to << store->printStorePath(path); break; diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 1fecd1c97b6..924233dea2c 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -143,8 +143,14 @@ StorePath writeDerivation(Store & store, auto suffix = std::string(drv.name) + drvExtension; auto contents = drv.unparse(store, false); return readOnly || settings.readOnlyMode - ? store.computeStorePathForText(suffix, contents, references) - : store.addTextToStore(suffix, contents, references, repair); + ? store.makeFixedOutputPathFromCA(suffix, TextInfo { + .hash = hashString(htSHA256, contents), + .references = std::move(references), + }) + : ({ + StringSource s { contents }; + store.addToStoreFromDump(s, suffix, TextIngestionMethod {}, htSHA256, references, repair); + }); } diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 821cda399c9..f52a309d13b 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -58,13 +58,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store RepairFlag repair, CheckSigsFlag checkSigs) override { unsupported("addToStore"); } - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) override - { unsupported("addTextToStore"); } - void narFromPath(const StorePath & path, Sink & sink) override { unsupported("narFromPath"); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 6c54ced5533..a77dd5b8942 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -277,13 +277,6 @@ struct LegacySSHStore : public virtual LegacySSHStoreConfig, public virtual Stor RepairFlag repair) override { unsupported("addToStore"); } - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) override - { unsupported("addTextToStore"); } - private: void putBuildSettings(Connection & conn) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 41d0399fe83..b71ba4f7d69 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1414,58 +1414,6 @@ StorePath LocalStore::addToStoreFromDump( } -StorePath LocalStore::addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, RepairFlag repair) -{ - auto hash = hashString(htSHA256, s); - auto dstPath = makeTextPath(name, TextInfo { - .hash = hash, - .references = references, - }); - - addTempRoot(dstPath); - - if (repair || !isValidPath(dstPath)) { - - auto realPath = Store::toRealPath(dstPath); - - PathLocks outputLock({realPath}); - - if (repair || !isValidPath(dstPath)) { - - deletePath(realPath); - - autoGC(); - - writeFile(realPath, s); - - canonicalisePathMetaData(realPath, {}); - - StringSink sink; - dumpString(s, sink); - auto narHash = hashString(htSHA256, sink.s); - - optimisePath(realPath, repair); - - ValidPathInfo info { dstPath, narHash }; - info.narSize = sink.s.size(); - info.references = references; - info.ca = { - .method = TextIngestionMethod {}, - .hash = hash, - }; - registerValidPath(info); - } - - outputLock.setDeletion(true); - } - - return dstPath; -} - - /* Create a temporary directory in the store that won't be garbage-collected until the returned FD is closed. */ std::pair LocalStore::createTempDirInStore() diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index c52ef4f2f6a..e6375286ed9 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -185,12 +185,6 @@ public: const StorePathSet & references, RepairFlag repair) override; - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) override; - void addTempRoot(const StorePath & path) override; private: diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 79ab42e92e8..ff13e8caa2f 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -608,16 +608,6 @@ void RemoteStore::addMultipleToStore( } -StorePath RemoteStore::addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) -{ - StringSource source(s); - return addCAToStore(source, name, TextIngestionMethod {}, htSHA256, references, repair)->path; -} - void RemoteStore::registerDrvOutput(const Realisation & info) { auto conn(getConnection()); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index e4a5704bf92..e37d3689a77 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -106,12 +106,6 @@ public: RepairFlag repair, CheckSigsFlag checkSigs) override; - StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair) override; - void registerDrvOutput(const Realisation & info) override; void queryRealisationUncached(const DrvOutput &, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2d87e671e52..648f7530f23 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -203,25 +203,19 @@ StorePath Store::makeFixedOutputPath(std::string_view name, const FixedOutputInf } -StorePath Store::makeTextPath(std::string_view name, const TextInfo & info) const -{ - assert(info.hash.type == htSHA256); - return makeStorePath( - makeType(*this, "text", StoreReferences { - .others = info.references, - .self = false, - }), - info.hash, - name); -} - - StorePath Store::makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const { // New template return std::visit(overloaded { [&](const TextInfo & ti) { - return makeTextPath(name, ti); + assert(ti.hash.type == htSHA256); + return makeStorePath( + makeType(*this, "text", StoreReferences { + .others = ti.references, + .self = false, + }), + ti.hash, + name); }, [&](const FixedOutputInfo & foi) { return makeFixedOutputPath(name, foi); @@ -255,18 +249,6 @@ std::pair Store::computeStorePath( } -StorePath Store::computeStorePathForText( - std::string_view name, - std::string_view s, - const StorePathSet & references) const -{ - return makeTextPath(name, TextInfo { - .hash = hashString(htSHA256, s), - .references = references, - }); -} - - StorePath Store::addToStore( std::string_view name, SourceAccessor & accessor, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a7f0838f447..b4a2e270571 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -286,8 +286,6 @@ public: StorePath makeFixedOutputPath(std::string_view name, const FixedOutputInfo & info) const; - StorePath makeTextPath(std::string_view name, const TextInfo & info) const; - StorePath makeFixedOutputPathFromCA(std::string_view name, const ContentAddressWithReferences & ca) const; /** @@ -303,27 +301,6 @@ public: const StorePathSet & references = {}, PathFilter & filter = defaultPathFilter) const; - /** - * Preparatory part of addTextToStore(). - * - * !!! Computation of the path should take the references given to - * addTextToStore() into account, otherwise we have a (relatively - * minor) security hole: a caller can register a source file with - * bogus references. If there are too many references, the path may - * not be garbage collected when it has to be (not really a problem, - * the caller could create a root anyway), or it may be garbage - * collected when it shouldn't be (more serious). - * - * Hashing the references would solve this (bogus references would - * simply yield a different store path, so other users wouldn't be - * affected), but it has some backwards compatibility issues (the - * hashing scheme changes), so I'm not doing that for now. - */ - StorePath computeStorePathForText( - std::string_view name, - std::string_view s, - const StorePathSet & references) const; - /** * Check whether a path is valid. */ @@ -561,16 +538,6 @@ public: RepairFlag repair = NoRepair) { unsupported("addToStoreFromDump"); } - /** - * Like addToStore, but the contents written to the output path is a - * regular file containing the given string. - */ - virtual StorePath addTextToStore( - std::string_view name, - std::string_view s, - const StorePathSet & references, - RepairFlag repair = NoRepair) = 0; - /** * Add a mapping indicating that `deriver!outputName` maps to the output path * `output`. diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 250224e7da0..3a0b2697b69 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -104,10 +104,15 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Also write a copy of the list of user environment elements to the store; we need it for future modifications of the environment. */ - std::ostringstream str; - manifest.print(state.symbols, str, true); - auto manifestFile = state.store->addTextToStore("env-manifest.nix", - str.str(), references); + auto manifestFile = ({ + std::ostringstream str; + manifest.print(state.symbols, str, true); + // TODO with C++20 we can use str.view() instead and avoid copy. + std::string str2 = str.str(); + StringSource source { str2 }; + state.store->addToStoreFromDump( + source, "env-manifest.nix", TextIngestionMethod {}, htSHA256, references); + }); /* Get the environment builder expression. */ Value envBuilder; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 38482ed42a4..f9d183e1928 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -223,7 +223,11 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore if (builder != "bash") throw Error("'nix develop' only works on derivations that use 'bash' as their builder"); - auto getEnvShPath = evalStore->addTextToStore("get-env.sh", getEnvSh, {}); + auto getEnvShPath = ({ + StringSource source { getEnvSh }; + evalStore->addToStoreFromDump( + source, "get-env.sh", TextIngestionMethod {}, htSHA256, {}); + }); drv.args = {store->printStorePath(getEnvShPath)};