WIP: store separate hasValidPath bool

src/libstore/binary-cache-store.cc

@@ -125,7 +125,6 @@ void BinaryCacheStore::addToStore(const ValidPathInfo & info, Source & narSource
        reads, but typically they'll already be cached. */
     for (auto & ref : info.references)
         try {
-            if (ref != info.path)
             queryPathInfo(ref);
         } catch (InvalidPath &) {
             throw Error("cannot add '%s' to the binary cache because the reference '%s' is not valid",

src/libstore/build.cc

@@ -3834,7 +3834,7 @@ void DerivationGoal::registerOutputs()
         ValidPathInfo info(worker.store.parseStorePath(path));
         info.narHash = hash.first;
         info.narSize = hash.second;
-        info.references = std::move(references);
+        info.setReferencesPossiblyToSelf(std::move(references));
         info.deriver = drvPath;
         info.ultimate = true;
         info.ca = ca;
@@ -3963,12 +3963,12 @@ void DerivationGoal::checkOutputs(const std::map<Path, ValidPathInfo> & outputs)
                 auto i = outputsByPath.find(worker.store.printStorePath(path));
                 if (i != outputsByPath.end()) {
                     closureSize += i->second.narSize;
-                    for (auto & ref : i->second.references)
+                    for (auto & ref : i->second.referencesPossiblyToSelf())
                         pathsLeft.push(ref);
                 } else {
                     auto info = worker.store.queryPathInfo(path);
                     closureSize += info->narSize;
-                    for (auto & ref : info->references)
+                    for (auto & ref : info->referencesPossiblyToSelf())
                         pathsLeft.push(ref);
                 }
             }
@@ -3997,7 +3997,7 @@ void DerivationGoal::checkOutputs(const std::map<Path, ValidPathInfo> & outputs)
 
                 auto used = recursive
                     ? getClosure(info.path).first
-                    : info.references;
+                    : info.referencesPossiblyToSelf();
 
                 if (recursive && checks.ignoreSelfRefs)
                     used.erase(info.path);
@@ -4466,7 +4466,6 @@ void SubstitutionGoal::tryNext()
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
     for (auto & i : info->references)
-        if (i != storePath) /* ignore self-references */
         addWaitee(worker.makeSubstitutionGoal(i));
 
     if (waitees.empty()) /* to prevent hang (no wake-up event) */
@@ -4487,7 +4486,6 @@ void SubstitutionGoal::referencesValid()
     }
 
     for (auto & i : info->references)
-        if (i != storePath) /* ignore self-references */
         assert(worker.store.isValidPath(i));
 
     state = &SubstitutionGoal::tryToRun;

src/libstore/daemon.cc

@@ -326,7 +326,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         logger->startWork();
         StorePathSet paths;
         if (op == wopQueryReferences)
-            for (auto & i : store->queryPathInfo(path)->references)
+            for (auto & i : store->queryPathInfo(path)->referencesPossiblyToSelf())
                 paths.insert(i);
         else if (op == wopQueryReferrers)
             store->queryReferrers(path, paths);
@@ -601,7 +601,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         else {
             to << 1
                << (i->second.deriver ? store->printStorePath(*i->second.deriver) : "");
-            writeStorePaths(*store, to, i->second.references);
+            writeStorePaths(*store, to, i->second.referencesPossiblyToSelf(path));
             to << i->second.downloadSize
                << i->second.narSize;
         }
@@ -618,7 +618,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         for (auto & i : infos) {
             to << store->printStorePath(i.first)
                << (i.second.deriver ? store->printStorePath(*i.second.deriver) : "");
-            writeStorePaths(*store, to, i.second.references);
+            writeStorePaths(*store, to, i.second.referencesPossiblyToSelf(i.first));
             to << i.second.downloadSize << i.second.narSize;
         }
         break;
@@ -647,7 +647,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
                 to << 1;
             to << (info->deriver ? store->printStorePath(*info->deriver) : "")
                << info->narHash.to_string(Base16, false);
-            writeStorePaths(*store, to, info->references);
+            writeStorePaths(*store, to, info->referencesPossiblyToSelf());
             to << info->registrationTime << info->narSize;
             if (GET_PROTOCOL_MINOR(clientVersion) >= 16) {
                 to << info->ultimate
@@ -707,7 +707,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
         if (deriver != "")
             info.deriver = store->parseStorePath(deriver);
         info.narHash = Hash(readString(from), htSHA256);
-        info.references = readStorePaths<StorePathSet>(*store, from);
+        info.setReferencesPossiblyToSelf(readStorePaths<StorePathSet>(*store, from));
         from >> info.registrationTime >> info.narSize >> info.ultimate;
         info.sigs = readStrings<StringSet>(from);
         info.ca = parseContentAddressOpt(readString(from));

src/libstore/export-import.cc

@@ -62,7 +62,7 @@ void Store::exportPath(const StorePath & path, Sink & sink)
     hashAndWriteSink
         << exportMagic
         << printStorePath(path);
-    writeStorePaths(*this, hashAndWriteSink, info->references);
+    writeStorePaths(*this, hashAndWriteSink, info->referencesPossiblyToSelf());
     hashAndWriteSink
         << (info->deriver ? printStorePath(*info->deriver) : "")
         << 0;
@@ -88,7 +88,7 @@ StorePaths Store::importPaths(Source & source, std::shared_ptr<FSAccessor> acces
 
         //Activity act(*logger, lvlInfo, format("importing path '%s'") % info.path);
 
-        info.references = readStorePaths<StorePathSet>(*this, source);
+        info.setReferencesPossiblyToSelf(readStorePaths<StorePathSet>(*this, source));
 
         auto deriver = readString(source);
         if (deriver != "")

src/libstore/legacy-ssh-store.cc

@@ -103,11 +103,10 @@ struct LegacySSHStore : public Store
             auto info = std::make_shared<ValidPathInfo>(parseStorePath(p));
             assert(path == info->path);
 
-            PathSet references;
             auto deriver = readString(conn->from);
             if (deriver != "")
                 info->deriver = parseStorePath(deriver);
-            info->references = readStorePaths<StorePathSet>(*this, conn->from);
+            info->setReferencesPossiblyToSelf(readStorePaths<StorePathSet>(*this, conn->from));
             readLongLong(conn->from); // download size
             info->narSize = readLongLong(conn->from);
 
@@ -140,7 +139,7 @@ struct LegacySSHStore : public Store
                 << printStorePath(info.path)
                 << (info.deriver ? printStorePath(*info.deriver) : "")
                 << info.narHash.to_string(Base16, false);
-            writeStorePaths(*this, conn->to, info.references);
+            writeStorePaths(*this, conn->to, info.referencesPossiblyToSelf());
             conn->to
                 << info.registrationTime
                 << info.narSize
@@ -169,7 +168,7 @@ struct LegacySSHStore : public Store
             conn->to
                 << exportMagic
                 << printStorePath(info.path);
-            writeStorePaths(*this, conn->to, info.references);
+            writeStorePaths(*this, conn->to, info.referencesPossiblyToSelf());
             conn->to
                 << (info.deriver ? printStorePath(*info.deriver) : "")
                 << 0

src/libstore/local-store.cc

@@ -671,8 +671,10 @@ void LocalStore::queryPathInfoUncached(const StorePath & path,
             /* Get the references. */
             auto useQueryReferences(state->stmtQueryReferences.use()(info->id));
 
-            while (useQueryReferences.next())
+            while (useQueryReferences.next()) {
-                info->references.insert(parseStorePath(useQueryReferences.getStr(0)));
+                info->insertReferencePossiblyToSelf(
+                    parseStorePath(useQueryReferences.getStr(0)));
+            }
 
             return info;
         }));
@@ -856,11 +858,13 @@ void LocalStore::querySubstitutablePathInfos(const StorePathSet & paths,
                 auto info = sub->queryPathInfo(path);
                 auto narInfo = std::dynamic_pointer_cast<const NarInfo>(
                     std::shared_ptr<const ValidPathInfo>(info));
-                infos.insert_or_assign(path, SubstitutablePathInfo{
+                infos.insert_or_assign(path, SubstitutablePathInfo {
-                    info->deriver,
                     info->references,
+                    info->hasSelfReference,
+                    info->deriver,
                     narInfo ? narInfo->fileSize : 0,
-                    info->narSize});
+                    info->narSize,
+                });
             } catch (InvalidPath &) {
             } catch (SubstituterDisabled &) {
             } catch (Error & e) {
@@ -907,7 +911,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
 
         for (auto & i : infos) {
             auto referrer = queryValidPathId(*state, i.path);
-            for (auto & j : i.references)
+            for (auto & j : i.referencesPossiblyToSelf())
                 state->stmtAddReference.use()(referrer)(queryValidPathId(*state, j)).exec();
         }
 
@@ -986,14 +990,13 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
             deletePath(realPath);
 
             // text hashing has long been allowed to have non-self-references because it is used for drv files.
-            bool refersToSelf = info.references.count(info.path) > 0;
+            if (info.ca.has_value() && !info.references.empty() && !(std::holds_alternative<TextHash>(*info.ca) && info.hasSelfReference))
-            if (info.ca.has_value() && !info.references.empty() && !(std::holds_alternative<TextHash>(*info.ca) && !refersToSelf))
                 settings.requireExperimentalFeature("ca-references");
 
             /* While restoring the path from the NAR, compute the hash
                of the NAR. */
             std::unique_ptr<AbstractHashSink> hashSink;
-            if (!info.ca.has_value() || !info.references.count(info.path))
+            if (!info.ca.has_value() || !info.hasSelfReference)
                 hashSink = std::make_unique<HashSink>(htSHA256);
             else
                 hashSink = std::make_unique<HashModuloSink>(htSHA256, std::string(info.path.hashPart()));
@@ -1254,7 +1257,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
                 printMsg(lvlTalkative, "checking contents of '%s'", printStorePath(i));
 
                 std::unique_ptr<AbstractHashSink> hashSink;
-                if (!info->ca || !info->references.count(info->path))
+                if (!info->ca || !info->hasSelfReference)
                     hashSink = std::make_unique<HashSink>(*info->narHash.type);
                 else
                     hashSink = std::make_unique<HashModuloSink>(*info->narHash.type, std::string(info->path.hashPart()));

src/libstore/misc.cc

@@ -61,7 +61,6 @@ void Store::computeFSClosure(const StorePathSet & startPaths,
                 } else {
 
                     for (auto & ref : info->references)
-                        if (ref != path)
                         enqueue(printStorePath(ref));
 
                     if (includeOutputs && path.isDerivation())
@@ -268,7 +267,7 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths)
         for (auto & i : references)
             /* Don't traverse into paths that don't exist.  That can
                happen due to substitutes for non-existent paths. */
-            if (i != path && paths.count(i))
+            if (paths.count(i))
                 dfsVisit(i, &path);
 
         sorted.push_back(path);

src/libstore/nar-info-disk-cache.cc

@@ -198,7 +198,7 @@ public:
             narInfo->narHash = Hash(queryNAR.getStr(6));
             narInfo->narSize = queryNAR.getInt(7);
             for (auto & r : tokenizeString<Strings>(queryNAR.getStr(8), " "))
-                narInfo->references.insert(StorePath(r));
+                narInfo->insertReferencePossiblyToSelf(StorePath(r));
             if (!queryNAR.isNull(9))
                 narInfo->deriver = StorePath(queryNAR.getStr(9));
             for (auto & sig : tokenizeString<Strings>(queryNAR.getStr(10), " "))

src/libstore/path-info.hh

@@ -24,8 +24,43 @@ template<typename Ref>
 struct PathReferences
 {
     std::set<Ref> 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<Ref> && refs);
 };
 
+template<typename Ref>
+StorePathSet PathReferences<Ref>::referencesPossiblyToSelf(const Ref & self) const
+{
+    StorePathSet references { references };
+    if (hasSelfReference)
+        references.insert(self);
+    return references;
+}
+
+template<typename Ref>
+void PathReferences<Ref>::insertReferencePossiblyToSelf(const Ref & self, Ref && ref)
+{
+    if (ref == self)
+        hasSelfReference = true;
+    else
+        references.insert(std::move(ref));
+}
+
+template<typename Ref>
+void PathReferences<Ref>::setReferencesPossiblyToSelf(const Ref & self, std::set<Ref> && refs)
+{
+    if (refs.count(self))
+        hasSelfReference = true;
+        refs.erase(self);
+
+    references = refs;
+}
+
 struct ValidPathInfo : PathReferences<StorePath>
 {
     StorePath path;
@@ -64,6 +99,7 @@ struct ValidPathInfo : PathReferences<StorePath>
         return
             path == i.path
             && narHash == i.narHash
+            && hasSelfReference == i.hasSelfReference
             && references == i.references;
     }
 
@@ -80,6 +116,12 @@ struct ValidPathInfo : PathReferences<StorePath>
     /* Return true iff the path is verifiably content-addressed. */
     bool isContentAddressed(const Store & store) const;
 
+	/* Functions to view references + hasSelfReference as one set, mainly for
+	   compatibility's sake. */
+	StorePathSet referencesPossiblyToSelf() const;
+    void insertReferencePossiblyToSelf(StorePath && ref);
+    void setReferencesPossiblyToSelf(StorePathSet && refs);
+
     static const size_t maxSigs = std::numeric_limits<size_t>::max();
 
     /* Return the number of signatures on this .narinfo that were
@@ -101,4 +143,14 @@ struct ValidPathInfo : PathReferences<StorePath>
 };
 
 typedef list<ValidPathInfo> ValidPathInfos;
+
+
+struct SubstitutablePathInfo : PathReferences<StorePath>
+{
+    std::optional<StorePath> deriver;
+    unsigned long long downloadSize; /* 0 = unknown or inapplicable */
+    unsigned long long narSize; /* 0 = unknown */
+};
+
+typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
 }

