From e84c26564507388f2e8b7c7f2b00eb2b57856da1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 17 May 2019 22:29:15 +0200 Subject: [PATCH 1/4] Run builds in a pseudo-terminal This allows many programs (e.g. gcc, clang, cmake) to print colorized log output (assuming $TERM is set to a value like "xterm"). There are other ways to get colors, in particular setting CLICOLOR_FORCE, but they're less widely supported and can break programs that parse tool output. --- src/libstore/build.cc | 54 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0bd738809..9de7613f8 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -38,6 +38,7 @@ #include #include #include +#include #include #include @@ -1558,8 +1559,8 @@ void DerivationGoal::buildDone() if (hook) { hook->builderOut.readSide = -1; hook->fromHook.readSide = -1; - } - else builderOut.readSide = -1; + } else + builderOut.readSide = -1; /* Close the log file. */ closeLogFile(); @@ -2181,7 +2182,43 @@ void DerivationGoal::startBuilder() Path logFile = openLogFile(); /* Create a pipe to get the output of the builder. */ - builderOut.create(); + //builderOut.create(); + + builderOut.readSide = posix_openpt(O_RDWR | O_NOCTTY); + if (!builderOut.readSide) + throw SysError("opening pseudoterminal master"); + + std::string slaveName(ptsname(builderOut.readSide.get())); + + if (chmod(slaveName.c_str(), 0600)) + throw SysError("changing mode of pseudoterminal slave"); + + if (buildUser && chown(slaveName.c_str(), buildUser->getUID(), 0)) + throw SysError("changing owner of pseudoterminal slave"); + + #if 0 + // Mount the pt in the sandbox so that the "tty" command works. + // FIXME: this doesn't work with the new devpts in the sandbox. + if (useChroot) + dirsInChroot[slaveName] = {slaveName, false}; + #endif + + if (unlockpt(builderOut.readSide.get())) + throw SysError("unlocking pseudoterminal"); + + builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY); + if (!builderOut.writeSide) + throw SysError("opening pseudoterminal slave"); + + // Put the pt into raw mode to prevent \n -> \r\n translation. + struct termios term; + if (tcgetattr(builderOut.writeSide.get(), &term)) + throw SysError("getting pseudoterminal attributes"); + + cfmakeraw(&term); + + if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) + throw SysError("putting pseudoterminal into raw mode"); result.startTime = time(0); @@ -4361,14 +4398,15 @@ void Worker::waitForInput() for (auto & k : fds2) { if (FD_ISSET(k, &fds)) { ssize_t rd = read(k, buffer.data(), buffer.size()); - if (rd == -1) { - if (errno != EINTR) - throw SysError(format("reading from %1%") - % goal->getName()); - } else if (rd == 0) { + // FIXME: is there a cleaner way to handle pt close + // than EIO? Is this even standard? + if (rd == 0 || (rd == -1 && errno == EIO)) { debug(format("%1%: got EOF") % goal->getName()); goal->handleEOF(k); j->fds.erase(k); + } else if (rd == -1) { + if (errno != EINTR) + throw SysError("%s: read failed", goal->getName()); } else { printMsg(lvlVomit, format("%1%: read %2% bytes") % goal->getName() % rd); From 82ca6ef390b752faf1bf27b22daa34b29a4b00d4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 19 May 2019 16:56:08 +0200 Subject: [PATCH 2/4] Set $TERM --- src/libstore/build.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 9de7613f8..5a75403cf 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2443,6 +2443,9 @@ void DerivationGoal::initEnv() may change that in the future. So tell the builder which file descriptor to use for that. */ env["NIX_LOG_FD"] = "2"; + + /* Trigger colored output in various tools. */ + env["TERM"] = "xterm-256color"; } From 2743bf0bb1727056ce6d6d6be4e38c3251aac1f8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Sun, 16 Jun 2019 20:02:40 +0200 Subject: [PATCH 3/4] Hopefully fix macOS tests --- src/libstore/build.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 5a75403cf..cccf0c7c6 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2190,11 +2190,13 @@ void DerivationGoal::startBuilder() std::string slaveName(ptsname(builderOut.readSide.get())); - if (chmod(slaveName.c_str(), 0600)) - throw SysError("changing mode of pseudoterminal slave"); + if (buildUser) { + if (chmod(slaveName.c_str(), 0600)) + throw SysError("changing mode of pseudoterminal slave"); - if (buildUser && chown(slaveName.c_str(), buildUser->getUID(), 0)) - throw SysError("changing owner of pseudoterminal slave"); + if (chown(slaveName.c_str(), buildUser->getUID(), 0)) + throw SysError("changing owner of pseudoterminal slave"); + } #if 0 // Mount the pt in the sandbox so that the "tty" command works. From 3cc1125595d97b4ab7369e37e4ad22f4cfecb8b2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jun 2019 08:08:04 +0200 Subject: [PATCH 4/4] Another attempt at getting pseudoterminals to work on macOS --- src/libstore/build.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index cccf0c7c6..5b38bcf3c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2196,6 +2196,9 @@ void DerivationGoal::startBuilder() if (chown(slaveName.c_str(), buildUser->getUID(), 0)) throw SysError("changing owner of pseudoterminal slave"); + } else { + if (grantpt(builderOut.readSide.get())) + throw SysError("granting access to pseudoterminal slave"); } #if 0