Report substituter errors to clients of the Nix daemon

scripts/download-from-binary-cache.pl.in

@@ -184,13 +184,9 @@ sub getAvailableCaches {
         my @trustedUrls = (@urls, strToList($Nix::Config::config{"trusted-binary-caches"} // ""));
         @urls = ();
         foreach my $url (@untrustedUrls) {
-            if (any { $url eq $_ } @trustedUrls) {
+            die "binary cache ‘$url’ is not trusted (please add it to ‘trusted-binary-caches’ in $Nix::Config::confDir/nix.conf)\n"
+                unless any { $url eq $_ } @trustedUrls;
             push @urls, $url;
-            } else {
-                # FIXME: should die here, but we currently can't
-                # deliver error messages to clients.
-                warn "warning: binary cache ‘$url’ is not trusted (please add it to ‘trusted-binary-caches’ in $Nix::Config::confDir/nix.conf)\n";
-            }
         }
     }
 

src/libstore/local-store.cc

@@ -909,10 +909,11 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
 
     debug(format("starting substituter program `%1%'") % substituter);
 
-    Pipe toPipe, fromPipe;
+    Pipe toPipe, fromPipe, errorPipe;
 
     toPipe.create();
     fromPipe.create();
+    errorPipe.create();
 
     run.pid = fork();
 
@@ -940,6 +941,8 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
                 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");
             closeMostFDs(set<int>());
             execl(substituter.c_str(), substituter.c_str(), "--query", NULL);
             throw SysError(format("executing `%1%'") % substituter);
@@ -953,6 +956,7 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
 
     run.to = toPipe.writeSide.borrow();
     run.from = fromPipe.readSide.borrow();
+    run.error = errorPipe.readSide.borrow();
 }
 
 
@@ -973,13 +977,21 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet & paths)
         RunningSubstituter & run(runningSubstituters[*i]);
         startSubstituter(*i, run);
         string s = "have ";
-        foreach (PathSet::const_iterator, i, paths)
-            if (res.find(*i) == res.end()) { s += *i; s += " "; }
+        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. */
+            try {
                 Path path = readLine(run.from);
                 if (path == "") break;
                 res.insert(path);
+            } catch (EndOfFile e) {
+                throw Error(format("substituter `%1%' failed: %2%") % *i % chomp(drainFD(run.error)));
+            }
         }
     }
     return res;
@@ -998,6 +1010,7 @@ void LocalStore::querySubstitutablePathInfos(const Path & substituter,
     writeLine(run.to, s);
 
     while (true) {
+        try {
             Path path = readLine(run.from);
             if (path == "") break;
             if (paths.find(path) == paths.end())
@@ -1014,6 +1027,9 @@ void LocalStore::querySubstitutablePathInfos(const Path & substituter,
             }
             info.downloadSize = getIntLine<long long>(run.from);
             info.narSize = getIntLine<long long>(run.from);
+        } catch (EndOfFile e) {
+            throw Error(format("substituter `%1%' failed: %2%") % substituter % chomp(drainFD(run.error)));
+        }
     }
 }
 

src/libstore/local-store.hh

@@ -45,7 +45,7 @@ struct OptimiseStats
 struct RunningSubstituter
 {
     Pid pid;
-    AutoCloseFD to, from;
+    AutoCloseFD to, from, error;
 };
 
 

src/libutil/util.cc

@@ -253,7 +253,7 @@ string readLine(int fd)
             if (errno != EINTR)
                 throw SysError("reading a line");
         } else if (rd == 0)
-            throw Error("unexpected EOF reading a line");
+            throw EndOfFile("unexpected EOF reading a line");
         else {
             if (ch == '\n') return s;
             s += ch;
@@ -1015,6 +1015,13 @@ string concatStringsSep(const string & sep, const Strings & ss)
 }
 
 
+string chomp(const string & s)
+{
+    size_t i = s.find_last_not_of(" \n\r\t");
+    return i == string::npos ? "" : string(s, 0, i);
+}
+
+
 string statusToString(int status)
 {
     if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) {

src/libutil/util.hh

@@ -292,6 +292,10 @@ Strings tokenizeString(const string & s, const string & separators = " \t\n\r");
 string concatStringsSep(const string & sep, const Strings & ss);
 
 
+/* Remove trailing whitespace from a string. */
+string chomp(const string & s);
+
+
 /* Convert the exit status of a child as returned by wait() into an
    error string. */
 string statusToString(int status);