From 225e62a56a7cebb030bebffb8d2bd7afe21cc64a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 29 Mar 2020 01:04:55 -0400 Subject: [PATCH 01/44] Replace some `bool recursive` with a new `FileIngestionMethod` enum --- perl/lib/Nix/Store.xs | 4 ++-- src/libexpr/eval.cc | 2 +- src/libexpr/primops.cc | 16 ++++++++-------- src/libexpr/primops/fetchGit.cc | 2 +- src/libexpr/primops/fetchMercurial.cc | 2 +- src/libstore/binary-cache-store.cc | 4 ++-- src/libstore/binary-cache-store.hh | 2 +- src/libstore/build.cc | 12 ++++++------ src/libstore/daemon.cc | 24 ++++++++++++++++-------- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 2 +- src/libstore/download.cc | 9 +++++---- src/libstore/legacy-ssh-store.cc | 2 +- src/libstore/local-store.cc | 12 ++++++------ src/libstore/local-store.hh | 4 ++-- src/libstore/path.hh | 5 +++++ src/libstore/remote-store.cc | 12 +++++++----- src/libstore/remote-store.hh | 2 +- src/libstore/store-api.cc | 18 +++++++++--------- src/libstore/store-api.hh | 11 +++++------ src/nix-prefetch-url/nix-prefetch-url.cc | 9 ++++++--- src/nix-store/nix-store.cc | 8 ++++---- src/nix/add-to-store.cc | 4 ++-- src/nix/command.cc | 10 +++++----- src/nix/command.hh | 4 ++-- src/nix/copy.cc | 2 +- src/nix/make-content-addressable.cc | 4 ++-- 27 files changed, 105 insertions(+), 87 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 1ca734e75..eab8ccacb 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -274,7 +274,7 @@ int checkSignature(SV * publicKey_, SV * sig_, char * msg) SV * addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { - auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, recursive, parseHashType(algo)); + auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, (FileIngestionMethod) recursive, parseHashType(algo)); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); @@ -285,7 +285,7 @@ SV * makeFixedOutputPath(int recursive, char * algo, char * hash, char * name) PPCODE: try { Hash h(hash, parseHashType(algo)); - auto path = store()->makeFixedOutputPath(recursive, h, name); + auto path = store()->makeFixedOutputPath((FileIngestionMethod) recursive, h, name); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index dac32b6f5..88bbf3b32 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1661,7 +1661,7 @@ string EvalState::copyPathToStore(PathSet & context, const Path & path) else { auto p = settings.readOnlyMode ? store->computeStorePathForPath(std::string(baseNameOf(path)), checkSourcePath(path)).first - : store->addToStore(std::string(baseNameOf(path)), checkSourcePath(path), true, htSHA256, defaultPathFilter, repair); + : store->addToStore(std::string(baseNameOf(path)), checkSourcePath(path), FileIngestionMethod::Recursive, htSHA256, defaultPathFilter, repair); dstPath = store->printStorePath(p); srcToStore.insert_or_assign(path, std::move(p)); printMsg(lvlChatty, "copied source '%1%' -> '%2%'", path, dstPath); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 8de234951..7d45733f4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -563,7 +563,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * std::optional outputHash; std::string outputHashAlgo; - bool outputHashRecursive = false; + FileIngestionMethod outputHashRecursive = FileIngestionMethod::Flat; StringSet outputs; outputs.insert("out"); @@ -574,8 +574,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * vomit("processing attribute '%1%'", key); auto handleHashMode = [&](const std::string & s) { - if (s == "recursive") outputHashRecursive = true; - else if (s == "flat") outputHashRecursive = false; + if (s == "recursive") outputHashRecursive = FileIngestionMethod::Recursive; + else if (s == "flat") outputHashRecursive = FileIngestionMethod::Flat; else throw EvalError("invalid value '%s' for 'outputHashMode' attribute, at %s", s, posDrvName); }; @@ -725,7 +725,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput(std::move(outPath), - (outputHashRecursive ? "r:" : "") + printHashType(h.type), + (static_cast(outputHashRecursive) ? "r:" : "") + printHashType(h.type), h.to_string(Base16, false))); } @@ -1038,7 +1038,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu static void addPath(EvalState & state, const Pos & pos, const string & name, const Path & path_, - Value * filterFun, bool recursive, const Hash & expectedHash, Value & v) + Value * filterFun, FileIngestionMethod recursive, const Hash & expectedHash, Value & v) { const auto path = evalSettings.pureEval && expectedHash ? path_ : @@ -1095,7 +1095,7 @@ static void prim_filterSource(EvalState & state, const Pos & pos, Value * * args if (args[0]->type != tLambda) throw TypeError(format("first argument in call to 'filterSource' is not a function but %1%, at %2%") % showType(*args[0]) % pos); - addPath(state, pos, std::string(baseNameOf(path)), path, args[0], true, Hash(), v); + addPath(state, pos, std::string(baseNameOf(path)), path, args[0], FileIngestionMethod::Recursive, Hash(), v); } static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value & v) @@ -1104,7 +1104,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value Path path; string name; Value * filterFun = nullptr; - auto recursive = true; + auto recursive = FileIngestionMethod::Recursive; Hash expectedHash; for (auto & attr : *args[0]->attrs) { @@ -1120,7 +1120,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value state.forceValue(*attr.value); filterFun = attr.value; } else if (n == "recursive") - recursive = state.forceBool(*attr.value, *attr.pos); + recursive = FileIngestionMethod { state.forceBool(*attr.value, *attr.pos) }; else if (n == "sha256") expectedHash = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA256); else diff --git a/src/libexpr/primops/fetchGit.cc b/src/libexpr/primops/fetchGit.cc index 4aee1073e..9f2b673ad 100644 --- a/src/libexpr/primops/fetchGit.cc +++ b/src/libexpr/primops/fetchGit.cc @@ -69,7 +69,7 @@ GitInfo exportGit(ref store, const std::string & uri, return files.count(file); }; - gitInfo.storePath = store->printStorePath(store->addToStore("source", uri, true, htSHA256, filter)); + gitInfo.storePath = store->printStorePath(store->addToStore("source", uri, FileIngestionMethod::Recursive, htSHA256, filter)); return gitInfo; } diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index db274fa4f..548f4e392 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -63,7 +63,7 @@ HgInfo exportMercurial(ref store, const std::string & uri, return files.count(file); }; - hgInfo.storePath = store->printStorePath(store->addToStore("source", uri, true, htSHA256, filter)); + hgInfo.storePath = store->printStorePath(store->addToStore("source", uri, FileIngestionMethod::Recursive, htSHA256, filter)); return hgInfo; } diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 3a2d84861..a61af4a00 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -327,7 +327,7 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, } StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { // FIXME: some cut&paste from LocalStore::addToStore(). @@ -336,7 +336,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath small files. */ StringSink sink; Hash h; - if (recursive) { + if (recursive == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); h = hashString(hashAlgo, *sink.s); } else { diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index aa13c1cb4..1a1ea636c 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -79,7 +79,7 @@ public: std::shared_ptr accessor) override; StorePath addToStore(const string & name, const Path & srcPath, - bool recursive, HashType hashAlgo, + FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) override; StorePath addTextToStore(const string & name, const string & s, diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 0e3a23a4d..224633106 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2712,7 +2712,7 @@ struct RestrictedStore : public LocalFSStore { throw Error("queryPathFromHashPart"); } StorePath addToStore(const string & name, const Path & srcPath, - bool recursive = true, HashType hashAlgo = htSHA256, + FileIngestionMethod recursive = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override { throw Error("addToStore"); } @@ -2725,7 +2725,7 @@ struct RestrictedStore : public LocalFSStore } StorePath addToStoreFromDump(const string & dump, const string & name, - bool recursive = true, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override + FileIngestionMethod recursive = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override { auto path = next->addToStoreFromDump(dump, name, recursive, hashAlgo, repair); goal.addDependency(path); @@ -3647,10 +3647,10 @@ void DerivationGoal::registerOutputs() if (fixedOutput) { - bool recursive; Hash h; + FileIngestionMethod recursive; Hash h; i.second.parseHashInfo(recursive, h); - if (!recursive) { + if (!static_cast(recursive)) { /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( @@ -3659,7 +3659,7 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ - Hash h2 = recursive ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath); + Hash h2 = static_cast(recursive) ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath); auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name()); @@ -3912,7 +3912,7 @@ void DerivationGoal::checkOutputs(const std::map & outputs) auto spec = parseReferenceSpecifiers(worker.store, *drv, *value); - auto used = recursive ? cloneStorePathSet(getClosure(info.path).first) : cloneStorePathSet(info.references); + auto used = static_cast(recursive) ? cloneStorePathSet(getClosure(info.path).first) : cloneStorePathSet(info.references); if (recursive && checks.ignoreSelfRefs) used.erase(info.path); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 8e9f9d71b..f1afdff69 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -355,20 +355,24 @@ static void performOp(TunnelLogger * logger, ref store, } case wopAddToStore: { - bool fixed, recursive; std::string s, baseName; - from >> baseName >> fixed /* obsolete */ >> recursive >> s; - /* Compatibility hack. */ - if (!fixed) { - s = "sha256"; - recursive = true; + FileIngestionMethod method; + { + bool fixed, recursive; + from >> baseName >> fixed /* obsolete */ >> recursive >> s; + method = FileIngestionMethod { recursive }; + /* Compatibility hack. */ + if (!fixed) { + s = "sha256"; + method = FileIngestionMethod::Recursive; + } } HashType hashAlgo = parseHashType(s); TeeSource savedNAR(from); RetrieveRegularNARSink savedRegular; - if (recursive) { + if (method == FileIngestionMethod::Recursive) { /* Get the entire NAR dump from the client and save it to a string so that we can pass it to addToStoreFromDump(). */ @@ -380,7 +384,11 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (!savedRegular.regular) throw Error("regular file expected"); - auto path = store->addToStoreFromDump(recursive ? *savedNAR.data : savedRegular.s, baseName, recursive, hashAlgo); + auto path = store->addToStoreFromDump( + method == FileIngestionMethod::Recursive ? *savedNAR.data : savedRegular.s, + baseName, + method, + hashAlgo); logger->stopWork(); to << store->printStorePath(path); diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 973ddc86a..5934c1912 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -9,13 +9,13 @@ namespace nix { -void DerivationOutput::parseHashInfo(bool & recursive, Hash & hash) const +void DerivationOutput::parseHashInfo(FileIngestionMethod & recursive, Hash & hash) const { - recursive = false; + recursive = FileIngestionMethod::Flat; string algo = hashAlgo; if (string(algo, 0, 2) == "r:") { - recursive = true; + recursive = FileIngestionMethod::Recursive; algo = string(algo, 2); } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 7222d25e5..b1224b93b 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -22,7 +22,7 @@ struct DerivationOutput , hashAlgo(std::move(hashAlgo)) , hash(std::move(hash)) { } - void parseHashInfo(bool & recursive, Hash & hash) const; + void parseHashInfo(FileIngestionMethod & recursive, Hash & hash) const; }; typedef std::map DerivationOutputs; diff --git a/src/libstore/download.cc b/src/libstore/download.cc index 149c84765..8a9b65899 100644 --- a/src/libstore/download.cc +++ b/src/libstore/download.cc @@ -814,7 +814,8 @@ CachedDownloadResult Downloader::downloadCached( std::optional expectedStorePath; if (request.expectedHash) { - expectedStorePath = store->makeFixedOutputPath(request.unpack, request.expectedHash, name); + auto method = request.unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + expectedStorePath = store->makeFixedOutputPath(method, request.expectedHash, name); if (store->isValidPath(*expectedStorePath)) { CachedDownloadResult result; result.storePath = store->printStorePath(*expectedStorePath); @@ -875,10 +876,10 @@ CachedDownloadResult Downloader::downloadCached( StringSink sink; dumpString(*res.data, sink); Hash hash = hashString(request.expectedHash ? request.expectedHash.type : htSHA256, *res.data); - ValidPathInfo info(store->makeFixedOutputPath(false, hash, name)); + ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name)); info.narHash = hashString(htSHA256, *sink.s); info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(false, hash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Flat, hash); store->addToStore(info, sink.s, NoRepair, NoCheckSigs); storePath = info.path.clone(); } @@ -914,7 +915,7 @@ CachedDownloadResult Downloader::downloadCached( if (members.size() != 1) throw nix::Error("tarball '%s' contains an unexpected number of top-level files", url); auto topDir = tmpDir + "/" + members.begin()->name; - unpackedStorePath = store->addToStore(name, topDir, true, htSHA256, defaultPathFilter, NoRepair); + unpackedStorePath = store->addToStore(name, topDir, FileIngestionMethod::Recursive, htSHA256, defaultPathFilter, NoRepair); } replaceSymlink(store->printStorePath(*unpackedStorePath), unpackedLink); storePath = std::move(*unpackedStorePath); diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 458266be0..af20d389b 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -195,7 +195,7 @@ struct LegacySSHStore : public Store { unsupported("queryPathFromHashPart"); } StorePath addToStore(const string & name, const Path & srcPath, - bool recursive, HashType hashAlgo, + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) override { unsupported("addToStore"); } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ae7513ad8..746f81beb 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -557,7 +557,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat if (out == drv.outputs.end()) throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); - bool recursive; Hash h; + FileIngestionMethod recursive; Hash h; out->second.parseHashInfo(recursive, h); check(makeFixedOutputPath(recursive, h, drvName), out->second.path, "out"); @@ -1043,7 +1043,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, StorePath LocalStore::addToStoreFromDump(const string & dump, const string & name, - bool recursive, HashType hashAlgo, RepairFlag repair) + FileIngestionMethod recursive, HashType hashAlgo, RepairFlag repair) { Hash h = hashString(hashAlgo, dump); @@ -1067,7 +1067,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam autoGC(); - if (recursive) { + if (recursive == FileIngestionMethod::Recursive) { StringSource source(dump); restorePath(realPath, source); } else @@ -1080,7 +1080,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam above (if called with recursive == true and hashAlgo == sha256); otherwise, compute it here. */ HashResult hash; - if (recursive) { + if (recursive == FileIngestionMethod::Recursive) { hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); hash.second = dump.size(); } else @@ -1103,7 +1103,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { Path srcPath(absPath(_srcPath)); @@ -1111,7 +1111,7 @@ StorePath 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 == FileIngestionMethod::Recursive) dumpPath(srcPath, sink, filter); else sink.s = make_ref(readFile(srcPath)); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 16aeab0ad..c1e75390c 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -149,7 +149,7 @@ public: std::shared_ptr accessor) override; StorePath addToStore(const string & name, const Path & srcPath, - bool recursive, HashType hashAlgo, + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) override; /* Like addToStore(), but the contents of the path are contained @@ -157,7 +157,7 @@ public: true) or simply the contents of a regular file (if recursive == false). */ StorePath addToStoreFromDump(const string & dump, const string & name, - bool recursive = true, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override; diff --git a/src/libstore/path.hh b/src/libstore/path.hh index c90bb1fff..73c4b8e29 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -73,6 +73,11 @@ const size_t storePathHashLen = 32; // i.e. 160 bits /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; +enum struct FileIngestionMethod : bool { + Flat = false, + Recursive = true +}; + struct StorePathWithOutputs { StorePath path; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 8c55da268..5c36693e6 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -484,7 +484,7 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, StorePath RemoteStore::addToStore(const string & name, const Path & _srcPath, - bool recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { if (repair) throw Error("repairing is not supported when building through the Nix daemon"); @@ -492,10 +492,12 @@ StorePath RemoteStore::addToStore(const string & name, const Path & _srcPath, Path srcPath(absPath(_srcPath)); - conn->to << wopAddToStore << name - << ((hashAlgo == htSHA256 && recursive) ? 0 : 1) /* backwards compatibility hack */ - << (recursive ? 1 : 0) - << printHashType(hashAlgo); + conn->to + << wopAddToStore + << name + << ((hashAlgo == htSHA256 && method == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ + << (method == FileIngestionMethod::Recursive ? 1 : 0) + << printHashType(hashAlgo); try { conn->to.written = 0; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index f301a97d8..3c86b4524 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -65,7 +65,7 @@ public: std::shared_ptr accessor) override; StorePath addToStore(const string & name, const Path & srcPath, - bool recursive = true, HashType hashAlgo = htSHA256, + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override; StorePath addTextToStore(const string & name, const string & s, diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index b9e894a9a..06aa0883c 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -171,18 +171,18 @@ static std::string makeType( StorePath Store::makeFixedOutputPath( - bool recursive, + FileIngestionMethod recursive, const Hash & hash, std::string_view name, const StorePathSet & references, bool hasSelfReference) const { - if (hash.type == htSHA256 && recursive) { + if (hash.type == htSHA256 && recursive == FileIngestionMethod::Recursive) { return makeStorePath(makeType(*this, "source", references, hasSelfReference), hash, name); } else { assert(references.empty()); return makeStorePath("output:out", hashString(htSHA256, - "fixed:out:" + (recursive ? (string) "r:" : "") + + "fixed:out:" + (static_cast(recursive) ? (string) "r:" : "") + hash.to_string(Base16) + ":"), name); } } @@ -200,9 +200,9 @@ StorePath Store::makeTextPath(std::string_view name, const Hash & hash, std::pair Store::computeStorePathForPath(std::string_view name, - const Path & srcPath, bool recursive, HashType hashAlgo, PathFilter & filter) const + const Path & srcPath, FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter) const { - Hash h = recursive ? hashPath(hashAlgo, srcPath, filter).first : hashFile(hashAlgo, srcPath); + Hash h = static_cast(recursive) ? hashPath(hashAlgo, srcPath, filter).first : hashFile(hashAlgo, srcPath); return std::make_pair(makeFixedOutputPath(recursive, h, name), h); } @@ -781,8 +781,8 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const } else if (hasPrefix(ca, "fixed:")) { - bool recursive = ca.compare(6, 2, "r:") == 0; - Hash hash(std::string(ca, recursive ? 8 : 6)); + FileIngestionMethod recursive { ca.compare(6, 2, "r:") == 0 }; + Hash hash(std::string(ca, static_cast(recursive) ? 8 : 6)); auto refs = cloneStorePathSet(references); bool hasSelfReference = false; if (refs.count(path)) { @@ -826,9 +826,9 @@ Strings ValidPathInfo::shortRefs() const } -std::string makeFixedOutputCA(bool recursive, const Hash & hash) +std::string makeFixedOutputCA(FileIngestionMethod recursive, const Hash & hash) { - return "fixed:" + (recursive ? (std::string) "r:" : "") + hash.to_string(); + return "fixed:" + (static_cast(recursive) ? (std::string) "r:" : "") + hash.to_string(); } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 0fa59be6a..32c24c500 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -43,7 +43,6 @@ enum CheckSigsFlag : bool { NoCheckSigs = false, CheckSigs = true }; enum SubstituteFlag : bool { NoSubstitute = false, Substitute = true }; enum AllowInvalidFlag : bool { DisallowInvalid = false, AllowInvalid = true }; - /* Magic header of exportPath() output (obsolete). */ const uint32_t exportMagic = 0x4558494e; @@ -346,7 +345,7 @@ public: StorePath makeOutputPath(const string & id, const Hash & hash, std::string_view name) const; - StorePath makeFixedOutputPath(bool recursive, + StorePath makeFixedOutputPath(FileIngestionMethod method, const Hash & hash, std::string_view name, const StorePathSet & references = {}, bool hasSelfReference = false) const; @@ -358,7 +357,7 @@ public: store path to which srcPath is to be copied. Returns the store path and the cryptographic hash of the contents of srcPath. */ std::pair computeStorePathForPath(std::string_view name, - const Path & srcPath, bool recursive = true, + const Path & srcPath, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter) const; /* Preparatory part of addTextToStore(). @@ -462,12 +461,12 @@ public: The function object `filter' can be used to exclude files (see libutil/archive.hh). */ virtual StorePath addToStore(const string & name, const Path & srcPath, - bool recursive = true, HashType hashAlgo = htSHA256, + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) = 0; // FIXME: remove? virtual StorePath addToStoreFromDump(const string & dump, const string & name, - bool recursive = true, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) { throw Error("addToStoreFromDump() is not supported by this store"); } @@ -850,7 +849,7 @@ std::optional decodeValidPathInfo( /* Compute the content-addressability assertion (ValidPathInfo::ca) for paths created by makeFixedOutputPath() / addToStore(). */ -std::string makeFixedOutputCA(bool recursive, const Hash & hash); +std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash); /* Split URI into protocol+hierarchy part and its parameter set. */ diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 18ced94b1..8c364a86a 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -159,7 +159,8 @@ static int _main(int argc, char * * argv) std::optional storePath; if (args.size() == 2) { expectedHash = Hash(args[1], ht); - storePath = store->makeFixedOutputPath(unpack, expectedHash, name); + const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + storePath = store->makeFixedOutputPath(recursive, expectedHash, name); if (store->isValidPath(*storePath)) hash = expectedHash; else @@ -208,13 +209,15 @@ static int _main(int argc, char * * argv) if (expectedHash != Hash(ht) && expectedHash != hash) throw Error(format("hash mismatch for '%1%'") % uri); + const auto recursive = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + /* Copy the file to the Nix store. FIXME: if RemoteStore implemented addToStoreFromDump() and downloadFile() supported a sink, we could stream the download directly into the Nix store. */ - storePath = store->addToStore(name, tmpFile, unpack, ht); + storePath = store->addToStore(name, tmpFile, recursive, ht); - assert(*storePath == store->makeFixedOutputPath(unpack, hash, name)); + assert(*storePath == store->makeFixedOutputPath(recursive, hash, name)); } stopProgressBar(); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 806ab7563..9a26eb57f 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -174,10 +174,10 @@ static void opAdd(Strings opFlags, Strings opArgs) store. */ static void opAddFixed(Strings opFlags, Strings opArgs) { - bool recursive = false; + FileIngestionMethod recursive = FileIngestionMethod::Flat; for (auto & i : opFlags) - if (i == "--recursive") recursive = true; + if (i == "--recursive") recursive = FileIngestionMethod::Recursive; else throw UsageError(format("unknown flag '%1%'") % i); if (opArgs.empty()) @@ -194,10 +194,10 @@ static void opAddFixed(Strings opFlags, Strings opArgs) /* Hack to support caching in `nix-prefetch-url'. */ static void opPrintFixedPath(Strings opFlags, Strings opArgs) { - bool recursive = false; + FileIngestionMethod recursive = FileIngestionMethod::Flat; for (auto i : opFlags) - if (i == "--recursive") recursive = true; + if (i == "--recursive") recursive = FileIngestionMethod::Recursive; else throw UsageError(format("unknown flag '%1%'") % i); if (opArgs.size() != 3) diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc index 139db3657..e82ab844e 100644 --- a/src/nix/add-to-store.cc +++ b/src/nix/add-to-store.cc @@ -42,10 +42,10 @@ struct CmdAddToStore : MixDryRun, StoreCommand auto narHash = hashString(htSHA256, *sink.s); - ValidPathInfo info(store->makeFixedOutputPath(true, narHash, *namePart)); + ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, *namePart)); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(true, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); if (!dryRun) store->addToStore(info, sink.s); diff --git a/src/nix/command.cc b/src/nix/command.cc index 442bc6c53..fce6c391c 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -28,20 +28,20 @@ void StoreCommand::run() run(getStore()); } -StorePathsCommand::StorePathsCommand(bool recursive) +StorePathsCommand::StorePathsCommand(FileIngestionMethod recursive) : recursive(recursive) { - if (recursive) + if (recursive == FileIngestionMethod::Recursive) mkFlag() .longName("no-recursive") .description("apply operation to specified paths only") - .set(&this->recursive, false); + .set(&this->recursive, FileIngestionMethod::Flat); else mkFlag() .longName("recursive") .shortName('r') .description("apply operation to closure of the specified paths") - .set(&this->recursive, true); + .set(&this->recursive, FileIngestionMethod::Recursive); mkFlag(0, "all", "apply operation to the entire store", &all); } @@ -61,7 +61,7 @@ void StorePathsCommand::run(ref store) for (auto & p : toStorePaths(store, realiseMode, installables)) storePaths.push_back(p.clone()); - if (recursive) { + if (recursive == FileIngestionMethod::Recursive) { StorePathSet closure; store->computeFSClosure(storePathsToSet(storePaths), closure, false, false); storePaths.clear(); diff --git a/src/nix/command.hh b/src/nix/command.hh index a954a7d04..4cdda9f79 100644 --- a/src/nix/command.hh +++ b/src/nix/command.hh @@ -114,7 +114,7 @@ struct StorePathsCommand : public InstallablesCommand { private: - bool recursive = false; + FileIngestionMethod recursive = FileIngestionMethod::Flat; bool all = false; protected: @@ -123,7 +123,7 @@ protected: public: - StorePathsCommand(bool recursive = false); + StorePathsCommand(FileIngestionMethod recursive = FileIngestionMethod::Flat); using StoreCommand::run; diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 85c777d38..a8ee3fce3 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -17,7 +17,7 @@ struct CmdCopy : StorePathsCommand SubstituteFlag substitute = NoSubstitute; CmdCopy() - : StorePathsCommand(true) + : StorePathsCommand(FileIngestionMethod::Recursive) { mkFlag() .longName("from") diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index f9c7fef3f..93eddbb1f 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -74,12 +74,12 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON auto narHash = hashModuloSink.finish().first; - ValidPathInfo info(store->makeFixedOutputPath(true, narHash, path.name(), references, hasSelfReference)); + ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference)); info.references = std::move(references); if (hasSelfReference) info.references.insert(info.path.clone()); info.narHash = narHash; info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(true, info.narHash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Recursive, info.narHash); if (!json) printError("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); From bbbb7c1bc7a66e1169931cda8bd863b68236726a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 30 Mar 2020 18:15:55 -0400 Subject: [PATCH 02/44] Use `auto` with some `FileIngestionMethod` local variables --- src/libexpr/primops.cc | 2 +- src/nix-store/nix-store.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 7d45733f4..71a511c41 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -563,7 +563,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * std::optional outputHash; std::string outputHashAlgo; - FileIngestionMethod outputHashRecursive = FileIngestionMethod::Flat; + auto outputHashRecursive = FileIngestionMethod::Flat; StringSet outputs; outputs.insert("out"); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 9a26eb57f..cde7d561b 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -174,7 +174,7 @@ static void opAdd(Strings opFlags, Strings opArgs) store. */ static void opAddFixed(Strings opFlags, Strings opArgs) { - FileIngestionMethod recursive = FileIngestionMethod::Flat; + auto recursive = FileIngestionMethod::Flat; for (auto & i : opFlags) if (i == "--recursive") recursive = FileIngestionMethod::Recursive; @@ -194,7 +194,7 @@ static void opAddFixed(Strings opFlags, Strings opArgs) /* Hack to support caching in `nix-prefetch-url'. */ static void opPrintFixedPath(Strings opFlags, Strings opArgs) { - FileIngestionMethod recursive = FileIngestionMethod::Flat; + auto recursive = FileIngestionMethod::Flat; for (auto i : opFlags) if (i == "--recursive") recursive = FileIngestionMethod::Recursive; From 51afea3af23640a3fb2cfe8cfae5cf3cc27eb3a1 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 30 Mar 2020 22:31:51 +0000 Subject: [PATCH 03/44] Never cast `FileIngestionMethod` to or from boolean --- src/libexpr/primops.cc | 9 ++++++--- src/libstore/build.cc | 10 +++++++--- src/libstore/path.hh | 2 +- src/libstore/store-api.cc | 21 ++++++++++++++------- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 71a511c41..4d5aaf86a 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -724,9 +724,12 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign("out", DerivationOutput(std::move(outPath), - (static_cast(outputHashRecursive) ? "r:" : "") + printHashType(h.type), - h.to_string(Base16, false))); + drv.outputs.insert_or_assign("out", DerivationOutput { + std::move(outPath), + (outputHashRecursive == FileIngestionMethod::Recursive ? "r:" : "") + + printHashType(h.type), + h.to_string(Base16, false), + }); } else { diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 224633106..be52e637e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3650,7 +3650,7 @@ void DerivationGoal::registerOutputs() FileIngestionMethod recursive; Hash h; i.second.parseHashInfo(recursive, h); - if (!static_cast(recursive)) { + if (recursive == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( @@ -3659,7 +3659,9 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ - Hash h2 = static_cast(recursive) ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath); + Hash h2 = recursive == FileIngestionMethod::Recursive + ? hashPath(h.type, actualPath).first + : hashFile(h.type, actualPath); auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name()); @@ -3912,7 +3914,9 @@ void DerivationGoal::checkOutputs(const std::map & outputs) auto spec = parseReferenceSpecifiers(worker.store, *drv, *value); - auto used = static_cast(recursive) ? cloneStorePathSet(getClosure(info.path).first) : cloneStorePathSet(info.references); + auto used = recursive + ? cloneStorePathSet(getClosure(info.path).first) + : cloneStorePathSet(info.references); if (recursive && checks.ignoreSelfRefs) used.erase(info.path); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 73c4b8e29..5122e7422 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -73,7 +73,7 @@ const size_t storePathHashLen = 32; // i.e. 160 bits /* Extension of derivations in the Nix store. */ const std::string drvExtension = ".drv"; -enum struct FileIngestionMethod : bool { +enum struct FileIngestionMethod : uint8_t { Flat = false, Recursive = true }; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 06aa0883c..ca1409588 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -177,13 +177,16 @@ StorePath Store::makeFixedOutputPath( const StorePathSet & references, bool hasSelfReference) const { - if (hash.type == htSHA256 && recursive == FileIngestionMethod::Recursive) { + if (hash.type == htSHA256 && recursive == FileIngestionMethod::Recursive) { return makeStorePath(makeType(*this, "source", references, hasSelfReference), hash, name); } else { assert(references.empty()); - return makeStorePath("output:out", hashString(htSHA256, - "fixed:out:" + (static_cast(recursive) ? (string) "r:" : "") + - hash.to_string(Base16) + ":"), name); + return makeStorePath("output:out", + hashString(htSHA256, + "fixed:out:" + + (recursive == FileIngestionMethod::Recursive ? (string) "r:" : "") + + hash.to_string(Base16) + ":"), + name); } } @@ -202,7 +205,9 @@ StorePath Store::makeTextPath(std::string_view name, const Hash & hash, std::pair Store::computeStorePathForPath(std::string_view name, const Path & srcPath, FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter) const { - Hash h = static_cast(recursive) ? hashPath(hashAlgo, srcPath, filter).first : hashFile(hashAlgo, srcPath); + Hash h = recursive == FileIngestionMethod::Recursive + ? hashPath(hashAlgo, srcPath, filter).first + : hashFile(hashAlgo, srcPath); return std::make_pair(makeFixedOutputPath(recursive, h, name), h); } @@ -782,7 +787,7 @@ bool ValidPathInfo::isContentAddressed(const Store & store) const else if (hasPrefix(ca, "fixed:")) { FileIngestionMethod recursive { ca.compare(6, 2, "r:") == 0 }; - Hash hash(std::string(ca, static_cast(recursive) ? 8 : 6)); + Hash hash(std::string(ca, recursive == FileIngestionMethod::Recursive ? 8 : 6)); auto refs = cloneStorePathSet(references); bool hasSelfReference = false; if (refs.count(path)) { @@ -828,7 +833,9 @@ Strings ValidPathInfo::shortRefs() const std::string makeFixedOutputCA(FileIngestionMethod recursive, const Hash & hash) { - return "fixed:" + (static_cast(recursive) ? (std::string) "r:" : "") + hash.to_string(); + return "fixed:" + + (recursive == FileIngestionMethod::Recursive ? (std::string) "r:" : "") + + hash.to_string(); } From 7e9a2718f0c39fde53a3447c3b7f3617a4a9b958 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 30 Mar 2020 22:36:15 +0000 Subject: [PATCH 04/44] s/outputHashRecursive/ingestionMethod/c --- src/libexpr/primops.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4d5aaf86a..415e77fe7 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -563,7 +563,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * std::optional outputHash; std::string outputHashAlgo; - auto outputHashRecursive = FileIngestionMethod::Flat; + auto ingestionMethod = FileIngestionMethod::Flat; StringSet outputs; outputs.insert("out"); @@ -574,8 +574,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * vomit("processing attribute '%1%'", key); auto handleHashMode = [&](const std::string & s) { - if (s == "recursive") outputHashRecursive = FileIngestionMethod::Recursive; - else if (s == "flat") outputHashRecursive = FileIngestionMethod::Flat; + if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive; + else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat; else throw EvalError("invalid value '%s' for 'outputHashMode' attribute, at %s", s, posDrvName); }; @@ -722,11 +722,11 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * HashType ht = outputHashAlgo.empty() ? htUnknown : parseHashType(outputHashAlgo); Hash h(*outputHash, ht); - auto outPath = state.store->makeFixedOutputPath(outputHashRecursive, h, drvName); + auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); if (!jsonObject) drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput { std::move(outPath), - (outputHashRecursive == FileIngestionMethod::Recursive ? "r:" : "") + (ingestionMethod == FileIngestionMethod::Recursive ? "r:" : "") + printHashType(h.type), h.to_string(Base16, false), }); From 8aa46cd340c1294c3d06cd52f85c906bdf749070 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 30 Mar 2020 22:40:41 +0000 Subject: [PATCH 05/44] Get rid of FileIngestionMethod casts in perl bindings, too --- perl/lib/Nix/Store.xs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index eab8ccacb..890310b3e 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -274,7 +274,8 @@ int checkSignature(SV * publicKey_, SV * sig_, char * msg) SV * addToStore(char * srcPath, int recursive, char * algo) PPCODE: try { - auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, (FileIngestionMethod) recursive, parseHashType(algo)); + auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + auto path = store()->addToStore(std::string(baseNameOf(srcPath)), srcPath, method, parseHashType(algo)); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); @@ -285,7 +286,8 @@ SV * makeFixedOutputPath(int recursive, char * algo, char * hash, char * name) PPCODE: try { Hash h(hash, parseHashType(algo)); - auto path = store()->makeFixedOutputPath((FileIngestionMethod) recursive, h, name); + auto method = recursive ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + auto path = store()->makeFixedOutputPath(method, h, name); XPUSHs(sv_2mortal(newSVpv(store()->printStorePath(path).c_str(), 0))); } catch (Error & e) { croak("%s", e.what()); From 04967dee9d08befe4661e6fa5da4a00da0538a13 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Tue, 5 May 2020 13:04:36 +0300 Subject: [PATCH 06/44] Wait for build users when none are available --- src/libstore/build.cc | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 147093fae..1b7e6d75e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,6 +507,9 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; + bool findFreeUser(); + + string user; uid_t uid; gid_t gid; @@ -526,10 +529,19 @@ public: }; - UserLock::UserLock() { assert(settings.buildUsersGroup != ""); + createDirs(settings.nixStateDir + "/userpool"); + + if (findFreeUser()) return; + + printError("waiting for build users"); + + do std::this_thread::sleep_for(std::chrono::seconds(2)); while (! findFreeUser()); +} + +bool UserLock::findFreeUser() { /* Get the members of the build-users-group. */ struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str()); @@ -559,7 +571,6 @@ UserLock::UserLock() throw Error(format("the user '%1%' in the group '%2%' does not exist") % i % settings.buildUsersGroup); - createDirs(settings.nixStateDir + "/userpool"); fnUserLock = (format("%1%/userpool/%2%") % settings.nixStateDir % pw->pw_uid).str(); @@ -590,16 +601,12 @@ UserLock::UserLock() supplementaryGIDs.resize(ngroups); #endif - return; + return true; } } - - throw Error(format("all build users are currently in use; " - "consider creating additional users and adding them to the '%1%' group") - % settings.buildUsersGroup); + return false; } - void UserLock::kill() { killUser(uid); From 14073fb76be0f46cb9335edf747fcfa1a5275b7b Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Fri, 8 May 2020 12:22:39 +0300 Subject: [PATCH 07/44] Don't block while waiting for build users --- src/libstore/build.cc | 54 +++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 25 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 1b7e6d75e..00101f553 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,9 +507,6 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; - bool findFreeUser(); - - string user; uid_t uid; gid_t gid; @@ -525,20 +522,19 @@ public: uid_t getGID() { assert(gid); return gid; } std::vector getSupplementaryGIDs() { return supplementaryGIDs; } + bool findFreeUser(); + bool enabled() { return uid != 0; } }; + UserLock::UserLock() { assert(settings.buildUsersGroup != ""); createDirs(settings.nixStateDir + "/userpool"); - - if (findFreeUser()) return; - - printError("waiting for build users"); - - do std::this_thread::sleep_for(std::chrono::seconds(2)); while (! findFreeUser()); + /* Mark that user is not enabled by default */ + uid = 0; } bool UserLock::findFreeUser() { @@ -1398,6 +1394,30 @@ void DerivationGoal::tryToBuild() { trace("trying to build"); + /* If `build-users-group' is not empty, then we have to build as + one of the members of that group. */ + if (settings.buildUsersGroup != "" && getuid() == 0) { +#if defined(__linux__) || defined(__APPLE__) + if (!buildUser) buildUser = std::make_unique(); + + if (!buildUser->enabled()) { + if (!buildUser->findFreeUser()) { + debug("waiting for build users"); + worker.waitForAWhile(shared_from_this()); + return; + } + + /* Make sure that no other processes are executing under this + uid. */ + buildUser->kill(); + } +#else + /* Don't know how to block the creation of setuid/setgid + binaries on this platform. */ + throw Error("build users are not supported on this platform for security reasons"); +#endif + } + /* Obtain locks on all output paths. The locks are automatically released when we exit this function or Nix crashes. If we can't acquire the lock, then continue; hopefully some other @@ -1950,22 +1970,6 @@ void DerivationGoal::startBuilder() #endif } - /* If `build-users-group' is not empty, then we have to build as - one of the members of that group. */ - if (settings.buildUsersGroup != "" && getuid() == 0) { -#if defined(__linux__) || defined(__APPLE__) - buildUser = std::make_unique(); - - /* Make sure that no other processes are executing under this - uid. */ - buildUser->kill(); -#else - /* Don't know how to block the creation of setuid/setgid - binaries on this platform. */ - throw Error("build users are not supported on this platform for security reasons"); -#endif - } - /* Create a temporary directory where the build will take place. */ tmpDir = createTempDir("", "nix-build-" + std::string(drvPath.name()), false, false, 0700); From 772e5db828c9924c803b4d126bca72d7e123614b Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Fri, 8 May 2020 12:29:00 +0300 Subject: [PATCH 08/44] Mention build users in the 'waiting for' message --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 00101f553..a8c1cd565 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -4830,7 +4830,7 @@ void Worker::waitForInput() if (!waitingForAWhile.empty()) { useTimeout = true; if (lastWokenUp == steady_time_point::min()) - printError("waiting for locks or build slots..."); + printError("waiting for locks, build slots or build users..."); if (lastWokenUp == steady_time_point::min() || lastWokenUp > before) lastWokenUp = before; timeout = std::max(1L, (long) std::chrono::duration_cast( From 183dd28266e0292116da6cfbc3e2643577adbfbe Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Thu, 14 May 2020 17:00:54 +0300 Subject: [PATCH 09/44] Don't lock a user while doing remote builds --- src/libstore/build.cc | 88 ++++++++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index a8c1cd565..f7fc23b35 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,6 +507,7 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; + bool isEnabled; string user; uid_t uid; gid_t gid; @@ -524,7 +525,7 @@ public: bool findFreeUser(); - bool enabled() { return uid != 0; } + bool enabled() { return isEnabled; } }; @@ -535,9 +536,11 @@ UserLock::UserLock() createDirs(settings.nixStateDir + "/userpool"); /* Mark that user is not enabled by default */ uid = 0; + isEnabled = false; } bool UserLock::findFreeUser() { + if (enabled()) return true; /* Get the members of the build-users-group. */ struct group * gr = getgrnam(settings.buildUsersGroup.get().c_str()); @@ -597,6 +600,7 @@ bool UserLock::findFreeUser() { supplementaryGIDs.resize(ngroups); #endif + isEnabled = true; return true; } } @@ -931,6 +935,7 @@ private: void closureRepaired(); void inputsRealised(); void tryToBuild(); + void tryLocalBuild(); void buildDone(); /* Is the build hook willing to perform the build? */ @@ -1002,6 +1007,8 @@ private: Goal::amDone(result); } + void started(); + void done(BuildResult::Status status, const string & msg = ""); StorePathSet exportReferences(const StorePathSet & storePaths); @@ -1389,35 +1396,24 @@ void DerivationGoal::inputsRealised() result = BuildResult(); } +void DerivationGoal::started() { + auto msg = fmt( + buildMode == bmRepair ? "repairing outputs of '%s'" : + buildMode == bmCheck ? "checking outputs of '%s'" : + nrRounds > 1 ? "building '%s' (round %d/%d)" : + "building '%s'", worker.store.printStorePath(drvPath), curRound, nrRounds); + fmt("building '%s'", worker.store.printStorePath(drvPath)); + if (hook) msg += fmt(" on '%s'", machineName); + act = std::make_unique(*logger, lvlInfo, actBuild, msg, + Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", curRound, nrRounds}); + mcRunningBuilds = std::make_unique>(worker.runningBuilds); + worker.updateProgress(); +} void DerivationGoal::tryToBuild() { trace("trying to build"); - /* If `build-users-group' is not empty, then we have to build as - one of the members of that group. */ - if (settings.buildUsersGroup != "" && getuid() == 0) { -#if defined(__linux__) || defined(__APPLE__) - if (!buildUser) buildUser = std::make_unique(); - - if (!buildUser->enabled()) { - if (!buildUser->findFreeUser()) { - debug("waiting for build users"); - worker.waitForAWhile(shared_from_this()); - return; - } - - /* Make sure that no other processes are executing under this - uid. */ - buildUser->kill(); - } -#else - /* Don't know how to block the creation of setuid/setgid - binaries on this platform. */ - throw Error("build users are not supported on this platform for security reasons"); -#endif - } - /* Obtain locks on all output paths. The locks are automatically released when we exit this function or Nix crashes. If we can't acquire the lock, then continue; hopefully some other @@ -1464,20 +1460,6 @@ void DerivationGoal::tryToBuild() supported for local builds. */ bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(); - auto started = [&]() { - auto msg = fmt( - buildMode == bmRepair ? "repairing outputs of '%s'" : - buildMode == bmCheck ? "checking outputs of '%s'" : - nrRounds > 1 ? "building '%s' (round %d/%d)" : - "building '%s'", worker.store.printStorePath(drvPath), curRound, nrRounds); - fmt("building '%s'", worker.store.printStorePath(drvPath)); - if (hook) msg += fmt(" on '%s'", machineName); - act = std::make_unique(*logger, lvlInfo, actBuild, msg, - Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", curRound, nrRounds}); - mcRunningBuilds = std::make_unique>(worker.runningBuilds); - worker.updateProgress(); - }; - /* Is the build hook willing to accept this job? */ if (!buildLocally) { switch (tryBuildHook()) { @@ -1510,6 +1492,34 @@ void DerivationGoal::tryToBuild() return; } + state = &DerivationGoal::tryLocalBuild; + worker.wakeUp(shared_from_this()); +} + +void DerivationGoal::tryLocalBuild() { + + /* If `build-users-group' is not empty, then we have to build as + one of the members of that group. */ + if (settings.buildUsersGroup != "" && getuid() == 0) { +#if defined(__linux__) || defined(__APPLE__) + if (!buildUser) buildUser = std::make_unique(); + + if (buildUser->findFreeUser()) { + /* Make sure that no other processes are executing under this + uid. */ + buildUser->kill(); + } else { + debug("waiting for build users"); + worker.waitForAWhile(shared_from_this()); + return; + } +#else + /* Don't know how to block the creation of setuid/setgid + binaries on this platform. */ + throw Error("build users are not supported on this platform for security reasons"); +#endif + } + try { /* Okay, we have to build. */ From e223eeac09c6468db80ee0497c84960b4ec73e02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Sat, 16 May 2020 08:40:55 +0100 Subject: [PATCH 10/44] Remove -j option from simple-build-testing By default Nix/NixOS already set a reasonable default `max-jobs = auto` so we don't need to mention it in this tutorial. The option is still documented in other parts of the documentation if users ever stumble over this. Fixes https://github.com/NixOS/nix/issues/2531 --- doc/manual/expressions/simple-building-testing.xml | 8 -------- 1 file changed, 8 deletions(-) diff --git a/doc/manual/expressions/simple-building-testing.xml b/doc/manual/expressions/simple-building-testing.xml index 7326a3e76..ce0a1636d 100644 --- a/doc/manual/expressions/simple-building-testing.xml +++ b/doc/manual/expressions/simple-building-testing.xml @@ -73,12 +73,4 @@ waiting for lock on `/nix/store/0h5b7hp8d4hqfrw8igvx97x1xawrjnac-hello-2.1.1x'make). -If you have a system with multiple CPUs, you may want to have -Nix build different derivations in parallel (insofar as possible). -Just pass the option , where -N is the maximum number of jobs to be run -in parallel, or set. Typically this should be the number of -CPUs. - From 5ef64f05e6e493d5fe69c87127328f68500b9e50 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 18 May 2020 15:50:29 +0200 Subject: [PATCH 11/44] Cleanup --- src/libstore/build.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 4f2f38d63..a2b16f95c 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -507,10 +507,10 @@ private: Path fnUserLock; AutoCloseFD fdUserLock; - bool isEnabled; + bool isEnabled = false; string user; - uid_t uid; - gid_t gid; + uid_t uid = 0; + gid_t gid = 0; std::vector supplementaryGIDs; public: @@ -534,9 +534,6 @@ UserLock::UserLock() { assert(settings.buildUsersGroup != ""); createDirs(settings.nixStateDir + "/userpool"); - /* Mark that user is not enabled by default */ - uid = 0; - isEnabled = false; } bool UserLock::findFreeUser() { From a73a820a5d73e0c595ff807a752f76ef1184ca2d Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Wed, 20 May 2020 16:27:53 +0200 Subject: [PATCH 12/44] Add unit testes for url.cc This adds tests for - parseURL - percentDecode - decodeQuery --- src/libutil/tests/tests.cc | 6 +- src/libutil/tests/url.cc | 266 +++++++++++++++++++++++++++++++++++++ 2 files changed, 271 insertions(+), 1 deletion(-) create mode 100644 src/libutil/tests/url.cc diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 5abfe2c9c..8e77ccbe1 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -5,6 +5,8 @@ namespace nix { +/* ----------- tests for util.hh ------------------------------------------------*/ + /* ---------------------------------------------------------------------------- * absPath * --------------------------------------------------------------------------*/ @@ -15,6 +17,9 @@ namespace nix { ASSERT_EQ(p, "/"); } + + + TEST(absPath, turnsEmptyPathIntoCWD) { char cwd[PATH_MAX+1]; auto p = absPath(""); @@ -581,5 +586,4 @@ namespace nix { ASSERT_EQ(filterANSIEscapes(s, true), "foo bar baz" ); } - } diff --git a/src/libutil/tests/url.cc b/src/libutil/tests/url.cc new file mode 100644 index 000000000..80646ad3e --- /dev/null +++ b/src/libutil/tests/url.cc @@ -0,0 +1,266 @@ +#include "url.hh" +#include + +namespace nix { + +/* ----------- tests for url.hh --------------------------------------------------*/ + + string print_map(std::map m) { + std::map::iterator it; + string s = "{ "; + for (it = m.begin(); it != m.end(); ++it) { + s += "{ "; + s += it->first; + s += " = "; + s += it->second; + s += " } "; + } + s += "}"; + return s; + } + + + std::ostream& operator<<(std::ostream& os, const ParsedURL& p) { + return os << "\n" + << "url: " << p.url << "\n" + << "base: " << p.base << "\n" + << "scheme: " << p.scheme << "\n" + << "authority: " << p.authority.value() << "\n" + << "path: " << p.path << "\n" + << "query: " << print_map(p.query) << "\n" + << "fragment: " << p.fragment << "\n"; + } + + TEST(parseURL, parsesSimpleHttpUrl) { + auto s = "http://www.example.org/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://www.example.org/file.tar.gz", + .base = "http://www.example.org/file.tar.gz", + .scheme = "http", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpsUrl) { + auto s = "https://www.example.org/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "https://www.example.org/file.tar.gz", + .base = "https://www.example.org/file.tar.gz", + .scheme = "https", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpUrlWithQueryAndFragment) { + auto s = "https://www.example.org/file.tar.gz?download=fast&when=now#hello"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "https://www.example.org/file.tar.gz", + .base = "https://www.example.org/file.tar.gz", + .scheme = "https", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { { "download", "fast" }, { "when", "now" } }, + .fragment = "hello", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesSimpleHttpUrlWithComplexFragment) { + auto s = "http://www.example.org/file.tar.gz?field=value#?foo=bar%23"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://www.example.org/file.tar.gz", + .base = "http://www.example.org/file.tar.gz", + .scheme = "http", + .authority = "www.example.org", + .path = "/file.tar.gz", + .query = (StringMap) { { "field", "value" } }, + .fragment = "?foo=bar#", + }; + + ASSERT_EQ(parsed, expected); + } + + + TEST(parseURL, parseIPv4Address) { + auto s = "http://127.0.0.1:8080/file.tar.gz?download=fast&when=now#hello"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://127.0.0.1:8080/file.tar.gz", + .base = "https://127.0.0.1:8080/file.tar.gz", + .scheme = "http", + .authority = "127.0.0.1:8080", + .path = "/file.tar.gz", + .query = (StringMap) { { "download", "fast" }, { "when", "now" } }, + .fragment = "hello", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parseIPv6Address) { + auto s = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .scheme = "http", + .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", + .path = "", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + + } + + TEST(parseURL, parseEmptyQueryParams) { + auto s = "http://127.0.0.1:8080/file.tar.gz?&&&&&"; + auto parsed = parseURL(s); + ASSERT_EQ(parsed.query, (StringMap) { }); + } + + TEST(parseURL, parseUserPassword) { + auto s = "http://user:pass@www.example.org:8080/file.tar.gz"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "http://user:pass@www.example.org/file.tar.gz", + .base = "http://user:pass@www.example.org/file.tar.gz", + .scheme = "http", + .authority = "user:pass@www.example.org:8080", + .path = "/file.tar.gz", + .query = (StringMap) { }, + .fragment = "", + }; + + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parseFileURLWithQueryAndFragment) { + auto s = "file:///none/of/your/business"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "", + .base = "", + .scheme = "file", + .authority = "", + .path = "/none/of/your/business", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + + } + + TEST(parseURL, parsedUrlsIsEqualToItself) { + auto s = "http://www.example.org/file.tar.gz"; + auto url = parseURL(s); + + ASSERT_TRUE(url == url); + } + + TEST(parseURL, parseFTPUrl) { + auto s = "ftp://ftp.nixos.org/downloads/nixos.iso"; + auto parsed = parseURL(s); + + ParsedURL expected { + .url = "ftp://ftp.nixos.org/downloads/nixos.iso", + .base = "ftp://ftp.nixos.org/downloads/nixos.iso", + .scheme = "ftp", + .authority = "ftp.nixos.org", + .path = "/downloads/nixos.iso", + .query = (StringMap) { }, + .fragment = "", + }; + + ASSERT_EQ(parsed, expected); + } + + TEST(parseURL, parsesAnythingInUriFormat) { + auto s = "whatever://github.com/NixOS/nixpkgs.git"; + auto parsed = parseURL(s); + } + + TEST(parseURL, parsesAnythingInUriFormatWithoutDoubleSlash) { + auto s = "whatever:github.com/NixOS/nixpkgs.git"; + auto parsed = parseURL(s); + } + + TEST(parseURL, emptyStringIsInvalidURL) { + ASSERT_THROW(parseURL(""), Error); + } + + /* ---------------------------------------------------------------------------- + * decodeQuery + * --------------------------------------------------------------------------*/ + + TEST(decodeQuery, emptyStringYieldsEmptyMap) { + auto d = decodeQuery(""); + ASSERT_EQ(d, (StringMap) { }); + } + + TEST(decodeQuery, simpleDecode) { + auto d = decodeQuery("yi=one&er=two"); + ASSERT_EQ(d, ((StringMap) { { "yi", "one" }, { "er", "two" } })); + } + + TEST(decodeQuery, decodeUrlEncodedArgs) { + auto d = decodeQuery("arg=%3D%3D%40%3D%3D"); + ASSERT_EQ(d, ((StringMap) { { "arg", "==@==" } })); + } + + TEST(decodeQuery, decodeArgWithEmptyValue) { + auto d = decodeQuery("arg="); + ASSERT_EQ(d, ((StringMap) { { "arg", ""} })); + } + + /* ---------------------------------------------------------------------------- + * percentDecode + * --------------------------------------------------------------------------*/ + + TEST(percentDecode, decodesUrlEncodedString) { + string s = "==@=="; + string d = percentDecode("%3D%3D%40%3D%3D"); + ASSERT_EQ(d, s); + } + + TEST(percentDecode, multipleDecodesAreIdempotent) { + string once = percentDecode("%3D%3D%40%3D%3D"); + string twice = percentDecode(once); + + ASSERT_EQ(once, twice); + } + + TEST(percentDecode, trailingPercent) { + string s = "==@==%"; + string d = percentDecode("%3D%3D%40%3D%3D%25"); + + ASSERT_EQ(d, s); + } + +} From c8cb558849da8ef88f15cc2a70c570f1f5013a30 Mon Sep 17 00:00:00 2001 From: Krzysztof Gogolewski Date: Thu, 21 May 2020 19:29:13 +0200 Subject: [PATCH 13/44] documentation: avoid unquoted URLs --- doc/manual/command-ref/conf-file.xml | 2 +- doc/manual/expressions/advanced-attributes.xml | 4 ++-- doc/manual/expressions/builtins.xml | 4 ++-- doc/manual/expressions/expression-syntax.xml | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/manual/command-ref/conf-file.xml b/doc/manual/command-ref/conf-file.xml index 1820598e5..1fa74a143 100644 --- a/doc/manual/command-ref/conf-file.xml +++ b/doc/manual/command-ref/conf-file.xml @@ -386,7 +386,7 @@ false. builtins.fetchurl { - url = https://example.org/foo-1.2.3.tar.xz; + url = "https://example.org/foo-1.2.3.tar.xz"; sha256 = "2c26b46b68ffc68ff99b453c1d30413413422d706483bfa0f98a5e886266e7ae"; } diff --git a/doc/manual/expressions/advanced-attributes.xml b/doc/manual/expressions/advanced-attributes.xml index 372d03de7..5759ff50e 100644 --- a/doc/manual/expressions/advanced-attributes.xml +++ b/doc/manual/expressions/advanced-attributes.xml @@ -178,7 +178,7 @@ impureEnvVars = [ "http_proxy" "https_proxy" ... ]; fetchurl { - url = http://ftp.gnu.org/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "http://ftp.gnu.org/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; } @@ -189,7 +189,7 @@ fetchurl { fetchurl { - url = ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; } diff --git a/doc/manual/expressions/builtins.xml b/doc/manual/expressions/builtins.xml index f71a8f3be..6709c4081 100644 --- a/doc/manual/expressions/builtins.xml +++ b/doc/manual/expressions/builtins.xml @@ -349,7 +349,7 @@ stdenv.mkDerivation { … } with import (fetchTarball { - url = https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz; + url = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz"; sha256 = "1jppksrfvbk5ypiqdz4cddxdl8z6zyzdb2srq8fcffr327ld5jj2"; }) {}; @@ -1406,7 +1406,7 @@ stdenv.mkDerivation { "; src = fetchurl { - url = http://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "http://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; }; inherit perl; diff --git a/doc/manual/expressions/expression-syntax.xml b/doc/manual/expressions/expression-syntax.xml index 42b9dca36..a3de20713 100644 --- a/doc/manual/expressions/expression-syntax.xml +++ b/doc/manual/expressions/expression-syntax.xml @@ -15,7 +15,7 @@ stdenv.mkDerivation { name = "hello-2.1.1"; builder = ./builder.sh; src = fetchurl { - url = ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz; + url = "ftp://ftp.nluug.nl/pub/gnu/hello/hello-2.1.1.tar.gz"; sha256 = "1md7jsfd8pa45z73bz1kszpp01yw6x5ljkjk2hx7wl800any6465"; }; inherit perl; From 0726ad5825f60e543d9cf535c62673685adbf5c8 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 15 Dec 2019 16:43:43 +0100 Subject: [PATCH 14/44] install: configure and bootstrap synthetic.conf on darwin Starting macOS 10.15 /nix can't be creasted directly anymore due to the readonly filesystem, but synthetic.conf was introduced to enable creating mountpoints or symlinks for special usecases like package managers. --- release.nix | 12 +++- scripts/create-darwin-volume.sh | 107 ++++++++++++++++++++++++++++ scripts/install-nix-from-closure.sh | 93 +++++++++++++++--------- 3 files changed, 176 insertions(+), 36 deletions(-) create mode 100755 scripts/create-darwin-volume.sh diff --git a/release.nix b/release.nix index f5729cee3..2cc4bb4c0 100644 --- a/release.nix +++ b/release.nix @@ -177,10 +177,10 @@ let } '' cp ${installerClosureInfo}/registration $TMPDIR/reginfo + cp ${./scripts/create-darwin-volume.sh} $TMPDIR/create-darwin-volume.sh substitute ${./scripts/install-nix-from-closure.sh} $TMPDIR/install \ --subst-var-by nix ${toplevel} \ --subst-var-by cacert ${cacert} - substitute ${./scripts/install-darwin-multi-user.sh} $TMPDIR/install-darwin-multi-user.sh \ --subst-var-by nix ${toplevel} \ --subst-var-by cacert ${cacert} @@ -195,6 +195,7 @@ let # SC1090: Don't worry about not being able to find # $nix/etc/profile.d/nix.sh shellcheck --exclude SC1090 $TMPDIR/install + shellcheck $TMPDIR/create-darwin-volume.sh shellcheck $TMPDIR/install-darwin-multi-user.sh shellcheck $TMPDIR/install-systemd-multi-user.sh @@ -210,6 +211,7 @@ let fi chmod +x $TMPDIR/install + chmod +x $TMPDIR/create-darwin-volume.sh chmod +x $TMPDIR/install-darwin-multi-user.sh chmod +x $TMPDIR/install-systemd-multi-user.sh chmod +x $TMPDIR/install-multi-user @@ -222,11 +224,15 @@ let --absolute-names \ --hard-dereference \ --transform "s,$TMPDIR/install,$dir/install," \ + --transform "s,$TMPDIR/create-darwin-volume.sh,$dir/create-darwin-volume.sh," \ --transform "s,$TMPDIR/reginfo,$dir/.reginfo," \ --transform "s,$NIX_STORE,$dir/store,S" \ - $TMPDIR/install $TMPDIR/install-darwin-multi-user.sh \ + $TMPDIR/install \ + $TMPDIR/create-darwin-volume.sh \ + $TMPDIR/install-darwin-multi-user.sh \ $TMPDIR/install-systemd-multi-user.sh \ - $TMPDIR/install-multi-user $TMPDIR/reginfo \ + $TMPDIR/install-multi-user \ + $TMPDIR/reginfo \ $(cat ${installerClosureInfo}/store-paths) ''); diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh new file mode 100755 index 000000000..f3e0d46f1 --- /dev/null +++ b/scripts/create-darwin-volume.sh @@ -0,0 +1,107 @@ +#!/bin/sh +set -e + +root_disks() { + diskutil list -plist / +} + +apfs_volumes_for() { + disk=$1 + diskutil apfs list -plist "$disk" +} + +disk_identifier() { + xpath "/plist/dict/key[text()='WholeDisks']/following-sibling::array[1]/string/text()" 2>/dev/null +} + +volume_get() { + key=$1 i=$2 + xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict[$i]/key[text()='$key']/following-sibling::string[1]/text()" 2> /dev/null +} + +find_nix_volume() { + disk=$1 + i=1 + volumes=$(apfs_volumes_for "$disk") + while true; do + name=$(echo "$volumes" | volume_get "Name" "$i") + if [ -z "$name" ]; then + break + fi + case "$name" in + [Nn]ix*) + echo "$name" + break + ;; + esac + i=$((i+1)) + done +} + +test_fstab() { + grep -q "/nix" /etc/fstab 2>/dev/null +} + +test_synthetic_conf() { + grep -q "^nix" /etc/synthetic.conf 2>/dev/null +} + +test_nix() { + test -d "/nix" +} + +main() { + ( + echo "" + echo " ------------------------------------------------------------------ " + echo " | This installer will create a volume for the nix store and |" + echo " | configure it to mount at /nix. Follow these steps to uninstall. |" + echo " ------------------------------------------------------------------ " + echo "" + echo " 1. Remove the entry from fstab using 'sudo vifs'" + echo " 2. Destroy the data volume using 'diskutil apfs deleteVolume'" + echo " 3. Delete /etc/synthetic.conf" + echo "" + ) >&2 + + if [ -L "/nix" ]; then + echo "error: /nix is a symlink, please remove it or edit synthetic.conf (requires reboot)" >&2 + echo " /nix -> $(readlink "/nix")" >&2 + exit 2 + fi + + if ! test_synthetic_conf; then + echo "Configuring /etc/synthetic.conf..." >&2 + echo nix | sudo tee /etc/synthetic.conf + /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + fi + + if ! test_nix; then + echo "Creating mountpoint for /nix..." >&2 + sudo mkdir /nix + fi + + disk=$(root_disks | disk_identifier) + volume=$(find_nix_volume "$disk") + if [ -z "$volume" ]; then + echo "Creating a Nix Store volume..." >&2 + sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix + volume="Nix Store" + else + echo "Using existing '$volume' volume" >&2 + fi + + if ! test_fstab; then + echo "Configuring /etc/fstab..." >&2 + label=$(echo "$volume" | sed 's/ /\\040/g') + printf "\$a\nLABEL=%s /nix apfs rw\n.\nwq\n" "$label" | EDITOR=ed sudo vifs + fi + + echo "The following options can be enabled to disable spotlight indexing" >&2 + echo "of the volume, which might be desirable." >&2 + echo "" >&2 + echo " $ mdutil -i off /nix" >&2 + echo "" >&2 +} + +main "$@" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index e06530ddf..34be7ee6a 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -40,44 +40,62 @@ elif [ "$(uname -s)" = "Linux" ] && [ -e /run/systemd/system ]; then fi INSTALL_MODE=no-daemon - +CREATE_DARWIN_VOLUME=0 # handle the command line flags -while [ "x${1:-}" != "x" ]; do - if [ "x${1:-}" = "x--no-daemon" ]; then - INSTALL_MODE=no-daemon - elif [ "x${1:-}" = "x--daemon" ]; then - INSTALL_MODE=daemon - elif [ "x${1:-}" = "x--no-channel-add" ]; then - NIX_INSTALLER_NO_CHANNEL_ADD=1 - elif [ "x${1:-}" = "x--no-modify-profile" ]; then - NIX_INSTALLER_NO_MODIFY_PROFILE=1 - elif [ "x${1:-}" != "x" ]; then - ( - echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" +while [ $# -gt 0 ]; do + case $1 in + --daemon) + INSTALL_MODE=daemon;; + --no-daemon) + INSTALL_MODE=no-daemon;; + --no-channel-add) + NIX_INSTALLER_NO_CHANNEL_ADD=1;; + --no-modify-profile) + NIX_INSTALLER_NO_MODIFY_PROFILE=1;; + --create-volume) + CREATE_DARWIN_VOLUME=1;; + *) + ( + echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" - echo "Choose installation method." - echo "" - echo " --daemon: Installs and configures a background daemon that manages the store," - echo " providing multi-user support and better isolation for local builds." - echo " Both for security and reproducibility, this method is recommended if" - echo " supported on your platform." - echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" - echo "" - echo " --no-daemon: Simple, single-user installation that does not require root and is" - echo " trivial to uninstall." - echo " (default)" - echo "" - echo " --no-channel-add: Don't add any channels. nixpkgs-unstable is installed by default." - echo "" - echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" - echo " is installed by default." - echo "" - ) >&2 - exit - fi + echo "Choose installation method." + echo "" + echo " --daemon: Installs and configures a background daemon that manages the store," + echo " providing multi-user support and better isolation for local builds." + echo " Both for security and reproducibility, this method is recommended if" + echo " supported on your platform." + echo " See https://nixos.org/nix/manual/#sect-multi-user-installation" + echo "" + echo " --no-daemon: Simple, single-user installation that does not require root and is" + echo " trivial to uninstall." + echo " (default)" + echo "" + echo " --no-channel-add: Don't add any channels. nixpkgs-unstable is installed by default." + echo "" + echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" + echo " is installed by default." + echo "" + ) >&2 + + if [ "$(uname -s)" = "Darwin" ]; then + ( + echo " --create-volume: Create an APFS volume for the store and create the /nix" + echo " mountpoint for it using synthetic.conf." + echo " (required on macOS >=10.15)" + echo " See https://nixos.org/nix/manual/#sect-darwin-apfs-volume" + echo "" + ) >&2 + fi + exit;; + esac shift done +if [ "$(uname -s)" = "Darwin" ] && [ "$CREATE_DARWIN_VOLUME" = 1 ]; then + printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' + "$self/create-darwin-volume.sh" +fi + if [ "$INSTALL_MODE" = "daemon" ]; then printf '\e[1;31mSwitching to the Daemon-based Installer\e[0m\n' exec "$self/install-multi-user" @@ -95,6 +113,15 @@ if ! [ -e $dest ]; then echo "directory $dest does not exist; creating it by running '$cmd' using sudo" >&2 if ! sudo sh -c "$cmd"; then echo "$0: please manually run '$cmd' as root to create $dest" >&2 + if [ "$(uname -s)" = "Darwin" ]; then + ( + echo "" + echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." + echo "Use --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "" + ) >&2 + fi exit 1 fi fi From 10202628b911980f05fc2c9460995af30f1bcbf3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sun, 15 Dec 2019 21:11:14 +0100 Subject: [PATCH 15/44] install: also configure ~/.zshenv The default login shell for users on macOS 10.15 changed from bash to zsh. So while generally nonstandard we need to configure it to make nix function out of the box on macOS. --- scripts/install-nix-from-closure.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 34be7ee6a..88275d1f0 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -197,6 +197,17 @@ if [ -z "$NIX_INSTALLER_NO_MODIFY_PROFILE" ]; then break fi done + for i in .zshenv .zshrc; do + fn="$HOME/$i" + if [ -w "$fn" ]; then + if ! grep -q "$p" "$fn"; then + echo "modifying $fn..." >&2 + echo "if [ -e $p ]; then . $p; fi # added by Nix installer" >> "$fn" + fi + added=1 + break + fi + done fi if [ -z "$added" ]; then From 083bb3bbfcdccebd06bde81a66f158d51ed6e455 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 13 Feb 2020 12:57:35 +0100 Subject: [PATCH 16/44] install: show macOS 10.15 message with --daemon --- scripts/create-darwin-volume.sh | 8 ++++---- scripts/install-nix-from-closure.sh | 28 ++++++++++++++++------------ 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index f3e0d46f1..d6fb44f43 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -1,8 +1,8 @@ #!/bin/sh set -e -root_disks() { - diskutil list -plist / +root_disk() { + diskutil info -plist / } apfs_volumes_for() { @@ -11,7 +11,7 @@ apfs_volumes_for() { } disk_identifier() { - xpath "/plist/dict/key[text()='WholeDisks']/following-sibling::array[1]/string/text()" 2>/dev/null + xpath "/plist/dict/key[text()='ParentWholeDisk']/following-sibling::string[1]/text()" 2>/dev/null } volume_get() { @@ -81,7 +81,7 @@ main() { sudo mkdir /nix fi - disk=$(root_disks | disk_identifier) + disk=$(root_disk | disk_identifier) volume=$(find_nix_volume "$disk") if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 88275d1f0..7d32bf92e 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -91,9 +91,22 @@ while [ $# -gt 0 ]; do shift done -if [ "$(uname -s)" = "Darwin" ] && [ "$CREATE_DARWIN_VOLUME" = 1 ]; then - printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' - "$self/create-darwin-volume.sh" +if [ "$(uname -s)" = "Darwin" ]; then + if [ "$CREATE_DARWIN_VOLUME" = 1 ]; then + printf '\e[1;31mCreating volume and mountpoint /nix.\e[0m\n' + "$self/create-darwin-volume.sh" + fi + + info=$(diskutil info -plist / | xpath "/plist/dict/key[text()='Writable']/following-sibling::true[1]" 2> /dev/null) + if ! [ -e $dest ] && [ -n "$info" ]; then + ( + echo "" + echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." + echo "Use --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "" + ) >&2 + fi fi if [ "$INSTALL_MODE" = "daemon" ]; then @@ -113,15 +126,6 @@ if ! [ -e $dest ]; then echo "directory $dest does not exist; creating it by running '$cmd' using sudo" >&2 if ! sudo sh -c "$cmd"; then echo "$0: please manually run '$cmd' as root to create $dest" >&2 - if [ "$(uname -s)" = "Darwin" ]; then - ( - echo "" - echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." - echo "" - ) >&2 - fi exit 1 fi fi From ee89b7797d4ec1db6dad9df5fb3bb8cc2f05de12 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Fri, 17 Jan 2020 23:27:29 +0100 Subject: [PATCH 17/44] manual: add apfs volume section --- doc/manual/installation/installing-binary.xml | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 3f57f47b5..86cbce6bf 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -136,6 +136,109 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist +
+ APFS Volume Installation + + + The root filesystem is read-only as of macOS 10.15 Catalina, all writable + paths to a separate data volume. This means creating or writing to /nix + is not allowed. While changing the default prefix would be possible, it's + a very intrusive change that has side effects we want to avoid for now. + + + + For common writable locations firmlinks where introduced, + described by Apple as a "bi-directional wormhole" between two filesystems. + Essentially a bind mount for APFS volumes. However this is (currently) not + user configurable and only available for paths like /Users. + + + + For special cases like NFS mount points or package manager roots synthetic.conf(5) + provides a mechanism for some limited, user-controlled file-creation at /. + This only applies on a reboot, but apfs.util can be used + to trigger the creation (not deletion) of new entries. + + + +alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + + + + + + The simplest solution is creating a symlink with /etc/synthetic.conf + to the data volume. (not recommended) + + + +nix /System/Volumes/Data/nix + + + +alice$ ls -l / +lrwxr-xr-x 1 root wheel 25 Jan 1 2019 nix -> /System/Volumes/Data/nix + + + + However builds that detect or resolve this symlink will leak the canonical + location or even fail in certain cases, making this approach undesirable. + + + + + + An empty directory can also be created using /etc/synthetic.conf, + this won't be writable but can be used as a mount point. And with + APFS it's relatively easy to create an separate + volume for nix instead. + + + +nix + + + +alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix +alice$ mount +/dev/disk1s6 on /nix (apfs, local, journaled) + + + + This does make the installation more complicated, requiring both + /etc/synthetic.conf as well as /etc/fstab + + + +# +# Warning - this file should only be modified with vifs(8) +# +# Failure to do so is unsupported and may be destructive. +# +LABEL=Nix\040Store /nix apfs rw + + + + On macOS volumes are also mounted quite late, launchd services or other + things that start during login will start before our volume is mounted. + For these cases eg. wait4path must be used for + things that depend on /nix. + + + + This new volume also won't be encrypted by default, and enabling is + only possible interactively? + + + +diskutil apfs enableFileVault /nix -user disk + + + + + +
+
Installing a pinned Nix version from a URL From caface1980344347f2a50701ec133e98e6094c8c Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 5 Mar 2020 23:12:52 +0100 Subject: [PATCH 18/44] install: hide the store volume on darwin --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index d6fb44f43..a1a67acea 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -94,7 +94,7 @@ main() { if ! test_fstab; then echo "Configuring /etc/fstab..." >&2 label=$(echo "$volume" | sed 's/ /\\040/g') - printf "\$a\nLABEL=%s /nix apfs rw\n.\nwq\n" "$label" | EDITOR=ed sudo vifs + printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi echo "The following options can be enabled to disable spotlight indexing" >&2 From 04f597c3f4d0ac8b8a677c798642329a5a6768e3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 7 Mar 2020 12:01:40 +0100 Subject: [PATCH 19/44] install: improve output and error handling --- doc/manual/installation/installing-binary.xml | 3 ++- scripts/create-darwin-volume.sh | 17 ++++++++++++++--- scripts/install-nix-from-closure.sh | 7 ++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 86cbce6bf..7f4bba3a0 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -199,6 +199,7 @@ nix +alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix alice$ mount /dev/disk1s6 on /nix (apfs, local, journaled) @@ -231,7 +232,7 @@ LABEL=Nix\040Store /nix apfs rw -diskutil apfs enableFileVault /nix -user disk +alice$ diskutil apfs enableFileVault /nix -user disk diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index a1a67acea..e2b49c9a0 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -73,12 +73,22 @@ main() { if ! test_synthetic_conf; then echo "Configuring /etc/synthetic.conf..." >&2 echo nix | sudo tee /etc/synthetic.conf - /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + if ! test_synthetic_conf; then + echo "error: failed to configure synthetic.conf" >&2 + exit 1 + fi fi if ! test_nix; then echo "Creating mountpoint for /nix..." >&2 - sudo mkdir /nix + /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B || true + if ! test_nix; then + sudo mkdir -p /nix 2>/dev/null || true + fi + if ! test_nix; then + echo "error: failed to bootstrap /nix, a reboot might be required" >&2 + exit 1 + fi fi disk=$(root_disk | disk_identifier) @@ -97,10 +107,11 @@ main() { printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi + echo "" >&2 echo "The following options can be enabled to disable spotlight indexing" >&2 echo "of the volume, which might be desirable." >&2 echo "" >&2 - echo " $ mdutil -i off /nix" >&2 + echo " $ sudo mdutil -i off /nix" >&2 echo "" >&2 } diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 7d32bf92e..2f291ed4c 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -82,7 +82,7 @@ while [ $# -gt 0 ]; do echo " --create-volume: Create an APFS volume for the store and create the /nix" echo " mountpoint for it using synthetic.conf." echo " (required on macOS >=10.15)" - echo " See https://nixos.org/nix/manual/#sect-darwin-apfs-volume" + echo " See https://nixos.org/nix/manual/#sect-apfs-volume-installation" echo "" ) >&2 fi @@ -102,10 +102,11 @@ if [ "$(uname -s)" = "Darwin" ]; then ( echo "" echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-darwin-apfs-volume." + echo "Use sh <(curl https://nixos.org/nix/install) --create-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" echo "" ) >&2 + exit 1 fi fi From bc24c09968bb35fce599151f86d123cb5984f727 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Wed, 18 Mar 2020 20:34:46 +0100 Subject: [PATCH 20/44] install: make synthetic.conf and fstab checks stricter --- scripts/create-darwin-volume.sh | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index e2b49c9a0..ea4133444 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -39,11 +39,15 @@ find_nix_volume() { } test_fstab() { - grep -q "/nix" /etc/fstab 2>/dev/null + grep -q "/nix apfs rw" /etc/fstab 2>/dev/null +} + +test_nix_symlink() { + [ -L "/nix" ] || grep -q "^nix." /etc/synthetic.conf 2>/dev/null } test_synthetic_conf() { - grep -q "^nix" /etc/synthetic.conf 2>/dev/null + grep -q "^nix$" /etc/synthetic.conf 2>/dev/null } test_nix() { @@ -60,12 +64,12 @@ main() { echo "" echo " 1. Remove the entry from fstab using 'sudo vifs'" echo " 2. Destroy the data volume using 'diskutil apfs deleteVolume'" - echo " 3. Delete /etc/synthetic.conf" + echo " 3. Remove the 'nix' line from /etc/synthetic.conf or the file" echo "" ) >&2 - if [ -L "/nix" ]; then - echo "error: /nix is a symlink, please remove it or edit synthetic.conf (requires reboot)" >&2 + if test_nix_symlink; then + echo "error: /nix is a symlink, please remove it and make sure it's not in synthetic.conf (in which case a reboot is required)" >&2 echo " /nix -> $(readlink "/nix")" >&2 exit 2 fi From 33865752960c9a2ff28eb9024f20d2103918685c Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 26 Mar 2020 20:14:08 +0100 Subject: [PATCH 21/44] manual: clarify volume creation section --- doc/manual/installation/installing-binary.xml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 7f4bba3a0..c11ba9cce 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -141,13 +141,14 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist The root filesystem is read-only as of macOS 10.15 Catalina, all writable - paths to a separate data volume. This means creating or writing to /nix - is not allowed. While changing the default prefix would be possible, it's - a very intrusive change that has side effects we want to avoid for now. + paths were moved to a separate data volume. This means creating or writing + to /nix is not allowed. While changing the default prefix + would be possible, it's a very intrusive change that has side effects we want to + avoid for now. - For common writable locations firmlinks where introduced, + For common writable locations firmlinks were introduced, described by Apple as a "bi-directional wormhole" between two filesystems. Essentially a bind mount for APFS volumes. However this is (currently) not user configurable and only available for paths like /Users. @@ -156,8 +157,10 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist For special cases like NFS mount points or package manager roots synthetic.conf(5) provides a mechanism for some limited, user-controlled file-creation at /. - This only applies on a reboot, but apfs.util can be used - to trigger the creation (not deletion) of new entries. + This only applies at boot time, however apfs.util can be used + to trigger the creation (not deletion) of new entries without a reboot. + It would be ideal if this could create firmlinks, however a symlink or mountpoint + are the only options. From 477d7c2d07e146c91950401b8b9d9380ce6787e5 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 26 Mar 2020 21:51:13 +0100 Subject: [PATCH 22/44] installer: refuse apfs volume creation when FileVault is enabled --- doc/manual/installation/installing-binary.xml | 6 +++-- scripts/create-darwin-volume.sh | 22 +++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index c11ba9cce..498248662 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -230,8 +230,10 @@ LABEL=Nix\040Store /nix apfs rw - This new volume also won't be encrypted by default, and enabling is - only possible interactively? + This new volume also won't be encrypted by default, and enabling it + requires extra setup. For machines with a T2 chip + all data is already entrypted at rest, older hardware won't even when + FileVault is enabled for the rest of the system. diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index ea4133444..a0da85f43 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -14,7 +14,12 @@ disk_identifier() { xpath "/plist/dict/key[text()='ParentWholeDisk']/following-sibling::string[1]/text()" 2>/dev/null } -volume_get() { +volume_list_true() { + key=$1 t=$2 + xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict/key[text()='$key']/following-sibling::true[1]" 2> /dev/null +} + +volume_get_string() { key=$1 i=$2 xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict[$i]/key[text()='$key']/following-sibling::string[1]/text()" 2> /dev/null } @@ -24,7 +29,7 @@ find_nix_volume() { i=1 volumes=$(apfs_volumes_for "$disk") while true; do - name=$(echo "$volumes" | volume_get "Name" "$i") + name=$(echo "$volumes" | volume_get_string "Name" "$i") if [ -z "$name" ]; then break fi @@ -54,6 +59,12 @@ test_nix() { test -d "/nix" } +test_filevault() { + disk=$1 + apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true || return + ! sudo xartutil --list >/dev/null 2>/dev/null +} + main() { ( echo "" @@ -99,6 +110,13 @@ main() { volume=$(find_nix_volume "$disk") if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 + + if test_filevault "$disk"; then + echo "error: FileVault detected, refusing to create unencrypted volume" >&2 + echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" >&2 + exit 1 + fi + sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix volume="Nix Store" else From 2b0a81d92d28994374465c44c79f020d5e044700 Mon Sep 17 00:00:00 2001 From: "Travis A. Everett" Date: Thu, 14 May 2020 21:59:10 -0500 Subject: [PATCH 23/44] focus on golden-path covering most scenarios This should handle installation scenarios we can handle with anything resembling confidence. Goal is approximating the existing setup--not enforcing a best-practice... Approaches (+ installer-handled, - manual) and configs each covers: + no change needed; /nix OK on boot volume: All pre-Catalina (regardless of T2 or FileVault use) + create new unencrypted volume: Catalina, pre-T2, no FileVault + create new encrypted-at-rest volume: Catalina, pre-T2, FileVault Catalina, T2, no FileVault - require user to pre-create encrypted volume Catalina, T2, FileVault --- doc/manual/installation/installing-binary.xml | 350 +++++++++++++----- scripts/create-darwin-volume.sh | 77 +++- scripts/install-nix-from-closure.sh | 19 +- 3 files changed, 331 insertions(+), 115 deletions(-) diff --git a/doc/manual/installation/installing-binary.xml b/doc/manual/installation/installing-binary.xml index 498248662..8d548f0ea 100644 --- a/doc/manual/installation/installing-binary.xml +++ b/doc/manual/installation/installing-binary.xml @@ -6,16 +6,30 @@ Installing a Binary Distribution -If you are using Linux or macOS, the easiest way to install Nix -is to run the following command: + + If you are using Linux or macOS versions up to 10.14 (Mojave), the + easiest way to install Nix is to run the following command: + $ sh <(curl https://nixos.org/nix/install) -As of Nix 2.1.0, the Nix installer will always default to creating a -single-user installation, however opting in to the multi-user -installation is highly recommended. + + If you're using macOS 10.15 (Catalina) or newer, consult + the macOS installation instructions + before installing. + + + + As of Nix 2.1.0, the Nix installer will always default to creating a + single-user installation, however opting in to the multi-user + installation is highly recommended. +
@@ -36,7 +50,7 @@ run this under your usual user account, not as root. The script will invoke sudo to create /nix if it doesn’t already exist. If you don’t have sudo, you should manually create -/nix first as root, e.g.: +/nix first as root, e.g.: $ mkdir /nix @@ -47,7 +61,7 @@ The install script will modify the first writable file from amongst .bash_profile, .bash_login and .profile to source ~/.nix-profile/etc/profile.d/nix.sh. You can set -the NIX_INSTALLER_NO_MODIFY_PROFILE environment +the NIX_INSTALLER_NO_MODIFY_PROFILE environment variable before executing the install script to disable this behaviour. @@ -81,12 +95,10 @@ $ rm -rf /nix You can instruct the installer to perform a multi-user installation on your system: - - - sh <(curl https://nixos.org/nix/install) --daemon - + sh <(curl https://nixos.org/nix/install) --daemon + The multi-user installation of Nix will create build users between the user IDs 30001 and 30032, and a group with the group ID 30000. @@ -136,82 +148,253 @@ sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist
-
- APFS Volume Installation +
+ macOS Installation - The root filesystem is read-only as of macOS 10.15 Catalina, all writable - paths were moved to a separate data volume. This means creating or writing - to /nix is not allowed. While changing the default prefix - would be possible, it's a very intrusive change that has side effects we want to - avoid for now. + Starting with macOS 10.15 (Catalina), the root filesystem is read-only. + This means /nix can no longer live on your system + volume, and that you'll need a workaround to install Nix. - For common writable locations firmlinks were introduced, - described by Apple as a "bi-directional wormhole" between two filesystems. - Essentially a bind mount for APFS volumes. However this is (currently) not - user configurable and only available for paths like /Users. + The recommended approach, which creates an unencrypted APFS volume + for your Nix store and a "synthetic" empty directory to mount it + over at /nix, is least likely to impair Nix + or your system. + + With all separate-volume approaches, it's possible something on + your system (particularly daemons/services and restored apps) may + need access to your Nix store before the volume is mounted. Adding + additional encryption makes this more likely. + + - For special cases like NFS mount points or package manager roots synthetic.conf(5) - provides a mechanism for some limited, user-controlled file-creation at /. - This only applies at boot time, however apfs.util can be used - to trigger the creation (not deletion) of new entries without a reboot. - It would be ideal if this could create firmlinks, however a symlink or mountpoint - are the only options. + If you're using a recent Mac with a + T2 chip, + your drive will still be encrypted at rest (in which case "unencrypted" + is a bit of a misnomer). To use this approach, just install Nix with: - -alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B - + $ sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume - - - - The simplest solution is creating a symlink with /etc/synthetic.conf - to the data volume. (not recommended) - + + If you don't like the sound of this, you'll want to weigh the + other approaches and tradeoffs detailed in this section. + - -nix /System/Volumes/Data/nix - + + Eventual solutions? + + All of the known workarounds have drawbacks, but we hope + better solutions will be available in the future. Some that + we have our eye on are: + + + + + A true firmlink would enable the Nix store to live on the + primary data volume without the build problems caused by + the symlink approach. End users cannot currently + create true firmlinks. + + + + + If the Nix store volume shared FileVault encryption + with the primary data volume (probably by using the same + volume group and role), FileVault encryption could be + easily supported by the installer without requiring + manual setup by each user. + + + + - -alice$ ls -l / -lrwxr-xr-x 1 root wheel 25 Jan 1 2019 nix -> /System/Volumes/Data/nix - +
+ Change the Nix store path prefix + + Changing the default prefix for the Nix store is a simple + approach which enables you to leave it on your root volume, + where it can take full advantage of FileVault encryption if + enabled. Unfortunately, this approach also opts your device out + of some benefits that are enabled by using the same prefix + across systems: - - However builds that detect or resolve this symlink will leak the canonical - location or even fail in certain cases, making this approach undesirable. - - + + + + Your system won't be able to take advantage of the binary + cache (unless someone is able to stand up and support + duplicate caching infrastructure), which means you'll + spend more time waiting for builds. + + + + + It's harder to build and deploy packages to Linux systems. + + + + - - - An empty directory can also be created using /etc/synthetic.conf, - this won't be writable but can be used as a mount point. And with - APFS it's relatively easy to create an separate - volume for nix instead. - + - -nix - + It would also possible (and often requested) to just apply this + change ecosystem-wide, but it's an intrusive process that has + side effects we want to avoid for now. + + + + +
- -alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B -alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix -alice$ mount -/dev/disk1s6 on /nix (apfs, local, journaled) - +
+ Use a separate encrypted volume + + If you like, you can also add encryption to the recommended + approach taken by the installer. You can do this by pre-creating + an encrypted volume before you run the installer--or you can + run the installer and encrypt the volume it creates later. + + + + In either case, adding encryption to a second volume isn't quite + as simple as enabling FileVault for your boot volume. Before you + dive in, there are a few things to weigh: + + + + + The additional volume won't be encrypted with your existing + FileVault key, so you'll need another mechanism to decrypt + the volume. + + + + + You can store the password in Keychain to automatically + decrypt the volume on boot--but it'll have to wait on Keychain + and may not mount before your GUI apps restore. If any of + your launchd agents or apps depend on Nix-installed software + (for example, if you use a Nix-installed login shell), the + restore may fail or break. + + + On a case-by-case basis, you may be able to work around this + problem by using wait4path to block + execution until your executable is available. + + + It's also possible to decrypt and mount the volume earlier + with a login hook--but this mechanism appears to be + deprecated and its future is unclear. + + + + + You can hard-code the password in the clear, so that your + store volume can be decrypted before Keychain is available. + + + + + If you are comfortable navigating these tradeoffs, you can encrypt the volume with + something along the lines of: + + + + alice$ diskutil apfs enableFileVault /nix -user disk + + +
+ +
+ + Symlink the Nix store to a custom location + + Another simple approach is using /etc/synthetic.conf + to symlink the Nix store to the data volume. This option also + enables your store to share any configured FileVault encryption. + Unfortunately, builds that resolve the symlink may leak the + canonical path or even fail. + + + Because of these downsides, we can't recommend this approach. + + +
+ +
+ Notes on the recommended approach + + This section goes into a little more detail on the recommended + approach. You don't need to understand it to run the installer, + but it can serve as a helpful reference if you run into trouble. + + + + + In order to compose user-writable locations into the new + read-only system root, Apple introduced a new concept called + firmlinks, which it describes as a + "bi-directional wormhole" between two filesystems. You can + see the current firmlinks in /usr/share/firmlinks. + Unfortunately, firmlinks aren't (currently?) user-configurable. + + + + For special cases like NFS mount points or package manager roots, + synthetic.conf(5) + supports limited user-controlled file-creation (of symlinks, + and synthetic empty directories) at /. + To create a synthetic empty directory for mounting at /nix, + add the following line to /etc/synthetic.conf + (create it if necessary): + + + nix + + + + + This configuration is applied at boot time, but you can use + apfs.util to trigger creation (not deletion) + of new entries without a reboot: + + + alice$ /System/Library/Filesystems/apfs.fs/Contents/Resources/apfs.util -B + + + + + Create the new APFS volume with diskutil: + + + alice$ sudo diskutil apfs addVolume diskX APFS 'Nix Store' -mountpoint /nix + + + + + Using vifs, add the new mount to + /etc/fstab. If it doesn't already have + other entries, it should look something like: + # @@ -219,29 +402,16 @@ alice$ mount # # Failure to do so is unsupported and may be destructive. # -LABEL=Nix\040Store /nix apfs rw +LABEL=Nix\040Store /nix apfs rw,nobrowse - - On macOS volumes are also mounted quite late, launchd services or other - things that start during login will start before our volume is mounted. - For these cases eg. wait4path must be used for - things that depend on /nix. - - - - This new volume also won't be encrypted by default, and enabling it - requires extra setup. For machines with a T2 chip - all data is already entrypted at rest, older hardware won't even when - FileVault is enabled for the rest of the system. - - - -alice$ diskutil apfs enableFileVault /nix -user disk - - - - + + The nobrowse setting will keep Spotlight from indexing this + volume, and keep it from showing up on your desktop. + + + +
diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index a0da85f43..47cc3e913 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -59,10 +59,45 @@ test_nix() { test -d "/nix" } -test_filevault() { +test_t2_chip_present(){ + # Use xartutil to see if system has a t2 chip. + # + # This isn't well-documented on its own; until it is, + # let's keep track of knowledge/assumptions. + # + # Warnings: + # - Don't search "xart" if porn will cause you trouble :) + # - Other xartutil flags do dangerous things. Don't run them + # naively. If you must, search "xartutil" first. + # + # Assumptions: + # - the "xART session seeds recovery utility" + # appears to interact with xartstorageremoted + # - `sudo xartutil --list` lists xART sessions + # and their seeds and exits 0 if successful. If + # not, it exits 1 and prints an error such as: + # xartutil: ERROR: No supported link to the SEP present + # - xART sessions/seeds are present when a T2 chip is + # (and not, otherwise) + # - the presence of a T2 chip means a newly-created + # volume on the primary drive will be + # encrypted at rest + # - all together: `sudo xartutil --list` + # should exit 0 if a new Nix Store volume will + # be encrypted at rest, and exit 1 if not. + sudo xartutil --list >/dev/null 2>/dev/null +} + +test_filevault_in_use() { disk=$1 - apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true || return - ! sudo xartutil --list >/dev/null 2>/dev/null + # list vols on disk | get value of Filevault key | value is true + apfs_volumes_for "$disk" | volume_list_true FileVault | grep -q true +} + +# use after error msg for conditions we don't understand +suggest_report_error(){ + # ex "error: something sad happened :(" >&2 + echo " please report this @ https://github.com/nixos/nix/issues" >&2 } main() { @@ -89,7 +124,8 @@ main() { echo "Configuring /etc/synthetic.conf..." >&2 echo nix | sudo tee /etc/synthetic.conf if ! test_synthetic_conf; then - echo "error: failed to configure synthetic.conf" >&2 + echo "error: failed to configure synthetic.conf;" >&2 + suggest_report_error exit 1 fi fi @@ -101,7 +137,8 @@ main() { sudo mkdir -p /nix 2>/dev/null || true fi if ! test_nix; then - echo "error: failed to bootstrap /nix, a reboot might be required" >&2 + echo "error: failed to bootstrap /nix; if a reboot doesn't help," >&2 + suggest_report_error exit 1 fi fi @@ -111,10 +148,25 @@ main() { if [ -z "$volume" ]; then echo "Creating a Nix Store volume..." >&2 - if test_filevault "$disk"; then - echo "error: FileVault detected, refusing to create unencrypted volume" >&2 - echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" >&2 - exit 1 + if test_filevault_in_use "$disk"; then + # TODO: Not sure if it's in-scope now, but `diskutil apfs list` + # shows both filevault and encrypted at rest status, and it + # may be the more semantic way to test for this? It'll show + # `FileVault: No (Encrypted at rest)` + # `FileVault: No` + # `FileVault: Yes (Unlocked)` + # and so on. + if test_t2_chip_present; then + echo "warning: boot volume is FileVault-encrypted, but the Nix store volume" >&2 + echo " is only encrypted at rest." >&2 + echo " See https://nixos.org/nix/manual/#sect-macos-installation" >&2 + else + echo "error: refusing to create Nix store volume because the boot volume is" >&2 + echo " FileVault encrypted, but encryption-at-rest is not available." >&2 + echo " Manually create a volume for the store and re-run this script." >&2 + echo " See https://nixos.org/nix/manual/#sect-macos-installation" >&2 + exit 1 + fi fi sudo diskutil apfs addVolume "$disk" APFS 'Nix Store' -mountpoint /nix @@ -128,13 +180,6 @@ main() { label=$(echo "$volume" | sed 's/ /\\040/g') printf "\$a\nLABEL=%s /nix apfs rw,nobrowse\n.\nwq\n" "$label" | EDITOR=ed sudo vifs fi - - echo "" >&2 - echo "The following options can be enabled to disable spotlight indexing" >&2 - echo "of the volume, which might be desirable." >&2 - echo "" >&2 - echo " $ sudo mdutil -i off /nix" >&2 - echo "" >&2 } main "$@" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 2f291ed4c..72aa5abf5 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -52,7 +52,7 @@ while [ $# -gt 0 ]; do NIX_INSTALLER_NO_CHANNEL_ADD=1;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; - --create-volume) + --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; *) ( @@ -77,12 +77,13 @@ while [ $# -gt 0 ]; do echo "" ) >&2 - if [ "$(uname -s)" = "Darwin" ]; then + # darwin and Catalina+ + if [ "$(uname -s)" = "Darwin" ] && [ "$macos_major" -gt 14 ]; then ( - echo " --create-volume: Create an APFS volume for the store and create the /nix" - echo " mountpoint for it using synthetic.conf." - echo " (required on macOS >=10.15)" - echo " See https://nixos.org/nix/manual/#sect-apfs-volume-installation" + echo " --darwin-use-unencrypted-nix-store-volume: Create an APFS volume for the Nix" + echo " store and mount it at /nix. This is the recommended way to create" + echo " /nix with a read-only / on macOS >=10.15." + echo " See: https://nixos.org/nix/manual/#sect-macos-installation" echo "" ) >&2 fi @@ -98,12 +99,12 @@ if [ "$(uname -s)" = "Darwin" ]; then fi info=$(diskutil info -plist / | xpath "/plist/dict/key[text()='Writable']/following-sibling::true[1]" 2> /dev/null) - if ! [ -e $dest ] && [ -n "$info" ]; then + if ! [ -e $dest ] && [ -n "$info" ] && [ "$macos_major" -gt 14 ]; then ( echo "" echo "Installing on macOS >=10.15 requires relocating the store to an apfs volume." - echo "Use sh <(curl https://nixos.org/nix/install) --create-volume or run the preparation steps manually." - echo "See https://nixos.org/nix/manual/#sect-apfs-volume-installation" + echo "Use sh <(curl https://nixos.org/nix/install) --darwin-use-unencrypted-nix-store-volume or run the preparation steps manually." + echo "See https://nixos.org/nix/manual/#sect-macos-installation" echo "" ) >&2 exit 1 From d3df1889a1f49b114a8d78270ea8a6d0523f5e35 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Thu, 21 May 2020 20:03:09 +0200 Subject: [PATCH 24/44] installer: don't clobber synthetic.conf --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index 47cc3e913..5214d1652 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -122,7 +122,7 @@ main() { if ! test_synthetic_conf; then echo "Configuring /etc/synthetic.conf..." >&2 - echo nix | sudo tee /etc/synthetic.conf + echo nix | sudo tee -a /etc/synthetic.conf if ! test_synthetic_conf; then echo "error: failed to configure synthetic.conf;" >&2 suggest_report_error From 909d8cb2934869c945ac1cc20dfb71df513042eb Mon Sep 17 00:00:00 2001 From: Suraj Barkale Date: Fri, 22 May 2020 11:05:25 +1000 Subject: [PATCH 25/44] Use /etc/zshenv instead of /etc/zshrc for profile As noted in https://github.com/NixOS/nix/issues/3456 the `/etc/zshenv` file provides a better place for sourcing the nix environment. --- scripts/install-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..838192733 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -28,7 +28,7 @@ readonly NIX_FIRST_BUILD_UID="30001" # default shell profile that comes with Nix doesn't support it. readonly NIX_ROOT="/nix" -readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshrc") +readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshenv") readonly PROFILE_BACKUP_SUFFIX=".backup-before-nix" readonly PROFILE_NIX_FILE="$NIX_ROOT/var/nix/profiles/default/etc/profile.d/nix-daemon.sh" From 2a7ea2eb6c54c82d5e858ea6ae9de929face5e55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Sat, 23 May 2020 11:12:05 +0200 Subject: [PATCH 26/44] scripts/create-darwin-volume.sh: remove unused variable --- scripts/create-darwin-volume.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/create-darwin-volume.sh b/scripts/create-darwin-volume.sh index 5214d1652..dac30d72d 100755 --- a/scripts/create-darwin-volume.sh +++ b/scripts/create-darwin-volume.sh @@ -15,7 +15,7 @@ disk_identifier() { } volume_list_true() { - key=$1 t=$2 + key=$1 xpath "/plist/dict/array/dict/key[text()='Volumes']/following-sibling::array/dict/key[text()='$key']/following-sibling::true[1]" 2> /dev/null } From 6f6bdd63a0f6dae4ab91422645f548837029dee9 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Sat, 23 May 2020 12:10:12 +0200 Subject: [PATCH 27/44] fix hydra build products Since the binary tarball was replaced none of the hydra builds include the manual. The dist phase isn't enabled by default the manual build products where not written. --- release.nix | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/release.nix b/release.nix index 2cc4bb4c0..2a320e1c3 100644 --- a/release.nix +++ b/release.nix @@ -115,17 +115,17 @@ let installFlags = "sysconfdir=$(out)/etc"; + postInstall = '' + mkdir -p $doc/nix-support + echo "doc manual $doc/share/doc/nix/manual" >> $doc/nix-support/hydra-build-products + ''; + doCheck = true; doInstallCheck = true; installCheckFlags = "sysconfdir=$(out)/etc"; separateDebugInfo = true; - - preDist = '' - mkdir -p $doc/nix-support - echo "doc manual $doc/share/doc/nix/manual" >> $doc/nix-support/hydra-build-products - ''; }); From e2af11ce071fc109ad3e517bf15a87024d0a9ae5 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Sat, 23 May 2020 15:26:59 +0200 Subject: [PATCH 28/44] Manpages: Do not refer to nixpkgs-channels Unless I am misinformed, using the `nixpkgs` repository directly is now preferred? --- doc/manual/command-ref/env-common.xml | 2 +- doc/manual/command-ref/nix-env.xml | 7 ++----- doc/manual/command-ref/nix-shell.xml | 6 +++--- doc/manual/expressions/builtins.xml | 4 ++-- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/doc/manual/command-ref/env-common.xml b/doc/manual/command-ref/env-common.xml index 0217de7b2..8466cc463 100644 --- a/doc/manual/command-ref/env-common.xml +++ b/doc/manual/command-ref/env-common.xml @@ -53,7 +53,7 @@ nixpkgs=/home/eelco/Dev/nixpkgs-branch:/etc/nixos NIX_PATH to -nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-15.09.tar.gz +nixpkgs=https://github.com/NixOS/nixpkgs/archive/nixos-15.09.tar.gz tells Nix to download the latest revision in the Nixpkgs/NixOS 15.09 channel. diff --git a/doc/manual/command-ref/nix-env.xml b/doc/manual/command-ref/nix-env.xml index 9c03ccce1..2b95b6819 100644 --- a/doc/manual/command-ref/nix-env.xml +++ b/doc/manual/command-ref/nix-env.xml @@ -526,13 +526,10 @@ these paths will be fetched (0.04 MiB download, 0.19 MiB unpacked): 14.12 channel: -$ nix-env -f https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz -iA firefox +$ nix-env -f https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz -iA firefox -(The GitHub repository nixpkgs-channels is updated -automatically from the main nixpkgs repository -after certain tests have succeeded and binaries have been built and -uploaded to the binary cache at cache.nixos.org.) + diff --git a/doc/manual/command-ref/nix-shell.xml b/doc/manual/command-ref/nix-shell.xml index 766482460..2fef323c5 100644 --- a/doc/manual/command-ref/nix-shell.xml +++ b/doc/manual/command-ref/nix-shell.xml @@ -258,7 +258,7 @@ path. You can override it by passing or setting containing the Pan package from a specific revision of Nixpkgs: -$ nix-shell -p pan -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/8a3eea054838b55aca962c3fbde9c83c102b8bf2.tar.gz +$ nix-shell -p pan -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/8a3eea054838b55aca962c3fbde9c83c102b8bf2.tar.gz [nix-shell:~]$ pan --version Pan 0.139 @@ -352,7 +352,7 @@ following Haskell script uses a specific branch of Nixpkgs/NixOS (the -#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/0672315759b3e15e2121365f067c1c8c56bb4722.tar.gz +#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/0672315759b3e15e2121365f067c1c8c56bb4722.tar.gz diff --git a/doc/manual/expressions/builtins.xml b/doc/manual/expressions/builtins.xml index 6709c4081..a18c5801a 100644 --- a/doc/manual/expressions/builtins.xml +++ b/doc/manual/expressions/builtins.xml @@ -324,7 +324,7 @@ if builtins ? getEnv then builtins.getEnv "PATH" else "" particular version of Nixpkgs, e.g. -with import (fetchTarball https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz) {}; +with import (fetchTarball https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz) {}; stdenv.mkDerivation { … } @@ -349,7 +349,7 @@ stdenv.mkDerivation { … } with import (fetchTarball { - url = "https://github.com/NixOS/nixpkgs-channels/archive/nixos-14.12.tar.gz"; + url = "https://github.com/NixOS/nixpkgs/archive/nixos-14.12.tar.gz"; sha256 = "1jppksrfvbk5ypiqdz4cddxdl8z6zyzdb2srq8fcffr327ld5jj2"; }) {}; From ecc5c90dfc0e9877aef89bf0192203cd1a8f2b21 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 11:50:41 +0200 Subject: [PATCH 29/44] Add unit tests for hashing functions --- src/libutil/tests/hash.cc | 80 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 src/libutil/tests/hash.cc diff --git a/src/libutil/tests/hash.cc b/src/libutil/tests/hash.cc new file mode 100644 index 000000000..7cb439817 --- /dev/null +++ b/src/libutil/tests/hash.cc @@ -0,0 +1,80 @@ +#include "hash.hh" +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * hashString + * --------------------------------------------------------------------------*/ + + TEST(hashString, testKnownMD5Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc1321 + auto s1 = ""; + auto hash = hashString(HashType::htMD5, s1); + ASSERT_EQ(hash.to_string(Base::Base16), "md5:d41d8cd98f00b204e9800998ecf8427e"); + } + + TEST(hashString, testKnownMD5Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc1321 + auto s2 = "abc"; + auto hash = hashString(HashType::htMD5, s2); + ASSERT_EQ(hash.to_string(Base::Base16), "md5:900150983cd24fb0d6963f7d28e17f72"); + } + + TEST(hashString, testKnownSHA1Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc3174 + auto s = "abc"; + auto hash = hashString(HashType::htSHA1, s); + ASSERT_EQ(hash.to_string(Base::Base16),"sha1:a9993e364706816aba3e25717850c26c9cd0d89d"); + } + + TEST(hashString, testKnownSHA1Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc3174 + auto s = "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"; + auto hash = hashString(HashType::htSHA1, s); + ASSERT_EQ(hash.to_string(Base::Base16),"sha1:84983e441c3bd26ebaae4aa1f95129e5e54670f1"); + } + + TEST(hashString, testKnownSHA256Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abc"; + + auto hash = hashString(HashType::htSHA256, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha256:ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad"); + } + + TEST(hashString, testKnownSHA256Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abcdbcdecdefdefgefghfghighijhijkijkljklmklmnlmnomnopnopq"; + auto hash = hashString(HashType::htSHA256, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha256:248d6a61d20638b8e5c026930c3e6039a33ce45964ff2167f6ecedd419db06c1"); + } + + TEST(hashString, testKnownSHA512Hashes1) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abc"; + auto hash = hashString(HashType::htSHA512, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha512:ddaf35a193617abacc417349ae20413112e6fa4e89a9" + "7ea20a9eeee64b55d39a2192992a274fc1a836ba3c23a3feebbd" + "454d4423643ce80e2a9ac94fa54ca49f"); + } + + TEST(hashString, testKnownSHA512Hashes2) { + // values taken from: https://tools.ietf.org/html/rfc4634 + auto s = "abcdefghbcdefghicdefghijdefghijkefghijklfghijklmghijklmnhijklmnoijklmnopjklmnopqklmnopqrlmnopqrsmnopqrstnopqrstu"; + + auto hash = hashString(HashType::htSHA512, s); + ASSERT_EQ(hash.to_string(Base::Base16), + "sha512:8e959b75dae313da8cf4f72814fc143f8f7779c6eb9f7fa1" + "7299aeadb6889018501d289e4900f7e4331b99dec4b5433a" + "c7d329eeb6dd26545e96e55b874be909"); + } + + TEST(hashString, hashingWithUnknownAlgoExits) { + auto s = "unknown"; + ASSERT_DEATH(hashString(HashType::htUnknown, s), ""); + } +} From c284700867ce09abee1f891a402271a42d207b4d Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 11:57:45 +0200 Subject: [PATCH 30/44] Add unit tests for "json.hh" --- src/libutil/tests/json.cc | 193 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 193 insertions(+) create mode 100644 src/libutil/tests/json.cc diff --git a/src/libutil/tests/json.cc b/src/libutil/tests/json.cc new file mode 100644 index 000000000..dea73f53a --- /dev/null +++ b/src/libutil/tests/json.cc @@ -0,0 +1,193 @@ +#include "json.hh" +#include +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * toJSON + * --------------------------------------------------------------------------*/ + + TEST(toJSON, quotesCharPtr) { + const char* input = "test"; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "\"test\""); + } + + TEST(toJSON, quotesStdString) { + std::string input = "test"; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "\"test\""); + } + + TEST(toJSON, convertsNullptrtoNull) { + auto input = nullptr; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "null"); + } + + TEST(toJSON, convertsNullToNull) { + const char* input = 0; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "null"); + } + + + TEST(toJSON, convertsFloat) { + auto input = 1.024f; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "1.024"); + } + + TEST(toJSON, convertsDouble) { + const double input = 1.024; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "1.024"); + } + + TEST(toJSON, convertsBool) { + auto input = false; + std::stringstream out; + toJSON(out, input); + + ASSERT_EQ(out.str(), "false"); + } + + TEST(toJSON, quotesTab) { + std::stringstream out; + toJSON(out, "\t"); + + ASSERT_EQ(out.str(), "\"\\t\""); + } + + TEST(toJSON, quotesNewline) { + std::stringstream out; + toJSON(out, "\n"); + + ASSERT_EQ(out.str(), "\"\\n\""); + } + + TEST(toJSON, quotesCreturn) { + std::stringstream out; + toJSON(out, "\r"); + + ASSERT_EQ(out.str(), "\"\\r\""); + } + + TEST(toJSON, quotesCreturnNewLine) { + std::stringstream out; + toJSON(out, "\r\n"); + + ASSERT_EQ(out.str(), "\"\\r\\n\""); + } + + TEST(toJSON, quotesDoublequotes) { + std::stringstream out; + toJSON(out, "\""); + + ASSERT_EQ(out.str(), "\"\\\"\""); + } + + TEST(toJSON, substringEscape) { + std::stringstream out; + const char *s = "foo\t"; + toJSON(out, s+3, s + strlen(s)); + + ASSERT_EQ(out.str(), "\"\\t\""); + } + + /* ---------------------------------------------------------------------------- + * JSONObject + * --------------------------------------------------------------------------*/ + + TEST(JSONObject, emptyObject) { + std::stringstream out; + { + JSONObject t(out); + } + ASSERT_EQ(out.str(), "{}"); + } + + TEST(JSONObject, objectWithList) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.list("list"); + l.elem("element"); + } + ASSERT_EQ(out.str(), R"#({"list":["element"]})#"); + } + + TEST(JSONObject, objectWithListIndent) { + std::stringstream out; + { + JSONObject t(out, true); + auto l = t.list("list"); + l.elem("element"); + } + ASSERT_EQ(out.str(), +R"#({ + "list": [ + "element" + ] +})#"); + } + + TEST(JSONObject, objectWithPlaceholderAndList) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.placeholder("list"); + l.list().elem("element"); + } + + ASSERT_EQ(out.str(), R"#({"list":["element"]})#"); + } + + TEST(JSONObject, objectWithPlaceholderAndObject) { + std::stringstream out; + { + JSONObject t(out); + auto l = t.placeholder("object"); + l.object().attr("key", "value"); + } + + ASSERT_EQ(out.str(), R"#({"object":{"key":"value"}})#"); + } + + /* ---------------------------------------------------------------------------- + * JSONList + * --------------------------------------------------------------------------*/ + + TEST(JSONList, empty) { + std::stringstream out; + { + JSONList l(out); + } + ASSERT_EQ(out.str(), R"#([])#"); + } + + TEST(JSONList, withElements) { + std::stringstream out; + { + JSONList l(out); + l.elem("one"); + l.object(); + l.placeholder().write("three"); + } + ASSERT_EQ(out.str(), R"#(["one",{},"three"])#"); + } +} + From 90b0c630a0e9e66f69f1d24b538982c20b5486b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Wed, 26 Feb 2020 10:50:40 +0100 Subject: [PATCH 31/44] install-multi-user: allow overriding user count --- scripts/install-multi-user.sh | 4 +++- scripts/install-nix-from-closure.sh | 3 +++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..991cf1998 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -20,7 +20,9 @@ readonly GREEN='\033[32m' readonly GREEN_UL='\033[4;32m' readonly RED='\033[31m' -readonly NIX_USER_COUNT="32" +# installer allows overriding build user count to speed up installation +# as creating each user takes non-trivial amount of time on macos +readonly NIX_USER_COUNT=${NIX_USER_COUNT:-32} readonly NIX_BUILD_GROUP_ID="30000" readonly NIX_BUILD_GROUP_NAME="nixbld" readonly NIX_FIRST_BUILD_UID="30001" diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 72aa5abf5..6ea23a44a 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -50,6 +50,9 @@ while [ $# -gt 0 ]; do INSTALL_MODE=no-daemon;; --no-channel-add) NIX_INSTALLER_NO_CHANNEL_ADD=1;; + --daemon-user-count) + NIX_USER_COUNT=$2 + shift;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) From 573ff8dfcaacc4495e7f7880d86f70145a074578 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Mon, 25 May 2020 17:31:46 +0200 Subject: [PATCH 32/44] Allow passing extra nix.conf to installer --- scripts/install-multi-user.sh | 1 + scripts/install-nix-from-closure.sh | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index a0f1deb98..b0cb51943 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -646,6 +646,7 @@ setup_default_profile() { place_nix_configuration() { cat < "$SCRATCH/nix.conf" +$NIX_EXTRA_CONF build-users-group = $NIX_BUILD_GROUP_NAME EOF _sudo "to place the default nix daemon configuration (part 2)" \ diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 72aa5abf5..3e0312c78 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -41,6 +41,7 @@ fi INSTALL_MODE=no-daemon CREATE_DARWIN_VOLUME=0 +NIX_EXTRA_CONF= # handle the command line flags while [ $# -gt 0 ]; do case $1 in @@ -54,6 +55,9 @@ while [ $# -gt 0 ]; do NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; + --nix-extra-conf-file) + NIX_EXTRA_CONF=$(cat $2) + shift;; *) ( echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" From 4b388e84318722f970326967e1648675655740fe Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Mon, 25 May 2020 18:34:55 +0200 Subject: [PATCH 33/44] Add unit tests for xml-writer --- src/libutil/tests/xml-writer.cc | 105 ++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) create mode 100644 src/libutil/tests/xml-writer.cc diff --git a/src/libutil/tests/xml-writer.cc b/src/libutil/tests/xml-writer.cc new file mode 100644 index 000000000..adcde25c9 --- /dev/null +++ b/src/libutil/tests/xml-writer.cc @@ -0,0 +1,105 @@ +#include "xml-writer.hh" +#include +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * XMLWriter + * --------------------------------------------------------------------------*/ + + TEST(XMLWriter, emptyObject) { + std::stringstream out; + { + XMLWriter t(false, out); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithEmptyElement) { + std::stringstream out; + { + XMLWriter t(false, out); + t.openElement("foobar"); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithAttrs) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithEmptyAttrs) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = {}; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, objectWithElementWithAttrsEscaping) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "", "" } + }; + t.openElement("foobar", attrs); + } + + // XXX: While "" is escaped, "" isn't which I think is a bug. + ASSERT_EQ(out.str(), "\n=\"<value>\">"); + } + + TEST(XMLWriter, objectWithElementWithAttrsIndented) { + std::stringstream out; + { + XMLWriter t(true, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.openElement("foobar", attrs); + } + + ASSERT_EQ(out.str(), "\n\n\n"); + } + + TEST(XMLWriter, writeEmptyElement) { + std::stringstream out; + { + XMLWriter t(false, out); + t.writeEmptyElement("foobar"); + } + + ASSERT_EQ(out.str(), "\n"); + } + + TEST(XMLWriter, writeEmptyElementWithAttributes) { + std::stringstream out; + { + XMLWriter t(false, out); + XMLAttrs attrs = { + { "foo", "bar" } + }; + t.writeEmptyElement("foobar", attrs); + + } + + ASSERT_EQ(out.str(), "\n"); + } + +} From 1a5ac894e929cefceec22c0f1ca248ee6be445ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 26 May 2020 15:49:26 +0200 Subject: [PATCH 34/44] Fix installer script bugs - --no-channel-add didn't have effect on multi-user installation - some new flags didn't work at all - document all installer flags --- scripts/install-multi-user.sh | 24 ++++++++++++++---------- scripts/install-nix-from-closure.sh | 13 ++++++++----- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 74cc6a5b0..ab41e0242 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -29,6 +29,7 @@ readonly NIX_FIRST_BUILD_UID="30001" # Please don't change this. We don't support it, because the # default shell profile that comes with Nix doesn't support it. readonly NIX_ROOT="/nix" +readonly NIX_EXTRA_CONF=${NIX_EXTRA_CONF:-} readonly PROFILE_TARGETS=("/etc/bashrc" "/etc/profile.d/nix.sh" "/etc/zshrc") readonly PROFILE_BACKUP_SUFFIX=".backup-before-nix" @@ -452,9 +453,11 @@ create_directories() { } place_channel_configuration() { - echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" - _sudo "to set up the default system channel (part 1)" \ - install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" + if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" + _sudo "to set up the default system channel (part 1)" \ + install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" + fi } welcome_to_nix() { @@ -636,13 +639,14 @@ setup_default_profile() { export NIX_SSL_CERT_FILE=/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt fi - # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, - # otherwise it will be lost in environments where sudo doesn't pass - # all the environment variables by default. - _sudo "to update the default channel in the default profile" \ - HOME="$ROOT_HOME" NIX_SSL_CERT_FILE="$NIX_SSL_CERT_FILE" "$NIX_INSTALLED_NIX/bin/nix-channel" --update nixpkgs \ - || channel_update_failed=1 - + if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, + # otherwise it will be lost in environments where sudo doesn't pass + # all the environment variables by default. + _sudo "to update the default channel in the default profile" \ + HOME="$ROOT_HOME" NIX_SSL_CERT_FILE="$NIX_SSL_CERT_FILE" "$NIX_INSTALLED_NIX/bin/nix-channel" --update nixpkgs \ + || channel_update_failed=1 + fi } diff --git a/scripts/install-nix-from-closure.sh b/scripts/install-nix-from-closure.sh index 635aaa16d..826ca8b8c 100644 --- a/scripts/install-nix-from-closure.sh +++ b/scripts/install-nix-from-closure.sh @@ -41,7 +41,6 @@ fi INSTALL_MODE=no-daemon CREATE_DARWIN_VOLUME=0 -NIX_EXTRA_CONF= # handle the command line flags while [ $# -gt 0 ]; do case $1 in @@ -50,20 +49,20 @@ while [ $# -gt 0 ]; do --no-daemon) INSTALL_MODE=no-daemon;; --no-channel-add) - NIX_INSTALLER_NO_CHANNEL_ADD=1;; + export NIX_INSTALLER_NO_CHANNEL_ADD=1;; --daemon-user-count) - NIX_USER_COUNT=$2 + export NIX_USER_COUNT=$2 shift;; --no-modify-profile) NIX_INSTALLER_NO_MODIFY_PROFILE=1;; --darwin-use-unencrypted-nix-store-volume) CREATE_DARWIN_VOLUME=1;; --nix-extra-conf-file) - NIX_EXTRA_CONF=$(cat $2) + export NIX_EXTRA_CONF="$(cat $2)" shift;; *) ( - echo "Nix Installer [--daemon|--no-daemon] [--no-channel-add] [--no-modify-profile]" + echo "Nix Installer [--daemon|--no-daemon] [--daemon-user-count INT] [--no-channel-add] [--no-modify-profile] [--darwin-use-unencrypted-nix-store-volume] [--nix-extra-conf-file FILE]" echo "Choose installation method." echo "" @@ -82,6 +81,10 @@ while [ $# -gt 0 ]; do echo " --no-modify-profile: Skip channel installation. When not provided nixpkgs-unstable" echo " is installed by default." echo "" + echo " --daemon-user-count: Number of build users to create. Defaults to 32." + echo "" + echo " --nix-extra-conf-file: Path to nix.conf to prepend when installing /etc/nix.conf" + echo "" ) >&2 # darwin and Catalina+ From 3d3c219d917525b0a131c4332dd65eadfc818f49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Domen=20Ko=C5=BEar?= Date: Tue, 26 May 2020 16:23:03 +0200 Subject: [PATCH 35/44] installer: fix unused variable --- scripts/install-multi-user.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index ab41e0242..d04b4bbbf 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -453,7 +453,7 @@ create_directories() { } place_channel_configuration() { - if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + if [ -z "${NIX_INSTALLER_NO_CHANNEL_ADD:-}" ]; then echo "https://nixos.org/channels/nixpkgs-unstable nixpkgs" > "$SCRATCH/.nix-channels" _sudo "to set up the default system channel (part 1)" \ install -m 0664 "$SCRATCH/.nix-channels" "$ROOT_HOME/.nix-channels" @@ -639,7 +639,7 @@ setup_default_profile() { export NIX_SSL_CERT_FILE=/nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt fi - if [ -z "$NIX_INSTALLER_NO_CHANNEL_ADD" ]; then + if [ -z "${NIX_INSTALLER_NO_CHANNEL_ADD:-}" ]; then # Have to explicitly pass NIX_SSL_CERT_FILE as part of the sudo call, # otherwise it will be lost in environments where sudo doesn't pass # all the environment variables by default. From b90241ceb13500238e428f8943d221f24bf5322b Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Tue, 26 May 2020 11:32:41 -0400 Subject: [PATCH 36/44] Change remaining bools with FileIngestionMethod --- src/libfetchers/git.cc | 4 ++-- src/libfetchers/mercurial.cc | 2 +- src/libfetchers/tarball.cc | 6 +++--- src/libfetchers/tree-info.cc | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 7c18cf67f..17cc60228 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -195,7 +195,7 @@ struct GitInput : Input return files.count(file); }; - auto storePath = store->addToStore("source", actualUrl, true, htSHA256, filter); + auto storePath = store->addToStore("source", actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); auto tree = Tree { .actualPath = store->printStorePath(storePath), @@ -347,7 +347,7 @@ struct GitInput : Input unpackTarfile(*source, tmpDir); } - auto storePath = store->addToStore(name, tmpDir, true, htSHA256, filter); + auto storePath = store->addToStore(name, tmpDir, FileIngestionMethod::Recursive, htSHA256, filter); auto lastModified = std::stoull(runProgram("git", true, { "-C", repoDir, "log", "-1", "--format=%ct", input->rev->gitRev() })); diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 1d6571571..2e0d4bf4d 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -114,7 +114,7 @@ struct MercurialInput : Input return files.count(file); }; - auto storePath = store->addToStore("source", actualUrl, true, htSHA256, filter); + auto storePath = store->addToStore("source", actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); return {Tree { .actualPath = store->printStorePath(storePath), diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 695525b31..bf2b2a5ff 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -67,10 +67,10 @@ DownloadFileResult downloadFile( StringSink sink; dumpString(*res.data, sink); auto hash = hashString(htSHA256, *res.data); - ValidPathInfo info(store->makeFixedOutputPath(false, hash, name)); + ValidPathInfo info(store->makeFixedOutputPath(FileIngestionMethod::Flat, hash, name)); info.narHash = hashString(htSHA256, *sink.s); info.narSize = sink.s->size(); - info.ca = makeFixedOutputCA(false, hash); + info.ca = makeFixedOutputCA(FileIngestionMethod::Flat, hash); store->addToStore(info, sink.s, NoRepair, NoCheckSigs); storePath = std::move(info.path); } @@ -141,7 +141,7 @@ Tree downloadTarball( throw nix::Error("tarball '%s' contains an unexpected number of top-level files", url); auto topDir = tmpDir + "/" + members.begin()->name; lastModified = lstat(topDir).st_mtime; - unpackedStorePath = store->addToStore(name, topDir, true, htSHA256, defaultPathFilter, NoRepair); + unpackedStorePath = store->addToStore(name, topDir, FileIngestionMethod::Recursive, htSHA256, defaultPathFilter, NoRepair); } Attrs infoAttrs({ diff --git a/src/libfetchers/tree-info.cc b/src/libfetchers/tree-info.cc index 5788e94a1..b2d8cfc8d 100644 --- a/src/libfetchers/tree-info.cc +++ b/src/libfetchers/tree-info.cc @@ -8,7 +8,7 @@ namespace nix::fetchers { StorePath TreeInfo::computeStorePath(Store & store) const { assert(narHash); - return store.makeFixedOutputPath(true, narHash, "source"); + return store.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, "source"); } } From 93129cf1ddf7f91ddec11ba7a6a364ab54f345c5 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Tue, 26 May 2020 17:49:31 +0200 Subject: [PATCH 37/44] Add unit tests for config.cc --- src/libutil/tests/config.cc | 204 ++++++++++++++++++++++++++++++++++++ 1 file changed, 204 insertions(+) create mode 100644 src/libutil/tests/config.cc diff --git a/src/libutil/tests/config.cc b/src/libutil/tests/config.cc new file mode 100644 index 000000000..b0be24d5e --- /dev/null +++ b/src/libutil/tests/config.cc @@ -0,0 +1,204 @@ +#include "json.hh" +#include "config.hh" +#include "args.hh" + +#include +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * Config + * --------------------------------------------------------------------------*/ + + TEST(Config, setUndefinedSetting) { + Config config; + ASSERT_EQ(config.set("undefined-key", "value"), false); + } + + TEST(Config, setDefinedSetting) { + Config config; + std::string value; + Setting foo{&config, value, "name-of-the-setting", "description"}; + ASSERT_EQ(config.set("name-of-the-setting", "value"), true); + } + + TEST(Config, getDefinedSetting) { + Config config; + std::string value; + std::map settings; + Setting foo{&config, value, "name-of-the-setting", "description"}; + + config.getSettings(settings, /* overridenOnly = */ false); + const auto iter = settings.find("name-of-the-setting"); + ASSERT_NE(iter, settings.end()); + ASSERT_EQ(iter->second.value, ""); + ASSERT_EQ(iter->second.description, "description"); + } + + TEST(Config, getDefinedOverridenSettingNotSet) { + Config config; + std::string value; + std::map settings; + Setting foo{&config, value, "name-of-the-setting", "description"}; + + config.getSettings(settings, /* overridenOnly = */ true); + const auto e = settings.find("name-of-the-setting"); + ASSERT_EQ(e, settings.end()); + } + + TEST(Config, getDefinedSettingSet1) { + Config config; + std::string value; + std::map settings; + Setting setting{&config, value, "name-of-the-setting", "description"}; + + setting.assign("value"); + + config.getSettings(settings, /* overridenOnly = */ false); + const auto iter = settings.find("name-of-the-setting"); + ASSERT_NE(iter, settings.end()); + ASSERT_EQ(iter->second.value, "value"); + ASSERT_EQ(iter->second.description, "description"); + } + + TEST(Config, getDefinedSettingSet2) { + Config config; + std::map settings; + Setting setting{&config, "", "name-of-the-setting", "description"}; + + ASSERT_TRUE(config.set("name-of-the-setting", "value")); + + config.getSettings(settings, /* overridenOnly = */ false); + const auto e = settings.find("name-of-the-setting"); + ASSERT_NE(e, settings.end()); + ASSERT_EQ(e->second.value, "value"); + ASSERT_EQ(e->second.description, "description"); + } + + TEST(Config, addSetting) { + class TestSetting : public AbstractSetting { + public: + TestSetting() : AbstractSetting("test", "test", {}) {} + void set(const std::string & value) {} + std::string to_string() const { return {}; } + }; + + Config config; + TestSetting setting; + + ASSERT_FALSE(config.set("test", "value")); + config.addSetting(&setting); + ASSERT_TRUE(config.set("test", "value")); + } + + TEST(Config, withInitialValue) { + const StringMap initials = { + { "key", "value" }, + }; + Config config(initials); + + { + std::map settings; + config.getSettings(settings, /* overridenOnly = */ false); + ASSERT_EQ(settings.find("key"), settings.end()); + } + + Setting setting{&config, "default-value", "key", "description"}; + + { + std::map settings; + config.getSettings(settings, /* overridenOnly = */ false); + ASSERT_EQ(settings["key"].value, "value"); + } + } + + TEST(Config, resetOverriden) { + Config config; + config.resetOverriden(); + } + + TEST(Config, resetOverridenWithSetting) { + Config config; + Setting setting{&config, "", "name-of-the-setting", "description"}; + + { + std::map settings; + + setting.set("foo"); + ASSERT_EQ(setting.get(), "foo"); + config.getSettings(settings, /* overridenOnly = */ true); + ASSERT_TRUE(settings.empty()); + } + + { + std::map settings; + + setting.override("bar"); + ASSERT_TRUE(setting.overriden); + ASSERT_EQ(setting.get(), "bar"); + config.getSettings(settings, /* overridenOnly = */ true); + ASSERT_FALSE(settings.empty()); + } + + { + std::map settings; + + config.resetOverriden(); + ASSERT_FALSE(setting.overriden); + config.getSettings(settings, /* overridenOnly = */ true); + ASSERT_TRUE(settings.empty()); + } + } + + TEST(Config, toJSONOnEmptyConfig) { + std::stringstream out; + { // Scoped to force the destructor of JSONObject to write the final `}` + JSONObject obj(out); + Config config; + config.toJSON(obj); + } + + ASSERT_EQ(out.str(), "{}"); + } + + TEST(Config, toJSONOnNonEmptyConfig) { + std::stringstream out; + { // Scoped to force the destructor of JSONObject to write the final `}` + JSONObject obj(out); + + Config config; + std::map settings; + Setting setting{&config, "", "name-of-the-setting", "description"}; + setting.assign("value"); + + config.toJSON(obj); + } + ASSERT_EQ(out.str(), R"#({"name-of-the-setting":{"description":"description","value":"value"}})#"); + } + + TEST(Config, setSettingAlias) { + Config config; + Setting setting{&config, "", "some-int", "best number", { "another-int" }}; + ASSERT_TRUE(config.set("some-int", "1")); + ASSERT_EQ(setting.get(), "1"); + ASSERT_TRUE(config.set("another-int", "2")); + ASSERT_EQ(setting.get(), "2"); + ASSERT_TRUE(config.set("some-int", "3")); + ASSERT_EQ(setting.get(), "3"); + } + + /* FIXME: The reapplyUnknownSettings method doesn't seem to do anything + * useful (these days). Whenever we add a new setting to Config the + * unknown settings are always considered. In which case is this function + * actually useful? Is there some way to register a Setting without calling + * addSetting? */ + TEST(Config, DISABLED_reapplyUnknownSettings) { + Config config; + ASSERT_FALSE(config.set("name-of-the-setting", "unknownvalue")); + Setting setting{&config, "default", "name-of-the-setting", "description"}; + ASSERT_EQ(setting.get(), "default"); + config.reapplyUnknownSettings(); + ASSERT_EQ(setting.get(), "unknownvalue"); + } +} From e1b8c64c04b40694418de87e3e0d118cf0677bfc Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Wed, 27 May 2020 14:39:02 +0200 Subject: [PATCH 38/44] config.cc: extract parts of applyConfigFile into applyConfig This moves the actual parsing of configuration contents into applyConfig which applyConfigFile is then going to call. By changing this we can now test the configuration file parsing without actually create a file on disk. --- src/libutil/config.cc | 103 ++++++++++++++++++++++-------------------- src/libutil/config.hh | 1 + 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/src/libutil/config.cc b/src/libutil/config.cc index f03e444ec..8fc700a2b 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -65,60 +65,63 @@ void Config::getSettings(std::map & res, bool override res.emplace(opt.first, SettingInfo{opt.second.setting->to_string(), opt.second.setting->description}); } +void AbstractConfig::applyConfig(const std::string & contents, const std::string & path) { + unsigned int pos = 0; + + while (pos < contents.size()) { + string line; + while (pos < contents.size() && contents[pos] != '\n') + line += contents[pos++]; + pos++; + + string::size_type hash = line.find('#'); + if (hash != string::npos) + line = string(line, 0, hash); + + vector tokens = tokenizeString >(line); + if (tokens.empty()) continue; + + if (tokens.size() < 2) + throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); + + auto include = false; + auto ignoreMissing = false; + if (tokens[0] == "include") + include = true; + else if (tokens[0] == "!include") { + include = true; + ignoreMissing = true; + } + + if (include) { + if (tokens.size() != 2) + throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); + auto p = absPath(tokens[1], dirOf(path)); + if (pathExists(p)) { + applyConfigFile(p); + } else if (!ignoreMissing) { + throw Error("file '%1%' included from '%2%' not found", p, path); + } + continue; + } + + if (tokens[1] != "=") + throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); + + string name = tokens[0]; + + vector::iterator i = tokens.begin(); + advance(i, 2); + + set(name, concatStringsSep(" ", Strings(i, tokens.end()))); // FIXME: slow + }; +} + void AbstractConfig::applyConfigFile(const Path & path) { try { string contents = readFile(path); - - unsigned int pos = 0; - - while (pos < contents.size()) { - string line; - while (pos < contents.size() && contents[pos] != '\n') - line += contents[pos++]; - pos++; - - string::size_type hash = line.find('#'); - if (hash != string::npos) - line = string(line, 0, hash); - - vector tokens = tokenizeString >(line); - if (tokens.empty()) continue; - - if (tokens.size() < 2) - throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); - - auto include = false; - auto ignoreMissing = false; - if (tokens[0] == "include") - include = true; - else if (tokens[0] == "!include") { - include = true; - ignoreMissing = true; - } - - if (include) { - if (tokens.size() != 2) - throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); - auto p = absPath(tokens[1], dirOf(path)); - if (pathExists(p)) { - applyConfigFile(p); - } else if (!ignoreMissing) { - throw Error("file '%1%' included from '%2%' not found", p, path); - } - continue; - } - - if (tokens[1] != "=") - throw UsageError("illegal configuration line '%1%' in '%2%'", line, path); - - string name = tokens[0]; - - vector::iterator i = tokens.begin(); - advance(i, 2); - - set(name, concatStringsSep(" ", Strings(i, tokens.end()))); // FIXME: slow - }; + applyConfig(contents, path); } catch (SysError &) { } } diff --git a/src/libutil/config.hh b/src/libutil/config.hh index 7ea78fdaf..b04cba88b 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -33,6 +33,7 @@ public: virtual void getSettings(std::map & res, bool overridenOnly = false) = 0; + void applyConfig(const std::string & contents, const std::string & path = ""); void applyConfigFile(const Path & path); virtual void resetOverriden() = 0; From 9df3d8ccd7a3130faccb73933d0c196a81d605eb Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Wed, 27 May 2020 14:41:01 +0200 Subject: [PATCH 39/44] tests/config.cc: add tests for Config::applyConfig --- src/libutil/tests/config.cc | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/libutil/tests/config.cc b/src/libutil/tests/config.cc index b0be24d5e..74c59fd31 100644 --- a/src/libutil/tests/config.cc +++ b/src/libutil/tests/config.cc @@ -201,4 +201,64 @@ namespace nix { config.reapplyUnknownSettings(); ASSERT_EQ(setting.get(), "unknownvalue"); } + + TEST(Config, applyConfigEmpty) { + Config config; + std::map settings; + config.applyConfig(""); + config.getSettings(settings); + ASSERT_TRUE(settings.empty()); + } + + TEST(Config, applyConfigEmptyWithComment) { + Config config; + std::map settings; + config.applyConfig("# just a comment"); + config.getSettings(settings); + ASSERT_TRUE(settings.empty()); + } + + TEST(Config, applyConfigAssignment) { + Config config; + std::map settings; + Setting setting{&config, "", "name-of-the-setting", "description"}; + config.applyConfig( + "name-of-the-setting = value-from-file #useful comment\n" + "# name-of-the-setting = foo\n" + ); + config.getSettings(settings); + ASSERT_FALSE(settings.empty()); + ASSERT_EQ(settings["name-of-the-setting"].value, "value-from-file"); + } + + TEST(Config, applyConfigWithReassignedSetting) { + Config config; + std::map settings; + Setting setting{&config, "", "name-of-the-setting", "description"}; + config.applyConfig( + "name-of-the-setting = first-value\n" + "name-of-the-setting = second-value\n" + ); + config.getSettings(settings); + ASSERT_FALSE(settings.empty()); + ASSERT_EQ(settings["name-of-the-setting"].value, "second-value"); + } + + TEST(Config, applyConfigFailsOnMissingIncludes) { + Config config; + std::map settings; + Setting setting{&config, "", "name-of-the-setting", "description"}; + + ASSERT_THROW(config.applyConfig( + "name-of-the-setting = value-from-file\n" + "# name-of-the-setting = foo\n" + "include /nix/store/does/not/exist.nix" + ), Error); + } + + TEST(Config, applyConfigInvalidThrows) { + Config config; + ASSERT_THROW(config.applyConfig("value == key"), UsageError); + ASSERT_THROW(config.applyConfig("value "), UsageError); + } } From fc137d2f007c32f27a49e1a00d5114c7d7663419 Mon Sep 17 00:00:00 2001 From: Andreas Rammhold Date: Wed, 27 May 2020 16:37:24 +0200 Subject: [PATCH 40/44] config.hh: Add documentation Provides some general overview on the mechanics of Config/Setting and comments for the public methods of Config. --- src/libutil/config.hh | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/src/libutil/config.hh b/src/libutil/config.hh index b04cba88b..5c7a70a2e 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -7,6 +7,38 @@ namespace nix { +/** + * The Config class provides Nix runtime configurations. + * + * What is a Configuration? + * A collection of uniquely named Settings. + * + * What is a Setting? + * Each property that you can set in a configuration corresponds to a + * `Setting`. A setting records value and description of a property + * with a default and optional aliases. + * + * A valid configuration consists of settings that are registered to a + * `Config` object instance: + * + * Config config; + * Setting systemSetting{&config, "x86_64-linux", "system", "the current system"}; + * + * The above creates a `Config` object and registers a setting called "system" + * via the variable `systemSetting` with it. The setting defaults to the string + * "x86_64-linux", it's description is "the current system". All of the + * registered settings can then be accessed as shown below: + * + * std::map settings; + * config.getSettings(settings); + * config["system"].description == "the current system" + * config["system"].value == "x86_64-linux" + * + * + * The above retrieves all currently known settings from the `Config` object + * and adds them to the `settings` map. + */ + class Args; class AbstractSetting; class JSONPlaceholder; @@ -23,6 +55,10 @@ protected: public: + /** + * Sets the value referenced by `name` to `value`. Returns true if the + * setting is known, false otherwise. + */ virtual bool set(const std::string & name, const std::string & value) = 0; struct SettingInfo @@ -31,19 +67,52 @@ public: std::string description; }; + /** + * Adds the currently known settings to the given result map `res`. + * - res: map to store settings in + * - overridenOnly: when set to true only overridden settings will be added to `res` + */ virtual void getSettings(std::map & res, bool overridenOnly = false) = 0; + /** + * Parses the configuration in `contents` and applies it + * - contents: configuration contents to be parsed and applied + * - path: location of the configuration file + */ void applyConfig(const std::string & contents, const std::string & path = ""); + + /** + * Applies a nix configuration file + * - path: the location of the config file to apply + */ void applyConfigFile(const Path & path); + /** + * Resets the `overridden` flag of all Settings + */ virtual void resetOverriden() = 0; + /** + * Outputs all settings to JSON + * - out: JSONObject to write the configuration to + */ virtual void toJSON(JSONObject & out) = 0; + /** + * Converts settings to `Args` to be used on the command line interface + * - args: args to write to + * - category: category of the settings + */ virtual void convertToArgs(Args & args, const std::string & category) = 0; + /** + * Logs a warning for each unregistered setting + */ void warnUnknownSettings(); + /** + * Re-applies all previously attempted changes to unknown settings + */ void reapplyUnknownSettings(); }; From 7873fd175d9e4e5bdb281e005b18c388bcfe1dd8 Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 27 May 2020 13:11:45 -0500 Subject: [PATCH 41/44] =?UTF-8?q?Don=E2=80=99t=20use=20FileIngestionMethod?= =?UTF-8?q?=20for=20StorePathsCommand?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a different recursive than used in makeFixedOutputPath. --- src/nix/command.cc | 10 +++++----- src/nix/command.hh | 4 ++-- src/nix/copy.cc | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/nix/command.cc b/src/nix/command.cc index ea0ade88e..71b027719 100644 --- a/src/nix/command.cc +++ b/src/nix/command.cc @@ -31,21 +31,21 @@ void StoreCommand::run() run(getStore()); } -StorePathsCommand::StorePathsCommand(FileIngestionMethod recursive) +StorePathsCommand::StorePathsCommand(bool recursive) : recursive(recursive) { - if (recursive == FileIngestionMethod::Recursive) + if (recursive) addFlag({ .longName = "no-recursive", .description = "apply operation to specified paths only", - .handler = {&this->recursive, FileIngestionMethod::Flat}, + .handler = {&this->recursive, false}, }); else addFlag({ .longName = "recursive", .shortName = 'r', .description = "apply operation to closure of the specified paths", - .handler = {&this->recursive, FileIngestionMethod::Recursive}, + .handler = {&this->recursive, true}, }); mkFlag(0, "all", "apply operation to the entire store", &all); @@ -66,7 +66,7 @@ void StorePathsCommand::run(ref store) for (auto & p : toStorePaths(store, realiseMode, installables)) storePaths.push_back(p.clone()); - if (recursive == FileIngestionMethod::Recursive) { + if (recursive) { StorePathSet closure; store->computeFSClosure(storePathsToSet(storePaths), closure, false, false); storePaths.clear(); diff --git a/src/nix/command.hh b/src/nix/command.hh index 09c621b5b..959d5f19d 100644 --- a/src/nix/command.hh +++ b/src/nix/command.hh @@ -92,7 +92,7 @@ struct StorePathsCommand : public InstallablesCommand { private: - FileIngestionMethod recursive = FileIngestionMethod::Flat; + bool recursive = false; bool all = false; protected: @@ -101,7 +101,7 @@ protected: public: - StorePathsCommand(FileIngestionMethod recursive = FileIngestionMethod::Flat); + StorePathsCommand(bool recursive = false); using StoreCommand::run; diff --git a/src/nix/copy.cc b/src/nix/copy.cc index 60aa3f14b..c7c38709d 100644 --- a/src/nix/copy.cc +++ b/src/nix/copy.cc @@ -17,7 +17,7 @@ struct CmdCopy : StorePathsCommand SubstituteFlag substitute = NoSubstitute; CmdCopy() - : StorePathsCommand(FileIngestionMethod::Recursive) + : StorePathsCommand(true) { addFlag({ .longName = "from", From c66441a646c2bbfab05fa4a4fd05c2ae897dda8b Mon Sep 17 00:00:00 2001 From: Matthew Bauer Date: Wed, 27 May 2020 13:04:20 -0500 Subject: [PATCH 42/44] =?UTF-8?q?Rename=20some=20variables=20named=20?= =?UTF-8?q?=E2=80=9Crecursive=E2=80=9D=20to=20=E2=80=9Cmethod=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is much less confusing since recursive is no longer a boolean. --- src/libexpr/primops.cc | 14 +++++++------- src/libstore/binary-cache-store.cc | 6 +++--- src/libstore/binary-cache-store.hh | 2 +- src/libstore/build.cc | 18 +++++++++--------- src/libstore/local-store.cc | 22 +++++++++++----------- src/libstore/store-api.cc | 6 +++--- 6 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index fcbb75ceb..d458ab272 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1042,7 +1042,7 @@ static void prim_toFile(EvalState & state, const Pos & pos, Value * * args, Valu static void addPath(EvalState & state, const Pos & pos, const string & name, const Path & path_, - Value * filterFun, FileIngestionMethod recursive, const Hash & expectedHash, Value & v) + Value * filterFun, FileIngestionMethod method, const Hash & expectedHash, Value & v) { const auto path = evalSettings.pureEval && expectedHash ? path_ : @@ -1073,12 +1073,12 @@ static void addPath(EvalState & state, const Pos & pos, const string & name, con std::optional expectedStorePath; if (expectedHash) - expectedStorePath = state.store->makeFixedOutputPath(recursive, expectedHash, name); + expectedStorePath = state.store->makeFixedOutputPath(method, expectedHash, name); Path dstPath; if (!expectedHash || !state.store->isValidPath(*expectedStorePath)) { dstPath = state.store->printStorePath(settings.readOnlyMode - ? state.store->computeStorePathForPath(name, path, recursive, htSHA256, filter).first - : state.store->addToStore(name, path, recursive, htSHA256, filter, state.repair)); + ? state.store->computeStorePathForPath(name, path, method, htSHA256, filter).first + : state.store->addToStore(name, path, method, htSHA256, filter, state.repair)); if (expectedHash && expectedStorePath != state.store->parseStorePath(dstPath)) throw Error("store path mismatch in (possibly filtered) path added from '%s'", path); } else @@ -1108,7 +1108,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value Path path; string name; Value * filterFun = nullptr; - auto recursive = FileIngestionMethod::Recursive; + auto method = FileIngestionMethod::Recursive; Hash expectedHash; for (auto & attr : *args[0]->attrs) { @@ -1124,7 +1124,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value state.forceValue(*attr.value, pos); filterFun = attr.value; } else if (n == "recursive") - recursive = FileIngestionMethod { state.forceBool(*attr.value, *attr.pos) }; + method = FileIngestionMethod { state.forceBool(*attr.value, *attr.pos) }; else if (n == "sha256") expectedHash = Hash(state.forceStringNoCtx(*attr.value, *attr.pos), htSHA256); else @@ -1135,7 +1135,7 @@ static void prim_path(EvalState & state, const Pos & pos, Value * * args, Value if (name.empty()) name = baseNameOf(path); - addPath(state, pos, name, path, filterFun, recursive, expectedHash, v); + addPath(state, pos, name, path, filterFun, method, expectedHash, v); } diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index a61af4a00..f13736c58 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -327,7 +327,7 @@ void BinaryCacheStore::queryPathInfoUncached(const StorePath & storePath, } StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath, - FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { // FIXME: some cut&paste from LocalStore::addToStore(). @@ -336,7 +336,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath small files. */ StringSink sink; Hash h; - if (recursive == FileIngestionMethod::Recursive) { + if (method == FileIngestionMethod::Recursive) { dumpPath(srcPath, sink, filter); h = hashString(hashAlgo, *sink.s); } else { @@ -345,7 +345,7 @@ StorePath BinaryCacheStore::addToStore(const string & name, const Path & srcPath h = hashString(hashAlgo, s); } - ValidPathInfo info(makeFixedOutputPath(recursive, h, name)); + ValidPathInfo info(makeFixedOutputPath(method, h, name)); addToStore(info, sink.s, repair, CheckSigs, nullptr); diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 1a1ea636c..4ff5609ed 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -79,7 +79,7 @@ public: std::shared_ptr accessor) override; StorePath addToStore(const string & name, const Path & srcPath, - FileIngestionMethod recursive, HashType hashAlgo, + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) override; StorePath addTextToStore(const string & name, const string & s, diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 272c4f1ae..f5c132a83 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2734,7 +2734,7 @@ struct RestrictedStore : public LocalFSStore { throw Error("queryPathFromHashPart"); } StorePath addToStore(const string & name, const Path & srcPath, - FileIngestionMethod recursive = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override { throw Error("addToStore"); } @@ -2747,9 +2747,9 @@ struct RestrictedStore : public LocalFSStore } StorePath addToStoreFromDump(const string & dump, const string & name, - FileIngestionMethod recursive = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override { - auto path = next->addToStoreFromDump(dump, name, recursive, hashAlgo, repair); + auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair); goal.addDependency(path); return path; } @@ -3692,10 +3692,10 @@ void DerivationGoal::registerOutputs() if (fixedOutput) { - FileIngestionMethod recursive; Hash h; - i.second.parseHashInfo(recursive, h); + FileIngestionMethod outputHashMode; Hash h; + i.second.parseHashInfo(outputHashMode, h); - if (recursive == FileIngestionMethod::Flat) { + if (outputHashMode == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ if (!S_ISREG(st.st_mode) || (st.st_mode & S_IXUSR) != 0) throw BuildError( @@ -3705,11 +3705,11 @@ void DerivationGoal::registerOutputs() /* Check the hash. In hash mode, move the path produced by the derivation to its content-addressed location. */ - Hash h2 = recursive == FileIngestionMethod::Recursive + Hash h2 = outputHashMode == FileIngestionMethod::Recursive ? hashPath(h.type, actualPath).first : hashFile(h.type, actualPath); - auto dest = worker.store.makeFixedOutputPath(recursive, h2, i.second.path.name()); + auto dest = worker.store.makeFixedOutputPath(outputHashMode, h2, i.second.path.name()); if (h != h2) { @@ -3738,7 +3738,7 @@ void DerivationGoal::registerOutputs() else assert(worker.store.parseStorePath(path) == dest); - ca = makeFixedOutputCA(recursive, h2); + ca = makeFixedOutputCA(outputHashMode, h2); } /* Get rid of all weird permissions. This also checks that diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 746f81beb..80851b591 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -557,10 +557,10 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat if (out == drv.outputs.end()) throw Error("derivation '%s' does not have an output named 'out'", printStorePath(drvPath)); - FileIngestionMethod recursive; Hash h; - out->second.parseHashInfo(recursive, h); + FileIngestionMethod method; Hash h; + out->second.parseHashInfo(method, h); - check(makeFixedOutputPath(recursive, h, drvName), out->second.path, "out"); + check(makeFixedOutputPath(method, h, drvName), out->second.path, "out"); } else { @@ -1043,11 +1043,11 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source, StorePath LocalStore::addToStoreFromDump(const string & dump, const string & name, - FileIngestionMethod recursive, HashType hashAlgo, RepairFlag repair) + FileIngestionMethod method, HashType hashAlgo, RepairFlag repair) { Hash h = hashString(hashAlgo, dump); - auto dstPath = makeFixedOutputPath(recursive, h, name); + auto dstPath = makeFixedOutputPath(method, h, name); addTempRoot(dstPath); @@ -1067,7 +1067,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam autoGC(); - if (recursive == FileIngestionMethod::Recursive) { + if (method == FileIngestionMethod::Recursive) { StringSource source(dump); restorePath(realPath, source); } else @@ -1080,7 +1080,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam above (if called with recursive == true and hashAlgo == sha256); otherwise, compute it here. */ HashResult hash; - if (recursive == FileIngestionMethod::Recursive) { + if (method == FileIngestionMethod::Recursive) { hash.first = hashAlgo == htSHA256 ? h : hashString(htSHA256, dump); hash.second = dump.size(); } else @@ -1091,7 +1091,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam ValidPathInfo info(dstPath.clone()); info.narHash = hash.first; info.narSize = hash.second; - info.ca = makeFixedOutputCA(recursive, h); + info.ca = makeFixedOutputCA(method, h); registerValidPath(info); } @@ -1103,7 +1103,7 @@ StorePath LocalStore::addToStoreFromDump(const string & dump, const string & nam StorePath LocalStore::addToStore(const string & name, const Path & _srcPath, - FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter, RepairFlag repair) + FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) { Path srcPath(absPath(_srcPath)); @@ -1111,12 +1111,12 @@ StorePath 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 == FileIngestionMethod::Recursive) + if (method == FileIngestionMethod::Recursive) dumpPath(srcPath, sink, filter); else sink.s = make_ref(readFile(srcPath)); - return addToStoreFromDump(*sink.s, name, recursive, hashAlgo, repair); + return addToStoreFromDump(*sink.s, name, method, hashAlgo, repair); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 86cb20a26..d479b86cb 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -204,12 +204,12 @@ StorePath Store::makeTextPath(std::string_view name, const Hash & hash, std::pair Store::computeStorePathForPath(std::string_view name, - const Path & srcPath, FileIngestionMethod recursive, HashType hashAlgo, PathFilter & filter) const + const Path & srcPath, FileIngestionMethod method, HashType hashAlgo, PathFilter & filter) const { - Hash h = recursive == FileIngestionMethod::Recursive + Hash h = method == FileIngestionMethod::Recursive ? hashPath(hashAlgo, srcPath, filter).first : hashFile(hashAlgo, srcPath); - return std::make_pair(makeFixedOutputPath(recursive, h, name), h); + return std::make_pair(makeFixedOutputPath(method, h, name), h); } From 4e6d7cb55a884ac19990b487c2020ea257e0f9c3 Mon Sep 17 00:00:00 2001 From: Daiderd Jordan Date: Wed, 27 May 2020 20:58:42 +0200 Subject: [PATCH 43/44] installer: don't require xz on darwin On macOS the system tar has builtin support for lzma while xz isn't available as a separate binary. There's no builtin package manager there available either so having to install lzma (without nix) would be rather painful. --- scripts/install.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/install.in b/scripts/install.in index 6709f00d4..1d26c4ff0 100644 --- a/scripts/install.in +++ b/scripts/install.in @@ -36,7 +36,9 @@ tarball="$tmpDir/$(basename "$tmpDir/nix-@nixVersion@-$system.tar.xz")" require_util curl "download the binary tarball" require_util tar "unpack the binary tarball" -require_util xz "unpack the binary tarball" +if [ "$(uname -s)" != "Darwin" ]; then + require_util xz "unpack the binary tarball" +fi echo "downloading Nix @nixVersion@ binary tarball for $system from '$url' to '$tmpDir'..." curl -L "$url" -o "$tarball" || oops "failed to download '$url'" From 0f96f45061e574c9cf217e3dd5634dfdbf210235 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 27 May 2020 23:50:11 -0400 Subject: [PATCH 44/44] Use `FileIngestionMethod` for `nix hash` There was an enum there that matched in perfectly. --- src/nix/hash.cc | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 366314227..0a7c343d0 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -9,15 +9,14 @@ using namespace nix; struct CmdHash : Command { - enum Mode { mFile, mPath }; - Mode mode; + FileIngestionMethod mode; Base base = SRI; bool truncate = false; HashType ht = htSHA256; std::vector paths; std::optional modulus; - CmdHash(Mode mode) : mode(mode) + CmdHash(FileIngestionMethod mode) : mode(mode) { mkFlag(0, "sri", "print hash in SRI format", &base, SRI); mkFlag(0, "base64", "print hash in base-64", &base, Base64); @@ -36,9 +35,14 @@ struct CmdHash : Command std::string description() override { - return mode == mFile - ? "print cryptographic hash of a regular file" - : "print cryptographic hash of the NAR serialisation of a path"; + const char* d; + switch (mode) { + case FileIngestionMethod::Flat: + d = "print cryptographic hash of a regular file"; + case FileIngestionMethod::Recursive: + d = "print cryptographic hash of the NAR serialisation of a path"; + }; + return d; } Category category() override { return catUtility; } @@ -53,10 +57,14 @@ struct CmdHash : Command else hashSink = std::make_unique(ht); - if (mode == mFile) + switch (mode) { + case FileIngestionMethod::Flat: readFile(path, *hashSink); - else + break; + case FileIngestionMethod::Recursive: dumpPath(path, *hashSink); + break; + } Hash h = hashSink->finish().first; if (truncate && h.hashSize > 20) h = compressHash(h, 20); @@ -65,8 +73,8 @@ struct CmdHash : Command } }; -static RegisterCommand r1("hash-file", [](){ return make_ref(CmdHash::mFile); }); -static RegisterCommand r2("hash-path", [](){ return make_ref(CmdHash::mPath); }); +static RegisterCommand r1("hash-file", [](){ return make_ref(FileIngestionMethod::Flat); }); +static RegisterCommand r2("hash-path", [](){ return make_ref(FileIngestionMethod::Recursive); }); struct CmdToBase : Command { @@ -137,7 +145,7 @@ static int compatNixHash(int argc, char * * argv) }); if (op == opHash) { - CmdHash cmd(flat ? CmdHash::mFile : CmdHash::mPath); + CmdHash cmd(flat ? FileIngestionMethod::Flat : FileIngestionMethod::Recursive); cmd.ht = ht; cmd.base = base32 ? Base32 : Base16; cmd.truncate = truncate;