diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8a19eab8f..0113659d1 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1535,7 +1535,8 @@ static void prim_readFile(EvalState & state, const PosIdx pos, Value * * args, V StorePathSet refs; if (state.store->isInStore(path)) { try { - refs = state.store->queryPathInfo(state.store->toStorePath(path).first)->references; + // FIXME: Are self references becoming non-self references OK? + refs = state.store->queryPathInfo(state.store->toStorePath(path).first)->referencesPossiblyToSelf(); } catch (Error &) { // FIXME: should be InvalidPathError } // Re-scan references to filter down to just the ones that actually occur in the file. @@ -1971,7 +1972,7 @@ static void addPath( try { auto [storePath, subPath] = state.store->toStorePath(path); // FIXME: we should scanForReferences on the path before adding it - refs = state.store->queryPathInfo(storePath)->references; + refs = state.store->queryPathInfo(storePath)->referencesPossiblyToSelf(); path = state.store->toRealPath(storePath) + subPath; } catch (Error &) { // FIXME: should be InvalidPathError } @@ -2010,7 +2011,7 @@ static void addPath( .method = method, .hash = *expectedHash, }, - {}, + .references = {}, }); if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index aac5e7b88..aa5aafdbf 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -180,8 +180,9 @@ ref BinaryCacheStore::addToStoreCommon( duration); /* Verify that all references are valid. This may do some .narinfo - reads, but typically they'll already be cached. */ - for (auto & ref : info.references) + reads, but typically they'll already be cached. Note that + self-references are always valid. */ + for (auto & ref : info.references.others) try { queryPathInfo(ref); } catch (InvalidPath &) { @@ -314,8 +315,8 @@ StorePath BinaryCacheStore::addToStoreFromDump(Source & dump, std::string_view n .hash = nar.first, }, .references = { - .references = references, - .hasSelfReference = false, + .others = references, + .self = false, }, }, }, @@ -434,8 +435,8 @@ StorePath BinaryCacheStore::addToStore( .hash = h, }, .references = { - .references = references, - .hasSelfReference = false, + .others = references, + .self = false, }, }, }, diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 3d8299bbf..ff24bd088 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2421,26 +2421,26 @@ DrvOutputs LocalDerivationGoal::registerOutputs() } }; - auto rewriteRefs = [&]() -> PathReferences { + auto rewriteRefs = [&]() -> StoreReferences { /* In the CA case, we need the rewritten refs to calculate the final path, therefore we look for a *non-rewritten self-reference, and use a bool rather try to solve the computationally intractable fixed point. */ - PathReferences res { - .hasSelfReference = false, + StoreReferences res { + .self = false, }; for (auto & r : references) { auto name = r.name(); auto origHash = std::string { r.hashPart() }; if (r == *scratchPath) { - res.hasSelfReference = true; + res.self = true; } else if (auto outputRewrite = get(outputRewrites, origHash)) { std::string newRef = *outputRewrite; newRef += '-'; newRef += name; - res.references.insert(StorePath { newRef }); + res.others.insert(StorePath { newRef }); } else { - res.references.insert(r); + res.others.insert(r); } } return res; @@ -2523,7 +2523,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() auto narHashAndSize = hashPath(htSHA256, actualPath); ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; - static_cast &>(newInfo0) = rewriteRefs(); + newInfo0.references = rewriteRefs(); return newInfo0; }, diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 307183505..36b0ea7a7 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -128,7 +128,7 @@ void PathSubstitutionGoal::tryNext() } if (info->path != storePath) { - if (info->isContentAddressed(*sub) && info->references.empty() && !info->hasSelfReference) { + if (info->isContentAddressed(*sub) && info->references.empty()) { auto info2 = std::make_shared(*info); info2->path = storePath; info = info2; @@ -165,7 +165,7 @@ void PathSubstitutionGoal::tryNext() /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ - for (auto & i : info->references) + for (auto & i : info->references.others) addWaitee(worker.makePathSubstitutionGoal(i)); if (waitees.empty()) /* to prevent hang (no wake-up event) */ @@ -187,7 +187,7 @@ void PathSubstitutionGoal::referencesValid() return; } - for (auto & i : info->references) + for (auto & i : info->references.others) assert(worker.store.isValidPath(i)); state = &PathSubstitutionGoal::tryToRun; diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index 6be4be4c5..f8a4d5370 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -4,6 +4,7 @@ #include "hash.hh" #include "path.hh" #include "comparator.hh" +#include "reference-set.hh" namespace nix { @@ -94,48 +95,7 @@ Hash getContentAddressHash(const ContentAddress & ca); * References set */ -template -struct PathReferences -{ - std::set references; - bool hasSelfReference = false; - - /* Functions to view references + hasSelfReference as one set, mainly for - compatibility's sake. */ - StorePathSet referencesPossiblyToSelf(const Ref & self) const; - void insertReferencePossiblyToSelf(const Ref & self, Ref && ref); - void setReferencesPossiblyToSelf(const Ref & self, std::set && refs); - - GENERATE_CMP(PathReferences, me->references, me->hasSelfReference); -}; - -template -StorePathSet PathReferences::referencesPossiblyToSelf(const Ref & self) const -{ - StorePathSet refs { references }; - if (hasSelfReference) - refs.insert(self); - return refs; -} - -template -void PathReferences::insertReferencePossiblyToSelf(const Ref & self, Ref && ref) -{ - if (ref == self) - hasSelfReference = true; - else - references.insert(std::move(ref)); -} - -template -void PathReferences::setReferencesPossiblyToSelf(const Ref & self, std::set && refs) -{ - if (refs.count(self)) - hasSelfReference = true; - refs.erase(self); - - references = refs; -} +typedef References StoreReferences; /* * Full content address @@ -153,7 +113,7 @@ struct TextInfo : TextHash { struct FixedOutputInfo : FixedOutputHash { // References for the paths - PathReferences references; + StoreReferences references; GENERATE_CMP(FixedOutputInfo, *(const FixedOutputHash *)me, me->references); }; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 6407e575a..605f871fc 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -758,7 +758,7 @@ static void performOp(TunnelLogger * logger, ref store, else { to << 1 << (i->second.deriver ? store->printStorePath(*i->second.deriver) : ""); - worker_proto::write(*store, to, i->second.referencesPossiblyToSelf(path)); + worker_proto::write(*store, to, i->second.references.possiblyToSelf(path)); to << i->second.downloadSize << i->second.narSize; } @@ -781,7 +781,7 @@ static void performOp(TunnelLogger * logger, ref store, for (auto & i : infos) { to << store->printStorePath(i.first) << (i.second.deriver ? store->printStorePath(*i.second.deriver) : ""); - worker_proto::write(*store, to, i.second.referencesPossiblyToSelf(i.first)); + worker_proto::write(*store, to, i.second.references.possiblyToSelf(i.first)); to << i.second.downloadSize << i.second.narSize; } break; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9f3a6db24..b32953f3f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1157,11 +1157,10 @@ void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, Subst auto narInfo = std::dynamic_pointer_cast( std::shared_ptr(info)); infos.insert_or_assign(path.first, SubstitutablePathInfo{ - info->references, - info->hasSelfReference, - info->deriver, - narInfo ? narInfo->fileSize : 0, - info->narSize, + .deriver = info->deriver, + .references = info->references, + .downloadSize = narInfo ? narInfo->fileSize : 0, + .narSize = info->narSize, }); } catch (InvalidPath &) { } catch (SubstituterDisabled &) { @@ -1228,7 +1227,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos) topoSort(paths, {[&](const StorePath & path) { auto i = infos.find(path); - return i == infos.end() ? StorePathSet() : i->second.references; + return i == infos.end() ? StorePathSet() : i->second.references.others; }}, {[&](const StorePath & path, const StorePath & parent) { return BuildError( @@ -1427,8 +1426,8 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, std::string_view name .hash = hash, }, .references = { - .references = references, - .hasSelfReference = false, + .others = references, + .self = false, }, }, }; @@ -1526,7 +1525,8 @@ StorePath LocalStore::addTextToStore( ValidPathInfo info { dstPath, narHash }; info.narSize = sink.s.size(); - info.references = references; + // No self reference allowed with text-hashing + info.references.others = references; info.ca = TextHash { .hash = hash }; registerValidPath(info); } diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index d6b6e87c9..5d7945eb1 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -27,15 +27,15 @@ std::map makeContentAddressed( StringMap rewrites; - PathReferences refs; - refs.hasSelfReference = oldInfo->hasSelfReference; - for (auto & ref : oldInfo->references) { + StoreReferences refs; + refs.self = oldInfo->references.self; + for (auto & ref : oldInfo->references.others) { auto i = remappings.find(ref); auto replacement = i != remappings.end() ? i->second : ref; // FIXME: warn about unremapped paths? if (replacement != ref) { rewrites.insert_or_assign(srcStore.printStorePath(ref), srcStore.printStorePath(replacement)); - refs.references.insert(std::move(replacement)); + refs.others.insert(std::move(replacement)); } } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index e0bb1fab0..87f85c3cc 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -21,16 +21,16 @@ void Store::computeFSClosure(const StorePathSet & startPaths, StorePathSet res; StorePathSet referrers; queryReferrers(path, referrers); - for (auto& ref : referrers) + for (auto & ref : referrers) if (ref != path) res.insert(ref); if (includeOutputs) - for (auto& i : queryValidDerivers(path)) + for (auto & i : queryValidDerivers(path)) res.insert(i); if (includeDerivers && path.isDerivation()) - for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path)) + for (auto & [_, maybeOutPath] : queryPartialDerivationOutputMap(path)) if (maybeOutPath && isValidPath(*maybeOutPath)) res.insert(*maybeOutPath); return res; @@ -40,11 +40,11 @@ void Store::computeFSClosure(const StorePathSet & startPaths, std::future> & fut) { StorePathSet res; auto info = fut.get(); - for (auto& ref : info->references) + for (auto & ref : info->references.others) res.insert(ref); if (includeOutputs && path.isDerivation()) - for (auto& [_, maybeOutPath] : queryPartialDerivationOutputMap(path)) + for (auto & [_, maybeOutPath] : queryPartialDerivationOutputMap(path)) if (maybeOutPath && isValidPath(*maybeOutPath)) res.insert(*maybeOutPath); @@ -223,7 +223,7 @@ void Store::queryMissing(const std::vector & targets, state->narSize += info->second.narSize; } - for (auto & ref : info->second.references) + for (auto & ref : info->second.references.others) pool.enqueue(std::bind(doPath, DerivedPath::Opaque { ref })); }, }, req.raw()); @@ -241,7 +241,7 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) return topoSort(paths, {[&](const StorePath & path) { try { - return queryPathInfo(path)->references; + return queryPathInfo(path)->references.others; } catch (InvalidPath &) { return StorePathSet(); } @@ -297,7 +297,7 @@ std::map drvOutputReferences( auto info = store.queryPathInfo(outputPath); - return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); + return drvOutputReferences(Realisation::closure(store, inputRealisations), info->referencesPossiblyToSelf()); } } diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh index 89886873a..9254835b7 100644 --- a/src/libstore/path-info.hh +++ b/src/libstore/path-info.hh @@ -14,20 +14,22 @@ namespace nix { class Store; -struct SubstitutablePathInfo : PathReferences +struct SubstitutablePathInfo { std::optional deriver; + StoreReferences references; uint64_t downloadSize; /* 0 = unknown or inapplicable */ uint64_t narSize; /* 0 = unknown */ }; typedef std::map SubstitutablePathInfos; -struct ValidPathInfo : PathReferences +struct ValidPathInfo { StorePath path; std::optional deriver; Hash narHash; + StoreReferences references; time_t registrationTime = 0; uint64_t narSize = 0; // 0 = unknown uint64_t id; // internal use only @@ -61,7 +63,6 @@ struct ValidPathInfo : PathReferences return path == i.path && narHash == i.narHash - && hasSelfReference == i.hasSelfReference && references == i.references; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 689ad3fbe..1f8098b85 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -402,7 +402,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S auto deriver = readString(conn->from); if (deriver != "") info.deriver = parseStorePath(deriver); - info.setReferencesPossiblyToSelf(i.first, worker_proto::read(*this, conn->from, Phantom {})); + info.references.setPossiblyToSelf(i.first, worker_proto::read(*this, conn->from, Phantom {})); info.downloadSize = readLongLong(conn->from); info.narSize = readLongLong(conn->from); infos.insert_or_assign(i.first, std::move(info)); @@ -426,7 +426,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S auto deriver = readString(conn->from); if (deriver != "") info.deriver = parseStorePath(deriver); - info.setReferencesPossiblyToSelf(path, worker_proto::read(*this, conn->from, Phantom {})); + info.references.setPossiblyToSelf(path, worker_proto::read(*this, conn->from, Phantom {})); info.downloadSize = readLongLong(conn->from); info.narSize = readLongLong(conn->from); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index cd48d616b..5490df292 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -169,13 +169,13 @@ StorePath Store::makeOutputPath(std::string_view id, static std::string makeType( const Store & store, std::string && type, - const PathReferences & references) + const StoreReferences & references) { - for (auto & i : references.references) { + for (auto & i : references.others) { type += ":"; type += store.printStorePath(i); } - if (references.hasSelfReference) type += ":self"; + if (references.self) type += ":self"; return std::move(type); } @@ -185,8 +185,7 @@ StorePath Store::makeFixedOutputPath(std::string_view name, const FixedOutputInf if (info.hash.type == htSHA256 && info.method == FileIngestionMethod::Recursive) { return makeStorePath(makeType(*this, "source", info.references), info.hash, name); } else { - assert(info.references.references.size() == 0); - assert(!info.references.hasSelfReference); + assert(info.references.size() == 0); return makeStorePath("output:out", hashString(htSHA256, "fixed:out:" @@ -201,7 +200,7 @@ StorePath Store::makeTextPath(std::string_view name, const TextInfo & info) cons { assert(info.hash.type == htSHA256); return makeStorePath( - makeType(*this, "text", PathReferences { info.references }), + makeType(*this, "text", StoreReferences { info.references }), info.hash, name); } @@ -311,7 +310,7 @@ void Store::addMultipleToStore( bytesExpected += info.narSize; act.setExpected(actCopyPath, bytesExpected); - return info.references; + return info.references.others; }, [&](const StorePath & path) { @@ -816,7 +815,7 @@ std::string Store::makeValidityRegistration(const StorePathSet & paths, s += (format("%1%\n") % info->references.size()).str(); - for (auto & j : info->references) + for (auto & j : info->referencesPossiblyToSelf()) s += printStorePath(j) + "\n"; } @@ -878,7 +877,7 @@ json Store::pathInfoToJSON(const StorePathSet & storePaths, { auto& jsonRefs = (jsonPath["references"] = json::array()); - for (auto & ref : info->references) + for (auto & ref : info->referencesPossiblyToSelf()) jsonRefs.emplace_back(printStorePath(ref)); } @@ -1231,17 +1230,17 @@ std::string showPaths(const PathSet & paths) StorePathSet ValidPathInfo::referencesPossiblyToSelf() const { - return PathReferences::referencesPossiblyToSelf(path); + return references.possiblyToSelf(path); } void ValidPathInfo::insertReferencePossiblyToSelf(StorePath && ref) { - return PathReferences::insertReferencePossiblyToSelf(path, std::move(ref)); + return references.insertPossiblyToSelf(path, std::move(ref)); } void ValidPathInfo::setReferencesPossiblyToSelf(StorePathSet && refs) { - return PathReferences::setReferencesPossiblyToSelf(path, std::move(refs)); + return references.setPossiblyToSelf(path, std::move(refs)); } std::string ValidPathInfo::fingerprint(const Store & store) const @@ -1271,16 +1270,16 @@ std::optional ValidPathInfo::fullStorePathDescriptorOpt() c .name = std::string { path.name() }, .info = std::visit(overloaded { [&](const TextHash & th) -> ContentAddressWithReferences { - assert(!hasSelfReference); + assert(!references.self); return TextInfo { th, - .references = references, + .references = references.others, }; }, [&](const FixedOutputHash & foh) -> ContentAddressWithReferences { return FixedOutputInfo { foh, - .references = static_cast>(*this), + .references = references, }; }, }, *ca), @@ -1341,11 +1340,14 @@ ValidPathInfo::ValidPathInfo( { std::visit(overloaded { [this](TextInfo && ti) { - this->references = std::move(ti.references); + this->references = { + .others = std::move(ti.references), + .self = false, + }; this->ca = std::move((TextHash &&) ti); }, [this](FixedOutputInfo && foi) { - *(static_cast *>(this)) = std::move(foi.references); + this->references = std::move(foi.references); this->ca = std::move((FixedOutputHash &&) foi); }, }, std::move(info.info)); diff --git a/src/libutil/reference-set.hh b/src/libutil/reference-set.hh new file mode 100644 index 000000000..ac4a9994e --- /dev/null +++ b/src/libutil/reference-set.hh @@ -0,0 +1,68 @@ +#pragma once + +#include "comparator.hh" + +#include + +namespace nix { + +template +struct References +{ + std::set others; + bool self = false; + + bool empty() const; + size_t size() const; + + /* Functions to view references + self as one set, mainly for + compatibility's sake. */ + std::set possiblyToSelf(const Ref & self) const; + void insertPossiblyToSelf(const Ref & self, Ref && ref); + void setPossiblyToSelf(const Ref & self, std::set && refs); + + GENERATE_CMP(References, me->others, me->self); +}; + +template +bool References::empty() const +{ + return !self && others.empty(); +} + +template +size_t References::size() const +{ + return (self ? 1 : 0) + others.size(); +} + +template +std::set References::possiblyToSelf(const Ref & selfRef) const +{ + std::set refs { others }; + if (self) + refs.insert(selfRef); + return refs; +} + +template +void References::insertPossiblyToSelf(const Ref & selfRef, Ref && ref) +{ + if (ref == selfRef) + self = true; + else + others.insert(std::move(ref)); +} + +template +void References::setPossiblyToSelf(const Ref & selfRef, std::set && refs) +{ + if (refs.count(selfRef)) { + self = true; + refs.erase(selfRef); + } + + others = refs; +} + +} diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 614a37eba..8a0f06435 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -205,12 +205,14 @@ struct ProfileManifest .method = FileIngestionMethod::Recursive, .hash = narHash, }, - .references = { references }, + .references = { + .others = std::move(references), + .self = false, + }, }, }, narHash, }; - info.references = std::move(references); info.narSize = sink.s.size(); StringSource source(sink.s); diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc index a08314a25..3d659d6d2 100644 --- a/src/nix/sigs.cc +++ b/src/nix/sigs.cc @@ -63,8 +63,7 @@ struct CmdCopySigs : StorePathsCommand binary. */ if (info->narHash != info2->narHash || info->narSize != info2->narSize || - info->references != info2->references || - info->hasSelfReference != info2->hasSelfReference) + info->references != info2->references) continue; for (auto & sig : info2->sigs) diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 76125e5e4..33cd13600 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -136,7 +136,7 @@ struct CmdWhyDepends : SourceExprCommand for (auto & path : closure) graph.emplace(path, Node { .path = path, - .refs = store->queryPathInfo(path)->references, + .refs = store->queryPathInfo(path)->references.others, .dist = path == dependencyPath ? 0 : inf });