Merge pull request #13715 from xokdvium/hash-result-struct

libutil: Make HashResult a proper struct

src/libfetchers/git.cc

@@ -901,7 +901,7 @@ struct GitInputScheme : InputScheme
                     writeString(file.abs(), hashSink);
                 }
                 return makeFingerprint(*repoInfo.workdirInfo.headRev)
-                       + ";d=" + hashSink.finish().first.to_string(HashFormat::Base16, false);
+                       + ";d=" + hashSink.finish().hash.to_string(HashFormat::Base16, false);
             }
             return std::nullopt;
         }

src/libstore/binary-cache-store.cc

@@ -368,16 +368,16 @@ StorePath BinaryCacheStore::addToStoreFromDump(
                        name,
                        ContentAddressWithReferences::fromParts(
                            hashMethod,
-                           caHash ? *caHash : nar.first,
+                           caHash ? *caHash : nar.hash,
                            {
                                .others = references,
                                // caller is not capable of creating a self-reference, because this is content-addressed
                                // without modulus
                                .self = false,
                            }),
-                       nar.first,
+                       nar.hash,
                    };
-                   info.narSize = nar.second;
+                   info.narSize = nar.numBytesDigested;
                    return info;
                })
         ->path;
@@ -493,9 +493,9 @@ StorePath BinaryCacheStore::addToStore(
                                // without modulus
                                .self = false,
                            }),
-                       nar.first,
+                       nar.hash,
                    };
-                   info.narSize = nar.second;
+                   info.narSize = nar.numBytesDigested;
                    return info;
                })
         ->path;

src/libstore/export-import.cc

@@ -33,7 +33,7 @@ void Store::exportPath(const StorePath & path, Sink & sink)
     /* Refuse to export paths that have changed.  This prevents
        filesystem corruption from spreading to other machines.
        Don't complain if the stored hash is zero (unknown). */
-    Hash hash = hashSink.currentHash().first;
+    Hash hash = hashSink.currentHash().hash;
     if (hash != info->narHash && info->narHash != Hash(info->narHash.algo))
         throw Error(
             "hash of path '%s' has changed from '%s' to '%s'!",

src/libstore/local-store.cc

@@ -1072,19 +1072,19 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, RepairF
 
                 auto hashResult = hashSink.finish();
 
-                if (hashResult.first != info.narHash)
+                if (hashResult.hash != info.narHash)
                     throw Error(
                         "hash mismatch importing path '%s';\n  specified: %s\n  got:       %s",
                         printStorePath(info.path),
                         info.narHash.to_string(HashFormat::Nix32, true),
-                        hashResult.first.to_string(HashFormat::Nix32, true));
+                        hashResult.hash.to_string(HashFormat::Nix32, true));
 
-                if (hashResult.second != info.narSize)
+                if (hashResult.numBytesDigested != info.narSize)
                     throw Error(
                         "size mismatch importing path '%s';\n  specified: %s\n  got:       %s",
                         printStorePath(info.path),
                         info.narSize,
-                        hashResult.second);
+                        hashResult.numBytesDigested);
 
                 if (info.ca) {
                     auto & specified = *info.ca;
@@ -1101,7 +1101,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, RepairF
                                 std::string{info.path.hashPart()},
                             };
                             dumpPath({accessor, path}, caSink, (FileSerialisationMethod) fim);
-                            h = caSink.finish().first;
+                            h = caSink.finish().hash;
                             break;
                         }
                         case FileIngestionMethod::Git:
