Merge pull request #6311 from edolstra/return-wanted-paths

Make buildPathsWithResults() only return info on wanted outputs

src/libstore/build/derivation-goal.cc

@@ -207,8 +207,6 @@ void DerivationGoal::haveDerivation()
     if (!drv->type().hasKnownOutputPaths())
         settings.requireExperimentalFeature(Xp::CaDerivations);
 
-    retrySubstitution = false;
-
     for (auto & i : drv->outputsAndOptPaths(worker.store))
         if (i.second.second)
             worker.store.addTempRoot(*i.second.second);
@@ -311,14 +309,11 @@ void DerivationGoal::outputsSubstitutionTried()
     gaveUpOnSubstitution();
 }
 
+
 /* At least one of the output paths could not be
    produced using a substitute.  So we have to build instead. */
 void DerivationGoal::gaveUpOnSubstitution()
 {
-    /* Make sure checkPathValidity() from now on checks all
-       outputs. */
-    wantedOutputs.clear();
-
     /* The inputs must be built before we can build this goal. */
     if (useDerivation)
         for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs)
@@ -426,7 +421,8 @@ void DerivationGoal::inputsRealised()
         return;
     }
 
-    if (retrySubstitution) {
+    if (retrySubstitution && !retriedSubstitution) {
+        retriedSubstitution = true;
         haveDerivation();
         return;
     }

src/libstore/build/derivation-goal.hh

@@ -61,8 +61,12 @@ struct DerivationGoal : public Goal
     bool needRestart = false;
 
     /* Whether to retry substituting the outputs after building the
-       inputs. */
-    bool retrySubstitution;
+       inputs. This is done in case of an incomplete closure. */
+    bool retrySubstitution = false;
+
+    /* Whether we've retried substitution, in which case we won't try
+       again. */
+    bool retriedSubstitution = false;
 
     /* The derivation stored at drvPath. */
     std::unique_ptr<Derivation> drv;

src/libstore/build/goal.cc

@@ -28,7 +28,7 @@ void Goal::addWaitee(GoalPtr waitee)
 
 void Goal::waiteeDone(GoalPtr waitee, ExitCode result)
 {
-    assert(waitees.find(waitee) != waitees.end());
+    assert(waitees.count(waitee));
     waitees.erase(waitee);
 
     trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size()));

src/libstore/build/goal.hh

@@ -40,21 +40,21 @@ struct Goal : public std::enable_shared_from_this<Goal>
     WeakGoals waiters;
 
     /* Number of goals we are/were waiting for that have failed. */
-    unsigned int nrFailed;
+    size_t nrFailed = 0;
 
     /* Number of substitution goals we are/were waiting for that
        failed because there are no substituters. */
-    unsigned int nrNoSubstituters;
+    size_t nrNoSubstituters = 0;
 
     /* Number of substitution goals we are/were waiting for that
        failed because they had unsubstitutable references. */
-    unsigned int nrIncompleteClosure;
+    size_t nrIncompleteClosure = 0;
 
     /* Name of this goal for debugging purposes. */
     std::string name;
 
     /* Whether the goal is finished. */
-    ExitCode exitCode;
+    ExitCode exitCode = ecBusy;
 
     /* Build result. */
     BuildResult buildResult;
@@ -65,10 +65,7 @@ struct Goal : public std::enable_shared_from_this<Goal>
     Goal(Worker & worker, DerivedPath path)
         : worker(worker)
         , buildResult { .path = std::move(path) }
-    {
-        nrFailed = nrNoSubstituters = nrIncompleteClosure = 0;
-        exitCode = ecBusy;
-    }
+    { }
 
     virtual ~Goal()
     {

src/libstore/build/local-derivation-goal.cc

@@ -2613,6 +2613,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             signRealisation(thisRealisation);
             worker.store.registerDrvOutput(thisRealisation);
         }
+        if (wantOutput(outputName, wantedOutputs))
             builtOutputs.emplace(thisRealisation.id, thisRealisation);
     }
 

tests/build.sh

@@ -1,15 +1,27 @@
 source common.sh
 
-expectedJSONRegex='\[\{"drvPath":".*multiple-outputs-a.drv","outputs":\{"first":".*multiple-outputs-a-first","second":".*multiple-outputs-a-second"}},\{"drvPath":".*multiple-outputs-b.drv","outputs":\{"out":".*multiple-outputs-b"}}]'
+clearStore
+
+# Make sure that 'nix build' only returns the outputs we asked for.
+nix build -f multiple-outputs.nix --json a --no-link | jq --exit-status '
+  (.[0] |
+    (.drvPath | match(".*multiple-outputs-a.drv")) and
+    (.outputs | keys | length == 1) and
+    (.outputs.first | match(".*multiple-outputs-a-first")))
+'
+
 nix build -f multiple-outputs.nix --json a.all b.all --no-link | jq --exit-status '
   (.[0] |
     (.drvPath | match(".*multiple-outputs-a.drv")) and
+    (.outputs | keys | length == 2) and
     (.outputs.first | match(".*multiple-outputs-a-first")) and
     (.outputs.second | match(".*multiple-outputs-a-second")))
   and (.[1] |
     (.drvPath | match(".*multiple-outputs-b.drv")) and
+    (.outputs | keys | length == 1) and
     (.outputs.out | match(".*multiple-outputs-b")))
 '
+
 testNormalization () {
     clearStore
     outPath=$(nix-build ./simple.nix --no-out-link)