1
1
Fork 0
mirror of https://github.com/NixOS/nix.git synced 2025-11-15 15:02:42 +01:00

Combine cleanupBuild and deleteTmpDir

It's hard to tell if I changed any behavior, but if I did, I think I
made it better, because now we explicitly move stuff out of the chroot
(if we were going to) before trying to delete the chroot.
This commit is contained in:
John Ericson 2025-08-28 12:26:29 -04:00
parent 4db6bf96b7
commit 557bbe969e
4 changed files with 10 additions and 24 deletions

View file

@ -66,7 +66,7 @@ DerivationBuildingGoal::~DerivationBuildingGoal()
ignoreExceptionInDestructor(); ignoreExceptionInDestructor();
} }
try { try {
builder->deleteTmpDir(false); builder->cleanupBuild(false);
} catch (...) { } catch (...) {
ignoreExceptionInDestructor(); ignoreExceptionInDestructor();
} }

View file

@ -200,8 +200,11 @@ struct DerivationBuilder : RestrictionContext
/** /**
* Delete the temporary directory, if we have one. * Delete the temporary directory, if we have one.
*
* @param force We know the build suceeded, so don't attempt to
* preseve anything for debugging.
*/ */
virtual void deleteTmpDir(bool force) = 0; virtual void cleanupBuild(bool force) = 0;
/** /**
* Kill any processes running under the build user UID or in the * Kill any processes running under the build user UID or in the

View file

@ -22,13 +22,6 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
PathsInChroot pathsInChroot; PathsInChroot pathsInChroot;
void deleteTmpDir(bool force) override
{
autoDelChroot.reset(); /* this runs the destructor */
DerivationBuilderImpl::deleteTmpDir(force);
}
bool needsHashRewrite() override bool needsHashRewrite() override
{ {
return false; return false;
@ -170,12 +163,9 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
{ {
DerivationBuilderImpl::cleanupBuild(force); DerivationBuilderImpl::cleanupBuild(force);
if (force)
return;
/* Move paths out of the chroot for easier debugging of /* Move paths out of the chroot for easier debugging of
build failures. */ build failures. */
if (buildMode == bmNormal) if (!force && buildMode == bmNormal)
for (auto & [_, status] : initialOutputs) { for (auto & [_, status] : initialOutputs) {
if (!status.known) if (!status.known)
continue; continue;
@ -185,6 +175,8 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
if (pathExists(chrootRootDir + p)) if (pathExists(chrootRootDir + p))
std::filesystem::rename((chrootRootDir + p), p); std::filesystem::rename((chrootRootDir + p), p);
} }
autoDelChroot.reset(); /* this runs the destructor */
} }
std::pair<Path, Path> addDependencyPrep(const StorePath & path) std::pair<Path, Path> addDependencyPrep(const StorePath & path)

View file

@ -344,14 +344,10 @@ private:
public: public:
void deleteTmpDir(bool force) override; void cleanupBuild(bool force) override;
void killSandbox(bool getStats) override; void killSandbox(bool getStats) override;
protected:
virtual void cleanupBuild(bool force);
private: private:
bool decideWhetherDiskFull(); bool decideWhetherDiskFull();
@ -513,11 +509,6 @@ SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
return builtOutputs; return builtOutputs;
} }
void DerivationBuilderImpl::cleanupBuild(bool force)
{
deleteTmpDir(force);
}
static void chmod_(const Path & path, mode_t mode) static void chmod_(const Path & path, mode_t mode)
{ {
if (chmod(path.c_str(), mode) == -1) if (chmod(path.c_str(), mode) == -1)
@ -1821,7 +1812,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
return builtOutputs; return builtOutputs;
} }
void DerivationBuilderImpl::deleteTmpDir(bool force) void DerivationBuilderImpl::cleanupBuild(bool force)
{ {
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