Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make nix-build --store whatever work #4387

Merged
merged 10 commits into from
Jan 25, 2021
13 changes: 9 additions & 4 deletions src/build-remote/build-remote.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,15 @@ static int main_build_remote(int argc, char * * argv)

initPlugins();

auto store = openStore().cast<LocalStore>();
auto store = openStore();

/* It would be more appropriate to use $XDG_RUNTIME_DIR, since
that gets cleared on reboot, but it wouldn't work on macOS. */
currentLoad = store->stateDir + "/current-load";
auto currentLoadName = "/current-load";
if (auto localStore = store.dynamic_pointer_cast<LocalFSStore>())
currentLoad = std::string { localStore->stateDir } + currentLoadName;
else
currentLoad = settings.nixStateDir + currentLoadName;

std::shared_ptr<Store> sshStore;
AutoCloseFD bestSlotLock;
Expand Down Expand Up @@ -288,8 +292,9 @@ static int main_build_remote(int argc, char * * argv)

if (!missing.empty()) {
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying outputs from '%s'", storeUri));
for (auto & i : missing)
store->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
if (auto localStore = store.dynamic_pointer_cast<LocalStore>())
for (auto & i : missing)
localStore->locksHeld.insert(store->printStorePath(i)); /* FIXME: ugly */
copyPaths(ref<Store>(sshStore), store, missing, NoRepair, NoCheckSigs, NoSubstitute);
}

Expand Down
7 changes: 0 additions & 7 deletions src/libstore/binary-cache-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,6 @@ public:

void narFromPath(const StorePath & path, Sink & sink) override;

BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildMode buildMode) override
{ unsupported("buildDerivation"); }

void ensurePath(const StorePath & path) override
{ unsupported("ensurePath"); }

ref<FSAccessor> getFSAccessor() override;

void addSignatures(const StorePath & storePath, const StringSet & sigs) override;
Expand Down
86 changes: 61 additions & 25 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -597,9 +597,17 @@ void DerivationGoal::tryToBuild()
PathSet lockFiles;
/* FIXME: Should lock something like the drv itself so we don't build same
CA drv concurrently */
for (auto & i : drv->outputsAndOptPaths(worker.store))
if (i.second.second)
lockFiles.insert(worker.store.Store::toRealPath(*i.second.second));
if (dynamic_cast<LocalStore *>(&worker.store))
/* If we aren't a local store, we might need to use the local store as
a build remote, but that would cause a deadlock. */
/* FIXME: Make it so we can use ourselves as a build remote even if we
are the local store (separate locking for building vs scheduling? */
/* FIXME: find some way to lock for scheduling for the other stores so
a forking daemon with --store still won't farm out redundant builds.
*/
for (auto & i : drv->outputsAndOptPaths(worker.store))
if (i.second.second)
lockFiles.insert(worker.store.Store::toRealPath(*i.second.second));

