nixos/nix
1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-09 12:06:01 +01:00
Code Activity

Merge pull request #13866 from obsidiansystems/more-derivation-builder-cleanup

Browse source 
Even more `DerivationBuilder` cleanup
This commit is contained in:
Jörg Thalheim 2025-09-01 20:35:16 +02:00 • committed by GitHub
commit a0ce514769
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 9 additions and 12 deletions
Download patch file Download diff file Expand all files Collapse all files

3
src/libstore/build/derivation-building-goal.cc
View file

@ -721,9 +721,11 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
try { try {
builtOutputs = builder->unprepareBuild(); builtOutputs = builder->unprepareBuild();
} catch (BuilderFailureError & e) { } catch (BuilderFailureError & e) {
builder.reset();
outputLocks.unlock(); outputLocks.unlock();
co_return doneFailure(fixupBuilderFailureErrorMessage(std::move(e))); co_return doneFailure(fixupBuilderFailureErrorMessage(std::move(e)));
} catch (BuildError & e) { } catch (BuildError & e) {
builder.reset();
outputLocks.unlock(); outputLocks.unlock();
// Allow selecting a subset of enum values // Allow selecting a subset of enum values
# pragma GCC diagnostic push # pragma GCC diagnostic push
@ -748,6 +750,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
co_return doneFailure(std::move(e)); co_return doneFailure(std::move(e));
} }
{ {
builder.reset();
StorePathSet outputPaths; StorePathSet outputPaths;
for (auto & [_, output] : builtOutputs) { for (auto & [_, output] : builtOutputs) {
// for sake of `bmRepair` // for sake of `bmRepair`

18
src/libstore/unix/build/derivation-builder.cc
View file

@ -485,14 +485,6 @@ bool DerivationBuilderImpl::prepareBuild()
SingleDrvOutputs DerivationBuilderImpl::unprepareBuild() SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
{ {
// FIXME: get rid of this, rely on RAII.
Finally releaseBuildUser([&]() {
/* Release the build user at the end of this function. We don't do
it right away because we don't want another build grabbing this
uid and then messing around with our output. */
buildUser.reset();
});
/* Since we got an EOF on the logger pipe, the builder is presumed /* Since we got an EOF on the logger pipe, the builder is presumed
to have terminated. In fact, the builder could also have to have terminated. In fact, the builder could also have
simply have closed its end of the pipe, so just to be sure, simply have closed its end of the pipe, so just to be sure,
@ -551,10 +543,6 @@ SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
being valid. */ being valid. */
auto builtOutputs = registerOutputs(); auto builtOutputs = registerOutputs();
/* Delete unused redirected outputs (when doing hash rewriting). */
for (auto & i : redirectedOutputs)
deletePath(store.Store::toRealPath(i.second));
cleanupBuild(true); cleanupBuild(true);
return builtOutputs; return builtOutputs;
@ -1858,6 +1846,12 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
void DerivationBuilderImpl::cleanupBuild(bool force) void DerivationBuilderImpl::cleanupBuild(bool force)
{ {
if (force) {
/* Delete unused redirected outputs (when doing hash rewriting). */
for (auto & i : redirectedOutputs)
deletePath(store.Store::toRealPath(i.second));
}
if (topTmpDir != "") { if (topTmpDir != "") {
/* As an extra precaution, even in the event of `deletePath` failing to /* As an extra precaution, even in the event of `deletePath` failing to
* clean up, the `tmpDir` will be chowned as if we were to move * clean up, the `tmpDir` will be chowned as if we were to move