nix-super/src/libstore/build/goal.cc

110 lines
2.5 KiB
C++
Raw Normal View History

#include "goal.hh"
#include "worker.hh"
namespace nix {
2017-12-11 20:05:14 +02:00
bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const {
std::string s1 = a->key();
std::string s2 = b->key();
return s1 < s2;
}
Make `KeyedBuildResult`, `BuildResult` like before, and fix bug another way In https://github.com/NixOS/nix/pull/6311#discussion_r834863823, I realized since derivation goals' wanted outputs can "grow" due to overlapping dependencies (See `DerivationGoal::addWantedOutputs`, called by `Worker::makeDerivationGoalCommon`), the previous bug fix had an unfortunate side effect of causing more pointless rebuilds. In paticular, we have this situation: 1. Goal made from `DerivedPath::Built { foo, {a} }`. 2. Goal gives on on substituting, starts building. 3. Goal made from `DerivedPath::Built { foo, {b} }`, in fact is just modified original goal. 4. Though the goal had gotten as far as building, so all outputs were going to be produced, `addWantedOutputs` no longer knows that and so the goal is flagged to be restarted. This might sound far-fetched with input-addressed drvs, where we usually basically have all our goals "planned out" before we start doing anything, but with CA derivation goals and especially RFC 92, where *drv resolution* means goals are created after some building is completed, it is more likely to happen. So the first thing to do was restore the clearing of `wantedOutputs` we used to do, and then filter the outputs in `buildPathsWithResults` to only get the ones we care about. But fix also has its own side effect in that the `DerivedPath` in the `BuildResult` in `DerivationGoal` cannot be trusted; it is merely the *first* `DerivedPath` for which this goal was originally created. To remedy this, I made `BuildResult` be like it was before, and instead made `KeyedBuildResult` be a subclass wit the path. Only `buildPathsWithResults` returns `KeyedBuildResult`s, everything else just becomes like it was before, where the "key" is unambiguous from context. I think separating the "primary key" field(s) from the other fields is good practical in general anyways. (I would like to do the same thing for `ValidPathInfo`.) Among other things, it allows constructions like `std::map<Key, ThingWithKey>` where doesn't contain duplicate keys and just precludes the possibility of those duplicate keys being out of sync. We might leverage the above someday to overload `buildPathsWithResults` to take a *set* of return a *map* per the above. ----- Unfortunately, we need to avoid C++20 strictness on designated initializers. (BTW https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2287r1.html this offers some new syntax for this use-case. Hopefully this will be adopted and we can eventually use it.) No having that yet, maybe it would be better to not make `KeyedBuildResult` a subclass to just avoid this. Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2022-03-25 03:26:07 +02:00
BuildResult Goal::getBuildResult(const DerivedPath & req) {
BuildResult res { buildResult };
if (auto pbp = std::get_if<DerivedPath::Built>(&req)) {
auto & bp = *pbp;
/* Because goals are in general shared between derived paths
that share the same derivation, we need to filter their
results to get back just the results we care about.
*/
for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) {
if (bp.outputs.contains(it->first))
Make `KeyedBuildResult`, `BuildResult` like before, and fix bug another way In https://github.com/NixOS/nix/pull/6311#discussion_r834863823, I realized since derivation goals' wanted outputs can "grow" due to overlapping dependencies (See `DerivationGoal::addWantedOutputs`, called by `Worker::makeDerivationGoalCommon`), the previous bug fix had an unfortunate side effect of causing more pointless rebuilds. In paticular, we have this situation: 1. Goal made from `DerivedPath::Built { foo, {a} }`. 2. Goal gives on on substituting, starts building. 3. Goal made from `DerivedPath::Built { foo, {b} }`, in fact is just modified original goal. 4. Though the goal had gotten as far as building, so all outputs were going to be produced, `addWantedOutputs` no longer knows that and so the goal is flagged to be restarted. This might sound far-fetched with input-addressed drvs, where we usually basically have all our goals "planned out" before we start doing anything, but with CA derivation goals and especially RFC 92, where *drv resolution* means goals are created after some building is completed, it is more likely to happen. So the first thing to do was restore the clearing of `wantedOutputs` we used to do, and then filter the outputs in `buildPathsWithResults` to only get the ones we care about. But fix also has its own side effect in that the `DerivedPath` in the `BuildResult` in `DerivationGoal` cannot be trusted; it is merely the *first* `DerivedPath` for which this goal was originally created. To remedy this, I made `BuildResult` be like it was before, and instead made `KeyedBuildResult` be a subclass wit the path. Only `buildPathsWithResults` returns `KeyedBuildResult`s, everything else just becomes like it was before, where the "key" is unambiguous from context. I think separating the "primary key" field(s) from the other fields is good practical in general anyways. (I would like to do the same thing for `ValidPathInfo`.) Among other things, it allows constructions like `std::map<Key, ThingWithKey>` where doesn't contain duplicate keys and just precludes the possibility of those duplicate keys being out of sync. We might leverage the above someday to overload `buildPathsWithResults` to take a *set* of return a *map* per the above. ----- Unfortunately, we need to avoid C++20 strictness on designated initializers. (BTW https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2287r1.html this offers some new syntax for this use-case. Hopefully this will be adopted and we can eventually use it.) No having that yet, maybe it would be better to not make `KeyedBuildResult` a subclass to just avoid this. Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
2022-03-25 03:26:07 +02:00
++it;
else
it = res.builtOutputs.erase(it);
}
}
return res;
}
2014-03-30 01:49:23 +02:00
void addToWeakGoals(WeakGoals & goals, GoalPtr p)
{
if (goals.find(p) != goals.end())
return;
goals.insert(p);
2014-03-30 01:49:23 +02:00
}
void Goal::addWaitee(GoalPtr waitee)
{
waitees.insert(waitee);
2014-03-30 01:49:23 +02:00
addToWeakGoals(waitee->waiters, shared_from_this());
}
void Goal::waiteeDone(GoalPtr waitee, ExitCode result)
{
2022-03-25 00:24:10 +02:00
assert(waitees.count(waitee));
waitees.erase(waitee);
trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size()));
2012-07-27 16:59:18 +03:00
if (result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure) ++nrFailed;
if (result == ecNoSubstituters) ++nrNoSubstituters;
2012-07-27 16:59:18 +03:00
if (result == ecIncompleteClosure) ++nrIncompleteClosure;
if (waitees.empty() || (result == ecFailed && !settings.keepGoing)) {
/* If we failed and keepGoing is not set, we remove all
remaining waitees. */
2015-07-17 20:24:28 +03:00
for (auto & goal : waitees) {
goal->waiters.extract(shared_from_this());
}
waitees.clear();
worker.wakeUp(shared_from_this());
}
}
void Goal::amDone(ExitCode result, std::optional<Error> ex)
{
trace("done");
assert(exitCode == ecBusy);
assert(result == ecSuccess || result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure);
exitCode = result;
if (ex) {
if (!waiters.empty())
logError(ex->info());
else
this->ex = std::move(*ex);
}
2015-07-17 20:24:28 +03:00
for (auto & i : waiters) {
GoalPtr goal = i.lock();
if (goal) goal->waiteeDone(shared_from_this(), result);
}
waiters.clear();
worker.removeGoal(shared_from_this());
cleanup();
}
void Goal::trace(std::string_view s)
{
debug("%1%: %2%", name, s);
}
}