[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
05/07: daemon: Run 'guix substitute' directly and assume a single substi
|
From: |
guix-commits |
|
Subject: |
05/07: daemon: Run 'guix substitute' directly and assume a single substituter. |
|
Date: |
Sun, 8 Sep 2019 06:02:43 -0400 (EDT) |
civodul pushed a commit to branch master
in repository guix.
commit f6919ebdc6b0ce0286814cc6ab0564b1a4c67f5f
Author: Ludovic Courtès <address@hidden>
Date: Wed Sep 4 11:04:44 2019 +0200
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.
---
config-daemon.ac | 2 -
nix/libstore/build.cc | 52 +++++++++--------------
nix/libstore/globals.hh | 5 ---
nix/libstore/local-store.cc | 97 ++++++++++++++++++++++++-------------------
nix/libstore/local-store.hh | 12 +++---
nix/local.mk | 3 --
nix/nix-daemon/guix-daemon.cc | 11 +----
nix/nix-daemon/nix-daemon.cc | 8 +++-
nix/scripts/substitute.in | 11 -----
9 files changed, 85 insertions(+), 116 deletions(-)
diff --git a/config-daemon.ac b/config-daemon.ac
index 3d92e8f..bf94815 100644
--- a/config-daemon.ac
+++ b/config-daemon.ac
@@ -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"])
diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 9f1f889..ad53b81 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -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;
}
diff --git a/nix/libstore/globals.hh b/nix/libstore/globals.hh
index 0d9315a..0069c85 100644
--- a/nix/libstore/globals.hh
+++ b/nix/libstore/globals.hh
@@ -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;
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 951c35f..3b08492 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -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;
-
- 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 (!settings.useSubstitutes || paths.empty()) return res;
+
+ 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);
}
}
diff --git a/nix/libstore/local-store.hh b/nix/libstore/local-store.hh
index 4e6b4cf..4113faf 100644
--- a/nix/libstore/local-store.hh
+++ b/nix/libstore/local-store.hh
@@ -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);
diff --git a/nix/local.mk b/nix/local.mk
index 8e52c77..18e9ba7 100644
--- a/nix/local.mk
+++ b/nix/local.mk
@@ -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
diff --git a/nix/nix-daemon/guix-daemon.cc b/nix/nix-daemon/guix-daemon.cc
index 73962af..6f9c404 100644
--- a/nix/nix-daemon/guix-daemon.cc
+++ b/nix/nix-daemon/guix-daemon.cc
@@ -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"));
diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc
index f29bcd2..ffac6cd 100644
--- a/nix/nix-daemon/nix-daemon.cc
+++ b/nix/nix-daemon/nix-daemon.cc
@@ -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++) {
diff --git a/nix/scripts/substitute.in b/nix/scripts/substitute.in
deleted file mode 100644
index 5a2eeb7..0000000
--- a/nix/scripts/substitute.in
+++ /dev/null
@@ -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
- branch master updated (7fcc2f9 -> cc98b00), guix-commits, 2019/09/08
- 02/07: daemon: Run 'guix authenticate' directly., guix-commits, 2019/09/08
- 03/07: daemon: Run 'guix perform-download' directly., guix-commits, 2019/09/08
- 06/07: daemon: Remove 'NIX_LIBEXEC_DIR'., guix-commits, 2019/09/08
- 07/07: etc: Remove references to libexec/guix* from SELinux policy., guix-commits, 2019/09/08
- 04/07: daemon: Run 'guix offload' directly., guix-commits, 2019/09/08
- 05/07: daemon: Run 'guix substitute' directly and assume a single substituter.,
guix-commits <=
- 01/07: daemon: Invoke 'guix gc --list-busy' instead of 'list-runtime-roots'., guix-commits, 2019/09/08