diff options
| author | Vincent Ambo <tazjin@google.com> | 2020-05-19T19·47+0100 |
|---|---|---|
| committer | Vincent Ambo <tazjin@google.com> | 2020-05-19T19·51+0100 |
| commit | 39087321811e81e26a1a47d6967df1088dcf0e95 (patch) | |
| tree | 57110be423eeb7869e9960466f4b17c0ea7cd961 /third_party/nix/src/libstore | |
| parent | cf40d08908ede4061eb15513b770c98877844b8b (diff) | |
style(3p/nix): Final act in the brace-wrapping saga r/777
This last change set was generated by a full clang-tidy run (including
compilation):
clang-tidy -p ~/projects/nix-build/ \
-checks=-*,readability-braces-around-statements -fix src/*/*.cc
Actually running clang-tidy requires some massaging to make it play
nice with Nix + meson, I'll be adding a wrapper or something for that soon.
Diffstat (limited to 'third_party/nix/src/libstore')
28 files changed, 954 insertions, 527 deletions
diff --git a/third_party/nix/src/libstore/binary-cache-store.cc b/third_party/nix/src/libstore/binary-cache-store.cc index 625ea1e11eae..10a0ddfc1363 100644 --- a/third_party/nix/src/libstore/binary-cache-store.cc +++ b/third_party/nix/src/libstore/binary-cache-store.cc @@ -19,9 +19,10 @@ namespace nix { BinaryCacheStore::BinaryCacheStore(const Params& params) : Store(params) { - if (secretKeyFile != "") + if (secretKeyFile != "") { secretKey = std::unique_ptr<SecretKey>(new SecretKey(readFile(secretKeyFile))); + } StringSink sink; sink << narVersionMagic1; @@ -44,10 +45,11 @@ void BinaryCacheStore::init() { auto name = line.substr(0, colon); auto value = trim(line.substr(colon + 1, std::string::npos)); if (name == "StoreDir") { - if (value != storeDir) + if (value != storeDir) { throw Error(format("binary cache '%s' is for Nix stores with prefix " "'%s', not '%s'") % getUri() % value % storeDir); + } } else if (name == "WantMassQuery") { wantMassQuery_ = value == "1"; } else if (name == "Priority") { @@ -108,9 +110,10 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo) { state_->pathInfoCache.upsert(hashPart, std::shared_ptr<NarInfo>(narInfo)); } - if (diskCache) + if (diskCache) { diskCache->upsertNarInfo(getUri(), hashPart, std::shared_ptr<NarInfo>(narInfo)); + } } void BinaryCacheStore::addToStore(const ValidPathInfo& info, @@ -123,7 +126,8 @@ void BinaryCacheStore::addToStore(const ValidPathInfo& info, /* Verify that all references are valid. This may do some .narinfo reads, but typically they'll already be cached. */ - for (auto& ref : info.references) try { + for (auto& ref : info.references) { + try { if (ref != info.path) { queryPathInfo(ref); } @@ -132,6 +136,7 @@ void BinaryCacheStore::addToStore(const ValidPathInfo& info, "reference '%s' is not valid") % info.path % ref); } + } assert(nar->compare(0, narMagic.size(), narMagic) == 0); @@ -140,10 +145,11 @@ void BinaryCacheStore::addToStore(const ValidPathInfo& info, narInfo->narSize = nar->size(); narInfo->narHash = hashString(htSHA256, *nar); - if (info.narHash && info.narHash != narInfo->narHash) + if (info.narHash && info.narHash != narInfo->narHash) { throw Error( format("refusing to copy corrupted path '%1%' to binary cache") % info.path); + } auto accessor_ = std::dynamic_pointer_cast<RemoteFSAccessor>(accessor); @@ -203,8 +209,9 @@ void BinaryCacheStore::addToStore(const ValidPathInfo& info, if (repair || !fileExists(narInfo->url)) { stats.narWrite++; upsertFile(narInfo->url, *narCompressed, "application/x-nix-nar"); - } else + } else { stats.narWriteAverted++; + } stats.narWriteBytes += nar->size(); stats.narWriteCompressedBytes += narCompressed->size(); @@ -349,9 +356,9 @@ void BinaryCacheStore::addSignatures(const Path& storePath, std::shared_ptr<std::string> BinaryCacheStore::getBuildLog(const Path& path) { Path drvPath; - if (isDerivation(path)) + if (isDerivation(path)) { drvPath = path; - else { + } else { try { auto info = queryPathInfo(path); // FIXME: add a "Log" field to .narinfo diff --git a/third_party/nix/src/libstore/build.cc b/third_party/nix/src/libstore/build.cc index 7aa44b47551a..b1f63a4fac76 100644 --- a/third_party/nix/src/libstore/build.cc +++ b/third_party/nix/src/libstore/build.cc @@ -333,10 +333,11 @@ class Worker { void addToWeakGoals(WeakGoals& goals, GoalPtr p) { // FIXME: necessary? // FIXME: O(n) - for (auto& i : goals) + for (auto& i : goals) { if (i.lock() == p) { return; } + } goals.push_back(p); } @@ -369,10 +370,11 @@ void Goal::waiteeDone(GoalPtr waitee, ExitCode result) { remaining waitees. */ for (auto& goal : waitees) { WeakGoals waiters2; - for (auto& j : goal->waiters) + for (auto& j : goal->waiters) { if (j.lock() != shared_from_this()) { waiters2.push_back(j); } + } goal->waiters = waiters2; } waitees.clear(); @@ -416,19 +418,23 @@ static void commonChildInit(Pipe& logPipe) { } /* Dup the write side of the logger pipe into stderr. */ - if (dup2(logPipe.writeSide.get(), STDERR_FILENO) == -1) + if (dup2(logPipe.writeSide.get(), STDERR_FILENO) == -1) { throw SysError("cannot pipe standard error into log file"); + } /* Dup stderr to stdout. */ - if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) + if (dup2(STDERR_FILENO, STDOUT_FILENO) == -1) { throw SysError("cannot dup stderr into stdout"); + } /* Reroute stdin to /dev/null. */ int fdDevNull = open(pathNullDevice.c_str(), O_RDWR); - if (fdDevNull == -1) + if (fdDevNull == -1) { throw SysError(format("cannot open '%1%'") % pathNullDevice); - if (dup2(fdDevNull, STDIN_FILENO) == -1) + } + if (dup2(fdDevNull, STDIN_FILENO) == -1) { throw SysError("cannot dup null device into stdin"); + } close(fdDevNull); } @@ -444,10 +450,11 @@ void handleDiffHook(uid_t uid, uid_t gid, Path tryA, Path tryB, Path drvPath, diffHookOptions.chdir = "/"; auto diffRes = runProgram(diffHookOptions); - if (!statusOk(diffRes.first)) + if (!statusOk(diffRes.first)) { throw ExecError(diffRes.first, fmt("diff-hook program '%1%' %2%", diffHook, statusToString(diffRes.first))); + } if (diffRes.second != "") { LOG(ERROR) << chomp(diffRes.second); @@ -503,11 +510,12 @@ UserLock::UserLock() { /* Get the members of the build-users-group. */ struct group* gr = getgrnam(settings.buildUsersGroup.get().c_str()); - if (!gr) + if (!gr) { throw Error( format( "the group '%1%' specified in 'build-users-group' does not exist") % settings.buildUsersGroup); + } gid = gr->gr_gid; /* Copy the result of getgrnam. */ @@ -517,9 +525,10 @@ UserLock::UserLock() { users.push_back(*p); } - if (users.empty()) + if (users.empty()) { throw Error(format("the build users group '%1%' has no members") % settings.buildUsersGroup); + } /* Find a user account that isn't currently in use for another build. */ @@ -527,9 +536,10 @@ UserLock::UserLock() { DLOG(INFO) << "trying user " << i; struct passwd* pw = getpwnam(i.c_str()); - if (!pw) + if (!pw) { throw Error(format("the user '%1%' in the group '%2%' does not exist") % i % settings.buildUsersGroup); + } createDirs(settings.nixStateDir + "/userpool"); @@ -538,9 +548,10 @@ UserLock::UserLock() { { auto lockedPaths(lockedPaths_.lock()); - if (lockedPaths->count(fnUserLock)) + if (lockedPaths->count(fnUserLock)) { /* We already have a lock on this one. */ continue; + } lockedPaths->insert(fnUserLock); } @@ -557,9 +568,10 @@ UserLock::UserLock() { uid = pw->pw_uid; /* Sanity check... */ - if (uid == getuid() || uid == geteuid()) + if (uid == getuid() || uid == geteuid()) { throw Error(format("the Nix user should not be a member of '%1%'") % settings.buildUsersGroup); + } #if __linux__ /* Get the list of supplementary groups of this build user. This @@ -568,10 +580,11 @@ UserLock::UserLock() { int ngroups = supplementaryGIDs.size(); int err = getgrouplist(pw->pw_name, pw->pw_gid, supplementaryGIDs.data(), &ngroups); - if (err == -1) + if (err == -1) { throw Error( format("failed to get list of supplementary groups for '%1%'") % pw->pw_name); + } supplementaryGIDs.resize(ngroups); #endif @@ -673,8 +686,9 @@ HookInstance::HookInstance() { sink = FdSink(toHook.writeSide.get()); std::map<std::string, Config::SettingInfo> settings; globalConfig.getSettings(settings); - for (auto& setting : settings) + for (auto& setting : settings) { sink << 1 << setting.first << setting.second.value; + } sink << 0; } @@ -696,8 +710,9 @@ typedef map<std::string, std::string> StringRewrites; std::string rewriteStrings(std::string s, const StringRewrites& rewrites) { for (auto& i : rewrites) { size_t j = 0; - while ((j = s.find(i.first, j)) != string::npos) + while ((j = s.find(i.first, j)) != string::npos) { s.replace(j, i.first.size(), i.second); + } } return s; } @@ -1045,8 +1060,9 @@ void DerivationGoal::killChild() { ::kill(-pid, SIGKILL); /* ignore the result */ buildUser->kill(); pid.wait(); - } else + } else { pid.kill(); + } assert(pid == -1); } @@ -1070,12 +1086,14 @@ void DerivationGoal::addWantedOutputs(const StringSet& outputs) { if (outputs.empty()) { wantedOutputs.clear(); needRestart = true; - } else - for (auto& i : outputs) + } else { + for (auto& i : outputs) { if (wantedOutputs.find(i) == wantedOutputs.end()) { wantedOutputs.insert(i); needRestart = true; } + } + } } void DerivationGoal::getDerivation() { @@ -1140,15 +1158,18 @@ 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 && parsedDrv->substitutesAllowed()) - for (auto& i : invalidOutputs) + if (settings.useSubstitutes && parsedDrv->substitutesAllowed()) { + for (auto& i : invalidOutputs) { addWaitee(worker.makeSubstitutionGoal( i, buildMode == bmRepair ? Repair : NoRepair)); + } + } - if (waitees.empty()) /* to prevent hang (no wake-up event) */ + if (waitees.empty()) { /* to prevent hang (no wake-up event) */ outputsSubstituted(); - else + } else { state = &DerivationGoal::outputsSubstituted; + } } void DerivationGoal::outputsSubstituted() { @@ -1189,10 +1210,11 @@ void DerivationGoal::outputsSubstituted() { repairClosure(); return; } - if (buildMode == bmCheck && nrInvalid > 0) + if (buildMode == bmCheck && nrInvalid > 0) { throw Error(format("some outputs of '%1%' are not valid, so checking is " "not possible") % drvPath); + } /* Otherwise, at least one of the output paths could not be produced using a substitute. So we have to build instead. */ @@ -1202,26 +1224,30 @@ void DerivationGoal::outputsSubstituted() { wantedOutputs = PathSet(); /* The inputs must be built before we can build this goal. */ - if (useDerivation) - for (auto& i : dynamic_cast<Derivation*>(drv.get())->inputDrvs) + if (useDerivation) { + for (auto& i : dynamic_cast<Derivation*>(drv.get())->inputDrvs) { addWaitee(worker.makeDerivationGoal( i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal)); + } + } for (auto& i : drv->inputSrcs) { if (worker.store.isValidPath(i)) { continue; } - if (!settings.useSubstitutes) + if (!settings.useSubstitutes) { throw Error(format("dependency '%1%' of '%2%' does not exist, and " "substitution is disabled") % i % drvPath); + } addWaitee(worker.makeSubstitutionGoal(i)); } - if (waitees.empty()) /* to prevent hang (no wake-up event) */ + if (waitees.empty()) { /* to prevent hang (no wake-up event) */ inputsRealised(); - else + } else { state = &DerivationGoal::inputsRealised; + } } void DerivationGoal::repairClosure() { @@ -1252,13 +1278,14 @@ void DerivationGoal::repairClosure() { worker.store.computeFSClosure(drvPath, inputClosure); } std::map<Path, Path> outputsToDrv; - for (auto& i : inputClosure) + for (auto& i : inputClosure) { if (isDerivation(i)) { Derivation drv = worker.store.derivationFromPath(i); for (auto& j : drv.outputs) { outputsToDrv[j.second.path] = i; } } + } /* Check each path (slow!). */ PathSet broken; @@ -1269,10 +1296,11 @@ void DerivationGoal::repairClosure() { LOG(ERROR) << "found corrupted or missing path '" << i << "' in the output closure of '" << drvPath << "'"; Path drvPath2 = outputsToDrv[i]; - if (drvPath2 == "") + if (drvPath2 == "") { addWaitee(worker.makeSubstitutionGoal(i, Repair)); - else + } else { addWaitee(worker.makeDerivationGoal(drvPath2, PathSet(), bmRepair)); + } } if (waitees.empty()) { @@ -1285,10 +1313,11 @@ void DerivationGoal::repairClosure() { void DerivationGoal::closureRepaired() { trace("closure repaired"); - if (nrFailed > 0) + if (nrFailed > 0) { throw Error(format("some paths in the output closure of derivation '%1%' " "could not be repaired") % drvPath); + } done(BuildResult::AlreadyValid); } @@ -1296,8 +1325,9 @@ void DerivationGoal::inputsRealised() { trace("all inputs realised"); if (nrFailed != 0) { - if (!useDerivation) + if (!useDerivation) { throw Error(format("some dependencies of '%1%' are missing") % drvPath); + } LOG(ERROR) << "cannot build derivation '" << drvPath << "': " << nrFailed << " dependencies couldn't be built"; done(BuildResult::DependencyFailed); @@ -1321,21 +1351,24 @@ void DerivationGoal::inputsRealised() { /* Determine the full set of input paths. */ /* First, the input derivations. */ - if (useDerivation) + if (useDerivation) { for (auto& i : dynamic_cast<Derivation*>(drv.get())->inputDrvs) { /* Add the relevant output closures of the input derivation `i' as input paths. Only add the closures of output paths that are specified as inputs. */ assert(worker.store.isValidPath(i.first)); Derivation inDrv = worker.store.derivationFromPath(i.first); - for (auto& j : i.second) - if (inDrv.outputs.find(j) != inDrv.outputs.end()) + for (auto& j : i.second) { + if (inDrv.outputs.find(j) != inDrv.outputs.end()) { worker.store.computeFSClosure(inDrv.outputs[j].path, inputPaths); - else + } else { throw Error(format("derivation '%1%' requires non-existent output " "'%2%' from input derivation '%3%'") % drvPath % j % i.first); + } + } } + } /* Second, the input sources. */ worker.store.computeFSClosure(drv->inputSrcs, inputPaths); @@ -1369,8 +1402,9 @@ void DerivationGoal::tryToBuild() { goal can start a build, and if not, the main loop will sleep a few seconds and then retry this goal. */ PathSet lockFiles; - for (auto& outPath : drv->outputPaths()) + for (auto& outPath : drv->outputPaths()) { lockFiles.insert(worker.store.toRealPath(outPath)); + } if (!outputLocks.lockPaths(lockFiles, "", false)) { worker.waitForAWhile(shared_from_this()); @@ -1394,10 +1428,11 @@ void DerivationGoal::tryToBuild() { } missingPaths = drv->outputPaths(); - if (buildMode != bmCheck) + if (buildMode != bmCheck) { for (auto& i : validPaths) { missingPaths.erase(i); } + } /* If any of the outputs already exist but are not valid, delete them. */ @@ -1494,8 +1529,9 @@ void replaceValidPath(const Path& storePath, const Path tmpPath) { if (pathExists(storePath)) { rename(storePath.c_str(), oldPath.c_str()); } - if (rename(tmpPath.c_str(), storePath.c_str()) == -1) + if (rename(tmpPath.c_str(), storePath.c_str()) == -1) { throw SysError(format("moving '%1%' to '%2%'") % tmpPath % storePath); + } deletePath(oldPath); } @@ -1527,8 +1563,9 @@ MakeError(NotDeterministic, BuildError) if (hook) { hook->builderOut.readSide = -1; hook->fromHook.readSide = -1; - } else + } else { builderOut.readSide = -1; + } /* Close the log file. */ closeLogFile(); @@ -1557,21 +1594,26 @@ MakeError(NotDeterministic, BuildError) 8ULL * 1024 * 1024; // FIXME: make configurable struct statvfs st; if (statvfs(worker.store.realStoreDir.c_str(), &st) == 0 && - (unsigned long long)st.f_bavail * st.f_bsize < required) + (unsigned long long)st.f_bavail * st.f_bsize < required) { diskFull = true; + } if (statvfs(tmpDir.c_str(), &st) == 0 && - (unsigned long long)st.f_bavail * st.f_bsize < required) + (unsigned long long)st.f_bavail * st.f_bsize < required) { diskFull = true; + } #endif deleteTmpDir(false); /* Move paths out of the chroot for easier debugging of build failures. */ - if (useChroot && buildMode == bmNormal) - for (auto& i : missingPaths) - if (pathExists(chrootRootDir + i)) + if (useChroot && buildMode == bmNormal) { + for (auto& i : missingPaths) { + if (pathExists(chrootRootDir + i)) { rename((chrootRootDir + i).c_str(), i.c_str()); + } + } + } std::string msg = (format("builder for '%1%' %2%") % drvPath % statusToString(status)) @@ -1683,10 +1725,10 @@ MakeError(NotDeterministic, BuildError) BuildResult::Status st = BuildResult::MiscFailure; - if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101) + if (hook && WIFEXITED(status) && WEXITSTATUS(status) == 101) { st = BuildResult::TimedOut; - else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) { + } else if (hook && (!WIFEXITED(status) || WEXITSTATUS(status) != 100)) { } else { @@ -1788,8 +1830,9 @@ HookReply DerivationGoal::tryBuildHook() { } void chmod_(const Path& path, mode_t mode) { - if (chmod(path.c_str(), mode) == -1) + if (chmod(path.c_str(), mode) == -1) { throw SysError(format("setting permissions on '%1%'") % path); + } } int childEntry(void* arg) { @@ -1802,18 +1845,20 @@ PathSet DerivationGoal::exportReferences(PathSet storePaths) { for (auto storePath : storePaths) { /* Check that the store path is valid. */ - if (!worker.store.isInStore(storePath)) + if (!worker.store.isInStore(storePath)) { throw BuildError( format("'exportReferencesGraph' contains a non-store path '%1%'") % storePath); + } storePath = worker.store.toStorePath(storePath); - if (!inputPaths.count(storePath)) + if (!inputPaths.count(storePath)) { throw BuildError( "cannot export references of path '%s' because it is not in the " "input closure of the derivation", storePath); + } worker.store.computeFSClosure(storePath, paths); } @@ -1827,8 +1872,9 @@ PathSet DerivationGoal::exportReferences(PathSet storePaths) { for (auto& j : paths2) { if (isDerivation(j)) { Derivation drv = worker.store.derivationFromPath(j); - for (auto& k : drv.outputs) + for (auto& k : drv.outputs) { worker.store.computeFSClosure(k.second.path, paths); + } } } @@ -1859,13 +1905,14 @@ static void preloadNSS() { void DerivationGoal::startBuilder() { /* Right platform? */ - if (!parsedDrv->canBuildLocally()) + if (!parsedDrv->canBuildLocally()) { throw Error( "a '%s' with features {%s} is required to build '%s', but I am a '%s' " "with features {%s}", drv->platform, concatStringsSep(", ", parsedDrv->getRequiredSystemFeatures()), drvPath, settings.thisSystem, concatStringsSep(", ", settings.systemFeatures)); + } if (drv->isBuiltin()) { preloadNSS(); @@ -1880,10 +1927,11 @@ void DerivationGoal::startBuilder() { { auto noChroot = parsedDrv->getBoolAttr("__noChroot"); if (settings.sandboxMode == smEnabled) { - if (noChroot) + if (noChroot) { throw Error(format("derivation '%1%' has '__noChroot' set, " "but that's not allowed when 'sandbox' is 'true'") % drvPath); + } #if __APPLE__ if (additionalSandboxProfile != "") throw Error( @@ -1892,10 +1940,11 @@ void DerivationGoal::startBuilder() { drvPath); #endif useChroot = true; - } else if (settings.sandboxMode == smDisabled) + } else if (settings.sandboxMode == smDisabled) { useChroot = false; - else if (settings.sandboxMode == smRelaxed) + } else if (settings.sandboxMode == smRelaxed) { useChroot = !fixedOutput && !noChroot; + } } if (worker.store.storeDir != worker.store.realStoreDir) { @@ -1932,8 +1981,9 @@ void DerivationGoal::startBuilder() { chownToBuilder(tmpDir); /* Substitute output placeholders with the actual output paths. */ - for (auto& output : drv->outputs) + for (auto& output : drv->outputs) { inputRewrites[hashPlaceholder(output.first)] = output.second.path; + } /* Construct the environment passed to the builder. */ initEnv(); @@ -1951,9 +2001,10 @@ void DerivationGoal::startBuilder() { fields are left empty. */ string s = get(drv->env, "exportReferencesGraph"); Strings ss = tokenizeString<Strings>(s); - if (ss.size() % 2 != 0) + if (ss.size() % 2 != 0) { throw BuildError( format("odd number of tokens in 'exportReferencesGraph': '%1%'") % s); + } for (Strings::iterator i = ss.begin(); i != ss.end();) { string fileName = *i++; checkStoreName(fileName); /* !!! abuse of this function */ @@ -1985,23 +2036,27 @@ void DerivationGoal::startBuilder() { i.pop_back(); } size_t p = i.find('='); - if (p == string::npos) + if (p == string::npos) { dirsInChroot[i] = {i, optional}; - else + } else { dirsInChroot[string(i, 0, p)] = {string(i, p + 1), optional}; + } } dirsInChroot[tmpDirInSandbox] = tmpDir; /* Add the closure of store paths to the chroot. */ PathSet closure; - for (auto& i : dirsInChroot) try { - if (worker.store.isInStore(i.second.source)) + for (auto& i : dirsInChroot) { + try { + if (worker.store.isInStore(i.second.source)) { worker.store.computeFSClosure( worker.store.toStorePath(i.second.source), closure); + } } catch (InvalidPath& e) { } catch (Error& e) { throw Error(format("while processing 'sandbox-paths': %s") % e.what()); } + } for (auto& i : closure) { dirsInChroot[i] = i; } @@ -2027,10 +2082,11 @@ void DerivationGoal::startBuilder() { break; } } - if (!found) + if (!found) { throw Error(format("derivation '%1%' requested impure path '%2%', but " "it was not in allowed-impure-host-deps") % drvPath % i); + } dirsInChroot[i] = i; } @@ -2048,12 +2104,15 @@ void DerivationGoal::startBuilder() { DLOG(INFO) << "setting up chroot environment in '" << chrootRootDir << "'"; - if (mkdir(chrootRootDir.c_str(), 0750) == -1) + if (mkdir(chrootRootDir.c_str(), 0750) == -1) { throw SysError(format("cannot create '%1%'") % chrootRootDir); + } - if (buildUser && chown(chrootRootDir.c_str(), 0, buildUser->getGID()) == -1) + if (buildUser && + chown(chrootRootDir.c_str(), 0, buildUser->getGID()) == -1) { throw SysError(format("cannot change ownership of '%1%'") % chrootRootDir); + } /* Create a writable /tmp in the chroot. Many builders need this. (Of course they should really respect $TMPDIR @@ -2082,9 +2141,10 @@ void DerivationGoal::startBuilder() { .str()); /* Create /etc/hosts with localhost entry. */ - if (!fixedOutput) + if (!fixedOutput) { writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); + } /* Make the closure of the inputs available in the chroot, rather than the whole Nix store. This prevents any access @@ -2098,18 +2158,20 @@ void DerivationGoal::startBuilder() { chmod_(chrootStoreDir, 01775); if (buildUser && - chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) + chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) { throw SysError(format("cannot change ownership of '%1%'") % chrootStoreDir); + } for (auto& i : inputPaths) { Path r = worker.store.toRealPath(i); struct stat st; - if (lstat(r.c_str(), &st)) + if (lstat(r.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % i); - if (S_ISDIR(st.st_mode)) + } + if (S_ISDIR(st.st_mode)) { dirsInChroot[i] = r; - else { + } else { Path p = chrootRootDir + i; DLOG(INFO) << "linking '" << p << "' to '" << r << "'"; if (link(r.c_str(), p.c_str()) == -1) { @@ -2117,8 +2179,9 @@ void DerivationGoal::startBuilder() { link count on a file (e.g. 32000 of ext3), which is quite possible after a `nix-store --optimise'. */ - if (errno != EMLINK) + if (errno != EMLINK) { throw SysError(format("linking '%1%' to '%2%'") % p % i); + } StringSink sink; dumpPath(r, sink); StringSource source(*sink.s); @@ -2145,8 +2208,9 @@ void DerivationGoal::startBuilder() { } if (needsHashRewrite()) { - if (pathExists(homeDir)) + if (pathExists(homeDir)) { throw Error(format("directory '%1%' exists; please remove it") % homeDir); + } /* We're not doing a chroot build, but we have some valid output paths. Since we can't just overwrite or delete @@ -2157,19 +2221,22 @@ void DerivationGoal::startBuilder() { corresponding to the valid outputs, and rewrite the contents of the new outputs to replace the dummy strings with the actual hashes. */ - if (validPaths.size() > 0) + if (validPaths.size() > 0) { for (auto& i : validPaths) { addHashRewrite(i); } + } /* If we're repairing, then we don't want to delete the corrupt outputs in advance. So rewrite them as well. */ - if (buildMode == bmRepair) - for (auto& i : missingPaths) + if (buildMode == bmRepair) { + for (auto& i : missingPaths) { if (worker.store.isValidPath(i) && pathExists(i)) { addHashRewrite(i); redirectedBadOutputs.insert(i); } + } + } } if (useChroot && settings.preBuildHook != "" && @@ -2196,10 +2263,11 @@ void DerivationGoal::startBuilder() { state = stBegin; } else { auto p = line.find('='); - if (p == string::npos) + if (p == string::npos) { dirsInChroot[line] = line; - else + } else { dirsInChroot[string(line, 0, p)] = string(line, p + 1); + } } } } @@ -2222,14 +2290,17 @@ void DerivationGoal::startBuilder() { std::string slaveName(ptsname(builderOut.readSide.get())); if (buildUser) { - if (chmod(slaveName.c_str(), 0600)) + if (chmod(slaveName.c_str(), 0600)) { throw SysError("changing mode of pseudoterminal slave"); + } - if (chown(slaveName.c_str(), buildUser->getUID(), 0)) + if (chown(slaveName.c_str(), buildUser->getUID(), 0)) { throw SysError("changing owner of pseudoterminal slave"); + } } else { - if (grantpt(builderOut.readSide.get())) + if (grantpt(builderOut.readSide.get())) { throw SysError("granting access to pseudoterminal slave"); + } } #if 0 @@ -2239,8 +2310,9 @@ void DerivationGoal::startBuilder() { dirsInChroot[slaveName] = {slaveName, false}; #endif - if (unlockpt(builderOut.readSide.get())) + if (unlockpt(builderOut.readSide.get())) { throw SysError("unlocking pseudoterminal"); + } builderOut.writeSide = open(slaveName.c_str(), O_RDWR | O_NOCTTY); if (!builderOut.writeSide) { @@ -2249,13 +2321,15 @@ void DerivationGoal::startBuilder() { // Put the pt into raw mode to prevent \n -> \r\n translation. struct termios term; - if (tcgetattr(builderOut.writeSide.get(), &term)) + if (tcgetattr(builderOut.writeSide.get(), &term)) { throw SysError("getting pseudoterminal attributes"); + } cfmakeraw(&term); - if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) + if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) { throw SysError("putting pseudoterminal into raw mode"); + } result.startTime = time(0); @@ -2316,8 +2390,9 @@ void DerivationGoal::startBuilder() { not seem to be a workaround for this. (But who can tell from reading user_namespaces(7)?) See also https://lwn.net/Articles/621612/. */ - if (getuid() == 0 && setgroups(0, 0) == -1) + if (getuid() == 0 && setgroups(0, 0) == -1) { throw SysError("setgroups failed"); + } size_t stackSize = 1 * 1024 * 1024; char* stack = @@ -2352,8 +2427,9 @@ void DerivationGoal::startBuilder() { parent. This is only done when sandbox-fallback is set to true (the default). */ if (child == -1 && (errno == EPERM || errno == EINVAL) && - settings.sandboxFallback) + settings.sandboxFallback) { _exit(1); + } if (child == -1) { throw SysError("cloning builder process"); } @@ -2368,8 +2444,9 @@ void DerivationGoal::startBuilder() { useChroot = false; initTmpDir(); goto fallback; - } else if (res != 0) + } else if (res != 0) { throw Error("unable to start build process"); + } userNamespaceSync.readSide = -1; @@ -2518,8 +2595,9 @@ void DerivationGoal::initEnv() { already know the cryptographic hash of the output). */ if (fixedOutput) { for (auto& i : - parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) + parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) { env[i] = getEnv(i); + } } /* Currently structured log messages piggyback on stderr, but we @@ -2543,8 +2621,9 @@ void DerivationGoal::writeStructuredAttrs() { /* Add an "outputs" object containing the output paths. */ nlohmann::json outputs; - for (auto& i : drv->outputs) + for (auto& i : drv->outputs) { outputs[i.first] = rewriteStrings(i.second.path, inputRewrites); + } json["outputs"] = outputs; /* Handle exportReferencesGraph. */ @@ -2592,8 +2671,9 @@ void DerivationGoal::writeStructuredAttrs() { return std::string("''"); } - if (value.is_boolean()) + if (value.is_boolean()) { return value.get<bool>() ? std::string("1") : std::string(""); + } return {}; }; @@ -2608,10 +2688,10 @@ void DerivationGoal::writeStructuredAttrs() { auto& value = i.value(); auto s = handleSimpleType(value); - if (s) + if (s) { jsonSh += fmt("declare %s=%s\n", i.key(), *s); - else if (value.is_array()) { + } else if (value.is_array()) { std::string s2; bool good = true; @@ -2657,8 +2737,9 @@ void DerivationGoal::chownToBuilder(const Path& path) { if (!buildUser) { return; } - if (chown(path.c_str(), buildUser->getUID(), buildUser->getGID()) == -1) + if (chown(path.c_str(), buildUser->getUID(), buildUser->getGID()) == -1) { throw SysError(format("cannot change ownership of '%1%'") % path); + } } void setupSeccomp() { @@ -2669,18 +2750,21 @@ void setupSeccomp() { #if HAVE_SECCOMP scmp_filter_ctx ctx; - if (!(ctx = seccomp_init(SCMP_ACT_ALLOW))) + if (!(ctx = seccomp_init(SCMP_ACT_ALLOW))) { throw SysError("unable to initialize seccomp mode 2"); + } Finally cleanup([&]() { seccomp_release(ctx); }); if (nativeSystem == "x86_64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_X86) != 0) + seccomp_arch_add(ctx, SCMP_ARCH_X86) != 0) { throw SysError("unable to add 32-bit seccomp architecture"); + } if (nativeSystem == "x86_64-linux" && - seccomp_arch_add(ctx, SCMP_ARCH_X32) != 0) + seccomp_arch_add(ctx, SCMP_ARCH_X32) != 0) { throw SysError("unable to add X32 seccomp architecture"); + } if (nativeSystem == "aarch64-linux" && seccomp_arch_add(ctx, SCMP_ARCH_ARM) != 0) { @@ -2692,18 +2776,21 @@ void setupSeccomp() { for (int perm : {S_ISUID, S_ISGID}) { if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(chmod), 1, SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t)perm, - (scmp_datum_t)perm)) != 0) + (scmp_datum_t)perm)) != 0) { throw SysError("unable to add seccomp rule"); + } if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(fchmod), 1, SCMP_A1(SCMP_CMP_MASKED_EQ, (scmp_datum_t)perm, - (scmp_datum_t)perm)) != 0) + (scmp_datum_t)perm)) != 0) { throw SysError("unable to add seccomp rule"); + } if (seccomp_rule_add(ctx, SCMP_ACT_ERRNO(EPERM), SCMP_SYS(fchmodat), 1, SCMP_A2(SCMP_CMP_MASKED_EQ, (scmp_datum_t)perm, - (scmp_datum_t)perm)) != 0) + (scmp_datum_t)perm)) != 0) { throw SysError("unable to add seccomp rule"); + } } /* Prevent builders from creating EAs or ACLs. Not all filesystems @@ -2714,15 +2801,18 @@ void setupSeccomp() { seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(lsetxattr), 0) != 0 || seccomp_rule_add(ctx, SCMP_ACT_ERRNO(ENOTSUP), SCMP_SYS(fsetxattr), 0) != - 0) + 0) { throw SysError("unable to add seccomp rule"); + } if (seccomp_attr_set(ctx, SCMP_FLTATR_CTL_NNP, - settings.allowNewPrivileges ? 0 : 1) != 0) + settings.allowNewPrivileges ? 0 : 1) != 0) { throw SysError("unable to set 'no new privileges' seccomp attribute"); + } - if (seccomp_load(ctx) != 0) + if (seccomp_load(ctx) != 0) { throw SysError("unable to load seccomp BPF program"); + } #else throw Error( "seccomp is not supported on this platform; " @@ -2754,8 +2844,9 @@ void DerivationGoal::runChild() { (which may run under a different uid and/or in a sandbox). */ std::string netrcData; try { - if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") + if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") { netrcData = readFile(settings.netrcFile); + } } catch (SysError&) { } @@ -2763,8 +2854,9 @@ void DerivationGoal::runChild() { if (useChroot) { userNamespaceSync.writeSide = -1; - if (drainFD(userNamespaceSync.readSide.get()) != "1") + if (drainFD(userNamespaceSync.readSide.get()) != "1") { throw Error("user namespace initialisation failed"); + } userNamespaceSync.readSide = -1; @@ -2778,17 +2870,20 @@ void DerivationGoal::runChild() { struct ifreq ifr; strcpy(ifr.ifr_name, "lo"); ifr.ifr_flags = IFF_UP | IFF_LOOPBACK | IFF_RUNNING; - if (ioctl(fd.get(), SIOCSIFFLAGS, &ifr) == -1) + if (ioctl(fd.get(), SIOCSIFFLAGS, &ifr) == -1) { throw SysError("cannot set loopback interface flags"); + } } /* Set the hostname etc. to fixed values. */ char hostname[] = "localhost"; - if (sethostname(hostname, sizeof(hostname)) == -1) + if (sethostname(hostname, sizeof(hostname)) == -1) { throw SysError("cannot set host name"); + } char domainname[] = "(none)"; // kernel default - if (setdomainname(domainname, sizeof(domainname)) == -1) + if (setdomainname(domainname, sizeof(domainname)) == -1) { throw SysError("cannot set domain name"); + } /* Make all filesystems private. This is necessary because subtrees may have been mounted as "shared" @@ -2805,8 +2900,9 @@ void DerivationGoal::runChild() { /* Bind-mount chroot directory to itself, to treat it as a different filesystem from /, as needed for pivot_root. */ if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == - -1) + -1) { throw SysError(format("unable to bind mount '%1%'") % chrootRootDir); + } /* Set up a nearly empty /dev, unless the user asked to bind-mount the host /dev. */ @@ -2816,8 +2912,9 @@ void DerivationGoal::runChild() { createDirs(chrootRootDir + "/dev/pts"); ss.push_back("/dev/full"); if (settings.systemFeatures.get().count("kvm") && - pathExists("/dev/kvm")) + pathExists("/dev/kvm")) { ss.push_back("/dev/kvm"); + } ss.push_back("/dev/null"); ss.push_back("/dev/random"); ss.push_back("/dev/tty"); @@ -2844,8 +2941,9 @@ void DerivationGoal::runChild() { ss.push_back("/etc/services"); ss.push_back("/etc/hosts"); - if (pathExists("/var/run/nscd/socket")) + if (pathExists("/var/run/nscd/socket")) { ss.push_back("/var/run/nscd/socket"); + } } for (auto& i : ss) { @@ -2860,21 +2958,23 @@ void DerivationGoal::runChild() { DLOG(INFO) << "bind mounting '" << source << "' to '" << target << "'"; struct stat st; if (stat(source.c_str(), &st) == -1) { - if (optional && errno == ENOENT) + if (optional && errno == ENOENT) { return; - else + } else { throw SysError("getting attributes of path '%1%'", source); + } } - if (S_ISDIR(st.st_mode)) + if (S_ISDIR(st.st_mode)) { createDirs(target); - else { + } else { createDirs(dirOf(target)); writeFile(target, ""); } if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == - -1) + -1) { throw SysError("bind mount from '%1%' to '%2%' failed", source, target); + } }; for (auto& i : dirsInChroot) { @@ -2886,15 +2986,18 @@ void DerivationGoal::runChild() { /* Bind a new instance of procfs on /proc. */ createDirs(chrootRootDir + "/proc"); - if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == -1) + if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == + -1) { throw SysError("mounting /proc"); + } /* Mount a new tmpfs on /dev/shm to ensure that whatever the builder puts in /dev/shm is cleaned up automatically. */ if (pathExists("/dev/shm") && mount("none", (chrootRootDir + "/dev/shm").c_str(), "tmpfs", 0, - fmt("size=%s", settings.sandboxShmSize).c_str()) == -1) + fmt("size=%s", settings.sandboxShmSize).c_str()) == -1) { throw SysError("mounting /dev/shm"); + } /* Mount a new devpts on /dev/pts. Note that this requires the kernel to be compiled with @@ -2920,26 +3023,32 @@ void DerivationGoal::runChild() { } /* Do the chroot(). */ - if (chdir(chrootRootDir.c_str()) == -1) + if (chdir(chrootRootDir.c_str()) == -1) { throw SysError(format("cannot change directory to '%1%'") % chrootRootDir); + } - if (mkdir("real-root", 0) == -1) + if (mkdir("real-root", 0) == -1) { throw SysError("cannot create real-root directory"); + } - if (pivot_root(".", "real-root") == -1) + if (pivot_root(".", "real-root") == -1) { throw SysError(format("cannot pivot old root directory onto '%1%'") % (chrootRootDir + "/real-root")); + } - if (chroot(".") == -1) + if (chroot(".") == -1) { throw SysError(format("cannot change root directory to '%1%'") % chrootRootDir); + } - if (umount2("real-root", MNT_DETACH) == -1) + if (umount2("real-root", MNT_DETACH) == -1) { throw SysError("cannot unmount real root filesystem"); + } - if (rmdir("real-root") == -1) + if (rmdir("real-root") == -1) { throw SysError("cannot remove real-root directory"); + } /* Switch to the sandbox uid/gid in the user namespace, which corresponds to the build user or calling user in @@ -2955,8 +3064,9 @@ void DerivationGoal::runChild() { } #endif - if (chdir(tmpDirInSandbox.c_str()) == -1) + if (chdir(tmpDirInSandbox.c_str()) == -1) { throw SysError(format("changing into '%1%'") % tmpDir); + } /* Close all other file descriptors. */ closeMostFDs({STDIN_FILENO, STDOUT_FILENO, STDERR_FILENO}); @@ -2970,8 +3080,9 @@ void DerivationGoal::runChild() { (settings.thisSystem == "x86_64-linux" || (!strcmp(utsbuf.sysname, "Linux") && !strcmp(utsbuf.machine, "x86_64")))) { - if (personality(PER_LINUX32) == -1) + if (personality(PER_LINUX32) == -1) { throw SysError("cannot set i686-linux personality"); + } } /* Impersonate a Linux 2.6 machine to get some determinism in @@ -3000,9 +3111,10 @@ void DerivationGoal::runChild() { /* Fill in the environment. */ Strings envStrs; - for (auto& i : env) + for (auto& i : env) { envStrs.push_back( rewriteStrings(i.first + "=" + i.second, inputRewrites)); + } /* If we are running in `build-users' mode, then switch to the user we allocated above. Make sure that we drop all root @@ -3015,16 +3127,19 @@ void DerivationGoal::runChild() { admins to specify groups such as "kvm". */ if (!buildUser->getSupplementaryGIDs().empty() && setgroups(buildUser->getSupplementaryGIDs().size(), - buildUser->getSupplementaryGIDs().data()) == -1) + buildUser->getSupplementaryGIDs().data()) == -1) { throw SysError("cannot set supplementary groups of build user"); + } if (setgid(buildUser->getGID()) == -1 || - getgid() != buildUser->getGID() || getegid() != buildUser->getGID()) + getgid() != buildUser->getGID() || getegid() != buildUser->getGID()) { throw SysError("setgid failed"); + } if (setuid(buildUser->getUID()) == -1 || - getuid() != buildUser->getUID() || geteuid() != buildUser->getUID()) + getuid() != buildUser->getUID() || geteuid() != buildUser->getUID()) { throw SysError("setuid failed"); + } } /* Fill in the arguments. */ @@ -3190,16 +3305,18 @@ void DerivationGoal::runChild() { if (drv->isBuiltin()) { try { BasicDerivation drv2(*drv); - for (auto& e : drv2.env) + for (auto& e : drv2.env) { e.second = rewriteStrings(e.second, inputRewrites); + } - if (drv->builder == "builtin:fetchurl") + if (drv->builder == "builtin:fetchurl") { builtinFetchurl(drv2, netrcData); - else if (drv->builder == "builtin:buildenv") + } else if (drv->builder == "builtin:buildenv") { builtinBuildenv(drv2); - else + } else { throw Error(format("unsupported builtin function '%1%'") % string(drv->builder, 8)); + } _exit(0); } catch (std::exception& e) { writeFull(STDERR_FILENO, "error: " + string(e.what()) + "\n"); @@ -3226,14 +3343,15 @@ PathSet parseReferenceSpecifiers(Store& store, const BasicDerivation& drv, const Strings& paths) { PathSet result; for (auto& i : paths) { - if (store.isStorePath(i)) + if (store.isStorePath(i)) { result.insert(i); - else if (drv.outputs.find(i) != drv.outputs.end()) + } else if (drv.outputs.find(i) != drv.outputs.end()) { result.insert(drv.outputs.find(i)->second.path); - else + } else { throw BuildError( format("derivation contains an illegal reference specifier '%1%'") % i); + } } return result; } @@ -3244,10 +3362,11 @@ void DerivationGoal::registerOutputs() { to do anything here. */ if (hook) { bool allValid = true; - for (auto& i : drv->outputs) + for (auto& i : drv->outputs) { if (!worker.store.isValidPath(i.second.path)) { allValid = false; } + } if (allValid) { return; } @@ -3281,14 +3400,15 @@ void DerivationGoal::registerOutputs() { actualPath = chrootRootDir + path; if (pathExists(actualPath)) { /* Move output paths from the chroot to the Nix store. */ - if (buildMode == bmRepair) + if (buildMode == bmRepair) { replaceValidPath(path, actualPath); - else if (buildMode != bmCheck && - rename(actualPath.c_str(), - worker.store.toRealPath(path).c_str()) == -1) + } else if (buildMode != bmCheck && + rename(actualPath.c_str(), + worker.store.toRealPath(path).c_str()) == -1) { throw SysError(format("moving build output '%1%' from the sandbox to " "the Nix store") % path); + } } if (buildMode != bmCheck) { actualPath = worker.store.toRealPath(path); @@ -3299,8 +3419,9 @@ void DerivationGoal::registerOutputs() { Path redirected = redirectedOutputs[path]; if (buildMode == bmRepair && redirectedBadOutputs.find(path) != redirectedBadOutputs.end() && - pathExists(redirected)) + pathExists(redirected)) { replaceValidPath(path, redirected); + } if (buildMode == bmCheck && redirected != "") { actualPath = redirected; } @@ -3308,10 +3429,11 @@ void DerivationGoal::registerOutputs() { struct stat st; if (lstat(actualPath.c_str(), &st) == -1) { - if (errno == ENOENT) + if (errno == ENOENT) { throw BuildError( format("builder for '%1%' failed to produce output path '%2%'") % drvPath % path); + } throw SysError(format("getting attributes of path '%1%'") % actualPath); } @@ -3321,10 +3443,11 @@ void DerivationGoal::registerOutputs() { build. Also, the output should be owned by the build user. */ if ((!S_ISLNK(st.st_mode) && (st.st_mode & (S_IWGRP | S_IWOTH))) || - (buildUser && st.st_uid != buildUser->getUID())) + (buildUser && st.st_uid != buildUser->getUID())) { throw BuildError(format("suspicious ownership or permission on '%1%'; " "rejecting this build output") % path); + } #endif /* Apply hash rewriting if necessary. */ @@ -3360,11 +3483,12 @@ void DerivationGoal::registerOutputs() { if (!recursive) { /* The output path should be a regular file without execute permission. */ - if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) + if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) { throw BuildError( format( "output path '%1%' should be a non-executable regular file") % path); + } } /* Check the hash. In hash mode, move the path produced by @@ -3386,20 +3510,23 @@ void DerivationGoal::registerOutputs() { Path actualDest = worker.store.toRealPath(dest); - if (worker.store.isValidPath(dest)) + if (worker.store.isValidPath(dest)) { std::rethrow_exception(delayedException); + } if (actualPath != actualDest) { PathLocks outputLocks({actualDest}); deletePath(actualDest); - if (rename(actualPath.c_str(), actualDest.c_str()) == -1) + if (rename(actualPath.c_str(), actualDest.c_str()) == -1) { throw SysError(format("moving '%1%' to '%2%'") % actualPath % dest); + } } path = dest; actualPath = actualDest; - } else + } else { assert(path == dest); + } info.ca = makeFixedOutputCA(recursive, h2); } @@ -3428,9 +3555,10 @@ void DerivationGoal::registerOutputs() { if (settings.runDiffHook || settings.keepFailed) { Path dst = worker.store.toRealPath(path + checkSuffix); deletePath(dst); - if (rename(actualPath.c_str(), dst.c_str())) + if (rename(actualPath.c_str(), dst.c_str())) { throw SysError(format("renaming '%1%' to '%2%'") % actualPath % dst); + } handleDiffHook(buildUser ? buildUser->getUID() : getuid(), buildUser ? buildUser->getGID() : getgid(), path, dst, @@ -3440,10 +3568,11 @@ void DerivationGoal::registerOutputs() { format("derivation '%1%' may not be deterministic: output '%2%' " "differs from '%3%'") % drvPath % path % dst); - } else + } else { throw NotDeterministic(format("derivation '%1%' may not be " "deterministic: output '%2%' differs") % drvPath % path); + } } /* Since we verified the build, it's now ultimately @@ -3461,10 +3590,11 @@ void DerivationGoal::registerOutputs() { paths. */ for (auto& i : inputPaths) { PathSet::iterator j = references.find(i); - if (j == references.end()) + if (j == references.end()) { DLOG(INFO) << "unreferenced input: '" << i << "'"; - else + } else { DLOG(INFO) << "referenced input: '" << i << "'"; + } } if (curRound == nrRounds) { @@ -3500,7 +3630,7 @@ void DerivationGoal::registerOutputs() { if (curRound > 1 && prevInfos != infos) { assert(prevInfos.size() == infos.size()); for (auto i = prevInfos.begin(), j = infos.begin(); i != prevInfos.end(); - ++i, ++j) + ++i, ++j) { if (!(*i == *j)) { result.isNonDeterministic = true; Path prev = i->second.path + checkSuffix; @@ -3524,6 +3654,7 @@ void DerivationGoal::registerOutputs() { LOG(ERROR) << msg; curRound = nrRounds; // we know enough, bail out early } + } } /* If this is the first round of several, then move the output out @@ -3534,8 +3665,9 @@ void DerivationGoal::registerOutputs() { Path prev = i.second.path + checkSuffix; deletePath(prev); Path dst = i.second.path + checkSuffix; - if (rename(i.second.path.c_str(), dst.c_str())) + if (rename(i.second.path.c_str(), dst.c_str())) { throw SysError(format("renaming '%1%' to '%2%'") % i.second.path % dst); + } } } @@ -3574,8 +3706,9 @@ void DerivationGoal::registerOutputs() { void DerivationGoal::checkOutputs( const std::map<Path, ValidPathInfo>& outputs) { std::map<Path, const ValidPathInfo&> outputsByPath; - for (auto& output : outputs) + for (auto& output : outputs) { outputsByPath.emplace(output.second.path, output.second); + } for (auto& output : outputs) { auto& outputName = output.first; @@ -3623,18 +3756,20 @@ void DerivationGoal::checkOutputs( }; auto applyChecks = [&](const Checks& checks) { - if (checks.maxSize && info.narSize > *checks.maxSize) + if (checks.maxSize && info.narSize > *checks.maxSize) { throw BuildError( "path '%s' is too large at %d bytes; limit is %d bytes", info.path, info.narSize, *checks.maxSize); + } if (checks.maxClosureSize) { uint64_t closureSize = getClosure(info.path).second; - if (closureSize > *checks.maxClosureSize) + if (closureSize > *checks.maxClosureSize) { throw BuildError( "closure of path '%s' is too large at %d bytes; limit is %d " "bytes", info.path, closureSize, *checks.maxClosureSize); + } } auto checkRefs = [&](const std::optional<Strings>& value, bool allowed, @@ -3654,7 +3789,7 @@ void DerivationGoal::checkOutputs( PathSet badPaths; - for (auto& i : used) + for (auto& i : used) { if (allowed) { if (!spec.count(i)) { badPaths.insert(i); @@ -3664,6 +3799,7 @@ void DerivationGoal::checkOutputs( badPaths.insert(i); } } + } if (!badPaths.empty()) { string badPathsStr; @@ -3692,23 +3828,26 @@ void DerivationGoal::checkOutputs( Checks checks; auto maxSize = output->find("maxSize"); - if (maxSize != output->end()) + if (maxSize != output->end()) { checks.maxSize = maxSize->get<uint64_t>(); + } auto maxClosureSize = output->find("maxClosureSize"); - if (maxClosureSize != output->end()) + if (maxClosureSize != output->end()) { checks.maxClosureSize = maxClosureSize->get<uint64_t>(); + } auto get = [&](const std::string& name) -> std::optional<Strings> { auto i = output->find(name); if (i != output->end()) { Strings res; for (auto j = i->begin(); j != i->end(); ++j) { - if (!j->is_string()) + if (!j->is_string()) { throw Error( "attribute '%s' of derivation '%s' must be a list of " "strings", name, drvPath); + } res.push_back(j->get<std::string>()); } checks.disallowedRequisites = res; @@ -3759,16 +3898,18 @@ Path DerivationGoal::openLogFile() { fdLogFile = open(logFileName.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0666); - if (!fdLogFile) + if (!fdLogFile) { throw SysError(format("creating log file '%1%'") % logFileName); + } logFileSink = std::make_shared<FdSink>(fdLogFile.get()); - if (settings.compressLog) + if (settings.compressLog) { logSink = std::shared_ptr<CompressionSink>( makeCompressionSink("bzip2", *logFileSink)); - else + } else { logSink = logFileSink; + } return logFileName; } @@ -3792,8 +3933,9 @@ void DerivationGoal::deleteTmpDir(bool force) { if (settings.keepFailed && !force && !drv->isBuiltin()) { LOG(INFO) << "note: keeping build directory '" << tmpDir << "'"; chmod(tmpDir.c_str(), 0755); - } else + } else { deletePath(tmpDir); + } tmpDir = ""; } } @@ -3811,27 +3953,32 @@ void DerivationGoal::handleChildOutput(int fd, const string& data) { return; } - for (auto c : data) + for (auto c : data) { if (c == '\r') { currentLogLinePos = 0; } else if (c == '\n') { flushLine(); } else { - if (currentLogLinePos >= currentLogLine.size()) + if (currentLogLinePos >= currentLogLine.size()) { currentLogLine.resize(currentLogLinePos + 1); + } currentLogLine[currentLogLinePos++] = c; } + } - if (logSink) (*logSink)(data); + if (logSink) { + (*logSink)(data); + } } if (hook && fd == hook->fromHook.readSide.get()) { - for (auto c : data) + for (auto c : data) { if (c == '\n') { currentHookLine.clear(); } else { currentHookLine += c; } + } } } @@ -3843,9 +3990,10 @@ void DerivationGoal::handleEOF(int fd) { } void DerivationGoal::flushLine() { - if (settings.verboseBuild && (settings.printRepeatedBuilds || curRound == 1)) + if (settings.verboseBuild && + (settings.printRepeatedBuilds || curRound == 1)) { LOG(INFO) << currentLogLine; - else { + } else { logTail.push_back(currentLogLine); if (logTail.size() > settings.logLines) { logTail.pop_front(); @@ -3893,8 +4041,9 @@ void DerivationGoal::done(BuildResult::Status status, const string& msg) { if (result.status == BuildResult::TimedOut) { worker.timedOut = true; } - if (result.status == BuildResult::PermanentFailure) + if (result.status == BuildResult::PermanentFailure) { worker.permanentFailure = true; + } mcExpectedBuilds.reset(); mcRunningBuilds.reset(); @@ -4021,11 +4170,12 @@ void SubstitutionGoal::init() { return; } - if (settings.readOnlyMode) + if (settings.readOnlyMode) { throw Error( format( "cannot substitute path '%1%' - no write access to the Nix store") % storePath); + } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>(); @@ -4110,14 +4260,17 @@ void SubstitutionGoal::tryNext() { /* To maintain the closure invariant, we first have to realise the paths referenced by this one. */ - for (auto& i : info->references) - if (i != storePath) /* ignore self-references */ + for (auto& i : info->references) { + if (i != storePath) { /* ignore self-references */ addWaitee(worker.makeSubstitutionGoal(i)); + } + } - if (waitees.empty()) /* to prevent hang (no wake-up event) */ + if (waitees.empty()) { /* to prevent hang (no wake-up event) */ referencesValid(); - else + } else { state = &SubstitutionGoal::referencesValid; + } } void SubstitutionGoal::referencesValid() { @@ -4131,9 +4284,11 @@ void SubstitutionGoal::referencesValid() { return; } - for (auto& i : info->references) - if (i != storePath) /* ignore self-references */ + for (auto& i : info->references) { + if (i != storePath) { /* ignore self-references */ assert(worker.store.isValidPath(i)); + } + } state = &SubstitutionGoal::tryToRun; worker.wakeUp(shared_from_this()); @@ -4304,14 +4459,16 @@ GoalPtr Worker::makeSubstitutionGoal(const Path& path, RepairFlag repair) { static void removeGoal(GoalPtr goal, WeakGoalMap& goalMap) { /* !!! inefficient */ - for (WeakGoalMap::iterator i = goalMap.begin(); i != goalMap.end();) + for (WeakGoalMap::iterator i = goalMap.begin(); i != goalMap.end();) { if (i->second.lock() == goal) { WeakGoalMap::iterator j = i; ++j; goalMap.erase(i); i = j; - } else + } else { ++i; + } + } } void Worker::removeGoal(GoalPtr goal) { @@ -4321,8 +4478,9 @@ void Worker::removeGoal(GoalPtr goal) { topGoals.erase(goal); /* If a top-level goal failed, then kill all other goals (unless keepGoing was set). */ - if (goal->getExitCode() == Goal::ecFailed && !settings.keepGoing) + if (goal->getExitCode() == Goal::ecFailed && !settings.keepGoing) { topGoals.clear(); + } } /* Wake up goals waiting for any goal to finish. */ @@ -4388,10 +4546,11 @@ void Worker::childTerminated(Goal* goal, bool wakeSleepers) { void Worker::waitForBuildSlot(GoalPtr goal) { DLOG(INFO) << "wait for build slot"; - if (getNrLocalBuilds() < settings.maxBuildJobs) + if (getNrLocalBuilds() < settings.maxBuildJobs) { wakeUp(goal); /* we can do it right away */ - else + } else { addToWeakGoals(wantingToBuild, goal); + } } void Worker::waitForAnyGoal(GoalPtr goal) { @@ -4441,13 +4600,14 @@ void Worker::run(const Goals& _topGoals) { } /* Wait for input. */ - if (!children.empty() || !waitingForAWhile.empty()) + if (!children.empty() || !waitingForAWhile.empty()) { waitForInput(); - else { - if (awake.empty() && 0 == settings.maxBuildJobs) + } else { + if (awake.empty() && 0 == settings.maxBuildJobs) { throw Error( "unable to start any build; either increase '--max-jobs' " "or enable remote builds"); + } assert(!awake.empty()); } } @@ -4478,19 +4638,22 @@ void Worker::waitForInput() { is a build timeout, then wait for input until the first deadline for any child. */ auto nearest = steady_time_point::max(); // nearest deadline - if (settings.minFree.get() != 0) + if (settings.minFree.get() != 0) { // Periodicallty wake up to see if we need to run the garbage collector. nearest = before + std::chrono::seconds(10); + } for (auto& i : children) { if (!i.respectTimeouts) { continue; } - if (0 != settings.maxSilentTime) + if (0 != settings.maxSilentTime) { nearest = std::min( nearest, i.lastOutput + std::chrono::seconds(settings.maxSilentTime)); - if (0 != settings.buildTimeout) + } + if (0 != settings.buildTimeout) { nearest = std::min( nearest, i.timeStarted + std::chrono::seconds(settings.buildTimeout)); + } } if (nearest != steady_time_point::max()) { timeout.tv_sec = std::max( @@ -4504,17 +4667,20 @@ void Worker::waitForInput() { up after a few seconds at most. */ if (!waitingForAWhile.empty()) { useTimeout = true; - if (lastWokenUp == steady_time_point::min()) + if (lastWokenUp == steady_time_point::min()) { DLOG(WARNING) << "waiting for locks or build slots..."; - if (lastWokenUp == steady_time_point::min() || lastWokenUp > before) + } + if (lastWokenUp == steady_time_point::min() || lastWokenUp > before) { lastWokenUp = before; + } timeout.tv_sec = std::max( 1L, (long)std::chrono::duration_cast<std::chrono::seconds>( lastWokenUp + std::chrono::seconds(settings.pollInterval) - before) .count()); - } else + } else { lastWokenUp = steady_time_point::min(); + } if (useTimeout) { DLOG(INFO) << "sleeping " << timeout.tv_sec << " seconds"; @@ -4570,8 +4736,9 @@ void Worker::waitForInput() { goal->handleEOF(k); j->fds.erase(k); } else if (rd == -1) { - if (errno != EINTR) + if (errno != EINTR) { throw SysError("%s: read failed", goal->getName()); + } } else { DLOG(INFO) << goal->getName() << ": read " << rd << " bytes"; string data((char*)buffer.data(), rd); @@ -4676,12 +4843,14 @@ static void primeCache(Store& store, const PathSet& paths) { store.queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); - if (!willBuild.empty() && 0 == settings.maxBuildJobs && getMachines().empty()) + if (!willBuild.empty() && 0 == settings.maxBuildJobs && + getMachines().empty()) { throw Error( "%d derivations need to be built, but neither local builds " "('--max-jobs') " "nor remote builds ('--builders') are enabled", willBuild.size()); + } } void LocalStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) { @@ -4692,11 +4861,12 @@ void LocalStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) { Goals goals; for (auto& i : drvPaths) { DrvPathWithOutputs i2 = parseDrvPathWithOutputs(i); - if (isDerivation(i2.first)) + if (isDerivation(i2.first)) { goals.insert(worker.makeDerivationGoal(i2.first, i2.second, buildMode)); - else + } else { goals.insert(worker.makeSubstitutionGoal( i, buildMode == bmRepair ? Repair : NoRepair)); + } } worker.run(goals); @@ -4705,15 +4875,17 @@ void LocalStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) { for (auto& i : goals) { if (i->getExitCode() != Goal::ecSuccess) { DerivationGoal* i2 = dynamic_cast<DerivationGoal*>(i.get()); - if (i2) + if (i2) { failed.insert(i2->getDrvPath()); - else + } else { failed.insert(dynamic_cast<SubstitutionGoal*>(i.get())->getStorePath()); + } } } - if (!failed.empty()) + if (!failed.empty()) { throw Error(worker.exitStatus(), "build of %s failed", showPaths(failed)); + } } BuildResult LocalStore::buildDerivation(const Path& drvPath, @@ -4749,9 +4921,10 @@ void LocalStore::ensurePath(const Path& path) { worker.run(goals); - if (goal->getExitCode() != Goal::ecSuccess) + if (goal->getExitCode() != Goal::ecSuccess) { throw Error(worker.exitStatus(), "path '%s' does not exist and cannot be created", path); + } } void LocalStore::repairPath(const Path& path) { @@ -4769,8 +4942,9 @@ void LocalStore::repairPath(const Path& path) { goals.clear(); goals.insert(worker.makeDerivationGoal(deriver, StringSet(), bmRepair)); worker.run(goals); - } else + } else { throw Error(worker.exitStatus(), "cannot repair path '%s'", path); + } } } diff --git a/third_party/nix/src/libstore/crypto.cc b/third_party/nix/src/libstore/crypto.cc index 7a169107c116..f26cabdf6dc7 100644 --- a/third_party/nix/src/libstore/crypto.cc +++ b/third_party/nix/src/libstore/crypto.cc @@ -32,8 +32,9 @@ Key::Key(const string& s) { SecretKey::SecretKey(const string& s) : Key(s) { #if HAVE_SODIUM - if (key.size() != crypto_sign_SECRETKEYBYTES) + if (key.size() != crypto_sign_SECRETKEYBYTES) { throw Error("secret key is not valid"); + } #endif } @@ -69,8 +70,9 @@ PublicKey SecretKey::toPublicKey() const { PublicKey::PublicKey(const string& s) : Key(s) { #if HAVE_SODIUM - if (key.size() != crypto_sign_PUBLICKEYBYTES) + if (key.size() != crypto_sign_PUBLICKEYBYTES) { throw Error("public key is not valid"); + } #endif } diff --git a/third_party/nix/src/libstore/derivations.cc b/third_party/nix/src/libstore/derivations.cc index 51bcfbe1cef7..1b6f160653c9 100644 --- a/third_party/nix/src/libstore/derivations.cc +++ b/third_party/nix/src/libstore/derivations.cc @@ -19,16 +19,18 @@ void DerivationOutput::parseHashInfo(bool& recursive, Hash& hash) const { } HashType hashType = parseHashType(algo); - if (hashType == htUnknown) + if (hashType == htUnknown) { throw Error(format("unknown hash algorithm '%1%'") % algo); + } hash = Hash(this->hash, hashType); } Path BasicDerivation::findOutput(const string& id) const { auto i = outputs.find(id); - if (i == outputs.end()) + if (i == outputs.end()) { throw Error(format("derivation has no output '%1%'") % id); + } return i->second.path; } @@ -57,8 +59,9 @@ Path writeDerivation(ref<Store> store, const Derivation& drv, static void expect(std::istream& str, const string& s) { char s2[s.size()]; str.read(s2, s.size()); - if (string(s2, s.size()) != s) + if (string(s2, s.size()) != s) { throw FormatError(format("expected string '%1%'") % s); + } } /* Read a C-style string from stream `str'. */ @@ -66,26 +69,30 @@ static string parseString(std::istream& str) { string res; expect(str, "\""); int c; - while ((c = str.get()) != '"') + while ((c = str.get()) != '"') { if (c == '\\') { c = str.get(); - if (c == 'n') + if (c == 'n') { res += '\n'; - else if (c == 'r') + } else if (c == 'r') { res += '\r'; - else if (c == 't') + } else if (c == 't') { res += '\t'; - else + } else { res += c; - } else + } + } else { res += c; + } + } return res; } static Path parsePath(std::istream& str) { string s = parseString(str); - if (s.size() == 0 || s[0] != '/') + if (s.size() == 0 || s[0] != '/') { throw FormatError(format("bad path '%1%' in derivation") % s); + } return s; } @@ -103,8 +110,9 @@ static bool endOfList(std::istream& str) { static StringSet parseStrings(std::istream& str, bool arePaths) { StringSet res; - while (!endOfList(str)) + while (!endOfList(str)) { res.insert(arePaths ? parsePath(str) : parseString(str)); + } return res; } @@ -147,7 +155,9 @@ static Derivation parseDerivation(const string& s) { /* Parse the builder arguments. */ expect(str, ",["); - while (!endOfList(str)) drv.args.push_back(parseString(str)); + while (!endOfList(str)) { + drv.args.push_back(parseString(str)); + } /* Parse the environment variables. */ expect(str, ",["); diff --git a/third_party/nix/src/libstore/download.cc b/third_party/nix/src/libstore/download.cc index fbc2069c086d..4b90a3765724 100644 --- a/third_party/nix/src/libstore/download.cc +++ b/third_party/nix/src/libstore/download.cc @@ -36,11 +36,12 @@ DownloadSettings downloadSettings; static GlobalConfig::Register r1(&downloadSettings); std::string resolveUri(const std::string& uri) { - if (uri.compare(0, 8, "channel:") == 0) + if (uri.compare(0, 8, "channel:") == 0) { return "https://nixos.org/channels/" + std::string(uri, 8) + "/nixexprs.tar.xz"; - else + } else { return uri; + } } struct CurlDownloader : public Downloader { @@ -92,18 +93,21 @@ struct CurlDownloader : public Downloader { writtenToSink += len; this->request.dataCallback((char*)data, len); } - } else + } else { this->result.data->append((char*)data, len); + } }) { LOG(INFO) << (request.data ? "uploading '" : "downloading '") << request.uri << "'"; - if (!request.expectedETag.empty()) + if (!request.expectedETag.empty()) { requestHeaders = curl_slist_append( requestHeaders, ("If-None-Match: " + request.expectedETag).c_str()); - if (!request.mimeType.empty()) + } + if (!request.mimeType.empty()) { requestHeaders = curl_slist_append( requestHeaders, ("Content-Type: " + request.mimeType).c_str()); + } } ~DownloadItem() { @@ -117,10 +121,11 @@ struct CurlDownloader : public Downloader { curl_slist_free_all(requestHeaders); } try { - if (!done) + if (!done) { fail(DownloadError( Interrupted, format("download of '%s' was interrupted") % request.uri)); + } } catch (...) { ignoreException(); } @@ -147,8 +152,9 @@ struct CurlDownloader : public Downloader { size_t realSize = size * nmemb; result.bodySize += realSize; - if (!decompressionSink) + if (!decompressionSink) { decompressionSink = makeDecompressionSink(encoding, finalSink); + } (*decompressionSink)((unsigned char*)contents, realSize); @@ -192,11 +198,12 @@ struct CurlDownloader : public Downloader { << "shutting down on 200 HTTP response with expected ETag"; return 0; } - } else if (name == "content-encoding") + } else if (name == "content-encoding") { encoding = trim(string(line, i + 1)); - else if (name == "accept-ranges" && - toLower(trim(std::string(line, i + 1))) == "bytes") + } else if (name == "accept-ranges" && + toLower(trim(std::string(line, i + 1))) == "bytes") { acceptRanges = true; + } } } return realSize; @@ -275,10 +282,11 @@ struct CurlDownloader : public Downloader { curl_easy_setopt(req, CURLOPT_PIPEWAIT, 1); #endif #if LIBCURL_VERSION_NUM >= 0x072f00 - if (downloadSettings.enableHttp2) + if (downloadSettings.enableHttp2) { curl_easy_setopt(req, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_2TLS); - else + } else { curl_easy_setopt(req, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1); + } #endif curl_easy_setopt(req, CURLOPT_WRITEFUNCTION, DownloadItem::writeCallbackWrapper); @@ -306,8 +314,9 @@ struct CurlDownloader : public Downloader { } if (request.verifyTLS) { - if (settings.caFile != "") + if (settings.caFile != "") { curl_easy_setopt(req, CURLOPT_CAINFO, settings.caFile.c_str()); + } } else { curl_easy_setopt(req, CURLOPT_SSL_VERIFYPEER, 0); curl_easy_setopt(req, CURLOPT_SSL_VERIFYHOST, 0); @@ -362,14 +371,14 @@ struct CurlDownloader : public Downloader { httpStatus = 304; } - if (writeException) + if (writeException) { failEx(writeException); - else if (code == CURLE_OK && - (httpStatus == 200 || httpStatus == 201 || httpStatus == 204 || - httpStatus == 206 || httpStatus == 304 || - httpStatus == 226 /* FTP */ || - httpStatus == 0 /* other protocol */)) { + } else if (code == CURLE_OK && + (httpStatus == 200 || httpStatus == 201 || httpStatus == 204 || + httpStatus == 206 || httpStatus == 304 || + httpStatus == 226 /* FTP */ || + httpStatus == 0 /* other protocol */)) { result.cached = httpStatus == 304; done = true; callback(std::move(result)); @@ -464,8 +473,9 @@ struct CurlDownloader : public Downloader { embargo = std::chrono::steady_clock::now() + std::chrono::milliseconds(ms); downloader.enqueueItem(shared_from_this()); - } else + } else { fail(exc); + } } } }; @@ -548,10 +558,11 @@ struct CurlDownloader : public Downloader { /* Let curl do its thing. */ int running; CURLMcode mc = curl_multi_perform(curlm, &running); - if (mc != CURLM_OK) + if (mc != CURLM_OK) { throw nix::Error( format("unexpected error from curl_multi_perform(): %s") % curl_multi_strerror(mc)); + } /* Set the promises of any finished requests. */ CURLMsg* msg; @@ -584,9 +595,10 @@ struct CurlDownloader : public Downloader { : maxSleepTimeMs; DLOG(INFO) << "download thread waiting for " << sleepTimeMs << " ms"; mc = curl_multi_wait(curlm, extraFDs, 1, sleepTimeMs, &numfds); - if (mc != CURLM_OK) + if (mc != CURLM_OK) { throw nix::Error(format("unexpected error from curl_multi_wait(): %s") % curl_multi_strerror(mc)); + } nextWakeup = std::chrono::steady_clock::time_point(); @@ -596,8 +608,9 @@ struct CurlDownloader : public Downloader { if (extraFDs[0].revents & CURL_WAIT_POLLIN) { char buf[1024]; auto res = read(extraFDs[0].fd, buf, sizeof(buf)); - if (res == -1 && errno != EINTR) + if (res == -1 && errno != EINTR) { throw SysError("reading curl wakeup socket"); + } } std::vector<std::shared_ptr<DownloadItem>> incoming; @@ -612,8 +625,9 @@ struct CurlDownloader : public Downloader { state->incoming.pop(); } else { if (nextWakeup == std::chrono::steady_clock::time_point() || - item->embargo < nextWakeup) + item->embargo < nextWakeup) { nextWakeup = item->embargo; + } break; } } @@ -643,22 +657,26 @@ struct CurlDownloader : public Downloader { { auto state(state_.lock()); - while (!state->incoming.empty()) state->incoming.pop(); + while (!state->incoming.empty()) { + state->incoming.pop(); + } state->quit = true; } } void enqueueItem(std::shared_ptr<DownloadItem> item) { if (item->request.data && !hasPrefix(item->request.uri, "http://") && - !hasPrefix(item->request.uri, "https://")) + !hasPrefix(item->request.uri, "https://")) { throw nix::Error("uploading to '%s' is not supported", item->request.uri); + } { auto state(state_.lock()); - if (state->quit) + if (state->quit) { throw nix::Error( "cannot enqueue download request because the download thread is " "shutting down"); + } state->incoming.push(item); } writeFull(wakeupPipe.writeSide.get(), " "); @@ -900,8 +918,9 @@ CachedDownloadResult Downloader::downloadCached( expectedETag = ss[1]; } } - } else + } else { storePath = ""; + } } if (!skip) { diff --git a/third_party/nix/src/libstore/export-import.cc b/third_party/nix/src/libstore/export-import.cc index 930ff90d269e..6a1862492d1f 100644 --- a/third_party/nix/src/libstore/export-import.cc +++ b/third_party/nix/src/libstore/export-import.cc @@ -46,9 +46,10 @@ void Store::exportPath(const Path& path, Sink& sink) { filesystem corruption from spreading to other machines. Don't complain if the stored hash is zero (unknown). */ Hash hash = hashAndWriteSink.currentHash(); - if (hash != info->narHash && info->narHash != Hash(info->narHash.type)) + if (hash != info->narHash && info->narHash != Hash(info->narHash.type)) { throw Error(format("hash of path '%1%' has changed from '%2%' to '%3%'!") % path % info->narHash.to_string() % hash.to_string()); + } hashAndWriteSink << exportMagic << path << info->references << info->deriver << 0; @@ -62,17 +63,19 @@ Paths Store::importPaths(Source& source, std::shared_ptr<FSAccessor> accessor, if (n == 0) { break; } - if (n != 1) + if (n != 1) { throw Error( "input doesn't look like something created by 'nix-store --export'"); + } /* Extract the NAR from the source. */ TeeSink tee(source); parseDump(tee, tee.source); uint32_t magic = readInt(source); - if (magic != exportMagic) + if (magic != exportMagic) { throw Error("Nix archive cannot be imported; wrong format"); + } ValidPathInfo info; diff --git a/third_party/nix/src/libstore/gc.cc b/third_party/nix/src/libstore/gc.cc index c7f2a507d367..3e79e6c336aa 100644 --- a/third_party/nix/src/libstore/gc.cc +++ b/third_party/nix/src/libstore/gc.cc @@ -61,8 +61,9 @@ static void makeSymlink(const Path& link, const Path& target) { createSymlink(target, tempLink); /* Atomically replace the old one. */ - if (rename(tempLink.c_str(), link.c_str()) == -1) + if (rename(tempLink.c_str(), link.c_str()) == -1) { throw SysError(format("cannot rename '%1%' to '%2%'") % tempLink % link); + } } void LocalStore::syncWithGC() { AutoCloseFD fdGCLock = openGCLock(ltRead); } @@ -80,18 +81,21 @@ Path LocalFSStore::addPermRoot(const Path& _storePath, const Path& _gcRoot, Path gcRoot(canonPath(_gcRoot)); assertStorePath(storePath); - if (isInStore(gcRoot)) + if (isInStore(gcRoot)) { throw Error(format("creating a garbage collector root (%1%) in the Nix " "store is forbidden " "(are you running nix-build inside the store?)") % gcRoot); + } if (indirect) { /* Don't clobber the link if it already exists and doesn't point to the Nix store. */ - if (pathExists(gcRoot) && (!isLink(gcRoot) || !isInStore(readLink(gcRoot)))) + if (pathExists(gcRoot) && + (!isLink(gcRoot) || !isInStore(readLink(gcRoot)))) { throw Error(format("cannot create symlink '%1%'; already exists") % gcRoot); + } makeSymlink(gcRoot, storePath); addIndirectRoot(gcRoot); } @@ -101,16 +105,18 @@ Path LocalFSStore::addPermRoot(const Path& _storePath, const Path& _gcRoot, Path rootsDir = canonPath((format("%1%/%2%") % stateDir % gcRootsDir).str()); - if (string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/") + if (string(gcRoot, 0, rootsDir.size() + 1) != rootsDir + "/") { throw Error(format("path '%1%' is not a valid garbage collector root; " "it's not in the directory '%2%'") % gcRoot % rootsDir); + } } - if (baseNameOf(gcRoot) == baseNameOf(storePath)) + if (baseNameOf(gcRoot) == baseNameOf(storePath)) { writeFile(gcRoot, ""); - else + } else { makeSymlink(gcRoot, storePath); + } } /* Check that the root can be found by the garbage collector. @@ -144,10 +150,11 @@ void LocalStore::addTempRoot(const Path& path) { while (1) { AutoCloseFD fdGCLock = openGCLock(ltRead); - if (pathExists(fnTempRoots)) + if (pathExists(fnTempRoots)) { /* It *must* be stale, since there can be no two processes with the same pid. */ unlink(fnTempRoots.c_str()); + } state->fdTempRoots = openLockFile(fnTempRoots, true); @@ -159,8 +166,9 @@ void LocalStore::addTempRoot(const Path& path) { /* Check whether the garbage collector didn't get in our way. */ struct stat st; - if (fstat(state->fdTempRoots.get(), &st) == -1) + if (fstat(state->fdTempRoots.get(), &st) == -1) { throw SysError(format("statting '%1%'") % fnTempRoots); + } if (st.st_size == 0) { break; } @@ -245,11 +253,12 @@ void LocalStore::findTempRoots(FDs& fds, Roots& tempRoots, bool censor) { void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) { auto foundRoot = [&](const Path& path, const Path& target) { Path storePath = toStorePath(target); - if (isStorePath(storePath) && isValidPath(storePath)) + if (isStorePath(storePath) && isValidPath(storePath)) { roots[storePath].emplace(path); - else + } else { LOG(INFO) << "skipping invalid root from '" << path << "' to '" << storePath << "'"; + } }; try { @@ -258,8 +267,9 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) { } if (type == DT_DIR) { - for (auto& i : readDirectory(path)) + for (auto& i : readDirectory(path)) { findRoots(path + "/" + i.name, i.type, roots); + } } else if (type == DT_LNK) { @@ -292,8 +302,9 @@ void LocalStore::findRoots(const Path& path, unsigned char type, Roots& roots) { else if (type == DT_REG) { Path storePath = storeDir + "/" + baseNameOf(path); - if (isStorePath(storePath) && isValidPath(storePath)) + if (isStorePath(storePath) && isValidPath(storePath)) { roots[storePath].emplace(path); + } } } @@ -395,8 +406,9 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) { } struct dirent* fd_ent; while (errno = 0, fd_ent = readdir(fdDir.get())) { - if (fd_ent->d_name[0] != '.') + if (fd_ent->d_name[0] != '.') { readProcLink(fmt("%s/%s", fdStr, fd_ent->d_name), unchecked); + } } if (errno) { if (errno == ESRCH) { @@ -412,8 +424,9 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) { readFile(mapFile, true), "\n"); for (const auto& line : mapLines) { auto match = std::smatch{}; - if (std::regex_match(line, match, mapRegex)) + if (std::regex_match(line, match, mapRegex)) { unchecked[match[1]].emplace(mapFile); + } } auto envFile = fmt("/proc/%s/environ", ent->d_name); @@ -421,8 +434,9 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) { auto env_end = std::sregex_iterator{}; for (auto i = std::sregex_iterator{envString.begin(), envString.end(), storePathRegex}; - i != env_end; ++i) + i != env_end; ++i) { unchecked[i->str()].emplace(envFile); + } } catch (SysError& e) { if (errno == ENOENT || errno == EACCES || errno == ESRCH) { continue; @@ -467,10 +481,11 @@ void LocalStore::findRuntimeRoots(Roots& roots, bool censor) { Path path = toStorePath(target); if (isStorePath(path) && isValidPath(path)) { DLOG(INFO) << "got additional root " << path; - if (censor) + if (censor) { roots[path].insert(censored); - else + } else { roots[path].insert(links.begin(), links.end()); + } } } } @@ -514,10 +529,11 @@ void LocalStore::deletePathRecursive(GCState& state, const Path& path) { if (isStorePath(path) && isValidPath(path)) { PathSet referrers; queryReferrers(path, referrers); - for (auto& i : referrers) + for (auto& i : referrers) { if (i != path) { deletePathRecursive(state, i); } + } size = queryPathInfo(path)->narSize; invalidatePathChecked(path); } @@ -546,12 +562,14 @@ void LocalStore::deletePathRecursive(GCState& state, const Path& path) { // if the path was not valid, need to determine the actual // size. try { - if (chmod(realPath.c_str(), st.st_mode | S_IWUSR) == -1) + if (chmod(realPath.c_str(), st.st_mode | S_IWUSR) == -1) { throw SysError(format("making '%1%' writable") % realPath); + } Path tmp = trashDir + "/" + baseNameOf(path); - if (rename(realPath.c_str(), tmp.c_str())) + if (rename(realPath.c_str(), tmp.c_str())) { throw SysError(format("unable to rename '%1%' to '%2%'") % realPath % tmp); + } state.bytesInvalidated += size; } catch (SysError& e) { if (e.errNo == ENOSPC) { @@ -560,8 +578,9 @@ void LocalStore::deletePathRecursive(GCState& state, const Path& path) { deleteGarbage(state, realPath); } } - } else + } else { deleteGarbage(state, realPath); + } if (state.results.bytesFreed + state.bytesInvalidated > state.options.maxFreed) { @@ -606,9 +625,11 @@ bool LocalStore::canReachRoot(GCState& state, PathSet& visited, don't delete the derivation if any of the outputs are alive. */ if (state.gcKeepDerivations && isDerivation(path)) { PathSet outputs = queryDerivationOutputs(path); - for (auto& i : outputs) - if (isValidPath(i) && queryPathInfo(i)->deriver == path) + for (auto& i : outputs) { + if (isValidPath(i) && queryPathInfo(i)->deriver == path) { incoming.insert(i); + } + } } /* If keep-outputs is set, then don't delete this path if there @@ -620,12 +641,14 @@ bool LocalStore::canReachRoot(GCState& state, PathSet& visited, } } - for (auto& i : incoming) - if (i != path) + for (auto& i : incoming) { + if (i != path) { if (canReachRoot(state, visited, i)) { state.alive.insert(path); return true; } + } + } return false; } @@ -701,8 +724,9 @@ void LocalStore::removeUnusedLinks(const GCState& state) { Path path = linksDir + "/" + name; struct stat st; - if (lstat(path.c_str(), &st) == -1) + if (lstat(path.c_str(), &st) == -1) { throw SysError(format("statting '%1%'") % path); + } if (st.st_nlink != 1) { actualSize += st.st_size; @@ -807,16 +831,18 @@ void LocalStore::collectGarbage(const GCOptions& options, GCResults& results) { for (auto& i : options.pathsToDelete) { assertStorePath(i); tryToDelete(state, i); - if (state.dead.find(i) == state.dead.end()) + if (state.dead.find(i) == state.dead.end()) { throw Error(format("cannot delete path '%1%' since it is still alive") % i); + } } } else if (options.maxFreed > 0) { - if (state.shouldDelete) + if (state.shouldDelete) { LOG(INFO) << "deleting garbage..."; - else + } else { LOG(ERROR) << "determining live/dead paths..."; + } try { AutoCloseDir dir(opendir(realStoreDir.c_str())); @@ -839,10 +865,11 @@ void LocalStore::collectGarbage(const GCOptions& options, GCResults& results) { continue; } Path path = storeDir + "/" + name; - if (isStorePath(path) && isValidPath(path)) + if (isStorePath(path) && isValidPath(path)) { entries.push_back(path); - else + } else { tryToDelete(state, path); + } } dir.reset(); @@ -897,8 +924,9 @@ void LocalStore::autoGC(bool sync) { static auto fakeFreeSpaceFile = getEnv("_NIX_TEST_FREE_SPACE_FILE", ""); auto getAvail = [this]() -> uint64_t { - if (!fakeFreeSpaceFile.empty()) + if (!fakeFreeSpaceFile.empty()) { return std::stoll(readFile(fakeFreeSpaceFile)); + } struct statvfs st; if (statvfs(realStoreDir.c_str(), &st)) { @@ -922,8 +950,9 @@ void LocalStore::autoGC(bool sync) { auto now = std::chrono::steady_clock::now(); if (now < state->lastGCCheck + - std::chrono::seconds(settings.minFreeCheckInterval)) + std::chrono::seconds(settings.minFreeCheckInterval)) { return; + } auto avail = getAvail(); diff --git a/third_party/nix/src/libstore/globals.cc b/third_party/nix/src/libstore/globals.cc index 19a0cde1ed2e..ade200f135ae 100644 --- a/third_party/nix/src/libstore/globals.cc +++ b/third_party/nix/src/libstore/globals.cc @@ -49,11 +49,12 @@ Settings::Settings() if (caFile == "") { for (auto& fn : {"/etc/ssl/certs/ca-certificates.crt", - "/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt"}) + "/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt"}) { if (pathExists(fn)) { caFile = fn; break; } + } } /* Backwards compatibility. */ @@ -112,26 +113,28 @@ const string nixVersion = PACKAGE_VERSION; template <> void BaseSetting<SandboxMode>::set(const std::string& str) { - if (str == "true") + if (str == "true") { value = smEnabled; - else if (str == "relaxed") + } else if (str == "relaxed") { value = smRelaxed; - else if (str == "false") + } else if (str == "false") { value = smDisabled; - else + } else { throw UsageError("option '%s' has invalid value '%s'", name, str); + } } template <> std::string BaseSetting<SandboxMode>::to_string() { - if (value == smEnabled) + if (value == smEnabled) { return "true"; - else if (value == smRelaxed) + } else if (value == smRelaxed) { return "relaxed"; - else if (value == smDisabled) + } else if (value == smDisabled) { return "false"; - else + } else { abort(); + } } template <> @@ -160,11 +163,12 @@ void BaseSetting<SandboxMode>::convertToArg(Args& args, } void MaxBuildJobsSetting::set(const std::string& str) { - if (str == "auto") + if (str == "auto") { value = std::max(1U, std::thread::hardware_concurrency()); - else if (!string2Int(str, value)) + } else if (!string2Int(str, value)) { throw UsageError( "configuration setting '%s' should be 'auto' or an integer", name); + } } void initPlugins() { @@ -172,8 +176,9 @@ void initPlugins() { Paths pluginFiles; try { auto ents = readDirectory(pluginFile); - for (const auto& ent : ents) + for (const auto& ent : ents) { pluginFiles.emplace_back(pluginFile + "/" + ent.name); + } } catch (SysError& e) { if (e.errNo != ENOTDIR) { throw; @@ -184,9 +189,10 @@ void initPlugins() { /* handle is purposefully leaked as there may be state in the DSO needed by the action of the plugin. */ void* handle = dlopen(file.c_str(), RTLD_LAZY | RTLD_LOCAL); - if (!handle) + if (!handle) { throw Error("could not dynamically open plugin file '%s': %s", file, dlerror()); + } } } diff --git a/third_party/nix/src/libstore/globals.hh b/third_party/nix/src/libstore/globals.hh index 7b4d614f7a67..924e8a0165f8 100644 --- a/third_party/nix/src/libstore/globals.hh +++ b/third_party/nix/src/libstore/globals.hh @@ -408,17 +408,10 @@ class Settings : public Config { this, true, "print-missing", "Whether to print what paths need to be built or downloaded."}; - Setting<std::string> preBuildHook { - this, -#if __APPLE__ - nixLibexecDir + "/nix/resolve-system-dependencies", -#else - "", -#endif - "pre-build-hook", - "A program to run just before a build to set derivation-specific build " - "settings." - }; + Setting<std::string> preBuildHook{ + this, "", "pre-build-hook", + "A program to run just before a build to set derivation-specific build " + "settings."}; Setting<std::string> postBuildHook{ this, "", "post-build-hook", diff --git a/third_party/nix/src/libstore/http-binary-cache-store.cc b/third_party/nix/src/libstore/http-binary-cache-store.cc index 2482b02c61bd..d3b551f08caa 100644 --- a/third_party/nix/src/libstore/http-binary-cache-store.cc +++ b/third_party/nix/src/libstore/http-binary-cache-store.cc @@ -113,9 +113,10 @@ class HttpBinaryCacheStore : public BinaryCacheStore { try { getDownloader()->download(std::move(request), sink); } catch (DownloadError& e) { - if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden) + if (e.error == Downloader::NotFound || e.error == Downloader::Forbidden) { throw NoSuchBinaryCacheFile( "file '%s' does not exist in binary cache '%s'", path, getUri()); + } maybeDisable(); throw; } @@ -137,8 +138,9 @@ class HttpBinaryCacheStore : public BinaryCacheStore { (*callbackPtr)(result.get().data); } catch (DownloadError& e) { if (e.error == Downloader::NotFound || - e.error == Downloader::Forbidden) + e.error == Downloader::Forbidden) { return (*callbackPtr)(std::shared_ptr<std::string>()); + } maybeDisable(); callbackPtr->rethrow(); } catch (...) { @@ -154,8 +156,9 @@ static RegisterStoreImplementation regStore( if (std::string(uri, 0, 7) != "http://" && std::string(uri, 0, 8) != "https://" && (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") != "1" || - std::string(uri, 0, 7) != "file://")) + std::string(uri, 0, 7) != "file://")) { return 0; + } auto store = std::make_shared<HttpBinaryCacheStore>(params, uri); store->init(); return store; diff --git a/third_party/nix/src/libstore/legacy-ssh-store.cc b/third_party/nix/src/libstore/legacy-ssh-store.cc index 8510895d0bfe..0cfe1bfe5d49 100644 --- a/third_party/nix/src/libstore/legacy-ssh-store.cc +++ b/third_party/nix/src/libstore/legacy-ssh-store.cc @@ -69,12 +69,14 @@ struct LegacySSHStore : public Store { conn->to.flush(); unsigned int magic = readInt(conn->from); - if (magic != SERVE_MAGIC_2) + if (magic != SERVE_MAGIC_2) { throw Error("protocol mismatch with 'nix-store --serve' on '%s'", host); + } conn->remoteVersion = readInt(conn->from); - if (GET_PROTOCOL_MAJOR(conn->remoteVersion) != 0x200) + if (GET_PROTOCOL_MAJOR(conn->remoteVersion) != 0x200) { throw Error("unsupported 'nix-store --serve' protocol version on '%s'", host); + } } catch (EndOfFile& e) { throw Error("cannot connect to '%1%'", host); @@ -160,9 +162,10 @@ struct LegacySSHStore : public Store { conn->to.flush(); } - if (readInt(conn->from) != 1) + if (readInt(conn->from) != 1) { throw Error( "failed to add path '%s' to remote host '%s', info.path, host"); + } } void narFromPath(const Path& path, Sink& sink) override { @@ -194,10 +197,12 @@ struct LegacySSHStore : public Store { conn->to << cmdBuildDerivation << drvPath << drv << settings.maxSilentTime << settings.buildTimeout; - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 2) + if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 2) { conn->to << settings.maxLogSize; - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) + } + if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) { conn->to << settings.buildRepeat << settings.enforceDeterminism; + } conn->to.flush(); @@ -205,9 +210,10 @@ struct LegacySSHStore : public Store { status.status = (BuildResult::Status)readInt(conn->from); conn->from >> status.errorMsg; - if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) + if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3) { conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime; + } return status; } diff --git a/third_party/nix/src/libstore/local-binary-cache-store.cc b/third_party/nix/src/libstore/local-binary-cache-store.cc index 925fb34de48f..29c74cedd834 100644 --- a/third_party/nix/src/libstore/local-binary-cache-store.cc +++ b/third_party/nix/src/libstore/local-binary-cache-store.cc @@ -26,9 +26,10 @@ class LocalBinaryCacheStore : public BinaryCacheStore { try { readFile(binaryCacheDir + "/" + path, sink); } catch (SysError& e) { - if (e.errNo == ENOENT) + if (e.errNo == ENOENT) { throw NoSuchBinaryCacheFile("file '%s' does not exist in binary cache", path); + } } } @@ -36,8 +37,9 @@ class LocalBinaryCacheStore : public BinaryCacheStore { PathSet paths; for (auto& entry : readDirectory(binaryCacheDir)) { - if (entry.name.size() != 40 || !hasSuffix(entry.name, ".narinfo")) + if (entry.name.size() != 40 || !hasSuffix(entry.name, ".narinfo")) { continue; + } paths.insert(storeDir + "/" + entry.name.substr(0, entry.name.size() - 8)); } @@ -55,8 +57,9 @@ static void atomicWrite(const Path& path, const std::string& s) { Path tmp = path + ".tmp." + std::to_string(getpid()); AutoDelete del(tmp, false); writeFile(tmp, s); - if (rename(tmp.c_str(), path.c_str())) + if (rename(tmp.c_str(), path.c_str())) { throw SysError(format("renaming '%1%' to '%2%'") % tmp % path); + } del.cancel(); } @@ -74,8 +77,9 @@ static RegisterStoreImplementation regStore( [](const std::string& uri, const Store::Params& params) -> std::shared_ptr<Store> { if (getEnv("_NIX_FORCE_HTTP_BINARY_CACHE_STORE") == "1" || - std::string(uri, 0, 7) != "file://") + std::string(uri, 0, 7) != "file://") { return 0; + } auto store = std::make_shared<LocalBinaryCacheStore>(params, std::string(uri, 7)); store->init(); diff --git a/third_party/nix/src/libstore/local-fs-store.cc b/third_party/nix/src/libstore/local-fs-store.cc index d9eaea2e93a8..bc282eac96b9 100644 --- a/third_party/nix/src/libstore/local-fs-store.cc +++ b/third_party/nix/src/libstore/local-fs-store.cc @@ -16,9 +16,10 @@ struct LocalStoreAccessor : public FSAccessor { Path toRealPath(const Path& path) { Path storePath = store->toStorePath(path); - if (!store->isValidPath(storePath)) + if (!store->isValidPath(storePath)) { throw InvalidPath(format("path '%1%' is not a valid store path") % storePath); + } return store->getRealStoreDir() + std::string(path, store->storeDir.size()); } @@ -27,13 +28,15 @@ struct LocalStoreAccessor : public FSAccessor { struct stat st; if (lstat(realPath.c_str(), &st)) { - if (errno == ENOENT || errno == ENOTDIR) + if (errno == ENOENT || errno == ENOTDIR) { return {Type::tMissing, 0, false}; + } throw SysError(format("getting status of '%1%'") % path); } - if (!S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode) && !S_ISLNK(st.st_mode)) + if (!S_ISREG(st.st_mode) && !S_ISDIR(st.st_mode) && !S_ISLNK(st.st_mode)) { throw Error(format("file '%1%' has unsupported type") % path); + } return {S_ISREG(st.st_mode) ? Type::tRegular diff --git a/third_party/nix/src/libstore/local-store.cc b/third_party/nix/src/libstore/local-store.cc index 4c431b6b381a..020813a35669 100644 --- a/third_party/nix/src/libstore/local-store.cc +++ b/third_party/nix/src/libstore/local-store.cc @@ -73,8 +73,9 @@ LocalStore::LocalStore(const Params& params) for (auto& perUserDir : {profilesDir + "/per-user", gcRootsDir + "/per-user"}) { createDirs(perUserDir); - if (chmod(perUserDir.c_str(), 0755) == -1) + if (chmod(perUserDir.c_str(), 0755) == -1) { throw SysError("could not set permissions on '%s' to 755", perUserDir); + } } createUser(getUserName(), getuid()); @@ -90,18 +91,21 @@ LocalStore::LocalStore(const Params& params) << "' specified in 'build-users-group' does not exist"; } else { struct stat st; - if (stat(realStoreDir.c_str(), &st)) + if (stat(realStoreDir.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % realStoreDir); + } if (st.st_uid != 0 || st.st_gid != gr->gr_gid || (st.st_mode & ~S_IFMT) != perm) { - if (chown(realStoreDir.c_str(), 0, gr->gr_gid) == -1) + if (chown(realStoreDir.c_str(), 0, gr->gr_gid) == -1) { throw SysError(format("changing ownership of path '%1%'") % realStoreDir); - if (chmod(realStoreDir.c_str(), perm) == -1) + } + if (chmod(realStoreDir.c_str(), perm) == -1) { throw SysError(format("changing permissions on path '%1%'") % realStoreDir); + } } } } @@ -111,13 +115,15 @@ LocalStore::LocalStore(const Params& params) Path path = realStoreDir; struct stat st; while (path != "/") { - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting status of '%1%'") % path); - if (S_ISLNK(st.st_mode)) + } + if (S_ISLNK(st.st_mode)) { throw Error(format("the path '%1%' is a symlink; " "this is not allowed for the Nix store and its " "parent directories") % path); + } path = dirOf(path); } } @@ -168,17 +174,19 @@ LocalStore::LocalStore(const Params& params) openDB(*state, true); writeFile(schemaPath, (format("%1%") % nixSchemaVersion).str()); } else if (curSchema < nixSchemaVersion) { - if (curSchema < 5) + if (curSchema < 5) { throw Error( "Your Nix store has a database in Berkeley DB format,\n" "which is no longer supported. To convert to the new format,\n" "please upgrade Nix to version 0.12 first."); + } - if (curSchema < 6) + if (curSchema < 6) { throw Error( "Your Nix store has a database in flat file format,\n" "which is no longer supported. To convert to the new format,\n" "please upgrade Nix to version 1.11 first."); + } if (!lockFile(globalLock.get(), ltWrite, false)) { LOG(INFO) << "waiting for exclusive access to the Nix store..."; @@ -295,24 +303,27 @@ int LocalStore::getSchema() { int curSchema = 0; if (pathExists(schemaPath)) { string s = readFile(schemaPath); - if (!string2Int(s, curSchema)) + if (!string2Int(s, curSchema)) { throw Error(format("'%1%' is corrupt") % schemaPath); + } } return curSchema; } void LocalStore::openDB(State& state, bool create) { - if (access(dbDir.c_str(), R_OK | W_OK)) + if (access(dbDir.c_str(), R_OK | W_OK)) { throw SysError(format("Nix database directory '%1%' is not writable") % dbDir); + } /* Open the Nix database. */ string dbPath = dbDir + "/db.sqlite"; auto& db(state.db); if (sqlite3_open_v2(dbPath.c_str(), &db.db, SQLITE_OPEN_READWRITE | (create ? SQLITE_OPEN_CREATE : 0), - 0) != SQLITE_OK) + 0) != SQLITE_OK) { throw Error(format("cannot open Nix database '%1%'") % dbPath); + } #ifdef __CYGWIN__ /* The cygwin version of sqlite3 has a patch which calls @@ -354,15 +365,17 @@ void LocalStore::openDB(State& state, bool create) { } if (prevMode != mode && sqlite3_exec(db, ("pragma main.journal_mode = " + mode + ";").c_str(), 0, - 0, 0) != SQLITE_OK) + 0, 0) != SQLITE_OK) { throwSQLiteError(db, "setting journal mode"); + } /* Increase the auto-checkpoint interval to 40000 pages. This seems enough to ensure that instantiating the NixOS system derivation is done in a single fsync(). */ if (mode == "wal" && sqlite3_exec(db, "pragma wal_autocheckpoint = 40000;", 0, - 0, 0) != SQLITE_OK) + 0, 0) != SQLITE_OK) { throwSQLiteError(db, "setting autocheckpoint interval"); + } /* Initialise the database schema, if necessary. */ if (create) { @@ -382,15 +395,18 @@ void LocalStore::makeStoreWritable() { } /* Check if /nix/store is on a read-only mount. */ struct statvfs stat; - if (statvfs(realStoreDir.c_str(), &stat) != 0) + if (statvfs(realStoreDir.c_str(), &stat) != 0) { throw SysError("getting info about the Nix store mount point"); + } if (stat.f_flag & ST_RDONLY) { - if (unshare(CLONE_NEWNS) == -1) + if (unshare(CLONE_NEWNS) == -1) { throw SysError("setting up a private mount namespace"); + } - if (mount(0, realStoreDir.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1) + if (mount(0, realStoreDir.c_str(), "none", MS_REMOUNT | MS_BIND, 0) == -1) { throw SysError(format("remounting %1% writable") % realStoreDir); + } } #endif } @@ -405,8 +421,9 @@ static void canonicaliseTimestampAndPermissions(const Path& path, if (mode != 0444 && mode != 0555) { mode = (st.st_mode & S_IFMT) | 0444 | (st.st_mode & S_IXUSR ? 0111 : 0); - if (chmod(path.c_str(), mode) == -1) + if (chmod(path.c_str(), mode) == -1) { throw SysError(format("changing mode of '%1%' to %2$o") % path % mode); + } } } @@ -417,20 +434,23 @@ static void canonicaliseTimestampAndPermissions(const Path& path, times[1].tv_sec = mtimeStore; times[1].tv_usec = 0; #if HAVE_LUTIMES - if (lutimes(path.c_str(), times) == -1) + if (lutimes(path.c_str(), times) == -1) { if (errno != ENOSYS || - (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1)) + (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1)) { #else if (!S_ISLNK(st.st_mode) && utimes(path.c_str(), times) == -1) #endif throw SysError(format("changing modification time of '%1%'") % path); - } + } + } + } // namespace nix } void canonicaliseTimestampAndPermissions(const Path& path) { struct stat st; - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % path); + } canonicaliseTimestampAndPermissions(path, st); } @@ -449,25 +469,29 @@ static void canonicalisePathMetaData_(const Path& path, uid_t fromUid, #endif struct stat st; - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % path); + } /* Really make sure that the path is of a supported type. */ - if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) + if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) { throw Error(format("file '%1%' has an unsupported type") % path); + } #if __linux__ /* Remove extended attributes / ACLs. */ ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0); if (eaSize < 0) { - if (errno != ENOTSUP && errno != ENODATA) + if (errno != ENOTSUP && errno != ENODATA) { throw SysError("querying extended attributes of '%s'", path); + } } else if (eaSize > 0) { std::vector<char> eaBuf(eaSize); - if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0) + if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0) { throw SysError("querying extended attributes of '%s'", path); + } for (auto& eaName : tokenizeString<Strings>( std::string(eaBuf.data(), eaSize), std::string("\000", 1))) { @@ -476,9 +500,10 @@ static void canonicalisePathMetaData_(const Path& path, uid_t fromUid, if (eaName == "security.selinux") { continue; } - if (lremovexattr(path.c_str(), eaName.c_str()) == -1) + if (lremovexattr(path.c_str(), eaName.c_str()) == -1) { throw SysError("removing extended attribute '%s' from '%s'", eaName, path); + } } } #endif @@ -491,8 +516,9 @@ static void canonicalisePathMetaData_(const Path& path, uid_t fromUid, (i.e. "touch $out/foo; ln $out/foo $out/bar"). */ if (fromUid != (uid_t)-1 && st.st_uid != fromUid) { assert(!S_ISDIR(st.st_mode)); - if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) + if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) { throw BuildError(format("invalid ownership on file '%1%'") % path); + } mode_t mode = st.st_mode & ~S_IFMT; assert(S_ISLNK(st.st_mode) || (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && @@ -513,18 +539,20 @@ static void canonicalisePathMetaData_(const Path& path, uid_t fromUid, users group); we check for this case below. */ if (st.st_uid != geteuid()) { #if HAVE_LCHOWN - if (lchown(path.c_str(), geteuid(), getegid()) == -1) + if (lchown(path.c_str(), geteuid(), getegid()) == -1) { #else if (!S_ISLNK(st.st_mode) && chown(path.c_str(), geteuid(), getegid()) == -1) #endif throw SysError(format("changing owner of '%1%' to %2%") % path % geteuid()); + } } if (S_ISDIR(st.st_mode)) { DirEntries entries = readDirectory(path); - for (auto& i : entries) + for (auto& i : entries) { canonicalisePathMetaData_(path + "/" + i.name, fromUid, inodesSeen); + } } } @@ -535,8 +563,9 @@ void canonicalisePathMetaData(const Path& path, uid_t fromUid, /* On platforms that don't have lchown(), the top-level path can't be a symlink, since we can't change its ownership. */ struct stat st; - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % path); + } if (st.st_uid != geteuid()) { assert(S_ISLNK(st.st_mode)); @@ -557,10 +586,11 @@ void LocalStore::checkDerivationOutputs(const Path& drvPath, if (drv.isFixedOutput()) { DerivationOutputs::const_iterator out = drv.outputs.find("out"); - if (out == drv.outputs.end()) + if (out == drv.outputs.end()) { throw Error( format("derivation '%1%' does not have an output named 'out'") % drvPath); + } bool recursive; Hash h; @@ -569,11 +599,12 @@ void LocalStore::checkDerivationOutputs(const Path& drvPath, StringPairs::const_iterator j = drv.env.find("out"); if (out->second.path != outPath || j == drv.env.end() || - j->second != outPath) + j->second != outPath) { throw Error( format( "derivation '%1%' has incorrect output '%2%', should be '%3%'") % drvPath % out->second.path % outPath); + } } else { @@ -589,21 +620,23 @@ void LocalStore::checkDerivationOutputs(const Path& drvPath, Path outPath = makeOutputPath(i.first, h, drvName); StringPairs::const_iterator j = drv.env.find(i.first); if (i.second.path != outPath || j == drv.env.end() || - j->second != outPath) + j->second != outPath) { throw Error(format("derivation '%1%' has incorrect output '%2%', " "should be '%3%'") % drvPath % i.second.path % outPath); + } } } } uint64_t LocalStore::addValidPath(State& state, const ValidPathInfo& info, bool checkOutputs) { - if (info.ca != "" && !info.isContentAddressed(*this)) + if (info.ca != "" && !info.isContentAddressed(*this)) { throw Error( "cannot add path '%s' to the Nix store because it claims to be " "content-addressed but isn't", info.path); + } state.stmtRegisterValidPath .use()(info.path)(info.narHash.to_string(Base16))( @@ -697,8 +730,9 @@ void LocalStore::queryPathInfoUncached( /* Get the references. */ auto useQueryReferences(state->stmtQueryReferences.use()(info->id)); - while (useQueryReferences.next()) + while (useQueryReferences.next()) { info->references.insert(useQueryReferences.getStr(0)); + } return info; })); @@ -740,10 +774,11 @@ bool LocalStore::isValidPathUncached(const Path& path) { PathSet LocalStore::queryValidPaths(const PathSet& paths, SubstituteFlag maybeSubstitute) { PathSet res; - for (auto& i : paths) + for (auto& i : paths) { if (isValidPath(i)) { res.insert(i); } + } return res; } @@ -752,7 +787,9 @@ PathSet LocalStore::queryAllValidPaths() { auto state(_state.lock()); auto use(state->stmtQueryValidPaths.use()); PathSet res; - while (use.next()) res.insert(use.getStr(0)); + while (use.next()) { + res.insert(use.getStr(0)); + } return res; }); } @@ -761,8 +798,9 @@ void LocalStore::queryReferrers(State& state, const Path& path, PathSet& referrers) { auto useQueryReferrers(state.stmtQueryReferrers.use()(path)); - while (useQueryReferrers.next()) + while (useQueryReferrers.next()) { referrers.insert(useQueryReferrers.getStr(0)); + } } void LocalStore::queryReferrers(const Path& path, PathSet& referrers) { @@ -782,8 +820,9 @@ PathSet LocalStore::queryValidDerivers(const Path& path) { auto useQueryValidDerivers(state->stmtQueryValidDerivers.use()(path)); PathSet derivers; - while (useQueryValidDerivers.next()) + while (useQueryValidDerivers.next()) { derivers.insert(useQueryValidDerivers.getStr(1)); + } return derivers; }); @@ -797,8 +836,9 @@ PathSet LocalStore::queryDerivationOutputs(const Path& path) { queryValidPathId(*state, path))); PathSet outputs; - while (useQueryDerivationOutputs.next()) + while (useQueryDerivationOutputs.next()) { outputs.insert(useQueryDerivationOutputs.getStr(1)); + } return outputs; }); @@ -812,8 +852,9 @@ StringSet LocalStore::queryDerivationOutputNames(const Path& path) { queryValidPathId(*state, path))); StringSet outputNames; - while (useQueryDerivationOutputs.next()) + while (useQueryDerivationOutputs.next()) { outputNames.insert(useQueryDerivationOutputs.getStr(0)); + } return outputNames; }); @@ -865,11 +906,13 @@ PathSet LocalStore::querySubstitutablePaths(const PathSet& paths) { auto valid = sub->queryValidPaths(remaining); PathSet remaining2; - for (auto& path : remaining) - if (valid.count(path)) + for (auto& path : remaining) { + if (valid.count(path)) { res.insert(path); - else + } else { remaining2.insert(path); + } + } std::swap(remaining, remaining2); } @@ -935,24 +978,26 @@ void LocalStore::registerValidPaths(const ValidPathInfos& infos) { for (auto& i : infos) { assert(i.narHash.type == htSHA256); - if (isValidPath_(*state, i.path)) + if (isValidPath_(*state, i.path)) { updatePathInfo(*state, i); - else + } else { addValidPath(*state, i, false); + } paths.insert(i.path); } for (auto& i : infos) { auto referrer = queryValidPathId(*state, i.path); - for (auto& j : i.references) + for (auto& j : i.references) { state->stmtAddReference.use()(referrer)(queryValidPathId(*state, j)) .exec(); + } } /* Check that the derivation outputs are correct. We can't do this in addValidPath() above, because the references might not be valid yet. */ - for (auto& i : infos) + for (auto& i : infos) { if (isDerivation(i.path)) { // FIXME: inefficient; we already loaded the // derivation in addValidPath(). @@ -960,6 +1005,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos& infos) { readDerivation(realStoreDir + "/" + baseNameOf(i.path)); checkDerivationOutputs(i.path, drv); } + } /* Do a topological sort of the paths. This will throw an error if a cycle is detected and roll back the @@ -989,8 +1035,9 @@ void LocalStore::invalidatePath(State& state, const Path& path) { const PublicKeys& LocalStore::getPublicKeys() { auto state(_state.lock()); - if (!state->publicKeys) + if (!state->publicKeys) { state->publicKeys = std::make_unique<PublicKeys>(getDefaultPublicKeys()); + } return *state->publicKeys; } @@ -1039,15 +1086,17 @@ void LocalStore::addToStore(const ValidPathInfo& info, Source& source, auto hashResult = hashSink.finish(); - if (hashResult.first != info.narHash) + if (hashResult.first != info.narHash) { throw Error( "hash mismatch importing path '%s';\n wanted: %s\n got: %s", info.path, info.narHash.to_string(), hashResult.first.to_string()); + } - if (hashResult.second != info.narSize) + if (hashResult.second != info.narSize) { throw Error( "size mismatch importing path '%s';\n wanted: %s\n got: %s", info.path, info.narSize, hashResult.second); + } autoGC(); @@ -1130,10 +1179,11 @@ Path LocalStore::addToStore(const string& name, const Path& _srcPath, method for very large paths, but `copyPath' is mainly used for small files. */ StringSink sink; - if (recursive) + if (recursive) { dumpPath(srcPath, sink, filter); - else + } else { sink.s = make_ref<std::string>(readFile(srcPath)); + } return addToStoreFromDump(*sink.s, name, recursive, hashAlgo, repair); } @@ -1206,10 +1256,11 @@ void LocalStore::invalidatePathChecked(const Path& path) { PathSet referrers; queryReferrers(*state, path, referrers); referrers.erase(path); /* ignore self-references */ - if (!referrers.empty()) + if (!referrers.empty()) { throw PathInUse( format("cannot delete path '%1%' because it is in use by %2%") % path % showPaths(referrers)); + } invalidatePath(*state, path); } @@ -1238,8 +1289,9 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair) { fdGCLock = -1; - for (auto& i : validPaths2) + for (auto& i : validPaths2) { verifyPath(i, store, done, validPaths, repair, errors); + } /* Optionally, check the content hashes (slow). */ if (checkContents) { @@ -1328,13 +1380,14 @@ void LocalStore::verifyPath(const Path& path, const PathSet& store, bool canInvalidate = true; PathSet referrers; queryReferrers(path, referrers); - for (auto& i : referrers) + for (auto& i : referrers) { if (i != path) { verifyPath(i, store, done, validPaths, repair, errors); if (validPaths.find(i) != validPaths.end()) { canInvalidate = false; } } + } if (canInvalidate) { LOG(WARNING) << "path '" << path @@ -1463,10 +1516,12 @@ void LocalStore::createUser(const std::string& userName, uid_t userId) { for (auto& dir : {fmt("%s/profiles/per-user/%s", stateDir, userName), fmt("%s/gcroots/per-user/%s", stateDir, userName)}) { createDirs(dir); - if (chmod(dir.c_str(), 0755) == -1) + if (chmod(dir.c_str(), 0755) == -1) { throw SysError("changing permissions of directory '%s'", dir); - if (chown(dir.c_str(), userId, getgid()) == -1) + } + if (chown(dir.c_str(), userId, getgid()) == -1) { throw SysError("changing owner of directory '%s'", dir); + } } } diff --git a/third_party/nix/src/libstore/misc.cc b/third_party/nix/src/libstore/misc.cc index daed88cdc995..a82fc1251c56 100644 --- a/third_party/nix/src/libstore/misc.cc +++ b/third_party/nix/src/libstore/misc.cc @@ -47,35 +47,44 @@ void Store::computeFSClosure(const PathSet& startPaths, PathSet& paths_, if (flipDirection) { PathSet referrers; queryReferrers(path, referrers); - for (auto& ref : referrers) + for (auto& ref : referrers) { if (ref != path) { enqueue(ref); } + } - if (includeOutputs) + if (includeOutputs) { for (auto& i : queryValidDerivers(path)) { enqueue(i); } + } - if (includeDerivers && isDerivation(path)) - for (auto& i : queryDerivationOutputs(path)) - if (isValidPath(i) && queryPathInfo(i)->deriver == path) + if (includeDerivers && isDerivation(path)) { + for (auto& i : queryDerivationOutputs(path)) { + if (isValidPath(i) && queryPathInfo(i)->deriver == path) { enqueue(i); + } + } + } } else { - for (auto& ref : info->references) + for (auto& ref : info->references) { if (ref != path) { enqueue(ref); } + } - if (includeOutputs && isDerivation(path)) - for (auto& i : queryDerivationOutputs(path)) + if (includeOutputs && isDerivation(path)) { + for (auto& i : queryDerivationOutputs(path)) { if (isValidPath(i)) { enqueue(i); } + } + } - if (includeDerivers && isValidPath(info->deriver)) + if (includeDerivers && isValidPath(info->deriver)) { enqueue(info->deriver); + } } { @@ -105,7 +114,9 @@ void Store::computeFSClosure(const PathSet& startPaths, PathSet& paths_, { auto state(state_.lock()); - while (state->pending) state.wait(done); + while (state->pending) { + state.wait(done); + } if (state->exc) { std::rethrow_exception(state->exc); } @@ -154,9 +165,10 @@ void Store::queryMissing(const PathSet& targets, PathSet& willBuild_, state->willBuild.insert(drvPath); } - for (auto& i : drv.inputDrvs) + for (auto& i : drv.inputDrvs) { pool.enqueue( std::bind(doPath, makeDrvPathWithOutputs(i.first, i.second))); + } }; auto checkOutput = [&](const Path& drvPath, ref<Derivation> drv, @@ -181,8 +193,9 @@ void Store::queryMissing(const PathSet& targets, PathSet& willBuild_, drvState->left--; drvState->outPaths.insert(outPath); if (!drvState->left) { - for (auto& path : drvState->outPaths) + for (auto& path : drvState->outPaths) { pool.enqueue(std::bind(doPath, path)); + } } } } @@ -211,20 +224,24 @@ void Store::queryMissing(const PathSet& targets, PathSet& willBuild_, ParsedDerivation parsedDrv(i2.first, drv); PathSet invalid; - for (auto& j : drv.outputs) - if (wantOutput(j.first, i2.second) && !isValidPath(j.second.path)) + for (auto& j : drv.outputs) { + if (wantOutput(j.first, i2.second) && !isValidPath(j.second.path)) { invalid.insert(j.second.path); + } + } if (invalid.empty()) { return; } if (settings.useSubstitutes && parsedDrv.substitutesAllowed()) { auto drvState = make_ref<Sync<DrvState>>(DrvState(invalid.size())); - for (auto& output : invalid) + for (auto& output : invalid) { pool.enqueue(std::bind(checkOutput, i2.first, make_ref<Derivation>(drv), output, drvState)); - } else + } + } else { mustBuildDrv(i2.first, drv); + } } else { if (isValidPath(path)) { @@ -250,8 +267,9 @@ void Store::queryMissing(const PathSet& targets, PathSet& willBuild_, state->narSize += info->second.narSize; } - for (auto& ref : info->second.references) + for (auto& ref : info->second.references) { pool.enqueue(std::bind(doPath, ref)); + } } }; @@ -269,10 +287,11 @@ Paths Store::topoSortPaths(const PathSet& paths) { std::function<void(const Path& path, const Path* parent)> dfsVisit; dfsVisit = [&](const Path& path, const Path* parent) { - if (parents.find(path) != parents.end()) + if (parents.find(path) != parents.end()) { throw BuildError( format("cycle detected in the references of '%1%' from '%2%'") % path % *parent); + } if (visited.find(path) != visited.end()) { return; @@ -286,12 +305,13 @@ Paths Store::topoSortPaths(const PathSet& paths) { } catch (InvalidPath&) { } - for (auto& i : references) + for (auto& i : references) { /* Don't traverse into paths that don't exist. That can happen due to substitutes for non-existent paths. */ if (i != path && paths.find(i) != paths.end()) { dfsVisit(i, &path); } + } sorted.push_front(path); parents.erase(path); diff --git a/third_party/nix/src/libstore/nar-accessor.cc b/third_party/nix/src/libstore/nar-accessor.cc index 8a57179ab34a..8be22ada2e36 100644 --- a/third_party/nix/src/libstore/nar-accessor.cc +++ b/third_party/nix/src/libstore/nar-accessor.cc @@ -45,14 +45,17 @@ struct NarAccessor : public FSAccessor { void createMember(const Path& path, NarMember member) { size_t level = std::count(path.begin(), path.end(), '/'); - while (parents.size() > level) parents.pop(); + while (parents.size() > level) { + parents.pop(); + } if (parents.empty()) { acc.root = std::move(member); parents.push(&acc.root); } else { - if (parents.top()->type != FSAccessor::Type::tDirectory) + if (parents.top()->type != FSAccessor::Type::tDirectory) { throw Error("NAR file missing parent directory of path '%s'", path); + } auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member)); parents.push(&result.first->second); @@ -118,8 +121,9 @@ struct NarAccessor : public FSAccessor { } else if (type == "symlink") { member.type = FSAccessor::Type::tSymlink; member.target = v.value("target", ""); - } else + } else { return; + } }; json v = json::parse(listing); @@ -157,8 +161,9 @@ struct NarAccessor : public FSAccessor { NarMember& get(const Path& path) { auto result = find(path); - if (result == nullptr) + if (result == nullptr) { throw Error("NAR file does not contain path '%1%'", path); + } return *result; } @@ -173,9 +178,10 @@ struct NarAccessor : public FSAccessor { StringSet readDirectory(const Path& path) override { auto i = get(path); - if (i.type != FSAccessor::Type::tDirectory) + if (i.type != FSAccessor::Type::tDirectory) { throw Error(format("path '%1%' inside NAR file is not a directory") % path); + } StringSet res; for (auto& child : i.children) { @@ -187,9 +193,10 @@ struct NarAccessor : public FSAccessor { std::string readFile(const Path& path) override { auto i = get(path); - if (i.type != FSAccessor::Type::tRegular) + if (i.type != FSAccessor::Type::tRegular) { throw Error(format("path '%1%' inside NAR file is not a regular file") % path); + } if (getNarBytes) { return getNarBytes(i.start, i.size); @@ -201,8 +208,9 @@ struct NarAccessor : public FSAccessor { std::string readLink(const Path& path) override { auto i = get(path); - if (i.type != FSAccessor::Type::tSymlink) + if (i.type != FSAccessor::Type::tSymlink) { throw Error(format("path '%1%' inside NAR file is not a symlink") % path); + } return i.target; } }; @@ -241,8 +249,9 @@ void listNar(JSONPlaceholder& res, ref<FSAccessor> accessor, const Path& path, if (recurse) { auto res3 = res2.placeholder(name); listNar(res3, accessor, path + "/" + name, true); - } else + } else { res2.object(name); + } } } break; diff --git a/third_party/nix/src/libstore/nar-info-disk-cache.cc b/third_party/nix/src/libstore/nar-info-disk-cache.cc index 6d9aeb129d22..3adda508d081 100644 --- a/third_party/nix/src/libstore/nar-info-disk-cache.cc +++ b/third_party/nix/src/libstore/nar-info-disk-cache.cc @@ -76,8 +76,9 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache { state->db = SQLite(dbPath); - if (sqlite3_busy_timeout(state->db, 60 * 60 * 1000) != SQLITE_OK) + if (sqlite3_busy_timeout(state->db, 60 * 60 * 1000) != SQLITE_OK) { throwSQLiteError(state->db, "setting timeout"); + } // We can always reproduce the cache. state->db.exec("pragma synchronous = off"); @@ -224,12 +225,15 @@ class NarInfoDiskCacheImpl : public NarInfoDiskCache { narInfo->fileSize = queryNAR.getInt(5); narInfo->narHash = Hash(queryNAR.getStr(6)); narInfo->narSize = queryNAR.getInt(7); - for (auto& r : tokenizeString<Strings>(queryNAR.getStr(8), " ")) + for (auto& r : tokenizeString<Strings>(queryNAR.getStr(8), " ")) { narInfo->references.insert(cache.storeDir + "/" + r); - if (!queryNAR.isNull(9)) + } + if (!queryNAR.isNull(9)) { narInfo->deriver = cache.storeDir + "/" + queryNAR.getStr(9); - for (auto& sig : tokenizeString<Strings>(queryNAR.getStr(10), " ")) + } + for (auto& sig : tokenizeString<Strings>(queryNAR.getStr(10), " ")) { narInfo->sigs.insert(sig); + } narInfo->ca = queryNAR.getStr(11); return {oValid, narInfo}; diff --git a/third_party/nix/src/libstore/nar-info.cc b/third_party/nix/src/libstore/nar-info.cc index fa7d72c8ae30..fee470fc7df6 100644 --- a/third_party/nix/src/libstore/nar-info.cc +++ b/third_party/nix/src/libstore/nar-info.cc @@ -40,19 +40,19 @@ NarInfo::NarInfo(const Store& store, const std::string& s, corrupt(); } path = value; - } else if (name == "URL") + } else if (name == "URL") { url = value; - else if (name == "Compression") + } else if (name == "Compression") { compression = value; - else if (name == "FileHash") + } else if (name == "FileHash") { fileHash = parseHashField(value); - else if (name == "FileSize") { + } else if (name == "FileSize") { if (!string2Int(value, fileSize)) { corrupt(); } - } else if (name == "NarHash") + } else if (name == "NarHash") { narHash = parseHashField(value); - else if (name == "NarSize") { + } else if (name == "NarSize") { if (!string2Int(value, narSize)) { corrupt(); } @@ -76,11 +76,11 @@ NarInfo::NarInfo(const Store& store, const std::string& s, } deriver = p; } - } else if (name == "System") + } else if (name == "System") { system = value; - else if (name == "Sig") + } else if (name == "Sig") { sigs.insert(value); - else if (name == "CA") { + } else if (name == "CA") { if (!ca.empty()) { corrupt(); } diff --git a/third_party/nix/src/libstore/optimise-store.cc b/third_party/nix/src/libstore/optimise-store.cc index ad37190236e9..8c0f24d382bd 100644 --- a/third_party/nix/src/libstore/optimise-store.cc +++ b/third_party/nix/src/libstore/optimise-store.cc @@ -17,10 +17,12 @@ namespace nix { static void makeWritable(const Path& path) { struct stat st; - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % path); - if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) + } + if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) { throw SysError(format("changing writability of '%1%'") % path); + } } struct MakeReadOnly { @@ -98,8 +100,9 @@ void LocalStore::optimisePath_(OptimiseStats& stats, const Path& path, checkInterrupt(); struct stat st; - if (lstat(path.c_str(), &st)) + if (lstat(path.c_str(), &st)) { throw SysError(format("getting attributes of path '%1%'") % path); + } #if __APPLE__ /* HFS/macOS has some undocumented security feature disabling hardlinking for @@ -192,8 +195,9 @@ retry: /* Yes! We've seen a file with the same contents. Replace the current file with a hard link to that file. */ struct stat stLink; - if (lstat(linkPath.c_str(), &stLink)) + if (lstat(linkPath.c_str(), &stLink)) { throw SysError(format("getting attributes of path '%1%'") % linkPath); + } if (st.st_ino == stLink.st_ino) { DLOG(INFO) << path << " is already linked to " << linkPath; diff --git a/third_party/nix/src/libstore/parsed-derivations.cc b/third_party/nix/src/libstore/parsed-derivations.cc index 29133664fd76..72ed36d32da6 100644 --- a/third_party/nix/src/libstore/parsed-derivations.cc +++ b/third_party/nix/src/libstore/parsed-derivations.cc @@ -20,40 +20,44 @@ std::optional<std::string> ParsedDerivation::getStringAttr( const std::string& name) const { if (structuredAttrs) { auto i = structuredAttrs->find(name); - if (i == structuredAttrs->end()) + if (i == structuredAttrs->end()) { return {}; - else { - if (!i->is_string()) + } else { + if (!i->is_string()) { throw Error("attribute '%s' of derivation '%s' must be a string", name, drvPath); + } return i->get<std::string>(); } } else { auto i = drv.env.find(name); - if (i == drv.env.end()) + if (i == drv.env.end()) { return {}; - else + } else { return i->second; + } } } bool ParsedDerivation::getBoolAttr(const std::string& name, bool def) const { if (structuredAttrs) { auto i = structuredAttrs->find(name); - if (i == structuredAttrs->end()) + if (i == structuredAttrs->end()) { return def; - else { - if (!i->is_boolean()) + } else { + if (!i->is_boolean()) { throw Error("attribute '%s' of derivation '%s' must be a Boolean", name, drvPath); + } return i->get<bool>(); } } else { auto i = drv.env.find(name); - if (i == drv.env.end()) + if (i == drv.env.end()) { return def; - else + } else { return i->second == "1"; + } } } @@ -61,48 +65,54 @@ std::optional<Strings> ParsedDerivation::getStringsAttr( const std::string& name) const { if (structuredAttrs) { auto i = structuredAttrs->find(name); - if (i == structuredAttrs->end()) + if (i == structuredAttrs->end()) { return {}; - else { - if (!i->is_array()) + } else { + if (!i->is_array()) { throw Error( "attribute '%s' of derivation '%s' must be a list of strings", name, drvPath); + } Strings res; for (auto j = i->begin(); j != i->end(); ++j) { - if (!j->is_string()) + if (!j->is_string()) { throw Error( "attribute '%s' of derivation '%s' must be a list of strings", name, drvPath); + } res.push_back(j->get<std::string>()); } return res; } } else { auto i = drv.env.find(name); - if (i == drv.env.end()) + if (i == drv.env.end()) { return {}; - else + } else { return tokenizeString<Strings>(i->second); + } } } StringSet ParsedDerivation::getRequiredSystemFeatures() const { StringSet res; - for (auto& i : getStringsAttr("requiredSystemFeatures").value_or(Strings())) + for (auto& i : getStringsAttr("requiredSystemFeatures").value_or(Strings())) { res.insert(i); + } return res; } bool ParsedDerivation::canBuildLocally() const { if (drv.platform != settings.thisSystem.get() && - !settings.extraPlatforms.get().count(drv.platform) && !drv.isBuiltin()) + !settings.extraPlatforms.get().count(drv.platform) && !drv.isBuiltin()) { return false; + } - for (auto& feature : getRequiredSystemFeatures()) + for (auto& feature : getRequiredSystemFeatures()) { if (!settings.systemFeatures.get().count(feature)) { return false; } + } return true; } diff --git a/third_party/nix/src/libstore/pathlocks.cc b/third_party/nix/src/libstore/pathlocks.cc index ffec3d7d7ea6..a0d7b721ec98 100644 --- a/third_party/nix/src/libstore/pathlocks.cc +++ b/third_party/nix/src/libstore/pathlocks.cc @@ -18,8 +18,9 @@ AutoCloseFD openLockFile(const Path& path, bool create) { AutoCloseFD fd; fd = open(path.c_str(), O_CLOEXEC | O_RDWR | (create ? O_CREAT : 0), 0600); - if (!fd && (create || errno != ENOENT)) + if (!fd && (create || errno != ENOENT)) { throw SysError(format("opening lock file '%1%'") % path); + } return fd; } @@ -37,22 +38,24 @@ void deleteLockFile(const Path& path, int fd) { bool lockFile(int fd, LockType lockType, bool wait) { int type; - if (lockType == ltRead) + if (lockType == ltRead) { type = LOCK_SH; - else if (lockType == ltWrite) + } else if (lockType == ltWrite) { type = LOCK_EX; - else if (lockType == ltNone) + } else if (lockType == ltNone) { type = LOCK_UN; - else + } else { abort(); + } if (wait) { while (flock(fd, type) != 0) { checkInterrupt(); - if (errno != EINTR) + if (errno != EINTR) { throw SysError(format("acquiring/releasing lock")); - else + } else { return false; + } } } else { while (flock(fd, type | LOCK_NB) != 0) { @@ -118,16 +121,18 @@ bool PathLocks::lockPaths(const PathSet& paths, const string& waitMsg, /* Check that the lock file hasn't become stale (i.e., hasn't been unlinked). */ struct stat st; - if (fstat(fd.get(), &st) == -1) + if (fstat(fd.get(), &st) == -1) { throw SysError(format("statting lock file '%1%'") % lockPath); - if (st.st_size != 0) + } + if (st.st_size != 0) { /* This lock file has been unlinked, so we're holding a lock on a deleted file. This means that other processes may create and acquire a lock on `lockPath', and proceed. So we must retry. */ DLOG(INFO) << "open lock file '" << lockPath << "' has become stale"; - else + } else { break; + } } /* Use borrow so that the descriptor isn't closed. */ diff --git a/third_party/nix/src/libstore/profiles.cc b/third_party/nix/src/libstore/profiles.cc index 38d0972e4400..38ad0c01e461 100644 --- a/third_party/nix/src/libstore/profiles.cc +++ b/third_party/nix/src/libstore/profiles.cc @@ -28,10 +28,11 @@ static int parseName(const string& profileName, const string& name) { return -1; } int n; - if (string2Int(string(s, 0, p), n) && n >= 0) + if (string2Int(string(s, 0, p), n) && n >= 0) { return n; - else + } else { return -1; + } } Generations findGenerations(Path profile, int& curGen) { @@ -47,8 +48,9 @@ Generations findGenerations(Path profile, int& curGen) { gen.path = profileDir + "/" + i.name; gen.number = n; struct stat st; - if (lstat(gen.path.c_str(), &st) != 0) + if (lstat(gen.path.c_str(), &st) != 0) { throw SysError(format("statting '%1%'") % gen.path); + } gen.creationTime = st.st_mtime; gens.push_back(gen); } @@ -105,8 +107,9 @@ Path createGeneration(ref<LocalFSStore> store, Path profile, Path outPath) { } static void removeFile(const Path& path) { - if (remove(path.c_str()) == -1) + if (remove(path.c_str()) == -1) { throw SysError(format("cannot unlink '%1%'") % path); + } } void deleteGeneration(const Path& profile, unsigned int gen) { @@ -134,9 +137,10 @@ void deleteGenerations(const Path& profile, int curGen; Generations gens = findGenerations(profile, curGen); - if (gensToDelete.find(curGen) != gensToDelete.end()) + if (gensToDelete.find(curGen) != gensToDelete.end()) { throw Error(format("cannot delete current generation of profile %1%'") % profile); + } for (auto& i : gens) { if (gensToDelete.find(i.number) == gensToDelete.end()) { @@ -176,10 +180,11 @@ void deleteOldGenerations(const Path& profile, bool dryRun) { int curGen; Generations gens = findGenerations(profile, curGen); - for (auto& i : gens) + for (auto& i : gens) { if (i.number != curGen) { deleteGeneration2(profile, i.number, dryRun); } + } } void deleteGenerationsOlderThan(const Path& profile, time_t t, bool dryRun) { @@ -190,7 +195,7 @@ void deleteGenerationsOlderThan(const Path& profile, time_t t, bool dryRun) { Generations gens = findGenerations(profile, curGen); bool canDelete = false; - for (auto i = gens.rbegin(); i != gens.rend(); ++i) + for (auto i = gens.rbegin(); i != gens.rend(); ++i) { if (canDelete) { assert(i->creationTime < t); if (i->number != curGen) { @@ -203,6 +208,7 @@ void deleteGenerationsOlderThan(const Path& profile, time_t t, bool dryRun) { time. */ canDelete = true; } + } } void deleteGenerationsOlderThan(const Path& profile, const string& timeSpec, @@ -211,8 +217,9 @@ void deleteGenerationsOlderThan(const Path& profile, const string& timeSpec, string strDays = string(timeSpec, 0, timeSpec.size() - 1); int days; - if (!string2Int(strDays, days) || days < 1) + if (!string2Int(strDays, days) || days < 1) { throw Error(format("invalid number of days specifier '%1%'") % timeSpec); + } time_t oldTime = curTime - days * 24 * 3600; diff --git a/third_party/nix/src/libstore/references.cc b/third_party/nix/src/libstore/references.cc index a46bdc758fbf..64b997ce3e68 100644 --- a/third_party/nix/src/libstore/references.cc +++ b/third_party/nix/src/libstore/references.cc @@ -21,20 +21,22 @@ static void search(const unsigned char* s, size_t len, StringSet& hashes, for (unsigned int i = 0; i < 256; ++i) { isBase32[i] = false; } - for (unsigned int i = 0; i < base32Chars.size(); ++i) + for (unsigned int i = 0; i < base32Chars.size(); ++i) { isBase32[(unsigned char)base32Chars[i]] = true; + } initialised = true; } for (size_t i = 0; i + refLength <= len;) { int j; bool match = true; - for (j = refLength - 1; j >= 0; --j) + for (j = refLength - 1; j >= 0; --j) { if (!isBase32[(unsigned char)s[i + j]]) { i += j + 1; match = false; break; } + } if (!match) { continue; } diff --git a/third_party/nix/src/libstore/remote-fs-accessor.cc b/third_party/nix/src/libstore/remote-fs-accessor.cc index 0b9bb805ebce..819f54c161e8 100644 --- a/third_party/nix/src/libstore/remote-fs-accessor.cc +++ b/third_party/nix/src/libstore/remote-fs-accessor.cc @@ -48,9 +48,10 @@ std::pair<ref<FSAccessor>, Path> RemoteFSAccessor::fetch(const Path& path_) { auto storePath = store->toStorePath(path); std::string restPath = std::string(path, storePath.size()); - if (!store->isValidPath(storePath)) + if (!store->isValidPath(storePath)) { throw InvalidPath(format("path '%1%' is not a valid store path") % storePath); + } auto i = nars.find(storePath); if (i != nars.end()) { @@ -73,8 +74,9 @@ std::pair<ref<FSAccessor>, Path> RemoteFSAccessor::fetch(const Path& path_) { throw SysError("opening NAR cache file '%s'", cacheFile); } - if (lseek(fd.get(), offset, SEEK_SET) != (off_t)offset) + if (lseek(fd.get(), offset, SEEK_SET) != (off_t)offset) { throw SysError("seeking in '%s'", cacheFile); + } std::string buf(length, 0); readFull(fd.get(), (unsigned char*)buf.data(), length); diff --git a/third_party/nix/src/libstore/remote-store.cc b/third_party/nix/src/libstore/remote-store.cc index e6849cb16031..b0025df98f5b 100644 --- a/third_party/nix/src/libstore/remote-store.cc +++ b/third_party/nix/src/libstore/remote-store.cc @@ -57,9 +57,10 @@ RemoteStore::RemoteStore(const Params& params) })) {} ref<RemoteStore::Connection> RemoteStore::openConnectionWrapper() { - if (failed) + if (failed) { throw Error("opening a connection to remote store '%s' previously failed", getUri()); + } try { return openConnection(); } catch (...) { @@ -105,12 +106,14 @@ ref<RemoteStore::Connection> UDSRemoteStore::openConnection() { struct sockaddr_un addr; addr.sun_family = AF_UNIX; - if (socketPath.size() + 1 >= sizeof(addr.sun_path)) + if (socketPath.size() + 1 >= sizeof(addr.sun_path)) { throw Error(format("socket path '%1%' is too long") % socketPath); + } strcpy(addr.sun_path, socketPath.c_str()); - if (::connect(conn->fd.get(), (struct sockaddr*)&addr, sizeof(addr)) == -1) + if (::connect(conn->fd.get(), (struct sockaddr*)&addr, sizeof(addr)) == -1) { throw SysError(format("cannot connect to daemon at '%1%'") % socketPath); + } conn->from.fd = conn->fd.get(); conn->to.fd = conn->fd.get(); @@ -134,18 +137,21 @@ void RemoteStore::initConnection(Connection& conn) { conn.from >> conn.daemonVersion; if (GET_PROTOCOL_MAJOR(conn.daemonVersion) != - GET_PROTOCOL_MAJOR(PROTOCOL_VERSION)) + GET_PROTOCOL_MAJOR(PROTOCOL_VERSION)) { throw Error("Nix daemon protocol version not supported"); - if (GET_PROTOCOL_MINOR(conn.daemonVersion) < 10) + } + if (GET_PROTOCOL_MINOR(conn.daemonVersion) < 10) { throw Error("the Nix daemon version is too old"); + } conn.to << PROTOCOL_VERSION; if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 14) { int cpu = sameMachine() && settings.lockCPU ? lockToCurrentCPU() : -1; - if (cpu != -1) + if (cpu != -1) { conn.to << 1 << cpu; - else + } else { conn.to << 0; + } } if (GET_PROTOCOL_MINOR(conn.daemonVersion) >= 11) { @@ -249,10 +255,11 @@ PathSet RemoteStore::queryValidPaths(const PathSet& paths, auto conn(getConnection()); if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 12) { PathSet res; - for (auto& i : paths) + for (auto& i : paths) { if (isValidPath(i)) { res.insert(i); } + } return res; } else { conn->to << wopQueryValidPaths << paths; @@ -344,8 +351,9 @@ void RemoteStore::queryPathInfoUncached( conn.processStderr(); } catch (Error& e) { // Ugly backwards compatibility hack. - if (e.msg().find("is not valid") != std::string::npos) + if (e.msg().find("is not valid") != std::string::npos) { throw InvalidPath(e.what()); + } throw; } if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 17) { @@ -456,9 +464,10 @@ void RemoteStore::addToStore(const ValidPathInfo& info, Source& source, Path RemoteStore::addToStore(const string& name, const Path& _srcPath, bool recursive, HashType hashAlgo, PathFilter& filter, RepairFlag repair) { - if (repair) + if (repair) { throw Error( "repairing is not supported when building through the Nix daemon"); + } auto conn(getConnection()); @@ -483,10 +492,12 @@ Path RemoteStore::addToStore(const string& name, const Path& _srcPath, } catch (SysError& e) { /* Daemon closed while we were sending the path. Probably OOM or I/O error. */ - if (e.errNo == EPIPE) try { + if (e.errNo == EPIPE) { + try { conn.processStderr(); } catch (EndOfFile& e) { } + } throw; } @@ -495,9 +506,10 @@ Path RemoteStore::addToStore(const string& name, const Path& _srcPath, Path RemoteStore::addTextToStore(const string& name, const string& s, const PathSet& references, RepairFlag repair) { - if (repair) + if (repair) { throw Error( "repairing is not supported when building through the Nix daemon"); + } auto conn(getConnection()); conn->to << wopAddTextToStore << name << s << references; @@ -511,15 +523,16 @@ void RemoteStore::buildPaths(const PathSet& drvPaths, BuildMode buildMode) { conn->to << wopBuildPaths; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13) { conn->to << drvPaths; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) { conn->to << buildMode; - else + } else /* Old daemons did not take a 'buildMode' parameter, so we need to validate it here on the client side. */ - if (buildMode != bmNormal) + if (buildMode != bmNormal) { throw Error( "repairing or checking is not supported when building through the " "Nix daemon"); + } } else { /* For backwards compatibility with old daemons, strip output identifiers. */ diff --git a/third_party/nix/src/libstore/sqlite.cc b/third_party/nix/src/libstore/sqlite.cc index 5aa661afe055..f2650ed75337 100644 --- a/third_party/nix/src/libstore/sqlite.cc +++ b/third_party/nix/src/libstore/sqlite.cc @@ -31,8 +31,9 @@ namespace nix { SQLite::SQLite(const Path& path) { if (sqlite3_open_v2(path.c_str(), &db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE, - 0) != SQLITE_OK) + 0) != SQLITE_OK) { throw Error(format("cannot open SQLite database '%s'") % path); + } } SQLite::~SQLite() { @@ -47,24 +48,27 @@ SQLite::~SQLite() { void SQLite::exec(const std::string& stmt) { retrySQLite<void>([&]() { - if (sqlite3_exec(db, stmt.c_str(), 0, 0, 0) != SQLITE_OK) + if (sqlite3_exec(db, stmt.c_str(), 0, 0, 0) != SQLITE_OK) { throwSQLiteError(db, format("executing SQLite statement '%s'") % stmt); + } }); } void SQLiteStmt::create(sqlite3* db, const string& sql) { checkInterrupt(); assert(!stmt); - if (sqlite3_prepare_v2(db, sql.c_str(), -1, &stmt, 0) != SQLITE_OK) + if (sqlite3_prepare_v2(db, sql.c_str(), -1, &stmt, 0) != SQLITE_OK) { throwSQLiteError(db, fmt("creating statement '%s'", sql)); + } this->db = db; this->sql = sql; } SQLiteStmt::~SQLiteStmt() { try { - if (stmt && sqlite3_finalize(stmt) != SQLITE_OK) + if (stmt && sqlite3_finalize(stmt) != SQLITE_OK) { throwSQLiteError(db, fmt("finalizing statement '%s'", sql)); + } } catch (...) { ignoreException(); } @@ -83,8 +87,9 @@ SQLiteStmt::Use& SQLiteStmt::Use::operator()(const std::string& value, bool notNull) { if (notNull) { if (sqlite3_bind_text(stmt, curArg++, value.c_str(), -1, - SQLITE_TRANSIENT) != SQLITE_OK) + SQLITE_TRANSIENT) != SQLITE_OK) { throwSQLiteError(stmt.db, "binding argument"); + } } else { bind(); } @@ -93,8 +98,9 @@ SQLiteStmt::Use& SQLiteStmt::Use::operator()(const std::string& value, SQLiteStmt::Use& SQLiteStmt::Use::operator()(int64_t value, bool notNull) { if (notNull) { - if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK) + if (sqlite3_bind_int64(stmt, curArg++, value) != SQLITE_OK) { throwSQLiteError(stmt.db, "binding argument"); + } } else { bind(); } @@ -113,14 +119,16 @@ int SQLiteStmt::Use::step() { return sqlite3_step(stmt); } void SQLiteStmt::Use::exec() { int r = step(); assert(r != SQLITE_ROW); - if (r != SQLITE_DONE) + if (r != SQLITE_DONE) { throwSQLiteError(stmt.db, fmt("executing SQLite statement '%s'", stmt.sql)); + } } bool SQLiteStmt::Use::next() { int r = step(); - if (r != SQLITE_DONE && r != SQLITE_ROW) + if (r != SQLITE_DONE && r != SQLITE_ROW) { throwSQLiteError(stmt.db, fmt("executing SQLite query '%s'", stmt.sql)); + } return r == SQLITE_ROW; } diff --git a/third_party/nix/src/libstore/ssh.cc b/third_party/nix/src/libstore/ssh.cc index 20add33c0046..1783732a1db5 100644 --- a/third_party/nix/src/libstore/ssh.cc +++ b/third_party/nix/src/libstore/ssh.cc @@ -10,13 +10,15 @@ SSHMaster::SSHMaster(const std::string& host, const std::string& keyFile, useMaster(useMaster && !fakeSSH), compress(compress), logFD(logFD) { - if (host == "" || hasPrefix(host, "-")) + if (host == "" || hasPrefix(host, "-")) { throw Error("invalid SSH host name '%s'", host); + } } void SSHMaster::addCommonSSHOpts(Strings& args) { - for (auto& i : tokenizeString<Strings>(getEnv("NIX_SSHOPTS"))) + for (auto& i : tokenizeString<Strings>(getEnv("NIX_SSHOPTS"))) { args.push_back(i); + } if (!keyFile.empty()) { args.insert(args.end(), {"-i", keyFile}); } @@ -44,12 +46,15 @@ std::unique_ptr<SSHMaster::Connection> SSHMaster::startCommand( close(in.writeSide.get()); close(out.readSide.get()); - if (dup2(in.readSide.get(), STDIN_FILENO) == -1) + if (dup2(in.readSide.get(), STDIN_FILENO) == -1) { throw SysError("duping over stdin"); - if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) + } + if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) { throw SysError("duping over stdout"); - if (logFD != -1 && dup2(logFD, STDERR_FILENO) == -1) + } + if (logFD != -1 && dup2(logFD, STDERR_FILENO) == -1) { throw SysError("duping over stderr"); + } Strings args; @@ -112,8 +117,9 @@ Path SSHMaster::startMaster() { close(out.readSide.get()); - if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) + if (dup2(out.writeSide.get(), STDOUT_FILENO) == -1) { throw SysError("duping over stdout"); + } Strings args = {"ssh", host.c_str(), "-M", "-N", @@ -136,8 +142,9 @@ Path SSHMaster::startMaster() { } catch (EndOfFile& e) { } - if (reply != "started") + if (reply != "started") { throw Error("failed to start SSH master connection to '%s'", host); + } return state->socketPath; } diff --git a/third_party/nix/src/libstore/store-api.cc b/third_party/nix/src/libstore/store-api.cc index 2a7b6a1ac48b..7658a5110092 100644 --- a/third_party/nix/src/libstore/store-api.cc +++ b/third_party/nix/src/libstore/store-api.cc @@ -25,18 +25,21 @@ bool Store::isStorePath(const Path& path) const { } void Store::assertStorePath(const Path& path) const { - if (!isStorePath(path)) + if (!isStorePath(path)) { throw Error(format("path '%1%' is not in the Nix store") % path); + } } Path Store::toStorePath(const Path& path) const { - if (!isInStore(path)) + if (!isInStore(path)) { throw Error(format("path '%1%' is not in the Nix store") % path); + } Path::size_type slash = path.find('/', storeDir.size() + 1); - if (slash == Path::npos) + if (slash == Path::npos) { return path; - else + } else { return Path(path, 0, slash); + } } Path Store::followLinksToStore(const Path& _path) const { @@ -48,8 +51,9 @@ Path Store::followLinksToStore(const Path& _path) const { string target = readLink(path); path = absPath(target, dirOf(path)); } - if (!isInStore(path)) + if (!isInStore(path)) { throw Error(format("path '%1%' is not in the Nix store") % path); + } return path; } @@ -86,17 +90,20 @@ void checkStoreName(const string& name) { /* Disallow names starting with a dot for possible security reasons (e.g., "." and ".."). */ - if (string(name, 0, 1) == ".") + if (string(name, 0, 1) == ".") { throw Error(baseError % "it is illegal to start the name with a period"); + } /* Disallow names longer than 211 characters. ext4’s max is 256, but we need extra space for the hash and .chroot extensions. */ - if (name.length() > 211) + if (name.length() > 211) { throw Error(baseError % "name must be less than 212 characters"); - for (auto& i : name) + } + for (auto& i : name) { if (!((i >= 'A' && i <= 'Z') || (i >= 'a' && i <= 'z') || (i >= '0' && i <= '9') || validChars.find(i) != string::npos)) { throw Error(baseError % (format("the '%1%' character is invalid") % i)); } + } } /* Store paths have the following form: @@ -261,9 +268,10 @@ bool Store::isValidPath(const Path& storePath) { bool valid = isValidPathUncached(storePath); - if (diskCache && !valid) + if (diskCache && !valid) { // FIXME: handle valid = true case. diskCache->upsertNarInfo(getUri(), hashPart, 0); + } return valid; } @@ -306,8 +314,9 @@ void Store::queryPathInfo(const Path& storePath, auto res = state.lock()->pathInfoCache.get(hashPart); if (res) { stats.narInfoReadAverted++; - if (!*res) + if (!*res) { throw InvalidPath(format("path '%s' is not valid") % storePath); + } return callback(ref<ValidPathInfo>(*res)); } } @@ -323,8 +332,9 @@ void Store::queryPathInfo(const Path& storePath, res.first == NarInfoDiskCache::oInvalid ? 0 : res.second); if (res.first == NarInfoDiskCache::oInvalid || (res.second->path != storePath && - storePathToName(storePath) != "")) + storePathToName(storePath) != "")) { throw InvalidPath(format("path '%s' is not valid") % storePath); + } } return callback(ref<ValidPathInfo>(res.second)); } @@ -483,8 +493,9 @@ void Store::pathInfoToJSON(JSONPlaceholder& jsonOut, const PathSet& storePaths, jsonPath.attr("deriver", info->deriver); } - if (info->registrationTime) + if (info->registrationTime) { jsonPath.attr("registrationTime", info->registrationTime); + } if (info->ultimate) { jsonPath.attr("ultimate", info->ultimate); @@ -504,12 +515,15 @@ void Store::pathInfoToJSON(JSONPlaceholder& jsonOut, const PathSet& storePaths, if (!narInfo->url.empty()) { jsonPath.attr("url", narInfo->url); } - if (narInfo->fileHash) + if (narInfo->fileHash) { jsonPath.attr("downloadHash", narInfo->fileHash.to_string()); - if (narInfo->fileSize) + } + if (narInfo->fileSize) { jsonPath.attr("downloadSize", narInfo->fileSize); - if (showClosureSize) + } + if (showClosureSize) { jsonPath.attr("closureDownloadSize", closureSizes.second); + } } } @@ -544,10 +558,11 @@ const Store::Stats& Store::getStats() { } void Store::buildPaths(const PathSet& paths, BuildMode buildMode) { - for (auto& path : paths) + for (auto& path : paths) { if (isDerivation(path)) { unsupported("buildPaths"); } + } if (queryValidPaths(paths).size() != paths.size()) { unsupported("buildPaths"); @@ -621,10 +636,11 @@ void copyPaths(ref<Store> srcStore, ref<Store> dstStore, PathSet valid = dstStore->queryValidPaths(storePaths, substitute); PathSet missing; - for (auto& path : storePaths) + for (auto& path : storePaths) { if (!valid.count(path)) { missing.insert(path); } + } if (missing.empty()) { return; @@ -729,10 +745,11 @@ string showPaths(const PathSet& paths) { } std::string ValidPathInfo::fingerprint() const { - if (narSize == 0 || !narHash) + if (narSize == 0 || !narHash) { throw Error(format("cannot calculate fingerprint of path '%s' because its " "size/hash is not known") % path); + } return "1;" + path + ";" + narHash.to_string(Base32) + ";" + std::to_string(narSize) + ";" + concatStringsSep(",", references); } @@ -749,10 +766,11 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const { if (hasPrefix(ca, "text:")) { Hash hash(std::string(ca, 5)); - if (store.makeTextPath(storePathToName(path), hash, references) == path) + if (store.makeTextPath(storePathToName(path), hash, references) == path) { return true; - else + } else { warn(); + } } else if (hasPrefix(ca, "fixed:")) { @@ -760,10 +778,11 @@ bool ValidPathInfo::isContentAddressed(const Store& store) const { Hash hash(std::string(ca, recursive ? 8 : 6)); if (references.empty() && store.makeFixedOutputPath(recursive, hash, storePathToName(path)) == - path) + path) { return true; - else + } else { warn(); + } } return false; @@ -776,10 +795,11 @@ size_t ValidPathInfo::checkSignatures(const Store& store, } size_t good = 0; - for (auto& sig : sigs) + for (auto& sig : sigs) { if (checkSignature(publicKeys, sig)) { good++; } + } return good; } @@ -838,16 +858,18 @@ std::pair<std::string, Store::Params> splitUriAndParams( std::string decoded; for (size_t i = 0; i < value.size();) { if (value[i] == '%') { - if (i + 2 >= value.size()) + if (i + 2 >= value.size()) { throw Error("invalid URI parameter '%s'", value); + } try { decoded += std::stoul(std::string(value, i + 1, 2), 0, 16); i += 3; } catch (...) { throw Error("invalid URI parameter '%s'", value); } - } else + } else { decoded += value[i++]; + } } params[s.substr(0, e)] = decoded; } @@ -880,9 +902,9 @@ StoreType getStoreType(const std::string& uri, const std::string& stateDir) { } else if (uri == "local" || hasPrefix(uri, "/")) { return tLocal; } else if (uri == "" || uri == "auto") { - if (access(stateDir.c_str(), R_OK | W_OK) == 0) + if (access(stateDir.c_str(), R_OK | W_OK) == 0) { return tLocal; - else if (pathExists(settings.nixDaemonSocketFile)) { + } else if (pathExists(settings.nixDaemonSocketFile)) { return tDaemon; } else { return tLocal; |