Merge pull request #4732 from NixOS/4725-always-register-the-realisations

Aways register the realisations

src/libcmd/installables.cc

@@ -736,18 +736,9 @@ std::set<RealisedPath> toRealisedPaths(
                                     output.first);
                             auto outputId = DrvOutput{outputHashes.at(output.first), output.first};
                             auto realisation = store->queryRealisation(outputId);
-                            if (!realisation) {
+                            if (!realisation)
-                                // TODO: remove this check once #4725 is fixed
+                                throw Error("cannot operate on an output of unbuilt content-addresed derivation '%s'", outputId.to_string());
-                                // as we’ll have the guaranty that if
+                            res.insert(RealisedPath{*realisation});
-                                // `output.second` exists, then the realisation
-                                // will be there too
-                                if (output.second)
-                                    res.insert(*output.second);
-                                else
-                                    throw Error("cannot operate on an output of unbuilt content-addresed derivation '%s'", outputId.to_string());
-                            } else {
-                                res.insert(RealisedPath{*realisation});
-                            }
                         }
                         else {
                             // If ca-derivations isn't enabled, behave as if

src/libstore/build/derivation-goal.cc

@@ -1269,12 +1269,23 @@ void DerivationGoal::checkPathValidity()
             };
         }
         if (settings.isExperimentalFeatureEnabled("ca-derivations")) {
-            if (auto real = worker.store.queryRealisation(
+            auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first};
-                    DrvOutput{initialOutputs.at(i.first).outputHash, i.first})) {
+            if (auto real = worker.store.queryRealisation(drvOutput)) {
                 info.known = {
                     .path = real->outPath,
                     .status = PathStatus::Valid,
                 };
+            } else if (info.known && info.known->status == PathStatus::Valid) {
+                // We know the output because it' a static output of the
+                // derivation, and the output path is valid, but we don't have
+                // its realisation stored (probably because it has been built
+                // without the `ca-derivations` experimental flag)
+                worker.store.registerDrvOutput(
+                    Realisation{
+                        drvOutput,
+                        info.known->path,
+                    }
+                );
             }
         }
     }

tests/ca/substitute.sh

@@ -8,7 +8,8 @@ sed -i 's/experimental-features .*/& ca-derivations ca-references/' "$NIX_CONF_D
 
 rm -rf $TEST_ROOT/binary_cache
 
-export REMOTE_STORE=file://$TEST_ROOT/binary_cache
+export REMOTE_STORE_DIR=$TEST_ROOT/binary_cache
+export REMOTE_STORE=file://$REMOTE_STORE_DIR
 
 buildDrvs () {
     nix build --file ./content-addressed.nix -L --no-link "$@"
@@ -22,3 +23,25 @@ buildDrvs --post-build-hook ../push-to-store.sh
 clearStore
 buildDrvs --substitute --substituters $REMOTE_STORE --no-require-sigs -j0
 
+# Same thing, but
+# 1. With non-ca derivations
+# 2. Erasing the realisations on the remote store
+#
+# Even in that case, realising the derivations should still produce the right
+# realisations on the local store
+#
+# Regression test for #4725
+clearStore
+nix build --file ../simple.nix -L --no-link --post-build-hook ../push-to-store.sh
+clearStore
+rm -r "$REMOTE_STORE_DIR/realisations"
+nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0
+# There's no easy way to check whether a realisation is present on the local
+# store − short of manually querying the db, but the build environment doesn't
+# have the sqlite binary − so we instead push things again, and check that the
+# realisations have correctly been pushed to the remote store
+nix copy --to "$REMOTE_STORE" --file ../simple.nix
+if [[ -z "$(ls "$REMOTE_STORE_DIR/realisations")" ]]; then
+    echo "Realisations not rebuilt"
+    exit 1
+fi