diff --git a/src/libstore/build/derivation-builder-impl.cc b/src/libstore/build/derivation-builder-impl.cc index 972b4794ec7..8befc097294 100644 --- a/src/libstore/build/derivation-builder-impl.cc +++ b/src/libstore/build/derivation-builder-impl.cc @@ -4,6 +4,7 @@ #include "nix/store/local-store.hh" #include "nix/store/path-references.hh" #include "nix/store/posix-fs-canonicalise.hh" +#include "nix/store/restricted-store.hh" #include "nix/util/archive.hh" #include "nix/util/file-content-address.hh" #include "nix/util/file-system.hh" @@ -100,7 +101,7 @@ static void replaceValidPath(const std::filesystem::path & storePath, const std: deletePath(oldPath); } -SingleDrvOutputs DerivationBuilderImpl::registerOutputs() +SingleDrvOutputs DerivationBuilderImpl::registerOutputs(LocalStore & localStore) { std::map infos; @@ -174,7 +175,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() throw BuildError( BuildResult::Failure::OutputRejected, "builder for '%s' failed to produce output path for output '%s' at %s", - store.printStorePath(drvPath), + store->printStorePath(drvPath), outputName, PathFmt(actualPath)); PosixStat & st = *optSt; @@ -244,7 +245,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() BuildResult::Failure::OutputRejected, "no output reference for '%s' in build of '%s'", name, - store.printStorePath(drvPath)); + store->printStorePath(drvPath)); return std::visit( overloaded{ /* Since we'll use the already installed versions of these, we @@ -263,7 +264,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() throw BuildError( BuildResult::Failure::OutputRejected, "cycle detected in build of '%s' in the references of output '%s' from output '%s'", - store.printStorePath(drvPath), + store->printStorePath(drvPath), cycle.path, cycle.parent); }, @@ -326,7 +327,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* Put the temporary copy in a directory inaccessible to the builder. actualPath might point inside the build chroot, which is controlled by the derivation builder. */ - auto [rewriteTempDir, rewriteTempDirFd] = store.createTempDirInStore(); + auto [rewriteTempDir, rewriteTempDirFd] = localStore.createTempDirInStore(); AutoDelete delRewriteTempDir(rewriteTempDir); std::filesystem::path tmpPath = rewriteTempDir / "x"; restorePath(tmpPath, *source); @@ -410,7 +411,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() }(); auto newInfo0 = ValidPathInfo::makeFromCA( - store, + *store, outputPathName(drv.name, outputName), ContentAddressWithReferences::fromParts(outputHash.method, std::move(got), rewriteRefs()), Hash::dummy); @@ -438,7 +439,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() auto moveOutputToTempDir = [&]() -> void { std::filesystem::path tempDir; - std::tie(tempDir, tempDirFd) = store.createTempDirInStore(); + std::tie(tempDir, tempDirFd) = localStore.createTempDirInStore(); delTempDir = AutoDelete(tempDir); auto tmpOutput = tempDir / "x"; @@ -446,7 +447,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* Copy files to break stale file descriptors. copyRecursive below will use reflinking to optimise the copying overhead. */ auto pathAccessor = makeFSSourceAccessor(actualPath); - RestoreSink restoreSink{store.config->getLocalSettings().fsyncStorePaths}; + RestoreSink restoreSink{store->getLocalSettings().fsyncStorePaths}; restoreSink.dstPath = tmpOutput; copyRecursive(*pathAccessor, CanonPath::root, restoreSink, CanonPath::root); /* This makes it slightly harder to make sense of the control flow. The rule @@ -471,7 +472,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() {makeFSSourceAccessor(actualPath), CanonPath::root}, FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); - ValidPathInfo newInfo0{requiredFinalPath, {store, narHashAndSize.hash}}; + ValidPathInfo newInfo0{requiredFinalPath, {*store, narHashAndSize.hash}}; newInfo0.narSize = narHashAndSize.numBytesDigested; auto refs = rewriteRefs(); newInfo0.references = std::move(refs.others); @@ -525,51 +526,51 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* Calculate where we'll move the output files. In the checking case we will leave leave them where they are, for now, rather than move to their usual "final destination" */ - auto finalDestPath = store.printStorePath(newInfo.path); + auto finalDestPath = store->printStorePath(newInfo.path); /* Lock final output path, if not already locked. This happens with floating CA derivations and hash-mismatching fixed-output derivations. */ PathLocks dynamicOutputLock; dynamicOutputLock.setDeletion(true); - auto optFixedPath = output->path(store, drv.name, outputName); - if (!optFixedPath || store.printStorePath(*optFixedPath) != finalDestPath) { + auto optFixedPath = output->path(*store, drv.name, outputName); + if (!optFixedPath || store->printStorePath(*optFixedPath) != finalDestPath) { assert(newInfo.ca); /* Don't wait on lock for the hash-mismatching fixed-output derivation case, to avoid a deadlock in the case where a build with the correct hash is in progress. */ - bool locked = dynamicOutputLock.lockPaths({store.toRealPath(newInfo.path)}, "", !optFixedPath); + bool locked = dynamicOutputLock.lockPaths({store->toRealPath(newInfo.path)}, "", !optFixedPath); /* If we can't lock the correct path, clean up and bail now. */ if (!locked) { debug( "failed to lock correct output path of %s, namely %s, not moving output", - store.printStorePath(drvPath), - PathFmt(store.toRealPath(newInfo.path))); + store->printStorePath(drvPath), + PathFmt(store->toRealPath(newInfo.path))); deletePath(actualPath); /* Trigger the hash-mismatch error. */ - checkCAOutput(store, drvPath, *output, newInfo, outputName); + checkCAOutput(*store, drvPath, *output, newInfo, outputName); unreachable(); } } /* Move files, if needed */ - if (store.toRealPath(newInfo.path) != actualPath) { + if (store->toRealPath(newInfo.path) != actualPath) { if (buildMode == bmRepair) { /* Path already exists, need to replace it */ - replaceValidPath(store.toRealPath(newInfo.path), actualPath); + replaceValidPath(store->toRealPath(newInfo.path), actualPath); } else if (buildMode == bmCheck) { /* Path already exists, and we want to compare, so we leave out new path in place. */ - } else if (store.isValidPath(newInfo.path)) { + } else if (localStore.isValidPath(newInfo.path)) { /* Path already exists because CA path produced by something else. No moving needed. */ assert(newInfo.ca); /* Can delete our scratch copy now. */ deletePath(actualPath); } else { - auto destPath = store.toRealPath(newInfo.path); + auto destPath = store->toRealPath(newInfo.path); deletePath(destPath); movePath(actualPath, destPath); } @@ -578,12 +579,12 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() if (buildMode == bmCheck) { /* Check against already registered outputs */ - if (store.isValidPath(newInfo.path)) { - ValidPathInfo oldInfo(*store.queryPathInfo(newInfo.path)); + if (localStore.isValidPath(newInfo.path)) { + ValidPathInfo oldInfo(*localStore.queryPathInfo(newInfo.path)); if (newInfo.narHash != oldInfo.narHash) { auto * diffHook = localSettings.getDiffHook(); if (diffHook || settings.keepFailed) { - auto dst = store.toRealPath(newInfo.path); + auto dst = store->toRealPath(newInfo.path); dst += ".check"; deletePath(dst); movePath(actualPath, dst); @@ -597,27 +598,27 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() #endif finalDestPath, dst, - store.printStorePath(drvPath), + store->printStorePath(drvPath), tmpDir); } throw NotDeterministic( "derivation '%s' may not be deterministic: output %s differs from %s", - store.printStorePath(drvPath), - PathFmt(store.toRealPath(newInfo.path)), + store->printStorePath(drvPath), + PathFmt(store->toRealPath(newInfo.path)), PathFmt(dst)); } else throw NotDeterministic( "derivation '%s' may not be deterministic: output %s differs", - store.printStorePath(drvPath), - PathFmt(store.toRealPath(newInfo.path))); + store->printStorePath(drvPath), + PathFmt(store->toRealPath(newInfo.path))); } /* Since we verified the build, it's now ultimately trusted. */ if (!oldInfo.ultimate) { oldInfo.ultimate = true; - store.signPathInfo(oldInfo); - store.registerValidPaths({{oldInfo.path, oldInfo}}); + localStore.signPathInfo(oldInfo); + localStore.registerValidPaths({{oldInfo.path, oldInfo}}); } } } else { @@ -626,17 +627,18 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* For debugging, print out the referenced and unreferenced paths. */ for (auto & i : inputPaths) { if (references.count(i)) - debug("referenced input: '%1%'", store.printStorePath(i)); + debug("referenced input: '%1%'", store->printStorePath(i)); else - debug("unreferenced input: '%1%'", store.printStorePath(i)); + debug("unreferenced input: '%1%'", store->printStorePath(i)); } - if (!store.isValidPath(newInfo.path)) - store.optimisePath(store.toRealPath(newInfo.path), NoRepair); // FIXME: combine with scanForReferences() + if (!localStore.isValidPath(newInfo.path)) + localStore.optimisePath( + store->toRealPath(newInfo.path), NoRepair); // FIXME: combine with scanForReferences() newInfo.deriver = drvPath; newInfo.ultimate = true; - store.signPathInfo(newInfo); + localStore.signPathInfo(newInfo); finish(newInfo.path); @@ -651,7 +653,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() possibly quite slow thing it was) doesn't have to be done again. */ if (newInfo.ca) - store.registerValidPaths({{newInfo.path, newInfo}}); + localStore.registerValidPaths({{newInfo.path, newInfo}}); } /* Do this in both the check and non-check cases, because we @@ -662,7 +664,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() /* Apply output checks. This includes checking of the wanted vs got hash of fixed-outputs. */ - checkOutputs(store, drvPath, drv, drvOptions.outputChecks, infos); + checkOutputs(localStore, drvPath, drv, drvOptions.outputChecks, infos); if (buildMode == bmCheck) { return {}; @@ -676,7 +678,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() for (auto & [outputName, newInfo] : infos) { infos2.insert_or_assign(newInfo.path, newInfo); } - store.registerValidPaths(infos2); + localStore.registerValidPaths(infos2); } /* If we made it this far, we are sure the output matches the @@ -699,8 +701,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() }, }; if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations) && !type(drv).isImpure()) { - store.signRealisation(thisRealisation); - store.registerDrvOutput(thisRealisation, NoCheckSigs); + localStore.signRealisation(thisRealisation); + localStore.registerDrvOutput(thisRealisation, NoCheckSigs); } builtOutputs.emplace(outputName, thisRealisation); } @@ -708,7 +710,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() return builtOutputs; } -SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() +SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs(LocalStore & localStore) { // Submitted outputs from the recursive nix daemon // It's fine to lock here since all other threads with the reference have been shut down. @@ -719,12 +721,12 @@ SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() std::map infos; for (auto & [outputName, outputPath] : *submittedOutputs) { - infos.emplace(outputName, *store.queryPathInfo(outputPath)); + infos.emplace(outputName, *localStore.queryPathInfo(outputPath)); } // checkOutputs only performs checks that make sense for both submitting and non-submitting derivations, // more verification steps needed afterward - checkOutputs(store, drvPath, drv, drvOptions.outputChecks, infos); + checkOutputs(localStore, drvPath, drv, drvOptions.outputChecks, infos); for (auto & [outputName, output] : drv.outputs) { // For some reason cannot be moved to checkOutputs, needs debugging @@ -732,7 +734,7 @@ SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() throw BuildError( BuildResult::Failure::OutputRejected, "builder for '%s' failed to submit output path for '%s'", - store.printStorePath(drvPath), + store->printStorePath(drvPath), outputName); } @@ -754,8 +756,8 @@ SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() }, }; - store.signRealisation(realisation); - store.registerDrvOutput(realisation, NoCheckSigs); + localStore.signRealisation(realisation); + localStore.registerDrvOutput(realisation, NoCheckSigs); builtOutputs.emplace(outputName, realisation); // TODO: handle --check @@ -764,4 +766,55 @@ SingleDrvOutputs DerivationBuilderImpl::checkSubmittedOutputs() return builtOutputs; } +BuildingStore::~BuildingStore() = default; + +namespace { + +struct LocalBuildingStore : BuildingStore +{ + LocalStore & localStore; + + LocalBuildingStore(LocalStore & localStore) + : BuildingStore{localStore.storeDir} + , localStore{localStore} + { + } + + std::filesystem::path getRealStoreDir() const override + { + return localStore.config->realStoreDir.get(); + } + + std::filesystem::path getBuildDir() const override + { + return localStore.config->getBuildDir(); + } + + const LocalSettings & getLocalSettings() const override + { + return localStore.config->getLocalSettings(); + } + + ref makeRecursiveNixStore(RestrictionContext & ctx) override + { + return makeRestrictedStore( + [&] { + auto config = make_ref(*localStore.config); + config->pathInfoCacheSize = 0; + config->stateDir = "/no-such-path"; + config->logDir = "/no-such-path"; + return config; + }(), + ref(std::dynamic_pointer_cast(localStore.shared_from_this())), + ctx); + } +}; + +} // namespace + +std::unique_ptr makeBuildingStoreFromLocalStore(LocalStore & localStore) +{ + return std::make_unique(localStore); +} + } // namespace nix diff --git a/src/libstore/build/derivation-builder-impl.hh b/src/libstore/build/derivation-builder-impl.hh index 6af88f1f0c3..b039b39eff3 100644 --- a/src/libstore/build/derivation-builder-impl.hh +++ b/src/libstore/build/derivation-builder-impl.hh @@ -36,7 +36,7 @@ protected: */ Pid pid; - LocalStore & store; + std::shared_ptr store; std::shared_ptr miscMethods; @@ -52,7 +52,7 @@ protected: */ const derivation::Type derivationType; - const LocalSettings & localSettings = store.config->getLocalSettings(); + const LocalSettings & localSettings = store->getLocalSettings(); #ifndef _WIN32 /** @@ -92,41 +92,31 @@ protected: */ virtual std::filesystem::path realPathInHost(const StorePath & p) { - return store.toRealPath(p); + return store->toRealPath(p); } public: DerivationBuilderImpl( - LocalStore & store, std::shared_ptr miscMethods, DerivationBuilderParams params) + std::shared_ptr store, + std::shared_ptr miscMethods, + DerivationBuilderParams params) : DerivationBuilderParams{std::move(params)} - , store{store} + , store{std::move(store)} , miscMethods{std::move(miscMethods)} , derivationType{derivation::type(drv)} { } -protected: - - /** - * Check that the derivation outputs all exist and register them - * as valid. - * - * For subclasses to call at the end of `unprepareBuild`. - */ - SingleDrvOutputs registerOutputs(); + SingleDrvOutputs registerOutputs(LocalStore & localStore) override; /** * Output paths from the `SubmitOutput` store command */ Sync submittedOutputs; - /** - * Check that the derivation outputs submitted by recursive-nix exist - * and attach them to the derivation - */ - SingleDrvOutputs checkSubmittedOutputs(); + SingleDrvOutputs checkSubmittedOutputs(LocalStore & localStore) override; }; } // namespace nix diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index e336eca6723..80c8f3dc25b 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -967,13 +967,13 @@ Goal::Co DerivationBuildingGoal::buildLocally( throw UnimplementedError("external builders are not yet supported on Windows") #else makeExternalDerivationBuilder( - localBuildCap.localStore, + makeBuildingStoreFromLocalStore(localBuildCap.localStore), std::make_shared(*this, openLogFile, closeLogFile), std::move(params), *localBuildCap.externalBuilder) #endif : makeDerivationBuilder( - localBuildCap.localStore, + makeBuildingStoreFromLocalStore(localBuildCap.localStore), std::make_shared(*this, openLogFile, closeLogFile), std::move(params) #ifdef _WIN32 @@ -1030,9 +1030,32 @@ Goal::Co DerivationBuildingGoal::buildLocally( trace("build done"); + auto [status, diskFull] = builder->unprepareBuild(); + + /* Check the exit status. */ + if (!statusOk(status)) { + builder->cleanupBuild(false); + builder.reset(); + outputLocks.unlock(); + co_return doneFailure(fixupBuilderFailureErrorMessage( + { + !derivation::type(*drv).isSandboxed() || diskFull ? BuildResult::Failure::TransientFailure + : BuildResult::Failure::PermanentFailure, + status, + diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "", + }, + *buildLog)); + } + SingleDrvOutputs builtOutputs; try { - builtOutputs = builder->unprepareBuild(); + /* Compute the FS closure of the outputs and register them as + being valid. With builder-rpc-v0 the builder already submitted + the outputs, so check those instead. */ + builtOutputs = drvOptions.getRequiredSystemFeatures(*drv).count(std::string{drvFeatureBuilderRpcV0}) + ? builder->checkSubmittedOutputs(localBuildCap.localStore) + : builder->registerOutputs(localBuildCap.localStore); + builder->cleanupBuild(true); } catch (BuilderFailureError & e) { builder.reset(); outputLocks.unlock(); diff --git a/src/libstore/build/derivation-check.cc b/src/libstore/build/derivation-check.cc index d432af46b93..537116799ac 100644 --- a/src/libstore/build/derivation-check.cc +++ b/src/libstore/build/derivation-check.cc @@ -10,7 +10,7 @@ namespace nix { void checkCAOutput( - StoreDirConfig & store, + const StoreDirConfig & store, const StorePath & drvPath, const DerivationOutput & outputSpec, const ValidPathInfo & info, diff --git a/src/libstore/build/derivation-check.hh b/src/libstore/build/derivation-check.hh index 5c8c75da172..a8eeac0fdef 100644 --- a/src/libstore/build/derivation-check.hh +++ b/src/libstore/build/derivation-check.hh @@ -13,7 +13,7 @@ namespace nix { * Do nothing if outputSpec is not a CAFixed or CAFloating output. */ void checkCAOutput( - StoreDirConfig & store, + const StoreDirConfig & store, const StorePath & drvPath, const DerivationOutput & outputSpec, const ValidPathInfo & info, diff --git a/src/libstore/darwin/build/darwin-derivation-builder.cc b/src/libstore/darwin/build/darwin-derivation-builder.cc index 1e403d49a13..1b2151b80f0 100644 --- a/src/libstore/darwin/build/darwin-derivation-builder.cc +++ b/src/libstore/darwin/build/darwin-derivation-builder.cc @@ -63,7 +63,7 @@ void DarwinDerivationBuilder::setUser() /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost path component this time, since it's typically /nix/store and we care about that. */ - std::filesystem::path cur = store.storeDir; + std::filesystem::path cur = store->storeDir; while (cur != "/") { ancestry.insert(cur.native()); cur = cur.parent_path(); @@ -71,13 +71,13 @@ void DarwinDerivationBuilder::setUser() /* Add all our input paths to the chroot */ for (auto & i : inputPaths) { - auto p = store.printStorePath(i); + auto p = store->printStorePath(i); pathsInChroot.insert_or_assign(p, ChrootPath{.source = p}); } /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be * configurable */ - if (store.config->getLocalSettings().darwinLogSandboxViolations) { + if (store->getLocalSettings().darwinLogSandboxViolations) { sandboxProfile += "(deny default)\n"; } else { sandboxProfile += "(deny default (with no-log))\n"; @@ -95,7 +95,7 @@ void DarwinDerivationBuilder::setUser() /* Add the output paths we'll use at build-time to the chroot */ sandboxProfile += "(allow file-read* file-write* process-exec\n"; for (auto & [_, path] : scratchOutputs) - sandboxProfile += fmt("\t(subpath \"%s\")\n", store.printStorePath(path)); + sandboxProfile += fmt("\t(subpath \"%s\")\n", store->printStorePath(path)); sandboxProfile += ")\n"; diff --git a/src/libstore/darwin/build/darwin-derivation-builder.hh b/src/libstore/darwin/build/darwin-derivation-builder.hh index 28263153eeb..8cb111be5b9 100644 --- a/src/libstore/darwin/build/darwin-derivation-builder.hh +++ b/src/libstore/darwin/build/darwin-derivation-builder.hh @@ -15,7 +15,7 @@ struct DarwinDerivationBuilder : UnixDerivationBuilderImpl bool useSandbox; DarwinDerivationBuilder( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, bool useSandbox) diff --git a/src/libstore/freebsd/build/chroot-freebsd-derivation-builder.hh b/src/libstore/freebsd/build/chroot-freebsd-derivation-builder.hh index cf1ec534ada..38910f39340 100644 --- a/src/libstore/freebsd/build/chroot-freebsd-derivation-builder.hh +++ b/src/libstore/freebsd/build/chroot-freebsd-derivation-builder.hh @@ -12,7 +12,9 @@ struct ChrootFreeBSDDerivationBuilder : ChrootDerivationBuilder, FreeBSDDerivati std::shared_ptr autoDelJail = std::make_shared(); ChrootFreeBSDDerivationBuilder( - LocalStore & store, std::shared_ptr miscMethods, DerivationBuilderParams params) + std::shared_ptr store, + std::shared_ptr miscMethods, + DerivationBuilderParams params) : UnixDerivationBuilderImpl{store, miscMethods, params} , ChrootDerivationBuilder{store, miscMethods, params} , FreeBSDDerivationBuilder{store, miscMethods, params} diff --git a/src/libstore/freebsd/build/freebsd-derivation-builder.cc b/src/libstore/freebsd/build/freebsd-derivation-builder.cc index 00bb17ee58b..ad3819d2454 100644 --- a/src/libstore/freebsd/build/freebsd-derivation-builder.cc +++ b/src/libstore/freebsd/build/freebsd-derivation-builder.cc @@ -210,7 +210,7 @@ void ChrootFreeBSDDerivationBuilder::prepareSandbox() .uid = 0, .gid = 0, .description = "Nix build user", - .home = store.config->getLocalSettings().sandboxBuildDir, + .home = store->getLocalSettings().sandboxBuildDir, .shell = "/noshell", }, { @@ -218,7 +218,7 @@ void ChrootFreeBSDDerivationBuilder::prepareSandbox() .uid = buildUser->getUID(), .gid = sandboxGid(), .description = "Nix build user", - .home = store.config->getLocalSettings().sandboxBuildDir, + .home = store->getLocalSettings().sandboxBuildDir, .shell = "/noshell", }, { @@ -303,7 +303,7 @@ void ChrootFreeBSDDerivationBuilder::prepareSandbox() debug("setting up a nullfs mount from %1% to %2%", PathFmt(chrootPath.source), PathFmt(path)); int flags = 0; - if (store.isInStore(target.native())) + if (store->isInStore(target.native())) /* While we are at it, enforce invariants about store paths. Anything located at the "logical" store location must be readonly (file permission canonicalisation enforces this on the host filesystem). Also the store must never contain setuid binaries for the same reason. This is just defense-in-depth. */ @@ -478,7 +478,7 @@ void ChrootFreeBSDDerivationBuilder::enterChroot() void ChrootFreeBSDDerivationBuilder::addDependencyImpl(const StorePath & path) { throw UnimplementedError( - "adding store path '%s' to the sandbox is not implemented (recursive-nix)", store.printStorePath(path)); + "adding store path '%s' to the sandbox is not implemented (recursive-nix)", store->printStorePath(path)); } } // namespace nix diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index 880c2dfbf0c..eedfc0c18ea 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -155,6 +155,23 @@ struct DerivationBuilderCallbacks daemon::RecursiveFlag recursiveFlag) = 0; }; +/** + * The outcome of tearing down the build environment, from + * `DerivationBuilder::unprepareBuild`. + */ +struct BuilderExit +{ + /** + * The builder's exit status. + */ + int status; + + /** + * Whether the disk seemed full when the builder exited. + */ + bool diskFull = false; +}; + /** * This class represents the state for building locally. * @@ -234,14 +251,34 @@ public: * Tear down build environment after the builder exits (either on * its own or if it is killed). * - * @returns The first case indicates failure during output - * processing. A status code and exception are returned, providing - * more information. The second case indicates success, and - * realisations for each output of the derivation are returned. + * @returns The builder's exit status and whether the disk seemed + * full at exit time. + */ + virtual BuilderExit unprepareBuild() = 0; + + /** + * Check that the derivation outputs all exist and register them + * as valid. + * + * Not used with `builder-rpc-v0`; see `checkSubmittedOutputs`. + */ + virtual SingleDrvOutputs registerOutputs(LocalStore & store) = 0; + + /** + * Check that the derivation outputs submitted by recursive-nix + * exist and attach them to the derivation. + * + * Only used with `builder-rpc-v0`. + */ + virtual SingleDrvOutputs checkSubmittedOutputs(LocalStore & store) = 0; + + /** + * Delete the temporary directory, if we have one. * - * @throws BuildError + * @param force We know the build succeeded, so don't attempt to + * preserve anything for debugging. */ - virtual SingleDrvOutputs unprepareBuild() = 0; + virtual void cleanupBuild(bool force) = 0; /** * Forcibly kill the child process, if any. @@ -272,6 +309,50 @@ struct ExternalBuilder std::vector args; }; +struct LocalSettings; + +/** + * This type exists to cut down @ref Store to what @ref DerivationBuilder + * actually needs. This serves two purposes: + * + * - Modularity: Now we better understand the requirements different + * components impose on one another. Conversely, @ref Store is more + * or less the union of many interfaces' requirements, so it obscures + * the division of labor to require it everywhere. + * + * - FFI: we cannot make a full @ref LocalStore with everything + * (including building, which uses this!) from FFI, but we do have a + * chance of making something that just has the methods we actually + * need from @ref LocalStore. + */ +struct BuildingStore : StoreDirConfig +{ + BuildingStore(const std::string & storeDir) + : StoreDirConfig{storeDir} + { + } + + virtual ~BuildingStore(); + + virtual std::filesystem::path getRealStoreDir() const = 0; + + virtual std::filesystem::path getBuildDir() const = 0; + + virtual const LocalSettings & getLocalSettings() const = 0; + + /** + * Make the store that recursive-Nix daemon connections talk to. + */ + virtual ref makeRecursiveNixStore(RestrictionContext & ctx) = 0; + + std::filesystem::path toRealPath(const StorePath & storePath) const + { + return getRealStoreDir() / std::string(storePath.to_string()); + } +}; + +std::unique_ptr makeBuildingStoreFromLocalStore(LocalStore &); + struct DerivationBuilderDeleter { void operator()(DerivationBuilder * builder) noexcept; @@ -283,7 +364,7 @@ using DerivationBuilderUnique = std::unique_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params #ifdef _WIN32 @@ -298,7 +379,7 @@ DerivationBuilderUnique makeDerivationBuilder( * derivation. */ DerivationBuilderUnique makeExternalDerivationBuilder( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, const ExternalBuilder & handler); diff --git a/src/libstore/linux/build/chroot-linux-derivation-builder.hh b/src/libstore/linux/build/chroot-linux-derivation-builder.hh index 04ffecb28aa..a0cb62958ed 100644 --- a/src/libstore/linux/build/chroot-linux-derivation-builder.hh +++ b/src/libstore/linux/build/chroot-linux-derivation-builder.hh @@ -31,7 +31,9 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu std::optional cgroup; ChrootLinuxDerivationBuilder( - LocalStore & store, std::shared_ptr miscMethods, DerivationBuilderParams params) + std::shared_ptr store, + std::shared_ptr miscMethods, + DerivationBuilderParams params) : UnixDerivationBuilderImpl{store, miscMethods, params} , ChrootDerivationBuilder{store, miscMethods, params} , LinuxDerivationBuilder{store, miscMethods, params} @@ -54,7 +56,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu void setUser() override; - SingleDrvOutputs unprepareBuild() override; + BuilderExit unprepareBuild() override; void killSandbox(bool getStats) override; diff --git a/src/libstore/linux/build/linux-derivation-builder.cc b/src/libstore/linux/build/linux-derivation-builder.cc index 088c7aa9084..a38c8ae4d17 100644 --- a/src/libstore/linux/build/linux-derivation-builder.cc +++ b/src/libstore/linux/build/linux-derivation-builder.cc @@ -398,7 +398,7 @@ static const std::filesystem::path procPath = "/proc"; void LinuxDerivationBuilder::enterChroot() { - auto & localSettings = store.config->getLocalSettings(); + auto & localSettings = store->getLocalSettings(); /* Set the NO_NEW_PRIVS before doing seccomp/landlock setup. landlock_restrict_self requires either NO_NEW_PRIVS or CAP_SYS_ADMIN. @@ -441,12 +441,12 @@ gid_t ChrootLinuxDerivationBuilder::sandboxGid() std::unique_ptr ChrootLinuxDerivationBuilder::getBuildUser() { return acquireUserLock( - settings.nixStateDir, store.config->getLocalSettings(), drvOptions.useUidRange(drv) ? 65536 : 1, true); + settings.nixStateDir, store->getLocalSettings(), drvOptions.useUidRange(drv) ? 65536 : 1, true); } void ChrootLinuxDerivationBuilder::prepareUser() { - if ((buildUser && buildUser->getUIDCount() != 1) || store.config->getLocalSettings().useCgroups) { + if ((buildUser && buildUser->getUIDCount() != 1) || store->getLocalSettings().useCgroups) { experimentalFeatureSettings.require(Xp::Cgroups); /* If we're running from the daemon, then this will return the @@ -566,7 +566,7 @@ void ChrootLinuxDerivationBuilder::startChild() if (setgroups(0, 0) == -1) { if (errno != EPERM) throw SysError("setgroups failed"); - if (store.config->getLocalSettings().requireDropSupplementaryGroups) + if (store->getLocalSettings().requireDropSupplementaryGroups) throw Error( "setgroups failed. Set the require-drop-supplementary-groups option to false to skip this step."); } @@ -650,7 +650,7 @@ void ChrootLinuxDerivationBuilder::startChild() "nobody:x:65534:65534:Nobody:/:/noshell\n", sandboxUid(), sandboxGid(), - store.config->getLocalSettings().sandboxBuildDir.get().native())); + store->getLocalSettings().sandboxBuildDir.get().native())); writeFile( chrootRootDir / "etc" / "group", @@ -737,7 +737,7 @@ void ChrootLinuxDerivationBuilder::enterChroot() Marking chrootRootDir as MS_SHARED causes pivot_root() to fail with EINVAL. Don't know why. */ - std::filesystem::path chrootStoreDir = chrootRootDir / std::filesystem::path(store.storeDir).relative_path(); + std::filesystem::path chrootStoreDir = chrootRootDir / std::filesystem::path(store->storeDir).relative_path(); if (mount(chrootStoreDir.c_str(), chrootStoreDir.c_str(), 0, MS_BIND, 0) == -1) throw SysError("unable to bind mount the Nix store at %1%", PathFmt(chrootStoreDir)); @@ -851,7 +851,7 @@ void ChrootLinuxDerivationBuilder::enterChroot() (chrootRootDir / "dev" / "shm").c_str(), "tmpfs", 0, - fmt("size=%s", store.config->getLocalSettings().sandboxShmSize).c_str()) + fmt("size=%s", store->getLocalSettings().sandboxShmSize).c_str()) == -1) throw SysError("mounting /dev/shm"); @@ -933,7 +933,7 @@ void ChrootLinuxDerivationBuilder::setUser() }); } -SingleDrvOutputs ChrootLinuxDerivationBuilder::unprepareBuild() +BuilderExit ChrootLinuxDerivationBuilder::unprepareBuild() { sandboxMountNamespace = -1; sandboxUserNamespace = -1; @@ -982,7 +982,7 @@ void ChrootLinuxDerivationBuilder::addDependencyImpl(const StorePath & path) int status = child.wait(); if (!statusOk(status)) - throw Error("could not add path '%s' to sandbox: %s", store.printStorePath(path), statusToString(status)); + throw Error("could not add path '%s' to sandbox: %s", store->printStorePath(path), statusToString(status)); } } // namespace nix diff --git a/src/libstore/unix/build/chroot-derivation-builder.cc b/src/libstore/unix/build/chroot-derivation-builder.cc index 866d4c33bf6..22419dee60b 100644 --- a/src/libstore/unix/build/chroot-derivation-builder.cc +++ b/src/libstore/unix/build/chroot-derivation-builder.cc @@ -21,7 +21,7 @@ std::filesystem::path ChrootDerivationBuilder::tmpDirInSandbox() { /* In a sandbox, for determinism, always use the same temporary directory. */ - return store.config->getLocalSettings().sandboxBuildDir.get(); + return store->getLocalSettings().sandboxBuildDir.get(); } gid_t ChrootDerivationBuilder::sandboxGid() @@ -33,11 +33,11 @@ void ChrootDerivationBuilder::prepareSandbox() { // Set up chroot parameters BuildChrootParams params{ - .chrootParentDir = store.toRealPath(drvPath) + ".chroot", + .chrootParentDir = store->toRealPath(drvPath) + ".chroot", .useUidRange = drvOptions.useUidRange(drv), .isSandboxed = derivationType.isSandboxed(), .buildUser = buildUser.get(), - .storeDir = store.storeDir, + .storeDir = store->storeDir, .chownToBuilder = [this](const std::filesystem::path & path) { this->chownToBuilder(path); }, }; @@ -50,8 +50,8 @@ void ChrootDerivationBuilder::prepareSandbox() pathsInChroot = getPathsInSandbox(); for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - pathsInChroot.insert_or_assign(p, ChrootPath{.source = store.toRealPath(i)}); + auto p = store->printStorePath(i); + pathsInChroot.insert_or_assign(p, ChrootPath{.source = store->toRealPath(i)}); } /* If we're repairing, checking or rebuilding part of a @@ -59,28 +59,28 @@ void ChrootDerivationBuilder::prepareSandbox() rebuilding a path that is in settings.sandbox-paths (typically the dependencies of /bin/sh). Throw them out. */ - for (auto & i : outputsAndOptPaths(drv, store)) { + for (auto & i : outputsAndOptPaths(drv, *store)) { /* If the name isn't known a priori (i.e. floating content-addressing derivation), the temporary location we use should be fresh. Freshness means it is impossible that the path is already in the sandbox, so we don't need to worry about removing it. */ if (i.second.second) - pathsInChroot.erase(store.printStorePath(*i.second.second)); + pathsInChroot.erase(store->printStorePath(*i.second.second)); } } Strings ChrootDerivationBuilder::getPreBuildHookArgs() { assert(!chrootRootDir.empty()); - return Strings({store.printStorePath(drvPath), chrootRootDir.native()}); + return Strings({store->printStorePath(drvPath), chrootRootDir.native()}); } std::filesystem::path ChrootDerivationBuilder::realPathInHost(const StorePath & p) { // FIXME: why the needsHashRewrite() conditional? - return !needsHashRewrite() ? chrootRootDir / std::filesystem::path{store.printStorePath(p)}.relative_path() - : store.toRealPath(p); + return !needsHashRewrite() ? chrootRootDir / std::filesystem::path{store->printStorePath(p)}.relative_path() + : store->toRealPath(p); } void ChrootDerivationBuilder::cleanupBuild(bool force) @@ -95,7 +95,7 @@ void ChrootDerivationBuilder::cleanupBuild(bool force) continue; if (buildMode != bmCheck && status.known->isValid()) continue; - std::filesystem::path p = store.toRealPath(status.known->path); + std::filesystem::path p = store->toRealPath(status.known->path); std::filesystem::path chrootPath = chrootRootDir / p.relative_path(); if (pathExists(chrootPath)) std::filesystem::rename(chrootPath, p); @@ -107,16 +107,16 @@ void ChrootDerivationBuilder::cleanupBuild(bool force) std::pair ChrootDerivationBuilder::addDependencyPrep(const StorePath & path) { - debug("materialising '%s' in the sandbox", store.printStorePath(path)); + debug("materialising '%s' in the sandbox", store->printStorePath(path)); - std::filesystem::path source = store.toRealPath(path); - auto targetRelPath = std::filesystem::path(store.printStorePath(path)).relative_path(); + std::filesystem::path source = store->toRealPath(path); + auto targetRelPath = std::filesystem::path(store->printStorePath(path)).relative_path(); std::filesystem::path target = chrootRootDir / targetRelPath; if (pathExists(target)) { // There is a similar debug message in doBind, so only run it in this block to not have double messages. debug("bind-mounting %s -> %s", PathFmt(target), PathFmt(source)); - throw Error("store path '%s' already exists in the sandbox", store.printStorePath(path)); + throw Error("store path '%s' already exists in the sandbox", store->printStorePath(path)); } return {source, targetRelPath}; diff --git a/src/libstore/unix/build/chroot-derivation-builder.hh b/src/libstore/unix/build/chroot-derivation-builder.hh index 31e924c0b12..54051e10feb 100644 --- a/src/libstore/unix/build/chroot-derivation-builder.hh +++ b/src/libstore/unix/build/chroot-derivation-builder.hh @@ -11,7 +11,9 @@ private: void anchor() override; public: ChrootDerivationBuilder( - LocalStore & store, std::shared_ptr miscMethods, DerivationBuilderParams params) + std::shared_ptr store, + std::shared_ptr miscMethods, + DerivationBuilderParams params) : UnixDerivationBuilderImpl{store, std::move(miscMethods), std::move(params)} { } diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index e4a2ec7f470..b42c89b6ab9 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -10,7 +10,7 @@ struct ExternalDerivationBuilder : UnixDerivationBuilderImpl ExternalBuilder externalBuilder; ExternalDerivationBuilder( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, ExternalBuilder externalBuilder) @@ -57,19 +57,19 @@ struct ExternalDerivationBuilder : UnixDerivationBuilderImpl json.emplace("topTmpDir", topTmpDir.native()); json.emplace("tmpDir", tmpDir.native()); json.emplace("tmpDirInSandbox", tmpDirInSandbox().native()); - json.emplace("storeDir", store.storeDir); - json.emplace("realStoreDir", store.config->realStoreDir.get()); + json.emplace("storeDir", store->storeDir); + json.emplace("realStoreDir", store->getRealStoreDir().native()); json.emplace("system", drv.platform); { auto l = nlohmann::json::array(); for (auto & i : inputPaths) - l.push_back(store.printStorePath(i)); + l.push_back(store->printStorePath(i)); json.emplace("inputPaths", std::move(l)); } { auto l = nlohmann::json::object(); for (auto & i : scratchOutputs) - l.emplace(i.first, store.printStorePath(i.second)); + l.emplace(i.first, store->printStorePath(i.second)); json.emplace("outputs", std::move(l)); } @@ -114,7 +114,7 @@ struct ExternalDerivationBuilder : UnixDerivationBuilderImpl } // namespace DerivationBuilderUnique makeExternalDerivationBuilder( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, const ExternalBuilder & handler) diff --git a/src/libstore/unix/build/unix-derivation-builder-impl.hh b/src/libstore/unix/build/unix-derivation-builder-impl.hh index 2d289a2c83b..244ba23f84d 100644 --- a/src/libstore/unix/build/unix-derivation-builder-impl.hh +++ b/src/libstore/unix/build/unix-derivation-builder-impl.hh @@ -143,7 +143,9 @@ public: std::optional startBuild() override; - SingleDrvOutputs unprepareBuild() override; + BuilderExit unprepareBuild() override; + + void cleanupBuild(bool force) override; protected: @@ -197,7 +199,7 @@ protected: virtual Strings getPreBuildHookArgs() { - return Strings({store.printStorePath(drvPath)}); + return Strings({store->printStorePath(drvPath)}); } /** @@ -311,14 +313,6 @@ protected: protected: - /** - * Delete the temporary directory, if we have one. - * - * @param force We know the build succeeded, so don't attempt to - * preserve anything for debugging. - */ - virtual void cleanupBuild(bool force); - /** * Kill any processes running under the build user UID or in the * cgroup of the build. diff --git a/src/libstore/unix/build/unix-derivation-builder.cc b/src/libstore/unix/build/unix-derivation-builder.cc index 5163e27039d..e1eddd44d7c 100644 --- a/src/libstore/unix/build/unix-derivation-builder.cc +++ b/src/libstore/unix/build/unix-derivation-builder.cc @@ -131,7 +131,7 @@ bool UnixDerivationBuilderImpl::killChild() return ret; } -SingleDrvOutputs UnixDerivationBuilderImpl::unprepareBuild() +BuilderExit UnixDerivationBuilderImpl::unprepareBuild() { /* Since we got an EOF on the logger pipe, the builder is presumed to have terminated. In fact, the builder could also have @@ -139,7 +139,7 @@ SingleDrvOutputs UnixDerivationBuilderImpl::unprepareBuild() kill it. */ int status = pid.kill(); - debug("builder process for '%s' finished", store.printStorePath(drvPath)); + debug("builder process for '%s' finished", store->printStorePath(drvPath)); buildResult.timesBuilt++; buildResult.stopTime = time(nullptr); @@ -166,40 +166,19 @@ SingleDrvOutputs UnixDerivationBuilderImpl::unprepareBuild() if (buildResult.cpuUser && buildResult.cpuSystem) { debug( "builder for '%s' terminated with status %d, user CPU %.3fs, system CPU %.3fs", - store.printStorePath(drvPath), + store->printStorePath(drvPath), status, ((double) buildResult.cpuUser->count()) / 1000000, ((double) buildResult.cpuSystem->count()) / 1000000); } /* Check the exit status. */ - if (!statusOk(status)) { - - /* Check *before* cleaning up. */ - bool diskFull = decideWhetherDiskFull(); - - cleanupBuild(false); - - throw BuilderFailureError{ - !derivationType.isSandboxed() || diskFull ? BuildResult::Failure::TransientFailure - : BuildResult::Failure::PermanentFailure, - status, - diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "", - }; - } - - SingleDrvOutputs builtOutputs; - if (usingSubmitted) { - builtOutputs = checkSubmittedOutputs(); + if (statusOk(status)) { + return {.status = status}; } else { - /* Compute the FS closure of the outputs and register them as - being valid. */ - builtOutputs = registerOutputs(); + /* Check *before* cleaning up. */ + return {.status = status, .diskFull = decideWhetherDiskFull()}; } - - cleanupBuild(true); - - return builtOutputs; } bool UnixDerivationBuilderImpl::decideWhetherDiskFull() @@ -215,8 +194,7 @@ bool UnixDerivationBuilderImpl::decideWhetherDiskFull() { uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable struct statvfs st; - if (statvfs(store.config->realStoreDir.get().c_str(), &st) == 0 - && (uint64_t) st.f_bavail * st.f_bsize < required) + if (statvfs(store->getRealStoreDir().c_str(), &st) == 0 && (uint64_t) st.f_bavail * st.f_bsize < required) diskFull = true; if (statvfs(tmpDir.c_str(), &st) == 0 && (uint64_t) st.f_bavail * st.f_bsize < required) diskFull = true; @@ -282,7 +260,7 @@ std::optional UnixDerivationBuilderImpl::startBuild() calls. */ prepareUser(); - auto buildDir = store.config->getBuildDir(); + auto buildDir = store->getBuildDir(); createDirs(buildDir); @@ -333,7 +311,7 @@ std::optional UnixDerivationBuilderImpl::startBuild() /* Substitute output placeholders with the scratch output paths. We'll use during the build. */ - inputRewrites[hashPlaceholder(outputName)] = store.printStorePath(scratchPath); + inputRewrites[hashPlaceholder(outputName)] = store->printStorePath(scratchPath); /* Additional tasks if we know the final path a priori. */ if (!status.known) @@ -346,7 +324,7 @@ std::optional UnixDerivationBuilderImpl::startBuild() continue; /* Ensure scratch path is ours to use. */ - deletePath(store.printStorePath(scratchPath)); + deletePath(store->printStorePath(scratchPath)); /* Rewrite and unrewrite paths */ { @@ -430,7 +408,7 @@ PathsInChroot UnixDerivationBuilderImpl::getPathsInSandbox() host file system. */ PathsInChroot pathsInChroot = defaultPathsInChroot; - if (hasPrefix(store.storeDir, tmpDirInSandbox().native())) { + if (hasPrefix(store->storeDir, tmpDirInSandbox().native())) { throw Error("`sandbox-build-dir` must not contain the storeDir"); } pathsInChroot[tmpDirInSandbox()] = {.source = tmpDir}; @@ -457,7 +435,7 @@ PathsInChroot UnixDerivationBuilderImpl::getPathsInSandbox() if (!found) throw Error( "derivation '%s' requested impure path '%s', but it was not in allowed-impure-host-deps", - store.printStorePath(drvPath), + store->printStorePath(drvPath), i); /* Allow files in drvOptions.impureHostDeps to be missing; e.g. @@ -579,7 +557,7 @@ void UnixDerivationBuilderImpl::processSandboxSetupMessages() e.addTrace( {}, "while waiting for the build environment for '%s' to initialize (%s, previous messages: %s)", - store.printStorePath(drvPath), + store->printStorePath(drvPath), status ? statusToString(status) : "no status", concatStringsSep("|", msgs)); throw; @@ -619,7 +597,7 @@ void UnixDerivationBuilderImpl::initEnv() shouldn't care, but this is useful for purity checking (e.g., the compiler or linker might only want to accept paths to files in the store or in the build directory). */ - env["NIX_STORE"] = store.storeDir; + env["NIX_STORE"] = store->storeDir; /* The maximum number of cores to utilize for parallel building. */ env["NIX_BUILD_CORES"] = fmt( @@ -699,16 +677,7 @@ void UnixDerivationBuilderImpl::startDaemon() experimentalFeatureSettings.require(Xp::RecursiveNix); } - auto store = makeRestrictedStore( - [&] { - auto config = make_ref(*this->store.config); - config->pathInfoCacheSize = 0; - config->stateDir = "/no-such-path"; - config->logDir = "/no-such-path"; - return config; - }(), - ref(std::dynamic_pointer_cast(this->store.shared_from_this())), - *this); + auto storeForDaemon = this->store->makeRecursiveNixStore(*this); state_.lock()->addedPaths.clear(); @@ -727,7 +696,7 @@ void UnixDerivationBuilderImpl::startDaemon() recursiveFlag = daemon::RecursiveFlag::Recursive; } - daemonThread = std::thread([this, store, recursiveFlag]() { + daemonThread = std::thread([this, storeForDaemon, recursiveFlag]() { while (true) { /* Accept a connection. */ @@ -749,21 +718,22 @@ void UnixDerivationBuilderImpl::startDaemon() auto doneFlag = make_ref(); - auto workerThread = std::thread([this, doneFlag, store, remote{std::move(remote)}, recursiveFlag]() { - try { - miscMethods->processDaemonConnection( - store, FdSource(remote.get()), FdSink(remote.get()), *this, recursiveFlag); - debug("terminated daemon connection"); - } catch (const Interrupted &) { - debug("interrupted daemon connection"); - } catch (...) { - /* Swallow all exceptions to avoid crashing the the process (exceptions that escape from the thread - * trigger std::terminate()). */ - ignoreExceptionExceptInterrupt(); - } - - doneFlag->test_and_set(std::memory_order_relaxed); - }); + auto workerThread = + std::thread([this, doneFlag, storeForDaemon, remote{std::move(remote)}, recursiveFlag]() { + try { + miscMethods->processDaemonConnection( + storeForDaemon, FdSource(remote.get()), FdSink(remote.get()), *this, recursiveFlag); + debug("terminated daemon connection"); + } catch (const Interrupted &) { + debug("interrupted daemon connection"); + } catch (...) { + /* Swallow all exceptions to avoid crashing the the process (exceptions that escape from the + * thread trigger std::terminate()). */ + ignoreExceptionExceptInterrupt(); + } + + doneFlag->test_and_set(std::memory_order_relaxed); + }); daemonWorkerThreads.push_back( DaemonWorkerState{ @@ -827,15 +797,15 @@ void UnixDerivationBuilderImpl::submitOutput(const SingleDerivedPath & path, con throw Error( "Attempted to submit Built path '%s' for output '%s'.\n" " Only Opaque paths are supported, see https://github.com/NixOS/nix/issues/12727", - path.to_string(store), + path.to_string(*store), output); if (submittedOutputs->contains(output)) throw Error( "Attempted to submit duplicate output '%s' (old '%s', new '%s')", output, - store.printStorePath(*get(*submittedOutputs, output)), - store.printStorePath(opaque->path)); + store->printStorePath(*get(*submittedOutputs, output)), + store->printStorePath(opaque->path)); submittedOutputs->insert_or_assign(output, opaque->path); } @@ -939,7 +909,7 @@ void UnixDerivationBuilderImpl::runChild(RunChildArgs args) logger = makeJSONLogger(getStandardError()).release(); for (auto & e : drv.outputs) - ctx.outputs.insert_or_assign(e.first, store.printStorePath(scratchOutputs.at(e.first))); + ctx.outputs.insert_or_assign(e.first, store->printStorePath(scratchOutputs.at(e.first))); std::string builtinName = drv.builder.substr(8); assert(RegisterBuiltinBuilder::builtinBuilders); @@ -1013,7 +983,7 @@ void UnixDerivationBuilderImpl::cleanupBuild(bool force) if (force) { /* Delete unused redirected outputs (when doing hash rewriting). */ for (auto & i : redirectedOutputs) - deletePath(store.toRealPath(i.second)); + deletePath(store->toRealPath(i.second)); } if (topTmpDir != "") { @@ -1046,7 +1016,7 @@ StorePath UnixDerivationBuilderImpl::makeFallbackPath(OutputNameView outputName) // TODO: We may want to separate the responsibilities of constructing the path fingerprint and of actually doing the // hashing auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":name:" + std::string(outputName); - return store.makeStorePath( + return store->makeStorePath( pathType, // pass an all-zeroes hash Hash(HashAlgorithm::SHA256), @@ -1058,7 +1028,7 @@ StorePath UnixDerivationBuilderImpl::makeFallbackPath(const StorePath & path) // This is a bogus path type, constructed this way to ensure that it doesn't collide with any other store path // See doc/manual/source/protocols/store-path.md for details auto pathType = "rewrite:" + std::string(drvPath.to_string()) + ":" + std::string(path.to_string()); - return store.makeStorePath( + return store->makeStorePath( pathType, // pass an all-zeroes hash Hash(HashAlgorithm::SHA256), @@ -1083,10 +1053,14 @@ void DerivationBuilderDeleter::operator()(DerivationBuilder * builder) noexcept } std::unique_ptr makeDerivationBuilder( - LocalStore & store, std::shared_ptr miscMethods, DerivationBuilderParams params) + std::shared_ptr store, + std::shared_ptr miscMethods, + DerivationBuilderParams params) { bool useSandbox = false; - const LocalSettings & localSettings = store.config->getLocalSettings(); + + const StoreDirConfig & storeDirConfig = *store; + const LocalSettings & localSettings = store->getLocalSettings(); /* Are we doing a sandboxed build? */ { @@ -1095,13 +1069,13 @@ std::unique_ptr makeDerivationBuild throw Error( "derivation '%s' has '__noChroot' set, " "but that's not allowed when 'sandbox' is 'true'", - store.printStorePath(params.drvPath)); + storeDirConfig.printStorePath(params.drvPath)); #ifdef __APPLE__ if (params.drvOptions.additionalSandboxProfile != "") throw Error( "derivation '%s' specifies a sandbox profile, " "but this is only allowed when 'sandbox' is 'relaxed'", - store.printStorePath(params.drvPath)); + storeDirConfig.printStorePath(params.drvPath)); #endif useSandbox = true; } else if (localSettings.sandboxMode == smDisabled) @@ -1111,7 +1085,7 @@ std::unique_ptr makeDerivationBuild useSandbox = type(params.drv).isSandboxed() && !params.drvOptions.noChroot; } - const bool isRelocatedStore = store.storeDir != store.config->realStoreDir.get(); + const bool isRelocatedStore = storeDirConfig.storeDir != store->getRealStoreDir(); if (isRelocatedStore) { #if defined(__linux__) || defined(__FreeBSD__) diff --git a/src/libstore/windows/build/windows-derivation-builder.cc b/src/libstore/windows/build/windows-derivation-builder.cc index ae78cf18263..910b46f6613 100644 --- a/src/libstore/windows/build/windows-derivation-builder.cc +++ b/src/libstore/windows/build/windows-derivation-builder.cc @@ -92,7 +92,7 @@ class WindowsDerivationBuilderImpl : public DerivationBuilderImpl public: WindowsDerivationBuilderImpl( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, HANDLE ioport) @@ -148,7 +148,13 @@ class WindowsDerivationBuilderImpl : public DerivationBuilderImpl /* --- DerivationBuilder --- */ std::optional startBuild() override; - SingleDrvOutputs unprepareBuild() override; + BuilderExit unprepareBuild() override; + + void cleanupBuild(bool force) override + { + deletePath(tmpDir); + } + bool killChild() override; private: @@ -203,7 +209,7 @@ OsString WindowsDerivationBuilderImpl::makeEnvBlock() env[OS_STR("TMPDIR")] = tmpDir.native(); env[OS_STR("TEMPDIR")] = tmpDir.native(); env[OS_STR("PWD")] = tmpDir.native(); - env[OS_STR("NIX_STORE")] = os(store.storeDir); + env[OS_STR("NIX_STORE")] = os(store->storeDir); /* Most Windows programs, `cmd.exe` included, will not start without these, and system tools live outside the store so `PATH` is needed to find them at all. @@ -321,7 +327,7 @@ std::optional WindowsDerivationBuilderImpl::startBuild() /* Clear anything a previous failed build left at the output paths. */ for (auto & [name, status] : initialOutputs) if (status.known) - deleteStalePath(store.toRealPath(status.known->path)); + deleteStalePath(store->toRealPath(status.known->path)); miscMethods->openLogFile(); @@ -348,7 +354,7 @@ bool WindowsDerivationBuilderImpl::killChild() return true; } -SingleDrvOutputs WindowsDerivationBuilderImpl::unprepareBuild() +BuilderExit WindowsDerivationBuilderImpl::unprepareBuild() { /* The caller only gets here once the log pipe hit EOF, which means the builder closed its handles. Reap anyway, so the exit code is settled. */ @@ -357,26 +363,13 @@ SingleDrvOutputs WindowsDerivationBuilderImpl::unprepareBuild() miscMethods->closeLogFile(); miscMethods->childTerminated(); - if (exitCode != 0) { - deletePath(tmpDir); - throw BuilderFailureError{ - BuildResult::Failure::PermanentFailure, - exitCode, - fmt("builder '%s' exited with status %d", drv.builder, exitCode), - }; - } - - auto builtOutputs = registerOutputs(); - - deletePath(tmpDir); - - return builtOutputs; + return {.status = exitCode}; } } // namespace DerivationBuilderUnique makeDerivationBuilder( - LocalStore & store, + std::shared_ptr store, std::shared_ptr miscMethods, DerivationBuilderParams params, HANDLE ioport)