daemon: Use 'Agent' to spawn 'guix substitute --query'.

* nix/libstore/local-store.hh (RunningSubstituter): Remove.
(LocalStore)[runningSubstituter]: Change to unique_ptr<Agent>.
[setSubstituterEnv, didSetSubstituterEnv]: Remove.
[getLineFromSubstituter, getIntLineFromSubstituter]: Take an 'Agent'.
* nix/libstore/local-store.cc (LocalStore::~LocalStore): Remove
reference to 'runningSubstituter'.
(LocalStore::setSubstituterEnv, LocalStore::startSubstituter): Remove.
(LocalStore::getLineFromSubstituter): Adjust to 'run' being an 'Agent'.
(LocalStore::querySubstitutablePaths): Spawn substituter agent if
needed.  Adjust to 'Agent' interface.
(LocalStore::querySubstitutablePathInfos): Likewise.
* nix/libstore/build.cc (SubstitutionGoal::tryToRun): Remove call to
'setSubstituterEnv' and add 'setenv' call for "_NIX_OPTIONS" instead.
(SubstitutionGoal::finished): Remove 'readLine' call for 'dummy'.
* guix/scripts/substitute.scm (%allow-unauthenticated-substitutes?):
Remove second argument to 'make-parameter'.
(process-query): Call 'warn-about-missing-authentication'
when (%allow-unauthenticated-substitutes?) is #t.
(guix-substitute): Wrap body in 'parameterize'.  Set 'guix-warning-port'
too.  No longer exit when 'substitute-urls' returns the empty list.  No
longer print newline initially.
* tests/substitute.scm (test-quit): Parameterize 'current-error-port' to
account for the port changes in 'guix-substitute'.

3 files changed, 43 insertions, 144 deletions

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index 8413819114..7e9ab3f39c 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -2986,8 +2986,6 @@ void SubstitutionGoal::tryToRun()
     if (pathExists(destPath))
         deletePath(destPath);
 
-    worker.store.setSubstituterEnv();
-
     /* Fill in the arguments. */
     Strings args;
     args.push_back("guix");
