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

Also pass unwanted outputs to post-build-hook

src/libstore/build/derivation-goal.cc

@@ -1020,51 +1020,41 @@ 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);
                 signRealisation(newRealisation);
                 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 {

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();
 

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;

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);
 }

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;
 

tests/post-hook.sh

@@ -17,10 +17,15 @@ 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
 
 # Ensure that the remote store contains both the runtime and build-time
 # 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

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

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