From b2748c6e99239ff6803ba0da76c362790c8be192 Mon Sep 17 00:00:00 2001 From: Lucas Franceschino Date: Mon, 25 May 2020 19:07:38 +0200 Subject: [PATCH 01/60] Make `functionArgs` primitive accept primops --- src/libexpr/primops.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 0a4236da4..5875457ac 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1346,6 +1346,10 @@ static void prim_catAttrs(EvalState & state, const Pos & pos, Value * * args, Va static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); + if (args[0]->type == tPrimOpApp || args[0]->type == tPrimOp) { + state.mkAttrs(v, 0); + return; + } if (args[0]->type != tLambda) throw TypeError(format("'functionArgs' requires a function, at %1%") % pos); From 0aa79dcc6f4e67891cdd9500ae4ce85e4189a951 Mon Sep 17 00:00:00 2001 From: Carlo Nucera Date: Fri, 17 Jul 2020 17:24:02 -0400 Subject: [PATCH 02/60] Remove StoreType abstraction and delegate regStore to each Store implementation. The generic regStore implementation will only be for the ambiguous shorthands, like "" and "auto". This also could get us close to simplifying the daemon command. --- src/libstore/local-store.cc | 16 ++++++++++++ src/libstore/remote-store.cc | 10 +++++--- src/libstore/store-api.cc | 47 ++++++++++-------------------------- src/libstore/store-api.hh | 10 -------- src/nix-daemon/nix-daemon.cc | 5 +++- src/nix/doctor.cc | 4 +-- 6 files changed, 41 insertions(+), 51 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index d49d00d6d..9752f3c5f 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1580,4 +1580,20 @@ void LocalStore::createUser(const std::string & userName, uid_t userId) } +static RegisterStoreImplementation regStore([]( + const std::string & uri, const Store::Params & params) + -> std::shared_ptr +{ + Store::Params params2 = params; + if (uri == "local") { + } else if (hasPrefix(uri, "/")) { + params2["root"] = uri; + } else if (hasPrefix(uri, "./")) { + params2["root"] = absPath(uri); + } else { + return nullptr; + } + return std::shared_ptr(std::make_shared(params2)); +}); + } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 9af4364b7..a9fbf9f82 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -843,14 +843,18 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } -static std::string uriScheme = "unix://"; +static std::string_view uriScheme = "unix://"; static RegisterStoreImplementation regStore([]( const std::string & uri, const Store::Params & params) -> std::shared_ptr { - if (std::string(uri, 0, uriScheme.size()) != uriScheme) return 0; - return std::make_shared(std::string(uri, uriScheme.size()), params); + if (hasPrefix(uri, uriScheme)) + return std::make_shared(std::string(uri, uriScheme.size()), params); + else if (uri == "daemon") + return std::make_shared(params); + else + return nullptr; }); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index d37e970df..7a380f127 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -915,44 +915,23 @@ ref openStore(const std::string & uri_, } -StoreType getStoreType(const std::string & uri, const std::string & stateDir) -{ - if (uri == "daemon") { - return tDaemon; - } else if (uri == "local" || hasPrefix(uri, "/") || hasPrefix(uri, "./")) { - return tLocal; - } else if (uri == "" || uri == "auto") { - if (access(stateDir.c_str(), R_OK | W_OK) == 0) - return tLocal; - else if (pathExists(settings.nixDaemonSocketFile)) - return tDaemon; - else - return tLocal; - } else { - return tOther; - } -} - - +// Specific prefixes are handled by the specific types of store, while here we +// handle the general cases not covered by the other ones. static RegisterStoreImplementation regStore([]( const std::string & uri, const Store::Params & params) -> std::shared_ptr { - switch (getStoreType(uri, get(params, "state").value_or(settings.nixStateDir))) { - case tDaemon: - return std::shared_ptr(std::make_shared(params)); - case tLocal: { - Store::Params params2 = params; - if (hasPrefix(uri, "/")) { - params2["root"] = uri; - } else if (hasPrefix(uri, "./")) { - params2["root"] = absPath(uri); - } - return std::shared_ptr(std::make_shared(params2)); - } - default: - return nullptr; - } + auto stateDir = get(params, "state").value_or(settings.nixStateDir); + if (uri == "" || uri == "auto") { + if (access(stateDir.c_str(), R_OK | W_OK) == 0) + return std::make_shared(params); + else if (pathExists(settings.nixDaemonSocketFile)) + return std::make_shared(params); + else + return std::make_shared(params); + } else { + return nullptr; + } }); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index a4be0411e..c38290add 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -796,16 +796,6 @@ ref openStore(const std::string & uri = settings.storeUri.get(), const Store::Params & extraParams = Store::Params()); -enum StoreType { - tDaemon, - tLocal, - tOther -}; - - -StoreType getStoreType(const std::string & uri = settings.storeUri.get(), - const std::string & stateDir = settings.nixStateDir); - /* Return the default substituter stores, defined by the ‘substituters’ option and various legacy options. */ std::list> getDefaultSubstituters(); diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index bcb86cbce..b52cd7989 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -1,5 +1,6 @@ #include "shared.hh" #include "local-store.hh" +#include "remote-store.hh" #include "util.hh" #include "serialise.hh" #include "archive.hh" @@ -277,7 +278,9 @@ static int _main(int argc, char * * argv) initPlugins(); if (stdio) { - if (getStoreType() == tDaemon) { + if (openUncachedStore().dynamic_pointer_cast()) { + // FIXME Use the connection the UDSRemoteStore opened + // Forward on this connection to the real daemon auto socketPath = settings.nixDaemonSocketFile; auto s = socket(PF_UNIX, SOCK_STREAM, 0); diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc index 82e92cdd0..683e91446 100644 --- a/src/nix/doctor.cc +++ b/src/nix/doctor.cc @@ -49,9 +49,7 @@ struct CmdDoctor : StoreCommand { logger->log("Running checks against store uri: " + store->getUri()); - auto type = getStoreType(); - - if (type < tOther) { + if (store.dynamic_pointer_cast()) { success &= checkNixInPath(); success &= checkProfileRoots(store); } From a83694c7a1212567ec2f032f84c0d72722bcf5ea Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 19 Aug 2020 19:34:47 +0000 Subject: [PATCH 03/60] Use `RemoteStore` to open connection for proxying daemon Removes duplicate websocket opening code, and also means we should be able to to ssh-ssh-... daemon relays, not just uds-uds-... ones. --- src/libstore/remote-store.cc | 13 ++++++++++--- src/libstore/remote-store.hh | 4 ++-- src/libstore/ssh-store.cc | 1 - src/nix-daemon/nix-daemon.cc | 36 +++++++++--------------------------- 4 files changed, 21 insertions(+), 33 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 553069b89..ff7149643 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -86,7 +86,16 @@ RemoteStore::RemoteStore(const Params & params) : Store(params) , connections(make_ref>( std::max(1, (int) maxConnections), - [this]() { return openConnectionWrapper(); }, + [this]() { + auto conn = openConnectionWrapper(); + try { + initConnection(*conn); + } catch (...) { + failed = true; + throw; + } + return conn; + }, [this](const ref & r) { return r->to.good() @@ -169,8 +178,6 @@ ref UDSRemoteStore::openConnection() conn->startTime = std::chrono::steady_clock::now(); - initConnection(*conn); - return conn; } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 72d2a6689..5ad2dc75b 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -102,8 +102,6 @@ public: void flushBadConnections(); -protected: - struct Connection { AutoCloseFD fd; @@ -119,6 +117,8 @@ protected: ref openConnectionWrapper(); +protected: + virtual ref openConnection() = 0; void initConnection(Connection & conn); diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index caae6b596..7e9b44afe 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -89,7 +89,6 @@ ref SSHStore::openConnection() + (remoteStore.get() == "" ? "" : " --store " + shellEscape(remoteStore.get()))); conn->to = FdSink(conn->sshConn->in.get()); conn->from = FdSource(conn->sshConn->out.get()); - initConnection(*conn); return conn; } diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index 9613cb7d3..bdf56d2e2 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -286,46 +286,28 @@ static int _main(int argc, char * * argv) initPlugins(); if (stdio) { - if (openUncachedStore().dynamic_pointer_cast()) { - // FIXME Use the connection the UDSRemoteStore opened + if (auto store = openUncachedStore().dynamic_pointer_cast()) { + auto conn = store->openConnectionWrapper(); + int from = conn->from.fd; + int to = conn->to.fd; - // Forward on this connection to the real daemon - auto socketPath = settings.nixDaemonSocketFile; - auto s = socket(PF_UNIX, SOCK_STREAM, 0); - if (s == -1) - throw SysError("creating Unix domain socket"); - - auto socketDir = dirOf(socketPath); - if (chdir(socketDir.c_str()) == -1) - throw SysError("changing to socket directory '%1%'", socketDir); - - auto socketName = std::string(baseNameOf(socketPath)); - auto addr = sockaddr_un{}; - addr.sun_family = AF_UNIX; - if (socketName.size() + 1 >= sizeof(addr.sun_path)) - throw Error("socket name %1% is too long", socketName); - strcpy(addr.sun_path, socketName.c_str()); - - if (connect(s, (struct sockaddr *) &addr, sizeof(addr)) == -1) - throw SysError("cannot connect to daemon at %1%", socketPath); - - auto nfds = (s > STDIN_FILENO ? s : STDIN_FILENO) + 1; + auto nfds = std::max(from, to) + 1; while (true) { fd_set fds; FD_ZERO(&fds); - FD_SET(s, &fds); + FD_SET(from, &fds); FD_SET(STDIN_FILENO, &fds); if (select(nfds, &fds, nullptr, nullptr, nullptr) == -1) throw SysError("waiting for data from client or server"); - if (FD_ISSET(s, &fds)) { - auto res = splice(s, nullptr, STDOUT_FILENO, nullptr, SSIZE_MAX, SPLICE_F_MOVE); + if (FD_ISSET(from, &fds)) { + auto res = splice(from, nullptr, STDOUT_FILENO, nullptr, SSIZE_MAX, SPLICE_F_MOVE); if (res == -1) throw SysError("splicing data from daemon socket to stdout"); else if (res == 0) throw EndOfFile("unexpected EOF from daemon socket"); } if (FD_ISSET(STDIN_FILENO, &fds)) { - auto res = splice(STDIN_FILENO, nullptr, s, nullptr, SSIZE_MAX, SPLICE_F_MOVE); + auto res = splice(STDIN_FILENO, nullptr, to, nullptr, SSIZE_MAX, SPLICE_F_MOVE); if (res == -1) throw SysError("splicing data from stdin to daemon socket"); else if (res == 0) From 3df78858f2ad91a80e30ba910119a0c16c05c66a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 20 Aug 2020 05:08:50 +0000 Subject: [PATCH 04/60] Fix max fd calc and add test --- src/nix-daemon/nix-daemon.cc | 2 +- tests/local.mk | 1 + tests/ssh-relay.sh | 16 ++++++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 tests/ssh-relay.sh diff --git a/src/nix-daemon/nix-daemon.cc b/src/nix-daemon/nix-daemon.cc index bdf56d2e2..6e652ccbf 100644 --- a/src/nix-daemon/nix-daemon.cc +++ b/src/nix-daemon/nix-daemon.cc @@ -291,7 +291,7 @@ static int _main(int argc, char * * argv) int from = conn->from.fd; int to = conn->to.fd; - auto nfds = std::max(from, to) + 1; + auto nfds = std::max(from, STDIN_FILENO) + 1; while (true) { fd_set fds; FD_ZERO(&fds); diff --git a/tests/local.mk b/tests/local.mk index 53035da41..fd9438386 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -15,6 +15,7 @@ nix_tests = \ linux-sandbox.sh \ build-dry.sh \ build-remote-input-addressed.sh \ + ssh-relay.sh \ nar-access.sh \ structured-attrs.sh \ fetchGit.sh \ diff --git a/tests/ssh-relay.sh b/tests/ssh-relay.sh new file mode 100644 index 000000000..dce50974b --- /dev/null +++ b/tests/ssh-relay.sh @@ -0,0 +1,16 @@ +source common.sh + +echo foo > $TEST_ROOT/hello.sh + +ssh_localhost=ssh://localhost +remote_store=?remote-store=$ssh_localhost + +store=$ssh_localhost + +store+=$remote_store +store+=$remote_store +store+=$remote_store + +out=$(nix add-to-store --store "$store" $TEST_ROOT/hello.sh) + +[ foo = $(< $out) ] From b7c02232b2c2c51b3b81389b247e400b7a115abc Mon Sep 17 00:00:00 2001 From: Marwan Aljubeh Date: Wed, 16 Sep 2020 17:56:43 +0100 Subject: [PATCH 05/60] Fix the nix-daemon Mac OS SSL CA cert Mac OS multi-user installations are currently broken because all requests made by nix-daemon to the binary cache fail with: ``` unable to download ... Problem with the SSL CA cert (path? access rights?) (77). ``` This change ensures that the nix-daemon knows where to find the SSL CA cert file. Fixes #2899 and #3261. --- misc/launchd/org.nixos.nix-daemon.plist.in | 2 ++ 1 file changed, 2 insertions(+) diff --git a/misc/launchd/org.nixos.nix-daemon.plist.in b/misc/launchd/org.nixos.nix-daemon.plist.in index 9f26296a9..c334639e2 100644 --- a/misc/launchd/org.nixos.nix-daemon.plist.in +++ b/misc/launchd/org.nixos.nix-daemon.plist.in @@ -4,6 +4,8 @@ EnvironmentVariables + NIX_SSL_CERT_FILE + /nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt OBJC_DISABLE_INITIALIZE_FORK_SAFETY YES From 10d1865f5f443ddd57bb408a99f0afd74436e963 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 17 Sep 2020 09:12:39 +0200 Subject: [PATCH 06/60] Remove corepkgs/derivation.nix --- corepkgs/local.mk | 1 - src/libexpr/local.mk | 2 +- src/libexpr/primops.cc | 12 +++++------- {corepkgs => src/libexpr/primops}/derivation.nix | 0 4 files changed, 6 insertions(+), 9 deletions(-) rename {corepkgs => src/libexpr/primops}/derivation.nix (100%) diff --git a/corepkgs/local.mk b/corepkgs/local.mk index 2c72d3a31..57f6d53a7 100644 --- a/corepkgs/local.mk +++ b/corepkgs/local.mk @@ -1,6 +1,5 @@ corepkgs_FILES = \ unpack-channel.nix \ - derivation.nix \ fetchurl.nix $(foreach file,config.nix $(corepkgs_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/corepkgs))) diff --git a/src/libexpr/local.mk b/src/libexpr/local.mk index d84b150e0..687a8ccda 100644 --- a/src/libexpr/local.mk +++ b/src/libexpr/local.mk @@ -42,6 +42,6 @@ $(eval $(call install-file-in, $(d)/nix-expr.pc, $(prefix)/lib/pkgconfig, 0644)) $(foreach i, $(wildcard src/libexpr/flake/*.hh), \ $(eval $(call install-file-in, $(i), $(includedir)/nix/flake, 0644))) -$(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh +$(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh $(d)/primops/derivation.nix.gen.hh $(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index d0b0c57b2..7e8526ea1 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3565,13 +3565,11 @@ void EvalState::createBaseEnv() /* Add a wrapper around the derivation primop that computes the `drvPath' and `outPath' attributes lazily. */ - try { - string path = canonPath(settings.nixDataDir + "/nix/corepkgs/derivation.nix", true); - sDerivationNix = symbols.create(path); - evalFile(path, v); - addConstant("derivation", v); - } catch (SysError &) { - } + sDerivationNix = symbols.create("//builtin/derivation.nix"); + eval(parse( + #include "primops/derivation.nix.gen.hh" + , foFile, sDerivationNix, "/", staticBaseEnv), v); + addConstant("derivation", v); /* Now that we've added all primops, sort the `builtins' set, because attribute lookups expect it to be sorted. */ diff --git a/corepkgs/derivation.nix b/src/libexpr/primops/derivation.nix similarity index 100% rename from corepkgs/derivation.nix rename to src/libexpr/primops/derivation.nix From 787469c7b66aec12ab6847e7db2cdc8aef5c325e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 17 Sep 2020 09:35:30 +0200 Subject: [PATCH 07/60] Remove corepkgs/unpack-channel.nix --- corepkgs/local.mk | 1 - src/nix-channel/nix-channel.cc | 11 +++++++++-- {corepkgs => src/nix-channel}/unpack-channel.nix | 0 src/nix/local.mk | 2 ++ 4 files changed, 11 insertions(+), 3 deletions(-) rename {corepkgs => src/nix-channel}/unpack-channel.nix (100%) diff --git a/corepkgs/local.mk b/corepkgs/local.mk index 57f6d53a7..1546b4ef3 100644 --- a/corepkgs/local.mk +++ b/corepkgs/local.mk @@ -1,5 +1,4 @@ corepkgs_FILES = \ - unpack-channel.nix \ fetchurl.nix $(foreach file,config.nix $(corepkgs_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/corepkgs))) diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index 3ccf620c9..e48f7af9a 100755 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -76,6 +76,13 @@ static void update(const StringSet & channelNames) auto store = openStore(); + auto [fd, unpackChannelPath] = createTempFile(); + writeFull(fd.get(), + #include "unpack-channel.nix.gen.hh" + ); + fd = -1; + AutoDelete del(unpackChannelPath, false); + // Download each channel. Strings exprs; for (const auto & channel : channels) { @@ -104,7 +111,7 @@ static void update(const StringSet & channelNames) bool unpacked = false; if (std::regex_search(filename, std::regex("\\.tar\\.(gz|bz2|xz)$"))) { - runProgram(settings.nixBinDir + "/nix-build", false, { "--no-out-link", "--expr", "import " + runProgram(settings.nixBinDir + "/nix-build", false, { "--no-out-link", "--expr", "import " + unpackChannelPath + "{ name = \"" + cname + "\"; channelName = \"" + name + "\"; src = builtins.storePath \"" + filename + "\"; }" }); unpacked = true; } @@ -125,7 +132,7 @@ static void update(const StringSet & channelNames) // Unpack the channel tarballs into the Nix store and install them // into the channels profile. std::cerr << "unpacking channels...\n"; - Strings envArgs{ "--profile", profile, "--file", "", "--install", "--from-expression" }; + Strings envArgs{ "--profile", profile, "--file", unpackChannelPath, "--install", "--from-expression" }; for (auto & expr : exprs) envArgs.push_back(std::move(expr)); envArgs.push_back("--quiet"); diff --git a/corepkgs/unpack-channel.nix b/src/nix-channel/unpack-channel.nix similarity index 100% rename from corepkgs/unpack-channel.nix rename to src/nix-channel/unpack-channel.nix diff --git a/src/nix/local.mk b/src/nix/local.mk index e96200685..ab4e9121b 100644 --- a/src/nix/local.mk +++ b/src/nix/local.mk @@ -29,3 +29,5 @@ $(eval $(call install-symlink, $(bindir)/nix, $(libexecdir)/nix/build-remote)) src/nix-env/user-env.cc: src/nix-env/buildenv.nix.gen.hh src/nix/develop.cc: src/nix/get-env.sh.gen.hh + +src/nix-channel/nix-channel.cc: src/nix-channel/unpack-channel.nix.gen.hh From c9f51e87057652db0013289a95deffba495b35e7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 17 Sep 2020 10:42:51 +0200 Subject: [PATCH 08/60] Remove corepkgs/config.nix This isn't used anywhere except in the configure script of the Perl bindings. I've changed the latter to use the C++ API's Settings object at runtime. --- .gitignore | 9 ------ corepkgs/config.nix.in | 13 --------- corepkgs/local.mk | 4 +-- perl/configure.ac | 13 --------- perl/lib/Nix/Config.pm.in | 10 ++----- perl/lib/Nix/Store.pm | 60 ++------------------------------------- perl/lib/Nix/Store.xs | 10 +++++++ tests/recursive.sh | 6 ++-- 8 files changed, 19 insertions(+), 106 deletions(-) delete mode 100644 corepkgs/config.nix.in diff --git a/.gitignore b/.gitignore index 3f81f8ef5..4711fa7fa 100644 --- a/.gitignore +++ b/.gitignore @@ -12,15 +12,6 @@ perl/Makefile.config /svn-revision /libtool -/corepkgs/config.nix - -# /corepkgs/channels/ -/corepkgs/channels/unpack.sh - -# /corepkgs/nar/ -/corepkgs/nar/nar.sh -/corepkgs/nar/unnar.sh - # /doc/manual/ /doc/manual/*.1 /doc/manual/*.5 diff --git a/corepkgs/config.nix.in b/corepkgs/config.nix.in deleted file mode 100644 index cb9945944..000000000 --- a/corepkgs/config.nix.in +++ /dev/null @@ -1,13 +0,0 @@ -# FIXME: remove this file? -let - fromEnv = var: def: - let val = builtins.getEnv var; in - if val != "" then val else def; -in rec { - nixBinDir = fromEnv "NIX_BIN_DIR" "@bindir@"; - nixPrefix = "@prefix@"; - nixLibexecDir = fromEnv "NIX_LIBEXEC_DIR" "@libexecdir@"; - nixLocalstateDir = "@localstatedir@"; - nixSysconfDir = "@sysconfdir@"; - nixStoreDir = fromEnv "NIX_STORE_DIR" "@storedir@"; -} diff --git a/corepkgs/local.mk b/corepkgs/local.mk index 1546b4ef3..0bc91cfab 100644 --- a/corepkgs/local.mk +++ b/corepkgs/local.mk @@ -1,6 +1,4 @@ corepkgs_FILES = \ fetchurl.nix -$(foreach file,config.nix $(corepkgs_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/corepkgs))) - -template-files += $(d)/config.nix +$(foreach file,$(corepkgs_FILES),$(eval $(call install-data-in,$(d)/$(file),$(datadir)/nix/corepkgs))) diff --git a/perl/configure.ac b/perl/configure.ac index c3769e142..255744afd 100644 --- a/perl/configure.ac +++ b/perl/configure.ac @@ -70,19 +70,6 @@ PKG_CHECK_MODULES([NIX], [nix-store]) NEED_PROG([NIX], [nix]) -# Get nix configure values -export NIX_REMOTE=daemon -nixbindir=$("$NIX" --experimental-features nix-command eval --raw -f '' nixBinDir) -nixlibexecdir=$("$NIX" --experimental-features nix-command eval --raw -f '' nixLibexecDir) -nixlocalstatedir=$("$NIX" --experimental-features nix-command eval --raw -f '' nixLocalstateDir) -nixsysconfdir=$("$NIX" --experimental-features nix-command eval --raw -f '' nixSysconfDir) -nixstoredir=$("$NIX" --experimental-features nix-command eval --raw -f '' nixStoreDir) -AC_SUBST(nixbindir) -AC_SUBST(nixlibexecdir) -AC_SUBST(nixlocalstatedir) -AC_SUBST(nixsysconfdir) -AC_SUBST(nixstoredir) - # Expand all variables in config.status. test "$prefix" = NONE && prefix=$ac_default_prefix test "$exec_prefix" = NONE && exec_prefix='${prefix}' diff --git a/perl/lib/Nix/Config.pm.in b/perl/lib/Nix/Config.pm.in index bc1749e60..f7c6f2484 100644 --- a/perl/lib/Nix/Config.pm.in +++ b/perl/lib/Nix/Config.pm.in @@ -4,14 +4,8 @@ use MIME::Base64; $version = "@PACKAGE_VERSION@"; -$binDir = $ENV{"NIX_BIN_DIR"} || "@nixbindir@"; -$libexecDir = $ENV{"NIX_LIBEXEC_DIR"} || "@nixlibexecdir@"; -$stateDir = $ENV{"NIX_STATE_DIR"} || "@nixlocalstatedir@/nix"; -$logDir = $ENV{"NIX_LOG_DIR"} || "@nixlocalstatedir@/log/nix"; -$confDir = $ENV{"NIX_CONF_DIR"} || "@nixsysconfdir@/nix"; -$storeDir = $ENV{"NIX_STORE_DIR"} || "@nixstoredir@"; - -$useBindings = 1; +$binDir = Nix::Store::getBinDir; +$storeDir = Nix::Store::getStoreDir; %config = (); diff --git a/perl/lib/Nix/Store.pm b/perl/lib/Nix/Store.pm index d226264d4..179f1dc90 100644 --- a/perl/lib/Nix/Store.pm +++ b/perl/lib/Nix/Store.pm @@ -2,7 +2,6 @@ package Nix::Store; use strict; use warnings; -use Nix::Config; require Exporter; @@ -22,6 +21,7 @@ our @EXPORT = qw( addToStore makeFixedOutputPath derivationFromPath addTempRoot + getBinDir getStoreDir ); our $VERSION = '0.15'; @@ -34,62 +34,8 @@ sub backtick { return $res; } -if ($Nix::Config::useBindings) { - require XSLoader; - XSLoader::load('Nix::Store', $VERSION); -} else { - - # Provide slow fallbacks of some functions on platforms that don't - # support the Perl bindings. - - use File::Temp; - use Fcntl qw/F_SETFD/; - - *hashFile = sub { - my ($algo, $base32, $path) = @_; - my $res = backtick("$Nix::Config::binDir/nix-hash", "--flat", $path, "--type", $algo, $base32 ? "--base32" : ()); - chomp $res; - return $res; - }; - - *hashPath = sub { - my ($algo, $base32, $path) = @_; - my $res = backtick("$Nix::Config::binDir/nix-hash", $path, "--type", $algo, $base32 ? "--base32" : ()); - chomp $res; - return $res; - }; - - *hashString = sub { - my ($algo, $base32, $s) = @_; - my $fh = File::Temp->new(); - print $fh $s; - my $res = backtick("$Nix::Config::binDir/nix-hash", $fh->filename, "--type", $algo, $base32 ? "--base32" : ()); - chomp $res; - return $res; - }; - - *addToStore = sub { - my ($srcPath, $recursive, $algo) = @_; - die "not implemented" if $recursive || $algo ne "sha256"; - my $res = backtick("$Nix::Config::binDir/nix-store", "--add", $srcPath); - chomp $res; - return $res; - }; - - *isValidPath = sub { - my ($path) = @_; - my $res = backtick("$Nix::Config::binDir/nix-store", "--check-validity", "--print-invalid", $path); - chomp $res; - return $res ne $path; - }; - - *queryPathHash = sub { - my ($path) = @_; - my $res = backtick("$Nix::Config::binDir/nix-store", "--query", "--hash", $path); - chomp $res; - return $res; - }; -} +require XSLoader; +XSLoader::load('Nix::Store', $VERSION); 1; __END__ diff --git a/perl/lib/Nix/Store.xs b/perl/lib/Nix/Store.xs index 8546f6307..599921151 100644 --- a/perl/lib/Nix/Store.xs +++ b/perl/lib/Nix/Store.xs @@ -351,3 +351,13 @@ void addTempRoot(char * storePath) } catch (Error & e) { croak("%s", e.what()); } + + +SV * getBinDir() + PPCODE: + XPUSHs(sv_2mortal(newSVpv(settings.nixBinDir.c_str(), 0))); + + +SV * getStoreDir() + PPCODE: + XPUSHs(sv_2mortal(newSVpv(settings.nixStore.c_str(), 0))); diff --git a/tests/recursive.sh b/tests/recursive.sh index cf10d55bf..80a178cc7 100644 --- a/tests/recursive.sh +++ b/tests/recursive.sh @@ -9,9 +9,8 @@ rm -f $TEST_ROOT/result export unreachable=$(nix add-to-store ./recursive.sh) -nix --experimental-features 'nix-command recursive-nix' build -o $TEST_ROOT/result -L --impure --expr ' +NIX_BIN_DIR=$(dirname $(type -p nix)) nix --experimental-features 'nix-command recursive-nix' build -o $TEST_ROOT/result -L --impure --expr ' with import ./config.nix; - with import ; mkDerivation { name = "recursive"; dummy = builtins.toFile "dummy" "bla bla"; @@ -24,9 +23,10 @@ nix --experimental-features 'nix-command recursive-nix' build -o $TEST_ROOT/resu buildCommand = '\'\'' mkdir $out - PATH=${nixBinDir}:$PATH opts="--experimental-features nix-command" + PATH=${builtins.getEnv "NIX_BIN_DIR"}:$PATH + # Check that we can query/build paths in our input closure. nix $opts path-info $dummy nix $opts build $dummy From 520895b1dac1b82be9f571df3981a15cfada2968 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 17 Sep 2020 13:36:58 +0200 Subject: [PATCH 09/60] Fix garbage collection of CA derivations Fix #4026 --- src/libstore/gc.cc | 9 ++++++--- tests/content-addressed.sh | 3 +++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index e6cbc525d..08b53c702 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -574,9 +574,12 @@ bool LocalStore::canReachRoot(GCState & state, StorePathSet & visited, const Sto /* If keep-derivations is set and this is a derivation, then don't delete the derivation if any of the outputs are alive. */ if (state.gcKeepDerivations && path.isDerivation()) { - for (auto & i : queryDerivationOutputs(path)) - if (isValidPath(i) && queryPathInfo(i)->deriver == path) - incoming.insert(i); + for (auto & [name, maybeOutPath] : queryPartialDerivationOutputMap(path)) + if (maybeOutPath && + isValidPath(*maybeOutPath) && + queryPathInfo(*maybeOutPath)->deriver == path + ) + incoming.insert(*maybeOutPath); } /* If keep-outputs is set, then don't delete this path if there diff --git a/tests/content-addressed.sh b/tests/content-addressed.sh index ae9e3c59e..0ae2852d2 100644 --- a/tests/content-addressed.sh +++ b/tests/content-addressed.sh @@ -15,3 +15,6 @@ out1=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 1) out2=$(nix-build "${commonArgs[@]}" ./content-addressed.nix --arg seed 2) test $out1 == $out2 + +nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 5 +nix-collect-garbage --experimental-features ca-derivations --option keep-derivations true From 3f93bc0d39661af52466a316a3daf2e49165755e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 17:38:25 +0200 Subject: [PATCH 10/60] Typo --- src/libutil/split.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/split.hh b/src/libutil/split.hh index d19d7d8ed..87a23b13e 100644 --- a/src/libutil/split.hh +++ b/src/libutil/split.hh @@ -9,7 +9,7 @@ namespace nix { // If `separator` is found, we return the portion of the string before the // separator, and modify the string argument to contain only the part after the -// separator. Otherwise, wer return `std::nullopt`, and we leave the argument +// separator. Otherwise, we return `std::nullopt`, and we leave the argument // string alone. static inline std::optional splitPrefixTo(std::string_view & string, char separator) { auto sepInstance = string.find(separator); From 9ee3122ec71ae43f5cd8bf0a9282777ba17342c5 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 13 Sep 2020 14:38:05 +0200 Subject: [PATCH 11/60] Remove redundant import --- src/libstore/daemon.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 8adabf549..2e068992b 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -2,7 +2,6 @@ #include "monitor-fd.hh" #include "worker-protocol.hh" #include "store-api.hh" -#include "local-store.hh" #include "finally.hh" #include "affinity.hh" #include "archive.hh" From 29c82ccc77714a74e8948ce8c531de5a8d870176 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sun, 13 Sep 2020 14:39:11 +0200 Subject: [PATCH 12/60] Add Source.drainInto(Sink) --- src/libutil/serialise.cc | 13 ++++++++++--- src/libutil/serialise.hh | 2 ++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 00c945113..a469a1e73 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -93,7 +93,7 @@ void Source::operator () (unsigned char * data, size_t len) } -std::string Source::drain() +void Source::drainInto(Sink & sink) { std::string s; std::vector buf(8192); @@ -101,12 +101,19 @@ std::string Source::drain() size_t n; try { n = read(buf.data(), buf.size()); - s.append((char *) buf.data(), n); + sink(buf.data(), n); } catch (EndOfFile &) { break; } } - return s; +} + + +std::string Source::drain() +{ + StringSink s; + drainInto(s); + return *s.s; } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 6f4f4c855..2fd06bb4d 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -69,6 +69,8 @@ struct Source virtual bool good() { return true; } + void drainInto(Sink & sink); + std::string drain(); }; From dfa547c6a81d6fb7ef3d3f69a98ebe969df42828 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 17:15:05 +0200 Subject: [PATCH 13/60] Add ContentAddressMethod and parse/render it --- src/libstore/content-address.cc | 62 ++++++++++++++++++++++++++------- src/libstore/content-address.hh | 19 ++++++++++ 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 0885c3d0e..bf56d3d77 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -37,48 +37,86 @@ std::string renderContentAddress(ContentAddress ca) { }, ca); } -ContentAddress parseContentAddress(std::string_view rawCa) { - auto rest = rawCa; +std::string renderContentAddressMethod(ContentAddressMethod cam) { + return std::visit(overloaded { + [](TextHashMethod &th) { + return std::string{"text:"} + printHashType(htSHA256); + }, + [](FixedOutputHashMethod &fshm) { + return "fixed:" + makeFileIngestionPrefix(fshm.fileIngestionMethod) + printHashType(fshm.hashType); + } + }, cam); +} + +/* + Parses content address strings up to the hash. + */ +static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest) { + std::string wholeInput(rest); std::string_view prefix; { auto optPrefix = splitPrefixTo(rest, ':'); if (!optPrefix) - throw UsageError("not a content address because it is not in the form ':': %s", rawCa); + throw UsageError("not a content address because it is not in the form ':': %s", wholeInput); prefix = *optPrefix; } auto parseHashType_ = [&](){ auto hashTypeRaw = splitPrefixTo(rest, ':'); if (!hashTypeRaw) - throw UsageError("content address hash must be in form ':', but found: %s", rawCa); + throw UsageError("content address hash must be in form ':', but found: %s", wholeInput); HashType hashType = parseHashType(*hashTypeRaw); return std::move(hashType); }; // Switch on prefix if (prefix == "text") { - // No parsing of the method, "text" only support flat. + // No parsing of the ingestion method, "text" only support flat. HashType hashType = parseHashType_(); if (hashType != htSHA256) throw Error("text content address hash should use %s, but instead uses %s", printHashType(htSHA256), printHashType(hashType)); - return TextHash { - .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), - }; + return TextHashMethod {}; } else if (prefix == "fixed") { // Parse method auto method = FileIngestionMethod::Flat; if (splitPrefix(rest, "r:")) method = FileIngestionMethod::Recursive; HashType hashType = parseHashType_(); - return FixedOutputHash { - .method = method, - .hash = Hash::parseNonSRIUnprefixed(rest, std::move(hashType)), + return FixedOutputHashMethod { + .fileIngestionMethod = method, + .hashType = std::move(hashType), }; } else throw UsageError("content address prefix '%s' is unrecognized. Recogonized prefixes are 'text' or 'fixed'", prefix); -}; +} + +ContentAddress parseContentAddress(std::string_view rawCa) { + auto rest = rawCa; + + ContentAddressMethod caMethod = parseContentAddressMethodPrefix(rest); + + return std::visit( + overloaded { + [&](TextHashMethod thm) { + return ContentAddress(TextHash { + .hash = Hash::parseNonSRIUnprefixed(rest, htSHA256) + }); + }, + [&](FixedOutputHashMethod fohMethod) { + return ContentAddress(FixedOutputHash { + .method = fohMethod.fileIngestionMethod, + .hash = Hash::parseNonSRIUnprefixed(rest, std::move(fohMethod.hashType)), + }); + }, + }, caMethod); +} + +ContentAddressMethod parseContentAddressMethod(std::string_view caMethod) { + std::string_view asPrefix {std::string{caMethod} + ":"}; + return parseContentAddressMethodPrefix(asPrefix); +} std::optional parseContentAddressOpt(std::string_view rawCaOpt) { return rawCaOpt == "" ? std::optional {} : parseContentAddress(rawCaOpt); diff --git a/src/libstore/content-address.hh b/src/libstore/content-address.hh index 22a039242..f6a6f5140 100644 --- a/src/libstore/content-address.hh +++ b/src/libstore/content-address.hh @@ -55,4 +55,23 @@ std::optional parseContentAddressOpt(std::string_view rawCaOpt); Hash getContentAddressHash(const ContentAddress & ca); +/* + We only have one way to hash text with references, so this is single-value + type is only useful in std::variant. +*/ +struct TextHashMethod { }; +struct FixedOutputHashMethod { + FileIngestionMethod fileIngestionMethod; + HashType hashType; +}; + +typedef std::variant< + TextHashMethod, + FixedOutputHashMethod + > ContentAddressMethod; + +ContentAddressMethod parseContentAddressMethod(std::string_view rawCaMethod); + +std::string renderContentAddressMethod(ContentAddressMethod caMethod); + } From 14b30b3f3d5af75c210a15cb128e67c0eff66149 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 17:36:16 +0200 Subject: [PATCH 14/60] Move FramedSource and FramedSink, extract withFramedSink --- src/libstore/daemon.cc | 47 ----------- src/libstore/remote-store.cc | 151 +++++++++++++++++++---------------- src/libutil/serialise.hh | 47 +++++++++++ 3 files changed, 127 insertions(+), 118 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 2e068992b..010d3bc2d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -748,59 +748,12 @@ static void performOp(TunnelLogger * logger, ref store, info.ultimate = false; if (GET_PROTOCOL_MINOR(clientVersion) >= 23) { - - struct FramedSource : Source - { - Source & from; - bool eof = false; - std::vector pending; - size_t pos = 0; - - FramedSource(Source & from) : from(from) - { } - - ~FramedSource() - { - if (!eof) { - while (true) { - auto n = readInt(from); - if (!n) break; - std::vector data(n); - from(data.data(), n); - } - } - } - - size_t read(unsigned char * data, size_t len) override - { - if (eof) throw EndOfFile("reached end of FramedSource"); - - if (pos >= pending.size()) { - size_t len = readInt(from); - if (!len) { - eof = true; - return 0; - } - pending = std::vector(len); - pos = 0; - from(pending.data(), len); - } - - auto n = std::min(len, pending.size() - pos); - memcpy(data, pending.data() + pos, n); - pos += n; - return n; - } - }; - logger->startWork(); - { FramedSource source(from); store->addToStore(info, source, (RepairFlag) repair, dontCheckSigs ? NoCheckSigs : CheckSigs); } - logger->stopWork(); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index e92b94975..993b05dc0 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -306,6 +306,8 @@ struct ConnectionHandle std::rethrow_exception(ex); } } + + void withFramedSink(std::function fun); }; @@ -564,78 +566,9 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, << repair << !checkSigs; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 23) { - - conn->to.flush(); - - std::exception_ptr ex; - - struct FramedSink : BufferedSink - { - ConnectionHandle & conn; - std::exception_ptr & ex; - - FramedSink(ConnectionHandle & conn, std::exception_ptr & ex) : conn(conn), ex(ex) - { } - - ~FramedSink() - { - try { - conn->to << 0; - conn->to.flush(); - } catch (...) { - ignoreException(); - } - } - - void write(const unsigned char * data, size_t len) override - { - /* Don't send more data if the remote has - encountered an error. */ - if (ex) { - auto ex2 = ex; - ex = nullptr; - std::rethrow_exception(ex2); - } - conn->to << len; - conn->to(data, len); - }; - }; - - /* Handle log messages / exceptions from the remote on a - separate thread. */ - std::thread stderrThread([&]() - { - try { - conn.processStderr(nullptr, nullptr, false); - } catch (...) { - ex = std::current_exception(); - } - }); - - Finally joinStderrThread([&]() - { - if (stderrThread.joinable()) { - stderrThread.join(); - if (ex) { - try { - std::rethrow_exception(ex); - } catch (...) { - ignoreException(); - } - } - } - }); - - { - FramedSink sink(conn, ex); + conn.withFramedSink([&](Sink & sink) { copyNAR(source, sink); - sink.flush(); - } - - stderrThread.join(); - if (ex) - std::rethrow_exception(ex); - + }); } else if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 21) { conn.processStderr(0, &source); } else { @@ -992,6 +925,82 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } + +struct FramedSink : nix::BufferedSink +{ + ConnectionHandle & conn; + std::exception_ptr & ex; + + FramedSink(ConnectionHandle & conn, std::exception_ptr & ex) : conn(conn), ex(ex) + { } + + ~FramedSink() + { + try { + conn->to << 0; + conn->to.flush(); + } catch (...) { + ignoreException(); + } + } + + void write(const unsigned char * data, size_t len) override + { + /* Don't send more data if the remote has + encountered an error. */ + if (ex) { + auto ex2 = ex; + ex = nullptr; + std::rethrow_exception(ex2); + } + conn->to << len; + conn->to(data, len); + }; +}; + +void +ConnectionHandle::withFramedSink(std::function fun) { + (*this)->to.flush(); + + std::exception_ptr ex; + + /* Handle log messages / exceptions from the remote on a + separate thread. */ + std::thread stderrThread([&]() + { + try { + processStderr(nullptr, nullptr, false); + } catch (...) { + ex = std::current_exception(); + } + }); + + Finally joinStderrThread([&]() + { + if (stderrThread.joinable()) { + stderrThread.join(); + if (ex) { + try { + std::rethrow_exception(ex); + } catch (...) { + ignoreException(); + } + } + } + }); + + { + FramedSink sink(*this, ex); + fun(sink); + sink.flush(); + } + + stderrThread.join(); + if (ex) + std::rethrow_exception(ex); + +} + static RegisterStoreImplementation regStore; } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 2fd06bb4d..6027d5961 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -406,4 +406,51 @@ struct StreamToSourceAdapter : Source }; +/* Like SizedSource, but guarantees that the whole frame is consumed after + destruction. This ensures that the original stream is in a known state. */ +struct FramedSource : Source +{ + Source & from; + bool eof = false; + std::vector pending; + size_t pos = 0; + + FramedSource(Source & from) : from(from) + { } + + ~FramedSource() + { + if (!eof) { + while (true) { + auto n = readInt(from); + if (!n) break; + std::vector data(n); + from(data.data(), n); + } + } + } + + size_t read(unsigned char * data, size_t len) override + { + if (eof) throw EndOfFile("reached end of FramedSource"); + + if (pos >= pending.size()) { + size_t len = readInt(from); + if (!len) { + eof = true; + return 0; + } + pending = std::vector(len); + pos = 0; + from(pending.data(), len); + } + + auto n = std::min(len, pending.size() - pos); + memcpy(data, pending.data() + pos, n); + pos += n; + return n; + } +}; + + } From 958bf5712377f59622c59f05a84641aa1093fd32 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 18 Sep 2020 13:10:42 +0200 Subject: [PATCH 15/60] nix build: find() -> get() find() returns an iterator so "!attr" doesn't work. --- src/nix/bundle.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 241c8699b..fc41da9e4 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -98,14 +98,14 @@ struct CmdBundle : InstallableCommand if (!evalState->isDerivation(*vRes)) throw Error("the bundler '%s' does not produce a derivation", bundler.what()); - auto attr1 = vRes->attrs->find(evalState->sDrvPath); + auto attr1 = vRes->attrs->get(evalState->sDrvPath); if (!attr1) throw Error("the bundler '%s' does not produce a derivation", bundler.what()); PathSet context2; StorePath drvPath = store->parseStorePath(evalState->coerceToPath(*attr1->pos, *attr1->value, context2)); - auto attr2 = vRes->attrs->find(evalState->sOutPath); + auto attr2 = vRes->attrs->get(evalState->sOutPath); if (!attr2) throw Error("the bundler '%s' does not produce a derivation", bundler.what()); From 2bcf8cbe7a36ebcd2ad27b4df18fdb911d7ab224 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 18 Sep 2020 14:10:45 +0200 Subject: [PATCH 16/60] libfetchers/github: allow `url` attribute Since 108debef6f703aa3ca1f6d6c45428880624ec6f8 we allow a `url`-attribute for the `github`-fetcher to fetch tarballs from self-hosted `gitlab`/`github` instances. However it's not used when defining e.g. a flake-input foobar = { type = "github"; url = "gitlab.myserver"; /* ... */ } and breaks with an evaluation-error: error: --- Error --------------------------------------nix unsupported input attribute 'url' (use '--show-trace' to show detailed location information) This patch allows flake-inputs to be fetched from self-hosted instances as well. --- src/libfetchers/github.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 1cc0c5e2e..f510ae5f5 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -77,7 +77,7 @@ struct GitArchiveInputScheme : InputScheme if (maybeGetStrAttr(attrs, "type") != type()) return {}; for (auto & [name, value] : attrs) - if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified") + if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "url") throw Error("unsupported input attribute '%s'", name); getStrAttr(attrs, "owner"); From 5fe375a8f1d60b49835b52df48686caddaa297a4 Mon Sep 17 00:00:00 2001 From: Bryan Richter Date: Fri, 18 Sep 2020 18:36:17 +0300 Subject: [PATCH 17/60] nix-prefetch-url: Add --executable flag pkgs.fetchurl supports an executable argument, which is especially nice when downloading a large executable. This patch adds the same option to nix-prefetch-url. I have tested this to work on the simple case of prefetching a little executable: 1. nix-prefetch-url --executable https://my/little/script 2. Paste the hash into a pkgs.fetchurl-based package, script-pkg.nix 3. Delete the output from the store to avoid any misidentified artifacts 4. Realise the package script-pkg.nix 5. Run the executable I repeated the above while using --name, as well. I suspect --executable would have no meaningful effect if combined with --unpack, but I have not tried it. --- doc/manual/src/command-ref/nix-prefetch-url.md | 3 +++ src/nix-prefetch-url/nix-prefetch-url.cc | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/command-ref/nix-prefetch-url.md b/doc/manual/src/command-ref/nix-prefetch-url.md index 1cd1063cd..1307c7c37 100644 --- a/doc/manual/src/command-ref/nix-prefetch-url.md +++ b/doc/manual/src/command-ref/nix-prefetch-url.md @@ -51,6 +51,9 @@ Nix store is also printed. result to the Nix store. The resulting hash can be used with functions such as Nixpkgs’s `fetchzip` or `fetchFromGitHub`. + - `--executable` + Set the executable bit on the downloaded file. + - `--name` *name* Override the name of the file in the Nix store. By default, this is `hash-basename`, where *basename* is the last component of *url*. diff --git a/src/nix-prefetch-url/nix-prefetch-url.cc b/src/nix-prefetch-url/nix-prefetch-url.cc index 1001f27af..377ae03a8 100644 --- a/src/nix-prefetch-url/nix-prefetch-url.cc +++ b/src/nix-prefetch-url/nix-prefetch-url.cc @@ -57,6 +57,7 @@ static int _main(int argc, char * * argv) bool fromExpr = false; string attrPath; bool unpack = false; + bool executable = false; string name; struct MyArgs : LegacyArgs, MixEvalArgs @@ -81,6 +82,8 @@ static int _main(int argc, char * * argv) } else if (*arg == "--unpack") unpack = true; + else if (*arg == "--executable") + executable = true; else if (*arg == "--name") name = getArg(*arg, arg, end); else if (*arg != "" && arg->at(0) == '-') @@ -175,7 +178,11 @@ static int _main(int argc, char * * argv) /* Download the file. */ { - AutoCloseFD fd = open(tmpFile.c_str(), O_WRONLY | O_CREAT | O_EXCL, 0600); + auto mode = 0600; + if (executable) + mode = 0700; + + AutoCloseFD fd = open(tmpFile.c_str(), O_WRONLY | O_CREAT | O_EXCL, mode); if (!fd) throw SysError("creating temporary file '%s'", tmpFile); FdSink sink(fd.get()); @@ -201,7 +208,7 @@ static int _main(int argc, char * * argv) tmpFile = unpacked; } - const auto method = unpack ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; + const auto method = unpack || executable ? FileIngestionMethod::Recursive : FileIngestionMethod::Flat; auto info = store->addToStoreSlow(name, tmpFile, method, ht, expectedHash); storePath = info.path; From c00e07834327a8ef626cf4f1ecb216ee1b6a0877 Mon Sep 17 00:00:00 2001 From: Marwan Aljubeh Date: Fri, 18 Sep 2020 17:10:39 +0100 Subject: [PATCH 18/60] Add a nix.conf option for allowing a symlinked store --- src/libstore/globals.cc | 1 + src/libstore/globals.hh | 13 +++++++++++++ src/libstore/local-store.cc | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 491c664db..ca2e75603 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -41,6 +41,7 @@ Settings::Settings() { buildUsersGroup = getuid() == 0 ? "nixbld" : ""; lockCPU = getEnv("NIX_AFFINITY_HACK") == "1"; + ignoreSymlinkStore = getEnv("NIX_IGNORE_SYMLINK_STORE") == "1"; caFile = getEnv("NIX_SSL_CERT_FILE").value_or(getEnv("SSL_CERT_FILE").value_or("")); if (caFile == "") { diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 02721285a..129cef6b4 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -881,6 +881,19 @@ public: Setting flakeRegistry{this, "https://github.com/NixOS/flake-registry/raw/master/flake-registry.json", "flake-registry", "Path or URI of the global flake registry."}; + + Setting ignoreSymlinkStore{ + this, false, "ignore-symlink-store", + R"( + If set to `true`, Nix will stop complaining if the store directory + (typically /nix/store) contains symlink components. + + This risks making some builds "impure" because builders sometimes + "canonicalise" paths by resolving all symlink components. Problems + occur if those builds are then deployed to machines where /nix/store + resolves to a different location from that of the build machine. You + can enable this setting if you are sure you're not going to do that. + )"}; }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c618203f0..24b9ea7bd 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -109,7 +109,7 @@ LocalStore::LocalStore(const Params & params) } /* Ensure that the store and its parents are not symlinks. */ - if (getEnv("NIX_IGNORE_SYMLINK_STORE") != "1") { + if (!settings.ignoreSymlinkStore) { Path path = realStoreDir; struct stat st; while (path != "/") { From e40772cd35adcd158d30727f7f294b823df8010a Mon Sep 17 00:00:00 2001 From: Marwan Aljubeh Date: Fri, 18 Sep 2020 17:18:45 +0100 Subject: [PATCH 19/60] Lint issue: replacing tabs with spaces --- src/libstore/globals.hh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 129cef6b4..ddc13898d 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -885,11 +885,11 @@ public: Setting ignoreSymlinkStore{ this, false, "ignore-symlink-store", R"( - If set to `true`, Nix will stop complaining if the store directory - (typically /nix/store) contains symlink components. + If set to `true`, Nix will stop complaining if the store directory + (typically /nix/store) contains symlink components. - This risks making some builds "impure" because builders sometimes - "canonicalise" paths by resolving all symlink components. Problems + This risks making some builds "impure" because builders sometimes + "canonicalise" paths by resolving all symlink components. Problems occur if those builds are then deployed to machines where /nix/store resolves to a different location from that of the build machine. You can enable this setting if you are sure you're not going to do that. From e34fe47d0ce19fc7657970fb0e610bffbc3e43f0 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 19:27:11 +0200 Subject: [PATCH 20/60] Overhaul wopAddToStore --- src/libstore/daemon.cc | 105 ++++++++++++++++++++------------ src/libstore/remote-store.cc | 103 +++++++++++++++++++------------ src/libstore/remote-store.hh | 7 +-- src/libstore/worker-protocol.hh | 2 +- 4 files changed, 133 insertions(+), 84 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 010d3bc2d..0a08a2e12 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -349,47 +349,74 @@ static void performOp(TunnelLogger * logger, ref store, } case wopAddToStore: { - HashType hashAlgo; - std::string baseName; - FileIngestionMethod method; - { - bool fixed; - uint8_t recursive; - std::string hashAlgoRaw; - from >> baseName >> fixed /* obsolete */ >> recursive >> hashAlgoRaw; - if (recursive > (uint8_t) FileIngestionMethod::Recursive) - throw Error("unsupported FileIngestionMethod with value of %i; you may need to upgrade nix-daemon", recursive); - method = FileIngestionMethod { recursive }; - /* Compatibility hack. */ - if (!fixed) { - hashAlgoRaw = "sha256"; - method = FileIngestionMethod::Recursive; + if (GET_PROTOCOL_MINOR(clientVersion) >= 25) { + auto name = readString(from); + auto camStr = readString(from); + auto refs = readStorePaths(*store, from); + + logger->startWork(); + StorePath path = [&]() -> StorePath { + // NB: FramedSource must be out of scope before logger->stopWork(); + ContentAddressMethod contentAddressMethod = parseContentAddressMethod(camStr); + FramedSource source(from); + return std::visit(overloaded { + [&](TextHashMethod &_) -> StorePath { + // We could stream this by changing Store + std::string contents = source.drain(); + return store->addTextToStore(name, contents, refs); + }, + [&](FixedOutputHashMethod &fohm) -> StorePath { + return store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType); + }, + }, contentAddressMethod); + }(); + logger->stopWork(); + + to << store->printStorePath(path); + + } else { + HashType hashAlgo; + std::string baseName; + FileIngestionMethod method; + { + bool fixed; + uint8_t recursive; + std::string hashAlgoRaw; + from >> baseName >> fixed /* obsolete */ >> recursive >> hashAlgoRaw; + if (recursive > (uint8_t) FileIngestionMethod::Recursive) + throw Error("unsupported FileIngestionMethod with value of %i; you may need to upgrade nix-daemon", recursive); + method = FileIngestionMethod { recursive }; + /* Compatibility hack. */ + if (!fixed) { + hashAlgoRaw = "sha256"; + method = FileIngestionMethod::Recursive; + } + hashAlgo = parseHashType(hashAlgoRaw); } - hashAlgo = parseHashType(hashAlgoRaw); + + StringSink saved; + TeeSource savedNARSource(from, saved); + RetrieveRegularNARSink savedRegular { saved }; + + if (method == FileIngestionMethod::Recursive) { + /* Get the entire NAR dump from the client and save it to + a string so that we can pass it to + addToStoreFromDump(). */ + ParseSink sink; /* null sink; just parse the NAR */ + parseDump(sink, savedNARSource); + } else + parseDump(savedRegular, from); + + logger->startWork(); + if (!savedRegular.regular) throw Error("regular file expected"); + + // FIXME: try to stream directly from `from`. + StringSource dumpSource { *saved.s }; + auto path = store->addToStoreFromDump(dumpSource, baseName, method, hashAlgo); + logger->stopWork(); + + to << store->printStorePath(path); } - - StringSink saved; - TeeSource savedNARSource(from, saved); - RetrieveRegularNARSink savedRegular { saved }; - - if (method == FileIngestionMethod::Recursive) { - /* Get the entire NAR dump from the client and save it to - a string so that we can pass it to - addToStoreFromDump(). */ - ParseSink sink; /* null sink; just parse the NAR */ - parseDump(sink, savedNARSource); - } else - parseDump(savedRegular, from); - - logger->startWork(); - if (!savedRegular.regular) throw Error("regular file expected"); - - // FIXME: try to stream directly from `from`. - StringSource dumpSource { *saved.s }; - auto path = store->addToStoreFromDump(dumpSource, baseName, method, hashAlgo); - logger->stopWork(); - - to << store->printStorePath(path); break; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 993b05dc0..14ad4767d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -526,6 +526,69 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & } +StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, + FileIngestionMethod method, HashType hashType, RepairFlag repair) +{ + if (repair) throw Error("repairing is not supported when adding to store through the Nix daemon"); + StorePathSet references; + + auto conn(getConnection()); + + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 25) { + + conn->to + << wopAddToStore + << name + << renderContentAddressMethod(FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }); + writeStorePaths(*this, conn->to, references); + + conn.withFramedSink([&](Sink & sink) { + dump.drainInto(sink); + }); + + return parseStorePath(readString(conn->from)); + + } + else { + if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); + + conn->to + << wopAddToStore + << name + << ((hashType == htSHA256 && method == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ + << (method == FileIngestionMethod::Recursive ? 1 : 0) + << printHashType(hashType); + + try { + conn->to.written = 0; + conn->to.warn = true; + connections->incCapacity(); + { + Finally cleanup([&]() { connections->decCapacity(); }); + if (method == FileIngestionMethod::Recursive) { + dump.drainInto(conn->to); + } else { + std::string contents = dump.drain(); + dumpString(contents, conn->to); + } + } + conn->to.warn = false; + conn.processStderr(); + } catch (SysError & e) { + /* Daemon closed while we were sending the path. Probably OOM + or I/O error. */ + if (e.errNo == EPIPE) + try { + conn.processStderr(); + } catch (EndOfFile & e) { } + throw; + } + + return parseStorePath(readString(conn->from)); + } +} + + void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) { @@ -579,46 +642,6 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, } -StorePath RemoteStore::addToStore(const string & name, const Path & _srcPath, - FileIngestionMethod method, HashType hashAlgo, PathFilter & filter, RepairFlag repair) -{ - if (repair) throw Error("repairing is not supported when building through the Nix daemon"); - - auto conn(getConnection()); - - Path srcPath(absPath(_srcPath)); - - conn->to - << wopAddToStore - << name - << ((hashAlgo == htSHA256 && method == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ - << (method == FileIngestionMethod::Recursive ? 1 : 0) - << printHashType(hashAlgo); - - try { - conn->to.written = 0; - conn->to.warn = true; - connections->incCapacity(); - { - Finally cleanup([&]() { connections->decCapacity(); }); - dumpPath(srcPath, conn->to, filter); - } - conn->to.warn = false; - conn.processStderr(); - } catch (SysError & e) { - /* Daemon closed while we were sending the path. Probably OOM - or I/O error. */ - if (e.errNo == EPIPE) - try { - conn.processStderr(); - } catch (EndOfFile & e) { } - throw; - } - - return parseStorePath(readString(conn->from)); -} - - StorePath RemoteStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 91c748006..14c22e764 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,13 +63,12 @@ public: void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) override; + StorePath addToStoreFromDump(Source & dump, const string & name, + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; + void addToStore(const ValidPathInfo & info, Source & nar, RepairFlag repair, CheckSigsFlag checkSigs) override; - StorePath addToStore(const string & name, const Path & srcPath, - FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, - PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override; - StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) override; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 13cf8d4ab..21e05d91c 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -6,7 +6,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION 0x118 +#define PROTOCOL_VERSION 0x119 #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) From c602ebfb34de3626fa0b9110face6ea4b171ac0f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 20:19:15 +0200 Subject: [PATCH 21/60] Refactor wopAddToStore to make wopAddTextToStore obsolete --- src/libstore/remote-store.cc | 91 ++++++++++++++++++--------------- src/libstore/remote-store.hh | 2 + src/libstore/worker-protocol.hh | 2 +- 3 files changed, 53 insertions(+), 42 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 14ad4767d..95e6a3291 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -526,12 +526,8 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & } -StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, - FileIngestionMethod method, HashType hashType, RepairFlag repair) +StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references) { - if (repair) throw Error("repairing is not supported when adding to store through the Nix daemon"); - StorePathSet references; - auto conn(getConnection()); if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 25) { @@ -539,7 +535,7 @@ StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, conn->to << wopAddToStore << name - << renderContentAddressMethod(FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }); + << renderContentAddressMethod(caMethod); writeStorePaths(*this, conn->to, references); conn.withFramedSink([&](Sink & sink) { @@ -552,42 +548,60 @@ StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, else { if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); - conn->to - << wopAddToStore - << name - << ((hashType == htSHA256 && method == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ - << (method == FileIngestionMethod::Recursive ? 1 : 0) - << printHashType(hashType); + std::visit(overloaded { + [&](TextHashMethod thm) -> void { + std::string s = dump.drain(); + conn->to << wopAddTextToStore << name << s; + writeStorePaths(*this, conn->to, references); + conn.processStderr(); + }, + [&](FixedOutputHashMethod fohm) -> void { + conn->to + << wopAddToStore + << name + << ((fohm.hashType == htSHA256 && fohm.fileIngestionMethod == FileIngestionMethod::Recursive) ? 0 : 1) /* backwards compatibility hack */ + << (fohm.fileIngestionMethod == FileIngestionMethod::Recursive ? 1 : 0) + << printHashType(fohm.hashType); - try { - conn->to.written = 0; - conn->to.warn = true; - connections->incCapacity(); - { - Finally cleanup([&]() { connections->decCapacity(); }); - if (method == FileIngestionMethod::Recursive) { - dump.drainInto(conn->to); - } else { - std::string contents = dump.drain(); - dumpString(contents, conn->to); - } - } - conn->to.warn = false; - conn.processStderr(); - } catch (SysError & e) { - /* Daemon closed while we were sending the path. Probably OOM - or I/O error. */ - if (e.errNo == EPIPE) try { + conn->to.written = 0; + conn->to.warn = true; + connections->incCapacity(); + { + Finally cleanup([&]() { connections->decCapacity(); }); + if (fohm.fileIngestionMethod == FileIngestionMethod::Recursive) { + dump.drainInto(conn->to); + } else { + std::string contents = dump.drain(); + dumpString(contents, conn->to); + } + } + conn->to.warn = false; conn.processStderr(); - } catch (EndOfFile & e) { } - throw; - } + } catch (SysError & e) { + /* Daemon closed while we were sending the path. Probably OOM + or I/O error. */ + if (e.errNo == EPIPE) + try { + conn.processStderr(); + } catch (EndOfFile & e) { } + throw; + } + } + }, caMethod); return parseStorePath(readString(conn->from)); } } +StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, + FileIngestionMethod method, HashType hashType, RepairFlag repair) +{ + if (repair) throw Error("repairing is not supported when adding to store through the Nix daemon"); + StorePathSet references; + return addCAToStore(dump, name, FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }, references); +} + void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, RepairFlag repair, CheckSigsFlag checkSigs) @@ -646,13 +660,8 @@ StorePath RemoteStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { if (repair) throw Error("repairing is not supported when building through the Nix daemon"); - - auto conn(getConnection()); - conn->to << wopAddTextToStore << name << s; - writeStorePaths(*this, conn->to, references); - - conn.processStderr(); - return parseStorePath(readString(conn->from)); + StringSource source(s); + return addCAToStore(source, name, TextHashMethod{}, references); } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 14c22e764..2c775cb79 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,6 +63,8 @@ public: void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) override; + StorePath addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references); + StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index 21e05d91c..b100d1550 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -18,7 +18,7 @@ typedef enum { wopQueryReferences = 5, // obsolete wopQueryReferrers = 6, wopAddToStore = 7, - wopAddTextToStore = 8, + wopAddTextToStore = 8, // obsolete since 1.25, Nix 3.0. Use wopAddToStore wopBuildPaths = 9, wopEnsurePath = 10, wopAddTempRoot = 11, From ecc8088cb7e91e4c45787f290c8ff61547fb838a Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 21:32:01 +0200 Subject: [PATCH 22/60] wopAddToStore: Throw to clarify unused refs Co-authored-by: John Ericson --- src/libstore/daemon.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 0a08a2e12..f2d789699 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -366,6 +366,8 @@ static void performOp(TunnelLogger * logger, ref store, return store->addTextToStore(name, contents, refs); }, [&](FixedOutputHashMethod &fohm) -> StorePath { + if (!refs.empty()) + throw UnimplementedError("cannot yet have refs with flat or nar-hashed data"); return store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType); }, }, contentAddressMethod); From 8279178b078103f816bf85f5a153a4845d30cc83 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 22:01:35 +0200 Subject: [PATCH 23/60] Move FramedSink next to FramedSource --- src/libstore/remote-store.cc | 35 +-------------------------- src/libutil/serialise.hh | 46 ++++++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 36 deletions(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 95e6a3291..cfbf23ac4 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -957,39 +957,6 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } - -struct FramedSink : nix::BufferedSink -{ - ConnectionHandle & conn; - std::exception_ptr & ex; - - FramedSink(ConnectionHandle & conn, std::exception_ptr & ex) : conn(conn), ex(ex) - { } - - ~FramedSink() - { - try { - conn->to << 0; - conn->to.flush(); - } catch (...) { - ignoreException(); - } - } - - void write(const unsigned char * data, size_t len) override - { - /* Don't send more data if the remote has - encountered an error. */ - if (ex) { - auto ex2 = ex; - ex = nullptr; - std::rethrow_exception(ex2); - } - conn->to << len; - conn->to(data, len); - }; -}; - void ConnectionHandle::withFramedSink(std::function fun) { (*this)->to.flush(); @@ -1022,7 +989,7 @@ ConnectionHandle::withFramedSink(std::function fun) { }); { - FramedSink sink(*this, ex); + FramedSink sink((*this)->to, ex); fun(sink); sink.flush(); } diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 6027d5961..3511e4cd8 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -406,8 +406,13 @@ struct StreamToSourceAdapter : Source }; -/* Like SizedSource, but guarantees that the whole frame is consumed after - destruction. This ensures that the original stream is in a known state. */ +/* A source that reads a distinct format of concatenated chunks back into its + logical form, in order to guarantee a known state to the original stream, + even in the event of errors. + + Use with FramedSink, which also allows the logical stream to be terminated + in the event of an exception. +*/ struct FramedSource : Source { Source & from; @@ -452,5 +457,42 @@ struct FramedSource : Source } }; +/* Write as chunks in the format expected by FramedSource. + + The exception_ptr reference can be used to terminate the stream when you + detect that an error has occurred on the remote end. +*/ +struct FramedSink : nix::BufferedSink +{ + BufferedSink & to; + std::exception_ptr & ex; + + FramedSink(BufferedSink & to, std::exception_ptr & ex) : to(to), ex(ex) + { } + + ~FramedSink() + { + try { + to << 0; + to.flush(); + } catch (...) { + ignoreException(); + } + } + + void write(const unsigned char * data, size_t len) override + { + /* Don't send more data if the remote has + encountered an error. */ + if (ex) { + auto ex2 = ex; + ex = nullptr; + std::rethrow_exception(ex2); + } + to << len; + to(data, len); + }; +}; + } From fbf509c1137fd59ebb3e7a993c8da42c17deb68a Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 17 Sep 2020 22:04:05 +0200 Subject: [PATCH 24/60] parseContentAddressMethodPrefix: use string_view Co-authored-by: John Ericson --- src/libstore/content-address.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index bf56d3d77..74215c545 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -52,7 +52,7 @@ std::string renderContentAddressMethod(ContentAddressMethod cam) { Parses content address strings up to the hash. */ static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest) { - std::string wholeInput(rest); + std::string_view wholeInput { rest }; std::string_view prefix; { From 7c682640857106f18d7020c9c75ea39b1ef8dd2c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 18 Sep 2020 10:06:34 +0200 Subject: [PATCH 25/60] wopAddToStore: add RepairFlag --- src/libstore/daemon.cc | 5 ++++- src/libstore/remote-store.cc | 9 ++++----- src/libstore/remote-store.hh | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index f2d789699..9f7350217 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -353,6 +353,9 @@ static void performOp(TunnelLogger * logger, ref store, auto name = readString(from); auto camStr = readString(from); auto refs = readStorePaths(*store, from); + bool repairBool; + from >> repairBool; + auto repair = RepairFlag{repairBool}; logger->startWork(); StorePath path = [&]() -> StorePath { @@ -368,7 +371,7 @@ static void performOp(TunnelLogger * logger, ref store, [&](FixedOutputHashMethod &fohm) -> StorePath { if (!refs.empty()) throw UnimplementedError("cannot yet have refs with flat or nar-hashed data"); - return store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType); + return store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair); }, }, contentAddressMethod); }(); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index cfbf23ac4..54d70e3e1 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -526,7 +526,7 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & } -StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references) +StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair) { auto conn(getConnection()); @@ -537,6 +537,7 @@ StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentA << name << renderContentAddressMethod(caMethod); writeStorePaths(*this, conn->to, references); + conn->to << repair; conn.withFramedSink([&](Sink & sink) { dump.drainInto(sink); @@ -597,9 +598,8 @@ StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentA StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method, HashType hashType, RepairFlag repair) { - if (repair) throw Error("repairing is not supported when adding to store through the Nix daemon"); StorePathSet references; - return addCAToStore(dump, name, FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }, references); + return addCAToStore(dump, name, FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }, references, repair); } @@ -659,9 +659,8 @@ void RemoteStore::addToStore(const ValidPathInfo & info, Source & source, StorePath RemoteStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { - if (repair) throw Error("repairing is not supported when building through the Nix daemon"); StringSource source(s); - return addCAToStore(source, name, TextHashMethod{}, references); + return addCAToStore(source, name, TextHashMethod{}, references, repair); } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 2c775cb79..23f7b8425 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,7 +63,7 @@ public: void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) override; - StorePath addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references); + StorePath addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair); StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; From fa08db5c4c92ed4674cfacd131d1b45906a9b146 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 18 Sep 2020 11:22:13 +0200 Subject: [PATCH 26/60] wopAddToStore: return ValidPathInfo A ValidPathInfo is created anyway. By returning it we can save a roundtrip and we have a nicer interface. --- src/libstore/daemon.cc | 38 ++++++++++++++++++------------- src/libstore/remote-store.cc | 43 +++++++++++++++++++++--------------- src/libstore/remote-store.hh | 4 +++- 3 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 9f7350217..8897a73f4 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -239,6 +239,18 @@ struct ClientSettings } }; +static void writeValidPathInfo(ref store, unsigned int clientVersion, Sink & to, std::shared_ptr info) { + to << (info->deriver ? store->printStorePath(*info->deriver) : "") + << info->narHash.to_string(Base16, false); + writeStorePaths(*store, to, info->references); + to << info->registrationTime << info->narSize; + if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { + to << info->ultimate + << info->sigs + << renderContentAddress(info->ca); + } +} + static void performOp(TunnelLogger * logger, ref store, TrustedFlag trusted, RecursiveFlag recursive, unsigned int clientVersion, Source & from, BufferedSink & to, unsigned int op) @@ -358,26 +370,30 @@ static void performOp(TunnelLogger * logger, ref store, auto repair = RepairFlag{repairBool}; logger->startWork(); - StorePath path = [&]() -> StorePath { + auto pathInfo = [&]() { // NB: FramedSource must be out of scope before logger->stopWork(); ContentAddressMethod contentAddressMethod = parseContentAddressMethod(camStr); FramedSource source(from); + // TODO this is essentially RemoteStore::addCAToStore. Move it up to Store. return std::visit(overloaded { - [&](TextHashMethod &_) -> StorePath { + [&](TextHashMethod &_) { // We could stream this by changing Store std::string contents = source.drain(); - return store->addTextToStore(name, contents, refs); + auto path = store->addTextToStore(name, contents, refs, repair); + return store->queryPathInfo(path); }, - [&](FixedOutputHashMethod &fohm) -> StorePath { + [&](FixedOutputHashMethod &fohm) { if (!refs.empty()) throw UnimplementedError("cannot yet have refs with flat or nar-hashed data"); - return store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair); + auto path = store->addToStoreFromDump(source, name, fohm.fileIngestionMethod, fohm.hashType, repair); + return store->queryPathInfo(path); }, }, contentAddressMethod); }(); logger->stopWork(); - to << store->printStorePath(path); + to << store->printStorePath(pathInfo->path); + writeValidPathInfo(store, clientVersion, to, pathInfo); } else { HashType hashAlgo; @@ -706,15 +722,7 @@ static void performOp(TunnelLogger * logger, ref store, if (info) { if (GET_PROTOCOL_MINOR(clientVersion) >= 17) to << 1; - to << (info->deriver ? store->printStorePath(*info->deriver) : "") - << info->narHash.to_string(Base16, false); - writeStorePaths(*store, to, info->references); - to << info->registrationTime << info->narSize; - if (GET_PROTOCOL_MINOR(clientVersion) >= 16) { - to << info->ultimate - << info->sigs - << renderContentAddress(info->ca); - } + writeValidPathInfo(store, clientVersion, to, info); } else { assert(GET_PROTOCOL_MINOR(clientVersion) >= 17); to << 0; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 54d70e3e1..3936ddb1d 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -421,11 +421,27 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S } +ref RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) { + auto deriver = readString(conn->from); + auto narHash = Hash::parseAny(readString(conn->from), htSHA256); + auto info = make_ref(path, narHash); + if (deriver != "") info->deriver = parseStorePath(deriver); + info->references = readStorePaths(*this, conn->from); + conn->from >> info->registrationTime >> info->narSize; + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { + conn->from >> info->ultimate; + info->sigs = readStrings(conn->from); + info->ca = parseContentAddressOpt(readString(conn->from)); + } + return info; +} + + void RemoteStore::queryPathInfoUncached(const StorePath & path, Callback> callback) noexcept { try { - std::shared_ptr info; + std::shared_ptr info; { auto conn(getConnection()); conn->to << wopQueryPathInfo << printStorePath(path); @@ -441,17 +457,7 @@ void RemoteStore::queryPathInfoUncached(const StorePath & path, bool valid; conn->from >> valid; if (!valid) throw InvalidPath("path '%s' is not valid", printStorePath(path)); } - auto deriver = readString(conn->from); - auto narHash = Hash::parseAny(readString(conn->from), htSHA256); - info = std::make_shared(path, narHash); - if (deriver != "") info->deriver = parseStorePath(deriver); - info->references = readStorePaths(*this, conn->from); - conn->from >> info->registrationTime >> info->narSize; - if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 16) { - conn->from >> info->ultimate; - info->sigs = readStrings(conn->from); - info->ca = parseContentAddressOpt(readString(conn->from)); - } + info = readValidPathInfo(conn, path); } callback(std::move(info)); } catch (...) { callback.rethrow(); } @@ -526,7 +532,7 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & } -StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair) +ref RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair) { auto conn(getConnection()); @@ -543,8 +549,8 @@ StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentA dump.drainInto(sink); }); - return parseStorePath(readString(conn->from)); - + auto path = parseStorePath(readString(conn->from)); + return readValidPathInfo(conn, path); } else { if (repair) throw Error("repairing is not supported when building through the Nix daemon protocol < 1.25"); @@ -591,7 +597,8 @@ StorePath RemoteStore::addCAToStore(Source & dump, const string & name, ContentA } }, caMethod); - return parseStorePath(readString(conn->from)); + auto path = parseStorePath(readString(conn->from)); + return queryPathInfo(path); } } @@ -599,7 +606,7 @@ StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method, HashType hashType, RepairFlag repair) { StorePathSet references; - return addCAToStore(dump, name, FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }, references, repair); + return addCAToStore(dump, name, FixedOutputHashMethod{ .fileIngestionMethod = method, .hashType = hashType }, references, repair)->path; } @@ -660,7 +667,7 @@ StorePath RemoteStore::addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair) { StringSource source(s); - return addCAToStore(source, name, TextHashMethod{}, references, repair); + return addCAToStore(source, name, TextHashMethod{}, references, repair)->path; } diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 23f7b8425..47f43ae8a 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,7 +63,7 @@ public: void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) override; - StorePath addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair); + ref addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair); StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; @@ -140,6 +140,8 @@ protected: virtual void narFromPath(const StorePath & path, Sink & sink) override; + ref readValidPathInfo(ConnectionHandle & conn, const StorePath & path); + private: std::atomic_bool failed{false}; From ca30abb3fb36440e5a13161c39647189036fc18f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 18 Sep 2020 11:58:45 +0200 Subject: [PATCH 27/60] Document addCAToStore/addToStoreFromDump source drainage Also checked that all usages satisfy the requirement and removed dead code. --- src/libstore/build.cc | 8 -------- src/libstore/remote-store.hh | 2 ++ src/libstore/store-api.hh | 3 ++- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 6e55f83d5..061f07546 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2949,14 +2949,6 @@ struct RestrictedStore : public LocalFSStore, public virtual RestrictedStoreConf goal.addDependency(info.path); } - StorePath addToStoreFromDump(Source & dump, const string & name, - FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override - { - auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair); - goal.addDependency(path); - return path; - } - StorePath addTextToStore(const string & name, const string & s, const StorePathSet & references, RepairFlag repair = NoRepair) override { diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 47f43ae8a..735a3c24e 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -63,8 +63,10 @@ public: void querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) override; + /* Add a content-addressable store path. `dump` will be drained. */ ref addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair); + /* Add a content-addressable store path. Does not support references. `dump` will be drained. */ StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 4d3f07dfc..591140874 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -449,7 +449,8 @@ public: /* Like addToStore(), but the contents of the path are contained in `dump', which is either a NAR serialisation (if recursive == true) or simply the contents of a regular file (if recursive == - false). */ + false). + `dump` may be drained */ // FIXME: remove? virtual StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) From 9aa0dafe205899f350382b587e284c493b223cdd Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 21 Sep 2020 13:11:31 +0200 Subject: [PATCH 28/60] Update lowdown version Fix #4042 According to https://github.com/kristapsdz/lowdown/commit/8aef9e9290de22a10c14ae138257bc1c7fa8ba1f, we shouldn't need to use a fork anymore so we can switch back to upstream --- flake.lock | 11 +++++------ flake.nix | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/flake.lock b/flake.lock index f4368b170..822a73332 100644 --- a/flake.lock +++ b/flake.lock @@ -3,16 +3,15 @@ "lowdown-src": { "flake": false, "locked": { - "lastModified": 1598296217, - "narHash": "sha256-ha7lyNY1d8m+osmDpPc9f/bfZ3ZC1IVIXwfyklSWg8I=", - "owner": "edolstra", + "lastModified": 1598695561, + "narHash": "sha256-gyH/5j+h/nWw0W8AcR2WKvNBUsiQ7QuxqSJNXAwV+8E=", + "owner": "kristapsdz", "repo": "lowdown", - "rev": "c7a4e715af1e233080842db82d15b261cb74cb28", + "rev": "1705b4a26fbf065d9574dce47a94e8c7c79e052f", "type": "github" }, "original": { - "owner": "edolstra", - "ref": "no-structs-in-anonymous-unions", + "owner": "kristapsdz", "repo": "lowdown", "type": "github" } diff --git a/flake.nix b/flake.nix index a50533a29..1b9eb4c77 100644 --- a/flake.nix +++ b/flake.nix @@ -2,7 +2,7 @@ description = "The purely functional package manager"; inputs.nixpkgs.url = "nixpkgs/nixos-20.03-small"; - inputs.lowdown-src = { url = "github:edolstra/lowdown/no-structs-in-anonymous-unions"; flake = false; }; + inputs.lowdown-src = { url = "github:kristapsdz/lowdown"; flake = false; }; outputs = { self, nixpkgs, lowdown-src }: From d110fdd03f8860b2a1cd689187f8056b9e22af09 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 13:28:51 +0200 Subject: [PATCH 29/60] Disable precompiled headers in 'nix develop' They're still enabled in regular builds though. --- flake.nix | 5 +---- mk/precompiled-headers.mk | 2 +- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/flake.nix b/flake.nix index 1b9eb4c77..0304557e8 100644 --- a/flake.nix +++ b/flake.nix @@ -136,7 +136,7 @@ enableParallelBuilding = true; - makeFlags = "profiledir=$(out)/etc/profile.d"; + makeFlags = "profiledir=$(out)/etc/profile.d PRECOMPILE_HEADERS=1"; doCheck = true; @@ -334,9 +334,6 @@ # syntax-check generated dot files, it still requires some # fonts. So provide those. FONTCONFIG_FILE = texFunctions.fontsConf; - - # To test building without precompiled headers. - makeFlagsArray = [ "PRECOMPILE_HEADERS=0" ]; }; # System tests. diff --git a/mk/precompiled-headers.mk b/mk/precompiled-headers.mk index 500c99e4a..1fdb4b3a4 100644 --- a/mk/precompiled-headers.mk +++ b/mk/precompiled-headers.mk @@ -1,4 +1,4 @@ -PRECOMPILE_HEADERS ?= 1 +PRECOMPILE_HEADERS ?= 0 print-var-help += \ echo " PRECOMPILE_HEADERS ($(PRECOMPILE_HEADERS)): Whether to use precompiled headers to speed up the build"; From 56f1e0df0546c744421c7d1bd31b1a341796513c Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 21 Sep 2020 16:29:08 +0200 Subject: [PATCH 30/60] libfetchers/github: rename `url` to `host` --- src/libfetchers/github.cc | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index f510ae5f5..a4db5c5fa 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -8,9 +8,9 @@ namespace nix::fetchers { -// A github or gitlab url -const static std::string urlRegexS = "[a-zA-Z0-9.]*"; // FIXME: check -std::regex urlRegex(urlRegexS, std::regex::ECMAScript); +// A github or gitlab host +const static std::string hostRegexS = "[a-zA-Z0-9.]*"; // FIXME: check +std::regex hostRegex(hostRegexS, std::regex::ECMAScript); struct GitArchiveInputScheme : InputScheme { @@ -50,9 +50,9 @@ struct GitArchiveInputScheme : InputScheme throw BadURL("URL '%s' contains multiple branch/tag names", url.url); ref = value; } - else if (name == "url") { - if (!std::regex_match(value, urlRegex)) - throw BadURL("URL '%s' contains an invalid instance url", url.url); + else if (name == "host") { + if (!std::regex_match(value, hostRegex)) + throw BadURL("URL '%s' contains an invalid instance host", url.url); host_url = value; } // FIXME: barf on unsupported attributes @@ -67,7 +67,7 @@ struct GitArchiveInputScheme : InputScheme input.attrs.insert_or_assign("repo", path[1]); if (rev) input.attrs.insert_or_assign("rev", rev->gitRev()); if (ref) input.attrs.insert_or_assign("ref", *ref); - if (host_url) input.attrs.insert_or_assign("url", *host_url); + if (host_url) input.attrs.insert_or_assign("host", *host_url); return input; } @@ -77,7 +77,7 @@ struct GitArchiveInputScheme : InputScheme if (maybeGetStrAttr(attrs, "type") != type()) return {}; for (auto & [name, value] : attrs) - if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "url") + if (name != "type" && name != "owner" && name != "repo" && name != "ref" && name != "rev" && name != "narHash" && name != "lastModified" && name != "host") throw Error("unsupported input attribute '%s'", name); getStrAttr(attrs, "owner"); @@ -210,7 +210,7 @@ struct GitHubInputScheme : GitArchiveInputScheme { // FIXME: use regular /archive URLs instead? api.github.com // might have stricter rate limits. - auto host_url = maybeGetStrAttr(input.attrs, "url").value_or("github.com"); + auto host_url = maybeGetStrAttr(input.attrs, "host").value_or("github.com"); auto url = fmt("https://api.%s/repos/%s/%s/tarball/%s", // FIXME: check if this is correct for self hosted instances host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), input.getRev()->to_string(Base16, false)); @@ -236,7 +236,7 @@ struct GitLabInputScheme : GitArchiveInputScheme Hash getRevFromRef(nix::ref store, const Input & input) const override { - auto host_url = maybeGetStrAttr(input.attrs, "url").value_or("gitlab.com"); + auto host_url = maybeGetStrAttr(input.attrs, "host").value_or("gitlab.com"); auto url = fmt("https://%s/api/v4/projects/%s%%2F%s/repository/commits?ref_name=%s", host_url, getStrAttr(input.attrs, "owner"), getStrAttr(input.attrs, "repo"), *input.getRef()); auto json = nlohmann::json::parse( From 4e1a04733d5075fdc09dbc6767755d4487e96da7 Mon Sep 17 00:00:00 2001 From: Marwan Aljubeh Date: Mon, 21 Sep 2020 16:32:22 +0100 Subject: [PATCH 31/60] Use a better name for the config option --- src/libstore/globals.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index ddc13898d..fcb9b0f63 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -883,7 +883,7 @@ public: "Path or URI of the global flake registry."}; Setting ignoreSymlinkStore{ - this, false, "ignore-symlink-store", + this, false, "allow-symlinked-store", R"( If set to `true`, Nix will stop complaining if the store directory (typically /nix/store) contains symlink components. From e8e1d420f364afbfface61d3f03889e10e6066c9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 18:22:45 +0200 Subject: [PATCH 32/60] Don't include in header files This reduces compilation time by ~15 seconds (CPU time). Issue #4045. --- src/libexpr/eval.cc | 1 + src/libexpr/eval.hh | 8 +++-- src/libexpr/flake/flakeref.cc | 1 + src/libexpr/flake/lockfile.cc | 1 + src/libexpr/primops.cc | 18 ++++++++--- src/libexpr/primops/fetchMercurial.cc | 3 +- src/libfetchers/git.cc | 1 + src/libfetchers/github.cc | 1 + src/libfetchers/indirect.cc | 1 + src/libfetchers/mercurial.cc | 1 + src/libstore/names.cc | 21 +++++++++++-- src/libstore/names.hh | 7 +++-- src/libutil/url-parts.hh | 44 +++++++++++++++++++++++++++ src/libutil/url.cc | 1 + src/libutil/url.hh | 38 ----------------------- src/nix-env/nix-env.cc | 2 +- 16 files changed, 96 insertions(+), 53 deletions(-) create mode 100644 src/libutil/url-parts.hh diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 139067f20..883fc27a7 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -356,6 +356,7 @@ EvalState::EvalState(const Strings & _searchPath, ref store) , sEpsilon(symbols.create("")) , repair(NoRepair) , store(store) + , regexCache(makeRegexCache()) , baseEnv(allocEnv(128)) , staticBaseEnv(false, 0) { diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 80078d8a5..0e1f61baa 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -6,7 +6,6 @@ #include "symbol-table.hh" #include "config.hh" -#include #include #include #include @@ -65,6 +64,11 @@ typedef std::list SearchPath; void initGC(); +struct RegexCache; + +std::shared_ptr makeRegexCache(); + + class EvalState { public: @@ -120,7 +124,7 @@ private: std::unordered_map resolvedPaths; /* Cache used by prim_match(). */ - std::unordered_map regexCache; + std::shared_ptr regexCache; public: diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 6363446f6..d5c2ffe66 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -1,6 +1,7 @@ #include "flakeref.hh" #include "store-api.hh" #include "url.hh" +#include "url-parts.hh" #include "fetchers.hh" #include "registry.hh" diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index a74846944..78431f000 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -1,5 +1,6 @@ #include "lockfile.hh" #include "store-api.hh" +#include "url-parts.hh" #include diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 7e8526ea1..9cfe3f402 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3085,17 +3085,25 @@ static RegisterPrimOp primop_hashString({ .fun = prim_hashString, }); -/* Match a regular expression against a string and return either - ‘null’ or a list containing substring matches. */ +struct RegexCache +{ + std::unordered_map cache; +}; + +std::shared_ptr makeRegexCache() +{ + return std::make_shared(); +} + void prim_match(EvalState & state, const Pos & pos, Value * * args, Value & v) { auto re = state.forceStringNoCtx(*args[0], pos); try { - auto regex = state.regexCache.find(re); - if (regex == state.regexCache.end()) - regex = state.regexCache.emplace(re, std::regex(re, std::regex::extended)).first; + auto regex = state.regexCache->cache.find(re); + if (regex == state.regexCache->cache.end()) + regex = state.regexCache->cache.emplace(re, std::regex(re, std::regex::extended)).first; PathSet context; const std::string str = state.forceString(*args[1], context, pos); diff --git a/src/libexpr/primops/fetchMercurial.cc b/src/libexpr/primops/fetchMercurial.cc index cef85cfef..1a064ed5c 100644 --- a/src/libexpr/primops/fetchMercurial.cc +++ b/src/libexpr/primops/fetchMercurial.cc @@ -3,8 +3,7 @@ #include "store-api.hh" #include "fetchers.hh" #include "url.hh" - -#include +#include "url-parts.hh" namespace nix { diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 5ca0f8521..ad7638d73 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -3,6 +3,7 @@ #include "globals.hh" #include "tarfile.hh" #include "store-api.hh" +#include "url-parts.hh" #include diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index a4db5c5fa..1737658a7 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -3,6 +3,7 @@ #include "fetchers.hh" #include "globals.hh" #include "store-api.hh" +#include "url-parts.hh" #include diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index b981d4d8e..74332ae3d 100644 --- a/src/libfetchers/indirect.cc +++ b/src/libfetchers/indirect.cc @@ -1,4 +1,5 @@ #include "fetchers.hh" +#include "url-parts.hh" namespace nix::fetchers { diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 3e76ffc4d..d80c2ea7a 100644 --- a/src/libfetchers/mercurial.cc +++ b/src/libfetchers/mercurial.cc @@ -3,6 +3,7 @@ #include "globals.hh" #include "tarfile.hh" #include "store-api.hh" +#include "url-parts.hh" #include diff --git a/src/libstore/names.cc b/src/libstore/names.cc index d1c8a6101..41e28dc99 100644 --- a/src/libstore/names.cc +++ b/src/libstore/names.cc @@ -1,10 +1,18 @@ #include "names.hh" #include "util.hh" +#include + namespace nix { +struct Regex +{ + std::regex regex; +}; + + DrvName::DrvName() { name = ""; @@ -30,11 +38,18 @@ DrvName::DrvName(std::string_view s) : hits(0) } +DrvName::~DrvName() +{ } + + bool DrvName::matches(DrvName & n) { if (name != "*") { - if (!regex) regex = std::unique_ptr(new std::regex(name, std::regex::extended)); - if (!std::regex_match(n.name, *regex)) return false; + if (!regex) { + regex = std::make_unique(); + regex->regex = std::regex(name, std::regex::extended); + } + if (!std::regex_match(n.name, regex->regex)) return false; } if (version != "" && version != n.version) return false; return true; @@ -99,7 +114,7 @@ DrvNames drvNamesFromArgs(const Strings & opArgs) { DrvNames result; for (auto & i : opArgs) - result.push_back(DrvName(i)); + result.emplace_back(i); return result; } diff --git a/src/libstore/names.hh b/src/libstore/names.hh index 00e14b8c7..bc62aac93 100644 --- a/src/libstore/names.hh +++ b/src/libstore/names.hh @@ -3,10 +3,11 @@ #include #include "types.hh" -#include namespace nix { +struct Regex; + struct DrvName { string fullName; @@ -16,10 +17,12 @@ struct DrvName DrvName(); DrvName(std::string_view s); + ~DrvName(); + bool matches(DrvName & n); private: - std::unique_ptr regex; + std::unique_ptr regex; }; typedef list DrvNames; diff --git a/src/libutil/url-parts.hh b/src/libutil/url-parts.hh new file mode 100644 index 000000000..64e06cfbc --- /dev/null +++ b/src/libutil/url-parts.hh @@ -0,0 +1,44 @@ +#pragma once + +#include +#include + +namespace nix { + +// URI stuff. +const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])"; +const static std::string schemeRegex = "(?:[a-z+.-]+)"; +const static std::string ipv6AddressRegex = "(?:\\[[0-9a-fA-F:]+\\])"; +const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])"; +const static std::string subdelimsRegex = "(?:[!$&'\"()*+,;=])"; +const static std::string hostnameRegex = "(?:(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + ")*)"; +const static std::string hostRegex = "(?:" + ipv6AddressRegex + "|" + hostnameRegex + ")"; +const static std::string userRegex = "(?:(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + "|:)*)"; +const static std::string authorityRegex = "(?:" + userRegex + "@)?" + hostRegex + "(?::[0-9]+)?"; +const static std::string pcharRegex = "(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + "|[:@])"; +const static std::string queryRegex = "(?:" + pcharRegex + "|[/? \"])*"; +const static std::string segmentRegex = "(?:" + pcharRegex + "+)"; +const static std::string absPathRegex = "(?:(?:/" + segmentRegex + ")*/?)"; +const static std::string pathRegex = "(?:" + segmentRegex + "(?:/" + segmentRegex + ")*/?)"; + +// A Git ref (i.e. branch or tag name). +const static std::string refRegexS = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"; // FIXME: check +extern std::regex refRegex; + +// Instead of defining what a good Git Ref is, we define what a bad Git Ref is +// This is because of the definition of a ref in refs.c in https://github.com/git/git +// See tests/fetchGitRefs.sh for the full definition +const static std::string badGitRefRegexS = "//|^[./]|/\\.|\\.\\.|[[:cntrl:][:space:]:?^~\[]|\\\\|\\*|\\.lock$|\\.lock/|@\\{|[/.]$|^@$|^$"; +extern std::regex badGitRefRegex; + +// A Git revision (a SHA-1 commit hash). +const static std::string revRegexS = "[0-9a-fA-F]{40}"; +extern std::regex revRegex; + +// A ref or revision, or a ref followed by a revision. +const static std::string refAndOrRevRegex = "(?:(" + revRegexS + ")|(?:(" + refRegexS + ")(?:/(" + revRegexS + "))?))"; + +const static std::string flakeIdRegexS = "[a-zA-Z][a-zA-Z0-9_-]*"; +extern std::regex flakeIdRegex; + +} diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 88c09eef9..c1bab866c 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -1,4 +1,5 @@ #include "url.hh" +#include "url-parts.hh" #include "util.hh" namespace nix { diff --git a/src/libutil/url.hh b/src/libutil/url.hh index 1f716ba10..6e77142e3 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -2,8 +2,6 @@ #include "error.hh" -#include - namespace nix { struct ParsedURL @@ -29,40 +27,4 @@ std::map decodeQuery(const std::string & query); ParsedURL parseURL(const std::string & url); -// URI stuff. -const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])"; -const static std::string schemeRegex = "(?:[a-z+.-]+)"; -const static std::string ipv6AddressRegex = "(?:\\[[0-9a-fA-F:]+\\])"; -const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])"; -const static std::string subdelimsRegex = "(?:[!$&'\"()*+,;=])"; -const static std::string hostnameRegex = "(?:(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + ")*)"; -const static std::string hostRegex = "(?:" + ipv6AddressRegex + "|" + hostnameRegex + ")"; -const static std::string userRegex = "(?:(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + "|:)*)"; -const static std::string authorityRegex = "(?:" + userRegex + "@)?" + hostRegex + "(?::[0-9]+)?"; -const static std::string pcharRegex = "(?:" + unreservedRegex + "|" + pctEncoded + "|" + subdelimsRegex + "|[:@])"; -const static std::string queryRegex = "(?:" + pcharRegex + "|[/? \"])*"; -const static std::string segmentRegex = "(?:" + pcharRegex + "+)"; -const static std::string absPathRegex = "(?:(?:/" + segmentRegex + ")*/?)"; -const static std::string pathRegex = "(?:" + segmentRegex + "(?:/" + segmentRegex + ")*/?)"; - -// A Git ref (i.e. branch or tag name). -const static std::string refRegexS = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"; // FIXME: check -extern std::regex refRegex; - -// Instead of defining what a good Git Ref is, we define what a bad Git Ref is -// This is because of the definition of a ref in refs.c in https://github.com/git/git -// See tests/fetchGitRefs.sh for the full definition -const static std::string badGitRefRegexS = "//|^[./]|/\\.|\\.\\.|[[:cntrl:][:space:]:?^~\[]|\\\\|\\*|\\.lock$|\\.lock/|@\\{|[/.]$|^@$|^$"; -extern std::regex badGitRefRegex; - -// A Git revision (a SHA-1 commit hash). -const static std::string revRegexS = "[0-9a-fA-F]{40}"; -extern std::regex revRegex; - -// A ref or revision, or a ref followed by a revision. -const static std::string refAndOrRevRegex = "(?:(" + revRegexS + ")|(?:(" + refRegexS + ")(?:/(" + revRegexS + "))?))"; - -const static std::string flakeIdRegexS = "[a-zA-Z][a-zA-Z0-9_-]*"; -extern std::regex flakeIdRegex; - } diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index e5a433ac0..3e7c453fb 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -230,7 +230,7 @@ static DrvInfos filterBySelector(EvalState & state, const DrvInfos & allElems, { DrvNames selectors = drvNamesFromArgs(args); if (selectors.empty()) - selectors.push_back(DrvName("*")); + selectors.emplace_back("*"); DrvInfos elems; set done; From f80ffeb8c9291f7168f098fdaadc15408492f3c2 Mon Sep 17 00:00:00 2001 From: Marwan Aljubeh Date: Mon, 21 Sep 2020 17:29:08 +0100 Subject: [PATCH 33/60] Update the variable name accordingly --- src/libstore/globals.cc | 2 +- src/libstore/globals.hh | 2 +- src/libstore/local-store.cc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index ca2e75603..ff6082aac 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -41,7 +41,7 @@ Settings::Settings() { buildUsersGroup = getuid() == 0 ? "nixbld" : ""; lockCPU = getEnv("NIX_AFFINITY_HACK") == "1"; - ignoreSymlinkStore = getEnv("NIX_IGNORE_SYMLINK_STORE") == "1"; + allowSymlinkedStore = getEnv("NIX_IGNORE_SYMLINK_STORE") == "1"; caFile = getEnv("NIX_SSL_CERT_FILE").value_or(getEnv("SSL_CERT_FILE").value_or("")); if (caFile == "") { diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index fcb9b0f63..fd0c6cbcc 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -882,7 +882,7 @@ public: Setting flakeRegistry{this, "https://github.com/NixOS/flake-registry/raw/master/flake-registry.json", "flake-registry", "Path or URI of the global flake registry."}; - Setting ignoreSymlinkStore{ + Setting allowSymlinkedStore{ this, false, "allow-symlinked-store", R"( If set to `true`, Nix will stop complaining if the store directory diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 24b9ea7bd..cc9fcfe6e 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -109,7 +109,7 @@ LocalStore::LocalStore(const Params & params) } /* Ensure that the store and its parents are not symlinks. */ - if (!settings.ignoreSymlinkStore) { + if (!settings.allowSymlinkedStore) { Path path = realStoreDir; struct stat st; while (path != "/") { From d51ba430473368b29abfd1a8c4655da74b3a780c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 18:40:11 +0200 Subject: [PATCH 34/60] Move Callback into its own header This gets rid of the inclusion of in util.hh, cutting compilation time by ~20s (CPU time). Issue #4045. --- src/libstore/binary-cache-store.cc | 1 + src/libstore/build.cc | 1 + src/libstore/dummy-store.cc | 1 + src/libstore/filetransfer.cc | 1 + src/libstore/http-binary-cache-store.cc | 1 + src/libstore/legacy-ssh-store.cc | 1 + src/libstore/local-store.cc | 1 + src/libstore/misc.cc | 2 +- src/libstore/remote-store.cc | 1 + src/libstore/store-api.cc | 4 +-- src/libutil/callback.hh | 46 +++++++++++++++++++++++++ src/libutil/util.hh | 38 +------------------- 12 files changed, 57 insertions(+), 41 deletions(-) create mode 100644 src/libutil/callback.hh diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index 34f844a18..ebc0bd6a4 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -11,6 +11,7 @@ #include "nar-accessor.hh" #include "json.hh" #include "thread-pool.hh" +#include "callback.hh" #include #include diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 6e55f83d5..0b51d90ea 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -17,6 +17,7 @@ #include "daemon.hh" #include "worker-protocol.hh" #include "topo-sort.hh" +#include "callback.hh" #include #include diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index 128832e60..49641c2ac 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -1,4 +1,5 @@ #include "store-api.hh" +#include "callback.hh" namespace nix { diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 4149f8155..6241b5e00 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -5,6 +5,7 @@ #include "s3.hh" #include "compression.hh" #include "finally.hh" +#include "callback.hh" #ifdef ENABLE_S3 #include diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index f4ab15a10..86be7c006 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -2,6 +2,7 @@ #include "filetransfer.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" +#include "callback.hh" namespace nix { diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index e9478c1d5..5af75669a 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -6,6 +6,7 @@ #include "worker-protocol.hh" #include "ssh.hh" #include "derivations.hh" +#include "callback.hh" namespace nix { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c618203f0..94ff34cb8 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -6,6 +6,7 @@ #include "derivations.hh" #include "nar-info.hh" #include "references.hh" +#include "callback.hh" #include #include diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index da3981696..ad4dccef9 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -5,7 +5,7 @@ #include "store-api.hh" #include "thread-pool.hh" #include "topo-sort.hh" - +#include "callback.hh" namespace nix { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index e92b94975..27535f1d0 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -10,6 +10,7 @@ #include "pool.hh" #include "finally.hh" #include "logging.hh" +#include "callback.hh" #include #include diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 2d5077ed0..1bbc74db8 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -8,9 +8,7 @@ #include "json.hh" #include "url.hh" #include "archive.hh" - -#include - +#include "callback.hh" namespace nix { diff --git a/src/libutil/callback.hh b/src/libutil/callback.hh new file mode 100644 index 000000000..ef31794be --- /dev/null +++ b/src/libutil/callback.hh @@ -0,0 +1,46 @@ +#pragma once + +#include +#include + +namespace nix { + +/* A callback is a wrapper around a lambda that accepts a valid of + type T or an exception. (We abuse std::future to pass the value or + exception.) */ +template +class Callback +{ + std::function)> fun; + std::atomic_flag done = ATOMIC_FLAG_INIT; + +public: + + Callback(std::function)> fun) : fun(fun) { } + + Callback(Callback && callback) : fun(std::move(callback.fun)) + { + auto prev = callback.done.test_and_set(); + if (prev) done.test_and_set(); + } + + void operator()(T && t) noexcept + { + auto prev = done.test_and_set(); + assert(!prev); + std::promise promise; + promise.set_value(std::move(t)); + fun(promise.get_future()); + } + + void rethrow(const std::exception_ptr & exc = std::current_exception()) noexcept + { + auto prev = done.test_and_set(); + assert(!prev); + std::promise promise; + promise.set_exception(exc); + fun(promise.get_future()); + } +}; + +} diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 082e26375..91e0a1543 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -17,7 +17,6 @@ #include #include #include -#include #include #ifndef HAVE_STRUCT_DIRENT_D_TYPE @@ -480,43 +479,8 @@ std::optional get(const T & map, const typename T::key_ } -/* A callback is a wrapper around a lambda that accepts a valid of - type T or an exception. (We abuse std::future to pass the value or - exception.) */ template -class Callback -{ - std::function)> fun; - std::atomic_flag done = ATOMIC_FLAG_INIT; - -public: - - Callback(std::function)> fun) : fun(fun) { } - - Callback(Callback && callback) : fun(std::move(callback.fun)) - { - auto prev = callback.done.test_and_set(); - if (prev) done.test_and_set(); - } - - void operator()(T && t) noexcept - { - auto prev = done.test_and_set(); - assert(!prev); - std::promise promise; - promise.set_value(std::move(t)); - fun(promise.get_future()); - } - - void rethrow(const std::exception_ptr & exc = std::current_exception()) noexcept - { - auto prev = done.test_and_set(); - assert(!prev); - std::promise promise; - promise.set_exception(exc); - fun(promise.get_future()); - } -}; +class Callback; /* Start a thread that handles various signals. Also block those signals From 340ca382c4de5863f0ecb5c67bae5461ea20c60b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 18:47:18 +0200 Subject: [PATCH 35/60] Don't include nlohmann/json.hpp in globals.hh This reduces compilation time by 207s. Issue #4045. --- src/libstore/globals.cc | 1 + src/libstore/globals.hh | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 491c664db..7b841d925 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "archive.hh" #include "args.hh" +#include "abstractsettingtojson.hh" #include #include diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 02721285a..8a2d3ff75 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -2,7 +2,6 @@ #include "types.hh" #include "config.hh" -#include "abstractsettingtojson.hh" #include "util.hh" #include From 0716adaa8b23855bd5e1a59f4115ec933b6a9205 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 18:49:43 +0200 Subject: [PATCH 36/60] abstractsettingtojson.hh -> abstract-setting-to-json.hh --- src/libstore/globals.cc | 2 +- .../{abstractsettingtojson.hh => abstract-setting-to-json.hh} | 0 src/libutil/config.cc | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename src/libutil/{abstractsettingtojson.hh => abstract-setting-to-json.hh} (100%) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 7b841d925..0beb9b2b7 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -2,7 +2,7 @@ #include "util.hh" #include "archive.hh" #include "args.hh" -#include "abstractsettingtojson.hh" +#include "abstract-setting-to-json.hh" #include #include diff --git a/src/libutil/abstractsettingtojson.hh b/src/libutil/abstract-setting-to-json.hh similarity index 100% rename from src/libutil/abstractsettingtojson.hh rename to src/libutil/abstract-setting-to-json.hh diff --git a/src/libutil/config.cc b/src/libutil/config.cc index 309d23b40..5e6a211df 100644 --- a/src/libutil/config.cc +++ b/src/libutil/config.cc @@ -1,6 +1,6 @@ #include "config.hh" #include "args.hh" -#include "abstractsettingtojson.hh" +#include "abstract-setting-to-json.hh" #include From 557d2427ee7c21b32e691e1c44f682b667673a70 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 18:57:02 +0200 Subject: [PATCH 37/60] Random header cleanup --- src/libutil/util.hh | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 91e0a1543..b8e201203 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -12,12 +12,9 @@ #include #include -#include -#include #include #include #include -#include #ifndef HAVE_STRUCT_DIRENT_D_TYPE #define DT_UNKNOWN 0 From ecc8672aa007af045d77434b495ca09541e9fee3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Sep 2020 19:06:23 +0200 Subject: [PATCH 38/60] fmt.hh: Don't include boost/algorithm/string/replace.hpp This cuts compilation time by ~49s. Issue #4045. --- src/libutil/fmt.hh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index a39de041f..6e69bdce2 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include "ansicolor.hh" From ba37299a0364c4b58d59e20cf7821ed9fade6dd6 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Sat, 19 Sep 2020 22:26:07 -0700 Subject: [PATCH 39/60] Serialize SandboxMode enum to string for JSON Rather than showing an integer as the default, instead show the boolean referenced in the description. The nix.conf.5 manpage used to show "default: 0", which is unnecessarily opaque and confusing (doesn't 0 mean false, even though the default is true?); now it properly shows that the default is true. --- src/libstore/globals.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 0beb9b2b7..f94d97cc8 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -147,6 +147,12 @@ bool Settings::isWSL1() const string nixVersion = PACKAGE_VERSION; +NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, { + {SandboxMode::smEnabled, true}, + {SandboxMode::smRelaxed, "relaxed"}, + {SandboxMode::smDisabled, false}, +}); + template<> void BaseSetting::set(const std::string & str) { if (str == "true") value = smEnabled; From d860295e116f9aa0f37e2637b5ec4b9c86fdf4d3 Mon Sep 17 00:00:00 2001 From: Michael Reilly Date: Sat, 19 Sep 2020 10:46:02 -0400 Subject: [PATCH 40/60] Bump nlohmann-json version to 3.9.1 --- src/libexpr/json-to-value.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libexpr/json-to-value.cc b/src/libexpr/json-to-value.cc index 76e1a26bf..9ca5ac86d 100644 --- a/src/libexpr/json-to-value.cc +++ b/src/libexpr/json-to-value.cc @@ -115,6 +115,14 @@ public: { return handle_value(mkString, val.c_str()); } +#if NLOHMANN_JSON_VERSION_MAJOR >= 3 && NLOHMANN_JSON_VERSION_MINOR >= 8 + bool binary(binary_t&) + { + // This function ought to be unreachable + assert(false); + return true; + } +#endif bool start_object(std::size_t len) { From 97b5154750d911a05992b97d7404d813d924de73 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 22 Sep 2020 10:04:25 +0200 Subject: [PATCH 41/60] Disable `FORTIFY_SOURCE` when compiling without optims Otherwise the build is cluttered with ``` /nix/store/fwpn2f7a4iqszyydw7ag61zlnp6xk5d3-glibc-2.30-dev/include/features.h:382:4: warning: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Wcpp] 382 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O) | ^~~~~~~ ``` when building with `OPTIMIZE=0` --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index f472ca7e5..c50d2c40f 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ OPTIMIZE = 1 ifeq ($(OPTIMIZE), 1) GLOBAL_CXXFLAGS += -O3 else - GLOBAL_CXXFLAGS += -O0 + GLOBAL_CXXFLAGS += -O0 -U_FORTIFY_SOURCE endif include mk/lib.mk From c1e79f870cf548e81d7cab0d296c6e923da9fa8c Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 22 Sep 2020 10:37:43 +0200 Subject: [PATCH 42/60] Silence a compiler warning in serialise.hh Explicitely cast to `uint64_t` in `readNum` to avoid a "comparison between signed and unsigned" warning --- src/libutil/serialise.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 6f4f4c855..7682a0f19 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -340,7 +340,7 @@ T readNum(Source & source) ((uint64_t) buf[6] << 48) | ((uint64_t) buf[7] << 56); - if (n > std::numeric_limits::max()) + if (n > (uint64_t)std::numeric_limits::max()) throw SerialisationError("serialised integer %d is too large for type '%s'", n, typeid(T).name()); return (T) n; From 35a0ac183858ecb03e313e088562c84fe211e20d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Sep 2020 11:40:19 +0200 Subject: [PATCH 43/60] Style fixes --- src/libstore/content-address.cc | 26 +++++++++++++++++--------- src/libstore/daemon.cc | 7 ++++++- src/libstore/remote-store.cc | 15 +++++++++++---- src/libstore/remote-store.hh | 7 ++++++- 4 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 74215c545..90a3ad1f5 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -4,11 +4,13 @@ namespace nix { -std::string FixedOutputHash::printMethodAlgo() const { +std::string FixedOutputHash::printMethodAlgo() const +{ return makeFileIngestionPrefix(method) + printHashType(hash.type); } -std::string makeFileIngestionPrefix(const FileIngestionMethod m) { +std::string makeFileIngestionPrefix(const FileIngestionMethod m) +{ switch (m) { case FileIngestionMethod::Flat: return ""; @@ -26,7 +28,8 @@ std::string makeFixedOutputCA(FileIngestionMethod method, const Hash & hash) + hash.to_string(Base32, true); } -std::string renderContentAddress(ContentAddress ca) { +std::string renderContentAddress(ContentAddress ca) +{ return std::visit(overloaded { [](TextHash th) { return "text:" + th.hash.to_string(Base32, true); @@ -37,7 +40,8 @@ std::string renderContentAddress(ContentAddress ca) { }, ca); } -std::string renderContentAddressMethod(ContentAddressMethod cam) { +std::string renderContentAddressMethod(ContentAddressMethod cam) +{ return std::visit(overloaded { [](TextHashMethod &th) { return std::string{"text:"} + printHashType(htSHA256); @@ -51,7 +55,8 @@ std::string renderContentAddressMethod(ContentAddressMethod cam) { /* Parses content address strings up to the hash. */ -static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest) { +static ContentAddressMethod parseContentAddressMethodPrefix(std::string_view & rest) +{ std::string_view wholeInput { rest }; std::string_view prefix; @@ -113,16 +118,19 @@ ContentAddress parseContentAddress(std::string_view rawCa) { }, caMethod); } -ContentAddressMethod parseContentAddressMethod(std::string_view caMethod) { +ContentAddressMethod parseContentAddressMethod(std::string_view caMethod) +{ std::string_view asPrefix {std::string{caMethod} + ":"}; return parseContentAddressMethodPrefix(asPrefix); } -std::optional parseContentAddressOpt(std::string_view rawCaOpt) { - return rawCaOpt == "" ? std::optional {} : parseContentAddress(rawCaOpt); +std::optional parseContentAddressOpt(std::string_view rawCaOpt) +{ + return rawCaOpt == "" ? std::optional() : parseContentAddress(rawCaOpt); }; -std::string renderContentAddress(std::optional ca) { +std::string renderContentAddress(std::optional ca) +{ return ca ? renderContentAddress(*ca) : ""; } diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 8897a73f4..83f8968b0 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -239,7 +239,12 @@ struct ClientSettings } }; -static void writeValidPathInfo(ref store, unsigned int clientVersion, Sink & to, std::shared_ptr info) { +static void writeValidPathInfo( + ref store, + unsigned int clientVersion, + Sink & to, + std::shared_ptr info) +{ to << (info->deriver ? store->printStorePath(*info->deriver) : "") << info->narHash.to_string(Base16, false); writeStorePaths(*store, to, info->references); diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 02a8b72b9..6f1f9769b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -422,7 +422,8 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S } -ref RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) { +ref RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) +{ auto deriver = readString(conn->from); auto narHash = Hash::parseAny(readString(conn->from), htSHA256); auto info = make_ref(path, narHash); @@ -533,7 +534,12 @@ std::optional RemoteStore::queryPathFromHashPart(const std::string & } -ref RemoteStore::addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair) +ref RemoteStore::addCAToStore( + Source & dump, + const string & name, + ContentAddressMethod caMethod, + const StorePathSet & references, + RepairFlag repair) { auto conn(getConnection()); @@ -603,6 +609,7 @@ ref RemoteStore::addCAToStore(Source & dump, const string & } } + StorePath RemoteStore::addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method, HashType hashType, RepairFlag repair) { @@ -964,8 +971,8 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * return nullptr; } -void -ConnectionHandle::withFramedSink(std::function fun) { +void ConnectionHandle::withFramedSink(std::function fun) +{ (*this)->to.flush(); std::exception_ptr ex; diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 735a3c24e..ec04be985 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -64,7 +64,12 @@ public: SubstitutablePathInfos & infos) override; /* Add a content-addressable store path. `dump` will be drained. */ - ref addCAToStore(Source & dump, const string & name, ContentAddressMethod caMethod, StorePathSet references, RepairFlag repair); + ref addCAToStore( + Source & dump, + const string & name, + ContentAddressMethod caMethod, + const StorePathSet & references, + RepairFlag repair); /* Add a content-addressable store path. Does not support references. `dump` will be drained. */ StorePath addToStoreFromDump(Source & dump, const string & name, From 980edd1f3a31eefe297d073f6a7cff099f21eb4a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Sep 2020 15:28:20 +0200 Subject: [PATCH 44/60] RemoteStore::addCAToStore(): Don't hold connection while calling queryPathInfo() This leads to a deadlock if we're at the connection limit. --- src/libstore/remote-store.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 6f1f9769b..be5eb4736 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -541,7 +541,8 @@ ref RemoteStore::addCAToStore( const StorePathSet & references, RepairFlag repair) { - auto conn(getConnection()); + std::optional conn_(getConnection()); + auto & conn = *conn_; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 25) { @@ -605,6 +606,8 @@ ref RemoteStore::addCAToStore( } }, caMethod); auto path = parseStorePath(readString(conn->from)); + // Release our connection to prevent a deadlock in queryPathInfo(). + conn_.reset(); return queryPathInfo(path); } } From 993229cdaf2e2347a204c876ecd660fc94048101 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 22 Aug 2020 20:44:47 +0000 Subject: [PATCH 45/60] Deduplicate basic derivation goals too See comments for security concerns. Also optimize goal creation by not traversing map twice. --- src/libstore/build.cc | 90 +++++++++++++++++++++++++++------------ src/libstore/daemon.cc | 14 ++++++ src/libstore/store-api.hh | 36 ++++++++++++++-- 3 files changed, 110 insertions(+), 30 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 07c5bceb2..8b206e819 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -296,9 +296,21 @@ public: ~Worker(); /* Make a goal (with caching). */ - GoalPtr makeDerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); - std::shared_ptr makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, BuildMode buildMode = bmNormal); + + /* derivation goal */ +private: + std::shared_ptr makeDerivationGoalCommon( + const StorePath & drvPath, const StringSet & wantedOutputs, + std::function()> mkDrvGoal); +public: + std::shared_ptr makeDerivationGoal( + const StorePath & drvPath, + const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + std::shared_ptr makeBasicDerivationGoal( + const StorePath & drvPath, const BasicDerivation & drv, + const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + + /* substitution goal */ GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); /* Remove a dead goal. */ @@ -949,10 +961,12 @@ private: friend struct RestrictedStore; public: - DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, - Worker & worker, BuildMode buildMode = bmNormal); + DerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - Worker & worker, BuildMode buildMode = bmNormal); + const StringSet & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); ~DerivationGoal(); /* Whether we need to perform hash rewriting if there are valid output paths. */ @@ -1085,8 +1099,8 @@ private: const Path DerivationGoal::homeDir = "/homeless-shelter"; -DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, - Worker & worker, BuildMode buildMode) +DerivationGoal::DerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker) , useDerivation(true) , drvPath(drvPath) @@ -1094,7 +1108,9 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & want , buildMode(buildMode) { state = &DerivationGoal::getDerivation; - name = fmt("building of '%s'", worker.store.printStorePath(this->drvPath)); + name = fmt( + "building of '%s' from .drv file", + StorePathWithOutputs { drvPath, wantedOutputs }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -1103,15 +1119,18 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & want DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - Worker & worker, BuildMode buildMode) + const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker) , useDerivation(false) , drvPath(drvPath) + , wantedOutputs(wantedOutputs) , buildMode(buildMode) { this->drv = std::make_unique(BasicDerivation(drv)); state = &DerivationGoal::haveDerivation; - name = fmt("building of %s", StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store)); + name = fmt( + "building of '%s' from in-memory derivation", + StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -5060,35 +5079,52 @@ Worker::~Worker() } -GoalPtr Worker::makeDerivationGoal(const StorePath & path, - const StringSet & wantedOutputs, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationGoalCommon( + const StorePath & drvPath, + const StringSet & wantedOutputs, + std::function()> mkDrvGoal) { - GoalPtr goal = derivationGoals[path].lock(); // FIXME - if (!goal) { - goal = std::make_shared(path, wantedOutputs, *this, buildMode); - derivationGoals.insert_or_assign(path, goal); + WeakGoalPtr & abstract_goal_weak = derivationGoals[drvPath]; + GoalPtr abstract_goal = abstract_goal_weak.lock(); // FIXME + std::shared_ptr goal; + if (!abstract_goal) { + goal = mkDrvGoal(); + abstract_goal_weak = goal; wakeUp(goal); - } else - (dynamic_cast(goal.get()))->addWantedOutputs(wantedOutputs); + } else { + goal = std::dynamic_pointer_cast(abstract_goal); + assert(goal); + goal->addWantedOutputs(wantedOutputs); + } return goal; } -std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, BuildMode buildMode) { - auto goal = std::make_shared(drvPath, drv, *this, buildMode); - wakeUp(goal); - return goal; + return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() { + return std::make_shared(drvPath, wantedOutputs, *this, buildMode); + }); +} + + +std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath & drvPath, + const BasicDerivation & drv, const StringSet & wantedOutputs, BuildMode buildMode) +{ + return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() { + return std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode); + }); } GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional ca) { - GoalPtr goal = substitutionGoals[path].lock(); // FIXME + WeakGoalPtr & goal_weak = substitutionGoals[path]; + GoalPtr goal = goal_weak.lock(); // FIXME if (!goal) { goal = std::make_shared(path, *this, repair, ca); - substitutionGoals.insert_or_assign(path, goal); + goal_weak = goal; wakeUp(goal); } return goal; @@ -5519,7 +5555,7 @@ BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDe BuildMode buildMode) { Worker worker(*this); - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, buildMode); + auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); BuildResult result; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 83f8968b0..ec3391a6d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -546,6 +546,20 @@ static void performOp(TunnelLogger * logger, ref store, are in fact content-addressed if we don't trust them. */ assert(derivationIsCA(drv.type()) || trusted); + /* Recompute the derivation path when we cannot trust the original. */ + if (!trusted) { + /* Recomputing the derivation path for input-address derivations + makes it harder to audit them after the fact, since we need the + original not-necessarily-resolved derivation to verify the drv + derivation as adequate claim to the input-addressed output + paths. */ + assert(derivationIsCA(drv.type())); + + Derivation drv2; + static_cast(drv2) = drv; + drvPath = writeDerivation(*store, Derivation { drv2 }); + } + auto res = store->buildDerivation(drvPath, drv, buildMode); logger->stopWork(); to << res.status << res.errorMsg; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 591140874..3ccee4f75 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -479,8 +479,38 @@ public: BuildMode buildMode = bmNormal); /* Build a single non-materialized derivation (i.e. not from an - on-disk .drv file). Note that ‘drvPath’ is only used for - informational purposes. */ + on-disk .drv file). + + ‘drvPath’ is used to deduplicate worker goals so it is imperative that + is correct. That said, it doesn't literally need to be store path that + would be calculated from writing this derivation to the store: it is OK + if it instead is that of a Derivation which would resolve to this (by + taking the outputs of it's input derivations and adding them as input + sources) such that the build time referenceable-paths are the same. + + In the input-addressed case, we usually *do* use an "original" + unresolved derivations's path, as that is what will be used in the + `buildPaths` case. Also, the input-addressed output paths are verified + only by that contents of that specific unresolved derivation, so it is + nice to keep that information around so if the original derivation is + ever obtained later, it can be verified whether the trusted user in fact + used the proper output path. + + In the content-addressed case, we want to always use the + resolved drv path calculated from the provided derivation. This serves + two purposes: + + - It keeps the operation trustless, by ruling out a maliciously + invalid drv path corresponding to a non-resolution-equivalent + derivation. + + - For the floating case in particular, it ensures that the derivation + to output mapping respects the resolution equivalence relation, so + one cannot choose different resolution-equivalent derivations to + subvert dependency coherence (i.e. the property that one doesn't end + up with multiple different versions of dependencies without + explicitly choosing to allow it). + */ virtual BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal) = 0; @@ -517,7 +547,7 @@ public: - The collector isn't running, or it's just started but hasn't acquired the GC lock yet. In that case we get and release the lock right away, then exit. The collector scans the - permanent root and sees our's. + permanent root and sees ours. In either case the permanent root is seen by the collector. */ virtual void syncWithGC() { }; From a2f5c921d41c941315ce783f66469bfb20e2c1b3 Mon Sep 17 00:00:00 2001 From: ujjwal Date: Tue, 22 Sep 2020 23:37:06 +0530 Subject: [PATCH 46/60] fixed typo --- src/libexpr/flake/flake.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 01f464859..783b98ef0 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -367,7 +367,7 @@ LockedFlake lockFlake( /* If we have an --update-input flag for an input of this input, then we must fetch the flake to - to update it. */ + update it. */ auto lb = lockFlags.inputUpdates.lower_bound(inputPath); auto hasChildUpdate = From 8a2e10827f2f57c3b9a0829357363d5f09af65e2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 14:08:23 +0200 Subject: [PATCH 47/60] Remove unused Flake::vOutputs field --- src/libexpr/flake/flake.cc | 5 ++--- src/libexpr/flake/flake.hh | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 783b98ef0..460eea5ea 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -215,10 +215,9 @@ static Flake getFlake( if (auto outputs = vInfo.attrs->get(sOutputs)) { expectType(state, tLambda, *outputs->value, *outputs->pos); - flake.vOutputs = allocRootValue(outputs->value); - if ((*flake.vOutputs)->lambda.fun->matchAttrs) { - for (auto & formal : (*flake.vOutputs)->lambda.fun->formals->formals) { + if (outputs->value->lambda.fun->matchAttrs) { + for (auto & formal : outputs->value->lambda.fun->formals->formals) { if (formal.name != state.sSelf) flake.inputs.emplace(formal.name, FlakeInput { .ref = parseFlakeRef(formal.name) diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index c2bb2888b..69c779af8 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -34,7 +34,6 @@ struct Flake std::optional description; std::shared_ptr sourceInfo; FlakeInputs inputs; - RootValue vOutputs; ~Flake(); }; From 21639b2d179e84805d5ab0949c7bdd74c9e2b7ae Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 23 Sep 2020 16:05:47 +0200 Subject: [PATCH 48/60] Use gold as the linker on Linux Saves ~7s in the linking phase --- flake.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/flake.nix b/flake.nix index 0304557e8..200417c3e 100644 --- a/flake.nix +++ b/flake.nix @@ -58,6 +58,7 @@ configureFlags = lib.optionals stdenv.isLinux [ "--with-sandbox-shell=${sh}/bin/busybox" + "LDFLAGS=-fuse-ld=gold" ]; buildDeps = From e8f0b1e996e95e4a1025976d6cfb7ece734129a2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 15:34:11 +0200 Subject: [PATCH 49/60] DerivationGoal::registerOutputs(): Fix bad format string --- src/libstore/build.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 07c5bceb2..2e23dd979 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3858,7 +3858,7 @@ void DerivationGoal::registerOutputs() something like that. */ canonicalisePathMetaData(actualPath, buildUser ? buildUser->getUID() : -1, inodesSeen); - debug("scanning for references for output %1 in temp location '%1%'", outputName, actualPath); + debug("scanning for references for output '%s' in temp location '%s'", outputName, actualPath); /* Pass blank Sink as we are not ready to hash data at this stage. */ NullSink blank; From d4f8163d104aee0f39e6d8b98160f8de12c18824 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 15:47:12 +0200 Subject: [PATCH 50/60] canonicalisePathMetaData_(): Change assertion to error message --- src/libstore/local-store.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c91f3fbf7..1f58977a5 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -478,8 +478,7 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe ensure that we don't fail on hard links within the same build (i.e. "touch $out/foo; ln $out/foo $out/bar"). */ if (fromUid != (uid_t) -1 && st.st_uid != fromUid) { - assert(!S_ISDIR(st.st_mode)); - if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end()) + if (S_ISDIR(st.st_mode) || !inodesSeen.count(Inode(st.st_dev, st.st_ino))) throw BuildError("invalid ownership on file '%1%'", path); mode_t mode = st.st_mode & ~S_IFMT; assert(S_ISLNK(st.st_mode) || (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore)); From cec94738715275ec4761071cefe4a9ae1a565960 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 15:47:30 +0200 Subject: [PATCH 51/60] DerivationGoal::registerOutputs(): Don't canonicalize twice Fixes #4021. --- src/libstore/build.cc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 2e23dd979..2979ce010 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3913,7 +3913,6 @@ void DerivationGoal::registerOutputs() outputRewrites[std::string { scratchPath.hashPart() }] = std::string { finalStorePath.hashPart() }; }; - bool rewritten = false; std::optional referencesOpt = std::visit(overloaded { [&](AlreadyRegistered skippedFinalPath) -> std::optional { finish(skippedFinalPath.path); @@ -3943,8 +3942,6 @@ void DerivationGoal::registerOutputs() sink.s = make_ref(rewriteStrings(*sink.s, outputRewrites)); StringSource source(*sink.s); restorePath(actualPath, source); - - rewritten = true; } }; @@ -4125,11 +4122,6 @@ void DerivationGoal::registerOutputs() } } - /* Get rid of all weird permissions. This also checks that - all files are owned by the build user, if applicable. */ - canonicalisePathMetaData(actualPath, - buildUser && !rewritten ? buildUser->getUID() : -1, inodesSeen); - if (buildMode == bmCheck) { if (!worker.store.isValidPath(newInfo.path)) continue; ValidPathInfo oldInfo(*worker.store.queryPathInfo(newInfo.path)); From 2548347bba2d4e6f9947dadf95ed24a16f8a1cdc Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Wed, 23 Sep 2020 17:39:19 +0200 Subject: [PATCH 52/60] libutil/archive: add preallocate-contents option Make archive preallocation (fallocate) optional because some filesystems like btrfs do not behave as expected with fallocate. See #3550. --- src/libutil/archive.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 14399dea3..cdf3d89ae 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -27,6 +27,8 @@ struct ArchiveSettings : Config #endif "use-case-hack", "Whether to enable a Darwin-specific hack for dealing with file name collisions."}; + Setting preallocateContents{this, true, "preallocate-contents", + "Whether to preallocate files when writing objects with known size."}; }; static ArchiveSettings archiveSettings; @@ -325,6 +327,9 @@ struct RestoreSink : ParseSink void preallocateContents(uint64_t len) { + if (!archiveSettings.preallocateContents) + return; + #if HAVE_POSIX_FALLOCATE if (len) { errno = posix_fallocate(fd.get(), 0, len); From 31ab4c3816675b5bf2f7b29dfb10112cd8ec9ceb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 19:09:58 +0200 Subject: [PATCH 53/60] Test whether build/repair results are read-only --- tests/repair.sh | 13 +++++-------- tests/simple.sh | 4 +++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/repair.sh b/tests/repair.sh index ec7ad5dca..ba019028d 100644 --- a/tests/repair.sh +++ b/tests/repair.sh @@ -13,14 +13,14 @@ hash=$(nix-hash $path2) chmod u+w $path2 touch $path2/bad -if nix-store --verify --check-contents -v; then - echo "nix-store --verify succeeded unexpectedly" >&2 - exit 1 -fi +(! nix-store --verify --check-contents -v) # The path can be repaired by rebuilding the derivation. nix-store --verify --check-contents --repair +(! [ -e $path2/bad ]) +(! [ -w $path2 ]) + nix-store --verify-path $path2 # Re-corrupt and delete the deriver. Now --verify --repair should @@ -30,10 +30,7 @@ touch $path2/bad nix-store --delete $(nix-store -qd $path2) -if nix-store --verify --check-contents --repair; then - echo "nix-store --verify --repair succeeded unexpectedly" >&2 - exit 1 -fi +(! nix-store --verify --check-contents --repair) nix-build dependencies.nix -o $TEST_ROOT/result --repair diff --git a/tests/simple.sh b/tests/simple.sh index 37631b648..15bd2bd16 100644 --- a/tests/simple.sh +++ b/tests/simple.sh @@ -10,13 +10,15 @@ outPath=$(nix-store -rvv "$drvPath") echo "output path is $outPath" +(! [ -w $outPath ]) + text=$(cat "$outPath"/hello) if test "$text" != "Hello World!"; then exit 1; fi # Directed delete: $outPath is not reachable from a root, so it should # be deleteable. nix-store --delete $outPath -if test -e $outPath/hello; then false; fi +(! [ -e $outPath/hello ]) outPath="$(NIX_REMOTE=local?store=/foo\&real=$TEST_ROOT/real-store nix-instantiate --readonly-mode hash-check.nix)" if test "$outPath" != "/foo/lfy1s6ca46rm5r6w4gg9hc0axiakjcnm-dependencies.drv"; then From 688bd4fb500c70cda4ffe6864bbf59dbc4da49a2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 19:10:16 +0200 Subject: [PATCH 54/60] After rewriting a path, make it read-only --- src/libstore/build.cc | 47 ++++++++++++++----------------------------- 1 file changed, 15 insertions(+), 32 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 2979ce010..cf04fbe56 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -3735,15 +3735,12 @@ void DerivationGoal::runChild() } -static void moveCheckToStore(const Path & src, const Path & dst) +/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if + it's a directory and we're not root (to be able to update the + directory's parent link ".."). */ +static void movePath(const Path & src, const Path & dst) { - /* For the rename of directory to succeed, we must be running as root or - the directory must be made temporarily writable (to update the - directory's parent link ".."). */ - struct stat st; - if (lstat(src.c_str(), &st) == -1) { - throw SysError("getting attributes of path '%1%'", src); - } + auto st = lstat(src); bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); @@ -3942,6 +3939,10 @@ void DerivationGoal::registerOutputs() sink.s = make_ref(rewriteStrings(*sink.s, outputRewrites)); StringSource source(*sink.s); restorePath(actualPath, source); + + /* FIXME: set proper permissions in restorePath() so + we don't have to do another traversal. */ + canonicalisePathMetaData(actualPath, -1, inodesSeen); } }; @@ -4024,7 +4025,7 @@ void DerivationGoal::registerOutputs() [&](DerivationOutputInputAddressed output) { /* input-addressed case */ auto requiredFinalPath = output.path; - /* Preemtively add rewrite rule for final hash, as that is + /* Preemptively add rewrite rule for final hash, as that is what the NAR hash will use rather than normalized-self references */ if (scratchPath != requiredFinalPath) outputRewrites.insert_or_assign( @@ -4098,27 +4099,9 @@ void DerivationGoal::registerOutputs() else. No moving needed. */ assert(newInfo.ca); } else { - /* Temporarily add write perm so we can move, will be fixed - later. */ - { - struct stat st; - auto & mode = st.st_mode; - if (lstat(actualPath.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", actualPath); - mode |= 0200; - /* Try to change the perms, but only if the file isn't a - symlink as symlinks permissions are mostly ignored and - calling `chmod` on it will just forward the call to the - target of the link. */ - if (!S_ISLNK(st.st_mode)) - if (chmod(actualPath.c_str(), mode) == -1) - throw SysError("changing mode of '%1%' to %2$o", actualPath, mode); - } - if (rename( - actualPath.c_str(), - worker.store.toRealPath(finalDestPath).c_str()) == -1) - throw SysError("moving build output '%1%' from it's temporary location to the Nix store", finalDestPath); - actualPath = worker.store.toRealPath(finalDestPath); + auto destPath = worker.store.toRealPath(finalDestPath); + movePath(actualPath, destPath); + actualPath = destPath; } } @@ -4128,9 +4111,9 @@ void DerivationGoal::registerOutputs() if (newInfo.narHash != oldInfo.narHash) { worker.checkMismatch = true; if (settings.runDiffHook || settings.keepFailed) { - Path dst = worker.store.toRealPath(finalDestPath + checkSuffix); + auto dst = worker.store.toRealPath(finalDestPath + checkSuffix); deletePath(dst); - moveCheckToStore(actualPath, dst); + movePath(actualPath, dst); handleDiffHook( buildUser ? buildUser->getUID() : getuid(), From 236d9ee7f72ca4238f5f44c244fd2b885691c6ad Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 19:17:28 +0200 Subject: [PATCH 55/60] lstat() cleanup --- src/libexpr/parser.y | 3 +-- src/libstore/build.cc | 9 ++------- src/libstore/gc.cc | 4 +--- src/libstore/local-store.cc | 18 +++++------------- src/libstore/optimise-store.cc | 12 +++--------- src/libstore/profiles.cc | 5 +---- src/libutil/archive.cc | 4 +--- .../resolve-system-dependencies.cc | 6 +----- 8 files changed, 15 insertions(+), 46 deletions(-) diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 24b21f7da..a4c84c526 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -614,8 +614,7 @@ Path resolveExprPath(Path path) // Basic cycle/depth limit to avoid infinite loops. if (++followCount >= maxFollow) throw Error("too many symbolic links encountered while traversing the path '%s'", path); - if (lstat(path.c_str(), &st)) - throw SysError("getting status of '%s'", path); + st = lstat(path); if (!S_ISLNK(st.st_mode)) break; path = absPath(readLink(path), dirOf(path)); } diff --git a/src/libstore/build.cc b/src/libstore/build.cc index cf04fbe56..6b53f529a 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -2367,10 +2367,7 @@ void DerivationGoal::startBuilder() for (auto & i : inputPaths) { auto p = worker.store.printStorePath(i); Path r = worker.store.toRealPath(p); - struct stat st; - if (lstat(r.c_str(), &st)) - throw SysError("getting attributes of path '%s'", p); - if (S_ISDIR(st.st_mode)) + if (S_ISDIR(lstat(r).st_mode)) dirsInChroot.insert_or_assign(p, r); else linkOrCopy(r, chrootRootDir + p); @@ -3144,9 +3141,7 @@ void DerivationGoal::addDependency(const StorePath & path) if (pathExists(target)) throw Error("store path '%s' already exists in the sandbox", worker.store.printStorePath(path)); - struct stat st; - if (lstat(source.c_str(), &st)) - throw SysError("getting attributes of path '%s'", source); + auto st = lstat(source); if (S_ISDIR(st.st_mode)) { diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 08b53c702..518a357ef 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -663,9 +663,7 @@ void LocalStore::removeUnusedLinks(const GCState & state) if (name == "." || name == "..") continue; Path path = linksDir + "/" + name; - struct stat st; - if (lstat(path.c_str(), &st) == -1) - throw SysError("statting '%1%'", path); + auto st = lstat(path); if (st.st_nlink != 1) { actualSize += st.st_size; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 1f58977a5..ee997ef3a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -114,8 +114,7 @@ LocalStore::LocalStore(const Params & params) Path path = realStoreDir; struct stat st; while (path != "/") { - if (lstat(path.c_str(), &st)) - throw SysError("getting status of '%1%'", path); + st = lstat(path); if (S_ISLNK(st.st_mode)) throw Error( "the path '%1%' is a symlink; " @@ -419,10 +418,7 @@ static void canonicaliseTimestampAndPermissions(const Path & path, const struct void canonicaliseTimestampAndPermissions(const Path & path) { - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); - canonicaliseTimestampAndPermissions(path, st); + canonicaliseTimestampAndPermissions(path, lstat(path)); } @@ -440,9 +436,7 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe } #endif - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); + auto st = lstat(path); /* Really make sure that the path is of a supported type. */ if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode))) @@ -521,9 +515,7 @@ void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & ino /* On platforms that don't have lchown(), the top-level path can't be a symlink, since we can't change its ownership. */ - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); + auto st = lstat(path); if (st.st_uid != geteuid()) { assert(S_ISLNK(st.st_mode)); @@ -1454,7 +1446,7 @@ static void makeMutable(const Path & path) { checkInterrupt(); - struct stat st = lstat(path); + auto st = lstat(path); if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode)) return; diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index e4b4b6213..c032a5e22 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -17,9 +17,7 @@ namespace nix { static void makeWritable(const Path & path) { - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); + auto st = lstat(path); if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1) throw SysError("changing writability of '%1%'", path); } @@ -94,9 +92,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, { checkInterrupt(); - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); + auto st = lstat(path); #if __APPLE__ /* HFS/macOS has some undocumented security feature disabling hardlinking for @@ -187,9 +183,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, /* Yes! We've seen a file with the same contents. Replace the current file with a hard link to that file. */ - struct stat stLink; - if (lstat(linkPath.c_str(), &stLink)) - throw SysError("getting attributes of path '%1%'", linkPath); + auto stLink = lstat(linkPath); if (st.st_ino == stLink.st_ino) { debug(format("'%1%' is already linked to '%2%'") % path % linkPath); diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index c20386e2b..c3809bad7 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -39,13 +39,10 @@ std::pair> findGenerations(Path pro for (auto & i : readDirectory(profileDir)) { if (auto n = parseName(profileName, i.name)) { auto path = profileDir + "/" + i.name; - struct stat st; - if (lstat(path.c_str(), &st) != 0) - throw SysError("statting '%1%'", path); gens.push_back({ .number = *n, .path = path, - .creationTime = st.st_mtime + .creationTime = lstat(path).st_mtime }); } } diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index 14399dea3..4f59c8ed5 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -66,9 +66,7 @@ static void dump(const Path & path, Sink & sink, PathFilter & filter) { checkInterrupt(); - struct stat st; - if (lstat(path.c_str(), &st)) - throw SysError("getting attributes of path '%1%'", path); + auto st = lstat(path); sink << "("; diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc index 434ad80a6..d30227e4e 100644 --- a/src/resolve-system-dependencies/resolve-system-dependencies.cc +++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc @@ -111,11 +111,7 @@ std::set runResolver(const Path & filename) bool isSymlink(const Path & path) { - struct stat st; - if (lstat(path.c_str(), &st) == -1) - throw SysError("getting attributes of path '%1%'", path); - - return S_ISLNK(st.st_mode); + return S_ISLNK(lstat(path).st_mode); } Path resolveSymlink(const Path & path) From 9a24ece122eb19f3b69f072f6ce3c39c5ae4d0ce Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 20:21:08 +0200 Subject: [PATCH 56/60] Fix exception --- src/libstore/build.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 6b53f529a..f0820e711 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1675,7 +1675,7 @@ void DerivationGoal::tryLocalBuild() { } -void replaceValidPath(const Path & storePath, const Path tmpPath) +void replaceValidPath(const Path & storePath, const Path & tmpPath) { /* We can't atomically replace storePath (the original) with tmpPath (the replacement), so we have to move it out of the @@ -1685,8 +1685,9 @@ void replaceValidPath(const Path & storePath, const Path tmpPath) if (pathExists(storePath)) rename(storePath.c_str(), oldPath.c_str()); if (rename(tmpPath.c_str(), storePath.c_str()) == -1) { + auto ex = SysError("moving '%s' to '%s'", tmpPath, storePath); rename(oldPath.c_str(), storePath.c_str()); // attempt to recover - throw SysError("moving '%s' to '%s'", tmpPath, storePath); + throw ex; } deletePath(oldPath); } From 4ce8a3ed452f06b18a40cffefc37d47c916927a8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 23 Sep 2020 21:29:10 +0200 Subject: [PATCH 57/60] Hopefully fix EPERM on macOS --- src/libstore/build.cc | 72 ++++++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 32 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index f0820e711..db7dbc17e 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -1675,6 +1675,33 @@ void DerivationGoal::tryLocalBuild() { } +static void chmod_(const Path & path, mode_t mode) +{ + if (chmod(path.c_str(), mode) == -1) + throw SysError("setting permissions on '%s'", path); +} + + +/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if + it's a directory and we're not root (to be able to update the + directory's parent link ".."). */ +static void movePath(const Path & src, const Path & dst) +{ + auto st = lstat(src); + + bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); + + if (changePerm) + chmod_(src, st.st_mode | S_IWUSR); + + if (rename(src.c_str(), dst.c_str())) + throw SysError("renaming '%1%' to '%2%'", src, dst); + + if (changePerm) + chmod_(dst, st.st_mode); +} + + void replaceValidPath(const Path & storePath, const Path & tmpPath) { /* We can't atomically replace storePath (the original) with @@ -1683,12 +1710,20 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath) we're repairing (say) Glibc, we end up with a broken system. */ Path oldPath = (format("%1%.old-%2%-%3%") % storePath % getpid() % random()).str(); if (pathExists(storePath)) - rename(storePath.c_str(), oldPath.c_str()); - if (rename(tmpPath.c_str(), storePath.c_str()) == -1) { - auto ex = SysError("moving '%s' to '%s'", tmpPath, storePath); - rename(oldPath.c_str(), storePath.c_str()); // attempt to recover - throw ex; + movePath(storePath, oldPath); + + try { + movePath(tmpPath, storePath); + } catch (...) { + try { + // attempt to recover + movePath(oldPath, storePath); + } catch (...) { + ignoreException(); + } + throw; } + deletePath(oldPath); } @@ -2006,13 +2041,6 @@ HookReply DerivationGoal::tryBuildHook() } -static void chmod_(const Path & path, mode_t mode) -{ - if (chmod(path.c_str(), mode) == -1) - throw SysError("setting permissions on '%s'", path); -} - - int childEntry(void * arg) { ((DerivationGoal *) arg)->runChild(); @@ -3731,26 +3759,6 @@ void DerivationGoal::runChild() } -/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if - it's a directory and we're not root (to be able to update the - directory's parent link ".."). */ -static void movePath(const Path & src, const Path & dst) -{ - auto st = lstat(src); - - bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR)); - - if (changePerm) - chmod_(src, st.st_mode | S_IWUSR); - - if (rename(src.c_str(), dst.c_str())) - throw SysError("renaming '%1%' to '%2%'", src, dst); - - if (changePerm) - chmod_(dst, st.st_mode); -} - - void DerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output From bd5f3dbe118d569ffb201ce14394572ac5fc412c Mon Sep 17 00:00:00 2001 From: Kevin Quick Date: Thu, 24 Sep 2020 12:30:03 -0700 Subject: [PATCH 58/60] Fixes fall-through to report correct description of hash-file command. --- src/nix/hash.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 0eca4f8ea..494f00a20 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -44,6 +44,7 @@ struct CmdHash : Command switch (mode) { case FileIngestionMethod::Flat: d = "print cryptographic hash of a regular file"; + break; case FileIngestionMethod::Recursive: d = "print cryptographic hash of the NAR serialisation of a path"; }; From 4d863a9fcb9460a9e4978466e03d2982d32e39f0 Mon Sep 17 00:00:00 2001 From: Paul Opiyo <59094545+paulopiyo777@users.noreply.github.com> Date: Thu, 24 Sep 2020 18:05:47 -0500 Subject: [PATCH 59/60] Remove redundant value checks std::optional had redundant checks for whether it had a value. An object is emplaced either way so it can be dereferenced without repeating a value check --- src/libexpr/flake/flake.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 460eea5ea..760ed1a6e 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -48,17 +48,17 @@ static std::tuple fetchOrSubstituteTree( resolvedRef = originalRef.resolve(state.store); auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef); if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store)); - flakeCache.push_back({resolvedRef, fetchedResolved.value()}); - fetched.emplace(fetchedResolved.value()); + flakeCache.push_back({resolvedRef, *fetchedResolved}); + fetched.emplace(*fetchedResolved); } else { throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef); } } - flakeCache.push_back({originalRef, fetched.value()}); + flakeCache.push_back({originalRef, *fetched}); } - auto [tree, lockedRef] = fetched.value(); + auto [tree, lockedRef] = *fetched; debug("got tree '%s' from '%s'", state.store->printStorePath(tree.storePath), lockedRef); From 7b2ae472ff05a39cd635ac10dbbce3cd17b60c93 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Sep 2020 10:27:40 +0200 Subject: [PATCH 60/60] expectArg(): Respect the 'optional' flag --- src/libutil/args.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 3c1f87f7e..f41242e17 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -192,7 +192,7 @@ public: { expectArgs({ .label = label, - .optional = true, + .optional = optional, .handler = {dest} }); }