From a7d2a3d087f21c004716808c94c63c387b2e689b Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Wed, 16 Nov 2022 15:23:59 +0100 Subject: Allow system certs access to fixed-output derivations --- src/libstore/build/local-derivation-goal.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 5cea3b590..a4ebd244f 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1715,6 +1715,8 @@ void LocalDerivationGoal::runChild() for (auto & path : { "/etc/resolv.conf", "/etc/services", "/etc/hosts" }) if (pathExists(path)) ss.push_back(path); + + dirsInChroot.emplace(settings.caFile, "/etc/ssl/certs/ca-certificates.crt"); } for (auto & i : ss) dirsInChroot.emplace(i, i); -- cgit v1.2.3 From 0e18254aa81b9315c13d6ae736cb38666d19f122 Mon Sep 17 00:00:00 2001 From: Moritz Angermann Date: Wed, 5 Apr 2023 17:05:17 +0800 Subject: Fix shutdown behavior and resource management for recursive-nix on macOS Previously, we relied on the `shutdown()` function to terminate `accept()` calls on a listening socket. However, this approach did not work on macOS as the waiting `accept()` call is not considered a connected socket, resulting in an `ENOTCONN` error. Instead, we now close the listening socket to terminate the `accept()` call. Additionally, we fixed a resource management issue where we set the `daemonSocket` variable to -1, triggering resource cleanup and causing the `stopDaemon` function to be called twice. This resulted in errors as the socket was already closed by the time the second `stopDaemon` call was made. Instead of setting `daemonSocket` to -1, we now release the socket using the `release()` method on a unique pointer. This properly transfers ownership and allows for correct resource cleanup. These changes ensure proper behavior and resource management for the recursive-nix feature on macOS. --- src/libstore/build/local-derivation-goal.cc | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 21cd6e7ee..4b978f2a4 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1457,7 +1457,7 @@ void LocalDerivationGoal::startDaemon() (struct sockaddr *) &remoteAddr, &remoteAddrLen); if (!remote) { if (errno == EINTR || errno == EAGAIN) continue; - if (errno == EINVAL) break; + if (errno == EINVAL || errno == ECONNABORTED) break; throw SysError("accepting connection"); } @@ -1487,8 +1487,22 @@ void LocalDerivationGoal::startDaemon() void LocalDerivationGoal::stopDaemon() { - if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1) - throw SysError("shutting down daemon socket"); + if (daemonSocket && shutdown(daemonSocket.get(), SHUT_RDWR) == -1) { + // According to the POSIX standard, the 'shutdown' function should + // return an ENOTCONN error when attempting to shut down a socket that + // hasn't been connected yet. This situation occurs when the 'accept' + // function is called on a socket without any accepted connections, + // leaving the socket unconnected. While Linux doesn't seem to produce + // an error for sockets that have only been accepted, more + // POSIX-compliant operating systems like OpenBSD, macOS, and others do + // return the ENOTCONN error. Therefore, we handle this error here to + // avoid raising an exception for compliant behaviour. + if (errno == ENOTCONN) { + daemonSocket.close(); + } else { + throw SysError("shutting down daemon socket"); + } + } if (daemonThread.joinable()) daemonThread.join(); @@ -1499,7 +1513,8 @@ void LocalDerivationGoal::stopDaemon() thread.join(); daemonWorkerThreads.clear(); - daemonSocket = -1; + // release the socket. + daemonSocket.close(); } -- cgit v1.2.3 From 36b7e30c1184035e72a7f24dc4656099ac6e208e Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Fri, 19 May 2023 22:47:40 +0200 Subject: Make mounting ssl cert file optional --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 36c89bee9..b50ff1f86 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1777,7 +1777,7 @@ void LocalDerivationGoal::runChild() if (pathExists(path)) ss.push_back(path); - dirsInChroot.emplace(settings.caFile, "/etc/ssl/certs/ca-certificates.crt"); + dirsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true); } for (auto & i : ss) dirsInChroot.emplace(i, i); -- cgit v1.2.3 From b14fea6fffaafa30faf67d89b6b6a01c14b3ddf3 Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Fri, 19 May 2023 23:30:35 +0200 Subject: Shortcircuit for empty caFile --- src/libstore/build/local-derivation-goal.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b50ff1f86..05d6685da 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1777,7 +1777,8 @@ void LocalDerivationGoal::runChild() if (pathExists(path)) ss.push_back(path); - dirsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true); + if (settings.caFile != "") + dirsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true); } for (auto & i : ss) dirsInChroot.emplace(i, i); -- cgit v1.2.3 From 3ebe1341abe1b0ad59bd4925517af18d9200f818 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Fri, 17 Mar 2023 15:51:08 +0100 Subject: Make `RewritingSink` accept a map of rewrites Giving it the same semantics as `rewriteStrings`. Also add some tests for it --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 05d6685da..6f7ab8a3d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -4,7 +4,7 @@ #include "worker.hh" #include "builtins.hh" #include "builtins/buildenv.hh" -#include "references.hh" +#include "path-references.hh" #include "finally.hh" #include "util.hh" #include "archive.hh" -- cgit v1.2.3 From a917fb0d53544f7fd29f24d5984702296d457347 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Fri, 17 Mar 2023 15:51:08 +0100 Subject: Use a RewritingSink in derivation goal Possibly this will make it stream --- src/libstore/build/local-derivation-goal.cc | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 6f7ab8a3d..4650b8bae 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2384,13 +2384,16 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() if (!outputRewrites.empty()) { debug("rewriting hashes in '%1%'; cross fingers", actualPath); - /* FIXME: this is in-memory. */ - StringSink sink; - dumpPath(actualPath, sink); + /* FIXME: Is this actually streaming? */ + auto source = sinkToSource([&](Sink & nextSink) { + RewritingSink rsink(outputRewrites, nextSink); + dumpPath(actualPath, rsink); + rsink.flush(); + }); + Path tmpPath = actualPath + ".tmp"; + restorePath(tmpPath, *source); deletePath(actualPath); - sink.s = rewriteStrings(sink.s, outputRewrites); - StringSource source(sink.s); - restorePath(actualPath, source); + movePath(tmpPath, actualPath); /* FIXME: set proper permissions in restorePath() so we don't have to do another traversal. */ -- cgit v1.2.3 From 34e1b464f0054623334355aac94cb1a3e4c35a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Fri, 17 Mar 2023 15:51:08 +0100 Subject: Normalize the hash-rewriting process when building derivations --- src/libstore/build/local-derivation-goal.cc | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4650b8bae..e9fe1eb73 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2379,14 +2379,14 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() continue; auto references = *referencesOpt; - auto rewriteOutput = [&]() { + auto rewriteOutput = [&](const StringMap & rewrites) { /* Apply hash rewriting if necessary. */ - if (!outputRewrites.empty()) { + if (!rewrites.empty()) { debug("rewriting hashes in '%1%'; cross fingers", actualPath); /* FIXME: Is this actually streaming? */ auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink(outputRewrites, nextSink); + RewritingSink rsink(rewrites, nextSink); dumpPath(actualPath, rsink); rsink.flush(); }); @@ -2442,7 +2442,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() "since recursive hashing is not enabled (one of outputHashMode={flat,text} is true)", actualPath); } - rewriteOutput(); + rewriteOutput(outputRewrites); /* FIXME optimize and deduplicate with addToStore */ std::string oldHashPart { scratchPath->hashPart() }; HashModuloSink caSink { outputHash.hashType, oldHashPart }; @@ -2480,16 +2480,14 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() Hash::dummy, }; if (*scratchPath != newInfo0.path) { - // Also rewrite the output path - auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(newInfo0.path.hashPart()), nextSink); - dumpPath(actualPath, rsink2); - rsink2.flush(); - }); - Path tmpPath = actualPath + ".tmp"; - restorePath(tmpPath, *source); - deletePath(actualPath); - movePath(tmpPath, actualPath); + // If the path has some self-references, we need to rewrite + // them. + // (note that this doesn't invalidate the ca hash we calculated + // above because it's computed *modulo the self-references*, so + // it already takes this rewrite into account). + rewriteOutput( + StringMap{{oldHashPart, + std::string(newInfo0.path.hashPart())}}); } HashResult narHashAndSize = hashPath(htSHA256, actualPath); @@ -2511,7 +2509,7 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs() outputRewrites.insert_or_assign( std::string { scratchPath->hashPart() }, std::string { requiredFinalPath.hashPart() }); - rewriteOutput(); + rewriteOutput(outputRewrites); auto narHashAndSize = hashPath(htSHA256, actualPath); ValidPathInfo newInfo0 { requiredFinalPath, narHashAndSize.first }; newInfo0.narSize = narHashAndSize.second; -- cgit v1.2.3 From d16a1994fb6048d4ea48090c5aabafb7ad89c84f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 25 May 2023 16:38:29 +0200 Subject: Properly report build errors on chrooted stores MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When encountering a build error, Nix moves the output paths out of the chroot into their final location (for “easier debugging of build failures”). However this was broken for chroot stores as it was moving it to the _logical_ location, not the _physical_ one. Fix it by moving to the physical (_real_) location. Fix https://github.com/NixOS/nix/issues/8395 --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 05d6685da..b0289ac75 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -357,7 +357,7 @@ bool LocalDerivationGoal::cleanupDecideWhetherDiskFull() for (auto & [_, status] : initialOutputs) { if (!status.known) continue; if (buildMode != bmCheck && status.known->isValid()) continue; - auto p = worker.store.printStorePath(status.known->path); + auto p = worker.store.toRealPath(status.known->path); if (pathExists(chrootRootDir + p)) renameFile((chrootRootDir + p), p); } -- cgit v1.2.3 From d2ce2e89b178e7e7467cf4c1e572704a4c2ca75e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 19 May 2023 10:56:59 -0400 Subject: Split `OptionalPathSetting` from `PathSetting` Rather than doing `allowEmpty` as boolean, have separate types and use `std::optional`. This makes it harder to forget the possibility of an empty path. The `build-hook` setting was categorized as a `PathSetting`, but actually it was split into arguments. No good! Now, it is `Setting` which actually reflects what it means and how it is used. Because of the subtyping, we now also have support for `Setting>` in general. I imagine this can be used to clean up many more settings also. --- src/libstore/build/local-derivation-goal.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7f87cdf55..cf44779b7 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -65,8 +65,9 @@ void handleDiffHook( const Path & tryA, const Path & tryB, const Path & drvPath, const Path & tmpDir) { - auto diffHook = settings.diffHook; - if (diffHook != "" && settings.runDiffHook) { + auto & diffHookOpt = settings.diffHook.get(); + if (diffHookOpt && settings.runDiffHook) { + auto & diffHook = *diffHookOpt; try { auto diffRes = runProgram(RunOptions { .program = diffHook, @@ -1428,7 +1429,8 @@ void LocalDerivationGoal::startDaemon() Store::Params params; params["path-info-cache-size"] = "0"; params["store"] = worker.store.storeDir; - params["root"] = getLocalStore().rootDir; + if (auto & optRoot = getLocalStore().rootDir.get()) + params["root"] = *optRoot; params["state"] = "/no-such-path"; params["log"] = "/no-such-path"; auto store = make_ref(params, -- cgit v1.2.3 From 469d06f9bc9b2543f48d8e633e47f9344fba35ab Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 8 Mar 2022 21:53:26 +0000 Subject: Split out worker protocol template definitions from declarations This is generally a fine practice: Putting implementations in headers makes them harder to read and slows compilation. Unfortunately it is necessary for templates, but we can ameliorate that by putting them in a separate header. Only files which need to instantiate those templates will need to include the header with the implementation; the rest can just include the declaration. This is now documenting in the contributing guide. Also, it just happens that these polymorphic serializers are the protocol agnostic ones. (Worker and serve protocol have the same logic for these container types.) This means by doing this general template cleanup, we are also getting a head start on better indicating which code is protocol-specific and which code is shared between protocols. --- src/libstore/build/local-derivation-goal.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7f87cdf55..6e06f6061 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -11,6 +11,7 @@ #include "compression.hh" #include "daemon.hh" #include "worker-protocol.hh" +#include "worker-protocol-impl.hh" #include "topo-sort.hh" #include "callback.hh" #include "json-utils.hh" -- cgit v1.2.3 From 3859cf6b21dddd7e9de729dc42b2f52d8523c239 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 19 Jun 2023 12:18:04 -0400 Subject: Remove unused `#include` from `local-derivation-goal.cc` These were never needed for this file, and date back to before this was split from `derivation-goal.cc`. --- src/libstore/build/local-derivation-goal.cc | 2 -- 1 file changed, 2 deletions(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 6e06f6061..09c1fcf08 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -10,8 +10,6 @@ #include "archive.hh" #include "compression.hh" #include "daemon.hh" -#include "worker-protocol.hh" -#include "worker-protocol-impl.hh" #include "topo-sort.hh" #include "callback.hh" #include "json-utils.hh" -- cgit v1.2.3 From eebfe989a5dc3aac622b9b5f2edef4461d8968c1 Mon Sep 17 00:00:00 2001 From: Yueh-Shun Li Date: Fri, 30 Jun 2023 21:11:59 +0800 Subject: linkOrCopy: Fallback upon cross-device link error (EXDEV) Fix building derivations in local chroot store on OpenAFS, where hard linking accross directories causes cross-device link error (EXDEV). --- src/libstore/build/local-derivation-goal.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/libstore/build/local-derivation-goal.cc') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index ea7c52098..ee66ee500 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,8 +395,9 @@ static void linkOrCopy(const Path & from, const Path & to) bind-mount in this case? It can also fail with EPERM in BeegFS v7 and earlier versions + or fail with EXDEV in OpenAFS which don't allow hard-links to other directories */ - if (errno != EMLINK && errno != EPERM) + if (errno != EMLINK && errno != EPERM && errno != EXDEV) throw SysError("linking '%s' to '%s'", to, from); copyPath(from, to); } -- cgit v1.2.3