daemon: Run 'guix substitute' directly and assume a single substituter.

The daemon had a mechanism that allows it to handle a list of
substituters and try them sequentially; this removes it.

* nix/scripts/substitute.in: Remove.
* nix/local.mk (nodist_pkglibexec_SCRIPTS): Remove.
* config-daemon.ac: Don't output 'nix/scripts/substitute'.
* nix/libstore/build.cc (SubstitutionGoal)[subs, sub, hasSubstitute]:
Remove.
[tryNext]: Make private.
(SubstitutionGoal::SubstitutionGoal, SubstitutionGoal::init): Remove now
unneeded initializers.
(SubstitutionGoal::tryNext): Adjust to assume a single substituter: call
'amDone' upfront when we couldn't find substitutes.
(SubstitutionGoal::tryToRun): Adjust to run 'guix substitute' via
'settings.guixProgram'.
(SubstitutionGoal::finished): Call 'amDone(ecFailed)' upon failure
instead of setting 'state' to 'tryNext'.
* nix/libstore/globals.hh (Settings)[substituters]: Remove.
* nix/libstore/local-store.cc (LocalStore::~LocalStore): Adjust to
handle a single substituter.
(LocalStore::startSubstituter): Remove 'path' parameter.  Adjust to
invoke 'settings.guixProgram'.  Don't refer to 'run.program', which no
longer exists.
(LocalStore::querySubstitutablePaths): Adjust for 'runningSubstituters'
being a singleton instead of a list.
(LocalStore::querySubstitutablePathInfos): Likewise, and remove
'substituter' parameter.
* nix/libstore/local-store.hh (RunningSubstituter)[program]: Remove.
(LocalStore)[runningSubstituters]: Remove.
[runningSubstituter]: New field.
[querySubstitutablePathInfos]: Remove 'substituter' parameter.
[startSubstituter]: Remove 'substituter' parameter.
* nix/nix-daemon/guix-daemon.cc (main): Remove references to
'settings.substituters'.
* nix/nix-daemon/nix-daemon.cc (performOp): Ignore the user's
"build-use-substitutes" value when 'settings.useSubstitutes' is false.
This commit is contained in:
Ludovic Courtès 2019-09-04 11:04:44 +02:00
parent bc69ea2d60
commit f6919ebdc6
No known key found for this signature in database
GPG Key ID: 090B11993D9AEBB5
9 changed files with 84 additions and 115 deletions

View File

@ -148,8 +148,6 @@ if test "x$guix_build_daemon" = "xyes"; then
AC_SUBST([GUIX_TEST_ROOT])
GUIX_CHECK_LOCALSTATEDIR
AC_CONFIG_FILES([nix/scripts/substitute],
[chmod +x nix/scripts/substitute])
fi
AM_CONDITIONAL([HAVE_LIBBZ2], [test "x$HAVE_LIBBZ2" = "xyes"])

View File

@ -2863,15 +2863,6 @@ private:
/* The store path that should be realised through a substitute. */
Path storePath;
/* The remaining substituters. */
Paths subs;
/* The current substituter. */
Path sub;
/* Whether any substituter can realise this path */
bool hasSubstitute;
/* Path info returned by the substituter's query info operation. */
SubstitutablePathInfo info;
@ -2897,6 +2888,8 @@ private:
typedef void (SubstitutionGoal::*GoalState)();
GoalState state;
void tryNext();
public:
SubstitutionGoal(const Path & storePath, Worker & worker, bool repair = false);
~SubstitutionGoal();
@ -2914,7 +2907,6 @@ public:
/* The states. */
void init();
void tryNext();
void gotInfo();
void referencesValid();
void tryToRun();
@ -2930,7 +2922,6 @@ public:
SubstitutionGoal::SubstitutionGoal(const Path & storePath, Worker & worker, bool repair)
: Goal(worker)
, hasSubstitute(false)
, repair(repair)
{
this->storePath = storePath;
@ -2980,37 +2971,31 @@ void SubstitutionGoal::init()
if (settings.readOnlyMode)
throw Error(format("cannot substitute path `%1%' - no write access to the store") % storePath);
subs = settings.substituters;
tryNext();
}
void SubstitutionGoal::tryNext()
{
trace("trying next substituter");
trace("trying substituter");
if (subs.size() == 0) {
SubstitutablePathInfos infos;
PathSet dummy(singleton<PathSet>(storePath));
worker.store.querySubstitutablePathInfos(dummy, infos);
SubstitutablePathInfos::iterator k = infos.find(storePath);
if (k == infos.end()) {
/* None left. Terminate this goal and let someone else deal
with it. */
debug(format("path `%1%' is required, but there is no substituter that can build it") % storePath);
/* Hack: don't indicate failure if there were no substituters.
In that case the calling derivation should just do a
build. */
amDone(hasSubstitute ? ecFailed : ecNoSubstituters);
return;
amDone(ecNoSubstituters);
return;
}
sub = subs.front();
subs.pop_front();
SubstitutablePathInfos infos;
PathSet dummy(singleton<PathSet>(storePath));
worker.store.querySubstitutablePathInfos(sub, dummy, infos);
SubstitutablePathInfos::iterator k = infos.find(storePath);
if (k == infos.end()) { tryNext(); return; }
/* Found a substitute. */
info = k->second;
hasSubstitute = true;
/* To maintain the closure invariant, we first have to realise the
paths referenced by this one. */
@ -3098,7 +3083,8 @@ void SubstitutionGoal::tryToRun()
/* Fill in the arguments. */
Strings args;
args.push_back(baseNameOf(sub));
args.push_back("guix");
args.push_back("substitute");
args.push_back("--substitute");
args.push_back(storePath);
args.push_back(destPath);
@ -3111,9 +3097,9 @@ void SubstitutionGoal::tryToRun()
if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
throw SysError("cannot dup output pipe into stdout");
execv(sub.c_str(), stringsToCharPtrs(args).data());
execv(settings.guixProgram.c_str(), stringsToCharPtrs(args).data());
throw SysError(format("executing `%1%'") % sub);
throw SysError(format("executing `%1% substitute'") % settings.guixProgram);
});
pid.setSeparatePG(true);
@ -3126,7 +3112,9 @@ void SubstitutionGoal::tryToRun()
state = &SubstitutionGoal::finished;
if (settings.printBuildTrace)
printMsg(lvlError, format("@ substituter-started %1% %2%") % storePath % sub);
/* The second element in the message used to be the name of the
substituter but we're left with only one. */
printMsg(lvlError, format("@ substituter-started %1% substitute") % storePath);
}
@ -3192,9 +3180,7 @@ void SubstitutionGoal::finished()
% storePath % status % e.msg());
}
/* Try the next substitute. */
state = &SubstitutionGoal::tryNext;
worker.wakeUp(shared_from_this());
amDone(ecFailed);
return;
}