src/libstore/remote-store.cc

@@ -326,7 +326,7 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths,
             auto deriver = readString(conn->from);
             if (deriver != "")
                 info.deriver = parseStorePath(deriver);
-            info.references = readStorePaths<StorePathSet>(*this, conn->from);
+            info.setReferencesPossiblyToSelf(i, readStorePaths<StorePathSet>(*this, conn->from));
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
             infos.insert_or_assign(i, std::move(info));
@@ -339,11 +339,12 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathSet & paths,
         conn.processStderr();
         size_t count = readNum<size_t>(conn->from);
         for (size_t n = 0; n < count; n++) {
-            SubstitutablePathInfo & info(infos[parseStorePath(readString(conn->from))]);
+        	auto path = parseStorePath(readString(conn->from));
+            SubstitutablePathInfo & info { infos[path] };
             auto deriver = readString(conn->from);
             if (deriver != "")
                 info.deriver = parseStorePath(deriver);
-            info.references = readStorePaths<StorePathSet>(*this, conn->from);
+            info.setReferencesPossiblyToSelf(path, readStorePaths<StorePathSet>(*this, conn->from));
             info.downloadSize = readLongLong(conn->from);
             info.narSize = readLongLong(conn->from);
         }
@@ -376,7 +377,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
             auto deriver = readString(conn->from);
             if (deriver != "") info->deriver = parseStorePath(deriver);
             info->narHash = Hash(readString(conn->from), htSHA256);