if (!outputLocks.lockPaths(lockFiles, "", false)) {
if (!actLock)
Expand Down Expand Up @@ -681,6 +689,12 @@ void DerivationGoal::tryToBuild()

void DerivationGoal::tryLocalBuild() {
/* Make sure that we are allowed to start a build. */
if (!dynamic_cast<LocalStore *>(&worker.store)) {
throw Error(
"unable to build with a primary store that isn't a local store; "
"either pass a different '--store' or enable remote builds."
"\nhttps://nixos.org/nix/manual/#chap-distributed-builds");
}
unsigned int curBuilds = worker.getNrLocalBuilds();
if (curBuilds >= settings.maxBuildJobs) {
worker.waitForBuildSlot(shared_from_this());
Expand Down Expand Up @@ -849,14 +863,16 @@ void DerivationGoal::buildDone()
So instead, check if the disk is (nearly) full now. If
so, we don't mark this build as a permanent failure. */
#if HAVE_STATVFS
uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable
struct statvfs st;
if (statvfs(worker.store.realStoreDir.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;
if (auto localStore = dynamic_cast<LocalStore *>(&worker.store)) {
uint64_t required = 8ULL * 1024 * 1024; // FIXME: make configurable
struct statvfs st;
if (statvfs(localStore->realStoreDir.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;
}
#endif

deleteTmpDir(false);
Expand Down Expand Up @@ -1216,12 +1232,15 @@ void DerivationGoal::startBuilder()
useChroot = !(derivationIsImpure(derivationType)) && !noChroot;
}

if (worker.store.storeDir != worker.store.realStoreDir) {
#if __linux__
useChroot = true;
#else
throw Error("building using a diverted store is not supported on this platform");
#endif
if (auto localStoreP = dynamic_cast<LocalStore *>(&worker.store)) {
auto & localStore = *localStoreP;
if (localStore.storeDir != localStore.realStoreDir) {
#if __linux__
useChroot = true;
#else
throw Error("building using a diverted store is not supported on this platform");
#endif
}
}

/* Create a temporary directory where the build will take
Expand Down Expand Up @@ -2181,7 +2200,8 @@ void DerivationGoal::startDaemon()
Store::Params params;
params["path-info-cache-size"] = "0";
params["store"] = worker.store.storeDir;
params["root"] = worker.store.rootDir;
if (auto localStore = dynamic_cast<LocalStore *>(&worker.store))
params["root"] = localStore->rootDir;
params["state"] = "/no-such-path";
params["log"] = "/no-such-path";
auto store = make_ref<RestrictedStore>(params,
Expand Down Expand Up @@ -3269,7 +3289,13 @@ void DerivationGoal::registerOutputs()
}
}

auto localStoreP = dynamic_cast<LocalStore *>(&worker.store);
if (!localStoreP)
throw Unsupported("can only register outputs with local store, but this is %s", worker.store.getUri());
auto & localStore = *localStoreP;

if (buildMode == bmCheck) {

if (!worker.store.isValidPath(newInfo.path)) continue;
ValidPathInfo oldInfo(*worker.store.queryPathInfo(newInfo.path));
if (newInfo.narHash != oldInfo.narHash) {
Expand All @@ -3294,8 +3320,8 @@ void DerivationGoal::registerOutputs()
/* Since we verified the build, it's now ultimately trusted. */
if (!oldInfo.ultimate) {
oldInfo.ultimate = true;
worker.store.signPathInfo(oldInfo);
worker.store.registerValidPaths({{oldInfo.path, oldInfo}});
localStore.signPathInfo(oldInfo);
localStore.registerValidPaths({{oldInfo.path, oldInfo}});
}

continue;
Expand All @@ -3311,21 +3337,21 @@ void DerivationGoal::registerOutputs()
}

if (curRound == nrRounds) {
worker.store.optimisePath(actualPath); // FIXME: combine with scanForReferences()
localStore.optimisePath(actualPath); // FIXME: combine with scanForReferences()
worker.markContentsGood(newInfo.path);
}

newInfo.deriver = drvPath;
newInfo.ultimate = true;
worker.store.signPathInfo(newInfo);
localStore.signPathInfo(newInfo);

finish(newInfo.path);

/* If it's a CA path, register it right away. This is necessary if it
isn't statically known so that we can safely unlock the path before
the next iteration */
if (newInfo.ca)
worker.store.registerValidPaths({{newInfo.path, newInfo}});
localStore.registerValidPaths({{newInfo.path, newInfo}});

infos.emplace(outputName, std::move(newInfo));
}
Expand Down Expand Up @@ -3398,11 +3424,16 @@ void DerivationGoal::registerOutputs()
paths referenced by each of them. If there are cycles in the
outputs, this will fail. */
{
auto localStoreP = dynamic_cast<LocalStore *>(&worker.store);
if (!localStoreP)
throw Unsupported("can only register outputs with local store, but this is %s", worker.store.getUri());
auto & localStore = *localStoreP;

ValidPathInfos infos2;
for (auto & [outputName, newInfo] : infos) {
infos2.insert_or_assign(newInfo.path, newInfo);
}
worker.store.registerValidPaths(infos2);
localStore.registerValidPaths(infos2);
}

/* In case of a fixed-output derivation hash mismatch, throw an
Expand Down Expand Up @@ -3600,7 +3631,12 @@ Path DerivationGoal::openLogFile()
auto baseName = std::string(baseNameOf(worker.store.printStorePath(drvPath)));

/* Create a log file. */
Path dir = fmt("%s/%s/%s/", worker.store.logDir, worker.store.drvsLogDir, string(baseName, 0, 2));
Path logDir;
if (auto localStore = dynamic_cast<LocalStore *>(&worker.store))
logDir = localStore->logDir;
else
logDir = settings.nixLogDir;
Path dir = fmt("%s/%s/%s/", logDir, LocalFSStore::drvsLogDir, string(baseName, 0, 2));
createDirs(dir);

Path logFileName = fmt("%s/%s%s", dir, string(baseName, 2),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace nix {

void LocalStore::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
void Store::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths, BuildMode buildMode)
{
Worker worker(*this);

Expand Down Expand Up @@ -43,7 +43,7 @@ void LocalStore::buildPaths(const std::vector<StorePathWithOutputs> & drvPaths,
}
}

BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildMode buildMode)
{
Worker worker(*this);
Expand All @@ -63,7 +63,7 @@ BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDe
}


void LocalStore::ensurePath(const StorePath & path)
void Store::ensurePath(const StorePath & path)
{
/* If the path is already valid, we're done. */
if (isValidPath(path)) return;
Expand Down
4 changes: 1 addition & 3 deletions src/libstore/build/substitution-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,7 @@ void SubstitutionGoal::tryNext()
/* Bail out early if this substituter lacks a valid
signature. LocalStore::addToStore() also checks for this, but
only after we've downloaded the path. */
if (worker.store.requireSigs
&& !sub->isTrusted
&& !info->checkSignatures(worker.store, worker.store.getPublicKeys()))
if (!sub->isTrusted && worker.store.pathInfoIsTrusted(*info))
{
logWarning({
.name = "Invalid path signature",
Expand Down
6 changes: 4 additions & 2 deletions src/libstore/build/worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

namespace nix {

Worker::Worker(LocalStore & store)
Worker::Worker(Store & store)
: act(*logger, actRealise)
, actDerivations(*logger, actBuilds)
, actSubstitutions(*logger, actCopyPaths)
Expand Down Expand Up @@ -229,7 +229,9 @@ void Worker::run(const Goals & _topGoals)

checkInterrupt();

store.autoGC(false);
// TODO GC interface?
if (auto localStore = dynamic_cast<LocalStore *>(&store))
localStore->autoGC(false);

/* Call every wake goal (in the ordering established by
CompareGoalPtrs). */
Expand Down
9 changes: 6 additions & 3 deletions src/libstore/build/worker.hh
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

#include "types.hh"
#include "lock.hh"
#include "local-store.hh"
#include "store-api.hh"
#include "goal.hh"

#include <future>
#include <thread>

namespace nix {

/* Forward definition. */
Expand Down Expand Up @@ -102,7 +105,7 @@ public:
/* Set if at least one derivation is not deterministic in check mode. */
bool checkMismatch;

LocalStore & store;
Store & store;

std::unique_ptr<HookInstance> hook;

Expand All @@ -124,7 +127,7 @@ public:
it answers with "decline-permanently", we don't try again. */
bool tryBuildHook = true;

Worker(LocalStore & store);
Worker(Store & store);
~Worker();

/* Make a goal (with caching). */
Expand Down
7 changes: 0 additions & 7 deletions src/libstore/dummy-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store
void narFromPath(const StorePath & path, Sink & sink) override
{ unsupported("narFromPath"); }

void ensurePath(const StorePath & path) override
{ unsupported("ensurePath"); }

BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildMode buildMode) override
{ unsupported("buildDerivation"); }

std::optional<const Realisation> queryRealisation(const DrvOutput&) override
{ unsupported("queryRealisation"); }
};
Expand Down
7 changes: 5 additions & 2 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1098,7 +1098,6 @@ void LocalStore::invalidatePath(State & state, const StorePath & path)
}
}


const PublicKeys & LocalStore::getPublicKeys()
{
auto state(_state.lock());
Expand All @@ -1107,11 +1106,15 @@ const PublicKeys & LocalStore::getPublicKeys()
return *state->publicKeys;
}

bool LocalStore::pathInfoIsTrusted(const ValidPathInfo & info)
{
return requireSigs && !info.checkSignatures(*this, getPublicKeys());
}

void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
RepairFlag repair, CheckSigsFlag checkSigs)
{
if (requireSigs && checkSigs && !info.checkSignatures(*this, getPublicKeys()))
if (checkSigs && pathInfoIsTrusted(info))
throw Error("cannot add path '%s' because it lacks a valid signature", printStorePath(info.path));

addTempRoot(info.path);
Expand Down
11 changes: 2 additions & 9 deletions src/libstore/local-store.hh
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ public:
void querySubstitutablePathInfos(const StorePathCAMap & paths,
SubstitutablePathInfos & infos) override;

bool pathInfoIsTrusted(const ValidPathInfo &) override;

void addToStore(const ValidPathInfo & info, Source & source,
RepairFlag repair, CheckSigsFlag checkSigs) override;

Expand All @@ -145,15 +147,6 @@ public:
StorePath addTextToStore(const string & name, const string & s,
const StorePathSet & references, RepairFlag repair) override;

void buildPaths(
const std::vector<StorePathWithOutputs> & paths,
BuildMode buildMode) override;

BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv,
BuildMode buildMode) override;

void ensurePath(const StorePath & path) override;

void addTempRoot(const StorePath & path) override;

void addIndirectRoot(const Path & path) override;
Expand Down
Loading