From 400d70a3a9ab494b2eebf5522789f89d16d000b2 Mon Sep 17 00:00:00 2001 From: toonn Date: Tue, 22 Feb 2022 16:28:24 +0100 Subject: [PATCH 01/98] doc: Add detailed uninstall section for macOS The multi-user installation on macOS, which is now the only option, has gotten complicated enough that it discourages some users from checking Nix out for fear of being left with a "dirty" system. Detailed uninstallation instructions should make this less of an issue. --- .../src/installation/installing-binary.md | 84 +++++++++++++++++-- 1 file changed, 79 insertions(+), 5 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index 4367654a2..ba90ce280 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -84,7 +84,9 @@ The installer will modify `/etc/bashrc`, and `/etc/zshrc` if they exist. The installer will first back up these files with a `.backup-before-nix` extension. The installer will also create `/etc/profile.d/nix.sh`. -You can uninstall Nix with the following commands: +## Uninstalling + +### Linux ```console sudo rm -rf /etc/profile/nix.sh /etc/nix /nix ~root/.nix-profile ~root/.nix-defexpr ~root/.nix-channels ~/.nix-profile ~/.nix-defexpr ~/.nix-channels @@ -95,15 +97,87 @@ sudo systemctl stop nix-daemon.service sudo systemctl disable nix-daemon.socket sudo systemctl disable nix-daemon.service sudo systemctl daemon-reload - -# If you are on macOS, you will need to run: -sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist -sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist ``` There may also be references to Nix in `/etc/profile`, `/etc/bashrc`, and `/etc/zshrc` which you may remove. +### macOS + +1. Edit `/etc/zshrc` and `/etc/bashrc` to remove the lines sourcing + `nix-daemon.sh`, which should look like this: + + ```bash + # Nix + if [ -e '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' ]; then + . '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' + fi + # End Nix + ``` + + If these files haven't been altered since installing Nix you can simply put + the backups back in place: + + ```console + sudo mv /etc/zshrc.backup-before-nix /etc/zshrc + sudo mv /etc/bashrc.backup-before-nix /etc/bashrc + ``` + + This will stop shells from sourcing the file and bringing everything you + installed using Nix in scope. + +2. Stop and remove the Nix daemon services: + + ```console + sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo launchctl unload /Library/LaunchDaemons/org.nixos.activate-system.plist + sudo rm /Library/LaunchDaemons/org.nixos.activate-system.plist + ``` + + This stops the Nix daemon and prevents it from being started next time you + boot the system. + +3. Remove the `nixbld` group and the `_nixbuildN` users: + + ```console + sudo dscl . -delete /Groups/nixbld + for u in $(sudo dscl . -list /Users | grep _nixbld); do sudo dscl . -delete /Users/$u; done + ``` + + This will remove all the build users that no longer serve a purpose. + +4. Edit fstab using `sudo vifs` to remove the line mounting the Nix Store + volume on `/nix`, which looks like this, + `LABEL=Nix\040Store /nix apfs rw,nobrowse`. This will prevent automatic + mounting of the Nix Store volume. + +5. Edit `/etc/synthetic.conf` to remove the `nix` line. If this is the only + line in the file you can remove it entirely, `sudo rm /etc/synthetic.conf`. + This will prevent the creation of the empty `/nix` directory to provide a + mountpoint for the Nix Store volume. + +6. Remove the files Nix added to your system: + + ```console + sudo rm -rf /etc/nix /var/root/.nix-profile /var/root/.nix-defexpr /var/root/.nix-channels ~/.nix-profile ~/.nix-defexpr ~/.nix-channels + ``` + + This gets rid of any data Nix may have created except for the store which is + removed next. + +7. Remove the Nix Store volume: + + ```console + sudo diskutil apfs deleteVolume /nix + ``` + + This will remove the Nix Store volume and everything that was added to the + store. + +The change to `/etc/synthetic.conf` will only take effect after a reboot but +you shouldn't have any traces of Nix left on your system. + # macOS Installation From 064cad7e9fec88a2906dd0f87084278ec119f3af Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 25 Feb 2022 10:32:45 +0100 Subject: [PATCH 02/98] doc: Add macOS uninstall note about /nix Clarify that `/nix` being present after the uninstall is normal and it will only disappear after a reboot. Co-authored-by: Travis A. Everett --- doc/manual/src/installation/installing-binary.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index ba90ce280..ea5c9febe 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -175,8 +175,16 @@ and `/etc/zshrc` which you may remove. This will remove the Nix Store volume and everything that was added to the store. -The change to `/etc/synthetic.conf` will only take effect after a reboot but -you shouldn't have any traces of Nix left on your system. +> **Note** +> +> After you complete the steps here, you will still have an empty `/nix` +> directory. This is an expected sign of a successful uninstall. The empty +> `/nix` directory will disappear the next time you reboot. +> +> You do not have to reboot to finish uninstalling Nix. The uninstall is +> complete. macOS (Catalina+) directly controls root directories and its +> read-only root will prevent you from manually deleting the empty `/nix` +> mountpoint. # macOS Installation From 2df23e2b3e9dd456a42b93264ccd5390e88d8542 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 25 Feb 2022 10:50:01 +0100 Subject: [PATCH 03/98] doc: Drop nix-darwin service from macOS uninstall --- doc/manual/src/installation/installing-binary.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index ea5c9febe..eb184c07b 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -131,8 +131,6 @@ and `/etc/zshrc` which you may remove. ```console sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist - sudo launchctl unload /Library/LaunchDaemons/org.nixos.activate-system.plist - sudo rm /Library/LaunchDaemons/org.nixos.activate-system.plist ``` This stops the Nix daemon and prevents it from being started next time you From 947d4761b35ffa5f689ad22d631696d35d28b941 Mon Sep 17 00:00:00 2001 From: toonn Date: Sat, 26 Feb 2022 14:16:35 +0100 Subject: [PATCH 04/98] doc: Add removal of darwin-store LaunchDaemon The uninstall instructions used to accidentally remove the nix-darwin LaunchDaemon, this was dropped. However, the original intent was to remove the Store volume mounting LaunchDaemon. --- doc/manual/src/installation/installing-binary.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index eb184c07b..e5fb50088 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -131,6 +131,8 @@ and `/etc/zshrc` which you may remove. ```console sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo launchctl unload /Library/LaunchDaemons/org.nixos.darwin-store.plist + sudo rm /Library/LaunchDaemons/org.nixos.darwin-store.plist ``` This stops the Nix daemon and prevents it from being started next time you From 91adfb8894b4b8183c2948712d56a97bb9d93d9f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 10 Mar 2021 02:20:32 +0000 Subject: [PATCH 05/98] Create some type aliases for string Contexts --- src/libexpr/eval-cache.cc | 2 +- src/libexpr/eval-cache.hh | 2 +- src/libexpr/eval.cc | 6 +++--- src/libexpr/eval.hh | 2 +- src/libexpr/primops/context.cc | 2 +- src/libexpr/value.hh | 4 +++- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 188223957..7e5b5c9c4 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -254,7 +254,7 @@ struct AttrDb return {{rowId, attrs}}; } case AttrType::String: { - std::vector> context; + NixStringContext context; if (!queryAttribute.isNull(3)) for (auto & s : tokenizeString>(queryAttribute.getStr(3), ";")) context.push_back(decodeContext(s)); diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 40f1d4ffc..c9a9bf471 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -52,7 +52,7 @@ struct misc_t {}; struct failed_t {}; typedef uint64_t AttrId; typedef std::pair AttrKey; -typedef std::pair>> string_t; +typedef std::pair string_t; typedef std::variant< std::vector, diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c65ef9738..126e0af8c 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1903,7 +1903,7 @@ std::string_view EvalState::forceString(Value & v, const Pos & pos) /* Decode a context string ‘!!’ into a pair . */ -std::pair decodeContext(std::string_view s) +NixStringContextElem decodeContext(std::string_view s) { if (s.at(0) == '!') { size_t index = s.find("!", 1); @@ -1921,9 +1921,9 @@ void copyContext(const Value & v, PathSet & context) } -std::vector> Value::getContext() +NixStringContext Value::getContext() { - std::vector> res; + NixStringContext res; assert(internalType == tString); if (string.context) for (const char * * p = string.context; *p; ++p) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index f1e00bae7..18480f290 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -430,7 +430,7 @@ std::string showType(const Value & v); /* Decode a context string ‘!!’ into a pair . */ -std::pair decodeContext(std::string_view s); +NixStringContextElem decodeContext(std::string_view s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 3701bd442..ad4de3840 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -82,7 +82,7 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, drv = std::string(p, 1); path = &drv; } else if (p.at(0) == '!') { - std::pair ctx = decodeContext(p); + NixStringContextElem ctx = decodeContext(p); drv = ctx.first; output = ctx.second; path = &drv; diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index d0fa93e92..ee9cb832a 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -64,6 +64,8 @@ class JSONPlaceholder; typedef int64_t NixInt; typedef double NixFloat; +typedef std::pair NixStringContextElem; +typedef std::vector NixStringContext; /* External values must descend from ExternalValueBase, so that * type-agnostic nix functions (e.g. showType) can be implemented @@ -368,7 +370,7 @@ public: non-trivial. */ bool isTrivial() const; - std::vector> getContext(); + NixStringContext getContext(); auto listItems() { From d60f3cf6e9c904912199ea64156fea295494430a Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Thu, 17 Mar 2022 22:59:43 +0100 Subject: [PATCH 06/98] nix-daemon.conf.in: add tmpfiles file to create nix/daemon-socket directory nix-daemon.socket is used to socket-activate nix-daemon.service when /nix/var/nix/daemon-socket/socket is accessed. In container usecases, sometimes /nix/var/nix/daemon-socket is bind-mounted read-only into the container. In these cases, we want to skip starting nix-daemon.socket. However, since systemd 250, `ConditionPathIsReadWrite` is also not met if /nix/var/nix/daemon-socket doesn't exist at all. This means, a regular NixOS system will skip starting nix-daemon.socket: > [ 237.187747] systemd[1]: Nix Daemon Socket was skipped because of a failed condition check (ConditionPathIsReadWrite=/nix/var/nix/daemon-socket). To prevent this from happening, ship a tmpfiles file that'll cause the directory to be created if it doesn't exist already. In the case of NixOS, we can just add Nix to `systemd.tmpfiles.packages` and have these files picked up automatically. --- misc/systemd/local.mk | 3 ++- misc/systemd/nix-daemon.conf.in | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 misc/systemd/nix-daemon.conf.in diff --git a/misc/systemd/local.mk b/misc/systemd/local.mk index 1fa037485..76121a0f9 100644 --- a/misc/systemd/local.mk +++ b/misc/systemd/local.mk @@ -1,7 +1,8 @@ ifdef HOST_LINUX $(foreach n, nix-daemon.socket nix-daemon.service, $(eval $(call install-file-in, $(d)/$(n), $(prefix)/lib/systemd/system, 0644))) + $(foreach n, nix-daemon.conf, $(eval $(call install-file-in, $(d)/$(n), $(prefix)/lib/tmpfiles.d, 0644))) - clean-files += $(d)/nix-daemon.socket $(d)/nix-daemon.service + clean-files += $(d)/nix-daemon.socket $(d)/nix-daemon.service $(d)/nix-daemon.conf endif diff --git a/misc/systemd/nix-daemon.conf.in b/misc/systemd/nix-daemon.conf.in new file mode 100644 index 000000000..e7b264234 --- /dev/null +++ b/misc/systemd/nix-daemon.conf.in @@ -0,0 +1 @@ +d @localstatedir@/nix/daemon-socket 0755 root root - - From 4d6a3806d24b54f06ddc0cf234ac993db028cf29 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 12 Mar 2022 00:28:00 +0000 Subject: [PATCH 07/98] Decode string context straight to using `StorePath`s I gather decoding happens on demand, so I hope don't think this should have any perf implications one way or the other. --- src/libexpr/eval-cache.cc | 19 +++++++++++-------- src/libexpr/eval.cc | 19 ++++++++++++++----- src/libexpr/eval.hh | 4 ++-- src/libexpr/primops.cc | 8 ++++---- src/libexpr/primops/context.cc | 4 ++-- src/libexpr/value.hh | 6 ++++-- src/nix/app.cc | 2 +- 7 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 7e5b5c9c4..54fa9b741 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -21,6 +21,8 @@ struct AttrDb { std::atomic_bool failed{false}; + const Store & cfg; + struct State { SQLite db; @@ -33,8 +35,9 @@ struct AttrDb std::unique_ptr> _state; - AttrDb(const Hash & fingerprint) - : _state(std::make_unique>()) + AttrDb(const Store & cfg, const Hash & fingerprint) + : cfg(cfg) + , _state(std::make_unique>()) { auto state(_state->lock()); @@ -257,7 +260,7 @@ struct AttrDb NixStringContext context; if (!queryAttribute.isNull(3)) for (auto & s : tokenizeString>(queryAttribute.getStr(3), ";")) - context.push_back(decodeContext(s)); + context.push_back(decodeContext(cfg, s)); return {{rowId, string_t{queryAttribute.getStr(2), context}}}; } case AttrType::Bool: @@ -274,10 +277,10 @@ struct AttrDb } }; -static std::shared_ptr makeAttrDb(const Hash & fingerprint) +static std::shared_ptr makeAttrDb(const Store & cfg, const Hash & fingerprint) { try { - return std::make_shared(fingerprint); + return std::make_shared(cfg, fingerprint); } catch (SQLiteError &) { ignoreException(); return nullptr; @@ -288,7 +291,7 @@ EvalCache::EvalCache( std::optional> useCache, EvalState & state, RootLoader rootLoader) - : db(useCache ? makeAttrDb(*useCache) : nullptr) + : db(useCache ? makeAttrDb(*state.store, *useCache) : nullptr) , state(state) , rootLoader(rootLoader) { @@ -546,7 +549,7 @@ string_t AttrCursor::getStringWithContext() if (auto s = std::get_if(&cachedValue->second)) { bool valid = true; for (auto & c : s->second) { - if (!root->state.store->isValidPath(root->state.store->parseStorePath(c.first))) { + if (!root->state.store->isValidPath(c.first)) { valid = false; break; } @@ -563,7 +566,7 @@ string_t AttrCursor::getStringWithContext() auto & v = forceValue(); if (v.type() == nString) - return {v.string.s, v.getContext()}; + return {v.string.s, v.getContext(*root->state.store)}; else if (v.type() == nPath) return {v.path, {}}; else diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 126e0af8c..f7911e32b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1903,13 +1903,22 @@ std::string_view EvalState::forceString(Value & v, const Pos & pos) /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s) +NixStringContextElem decodeContext(const Store & store, std::string_view s) { if (s.at(0) == '!') { size_t index = s.find("!", 1); - return {std::string(s.substr(index + 1)), std::string(s.substr(1, index - 1))}; + return { + store.parseStorePath(s.substr(index + 1)), + std::string(s.substr(1, index - 1)), + }; } else - return {s.at(0) == '/' ? std::string(s) : std::string(s.substr(1)), ""}; + return { + store.parseStorePath( + s.at(0) == '/' + ? s + : s.substr(1)), + "", + }; } @@ -1921,13 +1930,13 @@ void copyContext(const Value & v, PathSet & context) } -NixStringContext Value::getContext() +NixStringContext Value::getContext(const Store & store) { NixStringContext res; assert(internalType == tString); if (string.context) for (const char * * p = string.context; *p; ++p) - res.push_back(decodeContext(*p)); + res.push_back(decodeContext(store, *p)); return res; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 18480f290..198a62ad2 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -17,7 +17,7 @@ namespace nix { -class Store; +struct Store; class EvalState; class StorePath; enum RepairFlag : bool; @@ -430,7 +430,7 @@ std::string showType(const Value & v); /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s); +NixStringContextElem decodeContext(const Store & store, std::string_view s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3124025aa..566cbdfdb 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -43,8 +43,8 @@ StringMap EvalState::realiseContext(const PathSet & context) StringMap res; for (auto & i : context) { - auto [ctxS, outputName] = decodeContext(i); - auto ctx = store->parseStorePath(ctxS); + auto [ctx, outputName] = decodeContext(*store, i); + auto ctxS = store->printStorePath(ctx); if (!store->isValidPath(ctx)) throw InvalidPathError(store->printStorePath(ctx)); if (!outputName.empty() && ctx.isDerivation()) { @@ -1118,8 +1118,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Handle derivation outputs of the form ‘!!’. */ else if (path.at(0) == '!') { - auto ctx = decodeContext(path); - drv.inputDrvs[state.store->parseStorePath(ctx.first)].insert(ctx.second); + auto ctx = decodeContext(*state.store, path); + drv.inputDrvs[ctx.first].insert(ctx.second); } /* Otherwise it's a source file. */ diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index ad4de3840..6ec5e4a74 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -82,8 +82,8 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, drv = std::string(p, 1); path = &drv; } else if (p.at(0) == '!') { - NixStringContextElem ctx = decodeContext(p); - drv = ctx.first; + NixStringContextElem ctx = decodeContext(*state.store, p); + drv = state.store->printStorePath(ctx.first); output = ctx.second; path = &drv; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index ee9cb832a..d413060f9 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -57,6 +57,8 @@ struct ExprLambda; struct PrimOp; class Symbol; struct Pos; +class StorePath; +class Store; class EvalState; class XMLWriter; class JSONPlaceholder; @@ -64,7 +66,7 @@ class JSONPlaceholder; typedef int64_t NixInt; typedef double NixFloat; -typedef std::pair NixStringContextElem; +typedef std::pair NixStringContextElem; typedef std::vector NixStringContext; /* External values must descend from ExternalValueBase, so that @@ -370,7 +372,7 @@ public: non-trivial. */ bool isTrivial() const; - NixStringContext getContext(); + NixStringContext getContext(const Store &); auto listItems() { diff --git a/src/nix/app.cc b/src/nix/app.cc index 2563180fb..a4e0df06a 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -70,7 +70,7 @@ UnresolvedApp Installable::toApp(EvalState & state) std::vector context2; for (auto & [path, name] : context) - context2.push_back({state.store->parseStorePath(path), {name}}); + context2.push_back({path, {name}}); return UnresolvedApp{App { .context = std::move(context2), From 345a8ee0cb56775c57c3b7da99b0726290241e18 Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 10:56:13 -0300 Subject: [PATCH 08/98] Fix sourcehut tag ref resolving --- src/libfetchers/github.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index a1430f087..d52ea649d 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -390,7 +390,7 @@ struct SourceHutInputScheme : GitArchiveInputScheme ref_uri = line.substr(ref_index+5, line.length()-1); } else - ref_uri = fmt("refs/heads/%s", ref); + ref_uri = fmt("refs/(heads|tags)/%s", ref); auto file = store->toRealPath( downloadFile(store, fmt("%s/info/refs", base_url), "source", false, headers).storePath); @@ -399,9 +399,9 @@ struct SourceHutInputScheme : GitArchiveInputScheme std::string line; std::string id; while(getline(is, line)) { - auto index = line.find(ref_uri); - if (index != std::string::npos) { - id = line.substr(0, index-1); + std::regex pattern(ref_uri); + if (std::regex_search(line, pattern)) { + id = line.substr(0, line.find('\t')); break; } } From 9720797f695f7646744e9cd020fa79679441814f Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 11:04:04 -0300 Subject: [PATCH 09/98] Don't partial match sourcehut refs --- src/libfetchers/github.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index d52ea649d..58b6e7c04 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -399,7 +399,9 @@ struct SourceHutInputScheme : GitArchiveInputScheme std::string line; std::string id; while(getline(is, line)) { - std::regex pattern(ref_uri); + // Append $ to avoid partial name matches + std::regex pattern(fmt("%s$", ref_uri)); + if (std::regex_search(line, pattern)) { id = line.substr(0, line.find('\t')); break; From 31544b93ffa985661541309ec2360dfb7b527a80 Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 11:38:45 -0300 Subject: [PATCH 10/98] Fix sourcehut integration test The new implementation relies on tab separting the hash and ref (this is how sourcehut does it). This fixes the integration test to use a tab instead of a space. --- tests/sourcehut-flakes.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sourcehut-flakes.nix b/tests/sourcehut-flakes.nix index d1d89d149..6a1930904 100644 --- a/tests/sourcehut-flakes.nix +++ b/tests/sourcehut-flakes.nix @@ -59,7 +59,7 @@ let echo 'ref: refs/heads/master' > $out/HEAD mkdir -p $out/info - echo '${nixpkgs.rev} refs/heads/master' > $out/info/refs + echo -e '${nixpkgs.rev}\trefs/heads/master' > $out/info/refs ''; in From 0b42afe02794a446d20066c0e9931b9c759d36e2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 10:48:02 +0100 Subject: [PATCH 11/98] buildProfile(): Ignore manifest.{nix,json} If a package installs a file named manifest.json, it caused nix-env to consider the profile a new-style profile created by 'nix profile'. Fixes #6032. --- src/libstore/builtins/buildenv.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 25d015cb9..6f6ad57cb 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -47,9 +47,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw; } - /* The files below are special-cased to that they don't show up - * in user profiles, either because they are useless, or - * because they would cauase pointless collisions (e.g., each + /* The files below are special-cased to that they don't show + * up in user profiles, either because they are useless, or + * because they would cause pointless collisions (e.g., each * Python package brings its own * `$out/lib/pythonX.Y/site-packages/easy-install.pth'.) */ @@ -57,7 +57,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, hasSuffix(srcFile, "/nix-support") || hasSuffix(srcFile, "/perllocal.pod") || hasSuffix(srcFile, "/info/dir") || - hasSuffix(srcFile, "/log")) + hasSuffix(srcFile, "/log") || + hasSuffix(srcFile, "/manifest.nix") || + hasSuffix(srcFile, "/manifest.json")) continue; else if (S_ISDIR(srcSt.st_mode)) { From 732296ddc078f9cce8ffeb3131fef8898330b6ae Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 13:00:27 +0100 Subject: [PATCH 12/98] =?UTF-8?q?printValue():=20=20->=20=C2=ABrep?= =?UTF-8?q?eated=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures that it doesn't get parsed as a valid Nix expression. --- src/libexpr/eval.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f7911e32b..333ad68eb 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -126,7 +126,7 @@ void printValue(std::ostream & str, std::set & seen, const Value & break; case tAttrs: { if (!v.attrs->empty() && !seen.insert(v.attrs).second) - str << ""; + str << "«repeated»"; else { str << "{ "; for (auto & i : v.attrs->lexicographicOrder()) { @@ -142,7 +142,7 @@ void printValue(std::ostream & str, std::set & seen, const Value & case tList2: case tListN: if (v.listSize() && !seen.insert(v.listElems()).second) - str << ""; + str << "«repeated»"; else { str << "[ "; for (auto v2 : v.listItems()) { From a0259a21a416301573d9d6b994d6b5fb62a1bcf3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 13:18:11 +0100 Subject: [PATCH 13/98] Don't hide repeated values while generating manifest.nix Fixes #6243. --- src/libexpr/eval.cc | 38 ++++++++++++++++++++++---------------- src/libexpr/value.hh | 5 ++++- src/nix-env/user-env.cc | 4 +++- tests/user-envs.nix | 3 +++ 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 333ad68eb..3ec5ed202 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -96,20 +96,20 @@ RootValue allocRootValue(Value * v) } -void printValue(std::ostream & str, std::set & seen, const Value & v) +void Value::print(std::ostream & str, std::set * seen) const { checkInterrupt(); - switch (v.internalType) { + switch (internalType) { case tInt: - str << v.integer; + str << integer; break; case tBool: - str << (v.boolean ? "true" : "false"); + str << (boolean ? "true" : "false"); break; case tString: str << "\""; - for (const char * i = v.string.s; *i; i++) + for (const char * i = string.s; *i; i++) if (*i == '\"' || *i == '\\') str << "\\" << *i; else if (*i == '\n') str << "\\n"; else if (*i == '\r') str << "\\r"; @@ -119,19 +119,19 @@ void printValue(std::ostream & str, std::set & seen, const Value & str << "\""; break; case tPath: - str << v.path; // !!! escaping? + str << path; // !!! escaping? break; case tNull: str << "null"; break; case tAttrs: { - if (!v.attrs->empty() && !seen.insert(v.attrs).second) + if (seen && !attrs->empty() && !seen->insert(attrs).second) str << "«repeated»"; else { str << "{ "; - for (auto & i : v.attrs->lexicographicOrder()) { + for (auto & i : attrs->lexicographicOrder()) { str << i->name << " = "; - printValue(str, seen, *i->value); + i->value->print(str, seen); str << "; "; } str << "}"; @@ -141,12 +141,12 @@ void printValue(std::ostream & str, std::set & seen, const Value & case tList1: case tList2: case tListN: - if (v.listSize() && !seen.insert(v.listElems()).second) + if (seen && listSize() && !seen->insert(listElems()).second) str << "«repeated»"; else { str << "[ "; - for (auto v2 : v.listItems()) { - printValue(str, seen, *v2); + for (auto v2 : listItems()) { + v2->print(str, seen); str << " "; } str << "]"; @@ -166,10 +166,10 @@ void printValue(std::ostream & str, std::set & seen, const Value & str << ""; break; case tExternal: - str << *v.external; + str << *external; break; case tFloat: - str << v.fpoint; + str << fpoint; break; default: abort(); @@ -177,10 +177,16 @@ void printValue(std::ostream & str, std::set & seen, const Value & } -std::ostream & operator << (std::ostream & str, const Value & v) +void Value::print(std::ostream & str, bool showRepeated) const { std::set seen; - printValue(str, seen, v); + print(str, showRepeated ? nullptr : &seen); +} + + +std::ostream & operator << (std::ostream & str, const Value & v) +{ + v.print(str, false); return str; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index d413060f9..3d07c3198 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -119,10 +119,13 @@ private: InternalType internalType; friend std::string showType(const Value & v); - friend void printValue(std::ostream & str, std::set & seen, const Value & v); + + void print(std::ostream & str, std::set * seen) const; public: + void print(std::ostream & str, bool showRepeated = false) const; + // Functions needed to distinguish the type // These should be removed eventually, by putting the functionality that's // needed by callers into methods of this type diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index bcc7736ac..78692b9c6 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -105,8 +105,10 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Also write a copy of the list of user environment elements to the store; we need it for future modifications of the environment. */ + std::ostringstream str; + manifest.print(str, true); auto manifestFile = state.store->addTextToStore("env-manifest.nix", - fmt("%s", manifest), references); + str.str(), references); /* Get the environment builder expression. */ Value envBuilder; diff --git a/tests/user-envs.nix b/tests/user-envs.nix index 6ac896ed8..46f8b51dd 100644 --- a/tests/user-envs.nix +++ b/tests/user-envs.nix @@ -8,6 +8,8 @@ assert foo == "foo"; let + platforms = let x = "foobar"; in [ x x ]; + makeDrv = name: progName: (mkDerivation { name = assert progName != "fail"; name; inherit progName system; @@ -15,6 +17,7 @@ let } // { meta = { description = "A silly test package with some \${escaped anti-quotation} in it"; + inherit platforms; }; }); From 3b776cb0a7574bf11c20fe0a1d91ec1d21d8ebeb Mon Sep 17 00:00:00 2001 From: Hideaki Kawai Date: Tue, 22 Mar 2022 23:18:02 +0900 Subject: [PATCH 14/98] nix edit: support kakoune --- src/libcmd/command.cc | 3 ++- src/nix/edit.md | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index dc8fa9e5a..a53b029b7 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -204,7 +204,8 @@ Strings editorFor(const Pos & pos) if (pos.line > 0 && ( editor.find("emacs") != std::string::npos || editor.find("nano") != std::string::npos || - editor.find("vim") != std::string::npos)) + editor.find("vim") != std::string::npos || + editor.find("kak") != std::string::npos)) args.push_back(fmt("+%d", pos.line)); args.push_back(pos.file); return args; diff --git a/src/nix/edit.md b/src/nix/edit.md index 80563d06b..89bd09abf 100644 --- a/src/nix/edit.md +++ b/src/nix/edit.md @@ -24,8 +24,8 @@ this attribute to the location of the definition of the `meta.description`, `version` or `name` derivation attributes. The editor to invoke is specified by the `EDITOR` environment -variable. It defaults to `cat`. If the editor is `emacs`, `nano` or -`vim`, it is passed the line number of the derivation using the -argument `+`. +variable. It defaults to `cat`. If the editor is `emacs`, `nano`, +`vim` or `kak`, it is passed the line number of the derivation using +the argument `+`. )"" From 67af5f7eda1dade544cc15397a7d0d35d0913cb2 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 22 Mar 2022 17:06:09 +0100 Subject: [PATCH 15/98] scripts/install-systemd-multi-user.sh: install /etc/tmpfiles.d/nix-daemon.conf, too While `create_directories()` from install-multi-user.sh seems to already create parts of the directory structure, it's marked as deprecated, and it won't hurt also copying over the tmpfiles config and have it execute once. --- scripts/install-systemd-multi-user.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index f4a2dfc5d..24884a023 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -9,6 +9,8 @@ readonly SERVICE_DEST=/etc/systemd/system/nix-daemon.service readonly SOCKET_SRC=/lib/systemd/system/nix-daemon.socket readonly SOCKET_DEST=/etc/systemd/system/nix-daemon.socket +readonly TMPFILES_SRC=/lib/tmpfiles.d/nix-daemon.conf +readonly TMPFILES_DEST=/etc/tmpfiles.d/nix-daemon.conf # Path for the systemd override unit file to contain the proxy settings readonly SERVICE_OVERRIDE=${SERVICE_DEST}.d/override.conf @@ -83,6 +85,13 @@ EOF poly_configure_nix_daemon_service() { if [ -e /run/systemd/system ]; then task "Setting up the nix-daemon systemd service" + + _sudo "to create the nix-daemon tmpfiles config" \ + ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST + + _sudo "to run systemd-tmpfiles once to pick that path up" \ + sytemd-tmpfiles create --prefix=/nix/var/nix + _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From 9174d884d750b7b49a571bd55275f0883c2dabda Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Thu, 24 Mar 2022 08:10:33 +0000 Subject: [PATCH 16/98] lexer: add error location to lexer errors Before the change lexter errors did not report the location: $ nix build -f. mc error: path has a trailing slash (use '--show-trace' to show detailed location information) Note that it's not clear what file generates the error. After the change location is reported: $ src/nix/nix --extra-experimental-features nix-command build -f ~/nm mc error: path has a trailing slash at .../pkgs/development/libraries/glib/default.nix:54:18: 53| }; 54| src = /tmp/foo/; | ^ 55| (use '--show-trace' to show detailed location information) Here we see both problematic file and the string itself. --- src/libexpr/lexer.l | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index e276b0467..d574121b0 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -28,6 +28,13 @@ using namespace nix; namespace nix { +static inline Pos makeCurPos(const YYLTYPE & loc, ParseData * data) +{ + return Pos(data->origin, data->file, loc.first_line, loc.first_column); +} + +#define CUR_POS makeCurPos(*yylloc, data) + // backup to recover from yyless(0) YYLTYPE prev_yylloc; @@ -37,7 +44,6 @@ static void initLoc(YYLTYPE * loc) loc->first_column = loc->last_column = 1; } - static void adjustLoc(YYLTYPE * loc, const char * s, size_t len) { prev_yylloc = *loc; @@ -147,14 +153,20 @@ or { return OR_KW; } try { yylval->n = boost::lexical_cast(yytext); } catch (const boost::bad_lexical_cast &) { - throw ParseError("invalid integer '%1%'", yytext); + throw ParseError({ + .msg = hintfmt("invalid integer '%1%'", yytext), + .errPos = CUR_POS, + }); } return INT; } {FLOAT} { errno = 0; yylval->nf = strtod(yytext, 0); if (errno != 0) - throw ParseError("invalid float '%1%'", yytext); + throw ParseError({ + .msg = hintfmt("invalid float '%1%'", yytext), + .errPos = CUR_POS, + }); return FLOAT; } @@ -280,7 +292,10 @@ or { return OR_KW; } {ANY} | <> { - throw ParseError("path has a trailing slash"); + throw ParseError({ + .msg = hintfmt("path has a trailing slash"), + .errPos = CUR_POS, + }); } {SPATH} { yylval->path = {yytext, (size_t) yyleng}; return SPATH; } From 4546a007a46fe12b77a49ae8753ab186d0b55fdd Mon Sep 17 00:00:00 2001 From: Rok Garbas Date: Thu, 24 Mar 2022 12:28:38 +0100 Subject: [PATCH 17/98] Fix flake profile use of originalUrl vs. originalUri Fixes #5872 --- src/nix/profile.cc | 5 +++-- src/nix/profile.md | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index a8ff9c78a..da990ddc8 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -107,8 +107,9 @@ struct ProfileManifest element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; if (e.value("uri", "") != "") { + auto originalUrl = e.value("originalUrl", e["originalUri"]); element.source = ProfileElementSource{ - parseFlakeRef(e["originalUri"]), + parseFlakeRef(originalUrl), parseFlakeRef(e["uri"]), e["attrPath"] }; @@ -143,7 +144,7 @@ struct ProfileManifest obj["storePaths"] = paths; obj["active"] = element.active; if (element.source) { - obj["originalUri"] = element.source->originalRef.to_string(); + obj["originalUrl"] = element.source->originalRef.to_string(); obj["uri"] = element.source->resolvedRef.to_string(); obj["attrPath"] = element.source->attrPath; } diff --git a/src/nix/profile.md b/src/nix/profile.md index 0a4ff2fa9..8dade051d 100644 --- a/src/nix/profile.md +++ b/src/nix/profile.md @@ -70,7 +70,7 @@ are installed in this version of the profile. It looks like this: { "active": true, "attrPath": "legacyPackages.x86_64-linux.zoom-us", - "originalUri": "flake:nixpkgs", + "originalUrl": "flake:nixpkgs", "storePaths": [ "/nix/store/wbhg2ga8f3h87s9h5k0slxk0m81m4cxl-zoom-us-5.3.469451.0927" ], @@ -84,11 +84,11 @@ are installed in this version of the profile. It looks like this: Each object in the array `elements` denotes an installed package and has the following fields: -* `originalUri`: The [flake reference](./nix3-flake.md) specified by +* `originalUrl`: The [flake reference](./nix3-flake.md) specified by the user at the time of installation (e.g. `nixpkgs`). This is also the flake reference that will be used by `nix profile upgrade`. -* `uri`: The immutable flake reference to which `originalUri` +* `uri`: The immutable flake reference to which `originalUrl` resolved. * `attrPath`: The flake output attribute that provided this From bb0c4b9f25b6d064d91aedd71940f14b1b624291 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 12:46:46 +0100 Subject: [PATCH 18/98] install-multi-user.sh: Preserve symlinks We need to pass -P to ensure that symlinks are copied correctly. Fixes #6303. --- 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 5f2c93b7f..69b6676ea 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -739,7 +739,7 @@ install_from_extracted_nix() { cd "$EXTRACTED_NIX_PATH" _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ - cp -RLp ./store/* "$NIX_ROOT/store/" + cp -RPp ./store/* "$NIX_ROOT/store/" _sudo "to make the new store non-writable at $NIX_ROOT/store" \ chmod -R ugo-w "$NIX_ROOT/store/" From 0736f3651d56d3e0c3b742fe0d90958c90e9c968 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 23 Mar 2022 22:54:43 -0400 Subject: [PATCH 19/98] docs: genericClosure --- src/libexpr/primops.cc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index bfa18f898..e7e224b3a 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -694,7 +694,32 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar static RegisterPrimOp primop_genericClosure(RegisterPrimOp::Info { .name = "__genericClosure", + .args = {"attrset"}, .arity = 1, + .doc = R"( + Take an *attrset* with values named `startSet` and `operator` in order to + return a *list of attrsets* by starting with the `startSet`, recursively + applying the `operator` function to each element. The *attrsets* in the + `startSet` and produced by the `operator` must each contain value named + `key` which are comparable to each other. The result is produced by + repeatedly calling the operator for each element encountered with a + unique key, terminating when no new elements are produced. For example, + + ``` + builtins.genericClosure { + startSet = [ {key = 5;} ]; + operator = item: [{ + key = if (item.key / 2 ) * 2 == item.key + then item.key / 2 + else 3 * item.key + 1; + }]; + } + ``` + evaluates to + ``` + [ { key = 5; } { key = 16; } { key = 8; } { key = 4; } { key = 2; } { key = 1; } ] + ``` + )", .fun = prim_genericClosure, }); From f4bafc412fac79ce07c89f8d3ab9bd1c32f7b9cd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 11:31:01 +0100 Subject: [PATCH 20/98] Add builtins.fetchClosure This allows closures to be imported at evaluation time, without requiring the user to configure substituters. E.g. builtins.fetchClosure { storePath = /nix/store/f89g6yi63m1ywfxj96whv5sxsm74w5ka-python3.9-sqlparse-0.4.2; from = "https://cache.ngi0.nixos.org"; } --- src/libexpr/primops/fetchClosure.cc | 62 +++++++++++++++++++++++++++++ src/libexpr/primops/fetchTree.cc | 4 +- 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 src/libexpr/primops/fetchClosure.cc diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc new file mode 100644 index 000000000..4bcc716db --- /dev/null +++ b/src/libexpr/primops/fetchClosure.cc @@ -0,0 +1,62 @@ +#include "primops.hh" +#include "store-api.hh" + +namespace nix { + +static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + state.forceAttrs(*args[0], pos); + + std::optional storePath; + std::optional from; + + for (auto & attr : *args[0]->attrs) { + if (attr.name == "storePath") { + PathSet context; + storePath = state.coerceToStorePath(*attr.pos, *attr.value, context); + } + + else if (attr.name == "from") + from = state.forceStringNoCtx(*attr.value, *attr.pos); + + else + throw Error({ + .msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attr.name), + .errPos = pos + }); + } + + if (!storePath) + throw Error({ + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "storePath"), + .errPos = pos + }); + + if (!from) + throw Error({ + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "from"), + .errPos = pos + }); + + // FIXME: only allow some "trusted" store types (like BinaryCacheStore). + auto srcStore = openStore(*from); + + copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + + auto storePathS = state.store->printStorePath(*storePath); + + v.mkString(storePathS, {storePathS}); + + // FIXME: in pure mode, require a CA path or a NAR hash of the + // top-level path. +} + +static RegisterPrimOp primop_fetchClosure({ + .name = "__fetchClosure", + .args = {"args"}, + .doc = R"( + )", + .fun = prim_fetchClosure, +}); + +} diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 9c2da2178..42c98e312 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -145,7 +145,7 @@ static void fetchTree( if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) throw Error({ - .msg = hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), + .msg = hintfmt("attribute 'name' isn't supported in call to 'fetchTree'"), .errPos = pos }); @@ -329,7 +329,7 @@ static RegisterPrimOp primop_fetchTarball({ .fun = prim_fetchTarball, }); -static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) +static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Value & v) { fetchTree(state, pos, args, v, "git", FetchTreeParams { .emptyRevFallback = true, .allowNameArgument = true }); } From 41659418cfeb7a33fc76716a1847f79ae13d9b0c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 14:00:54 +0100 Subject: [PATCH 21/98] fetchClosure: Require a CA path in pure mode --- src/libexpr/primops/fetchClosure.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 4bcc716db..22a4649a4 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -43,12 +43,19 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + /* In pure mode, require a CA path. */ + if (evalSettings.pureEval) { + auto info = state.store->queryPathInfo(*storePath); + if (!info->isContentAddressed(*state.store)) + throw Error({ + .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", + state.store->printStorePath(*storePath)), + .errPos = pos + }); + } + auto storePathS = state.store->printStorePath(*storePath); - v.mkString(storePathS, {storePathS}); - - // FIXME: in pure mode, require a CA path or a NAR hash of the - // top-level path. } static RegisterPrimOp primop_fetchClosure({ From 7f6fe8ca1d41bceef32790aa0313aa62ae2b65fb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 14:34:45 +0100 Subject: [PATCH 22/98] Rename --- src/libexpr/primops/fetchClosure.cc | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 22a4649a4..8e60b2ccc 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,17 +7,17 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args { state.forceAttrs(*args[0], pos); - std::optional storePath; - std::optional from; + std::optional fromStoreUrl; + std::optional fromPath; for (auto & attr : *args[0]->attrs) { - if (attr.name == "storePath") { + if (attr.name == "fromPath") { PathSet context; - storePath = state.coerceToStorePath(*attr.pos, *attr.value, context); + fromPath = state.coerceToStorePath(*attr.pos, *attr.value, context); } - else if (attr.name == "from") - from = state.forceStringNoCtx(*attr.value, *attr.pos); + else if (attr.name == "fromStore") + fromStoreUrl = state.forceStringNoCtx(*attr.value, *attr.pos); else throw Error({ @@ -26,36 +26,36 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args }); } - if (!storePath) + if (!fromPath) throw Error({ - .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "storePath"), + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromPath"), .errPos = pos }); - if (!from) + if (!fromStoreUrl) throw Error({ - .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "from"), + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"), .errPos = pos }); // FIXME: only allow some "trusted" store types (like BinaryCacheStore). - auto srcStore = openStore(*from); + auto fromStore = openStore(*fromStoreUrl); - copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); /* In pure mode, require a CA path. */ if (evalSettings.pureEval) { - auto info = state.store->queryPathInfo(*storePath); + auto info = state.store->queryPathInfo(*fromPath); if (!info->isContentAddressed(*state.store)) throw Error({ .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", - state.store->printStorePath(*storePath)), + state.store->printStorePath(*fromPath)), .errPos = pos }); } - auto storePathS = state.store->printStorePath(*storePath); - v.mkString(storePathS, {storePathS}); + auto fromPathS = state.store->printStorePath(*fromPath); + v.mkString(fromPathS, {fromPathS}); } static RegisterPrimOp primop_fetchClosure({ From 545c2d0d8cbac86c169a6dd049c1ed9c3913774d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:14:58 +0100 Subject: [PATCH 23/98] fetchClosure: Allow a path to be rewritten to CA on the fly The advantage is that the resulting closure doesn't need to be signed, so you don't need to configure any binary cache keys on the client. --- src/libexpr/primops/fetchClosure.cc | 46 ++++++++++++-- src/libstore/make-content-addressed.cc | 79 ++++++++++++++++++++++++ src/libstore/make-content-addressed.hh | 12 ++++ src/nix/make-content-addressable.cc | 85 ++++++-------------------- 4 files changed, 150 insertions(+), 72 deletions(-) create mode 100644 src/libstore/make-content-addressed.cc create mode 100644 src/libstore/make-content-addressed.hh diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 8e60b2ccc..56fd53ed5 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -1,5 +1,6 @@ #include "primops.hh" #include "store-api.hh" +#include "make-content-addressed.hh" namespace nix { @@ -9,6 +10,8 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args std::optional fromStoreUrl; std::optional fromPath; + bool toCA = false; + std::optional toPath; for (auto & attr : *args[0]->attrs) { if (attr.name == "fromPath") { @@ -16,6 +19,15 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args fromPath = state.coerceToStorePath(*attr.pos, *attr.value, context); } + else if (attr.name == "toPath") { + state.forceValue(*attr.value, *attr.pos); + toCA = true; + if (attr.value->type() != nString || attr.value->string.s != std::string("")) { + PathSet context; + toPath = state.coerceToStorePath(*attr.pos, *attr.value, context); + } + } + else if (attr.name == "fromStore") fromStoreUrl = state.forceStringNoCtx(*attr.value, *attr.pos); @@ -41,21 +53,45 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args // FIXME: only allow some "trusted" store types (like BinaryCacheStore). auto fromStore = openStore(*fromStoreUrl); - copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + if (toCA) { + auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); + auto i = remappings.find(*fromPath); + assert(i != remappings.end()); + if (toPath && *toPath != i->second) + throw Error({ + .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second), + state.store->printStorePath(*toPath)), + .errPos = pos + }); + if (!toPath) + throw Error({ + .msg = hintfmt( + "rewriting '%s' to content-addressed form yielded '%s'; " + "please set this in the 'toPath' attribute passed to 'fetchClosure'", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second)), + .errPos = pos + }); + } else { + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + toPath = fromPath; + } /* In pure mode, require a CA path. */ if (evalSettings.pureEval) { - auto info = state.store->queryPathInfo(*fromPath); + auto info = state.store->queryPathInfo(*toPath); if (!info->isContentAddressed(*state.store)) throw Error({ .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", - state.store->printStorePath(*fromPath)), + state.store->printStorePath(*toPath)), .errPos = pos }); } - auto fromPathS = state.store->printStorePath(*fromPath); - v.mkString(fromPathS, {fromPathS}); + auto toPathS = state.store->printStorePath(*toPath); + v.mkString(toPathS, {toPathS}); } static RegisterPrimOp primop_fetchClosure({ diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc new file mode 100644 index 000000000..0b95ff37c --- /dev/null +++ b/src/libstore/make-content-addressed.cc @@ -0,0 +1,79 @@ +#include "make-content-addressed.hh" +#include "references.hh" + +namespace nix { + +std::map makeContentAddressed( + Store & srcStore, + Store & dstStore, + const StorePathSet & storePaths) +{ + // FIXME: use closure of storePaths. + + auto paths = srcStore.topoSortPaths(storePaths); + + std::reverse(paths.begin(), paths.end()); + + std::map remappings; + + for (auto & path : paths) { + auto pathS = srcStore.printStorePath(path); + auto oldInfo = srcStore.queryPathInfo(path); + std::string oldHashPart(path.hashPart()); + + StringSink sink; + srcStore.narFromPath(path, sink); + + StringMap rewrites; + + StorePathSet references; + bool hasSelfReference = false; + for (auto & ref : oldInfo->references) { + if (ref == path) + hasSelfReference = true; + else { + auto i = remappings.find(ref); + auto replacement = i != remappings.end() ? i->second : ref; + // FIXME: warn about unremapped paths? + if (replacement != ref) + rewrites.insert_or_assign(srcStore.printStorePath(ref), srcStore.printStorePath(replacement)); + references.insert(std::move(replacement)); + } + } + + sink.s = rewriteStrings(sink.s, rewrites); + + HashModuloSink hashModuloSink(htSHA256, oldHashPart); + hashModuloSink(sink.s); + + auto narHash = hashModuloSink.finish().first; + + ValidPathInfo info { + dstStore.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), + narHash, + }; + info.references = std::move(references); + if (hasSelfReference) info.references.insert(info.path); + info.narSize = sink.s.size(); + info.ca = FixedOutputHash { + .method = FileIngestionMethod::Recursive, + .hash = info.narHash, + }; + + printInfo("rewrote '%s' to '%s'", pathS, srcStore.printStorePath(info.path)); + + auto source = sinkToSource([&](Sink & nextSink) { + RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); + rsink2(sink.s); + rsink2.flush(); + }); + + dstStore.addToStore(info, *source); + + remappings.insert_or_assign(std::move(path), std::move(info.path)); + } + + return remappings; +} + +} diff --git a/src/libstore/make-content-addressed.hh b/src/libstore/make-content-addressed.hh new file mode 100644 index 000000000..c4a66ed41 --- /dev/null +++ b/src/libstore/make-content-addressed.hh @@ -0,0 +1,12 @@ +#pragma once + +#include "store-api.hh" + +namespace nix { + +std::map makeContentAddressed( + Store & srcStore, + Store & dstStore, + const StorePathSet & storePaths); + +} diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 2e75a3b61..a8579ea7c 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -1,6 +1,6 @@ #include "command.hh" #include "store-api.hh" -#include "references.hh" +#include "make-content-addressed.hh" #include "common-args.hh" #include "json.hh" @@ -27,74 +27,25 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON void run(ref store, StorePaths && storePaths) override { - auto paths = store->topoSortPaths(StorePathSet(storePaths.begin(), storePaths.end())); + auto remappings = makeContentAddressed(*store, *store, + StorePathSet(storePaths.begin(), storePaths.end())); - std::reverse(paths.begin(), paths.end()); - - std::map remappings; - - auto jsonRoot = json ? std::make_unique(std::cout) : nullptr; - auto jsonRewrites = json ? std::make_unique(jsonRoot->object("rewrites")) : nullptr; - - for (auto & path : paths) { - auto pathS = store->printStorePath(path); - auto oldInfo = store->queryPathInfo(path); - std::string oldHashPart(path.hashPart()); - - StringSink sink; - store->narFromPath(path, sink); - - StringMap rewrites; - - StorePathSet references; - bool hasSelfReference = false; - for (auto & ref : oldInfo->references) { - if (ref == path) - hasSelfReference = true; - else { - auto i = remappings.find(ref); - auto replacement = i != remappings.end() ? i->second : ref; - // FIXME: warn about unremapped paths? - if (replacement != ref) - rewrites.insert_or_assign(store->printStorePath(ref), store->printStorePath(replacement)); - references.insert(std::move(replacement)); - } + if (json) { + JSONObject jsonRoot(std::cout); + JSONObject jsonRewrites(jsonRoot.object("rewrites")); + for (auto & path : storePaths) { + auto i = remappings.find(path); + assert(i != remappings.end()); + jsonRewrites.attr(store->printStorePath(path), store->printStorePath(i->second)); + } + } else { + for (auto & path : storePaths) { + auto i = remappings.find(path); + assert(i != remappings.end()); + notice("rewrote '%s' to '%s'", + store->printStorePath(path), + store->printStorePath(i->second)); } - - sink.s = rewriteStrings(sink.s, rewrites); - - HashModuloSink hashModuloSink(htSHA256, oldHashPart); - hashModuloSink(sink.s); - - auto narHash = hashModuloSink.finish().first; - - ValidPathInfo info { - store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), - narHash, - }; - info.references = std::move(references); - if (hasSelfReference) info.references.insert(info.path); - info.narSize = sink.s.size(); - info.ca = FixedOutputHash { - .method = FileIngestionMethod::Recursive, - .hash = info.narHash, - }; - - if (!json) - notice("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); - - auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); - rsink2(sink.s); - rsink2.flush(); - }); - - store->addToStore(info, *source); - - if (json) - jsonRewrites->attr(store->printStorePath(path), store->printStorePath(info.path)); - - remappings.insert_or_assign(std::move(path), std::move(info.path)); } } }; From f18607549ce38545b1d754ed93f3b7c5417970d8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:47:50 +0100 Subject: [PATCH 24/98] Fix makeContentAddressed() on self-references LocalStore::addToStore() since 79ae9e4558cbefd743f28a5e73110c2303b03a85 expects a regular NAR hash, rather than a NAR hash modulo self-references. Fixes #6300. Also, makeContentAddressed() now rewrites the entire closure (so 'nix store make-content-addressable' no longer needs '-r'). See #6301. --- src/libstore/make-content-addressed.cc | 35 +++++++++++++------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index 0b95ff37c..fc11fcb27 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -8,9 +8,10 @@ std::map makeContentAddressed( Store & dstStore, const StorePathSet & storePaths) { - // FIXME: use closure of storePaths. + StorePathSet closure; + srcStore.computeFSClosure(storePaths, closure); - auto paths = srcStore.topoSortPaths(storePaths); + auto paths = srcStore.topoSortPaths(closure); std::reverse(paths.begin(), paths.end()); @@ -46,29 +47,29 @@ std::map makeContentAddressed( HashModuloSink hashModuloSink(htSHA256, oldHashPart); hashModuloSink(sink.s); - auto narHash = hashModuloSink.finish().first; + auto narModuloHash = hashModuloSink.finish().first; - ValidPathInfo info { - dstStore.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), - narHash, - }; + auto dstPath = dstStore.makeFixedOutputPath( + FileIngestionMethod::Recursive, narModuloHash, path.name(), references, hasSelfReference); + + printInfo("rewroting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); + + StringSink sink2; + RewritingSink rsink2(oldHashPart, std::string(dstPath.hashPart()), sink2); + rsink2(sink.s); + rsink2.flush(); + + ValidPathInfo info { dstPath, hashString(htSHA256, sink2.s) }; info.references = std::move(references); if (hasSelfReference) info.references.insert(info.path); info.narSize = sink.s.size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, - .hash = info.narHash, + .hash = narModuloHash, }; - printInfo("rewrote '%s' to '%s'", pathS, srcStore.printStorePath(info.path)); - - auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); - rsink2(sink.s); - rsink2.flush(); - }); - - dstStore.addToStore(info, *source); + StringSource source(sink2.s); + dstStore.addToStore(info, source); remappings.insert_or_assign(std::move(path), std::move(info.path)); } From 5acaf13d3564f689e5461f29a9cc5958809d5e93 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:54:49 +0100 Subject: [PATCH 25/98] Rename 'nix store make-content-addressable' to 'nix store make-content-addressed' --- doc/manual/src/release-notes/rl-next.md | 3 +++ src/nix/main.cc | 2 +- ...e-content-addressable.cc => make-content-addressed.cc} | 8 ++++---- ...e-content-addressable.md => make-content-addressed.md} | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) rename src/nix/{make-content-addressable.cc => make-content-addressed.cc} (83%) rename src/nix/{make-content-addressable.md => make-content-addressed.md} (94%) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index c9753f9aa..8fbc605e7 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -1,3 +1,6 @@ # Release X.Y (202?-??-??) * Various nix commands can now read expressions from stdin with `--file -`. + +* `nix store make-content-addressable` has been renamed to `nix store + make-content-addressed`. diff --git a/src/nix/main.cc b/src/nix/main.cc index b923f2535..9bc6c15fa 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -117,7 +117,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs {"hash-path", {"hash", "path"}}, {"ls-nar", {"nar", "ls"}}, {"ls-store", {"store", "ls"}}, - {"make-content-addressable", {"store", "make-content-addressable"}}, + {"make-content-addressable", {"store", "make-content-addressed"}}, {"optimise-store", {"store", "optimise"}}, {"ping-store", {"store", "ping"}}, {"sign-paths", {"store", "sign"}}, diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressed.cc similarity index 83% rename from src/nix/make-content-addressable.cc rename to src/nix/make-content-addressed.cc index a8579ea7c..dc0447cb8 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressed.cc @@ -6,9 +6,9 @@ using namespace nix; -struct CmdMakeContentAddressable : StorePathsCommand, MixJSON +struct CmdMakeContentAddressed : StorePathsCommand, MixJSON { - CmdMakeContentAddressable() + CmdMakeContentAddressed() { realiseMode = Realise::Outputs; } @@ -21,7 +21,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON std::string doc() override { return - #include "make-content-addressable.md" + #include "make-content-addressed.md" ; } @@ -50,4 +50,4 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON } }; -static auto rCmdMakeContentAddressable = registerCommand2({"store", "make-content-addressable"}); +static auto rCmdMakeContentAddressed = registerCommand2({"store", "make-content-addressed"}); diff --git a/src/nix/make-content-addressable.md b/src/nix/make-content-addressed.md similarity index 94% rename from src/nix/make-content-addressable.md rename to src/nix/make-content-addressed.md index 3dd847edc..215683e6d 100644 --- a/src/nix/make-content-addressable.md +++ b/src/nix/make-content-addressed.md @@ -5,7 +5,7 @@ R""( * Create a content-addressed representation of the closure of GNU Hello: ```console - # nix store make-content-addressable -r nixpkgs#hello + # nix store make-content-addressed nixpkgs#hello … rewrote '/nix/store/v5sv61sszx301i0x6xysaqzla09nksnd-hello-2.10' to '/nix/store/5skmmcb9svys5lj3kbsrjg7vf2irid63-hello-2.10' ``` @@ -29,7 +29,7 @@ R""( system closure: ```console - # nix store make-content-addressable -r /run/current-system + # nix store make-content-addressed /run/current-system ``` # Description From 7ffda0af6effbf32c8668f34cc3f0448c58bc3c1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 22:01:20 +0100 Subject: [PATCH 26/98] fetchClosure: Skip makeContentAddressed() if toPath is already valid --- src/libexpr/primops/fetchClosure.cc | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 56fd53ed5..c3f07b6d6 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -54,26 +54,28 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args auto fromStore = openStore(*fromStoreUrl); if (toCA) { - auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); - auto i = remappings.find(*fromPath); - assert(i != remappings.end()); - if (toPath && *toPath != i->second) - throw Error({ - .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", - state.store->printStorePath(*fromPath), - state.store->printStorePath(i->second), - state.store->printStorePath(*toPath)), - .errPos = pos - }); - if (!toPath) - throw Error({ - .msg = hintfmt( - "rewriting '%s' to content-addressed form yielded '%s'; " - "please set this in the 'toPath' attribute passed to 'fetchClosure'", - state.store->printStorePath(*fromPath), - state.store->printStorePath(i->second)), - .errPos = pos - }); + if (!toPath || !state.store->isValidPath(*toPath)) { + auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); + auto i = remappings.find(*fromPath); + assert(i != remappings.end()); + if (toPath && *toPath != i->second) + throw Error({ + .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second), + state.store->printStorePath(*toPath)), + .errPos = pos + }); + if (!toPath) + throw Error({ + .msg = hintfmt( + "rewriting '%s' to content-addressed form yielded '%s'; " + "please set this in the 'toPath' attribute passed to 'fetchClosure'", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second)), + .errPos = pos + }); + } } else { copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); toPath = fromPath; From 4120930ac19ab7296818fdc1d1389e7799168867 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 22:47:33 +0100 Subject: [PATCH 27/98] fetchClosure: Only allow some "safe" store types --- src/libexpr/primops/fetchClosure.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index c3f07b6d6..247bceb07 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -1,6 +1,7 @@ #include "primops.hh" #include "store-api.hh" #include "make-content-addressed.hh" +#include "url.hh" namespace nix { @@ -50,8 +51,15 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args .errPos = pos }); - // FIXME: only allow some "trusted" store types (like BinaryCacheStore). - auto fromStore = openStore(*fromStoreUrl); + auto parsedURL = parseURL(*fromStoreUrl); + + if (parsedURL.scheme != "http" && parsedURL.scheme != "https") + throw Error({ + .msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"), + .errPos = pos + }); + + auto fromStore = openStore(parsedURL.to_string()); if (toCA) { if (!toPath || !state.store->isValidPath(*toPath)) { From 28186b7044dca513e6e07c3e66b7de2143543ae4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:19:21 +0100 Subject: [PATCH 28/98] Add a test for fetchClosure and 'nix store make-content-addressed' --- src/libexpr/primops/fetchClosure.cc | 4 +- src/libstore/make-content-addressed.cc | 2 +- tests/binary-cache.sh | 2 +- tests/fetchClosure.sh | 57 ++++++++++++++++++++++++++ tests/local.mk | 3 +- 5 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 tests/fetchClosure.sh diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 247bceb07..47e2d2bf2 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -53,7 +53,9 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args auto parsedURL = parseURL(*fromStoreUrl); - if (parsedURL.scheme != "http" && parsedURL.scheme != "https") + if (parsedURL.scheme != "http" && + parsedURL.scheme != "https" && + !(getEnv("_NIX_IN_TEST").has_value() && parsedURL.scheme == "file")) throw Error({ .msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"), .errPos = pos diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index fc11fcb27..64d172918 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -52,7 +52,7 @@ std::map makeContentAddressed( auto dstPath = dstStore.makeFixedOutputPath( FileIngestionMethod::Recursive, narModuloHash, path.name(), references, hasSelfReference); - printInfo("rewroting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); + printInfo("rewriting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); StringSink sink2; RewritingSink rsink2(oldHashPart, std::string(dstPath.hashPart()), sink2); diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 2368884f7..0361ac6a8 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -1,6 +1,6 @@ source common.sh -needLocalStore "“--no-require-sigs” can’t be used with the daemon" +needLocalStore "'--no-require-sigs' can’t be used with the daemon" # We can produce drvs directly into the binary cache clearStore diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh new file mode 100644 index 000000000..811d44af9 --- /dev/null +++ b/tests/fetchClosure.sh @@ -0,0 +1,57 @@ +source common.sh + +needLocalStore "'--no-require-sigs' can’t be used with the daemon" + +clearStore +clearCacheCache + +# Initialize binary cache. +nonCaPath=$(nix build --json --file ./dependencies.nix | jq -r .[].outputs.out) +caPath=$(nix store make-content-addressed --json $nonCaPath | jq -r '.rewrites | map(.) | .[]') +nix copy --to file://$cacheDir $nonCaPath + +# Test basic fetchClosure rewriting from non-CA to CA. +clearStore + +[ ! -e $nonCaPath ] +[ ! -e $caPath ] + +[[ $(nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + toPath = $caPath; + } +") = $caPath ]] + +[ ! -e $nonCaPath ] +[ -e $caPath ] + +# In impure mode, we can use non-CA paths. +[[ $(nix eval --raw --no-require-sigs --impure --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + } +") = $nonCaPath ]] + +[ -e $nonCaPath ] + +# 'toPath' set to empty string should fail but print the expected path. +nix eval -v --json --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + toPath = \"\"; + } +" 2>&1 | grep "error: rewriting.*$nonCaPath.*yielded.*$caPath" + +# If fromPath is CA, then toPath isn't needed. +nix copy --to file://$cacheDir $caPath + +[[ $(nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $caPath; + } +") = $caPath ]] diff --git a/tests/local.mk b/tests/local.mk index c686be049..97971dd76 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -96,7 +96,8 @@ nix_tests = \ describe-stores.sh \ nix-profile.sh \ suggestions.sh \ - store-ping.sh + store-ping.sh \ + fetchClosure.sh ifeq ($(HAVE_LIBCPUID), 1) nix_tests += compute-levels.sh From 98658ae9d25e5e715f9349fbbb79d0ba31bb810c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:31:48 +0100 Subject: [PATCH 29/98] Document fetchClosure --- doc/manual/src/release-notes/rl-next.md | 7 ++++++ src/libexpr/primops/fetchClosure.cc | 33 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 8fbc605e7..33eaa8a2c 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -4,3 +4,10 @@ * `nix store make-content-addressable` has been renamed to `nix store make-content-addressed`. + +* New builtin function `builtins.fetchClosure` that copies a closure + from a binary cache at evaluation time and rewrites it to + content-addressed form (if it isn't already). Like + `builtins.storePath`, this allows importing pre-built store paths; + the difference is that it doesn't require the user to configure + binary caches and trusted public keys. diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 47e2d2bf2..045e97dbd 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -110,6 +110,39 @@ static RegisterPrimOp primop_fetchClosure({ .name = "__fetchClosure", .args = {"args"}, .doc = R"( + Fetch a Nix store closure from a binary cache, rewriting it into + content-addressed form. For example, + + ```nix + builtins.fetchClosure { + fromStore = "https://cache.nixos.org"; + fromPath = /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1; + toPath = /nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1; + } + ``` + + fetches `/nix/store/r2jd...` from the specified binary cache, + and rewrites it into the content-addressed store path + `/nix/store/ldbh...`. + + If `fromPath` is already content-addressed, or if you are + allowing impure evaluation (`--impure`), then `toPath` may be + omitted. + + To find out the correct value for `toPath` given a `fromPath`, + you can use `nix store make-content-addressed`: + + ```console + # nix store make-content-addressed /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 + rewrote '/nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1' to '/nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1' + ``` + + This function is similar to `builtins.storePath` in that it + allows you to use a previously built store path in a Nix + expression. However, it is more reproducible because it requires + specifying a binary cache from which the path can be fetched. + Also, requiring a content-addressed final store path avoids the + need for users to configure binary cache public keys. )", .fun = prim_fetchClosure, }); From e5f7029ba49a486c1b0c8011be79b8b41be20935 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:40:04 +0100 Subject: [PATCH 30/98] nix store make-content-addressed: Support --from / --to --- src/libexpr/primops/fetchClosure.cc | 2 +- src/nix/make-content-addressed.cc | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 045e97dbd..3e07d1d18 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -133,7 +133,7 @@ static RegisterPrimOp primop_fetchClosure({ you can use `nix store make-content-addressed`: ```console - # nix store make-content-addressed /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 + # nix store make-content-addressed --from https://cache.nixos.org /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 rewrote '/nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1' to '/nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1' ``` diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc index dc0447cb8..34860c38f 100644 --- a/src/nix/make-content-addressed.cc +++ b/src/nix/make-content-addressed.cc @@ -6,7 +6,7 @@ using namespace nix; -struct CmdMakeContentAddressed : StorePathsCommand, MixJSON +struct CmdMakeContentAddressed : virtual CopyCommand, virtual StorePathsCommand, MixJSON { CmdMakeContentAddressed() { @@ -25,9 +25,11 @@ struct CmdMakeContentAddressed : StorePathsCommand, MixJSON ; } - void run(ref store, StorePaths && storePaths) override + void run(ref srcStore, StorePaths && storePaths) override { - auto remappings = makeContentAddressed(*store, *store, + auto dstStore = dstUri.empty() ? openStore() : openStore(dstUri); + + auto remappings = makeContentAddressed(*srcStore, *dstStore, StorePathSet(storePaths.begin(), storePaths.end())); if (json) { @@ -36,15 +38,15 @@ struct CmdMakeContentAddressed : StorePathsCommand, MixJSON for (auto & path : storePaths) { auto i = remappings.find(path); assert(i != remappings.end()); - jsonRewrites.attr(store->printStorePath(path), store->printStorePath(i->second)); + jsonRewrites.attr(srcStore->printStorePath(path), srcStore->printStorePath(i->second)); } } else { for (auto & path : storePaths) { auto i = remappings.find(path); assert(i != remappings.end()); notice("rewrote '%s' to '%s'", - store->printStorePath(path), - store->printStorePath(i->second)); + srcStore->printStorePath(path), + srcStore->printStorePath(i->second)); } } } From f902f3c2cbfa1a78bec8d135297abe244452e794 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 21:33:20 +0100 Subject: [PATCH 31/98] Add experimental feature 'fetch-closure' --- src/libexpr/primops/fetchClosure.cc | 2 ++ src/libutil/experimental-features.cc | 1 + src/libutil/experimental-features.hh | 3 ++- tests/fetchClosure.sh | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 3e07d1d18..47f40ef33 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,6 +7,8 @@ namespace nix { static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) { + settings.requireExperimentalFeature(Xp::FetchClosure); + state.forceAttrs(*args[0], pos); std::optional fromStoreUrl; diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index b49f47e1d..01f318fa3 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -11,6 +11,7 @@ std::map stringifiedXpFeatures = { { Xp::NixCommand, "nix-command" }, { Xp::RecursiveNix, "recursive-nix" }, { Xp::NoUrlLiterals, "no-url-literals" }, + { Xp::FetchClosure, "fetch-closure" }, }; const std::optional parseExperimentalFeature(const std::string_view & name) diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 291a58e32..b5140dcfe 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -19,7 +19,8 @@ enum struct ExperimentalFeature Flakes, NixCommand, RecursiveNix, - NoUrlLiterals + NoUrlLiterals, + FetchClosure, }; /** diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh index 811d44af9..0c905ac43 100644 --- a/tests/fetchClosure.sh +++ b/tests/fetchClosure.sh @@ -1,5 +1,6 @@ source common.sh +enableFeatures "fetch-closure" needLocalStore "'--no-require-sigs' can’t be used with the daemon" clearStore From c85467a1b65d2a8907fcab3303572f76c071776d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 6 Feb 2022 14:47:07 +0100 Subject: [PATCH 32/98] Revert "TarArchive: Small refactoring" This reverts commit 50a35860ee9237d341948437c5f70a7f0987d393. With this change Nix fails to open bzip2 logfiles that were created from builds with no stdout/stderr. --- src/libutil/tarfile.cc | 26 ++++++++++++++------------ src/libutil/tarfile.hh | 3 --- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 790bc943a..a7db58559 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -39,30 +39,32 @@ void TarArchive::check(int err, const std::string & reason) throw Error(reason, archive_error_string(this->archive)); } -TarArchive::TarArchive(Source & source, bool raw) - : source(&source), buffer(4096) +TarArchive::TarArchive(Source & source, bool raw) : buffer(4096) { - init(); - if (!raw) + this->archive = archive_read_new(); + this->source = &source; + + if (!raw) { + archive_read_support_filter_all(archive); archive_read_support_format_all(archive); - else + } else { + archive_read_support_filter_all(archive); archive_read_support_format_raw(archive); + archive_read_support_format_empty(archive); + } check(archive_read_open(archive, (void *)this, callback_open, callback_read, callback_close), "Failed to open archive (%s)"); } + TarArchive::TarArchive(const Path & path) { - init(); + this->archive = archive_read_new(); + + archive_read_support_filter_all(archive); archive_read_support_format_all(archive); check(archive_read_open_filename(archive, path.c_str(), 16384), "failed to open archive: %s"); } -void TarArchive::init() -{ - archive = archive_read_new(); - archive_read_support_filter_all(archive); -} - void TarArchive::close() { check(archive_read_close(this->archive), "Failed to close archive (%s)"); diff --git a/src/libutil/tarfile.hh b/src/libutil/tarfile.hh index f107a7e2e..4d9141fd4 100644 --- a/src/libutil/tarfile.hh +++ b/src/libutil/tarfile.hh @@ -17,13 +17,10 @@ struct TarArchive { // disable copy constructor TarArchive(const TarArchive &) = delete; - void init(); - void close(); ~TarArchive(); }; - void unpackTarfile(Source & source, const Path & destDir); void unpackTarfile(const Path & tarFile, const Path & destDir); From d02e34ef06e24f98404d7a50130ddced15a12279 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 6 Feb 2022 14:52:04 +0100 Subject: [PATCH 33/98] Implement regression test for empty logs loaded via `nix log` --- tests/logging.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/logging.sh b/tests/logging.sh index c894ad3ff..1481b9b36 100644 --- a/tests/logging.sh +++ b/tests/logging.sh @@ -13,3 +13,14 @@ rm -rf $NIX_LOG_DIR (! nix-store -l $path) nix-build dependencies.nix --no-out-link --compress-build-log [ "$(nix-store -l $path)" = FOO ] + +# test whether empty logs work fine with `nix log`. +builder="$(mktemp)" +echo -e "#!/bin/sh\nmkdir \$out" > "$builder" +outp="$(nix-build -E \ + 'with import ./config.nix; mkDerivation { name = "fnord"; builder = '"$builder"'; }' \ + --out-link "$(mktemp -d)/result")" + +test -d "$outp" + +nix log "$outp" From 175c78591b1877c280936cc663b76e47f207dd77 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:09:43 +0100 Subject: [PATCH 34/98] Random cleanup --- src/libstore/build/goal.hh | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 07c752bb9..35121c5d9 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -40,21 +40,21 @@ struct Goal : public std::enable_shared_from_this WeakGoals waiters; /* Number of goals we are/were waiting for that have failed. */ - unsigned int nrFailed; + size_t nrFailed = 0; /* Number of substitution goals we are/were waiting for that failed because there are no substituters. */ - unsigned int nrNoSubstituters; + size_t nrNoSubstituters = 0; /* Number of substitution goals we are/were waiting for that failed because they had unsubstitutable references. */ - unsigned int nrIncompleteClosure; + size_t nrIncompleteClosure = 0; /* Name of this goal for debugging purposes. */ std::string name; /* Whether the goal is finished. */ - ExitCode exitCode; + ExitCode exitCode = ecBusy; /* Build result. */ BuildResult buildResult; @@ -65,10 +65,7 @@ struct Goal : public std::enable_shared_from_this Goal(Worker & worker, DerivedPath path) : worker(worker) , buildResult { .path = std::move(path) } - { - nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; - exitCode = ecBusy; - } + { } virtual ~Goal() { From 09796c026376b49a8bc5bb3a2865db015d3dfb06 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:24:10 +0100 Subject: [PATCH 35/98] Random cleanup --- src/libstore/build/goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index d2420b107..58e805f55 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -28,7 +28,7 @@ void Goal::addWaitee(GoalPtr waitee) void Goal::waiteeDone(GoalPtr waitee, ExitCode result) { - assert(waitees.find(waitee) != waitees.end()); + assert(waitees.count(waitee)); waitees.erase(waitee); trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size())); From fe5509df9a58359a67da8c72ed8721cb3a33371d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:24:48 +0100 Subject: [PATCH 36/98] Only return wanted outputs --- src/libstore/build/derivation-goal.cc | 5 +---- src/libstore/build/local-derivation-goal.cc | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 40c445836..7dc5737fb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -311,14 +311,11 @@ void DerivationGoal::outputsSubstitutionTried() gaveUpOnSubstitution(); } + /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ void DerivationGoal::gaveUpOnSubstitution() { - /* Make sure checkPathValidity() from now on checks all - outputs. */ - wantedOutputs.clear(); - /* The inputs must be built before we can build this goal. */ if (useDerivation) for (auto & i : dynamic_cast(drv.get())->inputDrvs) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 75e7e6ca3..ed83c4770 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2613,7 +2613,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs() signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - builtOutputs.emplace(thisRealisation.id, thisRealisation); + if (wantedOutputs.empty() || wantedOutputs.count(outputName)) + builtOutputs.emplace(thisRealisation.id, thisRealisation); } return builtOutputs; From 540d7e33d879e21d34de0935292a65bf27f5e312 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:25:12 +0100 Subject: [PATCH 37/98] Retry substitution after an incomplete closure only once This avoids an infinite loop in the final test in tests/binary-cache.sh. I think this was only not triggered previously by accident (because we were clearing wantedOutputs in between). --- src/libstore/build/derivation-goal.cc | 5 ++--- src/libstore/build/derivation-goal.hh | 8 ++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7dc5737fb..3d1c4fbc1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -207,8 +207,6 @@ void DerivationGoal::haveDerivation() if (!drv->type().hasKnownOutputPaths()) settings.requireExperimentalFeature(Xp::CaDerivations); - retrySubstitution = false; - for (auto & i : drv->outputsAndOptPaths(worker.store)) if (i.second.second) worker.store.addTempRoot(*i.second.second); @@ -423,7 +421,8 @@ void DerivationGoal::inputsRealised() return; } - if (retrySubstitution) { + if (retrySubstitution && !retriedSubstitution) { + retriedSubstitution = true; haveDerivation(); return; } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index ea2db89b2..f556b6f25 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -61,8 +61,12 @@ struct DerivationGoal : public Goal bool needRestart = false; /* Whether to retry substituting the outputs after building the - inputs. */ - bool retrySubstitution; + inputs. This is done in case of an incomplete closure. */ + bool retrySubstitution = false; + + /* Whether we've retried substitution, in which case we won't try + again. */ + bool retriedSubstitution = false; /* The derivation stored at drvPath. */ std::unique_ptr drv; From 187dc080a2ab3403c1b1b47b3face205a4a67fb4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:36:14 +0100 Subject: [PATCH 38/98] tests/build.sh: Test that 'nix build' only prints wanted outputs --- tests/build.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/build.sh b/tests/build.sh index c77f620f7..13a0f42be 100644 --- a/tests/build.sh +++ b/tests/build.sh @@ -1,15 +1,27 @@ source common.sh -expectedJSONRegex='\[\{"drvPath":".*multiple-outputs-a.drv","outputs":\{"first":".*multiple-outputs-a-first","second":".*multiple-outputs-a-second"}},\{"drvPath":".*multiple-outputs-b.drv","outputs":\{"out":".*multiple-outputs-b"}}]' +clearStore + +# Make sure that 'nix build' only returns the outputs we asked for. +nix build -f multiple-outputs.nix --json a --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys | length == 1) and + (.outputs.first | match(".*multiple-outputs-a-first"))) +' + nix build -f multiple-outputs.nix --json a.all b.all --no-link | jq --exit-status ' (.[0] | (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys | length == 2) and (.outputs.first | match(".*multiple-outputs-a-first")) and (.outputs.second | match(".*multiple-outputs-a-second"))) and (.[1] | (.drvPath | match(".*multiple-outputs-b.drv")) and + (.outputs | keys | length == 1) and (.outputs.out | match(".*multiple-outputs-b"))) ' + testNormalization () { clearStore outPath=$(nix-build ./simple.nix --no-out-link) From cbcb69a39ce7ceb300d5a5588258b7f8c0c0664d Mon Sep 17 00:00:00 2001 From: polykernel <81340136+polykernel@users.noreply.github.com> Date: Sun, 20 Mar 2022 11:39:36 -0400 Subject: [PATCH 39/98] nix: allow whitespace characters before command in repl Before this change, processLine always uses the first character as the start of the line. This cause whitespaces to matter at the beginning of the line whereas it does not matter anywhere else. This commit trims leading white spaces of the string line so that subsequent operations can be performed on the string without explicitly tracking starting and ending indices of the string. --- src/nix/repl.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 916353d8c..1f9d4fb4e 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -396,6 +396,7 @@ StorePath NixRepl::getDerivationPath(Value & v) { bool NixRepl::processLine(std::string line) { + line = trim(line); if (line == "") return true; _isInterrupted = false; From 50c229ad9aeece3b4728fedb741d8ef7adb2b2c1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 08:02:49 +0100 Subject: [PATCH 40/98] Use wantOutput Co-authored-by: John Ericson --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index ed83c4770..b176f318b 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2613,7 +2613,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - if (wantedOutputs.empty() || wantedOutputs.count(outputName)) + if (wantOutput(outputName, wantedOutputs)) builtOutputs.emplace(thisRealisation.id, thisRealisation); } From 86b05ccd54f2e98ac2b5cef3bcecb29ed6ec4fd8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 14:04:18 +0100 Subject: [PATCH 41/98] Only provide builtin.{getFlake,fetchClosure} is the corresponding experimental feature is enabled This allows writing fallback code like if builtins ? fetchClosure then builtins.fetchClose { ... } else builtins.storePath ... --- doc/manual/src/release-notes/rl-next.md | 9 ++++++--- src/libexpr/eval.cc | 17 ----------------- src/libexpr/eval.hh | 6 ------ src/libexpr/flake/flake.cc | 9 ++++++--- src/libexpr/primops.cc | 20 ++++++++++++-------- src/libexpr/primops.hh | 2 ++ src/libexpr/primops/fetchClosure.cc | 6 ++++-- src/nix/main.cc | 1 + 8 files changed, 31 insertions(+), 39 deletions(-) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 33eaa8a2c..2ec864ee4 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -5,9 +5,12 @@ * `nix store make-content-addressable` has been renamed to `nix store make-content-addressed`. -* New builtin function `builtins.fetchClosure` that copies a closure - from a binary cache at evaluation time and rewrites it to - content-addressed form (if it isn't already). Like +* New experimental builtin function `builtins.fetchClosure` that + copies a closure from a binary cache at evaluation time and rewrites + it to content-addressed form (if it isn't already). Like `builtins.storePath`, this allows importing pre-built store paths; the difference is that it doesn't require the user to configure binary caches and trusted public keys. + + This function is only available if you enable the experimental + feature `fetch-closure`. diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3ec5ed202..437c7fc53 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -507,23 +507,6 @@ EvalState::~EvalState() } -void EvalState::requireExperimentalFeatureOnEvaluation( - const ExperimentalFeature & feature, - const std::string_view fName, - const Pos & pos) -{ - if (!settings.isExperimentalFeatureEnabled(feature)) { - throw EvalError({ - .msg = hintfmt( - "Cannot call '%2%' because experimental Nix feature '%1%' is disabled. You can enable it via '--extra-experimental-features %1%'.", - feature, - fName - ), - .errPos = pos - }); - } -} - void EvalState::allowPath(const Path & path) { if (allowedPaths) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 198a62ad2..ab46b448c 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -149,12 +149,6 @@ public: std::shared_ptr buildStore = nullptr); ~EvalState(); - void requireExperimentalFeatureOnEvaluation( - const ExperimentalFeature &, - const std::string_view fName, - const Pos & pos - ); - void addToSearchPath(const std::string & s); SearchPath getSearchPath() { return searchPath; } diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 6a1aca40d..fdcfb18d7 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -706,8 +706,6 @@ void callFlake(EvalState & state, static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v) { - state.requireExperimentalFeatureOnEvaluation(Xp::Flakes, "builtins.getFlake", pos); - std::string flakeRefS(state.forceStringNoCtx(*args[0], pos)); auto flakeRef = parseFlakeRef(flakeRefS, {}, true); if (evalSettings.pureEval && !flakeRef.input.isLocked()) @@ -723,7 +721,12 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va v); } -static RegisterPrimOp r2("__getFlake", 1, prim_getFlake); +static RegisterPrimOp r2({ + .name = "__getFlake", + .args = {"args"}, + .fun = prim_getFlake, + .experimentalFeature = Xp::Flakes, +}); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f5d9aa9ae..f3eb5e925 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3814,7 +3814,7 @@ RegisterPrimOp::RegisterPrimOp(std::string name, size_t arity, PrimOpFun fun) .name = name, .args = {}, .arity = arity, - .fun = fun + .fun = fun, }); } @@ -3886,13 +3886,17 @@ void EvalState::createBaseEnv() if (RegisterPrimOp::primOps) for (auto & primOp : *RegisterPrimOp::primOps) - addPrimOp({ - .fun = primOp.fun, - .arity = std::max(primOp.args.size(), primOp.arity), - .name = symbols.create(primOp.name), - .args = primOp.args, - .doc = primOp.doc, - }); + if (!primOp.experimentalFeature + || settings.isExperimentalFeatureEnabled(*primOp.experimentalFeature)) + { + addPrimOp({ + .fun = primOp.fun, + .arity = std::max(primOp.args.size(), primOp.arity), + .name = symbols.create(primOp.name), + .args = primOp.args, + .doc = primOp.doc, + }); + } /* Add a wrapper around the derivation primop that computes the `drvPath' and `outPath' attributes lazily. */ diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh index 5b16e075f..905bd0366 100644 --- a/src/libexpr/primops.hh +++ b/src/libexpr/primops.hh @@ -16,6 +16,7 @@ struct RegisterPrimOp size_t arity = 0; const char * doc; PrimOpFun fun; + std::optional experimentalFeature; }; typedef std::vector PrimOps; @@ -35,6 +36,7 @@ struct RegisterPrimOp /* These primops are disabled without enableNativeCode, but plugins may wish to use them in limited contexts without globally enabling them. */ + /* Load a ValueInitializer from a DSO and return whatever it initializes */ void prim_importNative(EvalState & state, const Pos & pos, Value * * args, Value & v); diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 47f40ef33..efeb93daf 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,8 +7,6 @@ namespace nix { static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) { - settings.requireExperimentalFeature(Xp::FetchClosure); - state.forceAttrs(*args[0], pos); std::optional fromStoreUrl; @@ -145,8 +143,12 @@ static RegisterPrimOp primop_fetchClosure({ specifying a binary cache from which the path can be fetched. Also, requiring a content-addressed final store path avoids the need for users to configure binary cache public keys. + + This function is only available if you enable the experimental + feature `fetch-closure`. )", .fun = prim_fetchClosure, + .experimentalFeature = Xp::FetchClosure, }); } diff --git a/src/nix/main.cc b/src/nix/main.cc index 9bc6c15fa..6198681e7 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -289,6 +289,7 @@ void mainWrapped(int argc, char * * argv) } if (argc == 2 && std::string(argv[1]) == "__dump-builtins") { + settings.experimentalFeatures = {Xp::Flakes, Xp::FetchClosure}; evalSettings.pureEval = false; EvalState state({}, openStore("dummy://")); auto res = nlohmann::json::object(); From 8c363eb3eb3db96287a10eb5fcde23986a96164f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 14:19:55 +0100 Subject: [PATCH 42/98] Document getFlake Fixes #5523. --- src/libexpr/flake/flake.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index fdcfb18d7..22257c6b3 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -724,6 +724,24 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va static RegisterPrimOp r2({ .name = "__getFlake", .args = {"args"}, + .doc = R"( + Fetch a flake from a flake reference, and return its output attributes and some metadata. For example: + + ```nix + (builtins.getFlake "nix/55bc52401966fbffa525c574c14f67b00bc4fb3a").packages.x86_64-linux.nix + ``` + + Unless impure evaluation is allowed (`--impure`), the flake reference + must be "locked", e.g. contain a Git revision or content hash. An + example of an unlocked usage is: + + ```nix + (builtins.getFlake "github:edolstra/dwarffs").rev + ``` + + This function is only available if you enable the experimental feature + `flakes`. + )", .fun = prim_getFlake, .experimentalFeature = Xp::Flakes, }); From fc35b11a7cde047cd799a6c83b0da312b713e0fb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 15:22:22 +0100 Subject: [PATCH 43/98] Fix mismatched tag warning on clang --- src/libexpr/eval.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index ab46b448c..e7915dd99 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -17,7 +17,7 @@ namespace nix { -struct Store; +class Store; class EvalState; class StorePath; enum RepairFlag : bool; From 247d2cb661218762a8a1fc0ee475a8ca856fcf17 Mon Sep 17 00:00:00 2001 From: Artturin Date: Sat, 26 Mar 2022 00:58:19 +0200 Subject: [PATCH 44/98] scripts/install-systemd-multi-user.sh: fix typo sytemd-tmpfiles -> systemd-tmpfiles --- scripts/install-systemd-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index 24884a023..1d92c5388 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -90,7 +90,7 @@ poly_configure_nix_daemon_service() { ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST _sudo "to run systemd-tmpfiles once to pick that path up" \ - sytemd-tmpfiles create --prefix=/nix/var/nix + systemd-tmpfiles create --prefix=/nix/var/nix _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From 057f9ee1900312f42efe6c5cebb02b07b4ff2131 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Mar 2022 14:21:35 +0200 Subject: [PATCH 45/98] nix profile install: Don't use queryDerivationOutputMap() Instead get the outputs from Installable::build(). This will also allow 'nix profile install' to support impure derivations. Fixes #6286. --- src/libcmd/installables.cc | 139 ++++++++++++++++++++--------------- src/libcmd/installables.hh | 12 +-- src/libstore/derived-path.hh | 6 ++ src/nix/profile.cc | 35 ++++++--- 4 files changed, 116 insertions(+), 76 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 784117569..955bbe6fb 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -756,55 +756,20 @@ std::shared_ptr SourceExprCommand::parseInstallable( return installables.front(); } -BuiltPaths getBuiltPaths(ref evalStore, ref store, const DerivedPaths & hopefullyBuiltPaths) +BuiltPaths Installable::build( + ref evalStore, + ref store, + Realise mode, + const std::vector> & installables, + BuildMode bMode) { BuiltPaths res; - for (const auto & b : hopefullyBuiltPaths) - std::visit( - overloaded{ - [&](const DerivedPath::Opaque & bo) { - res.push_back(BuiltPath::Opaque{bo.path}); - }, - [&](const DerivedPath::Built & bfd) { - OutputPathMap outputs; - auto drv = evalStore->readDerivation(bfd.drvPath); - auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive - auto drvOutputs = drv.outputsAndOptPaths(*store); - for (auto & output : bfd.outputs) { - if (!outputHashes.count(output)) - throw Error( - "the derivation '%s' doesn't have an output named '%s'", - store->printStorePath(bfd.drvPath), output); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto outputId = - DrvOutput{outputHashes.at(output), output}; - auto realisation = - store->queryRealisation(outputId); - if (!realisation) - throw Error( - "cannot operate on an output of unbuilt " - "content-addressed derivation '%s'", - outputId.to_string()); - outputs.insert_or_assign( - output, realisation->outPath); - } else { - // If ca-derivations isn't enabled, assume that - // the output path is statically known. - assert(drvOutputs.count(output)); - assert(drvOutputs.at(output).second); - outputs.insert_or_assign( - output, *drvOutputs.at(output).second); - } - } - res.push_back(BuiltPath::Built{bfd.drvPath, outputs}); - }, - }, - b.raw()); - + for (auto & [_, builtPath] : build2(evalStore, store, mode, installables, bMode)) + res.push_back(builtPath); return res; } -BuiltPaths Installable::build( +std::vector, BuiltPath>> Installable::build2( ref evalStore, ref store, Realise mode, @@ -815,39 +780,93 @@ BuiltPaths Installable::build( settings.readOnlyMode = true; std::vector pathsToBuild; + std::map>> backmap; for (auto & i : installables) { - auto b = i->toDerivedPaths(); - pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end()); + for (auto b : i->toDerivedPaths()) { + pathsToBuild.push_back(b); + backmap[b].push_back(i); + } } + std::vector, BuiltPath>> res; + switch (mode) { + case Realise::Nothing: case Realise::Derivation: printMissing(store, pathsToBuild, lvlError); - return getBuiltPaths(evalStore, store, pathsToBuild); + + for (auto & path : pathsToBuild) { + for (auto & installable : backmap[path]) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + OutputPathMap outputs; + auto drv = evalStore->readDerivation(bfd.drvPath); + auto outputHashes = staticOutputHashes(*evalStore, drv); // FIXME: expensive + auto drvOutputs = drv.outputsAndOptPaths(*store); + for (auto & output : bfd.outputs) { + if (!outputHashes.count(output)) + throw Error( + "the derivation '%s' doesn't have an output named '%s'", + store->printStorePath(bfd.drvPath), output); + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + DrvOutput outputId { outputHashes.at(output), output }; + auto realisation = store->queryRealisation(outputId); + if (!realisation) + throw Error( + "cannot operate on an output of unbuilt " + "content-addressed derivation '%s'", + outputId.to_string()); + outputs.insert_or_assign(output, realisation->outPath); + } else { + // If ca-derivations isn't enabled, assume that + // the output path is statically known. + assert(drvOutputs.count(output)); + assert(drvOutputs.at(output).second); + outputs.insert_or_assign( + output, *drvOutputs.at(output).second); + } + } + res.push_back({installable, BuiltPath::Built { bfd.drvPath, outputs }}); + }, + [&](const DerivedPath::Opaque & bo) { + res.push_back({installable, BuiltPath::Opaque { bo.path }}); + }, + }, path.raw()); + } + } + + break; + case Realise::Outputs: { - BuiltPaths res; for (auto & buildResult : store->buildPathsWithResults(pathsToBuild, bMode, evalStore)) { if (!buildResult.success()) buildResult.rethrow(); - std::visit(overloaded { - [&](const DerivedPath::Built & bfd) { - std::map outputs; - for (auto & path : buildResult.builtOutputs) - outputs.emplace(path.first.outputName, path.second.outPath); - res.push_back(BuiltPath::Built { bfd.drvPath, outputs }); - }, - [&](const DerivedPath::Opaque & bo) { - res.push_back(BuiltPath::Opaque { bo.path }); - }, - }, buildResult.path.raw()); + + for (auto & installable : backmap[buildResult.path]) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + std::map outputs; + for (auto & path : buildResult.builtOutputs) + outputs.emplace(path.first.outputName, path.second.outPath); + res.push_back({installable, BuiltPath::Built { bfd.drvPath, outputs }}); + }, + [&](const DerivedPath::Opaque & bo) { + res.push_back({installable, BuiltPath::Opaque { bo.path }}); + }, + }, buildResult.path.raw()); + } } - return res; + + break; } + default: assert(false); } + + return res; } BuiltPaths Installable::toBuiltPaths( diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index e172b71b0..f4bf0d406 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -98,6 +98,13 @@ struct Installable const std::vector> & installables, BuildMode bMode = bmNormal); + static std::vector, BuiltPath>> build2( + ref evalStore, + ref store, + Realise mode, + const std::vector> & installables, + BuildMode bMode = bmNormal); + static std::set toStorePaths( ref evalStore, ref store, @@ -185,9 +192,4 @@ ref openEvalCache( EvalState & state, std::shared_ptr lockedFlake); -BuiltPaths getBuiltPaths( - ref evalStore, - ref store, - const DerivedPaths & hopefullyBuiltPaths); - } diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index 8ca0882a4..24a0ae773 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -25,6 +25,9 @@ struct DerivedPathOpaque { nlohmann::json toJSON(ref store) const; std::string to_string(const Store & store) const; static DerivedPathOpaque parse(const Store & store, std::string_view); + + bool operator < (const DerivedPathOpaque & b) const + { return path < b.path; } }; /** @@ -46,6 +49,9 @@ struct DerivedPathBuilt { std::string to_string(const Store & store) const; static DerivedPathBuilt parse(const Store & store, std::string_view); nlohmann::json toJSON(ref store) const; + + bool operator < (const DerivedPathBuilt & b) const + { return std::make_pair(drvPath, outputs) < std::make_pair(b.drvPath, b.outputs); } }; using _DerivedPathRaw = std::variant< diff --git a/src/nix/profile.cc b/src/nix/profile.cc index da990ddc8..f35947ddb 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -62,22 +62,21 @@ struct ProfileElement return std::tuple(describe(), storePaths) < std::tuple(other.describe(), other.storePaths); } - void updateStorePaths(ref evalStore, ref store, Installable & installable) + void updateStorePaths( + ref evalStore, + ref store, + const BuiltPaths & builtPaths) { // FIXME: respect meta.outputsToInstall storePaths.clear(); - for (auto & buildable : getBuiltPaths(evalStore, store, installable.toDerivedPaths())) { + for (auto & buildable : builtPaths) { std::visit(overloaded { [&](const BuiltPath::Opaque & bo) { storePaths.insert(bo.path); }, [&](const BuiltPath::Built & bfd) { - // TODO: Why are we querying if we know the output - // names already? Is it just to figure out what the - // default one is? - for (auto & output : store->queryDerivationOutputMap(bfd.drvPath)) { + for (auto & output : bfd.outputs) storePaths.insert(output.second); - } }, }, buildable.raw()); } @@ -236,6 +235,16 @@ struct ProfileManifest } }; +static std::map +builtPathsPerInstallable( + const std::vector, BuiltPath>> & builtPaths) +{ + std::map res; + for (auto & [installable, builtPath] : builtPaths) + res[installable.get()].push_back(builtPath); + return res; +} + struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { std::string description() override @@ -254,7 +263,9 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { ProfileManifest manifest(*getEvalState(), *profile); - auto builtPaths = Installable::build(getEvalStore(), store, Realise::Outputs, installables, bmNormal); + auto builtPaths = builtPathsPerInstallable( + Installable::build2( + getEvalStore(), store, Realise::Outputs, installables, bmNormal)); for (auto & installable : installables) { ProfileElement element; @@ -269,7 +280,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile }; } - element.updateStorePaths(getEvalStore(), store, *installable); + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); manifest.elements.push_back(std::move(element)); } @@ -457,12 +468,14 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf warn ("Use 'nix profile list' to see the current profile."); } - auto builtPaths = Installable::build(getEvalStore(), store, Realise::Outputs, installables, bmNormal); + auto builtPaths = builtPathsPerInstallable( + Installable::build2( + getEvalStore(), store, Realise::Outputs, installables, bmNormal)); for (size_t i = 0; i < installables.size(); ++i) { auto & installable = installables.at(i); auto & element = manifest.elements[indices.at(i)]; - element.updateStorePaths(getEvalStore(), store, *installable); + element.updateStorePaths(getEvalStore(), store, builtPaths[installable.get()]); } updateProfile(manifest.build(store)); From b266fd53dda9c303c55ceb55752c2117011fce69 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Mar 2022 14:58:38 +0200 Subject: [PATCH 46/98] nix {run,shell}: Print a better error message if the store is not local Closes #6317 --- src/nix/run.cc | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/nix/run.cc b/src/nix/run.cc index 033263c36..23e893fbf 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -38,9 +38,12 @@ void runProgramInStore(ref store, unshare(CLONE_NEWUSER) doesn't work in a multithreaded program (which "nix" is), so we exec() a single-threaded helper program (chrootHelper() below) to do the work. */ - auto store2 = store.dynamic_pointer_cast(); + auto store2 = store.dynamic_pointer_cast(); - if (store2 && store->storeDir != store2->getRealStoreDir()) { + if (!store2) + throw Error("store '%s' is not a local store so it does not support command execution", store->getUri()); + + if (store->storeDir != store2->getRealStoreDir()) { Strings helperArgs = { chrootHelperName, store->storeDir, store2->getRealStoreDir(), program }; for (auto & arg : args) helperArgs.push_back(arg); From 390269ed8784b1a73a3310e63eb96a4b62861654 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 16 Mar 2022 14:21:09 +0100 Subject: [PATCH 47/98] Simplify the handling of the hash modulo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than having four different but very similar types of hashes, make only one, with a tag indicating whether it corresponds to a regular of deferred derivation. This implies a slight logical change: The original Nix+multiple-outputs model assumed only one hash-modulo per derivation. Adding multiple-outputs CA derivations changed this as these have one hash-modulo per output. This change is now treating each derivation as having one hash modulo per output. This obviously means that we internally loose the guaranty that all the outputs of input-addressed derivations have the same hash modulo. But it turns out that it doesn’t matter because there’s nothing in the code taking advantage of that fact (and it probably shouldn’t anyways). The upside is that it is now much easier to work with these hashes, and we can get rid of a lot of useless `std::visit{ overloaded`. Co-authored-by: John Ericson --- src/libexpr/primops.cc | 48 ++++++++++------------- src/libstore/derivations.cc | 77 +++++++++++-------------------------- src/libstore/derivations.hh | 36 ++++------------- src/libstore/local-store.cc | 9 ++--- src/nix/develop.cc | 4 +- 5 files changed, 56 insertions(+), 118 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f3eb5e925..9f549e52f 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1222,34 +1222,26 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * DerivationOutput::Deferred { }); } - // Regular, non-CA derivation should always return a single hash and not - // hash per output. - auto hashModulo = hashDerivationModulo(*state.store, drv, true); - std::visit(overloaded { - [&](const DrvHash & drvHash) { - auto & h = drvHash.hash; - switch (drvHash.kind) { - case DrvHash::Kind::Deferred: - /* Outputs already deferred, nothing to do */ - break; - case DrvHash::Kind::Regular: - for (auto & [outputName, output] : drv.outputs) { - auto outPath = state.store->makeOutputPath(outputName, h, drvName); - drv.env[outputName] = state.store->printStorePath(outPath); - output = DerivationOutput::InputAddressed { - .path = std::move(outPath), - }; - } - break; - } - }, - [&](const CaOutputHashes &) { - // Shouldn't happen as the toplevel derivation is not CA. - assert(false); - }, - }, - hashModulo.raw()); - + auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); + switch (hashModulo.kind) { + case DrvHash::Kind::Regular: + for (auto & i : outputs) { + auto h = hashModulo.hashes.at(i); + auto outPath = state.store->makeOutputPath(i, h, drvName); + drv.env[i] = state.store->printStorePath(outPath); + drv.outputs.insert_or_assign( + i, + DerivationOutputInputAddressed { + .path = std::move(outPath), + }); + } + break; + ; + case DrvHash::Kind::Deferred: + for (auto & i : outputs) { + drv.outputs.insert_or_assign(i, DerivationOutputDeferred {}); + } + } } /* Write the resulting term into the Nix store directory. */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7fed80387..85d75523f 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -474,7 +474,7 @@ Sync drvHashes; /* Look up the derivation by value and memoize the `hashDerivationModulo` call. */ -static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & drvPath) +static const DrvHash pathDerivationModulo(Store & store, const StorePath & drvPath) { { auto hashes = drvHashes.lock(); @@ -509,7 +509,7 @@ static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & don't leak the provenance of fixed outputs, reducing pointless cache misses as the build itself won't know this. */ -DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) +DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { auto type = drv.type(); @@ -524,7 +524,10 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return outputHashes; + return DrvHash{ + .hashes = outputHashes, + .kind = DrvHash::Kind::Regular, + }; } auto kind = std::visit(overloaded { @@ -540,65 +543,36 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m }, }, drv.type().raw()); - /* For other derivations, replace the inputs paths with recursive - calls to this function. */ std::map inputs2; for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) { // Avoid lambda capture restriction with standard / Clang auto & inputOutputs = inputOutputs0; const auto & res = pathDerivationModulo(store, drvPath); - std::visit(overloaded { - // Regular non-CA derivation, replace derivation - [&](const DrvHash & drvHash) { - kind |= drvHash.kind; - inputs2.insert_or_assign(drvHash.hash.to_string(Base16, false), inputOutputs); - }, - // CA derivation's output hashes - [&](const CaOutputHashes & outputHashes) { - std::set justOut = { "out" }; - for (auto & output : inputOutputs) { - /* Put each one in with a single "out" output.. */ - const auto h = outputHashes.at(output); - inputs2.insert_or_assign( - h.to_string(Base16, false), - justOut); - } - }, - }, res.raw()); + if (res.kind == DrvHash::Kind::Deferred) + kind = DrvHash::Kind::Deferred; + for (auto & outputName : inputOutputs) { + const auto h = res.hashes.at(outputName); + inputs2[h.to_string(Base16, false)].insert(outputName); + } } auto hash = hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); - return DrvHash { .hash = hash, .kind = kind }; -} - - -void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept -{ - switch (other) { - case DrvHash::Kind::Regular: - break; - case DrvHash::Kind::Deferred: - self = other; - break; + std::map outputHashes; + for (const auto & [outputName, _] : drv.outputs) { + outputHashes.insert_or_assign(outputName, hash); } + + return DrvHash { + .hashes = outputHashes, + .kind = kind, + }; } std::map staticOutputHashes(Store & store, const Derivation & drv) { - std::map res; - std::visit(overloaded { - [&](const DrvHash & drvHash) { - for (auto & outputName : drv.outputNames()) { - res.insert({outputName, drvHash.hash}); - } - }, - [&](const CaOutputHashes & outputHashes) { - res = outputHashes; - }, - }, hashDerivationModulo(store, drv, true).raw()); - return res; + return hashDerivationModulo(store, drv, true).hashes; } @@ -747,7 +721,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.raw())) { - auto & h = hashModulo.requireNoFixedNonDeferred(); + auto & h = hashModulo.hashes.at(outputName); auto outPath = store.makeOutputPath(outputName, h, drv.name); drv.env[outputName] = store.printStorePath(outPath); output = DerivationOutput::InputAddressed { @@ -758,13 +732,6 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } -const Hash & DrvHashModulo::requireNoFixedNonDeferred() const { - auto * drvHashOpt = std::get_if(&raw()); - assert(drvHashOpt); - assert(drvHashOpt->kind == DrvHash::Kind::Regular); - return drvHashOpt->hash; -} - static bool tryResolveInput( Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, const StorePath & inputDrv, const StringSet & inputOutputs) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 8dea90abf..63ea5ef76 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -202,12 +202,14 @@ bool isDerivation(const std::string & fileName); the output name is "out". */ std::string outputPathName(std::string_view drvName, std::string_view outputName); -// known CA drv's output hashes, current just for fixed-output derivations -// whose output hashes are always known since they are fixed up-front. -typedef std::map CaOutputHashes; +// The hashes modulo of a derivation. +// +// Each output is given a hash, although in practice only the content-addressed +// derivations (fixed-output or not) will have a different hash for each +// output. struct DrvHash { - Hash hash; + std::map hashes; enum struct Kind: bool { // Statically determined derivations. @@ -222,28 +224,6 @@ struct DrvHash { void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; -typedef std::variant< - // Regular normalized derivation hash, and whether it was deferred (because - // an ancestor derivation is a floating content addressed derivation). - DrvHash, - // Fixed-output derivation hashes - CaOutputHashes -> _DrvHashModuloRaw; - -struct DrvHashModulo : _DrvHashModuloRaw { - using Raw = _DrvHashModuloRaw; - using Raw::Raw; - - /* Get hash, throwing if it is per-output CA hashes or a - deferred Drv hash. - */ - const Hash & requireNoFixedNonDeferred() const; - - inline const Raw & raw() const { - return static_cast(*this); - } -}; - /* Returns hashes with the details of fixed-output subderivations expunged. @@ -267,7 +247,7 @@ struct DrvHashModulo : _DrvHashModuloRaw { ATerm, after subderivations have been likewise expunged from that derivation. */ -DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); +DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs); /* Return a map associating each output to a hash that uniquely identifies its @@ -276,7 +256,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m std::map staticOutputHashes(Store& store, const Derivation& drv); /* Memoisation of hashDerivationModulo(). */ -typedef std::map DrvHashes; +typedef std::map DrvHashes; // FIXME: global, though at least thread-safe. extern Sync drvHashes; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 46a547db1..60fe53af1 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -695,16 +695,15 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat // combinations that are currently prohibited. drv.type(); - std::optional h; + std::optional hashesModulo; for (auto & i : drv.outputs) { std::visit(overloaded { [&](const DerivationOutput::InputAddressed & doia) { - if (!h) { + if (!hashesModulo) { // somewhat expensive so we do lazily - auto h0 = hashDerivationModulo(*this, drv, true); - h = h0.requireNoFixedNonDeferred(); + hashesModulo = hashDerivationModulo(*this, drv, true); } - StorePath recomputed = makeOutputPath(i.first, *h, drvName); + StorePath recomputed = makeOutputPath(i.first, hashesModulo->hashes.at(i.first), drvName); if (doia.path != recomputed) throw Error("derivation '%s' has incorrect output '%s', should be '%s'", printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index d2f9b5a6a..7fc74d34e 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -204,10 +204,10 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore output.second = DerivationOutput::Deferred { }; drv.env[output.first] = ""; } - auto h0 = hashDerivationModulo(*evalStore, drv, true); - const Hash & h = h0.requireNoFixedNonDeferred(); + auto hashesModulo = hashDerivationModulo(*evalStore, drv, true); for (auto & output : drv.outputs) { + Hash h = hashesModulo.hashes.at(output.first); auto outPath = store->makeOutputPath(output.first, h, drv.name); output.second = DerivationOutput::InputAddressed { .path = outPath, From 3b26dd51ff0d7b51ec90141bfe2d05b52a4ecfd4 Mon Sep 17 00:00:00 2001 From: Graham Christensen Date: Tue, 29 Mar 2022 21:05:57 -0400 Subject: [PATCH 48/98] nix-daemon.service: require mounts for /nix/var/nix/db Users may want to mount a filesystem just for the Nix database, with the filesystem's parameters specially tuned for sqlite. For example, on ZFS you might set the recordsize to 64k after changing the database's page size to 65536. --- misc/systemd/nix-daemon.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/misc/systemd/nix-daemon.service.in b/misc/systemd/nix-daemon.service.in index b4badf2ba..24d894898 100644 --- a/misc/systemd/nix-daemon.service.in +++ b/misc/systemd/nix-daemon.service.in @@ -3,6 +3,7 @@ Description=Nix Daemon Documentation=man:nix-daemon https://nixos.org/manual RequiresMountsFor=@storedir@ RequiresMountsFor=@localstatedir@ +RequiresMountsFor=@localstatedir@/nix/db ConditionPathIsReadWrite=@localstatedir@/nix/daemon-socket [Service] From 8dee15cd31f634183c32649e9e62e576e12e5cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 30 Mar 2022 11:42:47 +0200 Subject: [PATCH 49/98] =?UTF-8?q?Don=E2=80=99t=20create=20a=20file=20in=20?= =?UTF-8?q?the=20worktree=20in=20the=20fetchPath=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/fetchPath.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fetchPath.sh b/tests/fetchPath.sh index 8f17638e9..29be38ce2 100644 --- a/tests/fetchPath.sh +++ b/tests/fetchPath.sh @@ -1,6 +1,6 @@ source common.sh -touch foo -t 202211111111 +touch $TEST_ROOT/foo -t 202211111111 # We only check whether 2022-11-1* **:**:** is the last modified date since # `lastModified` is transformed into UTC in `builtins.fetchTarball`. -[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2022111.* ]] +[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$TEST_ROOT/foo\").lastModifiedDate")" =~ 2022111.* ]] From 87f867ef62e7c391955fb1ca3f78bfd532e6666b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 30 Mar 2022 11:43:08 +0200 Subject: [PATCH 50/98] Gitignore the generated systemd nix-daemon conf file --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 4b290425a..58e7377fb 100644 --- a/.gitignore +++ b/.gitignore @@ -90,6 +90,7 @@ perl/Makefile.config /misc/systemd/nix-daemon.service /misc/systemd/nix-daemon.socket +/misc/systemd/nix-daemon.conf /misc/upstart/nix-daemon.conf /src/resolve-system-dependencies/resolve-system-dependencies From fa83b865a2cfd21d7e63eedb206c1e07c8178965 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 30 Mar 2022 15:41:25 +0200 Subject: [PATCH 51/98] libexpr: Throw the correct error in toJSON BaseError::addTrace(...) returns a BaseError, but we want to throw a TypeError instead. Fixes #6336. --- src/libexpr/value-to-json.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libexpr/value-to-json.cc b/src/libexpr/value-to-json.cc index 517da4c01..7b35abca2 100644 --- a/src/libexpr/value-to-json.cc +++ b/src/libexpr/value-to-json.cc @@ -84,7 +84,8 @@ void printValueAsJSON(EvalState & state, bool strict, .msg = hintfmt("cannot convert %1% to JSON", showType(v)), .errPos = v.determinePos(pos) }); - throw e.addTrace(pos, hintfmt("message for the trace")); + e.addTrace(pos, hintfmt("message for the trace")); + throw e; } } From 629edd43ba7550be835660fe5df3b65cc4a515c7 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 30 Mar 2022 17:30:47 +0200 Subject: [PATCH 52/98] libutil: Change return value of addTrace to void The return value of BaseError::addTrace(...) is never used and error-prone as subclasses calling it will return a BaseError instead of the subclass. This commit changes its return value to be void. --- src/libutil/error.cc | 3 +-- src/libutil/error.hh | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index b2dfb35b2..02bc5caa5 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -9,10 +9,9 @@ namespace nix { const std::string nativeSystem = SYSTEM; -BaseError & BaseError::addTrace(std::optional e, hintformat hint) +void BaseError::addTrace(std::optional e, hintformat hint) { err.traces.push_front(Trace { .pos = e, .hint = hint }); - return *this; } // c++ std::exception descendants must have a 'const char* what()' function. diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 93b789f0b..6a757f9ad 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -175,12 +175,12 @@ public: const ErrorInfo & info() const { calcWhat(); return err; } template - BaseError & addTrace(std::optional e, const std::string & fs, const Args & ... args) + void addTrace(std::optional e, const std::string & fs, const Args & ... args) { - return addTrace(e, hintfmt(fs, args...)); + addTrace(e, hintfmt(fs, args...)); } - BaseError & addTrace(std::optional e, hintformat hint); + void addTrace(std::optional e, hintformat hint); bool hasTrace() const { return !err.traces.empty(); } }; From d77823b502ebc358d33a7719375677fed92291c8 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 30 Mar 2022 16:10:42 -0400 Subject: [PATCH 53/98] bundler: update default bundler to support new bundler API --- src/nix/bundle.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 7ed558dee..81fb8464a 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -9,7 +9,7 @@ using namespace nix; struct CmdBundle : InstallableCommand { - std::string bundler = "github:matthewbauer/nix-bundle"; + std::string bundler = "github:NixOS/bundlers"; std::optional outLink; CmdBundle() From 50f9f335c92a88e8c9bd3421e6befbcd06cac4ec Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 30 Mar 2022 16:35:26 -0400 Subject: [PATCH 54/98] profile!: consistent use of url/uri. create new version --- src/nix/profile.cc | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index f35947ddb..b151e48d6 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -97,19 +97,30 @@ struct ProfileManifest auto json = nlohmann::json::parse(readFile(manifestPath)); auto version = json.value("version", 0); - if (version != 1) - throw Error("profile manifest '%s' has unsupported version %d", manifestPath, version); + std::string sUrl; + std::string sOriginalUrl; + switch(version){ + case 1: + sUrl = "uri"; + sOriginalUrl = "originalUri"; + break; + case 2: + sUrl = "url"; + sOriginalUrl = "originalUrl"; + break; + default: + throw Error("profile manifest '%s' has unsupported version %d", manifestPath, version); + } for (auto & e : json["elements"]) { ProfileElement element; for (auto & p : e["storePaths"]) element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; - if (e.value("uri", "") != "") { - auto originalUrl = e.value("originalUrl", e["originalUri"]); + if (e.value(sUrl,"") != "") { element.source = ProfileElementSource{ - parseFlakeRef(originalUrl), - parseFlakeRef(e["uri"]), + parseFlakeRef(e[sOriginalUrl]), + parseFlakeRef(e[sUrl]), e["attrPath"] }; } @@ -144,13 +155,13 @@ struct ProfileManifest obj["active"] = element.active; if (element.source) { obj["originalUrl"] = element.source->originalRef.to_string(); - obj["uri"] = element.source->resolvedRef.to_string(); + obj["url"] = element.source->resolvedRef.to_string(); obj["attrPath"] = element.source->attrPath; } array.push_back(obj); } nlohmann::json json; - json["version"] = 1; + json["version"] = 2; json["elements"] = array; return json.dump(); } From 28309352d991f50c9d8b54a5a0ee99995a1a5297 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 10:39:53 +0200 Subject: [PATCH 55/98] replaceEnv(): Pass newEnv by reference --- src/libutil/util.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 70eaf4f9c..59e3aad6d 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -71,13 +71,11 @@ void clearEnv() unsetenv(name.first.c_str()); } -void replaceEnv(std::map newEnv) +void replaceEnv(const std::map & newEnv) { clearEnv(); - for (auto newEnvVar : newEnv) - { + for (auto & newEnvVar : newEnv) setenv(newEnvVar.first.c_str(), newEnvVar.second.c_str(), 1); - } } From 5cd72598feaff3c4bbcc7304a4844768f64a1ee0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 30 Mar 2022 16:31:01 +0200 Subject: [PATCH 56/98] Add support for impure derivations Impure derivations are derivations that can produce a different result every time they're built. Example: stdenv.mkDerivation { name = "impure"; __impure = true; # marks this derivation as impure outputHashAlgo = "sha256"; outputHashMode = "recursive"; buildCommand = "date > $out"; }; Some important characteristics: * This requires the 'impure-derivations' experimental feature. * Impure derivations are not "cached". Thus, running "nix-build" on the example above multiple times will cause a rebuild every time. * They are implemented similar to CA derivations, i.e. the output is moved to a content-addressed path in the store. The difference is that we don't register a realisation in the Nix database. * Pure derivations are not allowed to depend on impure derivations. In the future fixed-output derivations will be allowed to depend on impure derivations, thus forming an "impurity barrier" in the dependency graph. * When sandboxing is enabled, impure derivations can access the network in the same way as fixed-output derivations. In relaxed sandboxing mode, they can access the local filesystem. --- src/libexpr/eval.cc | 1 + src/libexpr/eval.hh | 2 +- src/libexpr/primops.cc | 32 ++- src/libstore/build/derivation-goal.cc | 182 +++++++++------ src/libstore/build/derivation-goal.hh | 7 + src/libstore/build/local-derivation-goal.cc | 21 +- src/libstore/derivations.cc | 235 +++++++++++++++----- src/libstore/derivations.hh | 52 ++++- src/libstore/local-store.cc | 3 + src/libstore/path.cc | 9 + src/libstore/path.hh | 2 + src/libutil/experimental-features.cc | 1 + src/libutil/experimental-features.hh | 1 + src/nix/show-derivation.cc | 4 + tests/impure-derivations.nix | 46 ++++ tests/impure-derivations.sh | 39 ++++ tests/local.mk | 3 +- 17 files changed, 486 insertions(+), 154 deletions(-) create mode 100644 tests/impure-derivations.nix create mode 100644 tests/impure-derivations.sh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 437c7fc53..b87e06ef5 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -436,6 +436,7 @@ EvalState::EvalState( , sBuilder(symbols.create("builder")) , sArgs(symbols.create("args")) , sContentAddressed(symbols.create("__contentAddressed")) + , sImpure(symbols.create("__impure")) , sOutputHash(symbols.create("outputHash")) , sOutputHashAlgo(symbols.create("outputHashAlgo")) , sOutputHashMode(symbols.create("outputHashMode")) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index e7915dd99..7ed376e8d 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -78,7 +78,7 @@ public: sSystem, sOverrides, sOutputs, sOutputName, sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, sRight, sWrong, sStructuredAttrs, sBuilder, sArgs, - sContentAddressed, + sContentAddressed, sImpure, sOutputHash, sOutputHashAlgo, sOutputHashMode, sRecurseForDerivations, sDescription, sSelf, sEpsilon, sStartSet, sOperator, sKey, sPath, diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9f549e52f..eaf04320e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -989,6 +989,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * PathSet context; bool contentAddressed = false; + bool isImpure = false; std::optional outputHash; std::string outputHashAlgo; auto ingestionMethod = FileIngestionMethod::Flat; @@ -1051,6 +1052,12 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * settings.requireExperimentalFeature(Xp::CaDerivations); } + else if (i->name == state.sImpure) { + isImpure = state.forceBool(*i->value, pos); + if (isImpure) + settings.requireExperimentalFeature(Xp::ImpureDerivations); + } + /* The `args' attribute is special: it supplies the command-line arguments to the builder. */ else if (i->name == state.sArgs) { @@ -1197,15 +1204,28 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * }); } - else if (contentAddressed) { + else if (contentAddressed || isImpure) { + if (contentAddressed && isImpure) + throw EvalError({ + .msg = hintfmt("derivation cannot be both content-addressed and impure"), + .errPos = posDrvName + }); + HashType ht = parseHashType(outputHashAlgo); for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); - drv.outputs.insert_or_assign(i, - DerivationOutput::CAFloating { - .method = ingestionMethod, - .hashType = ht, - }); + if (isImpure) + drv.outputs.insert_or_assign(i, + DerivationOutput::Impure { + .method = ingestionMethod, + .hashType = ht, + }); + else + drv.outputs.insert_or_assign(i, + DerivationOutput::CAFloating { + .method = ingestionMethod, + .hashType = ht, + }); } } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3d1c4fbc1..2f3490829 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -204,9 +204,34 @@ void DerivationGoal::haveDerivation() { trace("have derivation"); + parsedDrv = std::make_unique(drvPath, *drv); + if (!drv->type().hasKnownOutputPaths()) settings.requireExperimentalFeature(Xp::CaDerivations); + if (!drv->type().isPure()) { + settings.requireExperimentalFeature(Xp::ImpureDerivations); + + for (auto & [outputName, output] : drv->outputs) { + auto randomPath = StorePath::random(outputPathName(drv->name, outputName)); + assert(!worker.store.isValidPath(randomPath)); + initialOutputs.insert({ + outputName, + InitialOutput { + .wanted = true, + .outputHash = impureOutputHash, + .known = InitialOutputStatus { + .path = randomPath, + .status = PathStatus::Absent + } + } + }); + } + + gaveUpOnSubstitution(); + return; + } + for (auto & i : drv->outputsAndOptPaths(worker.store)) if (i.second.second) worker.store.addTempRoot(*i.second.second); @@ -230,9 +255,6 @@ void DerivationGoal::haveDerivation() return; } - parsedDrv = std::make_unique(drvPath, *drv); - - /* We are first going to try to create the invalid output paths through substitutes. If that doesn't work, we'll build them. */ @@ -266,6 +288,8 @@ void DerivationGoal::outputsSubstitutionTried() { trace("all outputs substituted (maybe)"); + assert(drv->type().isPure()); + if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", @@ -315,9 +339,21 @@ void DerivationGoal::outputsSubstitutionTried() void DerivationGoal::gaveUpOnSubstitution() { /* The inputs must be built before we can build this goal. */ + inputDrvOutputs.clear(); if (useDerivation) - for (auto & i : dynamic_cast(drv.get())->inputDrvs) + for (auto & i : dynamic_cast(drv.get())->inputDrvs) { + /* Ensure that pure derivations don't depend on impure + derivations. */ + if (drv->type().isPure()) { + auto inputDrv = worker.evalStore.readDerivation(i.first); + if (!inputDrv.type().isPure()) + throw Error("pure derivation '%s' depends on impure derivation '%s'", + worker.store.printStorePath(drvPath), + worker.store.printStorePath(i.first)); + } + addWaitee(worker.makeDerivationGoal(i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal)); + } /* Copy the input sources from the eval store to the build store. */ @@ -345,6 +381,8 @@ void DerivationGoal::gaveUpOnSubstitution() void DerivationGoal::repairClosure() { + assert(drv->type().isPure()); + /* If we're repairing, we now know that our own outputs are valid. Now check whether the other paths in the outputs closure are good. If not, then start derivation goals for the derivations @@ -452,22 +490,24 @@ void DerivationGoal::inputsRealised() drvs. */ : true); }, + [&](const DerivationType::Impure &) { + return true; + } }, drvType.raw()); - if (resolveDrv) - { + if (resolveDrv && !fullDrv.inputDrvs.empty()) { settings.requireExperimentalFeature(Xp::CaDerivations); /* We are be able to resolve this derivation based on the - now-known results of dependencies. If so, we become a stub goal - aliasing that resolved derivation goal */ - std::optional attempt = fullDrv.tryResolve(worker.store); + now-known results of dependencies. If so, we become a + stub goal aliasing that resolved derivation goal. */ + std::optional attempt = fullDrv.tryResolve(worker.store, inputDrvOutputs); assert(attempt); Derivation drvResolved { *std::move(attempt) }; auto pathResolved = writeDerivation(worker.store, drvResolved); - auto msg = fmt("Resolved derivation: '%s' -> '%s'", + auto msg = fmt("resolved derivation: '%s' -> '%s'", worker.store.printStorePath(drvPath), worker.store.printStorePath(pathResolved)); act = std::make_unique(*logger, lvlInfo, actBuildWaiting, msg, @@ -488,21 +528,13 @@ void DerivationGoal::inputsRealised() /* Add the relevant output closures of the input derivation `i' as input paths. Only add the closures of output paths that are specified as inputs. */ - assert(worker.evalStore.isValidPath(drvPath)); - auto outputs = worker.evalStore.queryPartialDerivationOutputMap(depDrvPath); - for (auto & j : wantedDepOutputs) { - if (outputs.count(j) > 0) { - auto optRealizedInput = outputs.at(j); - if (!optRealizedInput) - throw Error( - "derivation '%s' requires output '%s' from input derivation '%s', which is supposedly realized already, yet we still don't know what path corresponds to that output", - worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath)); - worker.store.computeFSClosure(*optRealizedInput, inputPaths); - } else + for (auto & j : wantedDepOutputs) + if (auto outPath = get(inputDrvOutputs, { depDrvPath, j })) + worker.store.computeFSClosure(*outPath, inputPaths); + else throw Error( "derivation '%s' requires non-existent output '%s' from input derivation '%s'", worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath)); - } } } @@ -923,7 +955,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - derivationType.isImpure() || diskFull ? BuildResult::TransientFailure : + derivationType.needsNetworkAccess() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -934,60 +966,52 @@ void DerivationGoal::buildDone() void DerivationGoal::resolvedFinished() { + trace("resolved derivation finished"); + assert(resolvedDrvGoal); auto resolvedDrv = *resolvedDrvGoal->drv; - - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); - - StorePathSet outputPaths; - - // `wantedOutputs` might be empty, which means “all the outputs” - auto realWantedOutputs = wantedOutputs; - if (realWantedOutputs.empty()) - realWantedOutputs = resolvedDrv.outputNames(); + auto & resolvedResult = resolvedDrvGoal->buildResult; DrvOutputs builtOutputs; - for (auto & wantedOutput : realWantedOutputs) { - assert(initialOutputs.count(wantedOutput) != 0); - assert(resolvedHashes.count(wantedOutput) != 0); - auto realisation = worker.store.queryRealisation( - DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} - ); - // We've just built it, but maybe the build failed, in which case the - // realisation won't be there - if (realisation) { - auto newRealisation = *realisation; - newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput}; - newRealisation.signatures.clear(); - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath); - signRealisation(newRealisation); - worker.store.registerDrvOutput(newRealisation); - outputPaths.insert(realisation->outPath); - builtOutputs.emplace(realisation->id, *realisation); - } else { - // If we don't have a realisation, then it must mean that something - // failed when building the resolved drv - assert(!buildResult.success()); + if (resolvedResult.success()) { + auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); + + StorePathSet outputPaths; + + // `wantedOutputs` might be empty, which means “all the outputs” + auto realWantedOutputs = wantedOutputs; + if (realWantedOutputs.empty()) + realWantedOutputs = resolvedDrv.outputNames(); + + for (auto & wantedOutput : realWantedOutputs) { + assert(initialOutputs.count(wantedOutput) != 0); + assert(resolvedHashes.count(wantedOutput) != 0); + auto realisation = resolvedResult.builtOutputs.at( + DrvOutput { resolvedHashes.at(wantedOutput), wantedOutput }); + if (drv->type().isPure()) { + auto newRealisation = realisation; + newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; + newRealisation.signatures.clear(); + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + signRealisation(newRealisation); + worker.store.registerDrvOutput(newRealisation); + } + outputPaths.insert(realisation.outPath); + builtOutputs.emplace(realisation.id, realisation); } + + runPostBuildHook( + worker.store, + *logger, + drvPath, + outputPaths + ); } - runPostBuildHook( - worker.store, - *logger, - drvPath, - outputPaths - ); - - auto status = [&]() { - auto & resolvedResult = resolvedDrvGoal->buildResult; - switch (resolvedResult.status) { - case BuildResult::AlreadyValid: - return BuildResult::ResolvesToAlreadyValid; - default: - return resolvedResult.status; - } - }(); + auto status = resolvedResult.status; + if (status == BuildResult::AlreadyValid) + status = BuildResult::ResolvesToAlreadyValid; done(status, std::move(builtOutputs)); } @@ -1236,6 +1260,7 @@ void DerivationGoal::flushLine() std::map> DerivationGoal::queryPartialDerivationOutputMap() { + assert(drv->type().isPure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { std::map> res; for (auto & [name, output] : drv->outputs) @@ -1248,6 +1273,7 @@ std::map> DerivationGoal::queryPartialDeri OutputPathMap DerivationGoal::queryDerivationOutputMap() { + assert(drv->type().isPure()); if (!useDerivation || drv->type().hasKnownOutputPaths()) { OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) @@ -1261,6 +1287,8 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() std::pair DerivationGoal::checkPathValidity() { + if (!drv->type().isPure()) return { false, {} }; + bool checkHash = buildMode == bmRepair; auto wantedOutputsLeft = wantedOutputs; DrvOutputs validOutputs; @@ -1304,6 +1332,7 @@ std::pair DerivationGoal::checkPathValidity() if (info.wanted && info.known && info.known->isValid()) validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); } + // If we requested all the outputs via the empty set, we are always fine. // If we requested specific elements, the loop above removes all the valid // ones, so any that are left must be invalid. @@ -1343,7 +1372,6 @@ void DerivationGoal::done( if (ex) // FIXME: strip: "error: " buildResult.errorMsg = ex->what(); - amDone(buildResult.success() ? ecSuccess : ecFailed, ex); if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; if (buildResult.status == BuildResult::PermanentFailure) @@ -1370,7 +1398,21 @@ void DerivationGoal::done( fs.open(traceBuiltOutputsFile, std::fstream::out); fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } + + amDone(buildResult.success() ? ecSuccess : ecFailed, ex); } +void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) +{ + Goal::waiteeDone(waitee, result); + + if (waitee->buildResult.success()) + if (auto bfd = std::get_if(&waitee->buildResult.path)) + for (auto & [output, realisation] : waitee->buildResult.builtOutputs) + inputDrvOutputs.insert_or_assign( + { bfd->drvPath, output.outputName }, + realisation.outPath); +} + } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index f556b6f25..2d8bfd592 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -57,6 +57,11 @@ struct DerivationGoal : public Goal them. */ StringSet wantedOutputs; + /* Mapping from input derivations + output names to actual store + paths. This is filled in by waiteeDone() as each dependency + finishes, before inputsRealised() is reached, */ + std::map, StorePath> inputDrvOutputs; + /* Whether additional wanted outputs have been added. */ bool needRestart = false; @@ -224,6 +229,8 @@ struct DerivationGoal : public Goal DrvOutputs builtOutputs = {}, std::optional ex = {}); + void waiteeDone(GoalPtr waitee, ExitCode result) override; + StorePathSet exportReferences(const StorePathSet & storePaths); }; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b176f318b..a6c07314f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !(derivationType.isImpure()) && !noChroot; + useChroot = !derivationType.needsNetworkAccess() && !noChroot; } auto & localStore = getLocalStore(); @@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (!(derivationType.isImpure())) + if (!derivationType.needsNetworkAccess()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -796,7 +796,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (!(derivationType.isImpure())) + if (!derivationType.needsNetworkAccess()) privateNetwork = true; userNamespaceSync.create(); @@ -1060,7 +1060,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationType.isImpure()) { + if (derivationType.needsNetworkAccess()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1674,7 +1674,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (derivationType.isImpure()) { + if (derivationType.needsNetworkAccess()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -2399,6 +2399,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs() assert(false); }, + [&](const DerivationOutput::Impure & doi) { + return newInfoFromCA(DerivationOutput::CAFloating { + .method = doi.method, + .hashType = doi.hashType, + }); + }, + }, output.raw()); /* FIXME: set proper permissions in restorePath() so @@ -2609,7 +2616,9 @@ DrvOutputs LocalDerivationGoal::registerOutputs() }, .outPath = newInfo.path }; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) + && drv->type().isPure()) + { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 85d75523f..75e0178bb 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -25,26 +25,42 @@ std::optional DerivationOutput::path(const Store & store, std::string [](const DerivationOutput::Deferred &) -> std::optional { return std::nullopt; }, + [](const DerivationOutput::Impure &) -> std::optional { + return std::nullopt; + }, }, raw()); } -StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const { +StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const +{ return store.makeFixedOutputPath( hash.method, hash.hash, outputPathName(drvName, outputName)); } -bool DerivationType::isCA() const { +bool DerivationType::isCA() const +{ /* Normally we do the full `std::visit` to make sure we have exhaustively handled all variants, but so long as there is a variant called `ContentAddressed`, it must be the only one for which `isCA` is true for this to make sense!. */ - return std::holds_alternative(raw()); + return std::visit(overloaded { + [](const InputAddressed & ia) { + return false; + }, + [](const ContentAddressed & ca) { + return true; + }, + [](const Impure &) { + return true; + }, + }, raw()); } -bool DerivationType::isFixed() const { +bool DerivationType::isFixed() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return false; @@ -52,10 +68,14 @@ bool DerivationType::isFixed() const { [](const ContentAddressed & ca) { return ca.fixed; }, + [](const Impure &) { + return false; + }, }, raw()); } -bool DerivationType::hasKnownOutputPaths() const { +bool DerivationType::hasKnownOutputPaths() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return !ia.deferred; @@ -63,11 +83,15 @@ bool DerivationType::hasKnownOutputPaths() const { [](const ContentAddressed & ca) { return ca.fixed; }, + [](const Impure &) { + return false; + }, }, raw()); } -bool DerivationType::isImpure() const { +bool DerivationType::needsNetworkAccess() const +{ return std::visit(overloaded { [](const InputAddressed & ia) { return false; @@ -75,6 +99,25 @@ bool DerivationType::isImpure() const { [](const ContentAddressed & ca) { return !ca.pure; }, + [](const Impure &) { + return true; + }, + }, raw()); +} + + +bool DerivationType::isPure() const +{ + return std::visit(overloaded { + [](const InputAddressed & ia) { + return true; + }, + [](const ContentAddressed & ca) { + return true; + }, + [](const Impure &) { + return false; + }, }, raw()); } @@ -176,7 +219,16 @@ static DerivationOutput parseDerivationOutput(const Store & store, hashAlgo = hashAlgo.substr(2); } const auto hashType = parseHashType(hashAlgo); - if (hash != "") { + if (hash == "impure") { + settings.requireExperimentalFeature(Xp::ImpureDerivations); + assert(pathS == ""); + return DerivationOutput { + .output = DerivationOutputImpure { + .method = std::move(method), + .hashType = std::move(hashType), + }, + }; + } else if (hash != "") { validatePath(pathS); return DerivationOutput::CAFixed { .hash = FixedOutputHash { @@ -345,6 +397,12 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs, s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); + }, + [&](const DerivationOutputImpure & doi) { + // FIXME + s += ','; printUnquotedString(s, ""); + s += ','; printUnquotedString(s, makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)); + s += ','; printUnquotedString(s, "impure"); } }, i.second.raw()); s += ')'; @@ -410,8 +468,14 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName DerivationType BasicDerivation::type() const { - std::set inputAddressedOutputs, fixedCAOutputs, floatingCAOutputs, deferredIAOutputs; + std::set + inputAddressedOutputs, + fixedCAOutputs, + floatingCAOutputs, + deferredIAOutputs, + impureOutputs; std::optional floatingHashType; + for (auto & i : outputs) { std::visit(overloaded { [&](const DerivationOutput::InputAddressed &) { @@ -426,43 +490,78 @@ DerivationType BasicDerivation::type() const floatingHashType = dof.hashType; } else { if (*floatingHashType != dof.hashType) - throw Error("All floating outputs must use the same hash type"); + throw Error("all floating outputs must use the same hash type"); } }, [&](const DerivationOutput::Deferred &) { - deferredIAOutputs.insert(i.first); + deferredIAOutputs.insert(i.first); + }, + [&](const DerivationOutput::Impure &) { + impureOutputs.insert(i.first); }, }, i.second.raw()); } - if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { - throw Error("Must have at least one output"); - } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) + throw Error("must have at least one output"); + + if (!inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::InputAddressed { .deferred = false, }; - } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + + if (inputAddressedOutputs.empty() + && !fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) + { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? - throw Error("Only one fixed output is allowed for now"); + throw Error("only one fixed output is allowed for now"); if (*fixedCAOutputs.begin() != "out") - throw Error("Single fixed output must be named \"out\""); + throw Error("single fixed output must be named \"out\""); return DerivationType::ContentAddressed { .pure = false, .fixed = true, }; - } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && ! floatingCAOutputs.empty() && deferredIAOutputs.empty()) { + } + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && !floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::ContentAddressed { .pure = true, .fixed = false, }; - } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && !deferredIAOutputs.empty()) { + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && !deferredIAOutputs.empty() + && impureOutputs.empty()) return DerivationType::InputAddressed { .deferred = true, }; - } else { - throw Error("Can't mix derivation output types"); - } + + if (inputAddressedOutputs.empty() + && fixedCAOutputs.empty() + && floatingCAOutputs.empty() + && deferredIAOutputs.empty() + && !impureOutputs.empty()) + return DerivationType::Impure { }; + + throw Error("can't mix derivation output types"); } @@ -524,12 +623,22 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut + store.printStorePath(dof.path(store, drv.name, i.first))); outputHashes.insert_or_assign(i.first, std::move(hash)); } - return DrvHash{ + return DrvHash { .hashes = outputHashes, .kind = DrvHash::Kind::Regular, }; } + if (!type.isPure()) { + std::map outputHashes; + for (const auto & [outputName, _] : drv.outputs) + outputHashes.insert_or_assign(outputName, impureOutputHash); + return DrvHash { + .hashes = outputHashes, + .kind = DrvHash::Kind::Deferred, + }; + } + auto kind = std::visit(overloaded { [](const DerivationType::InputAddressed & ia) { /* This might be a "pesimistically" deferred output, so we don't @@ -541,6 +650,9 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut ? DrvHash::Kind::Regular : DrvHash::Kind::Deferred; }, + [](const DerivationType::Impure &) -> DrvHash::Kind { + assert(false); + } }, drv.type().raw()); std::map inputs2; @@ -599,7 +711,8 @@ StringSet BasicDerivation::outputNames() const return names; } -DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const { +DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & store) const +{ DerivationOutputsAndOptPaths outsAndOptPaths; for (auto output : outputs) outsAndOptPaths.insert(std::make_pair( @@ -610,7 +723,8 @@ DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const Store & s return outsAndOptPaths; } -std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) { +std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) +{ auto nameWithSuffix = drvPath.name(); constexpr std::string_view extension = ".drv"; assert(hasSuffix(nameWithSuffix, extension)); @@ -672,6 +786,11 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr << "" << ""; }, + [&](const DerivationOutput::Impure & doi) { + out << "" + << (makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)) + << "impure"; + }, }, i.second.raw()); } worker_proto::write(store, out, drv.inputSrcs); @@ -697,21 +816,19 @@ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath } -static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) { - - debug("Rewriting the derivation"); - - for (auto &rewrite: rewrites) { +static void rewriteDerivation(Store & store, BasicDerivation & drv, const StringMap & rewrites) +{ + for (auto & rewrite : rewrites) { debug("rewriting %s as %s", rewrite.first, rewrite.second); } drv.builder = rewriteStrings(drv.builder, rewrites); - for (auto & arg: drv.args) { + for (auto & arg : drv.args) { arg = rewriteStrings(arg, rewrites); } StringPairs newEnv; - for (auto & envVar: drv.env) { + for (auto & envVar : drv.env) { auto envName = rewriteStrings(envVar.first, rewrites); auto envValue = rewriteStrings(envVar.second, rewrites); newEnv.emplace(envName, envValue); @@ -732,48 +849,48 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } -static bool tryResolveInput( - Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, - const StorePath & inputDrv, const StringSet & inputOutputs) +std::optional Derivation::tryResolve(Store & store) const { - auto inputDrvOutputs = store.queryPartialDerivationOutputMap(inputDrv); + std::map, StorePath> inputDrvOutputs; - auto getOutput = [&](const std::string & outputName) { - auto & actualPathOpt = inputDrvOutputs.at(outputName); - if (!actualPathOpt) - warn("output %s of input %s missing, aborting the resolving", - outputName, - store.printStorePath(inputDrv) - ); - return actualPathOpt; - }; + for (auto & input : inputDrvs) + for (auto & [outputName, outputPath] : store.queryPartialDerivationOutputMap(input.first)) + if (outputPath) + inputDrvOutputs.insert_or_assign({input.first, outputName}, *outputPath); - for (auto & outputName : inputOutputs) { - auto actualPathOpt = getOutput(outputName); - if (!actualPathOpt) return false; - auto actualPath = *actualPathOpt; - inputRewrites.emplace( - downstreamPlaceholder(store, inputDrv, outputName), - store.printStorePath(actualPath)); - inputSrcs.insert(std::move(actualPath)); - } - - return true; + return tryResolve(store, inputDrvOutputs); } -std::optional Derivation::tryResolve(Store & store) { +std::optional Derivation::tryResolve( + Store & store, + const std::map, StorePath> & inputDrvOutputs) const +{ BasicDerivation resolved { *this }; // Input paths that we'll want to rewrite in the derivation StringMap inputRewrites; - for (auto & [inputDrv, inputOutputs] : inputDrvs) - if (!tryResolveInput(store, resolved.inputSrcs, inputRewrites, inputDrv, inputOutputs)) - return std::nullopt; + for (auto & [inputDrv, inputOutputs] : inputDrvs) { + for (auto & outputName : inputOutputs) { + if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) { + inputRewrites.emplace( + downstreamPlaceholder(store, inputDrv, outputName), + store.printStorePath(*actualPath)); + resolved.inputSrcs.insert(*actualPath); + } else { + warn("output '%s' of input '%s' missing, aborting the resolving", + outputName, + store.printStorePath(inputDrv)); + return {}; + } + } + } rewriteDerivation(store, resolved, inputRewrites); return resolved; } +const Hash impureOutputHash = hashString(htSHA256, "impure"); + } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 63ea5ef76..b62e40786 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -41,15 +41,26 @@ struct DerivationOutputCAFloating }; /* Input-addressed output which depends on a (CA) derivation whose hash isn't - * known atm + * known yet. */ struct DerivationOutputDeferred {}; +/* Impure output which is moved to a content-addressed location (like + CAFloating) but isn't registered as a realization. + */ +struct DerivationOutputImpure +{ + /* information used for expected hash computation */ + FileIngestionMethod method; + HashType hashType; +}; + typedef std::variant< DerivationOutputInputAddressed, DerivationOutputCAFixed, DerivationOutputCAFloating, - DerivationOutputDeferred + DerivationOutputDeferred, + DerivationOutputImpure > _DerivationOutputRaw; struct DerivationOutput : _DerivationOutputRaw @@ -61,6 +72,7 @@ struct DerivationOutput : _DerivationOutputRaw using CAFixed = DerivationOutputCAFixed; using CAFloating = DerivationOutputCAFloating; using Deferred = DerivationOutputDeferred; + using Impure = DerivationOutputImpure; /* Note, when you use this function you should make sure that you're passing the right derivation name. When in doubt, you should use the safer @@ -94,9 +106,13 @@ struct DerivationType_ContentAddressed { bool fixed; }; +struct DerivationType_Impure { +}; + typedef std::variant< DerivationType_InputAddressed, - DerivationType_ContentAddressed + DerivationType_ContentAddressed, + DerivationType_Impure > _DerivationTypeRaw; struct DerivationType : _DerivationTypeRaw { @@ -104,7 +120,7 @@ struct DerivationType : _DerivationTypeRaw { using Raw::Raw; using InputAddressed = DerivationType_InputAddressed; using ContentAddressed = DerivationType_ContentAddressed; - + using Impure = DerivationType_Impure; /* Do the outputs of the derivation have paths calculated from their content, or from the derivation itself? */ @@ -114,10 +130,13 @@ struct DerivationType : _DerivationTypeRaw { non-CA derivations. */ bool isFixed() const; - /* Is the derivation impure and needs to access non-deterministic resources, or - pure and can be sandboxed? Note that whether or not we actually sandbox the - derivation is controlled separately. Never true for non-CA derivations. */ - bool isImpure() const; + /* Whether the derivation needs to access the network. Note that + whether or not we actually sandbox the derivation is controlled + separately. Never true for non-CA derivations. */ + bool needsNetworkAccess() const; + + /* FIXME */ + bool isPure() const; /* Does the derivation knows its own output paths? Only true when there's no floating-ca derivation involved in the @@ -173,7 +192,14 @@ struct Derivation : BasicDerivation added directly to input sources. 2. Input placeholders are replaced with realized input store paths. */ - std::optional tryResolve(Store & store); + std::optional tryResolve(Store & store) const; + + /* Like the above, but instead of querying the Nix database for + realisations, uses a given mapping from input derivation paths + + output names to actual output store paths. */ + std::optional tryResolve( + Store & store, + const std::map, StorePath> & inputDrvOutputs) const; Derivation() = default; Derivation(const BasicDerivation & bd) : BasicDerivation(bd) { } @@ -211,7 +237,7 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName struct DrvHash { std::map hashes; - enum struct Kind: bool { + enum struct Kind : bool { // Statically determined derivations. // This hash will be directly used to compute the output paths Regular, @@ -252,8 +278,10 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut /* Return a map associating each output to a hash that uniquely identifies its derivation (modulo the self-references). + + FIXME: what is the Hash in this map? */ -std::map staticOutputHashes(Store& store, const Derivation& drv); +std::map staticOutputHashes(Store & store, const Derivation & drv); /* Memoisation of hashDerivationModulo(). */ typedef std::map DrvHashes; @@ -286,4 +314,6 @@ std::string hashPlaceholder(const std::string_view outputName); dependency which is a CA derivation. */ std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName); +extern const Hash impureOutputHash; + } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 60fe53af1..d77fff963 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -719,6 +719,9 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat [&](const DerivationOutput::Deferred &) { /* Nothing to check */ }, + [&](const DerivationOutput::Impure &) { + /* Nothing to check */ + }, }, i.second.raw()); } } diff --git a/src/libstore/path.cc b/src/libstore/path.cc index e642abcd5..392db225e 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -1,5 +1,7 @@ #include "store-api.hh" +#include + namespace nix { static void checkName(std::string_view path, std::string_view name) @@ -41,6 +43,13 @@ bool StorePath::isDerivation() const StorePath StorePath::dummy("ffffffffffffffffffffffffffffffff-x"); +StorePath StorePath::random(std::string_view name) +{ + Hash hash(htSHA1); + randombytes_buf(hash.hash, hash.hashSize); + return StorePath(hash, name); +} + StorePath Store::parseStorePath(std::string_view path) const { auto p = canonPath(std::string(path)); diff --git a/src/libstore/path.hh b/src/libstore/path.hh index e65fee622..77fd0f8dc 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -58,6 +58,8 @@ public: } static StorePath dummy; + + static StorePath random(std::string_view name); }; typedef std::set StorePathSet; diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index 01f318fa3..e033a4116 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -7,6 +7,7 @@ namespace nix { std::map stringifiedXpFeatures = { { Xp::CaDerivations, "ca-derivations" }, + { Xp::ImpureDerivations, "impure-derivations" }, { Xp::Flakes, "flakes" }, { Xp::NixCommand, "nix-command" }, { Xp::RecursiveNix, "recursive-nix" }, diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index b5140dcfe..3a254b423 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -16,6 +16,7 @@ namespace nix { enum struct ExperimentalFeature { CaDerivations, + ImpureDerivations, Flakes, NixCommand, RecursiveNix, diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 0d9655732..fb46b4dbf 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -77,6 +77,10 @@ struct CmdShowDerivation : InstallablesCommand outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, [&](const DerivationOutput::Deferred &) {}, + [&](const DerivationOutput::Impure & doi) { + outputObj.attr("hashAlgo", makeFileIngestionPrefix(doi.method) + printHashType(doi.hashType)); + outputObj.attr("impure", true); + }, }, output.raw()); } } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix new file mode 100644 index 000000000..ba7d53146 --- /dev/null +++ b/tests/impure-derivations.nix @@ -0,0 +1,46 @@ +with import ./config.nix; + +rec { + + impure = mkDerivation { + name = "impure"; + outputs = [ "out" "stuff" ]; + buildCommand = + '' + x=$(< $TEST_ROOT/counter) + mkdir $out $stuff + echo $x > $out/n + ln -s $out/n $stuff/bla + printf $((x + 1)) > $TEST_ROOT/counter + ''; + __impure = true; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + impureEnvVars = [ "TEST_ROOT" ]; + }; + + impureOnImpure = mkDerivation { + name = "impure-on-impure"; + buildCommand = + '' + x=$(< ${impure}/n) + mkdir $out + printf X$x > $out/n + ln -s ${impure.stuff} $out/symlink + ln -s $out $out/self + ''; + __impure = true; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + }; + + # This is not allowed. + inputAddressed = mkDerivation { + name = "input-addressed"; + buildCommand = + '' + cat ${impure} > $out + ''; + }; + +} diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh new file mode 100644 index 000000000..cb1eaf84a --- /dev/null +++ b/tests/impure-derivations.sh @@ -0,0 +1,39 @@ +source common.sh + +requireDaemonNewerThan "2.8pre20220311" + +enableFeatures "ca-derivations ca-references impure-derivations" + +clearStore + +# Basic test of impure derivations: building one a second time should not use the previous result. +printf 0 > $TEST_ROOT/counter + +json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure) +path1=$(echo $json | jq -r .[].outputs.out) +path1_stuff=$(echo $json | jq -r .[].outputs.stuff) +[[ $(< $path1/n) = 0 ]] +[[ $(< $path1_stuff/bla) = 0 ]] + +[[ $(nix path-info --json $path1 | jq .[].ca) =~ fixed:r:sha256: ]] + +path2=$(nix build -L --no-link --json --file ./impure-derivations.nix impure | jq -r .[].outputs.out) +[[ $(< $path2/n) = 1 ]] + +# Test impure derivations that depend on impure derivations. +path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +[[ $(< $path3/n) = X2 ]] + +path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +[[ $(< $path4/n) = X3 ]] + +# Test that (self-)references work. +[[ $(< $path4/symlink/bla) = 3 ]] +[[ $(< $path4/self/n) = X3 ]] + +# Input-addressed derivations cannot depend on impure derivations directly. +nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1 | grep 'depends on impure derivation' + +drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) +[[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] +[[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.stuff.impure") = true ]] diff --git a/tests/local.mk b/tests/local.mk index 97971dd76..668b34500 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -97,7 +97,8 @@ nix_tests = \ nix-profile.sh \ suggestions.sh \ store-ping.sh \ - fetchClosure.sh + fetchClosure.sh \ + impure-derivations.sh ifeq ($(HAVE_LIBCPUID), 1) nix_tests += compute-levels.sh From 18935e8b9f152f18705e738d4b8cc6fce97c8b02 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Mar 2022 13:23:23 +0100 Subject: [PATCH 57/98] Support fixed-output derivations depending on impure derivations --- src/libstore/build/derivation-goal.cc | 9 +++++---- src/libstore/misc.cc | 7 ++++--- tests/impure-derivations.nix | 22 ++++++++++++++++++++++ tests/impure-derivations.sh | 19 +++++++++++++++++-- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 2f3490829..542a6f6ea 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -342,9 +342,9 @@ void DerivationGoal::gaveUpOnSubstitution() inputDrvOutputs.clear(); if (useDerivation) for (auto & i : dynamic_cast(drv.get())->inputDrvs) { - /* Ensure that pure derivations don't depend on impure - derivations. */ - if (drv->type().isPure()) { + /* Ensure that pure, non-fixed-output derivations don't + depend on impure derivations. */ + if (drv->type().isPure() && !drv->type().isFixed()) { auto inputDrv = worker.evalStore.readDerivation(i.first); if (!inputDrv.type().isPure()) throw Error("pure derivation '%s' depends on impure derivation '%s'", @@ -993,7 +993,8 @@ void DerivationGoal::resolvedFinished() auto newRealisation = realisation; newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput }; newRealisation.signatures.clear(); - newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); + if (!drv->type().isFixed()) + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 1f0bae7fe..2bbd7aa70 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -277,15 +277,15 @@ std::map drvOutputReferences( { std::set inputRealisations; - for (const auto& [inputDrv, outputNames] : drv.inputDrvs) { + for (const auto & [inputDrv, outputNames] : drv.inputDrvs) { auto outputHashes = staticOutputHashes(store, store.readDerivation(inputDrv)); - for (const auto& outputName : outputNames) { + for (const auto & outputName : outputNames) { auto thisRealisation = store.queryRealisation( DrvOutput{outputHashes.at(outputName), outputName}); if (!thisRealisation) throw Error( - "output '%s' of derivation '%s' isn’t built", outputName, + "output '%s' of derivation '%s' isn't built", outputName, store.printStorePath(inputDrv)); inputRealisations.insert(*thisRealisation); } @@ -295,4 +295,5 @@ std::map drvOutputReferences( return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); } + } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix index ba7d53146..2fed56fe7 100644 --- a/tests/impure-derivations.nix +++ b/tests/impure-derivations.nix @@ -7,6 +7,7 @@ rec { outputs = [ "out" "stuff" ]; buildCommand = '' + echo impure x=$(< $TEST_ROOT/counter) mkdir $out $stuff echo $x > $out/n @@ -23,6 +24,7 @@ rec { name = "impure-on-impure"; buildCommand = '' + echo impure-on-impure x=$(< ${impure}/n) mkdir $out printf X$x > $out/n @@ -43,4 +45,24 @@ rec { ''; }; + contentAddressed = mkDerivation { + name = "content-addressed"; + buildCommand = + '' + echo content-addressed + x=$(< ${impureOnImpure}/n) + printf ''${x:0:1} > $out + ''; + outputHashAlgo = "sha256"; + outputHashMode = "recursive"; + outputHash = "sha256-eBYxcgkuWuiqs4cKNgKwkb3vY/HR0vVsJnqe8itJGcQ="; + }; + + inputAddressedAfterCA = mkDerivation { + name = "input-addressed-after-ca"; + buildCommand = + '' + cat ${contentAddressed} > $out + ''; + }; } diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index cb1eaf84a..85e3e09cc 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -21,10 +21,10 @@ path2=$(nix build -L --no-link --json --file ./impure-derivations.nix impure | j [[ $(< $path2/n) = 1 ]] # Test impure derivations that depend on impure derivations. -path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +path3=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) [[ $(< $path3/n) = X2 ]] -path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure -vvvvv | jq -r .[].outputs.out) +path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnImpure | jq -r .[].outputs.out) [[ $(< $path4/n) = X3 ]] # Test that (self-)references work. @@ -37,3 +37,18 @@ nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>& drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.stuff.impure") = true ]] + +# Fixed-output derivations *can* depend on impure derivations. +path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) +[[ $(< $path5) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] + +# And they should not be rebuilt. +path5=$(nix build -L --no-link --json --file ./impure-derivations.nix contentAddressed | jq -r .[].outputs.out) +[[ $(< $path5) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] + +# Input-addressed derivations can depend on fixed-output derivations that depend on impure derivations. +path6=$(nix build -L --no-link --json --file ./impure-derivations.nix inputAddressedAfterCA | jq -r .[].outputs.out) +[[ $(< $path6) = X ]] +[[ $(< $TEST_ROOT/counter) = 5 ]] From b2ae922747adfe187d02c1bfca8231c2d8bceb75 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Mar 2022 15:56:22 +0100 Subject: [PATCH 58/98] tests/impure-derivations.sh: Restart daemon --- tests/impure-derivations.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index 85e3e09cc..1b33a785c 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -3,6 +3,7 @@ source common.sh requireDaemonNewerThan "2.8pre20220311" enableFeatures "ca-derivations ca-references impure-derivations" +restartDaemon clearStore From 162beb25955adcedeed76e97510feb577d4f86db Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 30 Mar 2022 17:01:32 +0200 Subject: [PATCH 59/98] Fix test --- tests/impure-derivations.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index 1b33a785c..aab5cb61a 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -10,7 +10,7 @@ clearStore # Basic test of impure derivations: building one a second time should not use the previous result. printf 0 > $TEST_ROOT/counter -json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure) +json=$(nix build -L --no-link --json --file ./impure-derivations.nix impure.all) path1=$(echo $json | jq -r .[].outputs.out) path1_stuff=$(echo $json | jq -r .[].outputs.stuff) [[ $(< $path1/n) = 0 ]] From d7fc33c8426768040b322a060b5a50433b3a78e6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 15:59:14 +0200 Subject: [PATCH 60/98] Fix macOS build --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a6c07314f..108c661c0 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1918,7 +1918,7 @@ void LocalDerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (derivationType.isImpure()) + if (derivationType.needsNetworkAccess()) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Add the output paths we'll use at build-time to the chroot */ From 4e043c2f32af3d3d49c53a2d06746e2fd6967836 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:01:50 +0200 Subject: [PATCH 61/98] Document isPure() --- src/libstore/derivations.hh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index b62e40786..98e59b64e 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -135,7 +135,10 @@ struct DerivationType : _DerivationTypeRaw { separately. Never true for non-CA derivations. */ bool needsNetworkAccess() const; - /* FIXME */ + /* Whether the derivation is expected to produce the same result + every time, and therefore it only needs to be built once. This + is only false for derivations that have the attribute '__impure + = true'. */ bool isPure() const; /* Does the derivation knows its own output paths? From e279fbb16a99896101006ec00277a5d9d50f5040 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:06:40 +0200 Subject: [PATCH 62/98] needsNetworkAccess() -> isSandboxed() --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 12 ++++++------ src/libstore/derivations.cc | 8 ++++---- src/libstore/derivations.hh | 10 ++++++---- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 542a6f6ea..6582497bd 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -955,7 +955,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - derivationType.needsNetworkAccess() || diskFull ? BuildResult::TransientFailure : + !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 108c661c0..40ef706a6 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !derivationType.needsNetworkAccess() && !noChroot; + useChroot = derivationType.isSandboxed() && !noChroot; } auto & localStore = getLocalStore(); @@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (!derivationType.needsNetworkAccess()) + if (derivationType.isSandboxed()) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -796,7 +796,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (!derivationType.needsNetworkAccess()) + if (derivationType.isSandboxed()) privateNetwork = true; userNamespaceSync.create(); @@ -1060,7 +1060,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationType.needsNetworkAccess()) { + if (!derivationType.isSandboxed()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1674,7 +1674,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (derivationType.needsNetworkAccess()) { + if (!derivationType.isSandboxed()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -1918,7 +1918,7 @@ void LocalDerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (derivationType.needsNetworkAccess()) + if (!derivationType.isSandboxed()) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Add the output paths we'll use at build-time to the chroot */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 75e0178bb..b4fb77f9f 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -90,17 +90,17 @@ bool DerivationType::hasKnownOutputPaths() const } -bool DerivationType::needsNetworkAccess() const +bool DerivationType::isSandboxed() const { return std::visit(overloaded { [](const InputAddressed & ia) { - return false; + return true; }, [](const ContentAddressed & ca) { - return !ca.pure; + return ca.pure; }, [](const Impure &) { - return true; + return false; }, }, raw()); } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 98e59b64e..489948c30 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -130,10 +130,12 @@ struct DerivationType : _DerivationTypeRaw { non-CA derivations. */ bool isFixed() const; - /* Whether the derivation needs to access the network. Note that - whether or not we actually sandbox the derivation is controlled - separately. Never true for non-CA derivations. */ - bool needsNetworkAccess() const; + /* Whether the derivation is fully sandboxed. If false, the + sandbox is opened up, e.g. the derivation has access to the + network. Note that whether or not we actually sandbox the + derivation is controlled separately. Always true for non-CA + derivations. */ + bool isSandboxed() const; /* Whether the derivation is expected to produce the same result every time, and therefore it only needs to be built once. This From 6051cc954b990fa57a6d3b75bd4b0aaaceb0ca82 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:12:25 +0200 Subject: [PATCH 63/98] Rename 'pure' -> 'sandboxed' for consistency --- src/libstore/derivations.cc | 6 +++--- src/libstore/derivations.hh | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b4fb77f9f..cc04119c3 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -97,7 +97,7 @@ bool DerivationType::isSandboxed() const return true; }, [](const ContentAddressed & ca) { - return ca.pure; + return ca.sandboxed; }, [](const Impure &) { return false; @@ -530,7 +530,7 @@ DerivationType BasicDerivation::type() const if (*fixedCAOutputs.begin() != "out") throw Error("single fixed output must be named \"out\""); return DerivationType::ContentAddressed { - .pure = false, + .sandboxed = false, .fixed = true, }; } @@ -541,7 +541,7 @@ DerivationType BasicDerivation::type() const && deferredIAOutputs.empty() && impureOutputs.empty()) return DerivationType::ContentAddressed { - .pure = true, + .sandboxed = true, .fixed = false, }; diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 489948c30..af198a767 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -102,7 +102,7 @@ struct DerivationType_InputAddressed { }; struct DerivationType_ContentAddressed { - bool pure; + bool sandboxed; bool fixed; }; From a99af85a770df462985b621c4c3dd710b8487f44 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:39:18 +0200 Subject: [PATCH 64/98] Fix macOS build --- src/libstore/derivations.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index cc04119c3..1c695de82 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -222,11 +222,9 @@ static DerivationOutput parseDerivationOutput(const Store & store, if (hash == "impure") { settings.requireExperimentalFeature(Xp::ImpureDerivations); assert(pathS == ""); - return DerivationOutput { - .output = DerivationOutputImpure { - .method = std::move(method), - .hashType = std::move(hashType), - }, + return DerivationOutput::Impure { + .method = std::move(method), + .hashType = std::move(hashType), }; } else if (hash != "") { validatePath(pathS); From 75370972847a1b992055085f39b38f1f659e5275 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 16:56:44 +0200 Subject: [PATCH 65/98] Provide default values for outputHashAlgo and outputHashMode --- src/libexpr/primops.cc | 19 +++++++++++-------- tests/impure-derivations.nix | 5 ----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index eaf04320e..969391725 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -991,8 +991,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * bool contentAddressed = false; bool isImpure = false; std::optional outputHash; - std::string outputHashAlgo; - auto ingestionMethod = FileIngestionMethod::Flat; + std::optional outputHashAlgo; + std::optional ingestionMethod; StringSet outputs; outputs.insert("out"); @@ -1190,15 +1190,16 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - std::optional ht = parseHashTypeOpt(outputHashAlgo); + std::optional ht = parseHashTypeOpt(outputHashAlgo.value_or("sha256")); Hash h = newHashAllowEmpty(*outputHash, ht); - auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); + auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); + auto outPath = state.store->makeFixedOutputPath(method, h, drvName); drv.env["out"] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign("out", DerivationOutput::CAFixed { .hash = FixedOutputHash { - .method = ingestionMethod, + .method = method, .hash = std::move(h), }, }); @@ -1211,19 +1212,21 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - HashType ht = parseHashType(outputHashAlgo); + auto ht = parseHashType(outputHashAlgo.value_or("sha256")); + auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); + for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); if (isImpure) drv.outputs.insert_or_assign(i, DerivationOutput::Impure { - .method = ingestionMethod, + .method = method, .hashType = ht, }); else drv.outputs.insert_or_assign(i, DerivationOutput::CAFloating { - .method = ingestionMethod, + .method = method, .hashType = ht, }); } diff --git a/tests/impure-derivations.nix b/tests/impure-derivations.nix index 2fed56fe7..98547e6c1 100644 --- a/tests/impure-derivations.nix +++ b/tests/impure-derivations.nix @@ -15,8 +15,6 @@ rec { printf $((x + 1)) > $TEST_ROOT/counter ''; __impure = true; - outputHashAlgo = "sha256"; - outputHashMode = "recursive"; impureEnvVars = [ "TEST_ROOT" ]; }; @@ -32,8 +30,6 @@ rec { ln -s $out $out/self ''; __impure = true; - outputHashAlgo = "sha256"; - outputHashMode = "recursive"; }; # This is not allowed. @@ -53,7 +49,6 @@ rec { x=$(< ${impureOnImpure}/n) printf ''${x:0:1} > $out ''; - outputHashAlgo = "sha256"; outputHashMode = "recursive"; outputHash = "sha256-eBYxcgkuWuiqs4cKNgKwkb3vY/HR0vVsJnqe8itJGcQ="; }; From d63a5f5dd3b47e629295eb68264b4a6aadc65aa7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 17:33:06 +0200 Subject: [PATCH 66/98] Update release notes --- doc/manual/src/release-notes/rl-next.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 2ec864ee4..4f3c9ce41 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -14,3 +14,21 @@ This function is only available if you enable the experimental feature `fetch-closure`. + +* New experimental feature: *impure derivations*. These are + derivations that can produce a different result every time they're + built. Here is an example: + + ```nix + stdenv.mkDerivation { + name = "impure"; + __impure = true; # marks this derivation as impure + buildCommand = "date > $out"; + } + ``` + + Running `nix build` twice on this expression will build the + derivation twice, producing two different content-addressed store + paths. Like fixed-output derivations, impure derivations have access + to the network. Only fixed-output derivations and impure derivations + can depend on an impure derivation. From 6377442c983f4399d0a997238b898298e349fc1b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 31 Mar 2022 17:38:15 +0200 Subject: [PATCH 67/98] tests/impure-derivations.sh: Ensure that inputAddressed build fails --- tests/impure-derivations.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/impure-derivations.sh b/tests/impure-derivations.sh index aab5cb61a..35ae3f5d3 100644 --- a/tests/impure-derivations.sh +++ b/tests/impure-derivations.sh @@ -5,6 +5,8 @@ requireDaemonNewerThan "2.8pre20220311" enableFeatures "ca-derivations ca-references impure-derivations" restartDaemon +set -o pipefail + clearStore # Basic test of impure derivations: building one a second time should not use the previous result. @@ -33,7 +35,7 @@ path4=$(nix build -L --no-link --json --file ./impure-derivations.nix impureOnIm [[ $(< $path4/self/n) = X3 ]] # Input-addressed derivations cannot depend on impure derivations directly. -nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1 | grep 'depends on impure derivation' +(! nix build -L --no-link --json --file ./impure-derivations.nix inputAddressed 2>&1) | grep 'depends on impure derivation' drvPath=$(nix eval --json --file ./impure-derivations.nix impure.drvPath | jq -r .) [[ $(nix show-derivation $drvPath | jq ".[\"$drvPath\"].outputs.out.impure") = true ]] From 7492030ed7d1d570b71e832b610150054a9f095b Mon Sep 17 00:00:00 2001 From: Artturin Date: Thu, 31 Mar 2022 22:14:53 +0300 Subject: [PATCH 68/98] scripts/install-systemd-multi-user.sh: fix another typo --- scripts/install-systemd-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index 1d92c5388..62397127a 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -90,7 +90,7 @@ poly_configure_nix_daemon_service() { ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST _sudo "to run systemd-tmpfiles once to pick that path up" \ - systemd-tmpfiles create --prefix=/nix/var/nix + systemd-tmpfiles --create --prefix=/nix/var/nix _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From fdfe737867920ed0ec29ba8ffb7d19854d8658bb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 1 Apr 2022 12:40:49 +0200 Subject: [PATCH 69/98] Fix handling of outputHash when outputHashAlgo is not specified https://hydra.nixos.org/build/171351131 --- src/libexpr/primops.cc | 7 +++---- tests/ca/content-addressed.nix | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 969391725..73817dbdd 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -991,7 +991,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * bool contentAddressed = false; bool isImpure = false; std::optional outputHash; - std::optional outputHashAlgo; + std::string outputHashAlgo; std::optional ingestionMethod; StringSet outputs; @@ -1190,8 +1190,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - std::optional ht = parseHashTypeOpt(outputHashAlgo.value_or("sha256")); - Hash h = newHashAllowEmpty(*outputHash, ht); + auto h = newHashAllowEmpty(*outputHash, parseHashTypeOpt(outputHashAlgo)); auto method = ingestionMethod.value_or(FileIngestionMethod::Flat); auto outPath = state.store->makeFixedOutputPath(method, h, drvName); @@ -1212,7 +1211,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * .errPos = posDrvName }); - auto ht = parseHashType(outputHashAlgo.value_or("sha256")); + auto ht = parseHashTypeOpt(outputHashAlgo).value_or(htSHA256); auto method = ingestionMethod.value_or(FileIngestionMethod::Recursive); for (auto & i : outputs) { diff --git a/tests/ca/content-addressed.nix b/tests/ca/content-addressed.nix index d328fc92c..1be3eeb6e 100644 --- a/tests/ca/content-addressed.nix +++ b/tests/ca/content-addressed.nix @@ -64,8 +64,7 @@ rec { dependentFixedOutput = mkDerivation { name = "dependent-fixed-output"; outputHashMode = "recursive"; - outputHashAlgo = "sha256"; - outputHash = "sha256-QvtAMbUl/uvi+LCObmqOhvNOapHdA2raiI4xG5zI5pA="; + outputHash = "sha512-7aJcmSuEuYP5tGKcmGY8bRr/lrCjJlOxP2mIUjO/vMQeg6gx/65IbzRWES8EKiPDOs9z+wF30lEfcwxM/cT4pw=="; buildCommand = '' cat ${dependentCA}/dep echo foo > $out From 435848cef12d065b209c82c96ce3a8bfa5e6a051 Mon Sep 17 00:00:00 2001 From: aszlig Date: Fri, 1 Apr 2022 09:23:43 -0700 Subject: [PATCH 70/98] libutil: Fix restoring mount namespace I regularly pass around simple scripts by using nix-shell as the script interpreter, eg. like this: #!/usr/bin/env nix-shell #!nix-shell -p dd_rescue coreutils bash -i bash While this works most of the time, I recently had one occasion where it would not and the above would result in the following: $ sudo ./myscript.sh bash: ./myscript.sh: No such file or directory Note the "sudo" here, because this error only occurs if we're root. The reason for the latter is because running Nix as root means that we can directly access the store, which makes sure we use a filesystem namespace to make the store writable. XXX - REWORD! So when stracing the process, I stumbled on the following sequence: openat(AT_FDCWD, "/proc/self/ns/mnt", O_RDONLY) = 3 unshare(CLONE_NEWNS) = 0 ... later ... getcwd("/the/real/cwd", 4096) = 14 setns(3, CLONE_NEWNS) = 0 getcwd("/", 4096) = 2 In the whole strace output there are no calls to chdir() whatsoever, so I decided to look into the kernel source to see what else could change directories and found this[1]: /* Update the pwd and root */ set_fs_pwd(fs, &root); set_fs_root(fs, &root); The set_fs_pwd() call is roughly equivalent to a chdir() syscall and this is called when the setns() syscall is invoked[2]. [1]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/fs/namespace.c#L4659 [2]: https://github.com/torvalds/linux/blob/b14ffae378aa1db993e62b01392e70d1e585fb23/kernel/nsproxy.c#L346 --- src/libutil/util.cc | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 59e3aad6d..e62672717 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -1688,13 +1689,17 @@ void setStackSize(size_t stackSize) #endif } +#if __linux__ static AutoCloseFD fdSavedMountNamespace; +std::optional savedCwd; +#endif void saveMountNamespace() { #if __linux__ static std::once_flag done; std::call_once(done, []() { + savedCwd.emplace(std::filesystem::current_path()); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); @@ -1712,6 +1717,12 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } + try { + if (savedCwd) + std::filesystem::current_path(*savedCwd); + } catch (std::filesystem::filesystem_error const &e) { + debug(e.what()); + } #endif } From 7f5caaa7c0f151520d05d4662415ac09d4cf34b0 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Fri, 1 Apr 2022 10:24:31 -0700 Subject: [PATCH 71/98] libutil: Don't use std::filesystem Just in case making libutil depend on std::filesystem is unacceptable, here is the non-filesystem approach. --- src/libutil/util.cc | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index e62672717..0b18f1027 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -10,7 +10,6 @@ #include #include #include -#include #include #include #include @@ -1691,7 +1690,7 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -std::optional savedCwd; +std::optional savedCwd; #endif void saveMountNamespace() @@ -1699,7 +1698,11 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - savedCwd.emplace(std::filesystem::current_path()); + char* cwd = getcwd(NULL, 0); + if (cwd == NULL) throw SysError("getting cwd"); + savedCwd.emplace(cwd); + free(cwd); + AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); @@ -1717,11 +1720,8 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } - try { - if (savedCwd) - std::filesystem::current_path(*savedCwd); - } catch (std::filesystem::filesystem_error const &e) { - debug(e.what()); + if (savedCwd && chdir(savedCwd->c_str()) == -1) { + throw SysError("restoring cwd"); } #endif } From 2a45cf54e4201a894254604676dcb51f4c1a471c Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Fri, 1 Apr 2022 12:20:34 -0700 Subject: [PATCH 72/98] libutil: Properly guard self-allocating getcwd on GNU It's a GNU extension, as pointed out by pennae. --- src/libutil/util.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 0b18f1027..28ab77adc 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1698,10 +1698,19 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - char* cwd = getcwd(NULL, 0); - if (cwd == NULL) throw SysError("getting cwd"); +#ifdef __GNU__ + // getcwd allocating its return value is a GNU extension. + char *cwd = getcwd(NULL, 0); + if (cwd == NULL) +#else + char cwd[PATH_MAX]; + if (!getcwd(cwd, sizeof(cwd))) +#endif + throw SysError("getting cwd"); savedCwd.emplace(cwd); +#ifdef __GNU__ free(cwd); +#endif AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) From c1e2ce4515c12ff0b4b1c3ab6d1e057507104979 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Thu, 31 Mar 2022 20:30:53 -0400 Subject: [PATCH 73/98] fix(run): set applyNixConfig lockFlag --- src/nix/run.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/run.cc b/src/nix/run.cc index 23e893fbf..25a8fa8d3 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -182,6 +182,7 @@ struct CmdRun : InstallableCommand { auto state = getEvalState(); + lockFlags.applyNixConfig = true; auto app = installable->toApp(*state).resolve(getEvalStore(), store); Strings allArgs{app.program}; From a4a1de69dcc3c6e0c40a093d67b5f20568a5f31e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 4 Apr 2022 16:49:39 +0200 Subject: [PATCH 74/98] Add missing #include --- src/libstore/build-result.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index cb6d19b8e..7f9bcce6d 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -1,6 +1,7 @@ #pragma once #include "realisation.hh" +#include "derived-path.hh" #include #include From e5b70d47aa656833e4609106f9f1ef48b67664cc Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:19:49 -0700 Subject: [PATCH 75/98] libutil: cleanup savedCwd logic Co-authored-by: Eelco Dolstra --- src/libutil/util.cc | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 28ab77adc..9b34d5d72 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1698,20 +1698,7 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { -#ifdef __GNU__ - // getcwd allocating its return value is a GNU extension. - char *cwd = getcwd(NULL, 0); - if (cwd == NULL) -#else - char cwd[PATH_MAX]; - if (!getcwd(cwd, sizeof(cwd))) -#endif - throw SysError("getting cwd"); - savedCwd.emplace(cwd); -#ifdef __GNU__ - free(cwd); -#endif - + savedCwd = absPath("."); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); From e135d223f66f77f2b03a11b979d714e582364013 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:32:45 -0700 Subject: [PATCH 76/98] libutil: save fd to cwd instead of cwd itself --- src/libutil/util.cc | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 9b34d5d72..a00a03978 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1690,7 +1690,7 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -std::optional savedCwd; +static AutoCloseFD fdSavedCwd; #endif void saveMountNamespace() @@ -1698,11 +1698,15 @@ void saveMountNamespace() #if __linux__ static std::once_flag done; std::call_once(done, []() { - savedCwd = absPath("."); AutoCloseFD fd = open("/proc/self/ns/mnt", O_RDONLY); if (!fd) throw SysError("saving parent mount namespace"); fdSavedMountNamespace = std::move(fd); + + fd = open("/proc/self/cwd", O_RDONLY); + if (!fd) + throw SysError("saving cwd"); + fdSavedCwd = std::move(fd); }); #endif } @@ -1716,7 +1720,7 @@ void restoreMountNamespace() } catch (Error & e) { debug(e.msg()); } - if (savedCwd && chdir(savedCwd->c_str()) == -1) { + if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { throw SysError("restoring cwd"); } #endif From f89b0f7846f2177b65eebdbff7f24a255d66819e Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 08:33:59 -0700 Subject: [PATCH 77/98] libutil: `try` restoring the cwd from fdSavedCwd --- src/libutil/util.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index a00a03978..1f800f3f4 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1717,12 +1717,12 @@ void restoreMountNamespace() try { if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); + if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { + throw SysError("restoring cwd"); + } } catch (Error & e) { debug(e.msg()); } - if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { - throw SysError("restoring cwd"); - } #endif } From 10b9c1b2b269b96dcb8b3d298491fa143d1663e8 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 10:16:30 -0700 Subject: [PATCH 78/98] libutil: save cwd fd in restoreMountNamespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This doesn't work very well (maybe I'm misunderstanding the desired implementation): : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' develop -c pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node --- src/libutil/util.cc | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1f800f3f4..701545589 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1690,7 +1690,6 @@ void setStackSize(size_t stackSize) #if __linux__ static AutoCloseFD fdSavedMountNamespace; -static AutoCloseFD fdSavedCwd; #endif void saveMountNamespace() @@ -1702,11 +1701,6 @@ void saveMountNamespace() if (!fd) throw SysError("saving parent mount namespace"); fdSavedMountNamespace = std::move(fd); - - fd = open("/proc/self/cwd", O_RDONLY); - if (!fd) - throw SysError("saving cwd"); - fdSavedCwd = std::move(fd); }); #endif } @@ -1715,6 +1709,10 @@ void restoreMountNamespace() { #if __linux__ try { + AutoCloseFD fdSavedCwd = open("/proc/self/cwd", O_RDONLY); + if (!fdSavedCwd) { + throw SysError("saving cwd"); + } if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { From 56009b2639dc878be11f94d096fae56ac14dcd1d Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Mon, 4 Apr 2022 10:21:56 -0700 Subject: [PATCH 79/98] libutil: don't save cwd fd, use path instead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Saving the cwd fd didn't actually work well -- prior to this commit, the following would happen: : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' run nixpkgs#coreutils -- --coreutils-prog=pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node : ~/w/vc/nix ; doas outputs/out/bin/nix --experimental-features 'nix-command flakes' develop -c pwd pwd: couldn't find directory entry in ‘../../../..’ with matching i-node --- src/libutil/util.cc | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 701545589..3132e7161 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1709,13 +1709,11 @@ void restoreMountNamespace() { #if __linux__ try { - AutoCloseFD fdSavedCwd = open("/proc/self/cwd", O_RDONLY); - if (!fdSavedCwd) { - throw SysError("saving cwd"); - } + auto savedCwd = absPath("."); + if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); - if (fdSavedCwd && fchdir(fdSavedCwd.get()) == -1) { + if (chdir(savedCwd.c_str()) == -1) { throw SysError("restoring cwd"); } } catch (Error & e) { From 5abe3f4aa61f33ac2124a624763e067257541871 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 5 Apr 2022 11:03:43 +0200 Subject: [PATCH 80/98] Allow `welcomeText` when checking a flake template Fix https://github.com/NixOS/nix/issues/6321 --- src/nix/flake.cc | 2 +- tests/flakes.sh | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 47a380238..ce7dc101a 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -463,7 +463,7 @@ struct CmdFlakeCheck : FlakeCommand for (auto & attr : *v.attrs) { std::string name(attr.name); - if (name != "path" && name != "description") + if (name != "path" && name != "description" && name != "welcomeText") throw Error("template '%s' has unsupported attribute '%s'", attrPath, name); } } catch (Error & e) { diff --git a/tests/flakes.sh b/tests/flakes.sh index ea629ae70..46e6a7982 100644 --- a/tests/flakes.sh +++ b/tests/flakes.sh @@ -376,6 +376,9 @@ cat > $templatesDir/flake.nix < Date: Tue, 5 Apr 2022 13:34:25 +0200 Subject: [PATCH 81/98] manual: Add some anchor targets for the nix.conf options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For each `nix.conf` option, add an empty html node with a unique `id` that can be used as an anchor target. Also make the name of the option be a link to that target so that it’s easily discoverable. We can’t rewrite the whole list as an html definition list like it’s done for the builtins because these options also appear in a man page, and the manpage renderer (lowdown) can’t render arbitrary html. But the hack here allows to keep the manpage and have the links in the html version. Fix https://github.com/NixOS/nix/issues/5745 --- doc/manual/generate-options.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/manual/generate-options.nix b/doc/manual/generate-options.nix index 84d90beb6..2d586fa1b 100644 --- a/doc/manual/generate-options.nix +++ b/doc/manual/generate-options.nix @@ -6,7 +6,8 @@ options: concatStrings (map (name: let option = options.${name}; in - " - `${name}` \n\n" + " - [`${name}`](#conf-${name})" + + "

