Merge pull request #15288 from roberth/flake-eval-cache-fixes

Flake eval cache fixes

Author: edolstra

src/libexpr/eval-cache.cc

@@ -58,6 +58,9 @@ struct AttrDb
         SQLiteStmt insertAttributeWithContext;
         SQLiteStmt queryAttribute;
         SQLiteStmt queryAttributes;
+        SQLiteStmt upsertAttribute;
+        SQLiteStmt insertAttributeIfNotExists;
+        SQLiteStmt deleteMissingChildren;
         std::unique_ptr<SQLiteTxn> txn;
     };
 
@@ -92,6 +95,17 @@ struct AttrDb
 
         state->queryAttributes.create(state->db, "select name from Attributes where parent = ?");
 
+        state->upsertAttribute.create(
+            state->db,
+            "insert into Attributes(parent, name, type, value) values (?, ?, ?, ?) "
+            "on conflict(parent, name) do update set type = excluded.type, value = excluded.value "
+            "returning rowid");
+
+        state->insertAttributeIfNotExists.create(
+            state->db, "insert or ignore into Attributes(parent, name, type, value) values (?, ?, ?, ?)");
+
+        state->deleteMissingChildren.create(state->db, "delete from Attributes where parent = ? and type = 3");
+
         state->txn = std::make_unique<SQLiteTxn>(state->db);
     }
 
@@ -126,18 +140,20 @@ struct AttrDb
         return doSQLite([&]() {
             auto state(_state->lock());
 
-            state->insertAttribute.use()
-                .apply(key.first)
-                .apply(symbols[key.second])
-                .apply(AttrType::FullAttrs)
-                .apply(0, false)
-                .exec();
-
-            AttrId rowId = state->db.getLastInsertedRowId();
+            // Seal the attribute names: we now know the complete set.
+            auto upsertAttribute(state->upsertAttribute.use()
+                                     .apply(key.first)
+                                     .apply(symbols[key.second])
+                                     .apply(AttrType::FullAttrs)
+                                     .apply(0, false));
+            upsertAttribute.next();
+            auto rowId = (AttrId) upsertAttribute.getInt(0);
             assert(rowId);
+            state->deleteMissingChildren.use().apply(rowId).exec();
 
+            // Insert children as placeholders, but don't replace existing entries
             for (auto & attr : attrs)
-                state->insertAttribute.use()
+                state->insertAttributeIfNotExists.use()
                     .apply(rowId)
                     .apply(symbols[attr])
                     .apply(AttrType::Placeholder)
@@ -461,9 +477,13 @@ Value & AttrCursor::forceValue()
     }
 
     if (root->db && (!cachedValue || std::get_if<placeholder_t>(&cachedValue->second))) {
-        if (v.type() == nString)
-            cachedValue = {root->db->setString(getKey(), v.string_view(), v.context()), string_t{v.string_view(), {}}};
-        else if (v.type() == nPath) {
+        if (v.type() == nString) {
+            NixStringContext context;
+            copyContext(v, context);
+            cachedValue = {
+                root->db->setString(getKey(), v.string_view(), v.context()),
+                string_t{v.string_view(), std::move(context)}};
+        } else if (v.type() == nPath) {
             auto path = v.path().path;
             cachedValue = {root->db->setString(getKey(), path.abs()), string_t{path.abs(), {}}};
         } else if (v.type() == nBool)

src/libexpr/meson.build

@@ -52,6 +52,10 @@ deps_other += boost
 nlohmann_json = dependency('nlohmann_json', version : '>= 3.9')
 deps_public += nlohmann_json
 
+# eval-cache uses `INSERT ... RETURNING` which requires >= 3.35
+sqlite = dependency('sqlite3', 'sqlite', version : '>= 3.35')
+deps_private += sqlite
+
 bdw_gc_required = get_option('gc').disable_if(
   'address' in get_option('b_sanitize'),
   error_message : 'Building with Boehm GC and ASAN is not supported',

src/libexpr/package.nix

@@ -13,6 +13,7 @@
   boost,
   boehmgc,
   nlohmann_json,
+  sqlite,
   toml11,
   libcpuid,
 
@@ -64,6 +65,7 @@ mkMesonLibrary (finalAttrs: {
   ];
 
   buildInputs = [
+    sqlite
     toml11
   ]
   ++ lib.optional stdenv.hostPlatform.isx86_64 libcpuid;

tests/functional/flakes/eval-cache.sh

@@ -4,6 +4,42 @@ source ./common.sh
 
 requireGit
 
+# Test basic caching: trace should only appear on first evaluation
+basicCacheDir="$TEST_ROOT/basic-cache-flake"
+createGitRepo "$basicCacheDir" ""
+cp "${config_nix}" "$basicCacheDir/"
+git -C "$basicCacheDir" add config.nix
+git -C "$basicCacheDir" commit -m "config.nix"
+
+cat >"$basicCacheDir/flake.nix" <<EOF
+{
+  description = "Basic cache test";
+  outputs = { self }: let inherit (import ./config.nix) mkDerivation; in {
+    # Assert pure-eval is enabled (builtins.currentSystem is unavailable in pure mode)
+    packages.$system.default = assert builtins ? currentSystem -> throw "pure-eval not enabled";
+      builtins.trace "basic-test-evaluating" (mkDerivation {
+        name = "cached-build";
+        buildCommand = "echo hello > \\\$out";
+      });
+  };
+}
+EOF
+
+git -C "$basicCacheDir" add flake.nix
+git -C "$basicCacheDir" commit -m "Init"
+
+clearBinaryCache
+
+# First build should show trace
+nix build --no-link "$basicCacheDir" 2>&1 | grepQuiet 'trace: basic-test-evaluating'
+
+# Second build should use cache, no trace output
+nix build --no-link "$basicCacheDir" 2>&1 | grepQuietInverse 'trace: basic-test-evaluating'
+
+# Third build should also use cache, no trace output
+nix build --no-link "$basicCacheDir" 2>&1 | grepQuietInverse 'trace: basic-test-evaluating'
+
+# Test edge cases with separate flake
 flake1Dir="$TEST_ROOT/eval-cache-flake"
 
 createGitRepo "$flake1Dir" ""