Merge pull request #14219 from lovesegfault/eval-copy-less

libstore: Avoid copying derivations to the store if they are already valid

src/libstore-tests/meson.build

@@ -83,6 +83,7 @@ sources = files(
   'store-reference.cc',
   'uds-remote-store.cc',
   'worker-protocol.cc',
+  'write-derivation.cc',
 )
 
 include_dirs = [ include_directories('.') ]

src/libstore-tests/write-derivation.cc

@@ -0,0 +1,57 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "nix/util/tests/gmock-matchers.hh"
+#include "nix/store/derivations.hh"
+#include "nix/store/dummy-store-impl.hh"
+#include "nix/store/tests/libstore.hh"
+
+namespace nix {
+namespace {
+
+class WriteDerivationTest : public LibStoreTest
+{
+protected:
+    WriteDerivationTest(ref<DummyStoreConfig> config_)
+        : LibStoreTest(config_->openDummyStore())
+        , config(std::move(config_))
+    {
+        config->readOnly = false;
+    }
+
+    WriteDerivationTest()
+        : WriteDerivationTest(make_ref<DummyStoreConfig>(DummyStoreConfig::Params{}))
+    {
+    }
+
+    ref<DummyStoreConfig> config;
+};
+
+static Derivation makeSimpleDrv()
+{
+    Derivation drv;
+    drv.name = "simple-derivation";
+    drv.platform = "system";
+    drv.builder = "foo";
+    drv.args = {"bar", "baz"};
+    drv.env = StringPairs{{"BIG_BAD", "WOLF"}};
+    return drv;
+}
+
+} // namespace
+
+TEST_F(WriteDerivationTest, addToStoreFromDumpCalledOnce)
+{
+    auto drv = makeSimpleDrv();
+
+    auto path1 = writeDerivation(*store, drv, NoRepair);
+    config->readOnly = true;
+    auto path2 = writeDerivation(*store, drv, NoRepair);
+    EXPECT_EQ(path1, path2);
+    EXPECT_THAT(
+        [&] { writeDerivation(*store, drv, Repair); },
+        ::testing::ThrowsMessage<Error>(testing::HasSubstrIgnoreANSIMatcher(
+            "operation 'addToStoreFromDump' is not supported by store 'dummy://'")));
+}
+
+} // namespace nix

src/libstore/derivations.cc

@@ -115,23 +115,25 @@ StorePath writeDerivation(Store & store, const Derivation & drv, RepairFlag repa
        held during a garbage collection). */
     auto suffix = std::string(drv.name) + drvExtension;
     auto contents = drv.unparse(store, false);
-    return readOnly || settings.readOnlyMode ? store.makeFixedOutputPathFromCA(
-                                                   suffix,
-                                                   TextInfo{
-                                                       .hash = hashString(HashAlgorithm::SHA256, contents),
-                                                       .references = std::move(references),
-                                                   })
-                                             : ({
-                                                   StringSource s{contents};
-                                                   store.addToStoreFromDump(
-                                                       s,
-                                                       suffix,
-                                                       FileSerialisationMethod::Flat,
-                                                       ContentAddressMethod::Raw::Text,
-                                                       HashAlgorithm::SHA256,
-                                                       references,
-                                                       repair);
-                                               });
+    auto hash = hashString(HashAlgorithm::SHA256, contents);
+    auto ca = TextInfo{.hash = hash, .references = references};
+    auto path = store.makeFixedOutputPathFromCA(suffix, ca);
+
+    if (readOnly || settings.readOnlyMode || (store.isValidPath(path) && !repair))
+        return path;
+
+    StringSource s{contents};
+    auto path2 = store.addToStoreFromDump(
+        s,
+        suffix,
+        FileSerialisationMethod::Flat,
+        ContentAddressMethod::Raw::Text,
+        HashAlgorithm::SHA256,
+        references,
+        repair);
+    assert(path2 == path);
+
+    return path;
 }
 
 namespace {