From 80e722278ca03bf303961e2f27487dc98d042803 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 30 Aug 2010 14:53:03 +0000 Subject: [PATCH] * When using the build hook, distinguish between the stderr of the hook script proper, and the stdout/stderr of the builder. Only the latter should be saved in /nix/var/log/nix/drvs. * Allow the verbosity to be set through an option. * Added a flag --quiet to lower the verbosity level. --- scripts/build-remote.pl.in | 4 ++-- scripts/nix-build.in | 7 ++++++- src/libmain/shared.cc | 10 +++++---- src/libstore/build.cc | 40 ++++++++++++++++++++++++++---------- src/libstore/gc.cc | 2 +- src/libstore/globals.cc | 2 +- src/libstore/remote-store.cc | 9 +------- src/libutil/types.hh | 2 +- 8 files changed, 47 insertions(+), 29 deletions(-) diff --git a/scripts/build-remote.pl.in b/scripts/build-remote.pl.in index 0c8081a0b..8e3da2553 100755 --- a/scripts/build-remote.pl.in +++ b/scripts/build-remote.pl.in @@ -225,14 +225,14 @@ system("NIX_SSHOPTS=\"@sshOpts\" @bindir@/nix-copy-closure $hostName $maybeSign # Perform the build. -my $buildFlags = "--max-silent-time $maxSilentTime --fallback --add-root $rootsDir/\$PPID.out"; +my $buildFlags = "--max-silent-time $maxSilentTime --fallback --add-root $rootsDir/\$PPID.out --option verbosity 0"; # `-tt' forces allocation of a pseudo-terminal. This is required to # make the remote nix-store process receive a signal when the # connection dies. Without it, the remote process might continue to # run indefinitely (that is, until it next tries to write to # stdout/stderr). -if (system("ssh $hostName @sshOpts -tt 'nix-store -r $drvPath $buildFlags > /dev/null'") != 0) { +if (system("ssh $hostName @sshOpts -tt 'nix-store -r $drvPath $buildFlags > /dev/null' >&4") != 0) { # If we couldn't run ssh or there was an ssh problem (indicated by # exit code 255), then we return exit code 1; otherwise we assume # that the builder failed, which we indicate to Nix using exit diff --git a/scripts/nix-build.in b/scripts/nix-build.in index ed85d5712..f9d81b36c 100644 --- a/scripts/nix-build.in +++ b/scripts/nix-build.in @@ -123,6 +123,11 @@ EOF $verbose = 1; } + elsif ($arg eq "--quiet") { + push @buildArgs, $arg; + push @instArgs, $arg; + } + elsif (substr($arg, 0, 1) eq "-") { push @buildArgs, $arg; } @@ -165,7 +170,7 @@ foreach my $expr (@exprs) { # Build. my @outPaths; - $pid = open(OUTPATHS, "-|") || exec "$binDir/nix-store", "--add-root", $outLink, "--indirect", "-rv", + $pid = open(OUTPATHS, "-|") || exec "$binDir/nix-store", "--add-root", $outLink, "--indirect", "-r", @buildArgs, @drvPaths; while () {chomp; push @outPaths, $_;} if (!close OUTPATHS) { diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index eddc4e64b..f58def403 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -196,17 +196,16 @@ static void initAndRun(int argc, char * * argv) remaining.clear(); /* Process default options. */ + int verbosityDelta = 0; for (Strings::iterator i = args.begin(); i != args.end(); ++i) { string arg = *i; - if (arg == "--verbose" || arg == "-v") - verbosity = (Verbosity) ((int) verbosity + 1); + if (arg == "--verbose" || arg == "-v") verbosityDelta++; + else if (arg == "--quiet") verbosityDelta--; else if (arg == "--log-type") { ++i; if (i == args.end()) throw UsageError("`--log-type' requires an argument"); setLogType(*i); } - else if (arg == "--build-output" || arg == "-B") - ; /* !!! obsolete - remove eventually */ else if (arg == "--no-build-output" || arg == "-Q") buildVerbosity = lvlVomit; else if (arg == "--print-build-trace") @@ -247,6 +246,9 @@ static void initAndRun(int argc, char * * argv) else remaining.push_back(arg); } + verbosityDelta += queryIntSetting("verbosity", lvlInfo); + verbosity = (Verbosity) (verbosityDelta < 0 ? 0 : verbosityDelta); + /* Automatically clean up the temporary roots file when we exit. */ RemoveTempRoots removeTempRoots __attribute__((unused)); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f9c9a0a1e..30e37e4e9 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -626,6 +626,9 @@ struct HookInstance /* Pipe for the hook's standard output/error. */ Pipe fromHook; + /* Pipe for the builder's standard output/error. */ + Pipe builderOut; + /* The process ID of the hook. */ Pid pid; @@ -647,6 +650,9 @@ HookInstance::HookInstance() /* Create the communication pipes. */ toHook.create(); + /* Create a pipe to get the output of the builder. */ + builderOut.create(); + /* Fork the hook. */ pid = fork(); switch (pid) { @@ -666,6 +672,11 @@ HookInstance::HookInstance() if (dup2(toHook.readSide, STDIN_FILENO) == -1) throw SysError("dupping to-hook read side"); + /* Use fd 4 for the builder's stdout/stderr. */ + builderOut.readSide.close(); + if (dup2(builderOut.writeSide, 4) == -1) + throw SysError("dupping builder's stdout/stderr"); + execl(buildHook.c_str(), buildHook.c_str(), thisSystem.c_str(), (format("%1%") % maxSilentTime).str().c_str(), NULL); @@ -740,7 +751,7 @@ private: AutoCloseFD fdLogFile; /* Pipe for the builder's standard output/error. */ - Pipe logPipe; + Pipe builderOut; /* The build hook. */ boost::shared_ptr hook; @@ -1208,7 +1219,11 @@ void DerivationGoal::buildDone() worker.childTerminated(savedPid); /* Close the read side of the logger pipe. */ - logPipe.readSide.close(); + if (hook) { + hook->builderOut.readSide.close(); + hook->fromHook.readSide.close(); + } + else builderOut.readSide.close(); /* Close the log file. */ fdLogFile.close(); @@ -1387,9 +1402,11 @@ HookReply DerivationGoal::tryBuildHook() /* Create the log file and pipe. */ Path logFile = openLogFile(); - - worker.childStarted(shared_from_this(), - hook->pid, singleton >(hook->fromHook.readSide), false, false); + + set fds; + fds.insert(hook->fromHook.readSide); + fds.insert(hook->builderOut.readSide); + worker.childStarted(shared_from_this(), hook->pid, fds, false, false); if (printBuildTrace) printMsg(lvlError, format("@ build-started %1% %2% %3% %4%") @@ -1679,8 +1696,8 @@ void DerivationGoal::startBuilder() /* Create the log file. */ Path logFile = openLogFile(); - /* Create a pipe to get the output of the child. */ - logPipe.create(); + /* Create a pipe to get the output of the builder. */ + builderOut.create(); /* Fork a child to build the package. Note that while we currently use forks to run and wait for the children, it @@ -1732,7 +1749,7 @@ void DerivationGoal::startBuilder() } #endif - commonChildInit(logPipe); + commonChildInit(builderOut); if (chdir(tmpDir.c_str()) == -1) throw SysError(format("changing into `%1%'") % tmpDir); @@ -1816,9 +1833,9 @@ void DerivationGoal::startBuilder() /* parent */ pid.setSeparatePG(true); - logPipe.writeSide.close(); + builderOut.writeSide.close(); worker.childStarted(shared_from_this(), pid, - singleton >(logPipe.readSide), true, true); + singleton >(builderOut.readSide), true, true); if (printBuildTrace) { printMsg(lvlError, format("@ build-started %1% %2% %3% %4%") @@ -2008,7 +2025,8 @@ void DerivationGoal::handleChildOutput(int fd, const string & data) { if (verbosity >= buildVerbosity) writeToStderr((unsigned char *) data.c_str(), data.size()); - if (fdLogFile != -1) + if ((hook && fd == hook->builderOut.readSide) || + (!hook && fd == builderOut.readSide)) writeFull(fdLogFile, (unsigned char *) data.c_str(), data.size()); } diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index b94442923..ea784bcd9 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -136,7 +136,7 @@ Path addPermRoot(const Path & _storePath, const Path & _gcRoot, "therefore, `%2%' might be removed by the garbage collector") % gcRoot % storePath); } - + /* Grab the global GC root, causing us to block while a GC is in progress. This prevents the set of permanent roots from increasing while a GC is in progress. */ diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 75d2f69c2..7069d104a 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -20,7 +20,7 @@ string nixBinDir = "/UNINIT"; bool keepFailed = false; bool keepGoing = false; bool tryFallback = false; -Verbosity buildVerbosity = lvlInfo; +Verbosity buildVerbosity = lvlError; unsigned int maxBuildJobs = 1; unsigned int buildCores = 1; bool readOnlyMode = false; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8f162daee..92d517bbb 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -97,10 +97,6 @@ void RemoteStore::forkSlave() if (worker == "") worker = nixBinDir + "/nix-worker"; - string verbosityArg = "-"; - for (int i = 1; i < verbosity; ++i) - verbosityArg += "v"; - child = fork(); switch (child) { @@ -120,10 +116,7 @@ void RemoteStore::forkSlave() close(fdSocket); close(fdChild); - execlp(worker.c_str(), worker.c_str(), "--slave", - /* hacky - must be at the end */ - verbosityArg == "-" ? NULL : verbosityArg.c_str(), - NULL); + execlp(worker.c_str(), worker.c_str(), "--slave", NULL); throw SysError(format("executing `%1%'") % worker); diff --git a/src/libutil/types.hh b/src/libutil/types.hh index f110188da..86801342f 100644 --- a/src/libutil/types.hh +++ b/src/libutil/types.hh @@ -63,7 +63,7 @@ typedef set PathSet; typedef enum { - lvlError, + lvlError = 0, lvlInfo, lvlTalkative, lvlChatty,