From 8173e7bfefc6a5771b2c9ec48bd6edd3b161dd90 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 14 Jul 2020 21:11:08 +0000
Subject: [PATCH 1/2] Fix localhost::addToStore(...Path...)

We were calculating the nar hash wrong when the file ingestion method
was flat. I don't think there's anything we can do in that case but dump
the file again, so that's what I do.

As an optomization, we again could reuse the original dump for just the
recursive and non-sha256 case, but I rather do that after this fix, and
after my other PRs which deduplicate this code.
---
 src/libstore/local-store.cc | 33 ++++++++++++++++-----------------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 26b226fe8..5827dfc58 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1097,15 +1097,8 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
 {
     Path srcPath(absPath(_srcPath));
 
-    /* For computing the NAR hash. */
-    auto sha256Sink = std::make_unique<HashSink>(htSHA256);
-
-    /* For computing the store path. In recursive SHA-256 mode, this
-       is the same as the NAR hash, so no need to do it again. */
-    std::unique_ptr<HashSink> hashSink =
-        method == FileIngestionMethod::Recursive && hashAlgo == htSHA256
-        ? nullptr
-        : std::make_unique<HashSink>(hashAlgo);
+    /* For computing the store path. */
+    auto hashSink = std::make_unique<HashSink>(hashAlgo);
 
     /* Read the source path into memory, but only if it's up to
        narBufferSize bytes. If it's larger, write it to a temporary
@@ -1114,13 +1107,12 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
        temporary path. Otherwise, we move it to the destination store
        path. */
     bool inMemory = true;
-    std::string nar;
+    std::string nar; // TODO rename from "nar" to "dump"
 
     auto source = sinkToSource([&](Sink & sink) {
 
         LambdaSink sink2([&](const unsigned char * buf, size_t len) {
-            (*sha256Sink)(buf, len);
-            if (hashSink) (*hashSink)(buf, len);
+            (*hashSink)(buf, len);
 
             if (inMemory) {
                 if (nar.size() + len > settings.narBufferSize) {
@@ -1165,9 +1157,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
         /* The NAR fits in memory, so we didn't do restorePath(). */
     }
 
-    auto sha256 = sha256Sink->finish();
-
-    Hash hash = hashSink ? hashSink->finish().first : sha256.first;
+    auto [hash, size] = hashSink->finish();
 
     auto dstPath = makeFixedOutputPath(method, hash, name);
 
@@ -1201,13 +1191,22 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
                     throw Error("renaming '%s' to '%s'", tempPath, realPath);
             }
 
+            /* 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 { hash, size };
+            if (method != FileIngestionMethod::Recursive || hashAlgo != htSHA256) {
+                HashSink narSink { htSHA256 };
+                dumpPath(realPath, narSink);
+                narHash = narSink.finish();
+            }
+
             canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath
 
             optimisePath(realPath);
 
             ValidPathInfo info(dstPath);
-            info.narHash = sha256.first;
-            info.narSize = sha256.second;
+            info.narHash = narHash.first;
+            info.narSize = narHash.second;
             info.ca = FixedOutputHash { .method = method, .hash = hash };
             registerValidPath(info);
         }

From 650c2c655810c375296b52997e2f85298c7c566a Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 14 Jul 2020 21:28:50 +0000
Subject: [PATCH 2/2] Rename variable `nar` -> `dump` according to TODO

---
 src/libstore/local-store.cc | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 5827dfc58..cd92f138c 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1107,7 +1107,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
        temporary path. Otherwise, we move it to the destination store
        path. */
     bool inMemory = true;
-    std::string nar; // TODO rename from "nar" to "dump"
+    std::string dump;
 
     auto source = sinkToSource([&](Sink & sink) {
 
@@ -1115,13 +1115,13 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
             (*hashSink)(buf, len);
 
             if (inMemory) {
-                if (nar.size() + len > settings.narBufferSize) {
+                if (dump.size() + len > settings.narBufferSize) {
                     inMemory = false;
                     sink << 1;
-                    sink((const unsigned char *) nar.data(), nar.size());
-                    nar.clear();
+                    sink((const unsigned char *) dump.data(), dump.size());
+                    dump.clear();
                 } else {
-                    nar.append((const char *) buf, len);
+                    dump.append((const char *) buf, len);
                 }
             }
 
@@ -1180,7 +1180,7 @@ StorePath LocalStore::addToStore(const string & name, const Path & _srcPath,
 
             if (inMemory) {
                 /* Restore from the NAR in memory. */
-                StringSource source(nar);
+                StringSource source(dump);
                 if (method == FileIngestionMethod::Recursive)
                     restorePath(realPath, source);
                 else