Minimize the usage of Hash::dummy

NixOS · Aug 6, 2020 · e89b5bd · e89b5bd
1 parent 5e59b25
commit e89b5bd
Show file tree

Hide file tree

Showing 18 changed files with 101 additions and 58 deletions.
diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc
@@ -67,8 +67,10 @@ DownloadFileResult downloadFile(
         StringSink sink;
         dumpString(*res.data, sink);
         auto hash = hashString(htSHA256, *res.data);
-        ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name));
-        info.narHash = hashString(htSHA256, *sink.s);
+        ValidPathInfo info {
+            store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name),
+            hashString(htSHA256, *sink.s),
+        };
         info.narSize = sink.s->size();
         info.ca = FixedOutputHash {
             .method = FileIngestionMethod::Flat,

diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
@@ -385,7 +385,10 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath
         h = hashString(hashAlgo, s);
     }
 
-    ValidPathInfo info(makeFixedOutputPath(method, *h, name));
+    ValidPathInfo info {
+        makeFixedOutputPath(method, *h, name),
+        Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash
+    };
 
     auto source = StringSource { *sink.s };
     addToStore(info, source, repair, CheckSigs);
@@ -396,7 +399,10 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath
 StorePath BinaryCacheStore::addTextToStore(const string & name, const string & s,
     const StorePathSet & references, RepairFlag repair)
 {
-    ValidPathInfo info(computeStorePathForText(name, s, references));
+    ValidPathInfo info {
+        computeStorePathForText(name, s, references),
+        Hash::dummy, // Will be fixed in addToStore, which recomputes nar hash
+    };
     info.references = references;
 
     if (repair || !isValidPath(info.path)) {

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
@@ -3864,8 +3864,10 @@ void DerivationGoal::registerOutputs()
             worker.markContentsGood(worker.store.parseStorePath(path));
         }
 
-        ValidPathInfo info(worker.store.parseStorePath(path));
-        info.narHash = hash.first;
+        ValidPathInfo info {
+            worker.store.parseStorePath(path),
+            hash.first,
+        };
         info.narSize = hash.second;
         info.references = std::move(references);
         info.deriver = drvPath;

diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc
@@ -694,11 +694,12 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
 
     case wopAddToStoreNar: {
         bool repair, dontCheckSigs;
-        ValidPathInfo info(store->parseStorePath(readString(from)));
+        auto path = store->parseStorePath(readString(from));
         auto deriver = readString(from);
+        auto narHash = Hash::parseAny(readString(from), htSHA256);
+        ValidPathInfo info { path, narHash };
         if (deriver != "")
             info.deriver = store->parseStorePath(deriver);
-        info.narHash = Hash::parseAny(readString(from), htSHA256);
         info.references = readStorePaths<StorePathSet>(*store, from);
         from >> info.registrationTime >> info.narSize >> info.ultimate;
         info.sigs = readStrings<StringSet>(from);

diff --git a/src/libstore/export-import.cc b/src/libstore/export-import.cc
@@ -69,17 +69,18 @@ StorePaths Store::importPaths(Source & source, CheckSigsFlag checkSigs)
         if (magic != exportMagic)
             throw Error("Nix archive cannot be imported; wrong format");
 
-        ValidPathInfo info(parseStorePath(readString(source)));
+        auto path = parseStorePath(readString(source));
 
         //Activity act(*logger, lvlInfo, format("importing path '%s'") % info.path);
 
-        info.references = readStorePaths<StorePathSet>(*this, source);
-
+        auto references = readStorePaths<StorePathSet>(*this, source);
         auto deriver = readString(source);
+        auto narHash = hashString(htSHA256, *saved.s);
+
+        ValidPathInfo info { path, narHash };
         if (deriver != "")
             info.deriver = parseStorePath(deriver);
-
-        info.narHash = hashString(htSHA256, *saved.s);
+        info.references = references;
         info.narSize = saved.s->size();
 
         // Ignore optional legacy signature.

diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc
@@ -93,15 +93,20 @@ struct LegacySSHStore : public Store
         try {
             auto conn(connections->get());
 
+            /* No longer support missing NAR hash */
+            assert(GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4);
+
             debug("querying remote host '%s' for info on '%s'", host, printStorePath(path));
 
             conn->to << cmdQueryPathInfos << PathSet{printStorePath(path)};
             conn->to.flush();
 
             auto p = readString(conn->from);
             if (p.empty()) return callback(nullptr);
-            auto info = std::make_shared<ValidPathInfo>(parseStorePath(p));
-            assert(path == info->path);
+            auto path2 = parseStorePath(p);
+            assert(path == path2);
+            /* Hash will be set below. FIXME construct ValidPathInfo at end. */
+            auto info = std::make_shared<ValidPathInfo>(path, Hash::dummy);
 
             PathSet references;
             auto deriver = readString(conn->from);
@@ -111,12 +116,14 @@ struct LegacySSHStore : public Store
             readLongLong(conn->from); // download size
             info->narSize = readLongLong(conn->from);
 
-            if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 4) {
+            {
                 auto s = readString(conn->from);
-                info->narHash = s.empty() ? Hash::dummy : Hash::parseAnyPrefixed(s);
-                info->ca = parseContentAddressOpt(readString(conn->from));
-                info->sigs = readStrings<StringSet>(conn->from);
+                if (s == "")
+                    throw Error("NAR hash is now mandatory");
+                info->narHash = Hash::parseAnyPrefixed(s);
             }
+            info->ca = parseContentAddressOpt(readString(conn->from));
+            info->sigs = readStrings<StringSet>(conn->from);
 
             auto s = readString(conn->from);
             assert(s == "");

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
@@ -641,25 +641,28 @@ void LocalStore::queryPathInfoUncached(const StorePath & path,
     Callback<std::shared_ptr<const ValidPathInfo>> callback) noexcept
 {
     try {
-        auto info = std::make_shared<ValidPathInfo>(path);
-
         callback(retrySQLite<std::shared_ptr<ValidPathInfo>>([&]() {
             auto state(_state.lock());
 
             /* Get the path info. */
-            auto useQueryPathInfo(state->stmtQueryPathInfo.use()(printStorePath(info->path)));
+            auto useQueryPathInfo(state->stmtQueryPathInfo.use()(printStorePath(path)));
 
             if (!useQueryPathInfo.next())
                 return std::shared_ptr<ValidPathInfo>();
 
-            info->id = useQueryPathInfo.getInt(0);
+            auto id = useQueryPathInfo.getInt(0);
 
+            auto narHash = Hash::dummy;
             try {
-                info->narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1));
+                narHash = Hash::parseAnyPrefixed(useQueryPathInfo.getStr(1));
             } catch (BadHash & e) {
-                throw Error("in valid-path entry for '%s': %s", printStorePath(path), e.what());
+                throw Error("invalid-path entry for '%s': %s", printStorePath(path), e.what());
             }
 
+            auto info = std::make_shared<ValidPathInfo>(path, narHash);
+
+            info->id = id;
+
             info->registrationTime = useQueryPathInfo.getInt(2);
 
             auto s = (const char *) sqlite3_column_text(state->stmtQueryPathInfo, 3);
@@ -1152,8 +1155,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name,
 
             optimisePath(realPath);
 
-            ValidPathInfo info(dstPath);
-            info.narHash = narHash.first;
+            ValidPathInfo info { dstPath, narHash.first };
             info.narSize = narHash.second;
             info.ca = FixedOutputHash { .method = method, .hash = hash };
             registerValidPath(info);
@@ -1196,8 +1198,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s,
 
             optimisePath(realPath);
 
-            ValidPathInfo info(dstPath);
-            info.narHash = narHash;
+            ValidPathInfo info { dstPath, narHash };
             info.narSize = sink.s->size();
             info.references = references;
             info.ca = TextHash { .hash = hash };

diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc
@@ -189,13 +189,14 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache
                 return {oInvalid, 0};
 
             auto namePart = queryNAR.getStr(1);
-            auto narInfo = make_ref<NarInfo>(StorePath(hashPart + "-" + namePart));
+            auto narInfo = make_ref<NarInfo>(
+                StorePath(hashPart + "-" + namePart),
+                Hash::parseAnyPrefixed(queryNAR.getStr(6)));
             narInfo->url = queryNAR.getStr(2);
             narInfo->compression = queryNAR.getStr(3);
             if (!queryNAR.isNull(4))
                 narInfo->fileHash = Hash::parseAnyPrefixed(queryNAR.getStr(4));
             narInfo->fileSize = queryNAR.getInt(5);
-            narInfo->narHash = Hash::parseAnyPrefixed(queryNAR.getStr(6));
             narInfo->narSize = queryNAR.getInt(7);
             for (auto & r : tokenizeString<Strings>(queryNAR.getStr(8), " "))
                 narInfo->references.insert(StorePath(r));

diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc
@@ -1,10 +1,11 @@
 #include "globals.hh"
 #include "nar-info.hh"
+#include "store-api.hh"
 
 namespace nix {
 
 NarInfo::NarInfo(const Store & store, const std::string & s, const std::string & whence)
-    : ValidPathInfo(StorePath(StorePath::dummy)) // FIXME: hack
+    : ValidPathInfo(StorePath(StorePath::dummy), Hash(Hash::dummy)) // FIXME: hack
 {
     auto corrupt = [&]() {
         return Error("NAR info file '%1%' is corrupt", whence);
@@ -19,6 +20,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string &
     };
 
     bool havePath = false;
+    bool haveNarHash = false;
 
     size_t pos = 0;
     while (pos < s.size()) {
@@ -46,8 +48,10 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string &
         else if (name == "FileSize") {
             if (!string2Int(value, fileSize)) throw corrupt();
         }
-        else if (name == "NarHash")
+        else if (name == "NarHash") {
             narHash = parseHashField(value);
+            haveNarHash = true;
+        }
         else if (name == "NarSize") {
             if (!string2Int(value, narSize)) throw corrupt();
         }
@@ -76,7 +80,7 @@ NarInfo::NarInfo(const Store & store, const std::string & s, const std::string &
 
     if (compression == "") compression = "bzip2";
 
-    if (!havePath || url.empty() || narSize == 0) throw corrupt();
+    if (!havePath || !haveNarHash || url.empty() || narSize == 0) throw corrupt();
 }
 
 std::string NarInfo::to_string(const Store & store) const

diff --git a/src/libstore/nar-info.hh b/src/libstore/nar-info.hh
@@ -2,10 +2,12 @@
 
 #include "types.hh"
 #include "hash.hh"
-#include "store-api.hh"
+#include "path-info.hh"
 
 namespace nix {
 
+class Store;
+
 struct NarInfo : ValidPathInfo
 {
     std::string url;
@@ -15,7 +17,7 @@ struct NarInfo : ValidPathInfo
     std::string system;
 
     NarInfo() = delete;
-    NarInfo(StorePath && path) : ValidPathInfo(std::move(path)) { }
+    NarInfo(StorePath && path, Hash narHash) : ValidPathInfo(std::move(path), narHash) { }
     NarInfo(const ValidPathInfo & info) : ValidPathInfo(info) { }
     NarInfo(const Store & store, const std::string & s, const std::string & whence);
 

diff --git a/src/libstore/path-info.hh b/src/libstore/path-info.hh
@@ -1,5 +1,6 @@
 #pragma once
 
+#include "crypto.hh"
 #include "path.hh"
 #include "hash.hh"
 #include "content-address.hh"
@@ -100,8 +101,8 @@ struct ValidPathInfo
 
     ValidPathInfo(const ValidPathInfo & other) = default;
 
-    ValidPathInfo(StorePath && path) : path(std::move(path)), narHash(Hash::dummy) { };
-    ValidPathInfo(const StorePath & path) : path(path), narHash(Hash::dummy) { };
+    ValidPathInfo(StorePath && path, Hash narHash) : path(std::move(path)), narHash(narHash) { };
+    ValidPathInfo(const StorePath & path, Hash narHash) : path(path), narHash(narHash) { };
 
     virtual ~ValidPathInfo() { }
 };

diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc
@@ -419,10 +419,10 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path,
                 bool valid; conn->from >> valid;
                 if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path));
             }
-            info = std::make_shared<ValidPathInfo>(StorePath(path));
             auto deriver = readString(conn->from);
+            auto narHash = Hash::parseAny(readString(conn->from), htSHA256);
+            info = std::make_shared<ValidPathInfo>(path, narHash);
             if (deriver != "") info->deriver = parseStorePath(deriver);
-            info->narHash = Hash::parseAny(readString(conn->from), htSHA256);
             info->references = readStorePaths<StorePathSet>(*this, conn->from);
             conn->from >> info->registrationTime >> info->narSize;
             if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) {

diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
@@ -320,8 +320,10 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
     if (expectedCAHash && expectedCAHash != hash)
         throw Error("hash mismatch for '%s'", srcPath);
 
-    ValidPathInfo info(makeFixedOutputPath(method, hash, name));
-    info.narHash = narHash;
+    ValidPathInfo info {
+        makeFixedOutputPath(method, hash, name),
+        narHash,
+    };
     info.narSize = narSize;
     info.ca = FixedOutputHash { .method = method, .hash = hash };
 
@@ -849,19 +851,22 @@ void copyClosure(ref<Store> srcStore, ref<Store> dstStore,
 }
 
 
-std::optional<ValidPathInfo> decodeValidPathInfo(const Store & store, std::istream & str, bool hashGiven)
+std::optional<ValidPathInfo> decodeValidPathInfo(const Store & store, std::istream & str, std::optional<HashResult> hashGiven)
 {
     std::string path;
     getline(str, path);
     if (str.eof()) { return {}; }
-    ValidPathInfo info(store.parseStorePath(path));
-    if (hashGiven) {
+    if (!hashGiven) {
         string s;
         getline(str, s);
-        info.narHash = Hash::parseAny(s, htSHA256);
+        auto narHash = Hash::parseAny(s, htSHA256);
         getline(str, s);
-        if (!string2Int(s, info.narSize)) throw Error("number expected");
+        uint64_t narSize;
+        if (!string2Int(s, narSize)) throw Error("number expected");
+        hashGiven = { narHash, narSize };
     }
+    ValidPathInfo info(store.parseStorePath(path), hashGiven->first);
+    info.narSize = hashGiven->second;
     std::string deriver;
     getline(str, deriver);
     if (deriver != "") info.deriver = store.parseStorePath(deriver);

diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
@@ -4,7 +4,6 @@
 #include "hash.hh"
 #include "content-address.hh"
 #include "serialise.hh"
-#include "crypto.hh"
 #include "lru-cache.hh"
 #include "sync.hh"
 #include "globals.hh"
@@ -761,7 +760,7 @@ string showPaths(const PathSet & paths);
 std::optional<ValidPathInfo> decodeValidPathInfo(
     const Store & store,
     std::istream & str,
-    bool hashGiven = false);
+    std::optional<HashResult> hashGiven = std::nullopt);
 
 /* Split URI into protocol+hierarchy part and its parameter set. */
 std::pair<std::string, Store::Params> splitUriAndParams(const std::string & uri);

diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
@@ -495,7 +495,10 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
     ValidPathInfos infos;
 
     while (1) {
-        auto info = decodeValidPathInfo(*store, cin, hashGiven);
+        // We use a dummy value because we'll set it below. FIXME be correct by
+        // construction and avoid dummy value.
+        auto hashResultOpt = !hashGiven ? std::optional<HashResult> { {Hash::dummy, -1} } : std::nullopt;
+        auto info = decodeValidPathInfo(*store, cin, hashResultOpt);
         if (!info) break;
         if (!store->isValidPath(info->path) || reregister) {
             /* !!! races */
@@ -944,11 +947,13 @@ static void opServe(Strings opFlags, Strings opArgs)
                 if (!writeAllowed) throw Error("importing paths is not allowed");
 
                 auto path = readString(in);
-                ValidPathInfo info(store->parseStorePath(path));
                 auto deriver = readString(in);
+                ValidPathInfo info {
+                    store->parseStorePath(path),
+                    Hash::parseAny(readString(in), htSHA256),
+                };
                 if (deriver != "")
                     info.deriver = store->parseStorePath(deriver);
-                info.narHash = Hash::parseAny(readString(in), htSHA256);
                 info.references = readStorePaths<StorePathSet>(*store, in);
                 in >> info.registrationTime >> info.narSize >> info.ultimate;
                 info.sigs = readStrings<StringSet>(in);

diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc
@@ -60,8 +60,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand
             hash = hsink.finish().first;
         }
 
-        ValidPathInfo info(store->makeFixedOutputPath(ingestionMethod, hash, *namePart));
-        info.narHash = narHash;
+        ValidPathInfo info {
+            store->makeFixedOutputPath(ingestionMethod, hash, *namePart),
+            narHash,
+        };
         info.narSize = sink.s->size();
         info.ca = std::optional { FixedOutputHash {
             .method = ingestionMethod,