From 5959c591a0a6000b6de14eaec37e8139e36dfe0a Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <eelco.dolstra@logicblox.com>
Date: Fri, 7 Jun 2013 15:02:14 +0200
Subject: [PATCH] Process stderr from substituters while doing have/info
 queries

---
 src/libstore/local-store.cc | 59 +++++++++++++++++++++++++++++++------
 src/libstore/local-store.hh |  1 +
 src/libutil/serialise.cc    |  6 ++++
 src/libutil/serialise.hh    |  2 ++
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index cfb8532052..056247b7e9 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -1007,10 +1007,6 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
     fromPipe.create();
     errorPipe.create();
 
-    /* Hack: prevent substituters that write too much to stderr from
-       deadlocking our read() from stdout. */
-    fcntl(errorPipe.writeSide, F_SETFL, O_NONBLOCK);
-
     setSubstituterEnv();
 
     run.pid = maybeVfork();
@@ -1038,20 +1034,65 @@ void LocalStore::startSubstituter(const Path & substituter, RunningSubstituter &
 
     /* Parent. */
 
+    run.program = baseNameOf(substituter);
     run.to = toPipe.writeSide.borrow();
     run.from = run.fromBuf.fd = fromPipe.readSide.borrow();
     run.error = errorPipe.readSide.borrow();
 }
 
 
+/* Read a line from the substituter's stdout, while also processing
+   its stderr. */
 string LocalStore::getLineFromSubstituter(RunningSubstituter & run)
 {
-    string s;
+    string res, err;
+
+    /* We might have stdout data left over from the last time. */
+    if (run.fromBuf.hasData()) goto haveData;
+
     while (1) {
-        unsigned char c;
-        run.fromBuf(&c, 1);
-        if (c == '\n') return s;
-        s += c;
+        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 Error(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());
+        }
     }
 }
 
diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh
index 4245156670..b1fde99cf9 100644
--- a/src/libstore/local-store.hh
+++ b/src/libstore/local-store.hh
@@ -44,6 +44,7 @@ struct OptimiseStats
 
 struct RunningSubstituter
 {
+    Path program;
     Pid pid;
     AutoCloseFD to, from, error;
     FdSource fromBuf;
diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc
index 9270806a77..1f817a8695 100644
--- a/src/libutil/serialise.cc
+++ b/src/libutil/serialise.cc
@@ -90,6 +90,12 @@ size_t BufferedSource::read(unsigned char * data, size_t len)
 }
 
 
+bool BufferedSource::hasData()
+{
+    return bufPosOut < bufPosIn;
+}
+
+
 size_t FdSource::readUnbuffered(unsigned char * data, size_t len)
 {
     ssize_t n;
diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index 42dd271176..e1c4514dbb 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -63,6 +63,8 @@ struct BufferedSource : Source
     
     /* Underlying read call, to be overriden. */
     virtual size_t readUnbuffered(unsigned char * data, size_t len) = 0;
+
+    bool hasData();
 };