View File

@ -115,11 +115,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;

View File

@ -184,13 +184,15 @@ LocalStore::LocalStore(bool reserveSpace)
LocalStore::~LocalStore()
{
try {
foreach (RunningSubstituters::iterator, i, 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);
if (runningSubstituter) {
RunningSubstituter &i = *runningSubstituter;
if (!i.disabled) {
i.to.close();
i.from.close();
i.error.close();
if (i.pid != -1)
i.pid.wait(true);
}
}
} catch (...) {
ignoreException();
@ -808,11 +810,12 @@ void LocalStore::setSubstituterEnv()
}
void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter & run)
void LocalStore::startSubstituter(RunningSubstituter & run)
{
if (run.disabled || run.pid != -1) return;
debug(format("starting substituter program `%1%'") % substituter);
debug(format("starting substituter program `%1% substitute'")
% settings.guixProgram);
Pipe toPipe, fromPipe, errorPipe;
@ -829,11 +832,10 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
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);
execl(settings.guixProgram.c_str(), "guix", "substitute", "--query", NULL);
throw SysError(format("executing `%1%'") % settings.guixProgram);
});
run.program = baseNameOf(substituter);
run.to = toPipe.writeSide.borrow();
run.from = run.fromBuf.fd = fromPipe.readSide.borrow();
run.error = errorPipe.readSide.borrow();
@ -889,13 +891,14 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
if (errno == EINTR) continue;
throw SysError("reading from substituter's stderr");
}
if (n == 0) throw EndOfFile(format("substituter `%1%' died unexpectedly") % run.program);
if (n == 0) throw EndOfFile(format("`%1% substitute' died unexpectedly")
% settings.guixProgram);
err.append(buf, n);
string::size_type p;
while (((p = err.find('\n')) != string::npos)
|| ((p = err.find('\r')) != string::npos)) {
string thing(err, 0, p + 1);
writeToStderr(run.program + ": " + thing);
writeToStderr("substitute: " + thing);
err = string(err, p + 1);
}
}
@ -907,7 +910,7 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
unsigned char c;
run.fromBuf(&c, 1);
if (c == '\n') {
if (!err.empty()) printMsg(lvlError, run.program + ": " + err);
if (!err.empty()) printMsg(lvlError, "substitute: " + err);
return res;
}
res += c;
@ -930,38 +933,47 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
{
PathSet res;
if (!settings.useSubstitutes) return res;
if (!settings.useSubstitutes || paths.empty()) return res;
foreach (Paths::iterator, i, settings.substituters) {
if (res.size() == paths.size()) break;
RunningSubstituter & run(runningSubstituters[*i]);
startSubstituter(*i, run);
if (run.disabled) continue;
string s = "have ";
foreach (PathSet::const_iterator, 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);
}
if (!runningSubstituter) {
std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
runningSubstituter.swap(fresh);
}
RunningSubstituter & run = *runningSubstituter;
startSubstituter(run);
if (!run.disabled) {
string s = "have ";
foreach (PathSet::const_iterator, 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);
}
}
return res;
}
void LocalStore::querySubstitutablePathInfos(const Path & substituter,
PathSet & paths, SubstitutablePathInfos & infos)
void LocalStore::querySubstitutablePathInfos(PathSet & paths, SubstitutablePathInfos & infos)
{
if (!settings.useSubstitutes) return;
RunningSubstituter & run(runningSubstituters[substituter]);
startSubstituter(substituter, run);
if (!runningSubstituter) {
std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
runningSubstituter.swap(fresh);
}
RunningSubstituter & run = *runningSubstituter;
startSubstituter(run);
if (run.disabled) return;
string s = "info ";
@ -993,10 +1005,9 @@ void LocalStore::querySubstitutablePathInfos(const Path & substituter,
void LocalStore::querySubstitutablePathInfos(const PathSet & paths,
SubstitutablePathInfos & infos)
{
PathSet todo = paths;
foreach (Paths::iterator, i, settings.substituters) {
if (todo.empty()) break;
querySubstitutablePathInfos(*i, todo, infos);
if (!paths.empty()) {
PathSet todo = paths;
querySubstitutablePathInfos(todo, infos);
}
}

View File

@ -40,7 +40,6 @@ struct OptimiseStats
struct RunningSubstituter
{
Path program;
Pid pid;
AutoCloseFD to, from, error;
FdSource fromBuf;
@ -52,8 +51,8 @@ struct RunningSubstituter
class LocalStore : public StoreAPI
{
private:
typedef std::map<Path, RunningSubstituter> RunningSubstituters;
RunningSubstituters runningSubstituters;
/* The currently running substituter or empty. */
std::unique_ptr<RunningSubstituter> runningSubstituter;
Path linksDir;
@ -93,8 +92,8 @@ public:
PathSet querySubstitutablePaths(const PathSet & paths);
void querySubstitutablePathInfos(const Path & substituter,
PathSet & paths, SubstitutablePathInfos & infos);
void querySubstitutablePathInfos(PathSet & paths,
SubstitutablePathInfos & infos);
void querySubstitutablePathInfos(const PathSet & paths,
SubstitutablePathInfos & infos);
@ -261,8 +260,7 @@ private:
void removeUnusedLinks(const GCState & state);
void startSubstituter(const Path & substituter,
RunningSubstituter & runningSubstituter);
void startSubstituter(RunningSubstituter & runningSubstituter);
string getLineFromSubstituter(RunningSubstituter & run);

View File

@ -154,9 +154,6 @@ noinst_HEADERS = \
(lambda (in) \
(write (get-string-all in) out)))))"
nodist_pkglibexec_SCRIPTS = \
%D%/scripts/substitute
# The '.service' files for systemd.
systemdservicedir = $(libdir)/systemd/system
nodist_systemdservice_DATA = etc/guix-daemon.service etc/guix-publish.service

View File

@ -466,8 +466,7 @@ main (int argc, char *argv[])
{
settings.processEnvironment ();
/* Use our substituter by default. */
settings.substituters.clear ();
/* Enable substitutes by default. */
settings.set ("build-use-substitutes", "true");
/* Use our substitute server by default. */
@ -490,14 +489,6 @@ main (int argc, char *argv[])
printMsg(lvlDebug,
format ("build log compression: %1%") % settings.logCompression);
if (settings.useSubstitutes)
settings.substituters.push_back (settings.nixLibexecDir
+ "/substitute");
else
/* Clear the substituter list to make sure nothing ever gets
substituted, regardless of the client's settings. */
settings.substituters.clear ();
if (geteuid () == 0 && settings.buildUsersGroup.empty ())
fprintf (stderr, _("warning: daemon is running as root, so \
using `--build-users-group' is highly recommended\n"));

View File

@ -596,8 +596,12 @@ static void performOp(bool trusted, unsigned int clientVersion,
if (GET_PROTOCOL_MINOR(clientVersion) >= 6
&& GET_PROTOCOL_MINOR(clientVersion) < 0x61)
settings.set("build-cores", std::to_string(readInt(from)));
if (GET_PROTOCOL_MINOR(clientVersion) >= 10)
settings.set("build-use-substitutes", readInt(from) ? "true" : "false");
if (GET_PROTOCOL_MINOR(clientVersion) >= 10) {
if (settings.useSubstitutes)
settings.set("build-use-substitutes", readInt(from) ? "true" : "false");
else
readInt(from); // substitutes remain disabled
}
if (GET_PROTOCOL_MINOR(clientVersion) >= 12) {
unsigned int n = readInt(from);
for (unsigned int i = 0; i < n; i++) {

View File

@ -1,11 +0,0 @@
#!@SHELL@
# A shorthand for "guix substitute", for use by the daemon.
if test "x$GUIX_UNINSTALLED" = "x"
then
prefix="@prefix@"
exec_prefix="@exec_prefix@"
exec "@bindir@/guix" substitute "$@"
else
exec guix substitute "$@"
fi