354/376: Ensure we're writing to stderr in the builder

[Ludovic Courtès]

civodul pushed a commit to tag 1.8
in repository guix.

commit 28f22b4653bfcf1be41b042c8068b6513dd3e931
Author: Eelco Dolstra <address@hidden>
Date:   Fri Dec 12 14:35:44 2014 +0100

    Ensure we're writing to stderr in the builder
    
    http://hydra.nixos.org/build/17862041
---
 src/libstore/build.cc      |    6 +++---
 src/libstore/gc.cc         |    4 ++--
 src/libstore/pathlocks.cc  |    2 +-
 src/libutil/util.cc        |   25 +++++++++++++------------
 src/libutil/util.hh        |    1 +
 src/nix-store/nix-store.cc |    2 +-
 6 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/libstore/build.cc b/src/libstore/build.cc
index d759d15..1ae2427 100644
--- a/src/libstore/build.cc
+++ b/src/libstore/build.cc
@@ -2166,7 +2166,7 @@ void DerivationGoal::runChild()
         restoreSIGPIPE();
 
         /* Indicate that we managed to set up the build environment. */
-        writeToStderr("\n");
+        writeFull(STDERR_FILENO, "\n");
 
         /* Execute the program.  This should not return. */
         execve(program.c_str(), (char * *) &args[0], (char * *) envArr);
@@ -2174,7 +2174,7 @@ void DerivationGoal::runChild()
         throw SysError(format("executing ‘%1%’") % drv.builder);
 
     } catch (std::exception & e) {
-        writeToStderr("while setting up the build environment: " + 
string(e.what()) + "\n");
+        writeFull(STDERR_FILENO, "while setting up the build environment: " + 
string(e.what()) + "\n");
         _exit(1);
     }
 }
@@ -2487,7 +2487,7 @@ void DerivationGoal::handleChildOutput(int fd, const 
string & data)
             BZ2_bzWrite(&err, bzLogFile, (unsigned char *) data.data(), 
data.size());
             if (err != BZ_OK) throw Error(format("cannot write to compressed 
log file (BZip2 error = %1%)") % err);
         } else if (fdLogFile != -1)
-            writeFull(fdLogFile, (unsigned char *) data.data(), data.size());
+            writeFull(fdLogFile, data);
     }
 
     if (hook && fd == hook->fromHook.readSide)
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 49cb11d..7959a59 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -191,7 +191,7 @@ void LocalStore::addTempRoot(const Path & path)
     lockFile(fdTempRoots, ltWrite, true);
 
     string s = path + '\0';
-    writeFull(fdTempRoots, (const unsigned char *) s.data(), s.size());
+    writeFull(fdTempRoots, s);
 
     /* Downgrade to a read lock. */
     debug(format("downgrading to read lock on ‘%1%’") % fnTempRoots);
@@ -231,7 +231,7 @@ static void readTempRoots(PathSet & tempRoots, FDs & fds)
         if (lockFile(*fd, ltWrite, false)) {
             printMsg(lvlError, format("removing stale temporary roots file 
‘%1%’") % path);
             unlink(path.c_str());
-            writeFull(*fd, (const unsigned char *) "d", 1);
+            writeFull(*fd, "d");
             continue;
         }
 
diff --git a/src/libstore/pathlocks.cc b/src/libstore/pathlocks.cc
index f26684a..9db37e8 100644
--- a/src/libstore/pathlocks.cc
+++ b/src/libstore/pathlocks.cc
@@ -33,7 +33,7 @@ void deleteLockFile(const Path & path, int fd)
        other processes waiting on this lock that the lock is stale
        (deleted). */
     unlink(path.c_str());
-    writeFull(fd, (const unsigned char *) "d", 1);
+    writeFull(fd, "d");
     /* Note that the result of unlink() is ignored; removing the lock
        file is an optimisation, not a necessity. */
 }
diff --git a/src/libutil/util.cc b/src/libutil/util.cc
index a91cf26..1f71f76 100644
--- a/src/libutil/util.cc
+++ b/src/libutil/util.cc
@@ -265,7 +265,7 @@ void writeFile(const Path & path, const string & s)
     AutoCloseFD fd = open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT, 0666);
     if (fd == -1)
         throw SysError(format("opening file ‘%1%’") % path);
-    writeFull(fd, (unsigned char *) s.data(), s.size());
+    writeFull(fd, s);
 }
 
 
@@ -292,7 +292,7 @@ string readLine(int fd)
 void writeLine(int fd, string s)
 {
     s += '\n';
-    writeFull(fd, (const unsigned char *) s.data(), s.size());
+    writeFull(fd, s);
 }
 
 
@@ -483,18 +483,13 @@ void warnOnce(bool & haveWarned, const FormatOrString & 
fs)
 }
 
 
-static void defaultWriteToStderr(const unsigned char * buf, size_t count)
-{
-    writeFull(STDERR_FILENO, buf, count);
-}
-
-
 void writeToStderr(const string & s)
 {
     try {
-        auto p = _writeToStderr;
-        if (!p) p = defaultWriteToStderr;
-        p((const unsigned char *) s.data(), s.size());
+        if (_writeToStderr)
+            _writeToStderr((const unsigned char *) s.data(), s.size());
+        else
+            writeFull(STDERR_FILENO, s);
     } catch (SysError & e) {
         /* Ignore failing writes to stderr if we're in an exception
            handler, otherwise throw an exception.  We need to ignore
@@ -506,7 +501,7 @@ void writeToStderr(const string & s)
 }
 
 
-void (*_writeToStderr) (const unsigned char * buf, size_t count) = 
defaultWriteToStderr;
+void (*_writeToStderr) (const unsigned char * buf, size_t count) = 0;
 
 
 void readFull(int fd, unsigned char * buf, size_t count)
@@ -540,6 +535,12 @@ void writeFull(int fd, const unsigned char * buf, size_t 
count)
 }
 
 
+void writeFull(int fd, const string & s)
+{
+    writeFull(fd, (const unsigned char *) s.data(), s.size());
+}
+
+
 string drainFD(int fd)
 {
     string result;
diff --git a/src/libutil/util.hh b/src/libutil/util.hh
index 09eb4d6..352b83e 100644
--- a/src/libutil/util.hh
+++ b/src/libutil/util.hh
@@ -171,6 +171,7 @@ extern void (*_writeToStderr) (const unsigned char * buf, 
size_t count);
    requested number of bytes. */
 void readFull(int fd, unsigned char * buf, size_t count);
 void writeFull(int fd, const unsigned char * buf, size_t count);
+void writeFull(int fd, const string & s);
 
 MakeError(EndOfFile, Error)
 
diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
index c91bca9..87bc8c3 100644
--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@@ -488,7 +488,7 @@ static void opReadLog(Strings opFlags, Strings opArgs)
             if (pathExists(logPath)) {
                 /* !!! Make this run in O(1) memory. */
                 string log = readFile(logPath);
-                writeFull(STDOUT_FILENO, (const unsigned char *) log.data(), 
log.size());
+                writeFull(STDOUT_FILENO, log);
                 found = true;
                 break;
             }