@@ -2999,6 +2997,9 @@ void SubstitutionGoal::tryToRun()
     /* Fork the substitute program. */
     pid = startProcess([&]() {
 
+	/* Communicate substitute-urls & co. to 'guix substitute'.  */
+        setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
+
         commonChildInit(logPipe);
 
         if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
@@ -3041,7 +3042,6 @@ void SubstitutionGoal::finished()
     logPipe.readSide.close();
 
     /* Get the hash info from stdout. */
-    string dummy = readLine(outPipe.readSide);
     string expectedHashStr = statusOk(status) ? readLine(outPipe.readSide) : "";
     outPipe.readSide.close();
 
diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc
index 8c479002ec..4219573a56 100644
--- a/nix/libstore/local-store.cc
+++ b/nix/libstore/local-store.cc
@@ -57,7 +57,6 @@ void checkStoreNotSymlink()
 
 
 LocalStore::LocalStore(bool reserveSpace)
-    : didSetSubstituterEnv(false)
 {
     schemaPath = settings.nixDBPath + "/schema";
 
@@ -183,21 +182,6 @@ LocalStore::LocalStore(bool reserveSpace)
 LocalStore::~LocalStore()
 {
     try {
-	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();
-    }
-
-    try {
         if (fdTempRoots != -1) {
             fdTempRoots.close();
             unlink(fnTempRoots.c_str());
@@ -796,96 +780,31 @@ Path LocalStore::queryPathFromHashPart(const string & hashPart)
     });
 }
 
-
-void LocalStore::setSubstituterEnv()
-{
-    if (didSetSubstituterEnv) return;
-
-    /* Pass configuration options (including those overridden with
-       --option) to substituters. */
-    setenv("_NIX_OPTIONS", settings.pack().c_str(), 1);
-
-    didSetSubstituterEnv = true;
-}
-
-
-void LocalStore::startSubstituter(RunningSubstituter & run)
-{
-    if (run.disabled || run.pid != -1) return;
-
-    debug(format("starting substituter program `%1% substitute'")
-	  % settings.guixProgram);
-
-    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(settings.guixProgram.c_str(), "guix", "substitute", "--query", NULL);
-        throw SysError(format("executing `%1%'") % settings.guixProgram);
-    });
-
-    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 LocalStore::getLineFromSubstituter(Agent & 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);
+        FD_SET(run.fromAgent.readSide, &fds);
+        FD_SET(run.builderOut.readSide, &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 (select(std::max(run.fromAgent.readSide, run.builderOut.readSide) + 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)) {
+        if (FD_ISSET(run.builderOut.readSide, &fds)) {
             char buf[4096];
-            ssize_t n = read(run.error, (unsigned char *) buf, sizeof(buf));
+            ssize_t n = read(run.builderOut.readSide, (unsigned char *) buf, sizeof(buf));
             if (n == -1) {
                 if (errno == EINTR) continue;
                 throw SysError("reading from substituter's stderr");
@@ -903,23 +822,20 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
         }
 
         /* 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, "substitute: " + err);
-                    return res;
-                }
-                res += c;
-            } while (run.fromBuf.hasData());
+        else if (FD_ISSET(run.fromAgent.readSide, &fds)) {
+	    unsigned char c;
+	    readFull(run.fromAgent.readSide, (unsigned char *) &c, 1);
+	    if (c == '\n') {
+		if (!err.empty()) printMsg(lvlError, "substitute: " + err);
+		return res;
+	    }
+	    res += c;
         }
     }
 }
 
 
-template<class T> T LocalStore::getIntLineFromSubstituter(RunningSubstituter & run)
+template<class T> T LocalStore::getIntLineFromSubstituter(Agent & run)
 {
     string s = getLineFromSubstituter(run);
     T res;
@@ -935,27 +851,26 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
     if (!settings.useSubstitutes || paths.empty()) return res;
 
     if (!runningSubstituter) {
-	std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+	const Strings args = { "substitute", "--query" };
+	const std::map<string, string> env = { { "_NIX_OPTIONS", settings.pack() } };
+	std::unique_ptr<Agent> fresh(new Agent(settings.guixProgram, args, env));
 	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);
-	}
+    Agent & run = *runningSubstituter;
+
+    string s = "have ";
+    foreach (PathSet::const_iterator, j, paths)
+	if (res.find(*j) == res.end()) { s += *j; s += " "; }
+    writeLine(run.toAgent.writeSide, 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;
@@ -967,18 +882,18 @@ void LocalStore::querySubstitutablePathInfos(PathSet & paths, SubstitutablePathI
     if (!settings.useSubstitutes) return;
 
     if (!runningSubstituter) {
-	std::unique_ptr<RunningSubstituter>fresh(new RunningSubstituter);
+	const Strings args = { "substitute", "--query" };
+	const std::map<string, string> env = { { "_NIX_OPTIONS", settings.pack() } };
+	std::unique_ptr<Agent> fresh(new Agent(settings.guixProgram, args, env));
 	runningSubstituter.swap(fresh);
     }
 
-    RunningSubstituter & run = *runningSubstituter;
-    startSubstituter(run);
-    if (run.disabled) return;
+    Agent & run = *runningSubstituter;
 
     string s = "info ";
     foreach (PathSet::const_iterator, i, paths)
         if (infos.find(*i) == infos.end()) { s += *i; s += " "; }
-    writeLine(run.to, s);
+    writeLine(run.toAgent.writeSide, s);
 
     while (true) {
         Path path = getLineFromSubstituter(run);
diff --git a/nix/libstore/local-store.hh b/nix/libstore/local-store.hh
index 2e48cf03e6..57d15bac7e 100644
--- a/nix/libstore/local-store.hh
+++ b/nix/libstore/local-store.hh
@@ -38,21 +38,11 @@ struct OptimiseStats
 };
 
 
-struct RunningSubstituter
-{
-    Pid pid;
-    AutoCloseFD to, from, error;
-    FdSource fromBuf;
-    bool disabled;
-    RunningSubstituter() : disabled(false) { };
-};
-
-
 class LocalStore : public StoreAPI
 {
 private:
     /* The currently running substituter or empty.  */
-    std::unique_ptr<RunningSubstituter> runningSubstituter;
+    std::unique_ptr<Agent> runningSubstituter;
 
     Path linksDir;
 
@@ -178,8 +168,6 @@ public:
 
     void markContentsGood(const Path & path);
 
-    void setSubstituterEnv();
-
     void createUser(const std::string & userName, uid_t userId);
 
 private:
@@ -213,8 +201,6 @@ private:
     /* Cache for pathContentsGood(). */
     std::map<Path, bool> pathContentsGoodCache;
 
-    bool didSetSubstituterEnv;
-
     /* The file to which we write our temporary roots. */
     Path fnTempRoots;
     AutoCloseFD fdTempRoots;
@@ -262,11 +248,9 @@ private:
 
     void removeUnusedLinks(const GCState & state);
 
-    void startSubstituter(RunningSubstituter & runningSubstituter);
-
-    string getLineFromSubstituter(RunningSubstituter & run);
+    string getLineFromSubstituter(Agent & run);
 
-    template<class T> T getIntLineFromSubstituter(RunningSubstituter & run);
+    template<class T> T getIntLineFromSubstituter(Agent & run);
 
     Path createTempDirInStore();