\n\n" + concatStrings (map (s: " ${s}\n") (splitLines option.description)) + "\n\n" + (if option.documentDefault then " **Default:** " + ( From 9a640afc1e63bed28926cb44fe85137fb7d2d2d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 5 Apr 2022 11:33:46 +0200 Subject: [PATCH 82/98] doctor: Always show the output Fix https://github.com/NixOS/nix/issues/6342 --- src/nix/doctor.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc index 4f3003448..ea87e3d87 100644 --- a/src/nix/doctor.cc +++ b/src/nix/doctor.cc @@ -24,12 +24,12 @@ std::string formatProtocol(unsigned int proto) } bool checkPass(const std::string & msg) { - logger->log(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); + notice(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); return true; } bool checkFail(const std::string & msg) { - logger->log(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); + notice(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); return false; } From f98d76ff1aeaaf18ea68a86f9eb91dd73b2d6ce4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Apr 2022 14:13:26 +0200 Subject: [PATCH 83/98] rl-2.7.md: Fix title --- doc/manual/src/release-notes/rl-2.7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/src/release-notes/rl-2.7.md b/doc/manual/src/release-notes/rl-2.7.md index dc92a9cde..2f3879422 100644 --- a/doc/manual/src/release-notes/rl-2.7.md +++ b/doc/manual/src/release-notes/rl-2.7.md @@ -1,4 +1,4 @@ -# Release X.Y (2022-03-07) +# Release 2.7 (2022-03-07) * Nix will now make some helpful suggestions when you mistype something on the command line. For instance, if you type `nix build From 8d6c937d6a233106dcf9c3e0f0e1c60148b7a71e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Apr 2022 16:41:40 +0200 Subject: [PATCH 84/98] flake.lock: Update MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flake lock file updates: • Updated input 'nixpkgs': 'github:NixOS/nixpkgs/82891b5e2c2359d7e58d08849e4c89511ab94234' (2021-09-28) → 'github:NixOS/nixpkgs/530a53dcbc9437363471167a5e4762c5fcfa34a1' (2022-02-19) --- flake.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/flake.lock b/flake.lock index 61eccb73c..cd79fa85e 100644 --- a/flake.lock +++ b/flake.lock @@ -18,11 +18,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1632864508, - "narHash": "sha256-d127FIvGR41XbVRDPVvozUPQ/uRHbHwvfyKHwEt5xFM=", + "lastModified": 1645296114, + "narHash": "sha256-y53N7TyIkXsjMpOG7RhvqJFGDacLs9HlyHeSTBioqYU=", "owner": "NixOS", "repo": "nixpkgs", - "rev": "82891b5e2c2359d7e58d08849e4c89511ab94234", + "rev": "530a53dcbc9437363471167a5e4762c5fcfa34a1", "type": "github" }, "original": { From 1fa03934791189f02208e1807d822128b216f087 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Tue, 5 Apr 2022 21:16:11 +0200 Subject: [PATCH 85/98] libutil: Reserve memory when en/decoding base64 The size of the output when encoding to and decoding from base64 is (roughly) known so we can allocate it in advance to prevent reallocation. --- src/libutil/util.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 59e3aad6d..cca66d5bf 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1471,6 +1471,7 @@ constexpr char base64Chars[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuv std::string base64Encode(std::string_view s) { std::string res; + res.reserve((s.size() + 2) / 3 * 4); int data = 0, nbits = 0; for (char c : s) { @@ -1502,6 +1503,9 @@ std::string base64Decode(std::string_view s) }(); std::string res; + // Some sequences are missing the padding consisting of up to two '='. + // vvv + res.reserve((s.size() + 2) / 4 * 3); unsigned int d = 0, bits = 0; for (char c : s) { From 513652d5947c1923a004318ea453cab786cabda1 Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Tue, 5 Apr 2022 22:13:45 +0200 Subject: [PATCH 86/98] tokenizeString: Fix semantic mistake `string_view::find_first_not_of(...)` and `string_view::find_first_of(...)` return `string_view::npos` on error not `string::npos`. --- src/libutil/util.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 59e3aad6d..8ab385837 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -1259,9 +1259,9 @@ template C tokenizeString(std::string_view s, std::string_view separato { C result; auto pos = s.find_first_not_of(separators, 0); - while (pos != std::string::npos) { + while (pos != std::string_view::npos) { auto end = s.find_first_of(separators, pos + 1); - if (end == std::string::npos) end = s.size(); + if (end == std::string_view::npos) end = s.size(); result.insert(result.end(), std::string(s, pos, end - pos)); pos = s.find_first_not_of(separators, end); } From 589f6f267b009bc2856597995db360f910e69a6f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 11:52:51 +0200 Subject: [PATCH 87/98] fetchClosure: Don't allow URL query parameters Allowing this is a potential security hole, since it allows the user to specify parameters like 'local-nar-cache'. --- src/libexpr/primops/fetchClosure.cc | 9 ++++++++- tests/fetchClosure.sh | 12 ++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index efeb93daf..821eba698 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -61,6 +61,12 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args .errPos = pos }); + if (!parsedURL.query.empty()) + throw Error({ + .msg = hintfmt("'fetchClosure' does not support URL query parameters (in '%s')", *fromStoreUrl), + .errPos = pos + }); + auto fromStore = openStore(parsedURL.to_string()); if (toCA) { @@ -87,7 +93,8 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args }); } } else { - copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + if (!state.store->isValidPath(*fromPath)) + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); toPath = fromPath; } diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh index 0c905ac43..96e4bb741 100644 --- a/tests/fetchClosure.sh +++ b/tests/fetchClosure.sh @@ -56,3 +56,15 @@ nix copy --to file://$cacheDir $caPath fromPath = $caPath; } ") = $caPath ]] + +# Check that URL query parameters aren't allowed. +clearStore +narCache=$TEST_ROOT/nar-cache +rm -rf $narCache +(! nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir?local-nar-cache=$narCache\"; + fromPath = $caPath; + } +") +(! [ -e $narCache ]) From 318936366d5198123ae9ba0292538529f706dce8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 12:41:18 +0200 Subject: [PATCH 88/98] Fix empty 'nix copy' error message This was caused by SubstitutionGoal not setting the errorMsg field in its BuildResult. We now get a more descriptive message than in 2.7.0, e.g. error: path '/nix/store/13mh...' is required, but there is no substituter that can build it instead of the misleading (since there was no build) error: build of '/nix/store/13mh...' failed Fixes #6295. --- .../build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/substitution-goal.cc | 19 ++++++++++++++----- src/libstore/build/substitution-goal.hh | 5 ++++- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index e50292c1e..b7f7b5ab1 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -41,7 +41,7 @@ void DrvOutputSubstitutionGoal::tryNext() if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal with it. */ - debug("drv output '%s' is required, but there is no substituter that can provide it", id.to_string()); + debug("derivation output '%s' is required, but there is no substituter that can provide it", id.to_string()); /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 31e6dbc9f..ca5218627 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -24,9 +24,16 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } -void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) +void PathSubstitutionGoal::done( + ExitCode result, + BuildResult::Status status, + std::optional errorMsg) { buildResult.status = status; + if (errorMsg) { + debug(*errorMsg); + buildResult.errorMsg = *errorMsg; + } amDone(result); } @@ -67,12 +74,14 @@ void PathSubstitutionGoal::tryNext() if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal with it. */ - debug("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath)); /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - done(substituterFailed ? ecFailed : ecNoSubstituters, BuildResult::NoSubstituters); + done( + substituterFailed ? ecFailed : ecNoSubstituters, + BuildResult::NoSubstituters, + fmt("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath))); if (substituterFailed) { worker.failedSubstitutions++; @@ -169,10 +178,10 @@ void PathSubstitutionGoal::referencesValid() trace("all references realised"); if (nrFailed > 0) { - debug("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)); done( nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, - BuildResult::DependencyFailed); + BuildResult::DependencyFailed, + fmt("some references of path '%s' could not be realised", worker.store.printStorePath(storePath))); return; } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 946f13841..a73f8e666 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -53,7 +53,10 @@ struct PathSubstitutionGoal : public Goal /* Content address for recomputing store path */ std::optional ca; - void done(ExitCode result, BuildResult::Status status); + void done( + ExitCode result, + BuildResult::Status status, + std::optional errorMsg = {}); public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); From a7b12c6bd90cd44432bdaf45cb32b0af916d499c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 13:34:25 +0200 Subject: [PATCH 89/98] curl: Use --fail to catch errors --- scripts/check-hydra-status.sh | 2 +- scripts/install.in | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/check-hydra-status.sh b/scripts/check-hydra-status.sh index 5e2f03429..e62705e94 100644 --- a/scripts/check-hydra-status.sh +++ b/scripts/check-hydra-status.sh @@ -14,7 +14,7 @@ curl -sS -H 'Accept: application/json' https://hydra.nixos.org/jobset/nix/master someBuildFailed=0 for buildId in $BUILDS_FOR_LATEST_EVAL; do - buildInfo=$(curl -sS -H 'Accept: application/json' "https://hydra.nixos.org/build/$buildId") + buildInfo=$(curl --fail -sS -H 'Accept: application/json' "https://hydra.nixos.org/build/$buildId") finished=$(echo "$buildInfo" | jq -r '.finished') diff --git a/scripts/install.in b/scripts/install.in index 38d1fb36f..af5f71080 100755 --- a/scripts/install.in +++ b/scripts/install.in @@ -82,7 +82,7 @@ if [ "$(uname -s)" != "Darwin" ]; then fi if command -v curl > /dev/null 2>&1; then - fetch() { curl -L "$1" -o "$2"; } + fetch() { curl --fail -L "$1" -o "$2"; } elif command -v wget > /dev/null 2>&1; then fetch() { wget "$1" -O "$2"; } else From 1e1cd6e7a926097683da366983cc362c2430867d Mon Sep 17 00:00:00 2001 From: Daniel Pauls Date: Wed, 6 Apr 2022 17:33:23 +0200 Subject: [PATCH 90/98] libfetchers: Fix assertion The filter expects all paths to have a prefix of the raw `actualUrl`, but `Store::addToStore(...)` provides absolute canonicalized paths. To fix this create an absolute and canonicalized path from the `actualUrl` and use it instead. Fixes #6195. --- src/libfetchers/git.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d75c5d3ae..f8433bc28 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -285,9 +285,11 @@ struct GitInputScheme : InputScheme auto files = tokenizeString>( runProgram("git", true, gitOpts), "\0"s); + Path actualPath(absPath(actualUrl)); + PathFilter filter = [&](const Path & p) -> bool { - assert(hasPrefix(p, actualUrl)); - std::string file(p, actualUrl.size() + 1); + assert(hasPrefix(p, actualPath)); + std::string file(p, actualPath.size() + 1); auto st = lstat(p); @@ -300,13 +302,13 @@ struct GitInputScheme : InputScheme return files.count(file); }; - auto storePath = store->addToStore(input.getName(), actualUrl, FileIngestionMethod::Recursive, htSHA256, filter); + auto storePath = store->addToStore(input.getName(), actualPath, FileIngestionMethod::Recursive, htSHA256, filter); // FIXME: maybe we should use the timestamp of the last // modified dirty file? input.attrs.insert_or_assign( "lastModified", - hasHead ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + hasHead ? std::stoull(runProgram("git", true, { "-C", actualPath, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); return {std::move(storePath), input}; } From b9c969a86667a7b16a7efa1df0e3d090ef2ead72 Mon Sep 17 00:00:00 2001 From: Rehno Lindeque Date: Wed, 6 Apr 2022 12:20:39 -0400 Subject: [PATCH 91/98] nix flake check: Warn about deprecated nixosModule output --- src/nix/flake.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index ce7dc101a..a876bb3af 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -508,6 +508,7 @@ struct CmdFlakeCheck : FlakeCommand name == "defaultBundler" ? "bundlers..default" : name == "overlay" ? "overlays.default" : name == "devShell" ? "devShells..default" : + name == "nixosModule" ? "nixosModules.default" : ""; if (replacement != "") warn("flake output attribute '%s' is deprecated; use '%s' instead", name, replacement); From 5ff4c42608bdf54d6f889f196f59a6c20c1631d6 Mon Sep 17 00:00:00 2001 From: Rehno Lindeque Date: Wed, 6 Apr 2022 12:24:35 -0400 Subject: [PATCH 92/98] Update release notes --- doc/manual/src/release-notes/rl-next.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 4f3c9ce41..8c8c0fd41 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -32,3 +32,11 @@ paths. Like fixed-output derivations, impure derivations have access to the network. Only fixed-output derivations and impure derivations can depend on an impure derivation. + +* The `nixosModule` flake output attribute has been renamed consistent + with the `.default` renames in nix 2.7. + + * `nixosModule` → `nixosModules.default` + + As before, the old output will continue to work, but `nix flake check` will + issue a warning about it. From 305d3a0ec3c7d53a5ceffee239c6cd4949f99423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 7 Apr 2022 17:31:12 +0200 Subject: [PATCH 93/98] Test fetchgit with path containing a `.` segment --- tests/fetchGit.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index ac23be5c0..9179e2071 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -7,7 +7,9 @@ fi clearStore -repo=$TEST_ROOT/git +# Intentionally not in a canonical form +# See https://github.com/NixOS/nix/issues/6195 +repo=$TEST_ROOT/./git export _NIX_FORCE_HTTP=1 From 2c2fd4946f96e6839ecbfb4cf61318d8910e7e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kasper=20Ga=C5=82kowski?= Date: Wed, 6 Apr 2022 19:28:12 +0200 Subject: [PATCH 94/98] don't assume that rev is a SHA1 hash This was a problem when writing a fetcher that uses e.g. sha256 hashes for revisions. This doesn't actually do anything new, but allows for creating such fetchers in the future (perhaps when support for Git's SHA256 object format gains more popularity). --- src/libfetchers/fetchers.cc | 15 ++++++++++++--- src/libutil/hash.cc | 2 +- src/libutil/hash.hh | 2 -- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 976f40d3b..6957d2da4 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -238,9 +238,18 @@ std::optional Input::getRef() const std::optional Input::getRev() const { - if (auto s = maybeGetStrAttr(attrs, "rev")) - return Hash::parseAny(*s, htSHA1); - return {}; + std::optional hash = {}; + + if (auto s = maybeGetStrAttr(attrs, "rev")) { + try { + hash = Hash::parseAnyPrefixed(*s); + } catch (BadHash &e) { + // Default to sha1 for backwards compatibility with existing flakes + hash = Hash::parseAny(*s, htSHA1); + } + } + + return hash; } std::optional Input::getRevCount() const diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index a4d632161..d2fd0c15a 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -155,7 +155,7 @@ static std::pair, bool> getParsedTypeAndSRI(std::string_ { bool isSRI = false; - // Parse the has type before the separater, if there was one. + // Parse the hash type before the separator, if there was one. std::optional optParsedType; { auto hashRaw = splitPrefixTo(rest, ':'); diff --git a/src/libutil/hash.hh b/src/libutil/hash.hh index 56b5938b3..00f70a572 100644 --- a/src/libutil/hash.hh +++ b/src/libutil/hash.hh @@ -93,13 +93,11 @@ public: std::string gitRev() const { - assert(type == htSHA1); return to_string(Base16, false); } std::string gitShortRev() const { - assert(type == htSHA1); return std::string(to_string(Base16, false), 0, 7); } From 168ef9f3ab73ac8ea2d8263304fd6df29a76e0e0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 13:02:26 +0200 Subject: [PATCH 95/98] Remove unused Error.name field --- src/libstore/sqlite.cc | 1 - src/libutil/error.cc | 3 --- src/libutil/error.hh | 1 - src/libutil/serialise.cc | 5 ++--- 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index e6ecadd7f..1d82b4ab1 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -215,7 +215,6 @@ void handleSQLiteBusy(const SQLiteBusy & e) if (now > lastWarned + 10) { lastWarned = now; logWarning({ - .name = "Sqlite busy", .msg = hintfmt(e.what()) }); } diff --git a/src/libutil/error.cc b/src/libutil/error.cc index 02bc5caa5..9172f67a6 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -21,12 +21,9 @@ const std::string & BaseError::calcWhat() const if (what_.has_value()) return *what_; else { - err.name = sname(); - std::ostringstream oss; showErrorInfo(oss, err, loggerSettings.showTrace); what_ = oss.str(); - return *what_; } } diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 6a757f9ad..005c2c225 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -109,7 +109,6 @@ struct Trace { struct ErrorInfo { Verbosity level; - std::string name; // FIXME: rename hintformat msg; std::optional errPos; std::list traces; diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 6445b3f1b..8ff904583 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -357,7 +357,7 @@ Sink & operator << (Sink & sink, const Error & ex) sink << "Error" << info.level - << info.name + << "Error" // removed << info.msg.str() << 0 // FIXME: info.errPos << info.traces.size(); @@ -426,11 +426,10 @@ Error readError(Source & source) auto type = readString(source); assert(type == "Error"); auto level = (Verbosity) readInt(source); - auto name = readString(source); + auto name = readString(source); // removed auto msg = readString(source); ErrorInfo info { .level = level, - .name = name, .msg = hintformat(std::move(format("%s") % msg)), }; auto havePos = readNum(source); From 8bd9ebf52c9f7c5381d071250660a48d133b5e87 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Apr 2022 15:05:08 +0200 Subject: [PATCH 96/98] Error: Remove unused sname() method --- src/libstore/filetransfer.hh | 2 -- src/libutil/error.hh | 5 ----- src/libutil/experimental-features.hh | 4 ---- 3 files changed, 11 deletions(-) diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index ca61e3937..1ad96bc10 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -123,8 +123,6 @@ public: template FileTransferError(FileTransfer::Error error, std::optional response, const Args & ... args); - - virtual const char* sname() const override { return "FileTransferError"; } }; bool isUri(std::string_view s); diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 005c2c225..348018f57 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -161,8 +161,6 @@ public: : err(e) { } - virtual const char* sname() const { return "BaseError"; } - #ifdef EXCEPTION_NEEDS_THROW_SPEC ~BaseError() throw () { }; const char * what() const throw () { return calcWhat().c_str(); } @@ -189,7 +187,6 @@ public: { \ public: \ using superClass::superClass; \ - virtual const char* sname() const override { return #newClass; } \ } MakeError(Error, BaseError); @@ -209,8 +206,6 @@ public: auto hf = hintfmt(args...); err.msg = hintfmt("%1%: %2%", normaltxt(hf.str()), strerror(errNo)); } - - virtual const char* sname() const override { return "SysError"; } }; } diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 3a254b423..266e41a22 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -49,10 +49,6 @@ public: ExperimentalFeature missingFeature; MissingExperimentalFeature(ExperimentalFeature); - virtual const char * sname() const override - { - return "MissingExperimentalFeature"; - } }; } From c68963eaea7005b5bfb954e50fefb36c36084414 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 Apr 2022 11:48:30 +0200 Subject: [PATCH 97/98] Remove duplicate "error:" --- src/libstore/build-result.hh | 2 ++ src/libstore/build/derivation-goal.cc | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index 7f9bcce6d..24fb1f763 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -31,6 +31,8 @@ struct BuildResult ResolvesToAlreadyValid, NoSubstituters, } status = MiscFailure; + + // FIXME: include entire ErrorInfo object. std::string errorMsg; std::string toString() const { diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 6582497bd..53f212c1d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1371,8 +1371,7 @@ void DerivationGoal::done( { buildResult.status = status; if (ex) - // FIXME: strip: "error: " - buildResult.errorMsg = ex->what(); + buildResult.errorMsg = fmt("%s", normaltxt(ex->info().msg)); if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; if (buildResult.status == BuildResult::PermanentFailure) From 63d9a818194f940fcd2da8fb68bef303c984f300 Mon Sep 17 00:00:00 2001 From: Sebastian Blunt Date: Sun, 10 Apr 2022 21:09:04 -0700 Subject: [PATCH 98/98] Log builder args and environment variables Previously it only logged the builder's path, this changes it to log the arguments at the same log level, and the environment variables at the vomit level. This helped me debug https://github.com/svanderburg/node2nix/issues/75 --- src/libstore/build/local-derivation-goal.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 40ef706a6..4c91fa4fb 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -704,6 +704,9 @@ void LocalDerivationGoal::startBuilder() /* Run the builder. */ printMsg(lvlChatty, "executing builder '%1%'", drv->builder); + printMsg(lvlChatty, "using builder args '%1%'", concatStringsSep(" ", drv->args)); + for (auto & i : drv->env) + printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second); /* Create the log file. */ Path logFile = openLogFile();