@@ -1279,7 +1279,7 @@ StorePath LocalStore::addToStoreFromDump(
 
             /* For computing the nar hash. In recursive SHA-256 mode, this
                is the same as the store hash, so no need to do it again. */
-            auto narHash = std::pair{dumpHash, size};
+            HashResult narHash = {dumpHash, size};
             if (dumpMethod != FileSerialisationMethod::NixArchive || hashAlgo != HashAlgorithm::SHA256) {
                 HashSink narSink{HashAlgorithm::SHA256};
                 dumpPath(realPath, narSink);
@@ -1295,8 +1295,8 @@ StorePath LocalStore::addToStoreFromDump(
                 syncParent(realPath);
             }
 
-            ValidPathInfo info{*this, name, std::move(desc), narHash.first};
+            ValidPathInfo info{*this, name, std::move(desc), narHash.hash};
-            info.narSize = narHash.second;
+            info.narSize = narHash.numBytesDigested;
             registerValidPath(info);
         }
 
@@ -1402,12 +1402,12 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
                 dumpPath(Store::toRealPath(i), hashSink);
                 auto current = hashSink.finish();
 
-                if (info->narHash != nullHash && info->narHash != current.first) {
+                if (info->narHash != nullHash && info->narHash != current.hash) {
                     printError(
                         "path '%s' was modified! expected hash '%s', got '%s'",
                         printStorePath(i),
                         info->narHash.to_string(HashFormat::Nix32, true),
-                        current.first.to_string(HashFormat::Nix32, true));
+                        current.hash.to_string(HashFormat::Nix32, true));
                     if (repair)
                         repairPath(i);
                     else
@@ -1419,14 +1419,14 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
                     /* Fill in missing hashes. */
                     if (info->narHash == nullHash) {
                         printInfo("fixing missing hash on '%s'", printStorePath(i));
-                        info->narHash = current.first;
+                        info->narHash = current.hash;
                         update = true;
                     }
 
                     /* Fill in missing narSize fields (from old stores). */
                     if (info->narSize == 0) {
-                        printInfo("updating size field on '%s' to %s", printStorePath(i), current.second);
+                        printInfo("updating size field on '%s' to %s", printStorePath(i), current.numBytesDigested);
-                        info->narSize = current.second;
+                        info->narSize = current.numBytesDigested;
                         update = true;
                     }
 

src/libstore/make-content-addressed.cc

@@ -43,7 +43,7 @@ std::map<StorePath, StorePath> makeContentAddressed(Store & srcStore, Store & ds
         HashModuloSink hashModuloSink(HashAlgorithm::SHA256, oldHashPart);
         hashModuloSink(sink.s);
 
-        auto narModuloHash = hashModuloSink.finish().first;
+        auto narModuloHash = hashModuloSink.finish().hash;
 
         ValidPathInfo info{
             dstStore,

src/libstore/optimise-store.cc

@@ -160,7 +160,7 @@ void LocalStore::optimisePath_(
             {make_ref<PosixSourceAccessor>(), CanonPath(path)},
             FileSerialisationMethod::NixArchive,
             HashAlgorithm::SHA256)
-            .first;
+            .hash;
     });
     debug("'%1%' has hash '%2%'", path, hash.to_string(HashFormat::Nix32, true));
 
@@ -175,7 +175,7 @@ void LocalStore::optimisePath_(
                                                                PosixSourceAccessor::createAtRoot(linkPath),
                                                                FileSerialisationMethod::NixArchive,
                                                                HashAlgorithm::SHA256)
-                                                               .first;
+                                                               .hash;
                                                        }))) {
             // XXX: Consider overwriting linkPath with our valid version.
             warn("removing corrupted link %s", linkPath);

src/libstore/store-api.cc

@@ -254,7 +254,7 @@ ValidPathInfo Store::addToStoreSlow(
 
     auto hash = method == ContentAddressMethod::Raw::NixArchive && hashAlgo == HashAlgorithm::SHA256 ? narHash
                 : method == ContentAddressMethod::Raw::Git ? git::dumpHash(hashAlgo, srcPath).hash
-                                                           : caHashSink.finish().first;
+                                                           : caHashSink.finish().hash;
 
     if (expectedCAHash && expectedCAHash != hash)
         throw Error("hash mismatch for '%s'", srcPath);
@@ -1035,8 +1035,8 @@ decodeValidPathInfo(const Store & store, std::istream & str, std::optional<HashR
             throw Error("number expected");
         hashGiven = {narHash, *narSize};
     }
-    ValidPathInfo info(store.parseStorePath(path), hashGiven->first);
+    ValidPathInfo info(store.parseStorePath(path), hashGiven->hash);
-    info.narSize = hashGiven->second;
+    info.narSize = hashGiven->numBytesDigested;
     std::string deriver;
     getline(str, deriver);
     if (deriver != "")

src/libstore/unix/build/derivation-builder.cc

@@ -1676,7 +1676,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                     HashModuloSink caSink{outputHash.hashAlgo, oldHashPart};
                     auto fim = outputHash.method.getFileIngestionMethod();
                     dumpPath({getFSSourceAccessor(), CanonPath(actualPath)}, caSink, (FileSerialisationMethod) fim);
-                    return caSink.finish().first;
+                    return caSink.finish().hash;
                 }
                 case FileIngestionMethod::Git: {
                     return git::dumpHash(outputHash.hashAlgo, {getFSSourceAccessor(), CanonPath(actualPath)}).hash;
@@ -1705,8 +1705,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                     {getFSSourceAccessor(), CanonPath(actualPath)},
                     FileSerialisationMethod::NixArchive,
                     HashAlgorithm::SHA256);
-                newInfo0.narHash = narHashAndSize.first;
+                newInfo0.narHash = narHashAndSize.hash;
-                newInfo0.narSize = narHashAndSize.second;
+                newInfo0.narSize = narHashAndSize.numBytesDigested;
             }
 
             assert(newInfo0.ca);
