From 322eeb87d0e5bb608ae1c176611a50297c93cbe8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ludovic=20Court=C3=A8s?= Date: Thu, 2 Jul 2015 23:37:29 +0200 Subject: [PATCH] Merge branch 'nix'. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a squashed commit of the following: commit 0dccab9f417b406f5d4aedc81900fc7b2f16c9f6 Author: Eelco Dolstra Date: Thu Jul 2 00:30:16 2015 +0200 Typo commit 2cd28517b13524c242c7758783b0b2d8250fdded Author: Ludovic Courtès Date: Wed Jul 1 14:56:34 2015 +0200 Preserve supplementary groups of build users The following patch is an attempt to address this bug (see ) by preserving the supplementary groups of build users in the build environment. In practice, I would expect that supplementary groups would contain only one or two groups: the build users group, and possibly the “kvm” group. [Changed &at(0) to data() and removed tabs - Eelco] commit 6e38685ef65284093df79ebe7378bac33b0e7e5d Author: Eelco Dolstra Date: Tue Jun 30 21:41:26 2015 +0200 GC: Handle ENOSPC creating/moving to the trash directory Issue #564. commit 5e0a9ae2e25a1016389f4893a6ed6682aadcf51d Author: Eelco Dolstra Date: Mon Jun 22 15:54:55 2015 +0200 Use posix_fallocate to create /nix/var/nix/db/reserved commit 4e5ab98d6d14f8b0e3bd1d77b2f4f2354e7a49a8 Author: Eelco Dolstra Date: Mon Jun 22 15:47:40 2015 +0200 Make /nix/var/nix/db/reserved bigger Issue #564. commit 60bda60fc06135aa97a93301b1a9e2270768f5b3 Author: Eelco Dolstra Date: Wed Jun 10 16:17:06 2015 +0200 Export outputPaths function This is useful for the new hydra-queue-runner. commit 5dfea34048aa8541f20aeb2fbcd163561b609a49 Author: Eelco Dolstra Date: Thu Jul 2 22:51:33 2015 +0200 Use std::vector::data() commit 2459458bc8257734ca78cb7a2db3df20bd730ec0 Author: Eelco Dolstra Date: Thu Jun 4 16:04:41 2015 +0200 Allow substitutes for builds that have preferLocalBuild set Not substituting builds with "preferLocalBuild = true" was a bad idea, because it didn't take the cost of dependencies into account. For instance, if we can't substitute a fetchgit call, then we have to download/build git and all its dependencies. Partially reverts 5558652709f27e8a887580b77b93c705659d7a4b and adds a new derivation attribute "allowSubstitutes" to specify whether a derivation may be substituted. --- nix/libstore/build.cc | 64 ++++++++++++++++++++---------------- nix/libstore/derivations.cc | 9 +++++ nix/libstore/derivations.hh | 1 + nix/libstore/gc.cc | 31 ++++++++++++----- nix/libstore/globals.cc | 2 +- nix/libstore/local-store.cc | 18 +++++++--- nix/libstore/misc.cc | 4 +-- nix/libstore/misc.hh | 2 ++ nix/libutil/util.cc | 11 +++---- nix/libutil/util.hh | 2 +- nix/nix-daemon/nix-daemon.cc | 5 +-- 11 files changed, 94 insertions(+), 55 deletions(-) diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index 85a818ba94..a9eedcef16 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -447,6 +447,7 @@ private: string user; uid_t uid; gid_t gid; + std::vector supplementaryGIDs; public: UserLock(); @@ -460,6 +461,7 @@ public: string getUser() { return user; } uid_t getUID() { return uid; } uid_t getGID() { return gid; } + std::vector getSupplementaryGIDs() { return supplementaryGIDs; } bool enabled() { return uid != 0; } @@ -539,6 +541,17 @@ void UserLock::acquire() throw Error(format("the Nix user should not be a member of `%1%'") % settings.buildUsersGroup); + /* Get the list of supplementary groups of this build user. This + is usually either empty or contains a group such as "kvm". */ + supplementaryGIDs.resize(10); + int ngroups = supplementaryGIDs.size(); + int err = getgrouplist(pw->pw_name, pw->pw_gid, + supplementaryGIDs.data(), &ngroups); + if (err == -1) + throw Error(format("failed to get list of supplementary groups for ‘%1%’") % pw->pw_name); + + supplementaryGIDs.resize(ngroups); + return; } } @@ -1000,7 +1013,7 @@ void DerivationGoal::haveDerivation() /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ - if (settings.useSubstitutes && !willBuildLocally(drv)) + if (settings.useSubstitutes && substitutesAllowed(drv)) foreach (PathSet::iterator, i, invalidOutputs) addWaitee(worker.makeSubstitutionGoal(*i, buildMode == bmRepair)); @@ -1188,22 +1201,6 @@ void DerivationGoal::inputsRealised() } -PathSet outputPaths(const DerivationOutputs & outputs) -{ - PathSet paths; - foreach (DerivationOutputs::const_iterator, i, outputs) - paths.insert(i->second.path); - return paths; -} - - -static string get(const StringPairs & map, const string & key) -{ - StringPairs::const_iterator i = map.find(key); - return i == map.end() ? (string) "" : i->second; -} - - static bool canBuildLocally(const string & platform) { return platform == settings.thisSystem @@ -1214,12 +1211,25 @@ static bool canBuildLocally(const string & platform) } +static string get(const StringPairs & map, const string & key, const string & def = "") +{ + StringPairs::const_iterator i = map.find(key); + return i == map.end() ? def : i->second; +} + + bool willBuildLocally(const Derivation & drv) { return get(drv.env, "preferLocalBuild") == "1" && canBuildLocally(drv.platform); } +bool substitutesAllowed(const Derivation & drv) +{ + return get(drv.env, "allowSubstitutes", "1") == "1"; +} + + void DerivationGoal::tryToBuild() { trace("trying to build"); @@ -1242,7 +1252,7 @@ void DerivationGoal::tryToBuild() can't acquire the lock, then continue; hopefully some other goal can start a build, and if not, the main loop will sleep a few seconds and then retry this goal. */ - if (!outputLocks.lockPaths(outputPaths(drv.outputs), "", false)) { + if (!outputLocks.lockPaths(outputPaths(drv), "", false)) { worker.waitForAWhile(shared_from_this()); return; } @@ -1263,7 +1273,7 @@ void DerivationGoal::tryToBuild() return; } - missingPaths = outputPaths(drv.outputs); + missingPaths = outputPaths(drv); if (buildMode != bmCheck) foreach (PathSet::iterator, i, validPaths) missingPaths.erase(*i); @@ -2168,7 +2178,6 @@ void DerivationGoal::runChild() Strings envStrs; foreach (Environment::const_iterator, i, env) envStrs.push_back(rewriteHashes(i->first + "=" + i->second, rewritesToTmp)); - auto envArr = stringsToCharPtrs(envStrs); /* If we are running in `build-users' mode, then switch to the user we allocated above. Make sure that we drop all root @@ -2177,10 +2186,11 @@ void DerivationGoal::runChild() setuid() when run as root sets the real, effective and saved UIDs. */ if (buildUser.enabled()) { - printMsg(lvlChatty, format("switching to user `%1%'") % buildUser.getUser()); - - if (setgroups(0, 0) == -1) - throw SysError("cannot clear the set of supplementary groups"); + /* Preserve supplementary groups of the build user, to allow + admins to specify groups such as "kvm". */ + if (setgroups(buildUser.getSupplementaryGIDs().size(), + buildUser.getSupplementaryGIDs().data()) == -1) + throw SysError("cannot set supplementary groups of build user"); if (setgid(buildUser.getGID()) == -1 || getgid() != buildUser.getGID() || @@ -2199,7 +2209,6 @@ void DerivationGoal::runChild() args.push_back(builderBasename); foreach (Strings::iterator, i, drv.args) args.push_back(rewriteHashes(*i, rewritesToTmp)); - auto argArr = stringsToCharPtrs(args); restoreSIGPIPE(); @@ -2207,7 +2216,7 @@ void DerivationGoal::runChild() writeFull(STDERR_FILENO, "\n"); /* Execute the program. This should not return. */ - execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) &envArr[0]); + execve(drv.builder.c_str(), stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); throw SysError(format("executing `%1%'") % drv.builder); @@ -2837,7 +2846,6 @@ void SubstitutionGoal::tryToRun() args.push_back("--substitute"); args.push_back(storePath); args.push_back(destPath); - auto argArr = stringsToCharPtrs(args); /* Fork the substitute program. */ pid = startProcess([&]() { @@ -2847,7 +2855,7 @@ void SubstitutionGoal::tryToRun() if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1) throw SysError("cannot dup output pipe into stdout"); - execv(sub.c_str(), (char * *) &argArr[0]); + execv(sub.c_str(), stringsToCharPtrs(args).data()); throw SysError(format("executing `%1%'") % sub); }); diff --git a/nix/libstore/derivations.cc b/nix/libstore/derivations.cc index b452aa2caf..d316f6c7bf 100644 --- a/nix/libstore/derivations.cc +++ b/nix/libstore/derivations.cc @@ -285,4 +285,13 @@ bool wantOutput(const string & output, const std::set & wanted) } +PathSet outputPaths(const Derivation & drv) +{ + PathSet paths; + for (auto & i : drv.outputs) + paths.insert(i.second.path); + return paths; +} + + } diff --git a/nix/libstore/derivations.hh b/nix/libstore/derivations.hh index 04b64dfc88..8d5e4d05d4 100644 --- a/nix/libstore/derivations.hh +++ b/nix/libstore/derivations.hh @@ -89,5 +89,6 @@ Path makeDrvPathWithOutputs(const Path & drvPath, const std::set & outpu bool wantOutput(const string & output, const std::set & wanted); +PathSet outputPaths(const Derivation & drv); } diff --git a/nix/libstore/gc.cc b/nix/libstore/gc.cc index 34768324c2..72eff52426 100644 --- a/nix/libstore/gc.cc +++ b/nix/libstore/gc.cc @@ -376,6 +376,7 @@ struct LocalStore::GCState bool gcKeepOutputs; bool gcKeepDerivations; unsigned long long bytesInvalidated; + bool moveToTrash = true; Path trashDir; bool shouldDelete; GCState(GCResults & results_) : results(results_), bytesInvalidated(0) { } @@ -428,16 +429,23 @@ void LocalStore::deletePathRecursive(GCState & state, const Path & path) not holding the global GC lock) we can delete the path without being afraid that the path has become alive again. Otherwise delete it right away. */ - if (S_ISDIR(st.st_mode)) { + if (state.moveToTrash && S_ISDIR(st.st_mode)) { // Estimate the amount freed using the narSize field. FIXME: // if the path was not valid, need to determine the actual // size. - state.bytesInvalidated += size; - if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) - throw SysError(format("making `%1%' writable") % path); - Path tmp = state.trashDir + "/" + baseNameOf(path); - if (rename(path.c_str(), tmp.c_str())) - throw SysError(format("unable to rename `%1%' to `%2%'") % path % tmp); + try { + if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) + throw SysError(format("making `%1%' writable") % path); + Path tmp = state.trashDir + "/" + baseNameOf(path); + if (rename(path.c_str(), tmp.c_str())) + throw SysError(format("unable to rename `%1%' to `%2%'") % path % tmp); + state.bytesInvalidated += size; + } catch (SysError & e) { + if (e.errNo == ENOSPC) { + printMsg(lvlInfo, format("note: can't create move `%1%': %2%") % path % e.msg()); + deleteGarbage(state, path); + } + } } else deleteGarbage(state, path); @@ -636,7 +644,14 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) if (state.shouldDelete) { if (pathExists(state.trashDir)) deleteGarbage(state, state.trashDir); - createDirs(state.trashDir); + try { + createDirs(state.trashDir); + } catch (SysError & e) { + if (e.errNo == ENOSPC) { + printMsg(lvlInfo, format("note: can't create trash directory: %1%") % e.msg()); + state.moveToTrash = false; + } + } } /* Now either delete all garbage paths, or just the specified diff --git a/nix/libstore/globals.cc b/nix/libstore/globals.cc index bb08a7d0b0..07f23d469c 100644 --- a/nix/libstore/globals.cc +++ b/nix/libstore/globals.cc @@ -36,7 +36,7 @@ Settings::Settings() buildTimeout = 0; useBuildHook = true; printBuildTrace = false; - reservedSize = 1024 * 1024; + reservedSize = 8 * 1024 * 1024; fsyncMetadata = true; useSQLiteWAL = true; syncBeforeRegistering = false; diff --git a/nix/libstore/local-store.cc b/nix/libstore/local-store.cc index 630cb80c41..b2c78477b6 100644 --- a/nix/libstore/local-store.cc +++ b/nix/libstore/local-store.cc @@ -288,7 +288,17 @@ LocalStore::LocalStore(bool reserveSpace) struct stat st; if (stat(reservedPath.c_str(), &st) == -1 || st.st_size != settings.reservedSize) - writeFile(reservedPath, string(settings.reservedSize, 'X')); + { + AutoCloseFD fd = open(reservedPath.c_str(), O_WRONLY | O_CREAT, 0600); + int res = -1; +#if HAVE_POSIX_FALLOCATE + res = posix_fallocate(fd, 0, settings.reservedSize); +#endif + if (res == -1) { + writeFull(fd, string(settings.reservedSize, 'X')); + ftruncate(fd, settings.reservedSize); + } + } } else deletePath(reservedPath); @@ -1166,10 +1176,8 @@ string LocalStore::getLineFromSubstituter(RunningSubstituter & run) if (n == 0) throw EndOfFile(format("substituter `%1%' died unexpectedly") % run.program); err.append(buf, n); string::size_type p; - while (((p = err.find('\n')) != string::npos) - || ((p = err.find('\r')) != string::npos)) { - string thing(err, 0, p + 1); - writeToStderr(run.program + ": " + thing); + while ((p = err.find('\n')) != string::npos) { + printMsg(lvlError, run.program + ": " + string(err, 0, p)); err = string(err, p + 1); } } diff --git a/nix/libstore/misc.cc b/nix/libstore/misc.cc index 6ecf8787cf..22363af126 100644 --- a/nix/libstore/misc.cc +++ b/nix/libstore/misc.cc @@ -120,7 +120,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets, if (invalid.empty()) continue; todoDrv.insert(*i); - if (settings.useSubstitutes && !willBuildLocally(drv)) + if (settings.useSubstitutes && substitutesAllowed(drv)) query.insert(invalid.begin(), invalid.end()); } @@ -144,7 +144,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets, PathSet outputs; bool mustBuild = false; - if (settings.useSubstitutes && !willBuildLocally(drv)) { + if (settings.useSubstitutes && substitutesAllowed(drv)) { foreach (DerivationOutputs::iterator, j, drv.outputs) { if (!wantOutput(j->first, i2.second)) continue; if (!store.isValidPath(j->second.path)) { diff --git a/nix/libstore/misc.hh b/nix/libstore/misc.hh index 144cb7f457..d3e31d51f7 100644 --- a/nix/libstore/misc.hh +++ b/nix/libstore/misc.hh @@ -34,5 +34,7 @@ void queryMissing(StoreAPI & store, const PathSet & targets, bool willBuildLocally(const Derivation & drv); +bool substitutesAllowed(const Derivation & drv); + } diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index dab4235b04..14026ab829 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -897,10 +897,10 @@ pid_t startProcess(std::function fun, } -std::vector stringsToCharPtrs(const Strings & ss) +std::vector stringsToCharPtrs(const Strings & ss) { - std::vector res; - for (auto & s : ss) res.push_back(s.c_str()); + std::vector res; + for (auto & s : ss) res.push_back((char *) s.c_str()); res.push_back(0); return res; } @@ -921,12 +921,11 @@ string runProgram(Path program, bool searchPath, const Strings & args) Strings args_(args); args_.push_front(program); - auto cargs = stringsToCharPtrs(args_); if (searchPath) - execvp(program.c_str(), (char * *) &cargs[0]); + execvp(program.c_str(), stringsToCharPtrs(args_).data()); else - execv(program.c_str(), (char * *) &cargs[0]); + execv(program.c_str(), stringsToCharPtrs(args_).data()); throw SysError(format("executing `%1%'") % program); }); diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh index 6a84ed8851..24e16ba36a 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -284,7 +284,7 @@ MakeError(ExecError, Error) /* Convert a list of strings to a null-terminated vector of char *'s. The result must not be accessed beyond the lifetime of the list of strings. */ -std::vector stringsToCharPtrs(const Strings & ss); +std::vector stringsToCharPtrs(const Strings & ss); /* Close all file descriptors except stdin, stdout, stderr, and those listed in the given set. Good practice in child processes. */ diff --git a/nix/nix-daemon/nix-daemon.cc b/nix/nix-daemon/nix-daemon.cc index 10159db62e..2b89190dbe 100644 --- a/nix/nix-daemon/nix-daemon.cc +++ b/nix/nix-daemon/nix-daemon.cc @@ -440,10 +440,7 @@ static void performOp(bool trusted, unsigned int clientVersion, case wopImportPaths: { startWork(); TunnelSource source(from); - - /* Unlike Nix, always require a signature, even for "trusted" - users. */ - Paths paths = store->importPaths(true, source); + Paths paths = store->importPaths(!trusted, source); stopWork(); writeStrings(paths, to); break;