Merge pull request #7721 from yorickvP/post-build-hook

Also pass unwanted outputs to post-build-hook

8 files changed, 53 insertions, 30 deletions

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index a4bb94b0e..23724b0d9 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1020,43 +1020,33 @@ void DerivationGoal::resolvedFinished()
 
         StorePathSet outputPaths;
 
-        // `wantedOutputs` might merely indicate “all the outputs”
-        auto realWantedOutputs = std::visit(overloaded {
-            [&](const OutputsSpec::All &) {
-                return resolvedDrv.outputNames();
-            },
-            [&](const OutputsSpec::Names & names) {
-                return static_cast<std::set<std::string>>(names);
-            },
-        }, wantedOutputs.raw());
-
-        for (auto & wantedOutput : realWantedOutputs) {
-            auto initialOutput = get(initialOutputs, wantedOutput);
-            auto resolvedHash = get(resolvedHashes, wantedOutput);
+        for (auto & outputName : resolvedDrv.outputNames()) {
+            auto initialOutput = get(initialOutputs, outputName);
+            auto resolvedHash = get(resolvedHashes, outputName);
             if ((!initialOutput) || (!resolvedHash))
                 throw Error(
                     "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,resolve)",
-                    worker.store.printStorePath(drvPath), wantedOutput);
+                    worker.store.printStorePath(drvPath), outputName);
 
             auto realisation = [&]{
-              auto take1 = get(resolvedResult.builtOutputs, wantedOutput);
+              auto take1 = get(resolvedResult.builtOutputs, outputName);
               if (take1) return *take1;
 
               /* The above `get` should work. But sateful tracking of
                  outputs in resolvedResult, this can get out of sync with the
                  store, which is our actual source of truth. For now we just
                  check the store directly if it fails. */
-              auto take2 = worker.evalStore.queryRealisation(DrvOutput { *resolvedHash, wantedOutput });
+              auto take2 = worker.evalStore.queryRealisation(DrvOutput { *resolvedHash, outputName });
               if (take2) return *take2;
 
               throw Error(
                   "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/resolvedFinished,realisation)",
-                  worker.store.printStorePath(resolvedDrvGoal->drvPath), wantedOutput);
+                  worker.store.printStorePath(resolvedDrvGoal->drvPath), outputName);
             }();
 
             if (drv->type().isPure()) {
                 auto newRealisation = realisation;
-                newRealisation.id = DrvOutput { initialOutput->outputHash, wantedOutput };
+                newRealisation.id = DrvOutput { initialOutput->outputHash, outputName };
                 newRealisation.signatures.clear();
                 if (!drv->type().isFixed())
                     newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath);
@@ -1064,7 +1054,7 @@ void DerivationGoal::resolvedFinished()
                 worker.store.registerDrvOutput(newRealisation);
             }
             outputPaths.insert(realisation.outPath);
-            builtOutputs.emplace(wantedOutput, realisation);
+            builtOutputs.emplace(outputName, realisation);
         }
 
         runPostBuildHook(
@@ -1406,7 +1396,7 @@ std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
                 );
             }
         }
-        if (info.wanted && info.known && info.known->isValid())
+        if (info.known && info.known->isValid())
             validOutputs.emplace(i.first, Realisation { drvOutput, info.known->path });
     }
 
@@ -1457,8 +1447,9 @@ void DerivationGoal::done(
     mcRunningBuilds.reset();
 
     if (buildResult.success()) {
-        assert(!builtOutputs.empty());
-        buildResult.builtOutputs = std::move(builtOutputs);
+        auto wantedBuiltOutputs = filterDrvOutputs(wantedOutputs, std::move(builtOutputs));
+        assert(!wantedBuiltOutputs.empty());
+        buildResult.builtOutputs = std::move(wantedBuiltOutputs);
         if (status == BuildResult::Built)
             worker.doneBuilds++;
     } else {
diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index 7033b7a58..01a08d391 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -306,15 +306,13 @@ struct DerivationGoal : public Goal
      * Update 'initialOutputs' to determine the current status of the
      * outputs of the derivation. Also returns a Boolean denoting
      * whether all outputs are valid and non-corrupt, and a
-     * 'SingleDrvOutputs' structure containing the valid and wanted
-     * outputs.
+     * 'SingleDrvOutputs' structure containing the valid outputs.
      */
     std::pair<bool, SingleDrvOutputs> checkPathValidity();
 
     /**
      * Aborts if any output is not valid or corrupt, and otherwise
-     * returns a 'SingleDrvOutputs' structure containing the wanted
-     * outputs.
+     * returns a 'SingleDrvOutputs' structure containing all outputs.
      */
     SingleDrvOutputs assertPathValidity();
 
diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 21cd6e7ee..2f545627b 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -2701,8 +2701,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
             signRealisation(thisRealisation);
             worker.store.registerDrvOutput(thisRealisation);
         }
-        if (wantedOutputs.contains(outputName))
-            builtOutputs.emplace(outputName, thisRealisation);
+        builtOutputs.emplace(outputName, thisRealisation);
     }
 
     return builtOutputs;
diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc
index d63ec5ea2..93ddb5b20 100644
--- a/src/libstore/realisation.cc
+++ b/src/libstore/realisation.cc
@@ -136,6 +136,19 @@ size_t Realisation::checkSignatures(const PublicKeys & publicKeys) const
     return good;
 }
 
+
+SingleDrvOutputs filterDrvOutputs(const OutputsSpec& wanted, SingleDrvOutputs&& outputs)
+{
+    SingleDrvOutputs ret = std::move(outputs);
+    for (auto it = ret.begin(); it != ret.end(); ) {
+        if (!wanted.contains(it->first))
+            it = ret.erase(it);
+        else
+            ++it;
+    }
+    return ret;
+}
+
 StorePath RealisedPath::path() const {
     return std::visit([](auto && arg) { return arg.getPath(); }, raw);
 }
diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh
index 3922d1267..2a093c128 100644
--- a/src/libstore/realisation.hh
+++ b/src/libstore/realisation.hh
@@ -12,6 +12,7 @@
 namespace nix {
 
 class Store;
+struct OutputsSpec;
 
 /**
  * A general `Realisation` key.
@@ -93,6 +94,14 @@ typedef std::map<std::string, Realisation> SingleDrvOutputs;
  */
 typedef std::map<DrvOutput, Realisation> DrvOutputs;
 
+/**
+ * Filter a SingleDrvOutputs to include only specific output names
+ *
+ * Moves the `outputs` input.
+ */
+SingleDrvOutputs filterDrvOutputs(const OutputsSpec&, SingleDrvOutputs&&);
+
+
 struct OpaquePath {
     StorePath path;
 
diff --git a/tests/post-hook.sh b/tests/post-hook.sh
index 0266eb15d..752f8220c 100644
--- a/tests/post-hook.sh
+++ b/tests/post-hook.sh
@@ -17,6 +17,10 @@ fi
 
 # Build the dependencies and push them to the remote store.
 nix-build -o $TEST_ROOT/result dependencies.nix --post-build-hook "$pushToStore"
+# See if all outputs are passed to the post-build hook by only specifying one
+# We're not able to test CA tests this way
+export BUILD_HOOK_ONLY_OUT_PATHS=$([ ! $NIX_TESTS_CA_BY_DEFAULT ])
+nix-build -o $TEST_ROOT/result-mult multiple-outputs.nix -A a.first --post-build-hook "$pushToStore"
 
 clearStore
 
@@ -24,3 +28,4 @@ clearStore
 # closure of what we've just built.
 nix copy --from "$REMOTE_STORE" --no-require-sigs -f dependencies.nix
 nix copy --from "$REMOTE_STORE" --no-require-sigs -f dependencies.nix input1_drv
+nix copy --from "$REMOTE_STORE" --no-require-sigs -f multiple-outputs.nix a^second
diff --git a/tests/push-to-store-old.sh b/tests/push-to-store-old.sh
index b1495c9e2..4187958b2 100755
--- a/tests/push-to-store-old.sh
+++ b/tests/push-to-store-old.sh
@@ -7,4 +7,8 @@ set -e
 [ -n "$DRV_PATH" ]
 
 echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
-printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+if [ -n "$BUILD_HOOK_ONLY_OUT_PATHS" ]; then
+    printf "%s" "$OUT_PATHS" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+else
+    printf "%s" "$DRV_PATH" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+fi
diff --git a/tests/push-to-store.sh b/tests/push-to-store.sh
index 0b090e1b3..9e4e475e0 100755
--- a/tests/push-to-store.sh
+++ b/tests/push-to-store.sh
@@ -7,4 +7,8 @@ set -e
 [ -n "$DRV_PATH" ]
 
 echo Pushing "$OUT_PATHS" to "$REMOTE_STORE"
-printf "%s" "$DRV_PATH"^'*' | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+if [ -n "$BUILD_HOOK_ONLY_OUT_PATHS" ]; then
+    printf "%s" "$OUT_PATHS" | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+else
+    printf "%s" "$DRV_PATH"^'*' | xargs nix copy --to "$REMOTE_STORE" --no-require-sigs
+fi