From 9dfa20a22ae0be3d3b01a7b3d422af97428c627e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= <ludo@gnu.org>
Date: Sun, 13 Dec 2020 22:46:03 +0100
Subject: daemon: Let 'guix substitute' perform hash checks.

This way, the hash of the store item can be computed as it is restored,
thereby avoiding an additional file tree traversal ('hashPath' call)
later on in the daemon.  Consequently, it should reduce latency between
subsequent substitute downloads.

This is a followup to 5ff521452b9ec2aae9ed8e4bb7bdc250a581f203.

* guix/scripts/substitute.scm (narinfo-hash-algorithm+value): New
procedure.
(process-substitution): Wrap INPUT into a hash input port, 'hashed', and
read from it.  Compare the actual and expected hashes, and print a
"hash-mismatch" status line when they differ.  When they match, print
not just "success" but also the nar hash and size.
* nix/libstore/build.cc (class SubstitutionGoal)[expectedHashStr]:
Remove.
(SubstitutionGoal::finished): Tokenize 'status'.  Parse it and handle
"success" and "hash-mismatch" accordingly.  Call 'hashPath' only when
the returned hash is not SHA256.
(SubstitutionGoal::handleChildOutput): Remove 'expectedHashStr'
handling.
* tests/substitute.scm ("substitute, invalid hash"): Rename to...
("substitute, invalid narinfo hash"): ... this.
("substitute, invalid hash"): New test.
---
 nix/libstore/build.cc | 73 +++++++++++++++++++++++++++------------------------
 1 file changed, 39 insertions(+), 34 deletions(-)

(limited to 'nix')

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index b5551b87ae..b19471a68f 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2790,10 +2790,6 @@ private:
     /* The substituter. */
     std::shared_ptr<Agent> substituter;
 
-    /* Either the empty string, or the expected hash as returned by the
-       substituter.  */
-    string expectedHashStr;
-
     /* Either the empty string, or the status phrase returned by the
        substituter.  */
     string status;
@@ -3032,36 +3028,47 @@ void SubstitutionGoal::finished()
     /* Check the exit status and the build result. */
     HashResult hash;
     try {
-
-        if (status != "success")
-            throw SubstError(format("fetching path `%1%' (status: '%2%')")
-                % storePath % status);
-
-        if (!pathExists(destPath))
-            throw SubstError(format("substitute did not produce path `%1%'") % destPath);
-
-	if (expectedHashStr == "")
-	    throw SubstError(format("substituter did not communicate hash for `%1'") % storePath);
-
-        hash = hashPath(htSHA256, destPath);
-
-        /* Verify the expected hash we got from the substituer. */
-	size_t n = expectedHashStr.find(':');
-	if (n == string::npos)
-	    throw Error(format("bad hash from substituter: %1%") % expectedHashStr);
-	HashType hashType = parseHashType(string(expectedHashStr, 0, n));
-	if (hashType == htUnknown)
-	    throw Error(format("unknown hash algorithm in `%1%'") % expectedHashStr);
-	Hash expectedHash = parseHash16or32(hashType, string(expectedHashStr, n + 1));
-	Hash actualHash = hashType == htSHA256 ? hash.first : hashPath(hashType, destPath).first;
-	if (expectedHash != actualHash) {
-	    if (settings.printBuildTrace)
+	auto statusList = tokenizeString<vector<string> >(status);
+
+	if (statusList.empty()) {
+            throw SubstError(format("fetching path `%1%' (empty status: '%2%')")
+			     % storePath % status);
+	} else if (statusList[0] == "hash-mismatch") {
+	    if (settings.printBuildTrace) {
+		auto hashType = statusList[1];
+		auto expectedHash = statusList[2];
+		auto actualHash = statusList[3];
 		printMsg(lvlError, format("@ hash-mismatch %1% %2% %3% %4%")
-			 % storePath % "sha256"
-			 % printHash16or32(expectedHash)
-			 % printHash16or32(actualHash));
+			 % storePath
+			 % hashType % expectedHash % actualHash);
+	    }
 	    throw SubstError(format("hash mismatch for substituted item `%1%'") % storePath);
+	} else if (statusList[0] == "success") {
+	    if (!pathExists(destPath))
+		throw SubstError(format("substitute did not produce path `%1%'") % destPath);
+
+	    std::string hashStr = statusList[1];
+	    size_t n = hashStr.find(':');
+	    if (n == string::npos)
+		throw Error(format("bad hash from substituter: %1%") % hashStr);
+
+	    HashType hashType = parseHashType(string(hashStr, 0, n));
+	    switch (hashType) {
+	    case htUnknown:
+		throw Error(format("unknown hash algorithm in `%1%'") % hashStr);
+	    case htSHA256:
+		hash.first = parseHash16or32(hashType, string(hashStr, n + 1));
+		hash.second = std::atoi(statusList[2].c_str());
+		break;
+	    default:
+		/* The database only stores SHA256 hashes, so compute it.  */
+		hash = hashPath(htSHA256, destPath);
+		break;
+	    }
 	}
+	else
+            throw SubstError(format("fetching path `%1%' (status: '%2%')")
+                % storePath % status);
 
     } catch (SubstError & e) {
 
@@ -3123,9 +3130,7 @@ void SubstitutionGoal::handleChildOutput(int fd, const string & data)
 	    string trimmed = (end != string::npos) ? input.substr(0, end) : input;
 
 	    /* Update the goal's state accordingly.  */
-	    if (expectedHashStr == "") {
-		expectedHashStr = trimmed;
-	    } else if (status == "") {
+	    if (status == "") {
 		status = trimmed;
 		worker.wakeUp(shared_from_this());
 	    } else {
-- 
cgit v1.2.3