From 92656da0b953b92228ef4a252d504c07710e4b47 Mon Sep 17 00:00:00 2001
From: regnat <rg@regnat.ovh>
Date: Wed, 3 Nov 2021 16:30:22 +0100
Subject: [PATCH] Fix the gc with indirect self-references via the realisations
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If the derivation `foo` depends on `bar`, and they both have the same
output path (because they are CA derivations), then this output path
will depend both on the realisation of `foo` and of `bar`, which
themselves depend on each other.
This confuses SQLite which isn’t able to automatically solve this
diamond dependency scheme.

Help it by adding a trigger to delete all the references between the
relevant realisations.

Fix #5320
---
 src/libstore/ca-specific-schema.sql | 13 +++++++++++++
 src/libstore/local-store.cc         | 15 ++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql
index 64cc97fde..d2ea347fb 100644
--- a/src/libstore/ca-specific-schema.sql
+++ b/src/libstore/ca-specific-schema.sql
@@ -13,6 +13,19 @@ create table if not exists Realisations (
 
 create index if not exists IndexRealisations on Realisations(drvPath, outputName);
 
+-- We can end-up in a weird edge-case where a path depends on itself because
+-- it’s an output of a CA derivation, that happens to be the same as one of its
+-- dependencies.
+-- In that case we have a dependency loop (path -> realisation1 -> realisation2
+-- -> path) that we need to break by removing the dependencies between the
+-- realisations
+create trigger if not exists DeleteSelfRefsViaRealisations before delete on ValidPaths
+  begin
+    delete from RealisationsRefs where realisationReference = (
+      select id from Realisations where outputPath = old.id
+    );
+  end;
+
 create table if not exists RealisationsRefs (
     referrer integer not null,
     realisationReference integer,
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index ece5bb5ef..e64917956 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -81,7 +81,7 @@ int getSchema(Path schemaPath)
 
 void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
 {
-    const int nixCASchemaVersion = 3;
+    const int nixCASchemaVersion = 4;
     int curCASchema = getSchema(schemaPath);
     if (curCASchema != nixCASchemaVersion) {
         if (curCASchema > nixCASchemaVersion) {
@@ -143,6 +143,19 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd)
             )");
             txn.commit();
         }
+        if (curCASchema < 4) {
+            SQLiteTxn txn(db);
+            db.exec(R"(
+                create trigger if not exists DeleteSelfRefsViaRealisations before delete on ValidPaths
+                begin
+                    delete from RealisationsRefs where realisationReference = (
+                    select id from Realisations where outputPath = old.id
+                    );
+                end;
+            )");
+            txn.commit();
+        }
+
         writeFile(schemaPath, fmt("%d", nixCASchemaVersion));
         lockFile(lockFd.get(), ltRead, true);
     }