From 4bb97b2a9b4e96b825e621810900ce077fdf7efb Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 19 Jan 2025 16:40:26 +0100 Subject: [PATCH] libstore: don't return promises from LocalDerivationGoal::startBuilder this is misleading at best and dangerous at worst. callers expect all exceptions thrown here to signal build startup failure, and errors in the build itself to be signaled through the promise. since this isn't a virtual method it will always return the same kind of promise, i.e. we can just call handleChildOutput in the one caller of startBuilder. Change-Id: I65d06f96ec05a3fcdf2050dbbd87c417c308ecc9 --- lix/libstore/build/local-derivation-goal.cc | 10 +++------- lix/libstore/build/local-derivation-goal.hh | 2 +- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index 625f33cba..060d4acca 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -247,10 +247,10 @@ retry: try { /* Okay, we have to build. */ - auto promise = startBuilder(); + startBuilder(); started(); - auto r = co_await promise; + auto r = co_await handleChildOutput(); if (r.has_value()) { // all good so far } else if (r.has_error()) { @@ -399,9 +399,7 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck() cleanupPostOutputsRegisteredModeCheck(); } -// NOTE this one isn't noexcept because it's called from places that expect -// exceptions to signal failure to launch. we should change this some time. -kj::Promise> LocalDerivationGoal::startBuilder() +void LocalDerivationGoal::startBuilder() { if ((buildUser && buildUser->getUIDCount() != 1) #if __linux__ @@ -789,8 +787,6 @@ kj::Promise> LocalDerivationGoal::startBuilder() debug("sandbox setup: " + msg); msgs.push_back(std::move(msg)); } - - return handleChildOutput(); } diff --git a/lix/libstore/build/local-derivation-goal.hh b/lix/libstore/build/local-derivation-goal.hh index 96f0c41a0..6c8ec4976 100644 --- a/lix/libstore/build/local-derivation-goal.hh +++ b/lix/libstore/build/local-derivation-goal.hh @@ -218,7 +218,7 @@ struct LocalDerivationGoal : public DerivationGoal /** * Start building a derivation. */ - kj::Promise> startBuilder(); + void startBuilder(); /** * Fill in the environment for the builder.