From aa3bc3d5dcff5ff6567a4e00320cb9caa28c5a93 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 29 Apr 2016 13:57:08 +0200 Subject: [PATCH] Eliminate the substituter mechanism Substitution is now simply a Store -> Store copy operation, most typically from BinaryCacheStore to LocalStore. --- scripts/local.mk | 8 +- src/libstore/build.cc | 292 +++++++++++------------------------- src/libstore/globals.cc | 13 -- src/libstore/globals.hh | 5 - src/libstore/local-store.cc | 227 +++------------------------- src/libstore/local-store.hh | 27 ---- src/libstore/local.mk | 2 +- src/libstore/store-api.cc | 35 +++++ src/libstore/store-api.hh | 6 + src/libutil/finally.hh | 12 ++ tests/fallback.sh | 20 --- tests/local.mk | 3 +- tests/substituter.sh | 37 ----- tests/substituter2.sh | 33 ---- tests/substitutes.sh | 22 --- tests/substitutes2.sh | 21 --- 16 files changed, 166 insertions(+), 597 deletions(-) create mode 100644 src/libutil/finally.hh delete mode 100644 tests/fallback.sh delete mode 100755 tests/substituter.sh delete mode 100755 tests/substituter2.sh delete mode 100644 tests/substitutes.sh delete mode 100644 tests/substitutes2.sh diff --git a/scripts/local.mk b/scripts/local.mk index f6542a4cb..13b13a86b 100644 --- a/scripts/local.mk +++ b/scripts/local.mk @@ -7,18 +7,13 @@ nix_bin_scripts := \ bin-scripts += $(nix_bin_scripts) -nix_substituters := \ - $(d)/copy-from-other-stores.pl \ - $(d)/download-from-binary-cache.pl - nix_noinst_scripts := \ $(d)/build-remote.pl \ $(d)/find-runtime-roots.pl \ $(d)/resolve-system-dependencies.pl \ $(d)/nix-http-export.cgi \ $(d)/nix-profile.sh \ - $(d)/nix-reduce-build \ - $(nix_substituters) + $(d)/nix-reduce-build noinst-scripts += $(nix_noinst_scripts) @@ -28,7 +23,6 @@ $(eval $(call install-file-as, $(d)/nix-profile.sh, $(profiledir)/nix.sh, 0644)) $(eval $(call install-program-in, $(d)/find-runtime-roots.pl, $(libexecdir)/nix)) $(eval $(call install-program-in, $(d)/build-remote.pl, $(libexecdir)/nix)) $(eval $(call install-program-in, $(d)/resolve-system-dependencies.pl, $(libexecdir)/nix)) -$(foreach prog, $(nix_substituters), $(eval $(call install-program-in, $(prog), $(libexecdir)/nix/substituters))) $(eval $(call install-symlink, nix-build, $(bindir)/nix-shell)) clean-files += $(nix_bin_scripts) $(nix_noinst_scripts) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 050a48ef0..65df2eea5 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -8,11 +8,14 @@ #include "archive.hh" #include "affinity.hh" #include "builtins.hh" +#include "finally.hh" #include #include #include #include +#include +#include #include #include @@ -199,8 +202,6 @@ struct Child time_t timeStarted; }; -typedef map Children; - /* The worker class. */ class Worker @@ -220,7 +221,7 @@ private: WeakGoals wantingToBuild; /* Child processes currently running. */ - Children children; + std::list children; /* Number of build slots occupied. This includes local builds and substitutions but not remote builds via the build hook. */ @@ -278,14 +279,14 @@ public: /* Registers a running child process. `inBuildSlot' means that the process counts towards the jobs limit. */ - void childStarted(GoalPtr goal, pid_t pid, - const set & fds, bool inBuildSlot, bool respectTimeouts); + void childStarted(GoalPtr goal, const set & fds, + bool inBuildSlot, bool respectTimeouts); /* Unregisters a running child process. `wakeSleepers' should be false if there is no sense in waking up goals that are sleeping because they can't run yet (e.g., there is no free build slot, or the hook would still say `postpone'). */ - void childTerminated(pid_t pid, bool wakeSleepers = true); + void childTerminated(GoalPtr goal, bool wakeSleepers = true); /* Put `goal' to sleep until a build slot becomes available (which might be right away). */ @@ -942,7 +943,7 @@ DerivationGoal::~DerivationGoal() void DerivationGoal::killChild() { if (pid != -1) { - worker.childTerminated(pid); + worker.childTerminated(shared_from_this()); if (buildUser.enabled()) { /* If we're using a build user, then there is a tricky @@ -1403,22 +1404,14 @@ void DerivationGoal::buildDone() to have terminated. In fact, the builder could also have simply have closed its end of the pipe --- just don't do that :-) */ - int status; - pid_t savedPid; - if (hook) { - savedPid = hook->pid; - status = hook->pid.wait(true); - } else { - /* !!! this could block! security problem! solution: kill the - child */ - savedPid = pid; - status = pid.wait(true); - } + /* !!! this could block! security problem! solution: kill the + child */ + int status = hook ? hook->pid.wait(true) : pid.wait(true); debug(format("builder process for ‘%1%’ finished") % drvPath); /* So the child is gone now. */ - worker.childTerminated(savedPid); + worker.childTerminated(shared_from_this()); /* Close the read side of the logger pipe. */ if (hook) { @@ -1621,7 +1614,7 @@ HookReply DerivationGoal::tryBuildHook() set fds; fds.insert(hook->fromHook.readSide); fds.insert(hook->builderOut.readSide); - worker.childStarted(shared_from_this(), hook->pid, fds, false, false); + worker.childStarted(shared_from_this(), fds, false, false); return rpAccept; } @@ -2155,7 +2148,7 @@ void DerivationGoal::startBuilder() /* parent */ pid.setSeparatePG(true); builderOut.writeSide.close(); - worker.childStarted(shared_from_this(), pid, + worker.childStarted(shared_from_this(), singleton >(builderOut.readSide), true, true); /* Check if setting up the build environment failed. */ @@ -3032,28 +3025,24 @@ private: Path storePath; /* The remaining substituters. */ - Paths subs; + std::list> subs; /* The current substituter. */ - Path sub; + std::shared_ptr sub; - /* Whether any substituter can realise this path */ + /* Whether any substituter can realise this path. */ bool hasSubstitute; /* Path info returned by the substituter's query info operation. */ - SubstitutablePathInfo info; + std::shared_ptr info; /* Pipe for the substituter's standard output. */ Pipe outPipe; - /* Pipe for the substituter's standard error. */ - Pipe logPipe; + /* The substituter thread. */ + std::thread thr; - /* The process ID of the builder. */ - Pid pid; - - /* Lock on the store path. */ - std::shared_ptr outputLock; + std::promise promise; /* Whether to try to repair a valid path. */ bool repair; @@ -3069,7 +3058,7 @@ public: SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false); ~SubstitutionGoal(); - void timedOut(); + void timedOut() { abort(); }; string key() { @@ -3110,18 +3099,14 @@ SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, bool SubstitutionGoal::~SubstitutionGoal() { - if (pid != -1) worker.childTerminated(pid); -} - - -void SubstitutionGoal::timedOut() -{ - if (pid != -1) { - pid_t savedPid = pid; - pid.kill(); - worker.childTerminated(savedPid); + try { + if (thr.joinable()) { + thr.join(); + worker.childTerminated(shared_from_this()); + } + } catch (...) { + ignoreException(); } - amDone(ecFailed); } @@ -3146,7 +3131,7 @@ void SubstitutionGoal::init() if (settings.readOnlyMode) throw Error(format("cannot substitute path ‘%1%’ - no write access to the Nix store") % storePath); - subs = settings.substituters; + subs = getDefaultSubstituters(); tryNext(); } @@ -3171,17 +3156,19 @@ void SubstitutionGoal::tryNext() sub = subs.front(); subs.pop_front(); - SubstitutablePathInfos infos; - PathSet dummy(singleton(storePath)); - worker.store.querySubstitutablePathInfos(sub, dummy, infos); - SubstitutablePathInfos::iterator k = infos.find(storePath); - if (k == infos.end()) { tryNext(); return; } - info = k->second; + try { + // FIXME: make async + info = sub->queryPathInfo(storePath); + } catch (InvalidPath &) { + tryNext(); + return; + } + hasSubstitute = true; /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ - for (auto & i : info.references) + for (auto & i : info->references) if (i != storePath) /* ignore self-references */ addWaitee(worker.makeSubstitutionGoal(i)); @@ -3202,7 +3189,7 @@ void SubstitutionGoal::referencesValid() return; } - for (auto & i : info.references) + for (auto & i : info->references) if (i != storePath) /* ignore self-references */ assert(worker.store.isValidPath(i)); @@ -3224,70 +3211,30 @@ void SubstitutionGoal::tryToRun() return; } - /* Maybe a derivation goal has already locked this path - (exceedingly unlikely, since it should have used a substitute - first, but let's be defensive). */ - outputLock.reset(); // make sure this goal's lock is gone - if (pathIsLockedByMe(storePath)) { - debug(format("restarting substitution of ‘%1%’ because it's locked by another goal") - % storePath); - worker.waitForAnyGoal(shared_from_this()); - return; /* restart in the tryToRun() state when another goal finishes */ - } - - /* Acquire a lock on the output path. */ - outputLock = std::make_shared(); - if (!outputLock->lockPaths(singleton(storePath), "", false)) { - worker.waitForAWhile(shared_from_this()); - return; - } - - /* Check again whether the path is invalid. */ - if (!repair && worker.store.isValidPath(storePath)) { - debug(format("store path ‘%1%’ has become valid") % storePath); - outputLock->setDeletion(true); - amDone(ecSuccess); - return; - } - printMsg(lvlInfo, format("fetching path ‘%1%’...") % storePath); outPipe.create(); - logPipe.create(); - destPath = repair ? storePath + ".tmp" : storePath; + promise = std::promise(); - /* Remove the (stale) output path if it exists. */ - deletePath(destPath); + thr = std::thread([this]() { + try { + /* Wake up the worker loop when we're done. */ + Finally updateStats([this]() { outPipe.writeSide.close(); }); - worker.store.setSubstituterEnv(); + StringSink sink; + sub->exportPaths({storePath}, false, sink); - /* Fill in the arguments. */ - Strings args; - args.push_back(baseNameOf(sub)); - args.push_back("--substitute"); - args.push_back(storePath); - args.push_back(destPath); + StringSource source(*sink.s); + worker.store.importPaths(false, source, 0); - /* Fork the substitute program. */ - pid = startProcess([&]() { - - commonChildInit(logPipe); - - if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1) - throw SysError("cannot dup output pipe into stdout"); - - execv(sub.c_str(), stringsToCharPtrs(args).data()); - - throw SysError(format("executing ‘%1%’") % sub); + promise.set_value(); + } catch (...) { + promise.set_exception(std::current_exception()); + } }); - pid.setSeparatePG(true); - pid.setKillSignal(SIGTERM); - outPipe.writeSide.close(); - logPipe.writeSide.close(); - worker.childStarted(shared_from_this(), - pid, singleton >(logPipe.readSide), true, true); + worker.childStarted(shared_from_this(), {outPipe.readSide}, true, false); state = &SubstitutionGoal::finished; } @@ -3297,52 +3244,12 @@ void SubstitutionGoal::finished() { trace("substitute finished"); - /* Since we got an EOF on the logger pipe, the substitute is - presumed to have terminated. */ - pid_t savedPid = pid; - int status = pid.wait(true); + thr.join(); + worker.childTerminated(shared_from_this()); - /* So the child is gone now. */ - worker.childTerminated(savedPid); - - /* Close the read side of the logger pipe. */ - logPipe.readSide.close(); - - /* Get the hash info from stdout. */ - string dummy = readLine(outPipe.readSide); - string expectedHashStr = statusOk(status) ? readLine(outPipe.readSide) : ""; - outPipe.readSide.close(); - - /* Check the exit status and the build result. */ - HashResult hash; try { - - if (!statusOk(status)) - throw SubstError(format("fetching path ‘%1%’ %2%") - % storePath % statusToString(status)); - - if (!pathExists(destPath)) - throw SubstError(format("substitute did not produce path ‘%1%’") % destPath); - - hash = hashPath(htSHA256, destPath); - - /* Verify the expected hash we got from the substituer. */ - if (expectedHashStr != "") { - size_t n = expectedHashStr.find(':'); - if (n == string::npos) - throw Error(format("bad hash from substituter: %1%") % expectedHashStr); - HashType hashType = parseHashType(string(expectedHashStr, 0, n)); - if (hashType == htUnknown) - throw Error(format("unknown hash algorithm in ‘%1%’") % expectedHashStr); - Hash expectedHash = parseHash16or32(hashType, string(expectedHashStr, n + 1)); - Hash actualHash = hashType == htSHA256 ? hash.first : hashPath(hashType, destPath).first; - if (expectedHash != actualHash) - throw SubstError(format("hash mismatch in downloaded path ‘%1%’: expected %2%, got %3%") - % storePath % printHash(expectedHash) % printHash(actualHash)); - } - - } catch (SubstError & e) { - + promise.get_future().get(); + } catch (Error & e) { printMsg(lvlInfo, e.msg()); /* Try the next substitute. */ @@ -3351,23 +3258,6 @@ void SubstitutionGoal::finished() return; } - if (repair) replaceValidPath(storePath, destPath); - - canonicalisePathMetaData(storePath, -1); - - worker.store.optimisePath(storePath); // FIXME: combine with hashPath() - - ValidPathInfo info2; - info2.path = storePath; - info2.narHash = hash.first; - info2.narSize = hash.second; - info2.references = info.references; - info2.deriver = info.deriver; - worker.store.registerValidPath(info2); - - outputLock->setDeletion(true); - outputLock.reset(); - worker.markContentsGood(storePath); printMsg(lvlChatty, @@ -3379,18 +3269,15 @@ void SubstitutionGoal::finished() void SubstitutionGoal::handleChildOutput(int fd, const string & data) { - assert(fd == logPipe.readSide); - printMsg(lvlError, data); // FIXME } void SubstitutionGoal::handleEOF(int fd) { - if (fd == logPipe.readSide) worker.wakeUp(shared_from_this()); + if (fd == outPipe.readSide) worker.wakeUp(shared_from_this()); } - ////////////////////////////////////////////////////////////////////// @@ -3506,9 +3393,8 @@ unsigned Worker::getNrLocalBuilds() } -void Worker::childStarted(GoalPtr goal, - pid_t pid, const set & fds, bool inBuildSlot, - bool respectTimeouts) +void Worker::childStarted(GoalPtr goal, const set & fds, + bool inBuildSlot, bool respectTimeouts) { Child child; child.goal = goal; @@ -3516,30 +3402,29 @@ void Worker::childStarted(GoalPtr goal, child.timeStarted = child.lastOutput = time(0); child.inBuildSlot = inBuildSlot; child.respectTimeouts = respectTimeouts; - children[pid] = child; + children.emplace_back(child); if (inBuildSlot) nrLocalBuilds++; } -void Worker::childTerminated(pid_t pid, bool wakeSleepers) +void Worker::childTerminated(GoalPtr goal, bool wakeSleepers) { - assert(pid != -1); /* common mistake */ - - Children::iterator i = children.find(pid); + auto i = std::find_if(children.begin(), children.end(), + [&](const Child & child) { return child.goal.lock() == goal; }); assert(i != children.end()); - if (i->second.inBuildSlot) { + if (i->inBuildSlot) { assert(nrLocalBuilds > 0); nrLocalBuilds--; } - children.erase(pid); + children.erase(i); if (wakeSleepers) { /* Wake up goals waiting for a build slot. */ - for (auto & i : wantingToBuild) { - GoalPtr goal = i.lock(); + for (auto & j : wantingToBuild) { + GoalPtr goal = j.lock(); if (goal) wakeUp(goal); } @@ -3641,11 +3526,11 @@ void Worker::waitForInput() assert(sizeof(time_t) >= sizeof(long)); time_t nearest = LONG_MAX; // nearest deadline for (auto & i : children) { - if (!i.second.respectTimeouts) continue; + if (!i.respectTimeouts) continue; if (settings.maxSilentTime != 0) - nearest = std::min(nearest, i.second.lastOutput + settings.maxSilentTime); + nearest = std::min(nearest, i.lastOutput + settings.maxSilentTime); if (settings.buildTimeout != 0) - nearest = std::min(nearest, i.second.timeStarted + settings.buildTimeout); + nearest = std::min(nearest, i.timeStarted + settings.buildTimeout); } if (nearest != LONG_MAX) { timeout.tv_sec = std::max((time_t) 1, nearest - before); @@ -3663,7 +3548,6 @@ void Worker::waitForInput() timeout.tv_sec = std::max((time_t) 1, (time_t) (lastWokenUp + settings.pollInterval - before)); } else lastWokenUp = 0; - using namespace std; /* Use select() to wait for the input side of any logger pipe to become `available'. Note that `available' (i.e., non-blocking) includes EOF. */ @@ -3671,7 +3555,7 @@ void Worker::waitForInput() FD_ZERO(&fds); int fdMax = 0; for (auto & i : children) { - for (auto & j : i.second.fds) { + for (auto & j : i.fds) { FD_SET(j, &fds); if (j >= fdMax) fdMax = j + 1; } @@ -3685,22 +3569,16 @@ void Worker::waitForInput() time_t after = time(0); /* Process all available file descriptors. */ + decltype(children)::iterator i; + for (auto j = children.begin(); j != children.end(); j = i) { + i = std::next(j); - /* Since goals may be canceled from inside the loop below (causing - them go be erased from the `children' map), we have to be - careful that we don't keep iterators alive across calls to - timedOut(). */ - set pids; - for (auto & i : children) pids.insert(i.first); - - for (auto & i : pids) { checkInterrupt(); - Children::iterator j = children.find(i); - if (j == children.end()) continue; // child destroyed - GoalPtr goal = j->second.goal.lock(); + + GoalPtr goal = j->goal.lock(); assert(goal); - set fds2(j->second.fds); + set fds2(j->fds); for (auto & k : fds2) { if (FD_ISSET(k, &fds)) { unsigned char buffer[4096]; @@ -3712,12 +3590,12 @@ void Worker::waitForInput() } else if (rd == 0) { debug(format("%1%: got EOF") % goal->getName()); goal->handleEOF(k); - j->second.fds.erase(k); + j->fds.erase(k); } else { printMsg(lvlVomit, format("%1%: read %2% bytes") % goal->getName() % rd); string data((char *) buffer, rd); - j->second.lastOutput = after; + j->lastOutput = after; goal->handleChildOutput(k, data); } } @@ -3725,8 +3603,8 @@ void Worker::waitForInput() if (goal->getExitCode() == Goal::ecBusy && settings.maxSilentTime != 0 && - j->second.respectTimeouts && - after - j->second.lastOutput >= (time_t) settings.maxSilentTime) + j->respectTimeouts && + after - j->lastOutput >= (time_t) settings.maxSilentTime) { printMsg(lvlError, format("%1% timed out after %2% seconds of silence") @@ -3736,8 +3614,8 @@ void Worker::waitForInput() else if (goal->getExitCode() == Goal::ecBusy && settings.buildTimeout != 0 && - j->second.respectTimeouts && - after - j->second.timeStarted >= (time_t) settings.buildTimeout) + j->respectTimeouts && + after - j->timeStarted >= (time_t) settings.buildTimeout) { printMsg(lvlError, format("%1% timed out after %2% seconds") diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 90539ea85..c12178e40 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -184,19 +184,6 @@ void Settings::update() _get(enableImportNative, "allow-unsafe-native-code-during-evaluation"); _get(useCaseHack, "use-case-hack"); _get(preBuildHook, "pre-build-hook"); - - string subs = getEnv("NIX_SUBSTITUTERS", "default"); - if (subs == "default") { - substituters.clear(); -#if 0 - if (getEnv("NIX_OTHER_STORES") != "") - substituters.push_back(nixLibexecDir + "/nix/substituters/copy-from-other-stores.pl"); -#endif - substituters.push_back(nixLibexecDir + "/nix/substituters/download-from-binary-cache.pl"); - if (useSshSubstituter && !sshSubstituterHosts.empty()) - substituters.push_back(nixLibexecDir + "/nix/substituters/download-via-ssh"); - } else - substituters = tokenizeString(subs, ":"); } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 60251ef42..65f763ace 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -110,11 +110,6 @@ struct Settings { means infinity. */ time_t buildTimeout; - /* The substituters. There are programs that can somehow realise - a store path without building, e.g., by downloading it or - copying it from a CD. */ - Paths substituters; - /* Whether to use build hooks (for distributed builds). Sometimes users want to disable this from the command-line. */ bool useBuildHook; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index baf3a528b..45ffbad67 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -5,12 +5,11 @@ #include "pathlocks.hh" #include "worker-protocol.hh" #include "derivations.hh" -#include "affinity.hh" +#include "nar-info.hh" #include #include #include -#include #include #include @@ -219,19 +218,6 @@ LocalStore::~LocalStore() { auto state(_state.lock()); - try { - for (auto & i : state->runningSubstituters) { - if (i.second.disabled) continue; - i.second.to.close(); - i.second.from.close(); - i.second.error.close(); - if (i.second.pid != -1) - i.second.pid.wait(true); - } - } catch (...) { - ignoreException(); - } - try { if (state->fdTempRoots != -1) { state->fdTempRoots.close(); @@ -792,205 +778,42 @@ Path LocalStore::queryPathFromHashPart(const string & hashPart) } -void LocalStore::setSubstituterEnv() -{ - static std::atomic_flag done; - - if (done.test_and_set()) return; - - /* Pass configuration options (including those overridden with - --option) to substituters. */ - setenv("_NIX_OPTIONS", settings.pack().c_str(), 1); -} - - -void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run) -{ - if (run.disabled || run.pid != -1) return; - - debug(format("starting substituter program ‘%1%’") % substituter); - - Pipe toPipe, fromPipe, errorPipe; - - toPipe.create(); - fromPipe.create(); - errorPipe.create(); - - setSubstituterEnv(); - - run.pid = startProcess([&]() { - if (dup2(toPipe.readSide, STDIN_FILENO) == -1) - throw SysError("dupping stdin"); - if (dup2(fromPipe.writeSide, STDOUT_FILENO) == -1) - throw SysError("dupping stdout"); - if (dup2(errorPipe.writeSide, STDERR_FILENO) == -1) - throw SysError("dupping stderr"); - execl(substituter.c_str(), substituter.c_str(), "--query", NULL); - throw SysError(format("executing ‘%1%’") % substituter); - }); - - run.program = baseNameOf(substituter); - run.to = toPipe.writeSide.borrow(); - run.from = run.fromBuf.fd = fromPipe.readSide.borrow(); - run.error = errorPipe.readSide.borrow(); - - toPipe.readSide.close(); - fromPipe.writeSide.close(); - errorPipe.writeSide.close(); - - /* The substituter may exit right away if it's disabled in any way - (e.g. copy-from-other-stores.pl will exit if no other stores - are configured). */ - try { - getLineFromSubstituter(run); - } catch (EndOfFile & e) { - run.to.close(); - run.from.close(); - run.error.close(); - run.disabled = true; - if (run.pid.wait(true) != 0) throw; - } -} - - -/* Read a line from the substituter's stdout, while also processing - its stderr. */ -string LocalStore::getLineFromSubstituter(RunningSubstituter & run) -{ - string res, err; - - /* We might have stdout data left over from the last time. */ - if (run.fromBuf.hasData()) goto haveData; - - while (1) { - checkInterrupt(); - - fd_set fds; - FD_ZERO(&fds); - FD_SET(run.from, &fds); - FD_SET(run.error, &fds); - - /* Wait for data to appear on the substituter's stdout or - stderr. */ - if (select(run.from > run.error ? run.from + 1 : run.error + 1, &fds, 0, 0, 0) == -1) { - if (errno == EINTR) continue; - throw SysError("waiting for input from the substituter"); - } - - /* Completely drain stderr before dealing with stdout. */ - if (FD_ISSET(run.error, &fds)) { - char buf[4096]; - ssize_t n = read(run.error, (unsigned char *) buf, sizeof(buf)); - if (n == -1) { - if (errno == EINTR) continue; - throw SysError("reading from substituter's stderr"); - } - if (n == 0) throw EndOfFile(format("substituter ‘%1%’ died unexpectedly") % run.program); - err.append(buf, n); - string::size_type p; - while ((p = err.find('\n')) != string::npos) { - printMsg(lvlError, run.program + ": " + string(err, 0, p)); - err = string(err, p + 1); - } - } - - /* Read from stdout until we get a newline or the buffer is empty. */ - else if (run.fromBuf.hasData() || FD_ISSET(run.from, &fds)) { - haveData: - do { - unsigned char c; - run.fromBuf(&c, 1); - if (c == '\n') { - if (!err.empty()) printMsg(lvlError, run.program + ": " + err); - return res; - } - res += c; - } while (run.fromBuf.hasData()); - } - } -} - - -template T LocalStore::getIntLineFromSubstituter(RunningSubstituter & run) -{ - string s = getLineFromSubstituter(run); - T res; - if (!string2Int(s, res)) throw Error("integer expected from stream"); - return res; -} - - PathSet LocalStore::querySubstitutablePaths(const PathSet & paths) { - auto state(_state.lock()); - PathSet res; - for (auto & i : settings.substituters) { - if (res.size() == paths.size()) break; - RunningSubstituter & run(state->runningSubstituters[i]); - startSubstituter(i, run); - if (run.disabled) continue; - string s = "have "; - for (auto & j : paths) - if (res.find(j) == res.end()) { s += j; s += " "; } - writeLine(run.to, s); - while (true) { - /* FIXME: we only read stderr when an error occurs, so - substituters should only write (short) messages to - stderr when they fail. I.e. they shouldn't write debug - output. */ - Path path = getLineFromSubstituter(run); - if (path == "") break; - res.insert(path); + for (auto & sub : getDefaultSubstituters()) { + for (auto & path : paths) { + if (res.count(path)) continue; + debug(format("checking substituter ‘%s’ for path ‘%s’") + % sub->getUri() % path); + if (sub->isValidPath(path)) + res.insert(path); } } - return res; } -void LocalStore::querySubstitutablePathInfos(const Path & substituter, - PathSet & paths, SubstitutablePathInfos & infos) -{ - auto state(_state.lock()); - - RunningSubstituter & run(state->runningSubstituters[substituter]); - startSubstituter(substituter, run); - if (run.disabled) return; - - string s = "info "; - for (auto & i : paths) - if (infos.find(i) == infos.end()) { s += i; s += " "; } - writeLine(run.to, s); - - while (true) { - Path path = getLineFromSubstituter(run); - if (path == "") break; - if (paths.find(path) == paths.end()) - throw Error(format("got unexpected path ‘%1%’ from substituter") % path); - paths.erase(path); - SubstitutablePathInfo & info(infos[path]); - info.deriver = getLineFromSubstituter(run); - if (info.deriver != "") assertStorePath(info.deriver); - int nrRefs = getIntLineFromSubstituter(run); - while (nrRefs--) { - Path p = getLineFromSubstituter(run); - assertStorePath(p); - info.references.insert(p); - } - info.downloadSize = getIntLineFromSubstituter(run); - info.narSize = getIntLineFromSubstituter(run); - } -} - - void LocalStore::querySubstitutablePathInfos(const PathSet & paths, SubstitutablePathInfos & infos) { - PathSet todo = paths; - for (auto & i : settings.substituters) { - if (todo.empty()) break; - querySubstitutablePathInfos(i, todo, infos); + for (auto & sub : getDefaultSubstituters()) { + for (auto & path : paths) { + if (infos.count(path)) continue; + debug(format("checking substituter ‘%s’ for path ‘%s’") + % sub->getUri() % path); + try { + auto info = sub->queryPathInfo(path); + auto narInfo = std::dynamic_pointer_cast( + std::shared_ptr(info)); + infos[path] = SubstitutablePathInfo{ + info->deriver, + info->references, + narInfo ? narInfo->fileSize : 0, + info->narSize}; + } catch (InvalidPath) { + } + } } } diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index daf394c92..abc4f719b 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -40,17 +40,6 @@ struct OptimiseStats }; -struct RunningSubstituter -{ - Path program; - Pid pid; - AutoCloseFD to, from, error; - FdSource fromBuf; - bool disabled; - RunningSubstituter() : disabled(false) { }; -}; - - class LocalStore : public LocalFSStore { private: @@ -80,10 +69,6 @@ private: /* The file to which we write our temporary roots. */ Path fnTempRoots; AutoCloseFD fdTempRoots; - - typedef std::map RunningSubstituters; - RunningSubstituters runningSubstituters; - }; Sync _state; @@ -122,9 +107,6 @@ public: PathSet querySubstitutablePaths(const PathSet & paths) override; - void querySubstitutablePathInfos(const Path & substituter, - PathSet & paths, SubstitutablePathInfos & infos); - void querySubstitutablePathInfos(const PathSet & paths, SubstitutablePathInfos & infos) override; @@ -192,8 +174,6 @@ public: a substituter (if available). */ void repairPath(const Path & path); - void setSubstituterEnv(); - void addSignatures(const Path & storePath, const StringSet & sigs) override; static bool haveWriteAccess(); @@ -246,13 +226,6 @@ private: void removeUnusedLinks(const GCState & state); - void startSubstituter(const Path & substituter, - RunningSubstituter & runningSubstituter); - - string getLineFromSubstituter(RunningSubstituter & run); - - template T getIntLineFromSubstituter(RunningSubstituter & run); - Path createTempDirInStore(); Path importPath(bool requireSignature, Source & source); diff --git a/src/libstore/local.mk b/src/libstore/local.mk index 15fa91b5c..22b0f235e 100644 --- a/src/libstore/local.mk +++ b/src/libstore/local.mk @@ -8,7 +8,7 @@ libstore_SOURCES := $(wildcard $(d)/*.cc) libstore_LIBS = libutil libformat -libstore_LDFLAGS = $(SQLITE3_LIBS) -lbz2 $(LIBCURL_LIBS) $(SODIUM_LIBS) -laws-cpp-sdk-s3 -laws-cpp-sdk-core +libstore_LDFLAGS = $(SQLITE3_LIBS) -lbz2 $(LIBCURL_LIBS) $(SODIUM_LIBS) -laws-cpp-sdk-s3 -laws-cpp-sdk-core -pthread ifeq ($(OS), SunOS) libstore_LDFLAGS += -lsocket diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 14932f9b0..2763f5ad4 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -501,4 +501,39 @@ static RegisterStoreImplementation regStore([](const std::string & uri) -> std:: }); +std::list> getDefaultSubstituters() +{ + struct State { + bool done = false; + std::list> stores; + }; + static Sync state_; + + auto state(state_.lock()); + + if (state->done) return state->stores; + + StringSet done; + + auto addStore = [&](const std::string & uri) { + if (done.count(uri)) return; + done.insert(uri); + state->stores.push_back(openStoreAt(uri)); + }; + + for (auto uri : settings.get("substituters", Strings())) + addStore(uri); + + for (auto uri : settings.get("binary-caches", Strings())) + addStore(uri); + + for (auto uri : settings.get("extra-binary-caches", Strings())) + addStore(uri); + + state->done = true; + + return state->stores; +} + + } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index e0cc32296..87b8e8847 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -533,6 +533,12 @@ ref openLocalBinaryCacheStore(std::shared_ptr localStore, const Path & secretKeyFile, const Path & binaryCacheDir); +/* Return the default substituter stores, defined by the + ‘substituters’ option and various legacy options like + ‘binary-caches’. */ +std::list> getDefaultSubstituters(); + + /* Store implementation registration. */ typedef std::function(const std::string & uri)> OpenStore; diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh new file mode 100644 index 000000000..47c64deae --- /dev/null +++ b/src/libutil/finally.hh @@ -0,0 +1,12 @@ +#pragma once + +/* A trivial class to run a function at the end of a scope. */ +class Finally +{ +private: + std::function fun; + +public: + Finally(std::function fun) : fun(fun) { } + ~Finally() { fun(); } +}; diff --git a/tests/fallback.sh b/tests/fallback.sh deleted file mode 100644 index f3a6b5051..000000000 --- a/tests/fallback.sh +++ /dev/null @@ -1,20 +0,0 @@ -source common.sh - -clearStore - -drvPath=$(nix-instantiate simple.nix) -echo "derivation is $drvPath" - -outPath=$(nix-store -q --fallback "$drvPath") -echo "output path is $outPath" - -# Build with a substitute that fails. This should fail. -export NIX_SUBSTITUTERS=$(pwd)/substituter2.sh -if nix-store -r "$drvPath"; then echo unexpected fallback; exit 1; fi - -# Build with a substitute that fails. This should fall back to a source build. -export NIX_SUBSTITUTERS=$(pwd)/substituter2.sh -nix-store -r --fallback "$drvPath" - -text=$(cat "$outPath"/hello) -if test "$text" != "Hello World!"; then exit 1; fi diff --git a/tests/local.mk b/tests/local.mk index 471821b27..7c5a553d3 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -3,8 +3,7 @@ check: nix_tests = \ init.sh hash.sh lang.sh add.sh simple.sh dependencies.sh \ - build-hook.sh substitutes.sh substitutes2.sh \ - fallback.sh nix-push.sh gc.sh gc-concurrent.sh \ + build-hook.sh nix-push.sh gc.sh gc-concurrent.sh \ referrers.sh user-envs.sh logging.sh nix-build.sh misc.sh fixed.sh \ gc-runtime.sh install-package.sh check-refs.sh filter-source.sh \ remote-store.sh export.sh export-graph.sh \ diff --git a/tests/substituter.sh b/tests/substituter.sh deleted file mode 100755 index 9aab295de..000000000 --- a/tests/substituter.sh +++ /dev/null @@ -1,37 +0,0 @@ -#! /bin/sh -e -echo -echo substituter args: $* >&2 - -if test $1 = "--query"; then - while read cmd args; do - echo "CMD = $cmd, ARGS = $args" >&2 - if test "$cmd" = "have"; then - for path in $args; do - read path - if grep -q "$path" $TEST_ROOT/sub-paths; then - echo $path - fi - done - echo - elif test "$cmd" = "info"; then - for path in $args; do - echo $path - echo "" # deriver - echo 0 # nr of refs - echo $((1 * 1024 * 1024)) # download size - echo $((2 * 1024 * 1024)) # nar size - done - echo - else - echo "bad command $cmd" - exit 1 - fi - done -elif test $1 = "--substitute"; then - mkdir $2 - echo "Hallo Wereld" > $2/hello - echo # no expected hash -else - echo "unknown substituter operation" - exit 1 -fi diff --git a/tests/substituter2.sh b/tests/substituter2.sh deleted file mode 100755 index 5d1763599..000000000 --- a/tests/substituter2.sh +++ /dev/null @@ -1,33 +0,0 @@ -#! /bin/sh -e -echo -echo substituter2 args: $* >&2 - -if test $1 = "--query"; then - while read cmd args; do - if test "$cmd" = have; then - for path in $args; do - if grep -q "$path" $TEST_ROOT/sub-paths; then - echo $path - fi - done - echo - elif test "$cmd" = info; then - for path in $args; do - echo $path - echo "" # deriver - echo 0 # nr of refs - echo 0 # download size - echo 0 # nar size - done - echo - else - echo "bad command $cmd" - exit 1 - fi - done -elif test $1 = "--substitute"; then - exit 1 -else - echo "unknown substituter operation" - exit 1 -fi diff --git a/tests/substitutes.sh b/tests/substitutes.sh deleted file mode 100644 index 0c6adf260..000000000 --- a/tests/substitutes.sh +++ /dev/null @@ -1,22 +0,0 @@ -source common.sh - -clearStore - -# Instantiate. -drvPath=$(nix-instantiate simple.nix) -echo "derivation is $drvPath" - -# Find the output path. -outPath=$(nix-store -qvv "$drvPath") -echo "output path is $outPath" - -echo $outPath > $TEST_ROOT/sub-paths - -export NIX_SUBSTITUTERS=$(pwd)/substituter.sh - -nix-store -r "$drvPath" --dry-run 2>&1 | grep -q "1.00 MiB.*2.00 MiB" - -nix-store -rvv "$drvPath" - -text=$(cat "$outPath"/hello) -if test "$text" != "Hallo Wereld"; then echo "wrong substitute output: $text"; exit 1; fi diff --git a/tests/substitutes2.sh b/tests/substitutes2.sh deleted file mode 100644 index bd914575c..000000000 --- a/tests/substitutes2.sh +++ /dev/null @@ -1,21 +0,0 @@ -source common.sh - -clearStore - -# Instantiate. -drvPath=$(nix-instantiate simple.nix) -echo "derivation is $drvPath" - -# Find the output path. -outPath=$(nix-store -qvvvvv "$drvPath") -echo "output path is $outPath" - -echo $outPath > $TEST_ROOT/sub-paths - -# First try a substituter that fails, then one that succeeds -export NIX_SUBSTITUTERS=$(pwd)/substituter2.sh:$(pwd)/substituter.sh - -nix-store -j0 -rvv "$drvPath" - -text=$(cat "$outPath"/hello) -if test "$text" != "Hallo Wereld"; then echo "wrong substitute output: $text"; exit 1; fi