-            info->references = readStorePaths<StorePathSet>(*this, conn->from);
+            info->setReferencesPossiblyToSelf(readStorePaths<StorePathSet>(*this, conn->from));
             conn->from >> info->registrationTime >> info->narSize;
             if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) {
                 conn->from >> info->ultimate;
@@ -455,7 +456,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source,
         conn.processStderr(0, source2.get());
 
         auto importedPaths = readStorePaths<StorePathSet>(*this, conn->from);
-        assert(importedPaths.size() <= 1);
+        assert(importedPaths.empty() == 0); // doesn't include possible self reference
     }
 
     else {

src/libstore/store-api.cc

@@ -715,7 +715,7 @@ std::optional<ValidPathInfo> decodeValidPathInfo(const Store & store, std::istre
     if (!string2Int(s, n)) throw Error("number expected");
     while (n--) {
         getline(str, s);
-        info.references.insert(store.parseStorePath(s));
+        info.insertReferencePossiblyToSelf(store.parseStorePath(s));
     }
     if (!str || str.eof()) throw Error("missing input");
     return std::optional<ValidPathInfo>(std::move(info));
@@ -738,6 +738,20 @@ string showPaths(const PathSet & paths)
     return concatStringsSep(", ", quoteStrings(paths));
 }
 
+StorePathSet ValidPathInfo::referencesPossiblyToSelf() const
+{
+	return PathReferences<StorePath>::referencesPossiblyToSelf(path);
+}
+
+void ValidPathInfo::insertReferencePossiblyToSelf(StorePath && ref)
+{
+	return PathReferences<StorePath>::insertReferencePossiblyToSelf(path, std::move(ref));
+}
+
+void ValidPathInfo::setReferencesPossiblyToSelf(StorePathSet && refs)
+{
+	return PathReferences<StorePath>::setReferencesPossiblyToSelf(path, std::move(refs));
+}
 
 std::string ValidPathInfo::fingerprint(const Store & store) const
 {
@@ -748,7 +762,7 @@ std::string ValidPathInfo::fingerprint(const Store & store) const
         "1;" + store.printStorePath(path) + ";"
         + narHash.to_string(Base32, true) + ";"
         + std::to_string(narSize) + ";"
-        + concatStringsSep(",", store.printStorePathSet(references));
+        + concatStringsSep(",", store.printStorePathSet(referencesPossiblyToSelf()));
 }
 
 
@@ -767,16 +781,11 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const
 
     auto caPath = std::visit(overloaded {
         [&](TextHash th) {
+            assert(!hasSelfReference);
             return store.makeTextPath(path.name(), th.hash, references);
         },
         [&](FixedOutputHash fsh) {
-            auto refs = references;
+            return store.makeFixedOutputPath(fsh.method, fsh.hash, path.name(), references, hasSelfReference);
-            bool hasSelfReference = false;
-            if (refs.count(path)) {
-                hasSelfReference = true;
-                refs.erase(path);
-            }
-            return store.makeFixedOutputPath(fsh.method, fsh.hash, path.name(), refs, hasSelfReference);
         }
     }, *ca);
 
@@ -810,7 +819,7 @@ bool ValidPathInfo::checkSignature(const Store & store, const PublicKeys & publi
 Strings ValidPathInfo::shortRefs() const
 {
     Strings refs;
-    for (auto & r : references)
+    for (auto & r : referencesPossiblyToSelf())
         refs.push_back(std::string(r.to_string()));
     return refs;
 }

src/libstore/store-api.hh

@@ -102,17 +102,6 @@ struct GCResults
 };
 
 
-struct SubstitutablePathInfo
-{
-    std::optional<StorePath> deriver;
-    StorePathSet references;
-    unsigned long long downloadSize; /* 0 = unknown or inapplicable */
-    unsigned long long narSize; /* 0 = unknown */
-};
-
-typedef std::map<StorePath, SubstitutablePathInfo> SubstitutablePathInfos;
-
-
 enum BuildMode { bmNormal, bmRepair, bmCheck };
 
 

src/nix-store/dotgraph.cc

@@ -58,7 +58,7 @@ void printDotGraph(ref<Store> store, StorePathSet && roots)
 
         cout << makeNode(std::string(path.to_string()), path.name(), "#ff0000");
 
-        for (auto & p : store->queryPathInfo(path)->references) {
+        for (auto & p : store->queryPathInfo(path)->referencesPossiblyToSelf()) {
             if (p != path) {
                 workList.insert(p);
                 cout << makeEdge(std::string(p.to_string()), std::string(path.to_string()));

src/nix-store/graphml.cc

@@ -71,7 +71,7 @@ void printGraphML(ref<Store> store, StorePathSet && roots)
         auto info = store->queryPathInfo(path);
         cout << makeNode(*info);
 
-        for (auto & p : info->references) {
+        for (auto & p : info->referencesPossiblyToSelf()) {
             if (p != path) {
                 workList.insert(p);
                 cout << makeEdge(path.to_string(), p.to_string());

src/nix-store/nix-store.cc

@@ -245,7 +245,7 @@ static void printTree(const StorePath & path,
        closure(B).  That is, if derivation A is an (possibly indirect)
        input of B, then A is printed first.  This has the effect of
        flattening the tree, preventing deeply nested structures.  */
-    auto sorted = store->topoSortPaths(info->references);
+    auto sorted = store->topoSortPaths(info->referencesPossiblyToSelf());
     reverse(sorted.begin(), sorted.end());
 
     for (const auto &[n, i] : enumerate(sorted)) {
@@ -328,7 +328,7 @@ static void opQuery(Strings opFlags, Strings opArgs)
                 for (auto & j : ps) {
                     if (query == qRequisites) store->computeFSClosure(j, paths, false, includeOutputs);
                     else if (query == qReferences) {
-                        for (auto & p : store->queryPathInfo(j)->references)
+                        for (auto & p : store->queryPathInfo(j)->referencesPossiblyToSelf())
                             paths.insert(p);
                     }
                     else if (query == qReferrers) {
@@ -859,7 +859,7 @@ static void opServe(Strings opFlags, Strings opArgs)
                         auto info = store->queryPathInfo(i);
                         out << store->printStorePath(info->path)
                             << (info->deriver ? store->printStorePath(*info->deriver) : "");
-                        writeStorePaths(*store, out, info->references);
+                        writeStorePaths(*store, out, info->referencesPossiblyToSelf());
                         // !!! Maybe we want compression?
                         out << info->narSize // downloadSize
                             << info->narSize;
@@ -949,7 +949,7 @@ static void opServe(Strings opFlags, Strings opArgs)
                 if (deriver != "")
                     info.deriver = store->parseStorePath(deriver);
                 info.narHash = Hash(readString(in), htSHA256);
-                info.references = readStorePaths<StorePathSet>(*store, in);
+                info.setReferencesPossiblyToSelf(readStorePaths<StorePathSet>(*store, in));
                 in >> info.registrationTime >> info.narSize >> info.ultimate;
                 info.sigs = readStrings<StringSet>(in);
                 info.ca = parseContentAddressOpt(readString(in));

src/nix/make-content-addressable.cc

@@ -79,7 +79,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON
 
             ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference));
             info.references = std::move(references);
-            if (hasSelfReference) info.references.insert(info.path);
+            info.hasSelfReference = std::move(hasSelfReference);
             info.narHash = narHash;
             info.narSize = sink.s->size();
             info.ca = FixedOutputHash {

src/nix/sigs.cc

@@ -65,7 +65,8 @@ struct CmdCopySigs : StorePathsCommand
                        binary. */
                     if (info->narHash != info2->narHash ||
                         info->narSize != info2->narSize ||
-                        info->references != info2->references)
+                        info->references != info2->references ||
+                        info->hasSelfReference != info2->hasSelfReference)
                         continue;
 
                     for (auto & sig : info2->sigs)