@@ -1729,8 +1729,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
                         {getFSSourceAccessor(), CanonPath(actualPath)},
                         FileSerialisationMethod::NixArchive,
                         HashAlgorithm::SHA256);
-                    ValidPathInfo newInfo0{requiredFinalPath, narHashAndSize.first};
+                    ValidPathInfo newInfo0{requiredFinalPath, narHashAndSize.hash};
-                    newInfo0.narSize = narHashAndSize.second;
+                    newInfo0.narSize = narHashAndSize.numBytesDigested;
                     auto refs = rewriteRefs();
                     newInfo0.references = std::move(refs.others);
                     if (refs.self)

src/libutil-tests/git.cc

@@ -270,7 +270,7 @@ TEST_F(GitTest, both_roundrip)
             HashSink hashSink{hashAlgo};
             TeeSink s2{s, hashSink};
             auto mode = dump(path, s2, dumpHook, defaultPathFilter, mockXpSettings);
-            auto hash = hashSink.finish().first;
+            auto hash = hashSink.finish().hash;
             cas.insert_or_assign(hash, std::move(s.s));
             return TreeEntry{
                 .mode = mode,

src/libutil/file-content-address.cc

@@ -101,7 +101,7 @@ hashPath(const SourcePath & path, FileIngestionMethod method, HashAlgorithm ht,
     case FileIngestionMethod::Flat:
     case FileIngestionMethod::NixArchive: {
         auto res = hashPath(path, (FileSerialisationMethod) method, ht, filter);
-        return {res.first, {res.second}};
+        return {res.hash, res.numBytesDigested};
     }
     case FileIngestionMethod::Git:
         return {git::dumpHash(ht, path, filter).hash, std::nullopt};

src/libutil/git.cc

@@ -329,7 +329,7 @@ TreeEntry dumpHash(HashAlgorithm ha, const SourcePath & path, PathFilter & filte
     hook = [&](const SourcePath & path) -> TreeEntry {
         auto hashSink = HashSink(ha);
         auto mode = dump(path, hashSink, hook, filter);
-        auto hash = hashSink.finish().first;
+        auto hash = hashSink.finish().hash;
         return {
             .mode = mode,
             .hash = hash,

src/libutil/hash.cc

@@ -338,7 +338,7 @@ Hash hashFile(HashAlgorithm ha, const Path & path)
 {
     HashSink sink(ha);
     readFile(path, sink);
-    return sink.finish().first;
+    return sink.finish().hash;
 }
 
 HashSink::HashSink(HashAlgorithm ha)

src/libutil/include/nix/util/hash.hh

@@ -153,10 +153,12 @@ Hash hashFile(HashAlgorithm ha, const Path & path);
 
 /**
  * The final hash and the number of bytes digested.
- *
- * @todo Convert to proper struct
  */
-typedef std::pair<Hash, uint64_t> HashResult;
+struct HashResult
+{
+    Hash hash;
+    uint64_t numBytesDigested;
+};
 
 /**
  * Compress a hash to the specified number of bytes by cyclically

src/libutil/references.cc

@@ -120,7 +120,7 @@ HashResult HashModuloSink::finish()
         hashSink(fmt("|%d", pos));
 
     auto h = hashSink.finish();
-    return {h.first, rewritingSink.pos};
+    return {.hash = h.hash, .numBytesDigested = rewritingSink.pos};
 }
 
 } // namespace nix

src/libutil/source-accessor.cc

@@ -65,7 +65,7 @@ Hash SourceAccessor::hashPath(const CanonPath & path, PathFilter & filter, HashA
 {
     HashSink sink(ha);
     dumpPath(path, sink, filter);
-    return sink.finish().first;
+    return sink.finish().hash;
 }
 
 SourceAccessor::Stat SourceAccessor::lstat(const CanonPath & path)

src/nix/hash.cc

@@ -100,14 +100,14 @@ struct CmdHashBase : Command
                 // so we don't need to go low-level, or reject symlink `path`s.
                 auto hashSink = makeSink();
                 readFile(path, *hashSink);
-                h = hashSink->finish().first;
+                h = hashSink->finish().hash;
                 break;
             }
             case FileIngestionMethod::NixArchive: {
                 auto sourcePath = makeSourcePath();
                 auto hashSink = makeSink();
                 dumpPath(sourcePath, *hashSink, (FileSerialisationMethod) mode);
-                h = hashSink->finish().first;
+                h = hashSink->finish().hash;
                 break;
             }
             case FileIngestionMethod::Git: {
@@ -116,7 +116,7 @@ struct CmdHashBase : Command
                 hook = [&](const SourcePath & path) -> git::TreeEntry {
                     auto hashSink = makeSink();
                     auto mode = dump(path, *hashSink, hook);
-                    auto hash = hashSink->finish().first;
+                    auto hash = hashSink->finish().hash;
                     return {
                         .mode = mode,
                         .hash = hash,

src/nix/nix-store/nix-store.cc

@@ -582,7 +582,11 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
     while (1) {
         // 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 hashResultOpt = !hashGiven ? std::optional<HashResult>{{
+                                              Hash::dummy,
+                                              std::numeric_limits<uint64_t>::max(),
+                                          }}
+                                        : std::nullopt;
         auto info = decodeValidPathInfo(*store, cin, hashResultOpt);
         if (!info)
             break;
@@ -599,8 +603,8 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
                     {store->getFSAccessor(false), CanonPath{info->path.to_string()}},
                     FileSerialisationMethod::NixArchive,
                     HashAlgorithm::SHA256);
-                info->narHash = hash.first;
+                info->narHash = hash.hash;
-                info->narSize = hash.second;
+                info->narSize = hash.numBytesDigested;
             }
             infos.insert_or_assign(info->path, *info);
         }
@@ -836,12 +840,12 @@ static void opVerifyPath(Strings opFlags, Strings opArgs)
         HashSink sink(info->narHash.algo);
         store->narFromPath(path, sink);
         auto current = sink.finish();
-        if (current.first != info->narHash) {
+        if (current.hash != info->narHash) {
             printError(
                 "path '%s' was modified! expected hash '%s', got '%s'",
                 store->printStorePath(path),
                 info->narHash.to_string(HashFormat::Nix32, true),
-                current.first.to_string(HashFormat::Nix32, true));
+                current.hash.to_string(HashFormat::Nix32, true));
             status = 1;
         }
     }

src/nix/verify.cc

@@ -103,14 +103,14 @@ struct CmdVerify : StorePathsCommand
 
                     auto hash = hashSink.finish();
 
-                    if (hash.first != info->narHash) {
+                    if (hash.hash != info->narHash) {
                         corrupted++;
                         act2.result(resCorruptedPath, store->printStorePath(info->path));
                         printError(
                             "path '%s' was modified! expected hash '%s', got '%s'",
                             store->printStorePath(info->path),
                             info->narHash.to_string(HashFormat::Nix32, true),
-                            hash.first.to_string(HashFormat::Nix32, true));
+                            hash.hash.to_string(HashFormat::Nix32, true));
                     }
                 }