jade wip

Change-Id: I4d5a0bebeeefc35dac0bbc065c104c70c24aae17

.clang-tidy

@@ -8,16 +8,12 @@ Checks:
   - -bugprone-narrowing-conversions
     # kind of nonsense
   - -bugprone-easily-swappable-parameters
-    # too many warnings for now
-  - -bugprone-implicit-widening-of-multiplication-result
     # Lix's exception handling is Questionable
   - -bugprone-empty-catch
     # many warnings
   - -bugprone-unchecked-optional-access
     # many warnings, seems like a questionable lint
   - -bugprone-branch-clone
-    # extremely noisy before clang 19: https://github.com/llvm/llvm-project/issues/93959
-  - -bugprone-multi-level-implicit-pointer-conversion
     # we don't compile out our asserts
   - -bugprone-assert-side-effect
     # FIXME(jade): figure out if this warning is any good
@@ -29,9 +25,6 @@ Checks:
     # crimes must be appropriately declared as crimes
   - cppcoreguidelines-pro-type-cstyle-cast
   - lix-*
-    # This can not yet be applied to Lix itself since we need to do source
-    # reorganization so that lix/ include paths work.
-  - -lix-fixincludes
     # This lint is included as an example, but the lib function it replaces is
     # already gone.
   - -lix-hasprefixsuffix
@@ -40,3 +33,5 @@ Checks:
 CheckOptions:
   bugprone-reserved-identifier.AllowedIdentifiers: '__asan_default_options'
   bugprone-unused-return-value.AllowCastToVoid: true
+
+ExtraArgs: ["-Werror=unnecessary-virtual-specifier"]

doc/manual/rl-next/added-shell-dir-length.md

@@ -0,0 +1,13 @@
+---
+synopsis: "Nix shells' $NIX_BUILD_TOP are shorter"
+cls: [4663]
+issues: [fj#1044]
+category: "Fixes"
+credits: [raito]
+---
+
+Following the changes in 2.94.0 to shorten build directory paths, aimed at [resolving UNIX domain socket length issues](https://gerrit.lix.systems/c/lix/+/4168/13) and [improving nix-shell](https://git.lix.systems/lix-project/lix/issues/940), we inadvertently introduced an excessively long path for the `$NIX_BUILD_TOP` environment variable used by Nix shells (their effective temporary `/build` directory).
+
+To fix this, we replaced the `build-top-$HASH` directory name with simply `build-top`, reducing these paths by at least 30 characters.
+
+We also added a test to ensure that Nix shells do not introduce more than 50 extra characters relative to their base directory (e.g., `/tmp` when `$TMPDIR` is not set).

doc/manual/rl-next/shells-honors-nix-log-fd.md

@@ -0,0 +1,14 @@
+---
+synopsis: "Shells supports $NIX_LOG_FD now"
+cls: [4694, 4695]
+issues: [fj#336]
+category: "Improvements"
+credits: [raito]
+---
+
+Lix's "debugging" shells (`nix3-develop` and `nix-shell`) now supports
+`$NIX_LOG_FD` environment variable.
+
+This means that [hook logging in
+stdenv](https://github.com/NixOS/nixpkgs/pull/310387) appears while debugging
+derivations via `nix3-develop` or `nix-shell`.

lix/legacy/build-remote.cc

@@ -527,7 +527,7 @@ kj::Promise<void> AcceptedBuild::runImpl(RunContext context)
                 AIO().timeoutAfter(15 * kj::MINUTES, lockFileAsync(uploadLock.get(), ltWrite))
             );
             if (!result) {
-                printError("somebody is hogging the upload lock for '%s', continuing...");
+                printError("somebody is hogging the upload lock for '%s', continuing...", storeUri);
             }
         }
 

lix/legacy/nix-build.cc

@@ -192,7 +192,11 @@ static int main_nix_build(AsyncIoRoot & aio, std::string programName, Strings ar
         throw UsageError("'-p' and '-E' are mutually exclusive");
 
     AutoDelete tmpDir(createTempDir(myName));
-    AutoDelete buildTopTmpDir(createTempSubdir(tmpDir, "build-top"));
+    // NOTE: we assume there's no `build-top` directory created inside of `tmpDir` and we have
+    // ownership of this.
+    auto buildTopTmpDir = tmpDir + "/build-top";
+    createDirs(buildTopTmpDir);
+
     if (outLink.empty())
         outLink = (Path) tmpDir + "/result";
 
@@ -434,6 +438,10 @@ static int main_nix_build(AsyncIoRoot & aio, std::string programName, Strings ar
             + 1
         );
 
+        // We re-export similarly to what occurs inside of a derivation goal `NIX_LOG_FD` to stderr.
+        // So that stdenv hooks that logs information can be observed inside this debugging tool.
+        env["NIX_LOG_FD"] = "2";
+
         // Don't use defaultTempDir() here! We want to preserve the user's TMPDIR for the shell
         env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] =
             getEnvNonEmpty("TMPDIR").value_or(buildTopTmpDir);

lix/libcmd/command.hh

@@ -15,8 +15,6 @@ namespace nix {
 
 extern std::string programPath;
 
-extern char * * savedArgv;
-
 class EvalState;
 struct Pos;
 class Store;

lix/libcmd/legacy.hh

@@ -5,18 +5,32 @@
 #include <functional>
 #include <list>
 #include <map>
+#include <span>
 #include <string>
 
 namespace nix {
 
-typedef std::function<int(AsyncIoRoot &, std::string, std::list<std::string>)> MainFunction;
-
 struct LegacyCommandRegistry
 {
-    using LegacyCommandMap = std::map<std::string, MainFunction>;
+    typedef std::function<int(AsyncIoRoot &, std::string, std::list<std::string>)> MainFunction;
+    typedef std::function<
+        int(AsyncIoRoot &, std::string, std::list<std::string>, std::span<char *>)>
+        RawMainFunction;
+
+    using LegacyCommandMap = std::map<std::string, RawMainFunction>;
     static LegacyCommandMap * commands;
 
     static void add(const std::string & name, MainFunction fun)
+    {
+        addWithRaw(
+            name,
+            [fun](AsyncIoRoot & aio, std::string name, std::list<std::string> args, std::span<char *>) {
+                return fun(aio, name, args);
+            }
+        );
+    }
+
+    static void addWithRaw(const std::string & name, RawMainFunction fun)
     {
         if (!commands) commands = new LegacyCommandMap;
         (*commands)[name] = fun;

lix/libcmd/repl-interacter.cc

@@ -48,7 +48,7 @@ char ** copyCompletions(const StringSet& possible)
             if (vp) {
                 while (--ac >= 0)
                     free(vp[ac]);
-                free(vp);
+                free(static_cast<void *>(vp));
             }
             throw Error("allocation failure");
         }

lix/libcmd/repl-interacter.hh

@@ -46,7 +46,7 @@ public:
      *
      * This function logs but ignores errors from readline's write_history().
      */
-    virtual void writeHistory();
+    void writeHistory();
     virtual ~ReadlineLikeInteracter() override;
 };
 

lix/libexpr/attr-set.hh

@@ -121,7 +121,6 @@ public:
 
 private:
     Bindings * bindings;
-    EvalMemory & mem;
     SymbolTable & symbols;
     Size capacity;
 
@@ -129,9 +128,8 @@ public:
     // needed by std::back_inserter
     using value_type = Attr;
 
-    BindingsBuilder(EvalMemory & mem, SymbolTable & symbols, Bindings * bindings, Size capacity)
+    BindingsBuilder(SymbolTable & symbols, Bindings * bindings, Size capacity)
         : bindings(bindings)
-        , mem(mem)
         , symbols(symbols)
         , capacity(capacity)
     {

lix/libexpr/eval.cc

@@ -210,9 +210,9 @@ void initLibExpr()
        (resident) memory to be allocated.  This might be a problem on
        systems that don't overcommit. */
     if (!getEnv("GC_INITIAL_HEAP_SIZE")) {
-        int64_t size = 32 * 1024 * 1024;
+        int64_t size = 32l * 1024 * 1024;
 #if HAVE_SYSCONF && defined(_SC_PAGESIZE) && defined(_SC_PHYS_PAGES)
-        int64_t maxSize = 384 * 1024 * 1024;
+        int64_t maxSize = 384l * 1024 * 1024;
         int64_t pageSize = sysconf(_SC_PAGESIZE);
         int64_t pages = sysconf(_SC_PHYS_PAGES);
         if (pageSize != -1) {
@@ -291,14 +291,14 @@ EvalMemory::EvalMemory()
 {
     assert(libexprInitialised);
 #if HAVE_BOEHMGC
-    GC_add_roots(gcCache, gcCache + CACHES);
+    GC_add_roots(static_cast<void *>(gcCache), static_cast<void *>(gcCache + CACHES));
 #endif
 }
 
 EvalMemory::~EvalMemory()
 {
 #if HAVE_BOEHMGC
-    GC_remove_roots(gcCache, gcCache + CACHES);
+    GC_remove_roots(static_cast<void *>(gcCache), static_cast<void *>(gcCache + CACHES));
 #endif
 }
 

lix/libexpr/eval.hh

@@ -216,7 +216,7 @@ public:
 
     BindingsBuilder buildBindings(SymbolTable & symbols, size_t capacity)
     {
-        return BindingsBuilder(*this, symbols, allocBindings(capacity), capacity);
+        return BindingsBuilder(symbols, allocBindings(capacity), capacity);
     }
 
     const Statistics getStats() const { return stats; }

lix/libmain/shared.cc

@@ -33,8 +33,6 @@
 
 namespace nix {
 
-char * * savedArgv;
-
 static bool gcWarning = true;
 
 void printGCWarning()

lix/libmain/stack.cc

@@ -48,7 +48,7 @@ void detectStackOverflow()
        requires an alternative stack, otherwise the signal cannot be
        delivered when we're out of stack space. */
     stack_t stack;
-    stack.ss_size = 4096 * 4 + MINSIGSTKSZ;
+    stack.ss_size = 4096ul * 4 + MINSIGSTKSZ;
     static auto stackBuf = std::make_unique<std::vector<char>>(stack.ss_size);
     stack.ss_sp = stackBuf->data();
     if (!stack.ss_sp) throw Error("cannot allocate alternative stack");

lix/libstore/build/local-derivation-goal.cc

@@ -227,18 +227,20 @@ retry:
     }
 
     auto & localStore = getLocalStore();
-    bool sandboxFallbackAllowed = true;
-    if (localStore.config().storeDir != localStore.config().realStoreDir.get()) {
-        #if __linux__
+    const bool sandboxFallbackAllowed = [&]() {
+        if (localStore.config().storeDir != localStore.config().realStoreDir.get()) {
+#if __linux__
             if (!useChroot) {
                 printTaggedWarning("auto-enabling the sandbox due to using a diverted store");
             }
             useChroot = true;
-            sandboxFallbackAllowed = false;
-        #else
+            return false;
+#else
             throw Error("building using a diverted store is not supported on this platform");
-        #endif
-    }
+#endif
+        }
+        return true;
+    }();
 
     if (useBuildUsers()) {
         if (!buildUser)
@@ -1832,15 +1834,18 @@ try {
             );
         }
 
+        outputGraph[scratchOutputs.at(name)] = StorePathSet{};
         std::visit(
             overloaded{/* Since we'll use the already installed versions of these, we
                                    can treat them as leaves and ignore any references they
                                    have. */
-                       [&](const AlreadyRegistered &) {
-                           outputGraph[scratchOutputs.at(name)] = StorePathSet{};
-                       },
+                       [&](const AlreadyRegistered &) {},
                        [&](const PerhapsNeedToRegister & refs) {
-                           outputGraph[scratchOutputs.at(name)] = refs.refs;
+                           for (auto & ref : refs.refs) {
+                               if (inverseOutputMap.find(ref) != inverseOutputMap.end()) {
+                                   outputGraph[scratchOutputs.at(name)].insert(ref);
+                               }
+                           }
                        }
             },
             *orifu
@@ -1851,10 +1856,8 @@ try {
         topoSort(outputsToSort, {[&](const std::string & name) {
                      StringSet dependencies;
                      for (auto & path : outputGraph.at(scratchOutputs.at(name))) {
-                         auto outputName = inverseOutputMap.find(path);
-                         if (outputName != inverseOutputMap.end()) {
-                             dependencies.insert(outputName->second);
-                         }
+                         auto outputName = inverseOutputMap.at(path);
+                         dependencies.insert(outputName);
                      }
                      return dependencies;
                  }});

lix/libstore/derivations.cc

@@ -142,6 +142,13 @@ static void expect(StringViewStream & str, std::string_view s)
     str.remaining.remove_prefix(s.size());
 }
 
+static void expectWithErrorMsg(StringViewStream & str, std::string_view s, const char * errorMsg)
+{
+    if (!str.remaining.starts_with(s)) {
+        throw FormatError(errorMsg);
+    }
+    str.remaining.remove_prefix(s.size());
+}
 
 /* Read a C-style string from stream `str'. */
 static BackedStringView parseString(StringViewStream & str)
@@ -276,13 +283,16 @@ Derivation parseDerivation(
     drv.name = name;
 
     StringViewStream str{s};
-    expect(str, "D");
+
+    const char * genericErrorMsg = "derivation does not start with 'Derive' or 'DrvWithVersion'";
+
+    expectWithErrorMsg(str, "D", genericErrorMsg);
     switch (str.peek()) {
     case 'e':
-        expect(str, "erive(");
+        expectWithErrorMsg(str, "erive(", genericErrorMsg);
         break;
     case 'r': {
-        expect(str, "rvWithVersion(");
+        expectWithErrorMsg(str, "rvWithVersion(", genericErrorMsg);
         auto versionS = parseString(str);
         throw FormatError("Unknown derivation ATerm format version '%s'", *versionS);
     }
@@ -342,7 +352,7 @@ Derivation parseDerivation(
  */
 static void printString(std::string & res, std::string_view s)
 {
-    boost::container::small_vector<char, 64 * 1024> buffer;
+    boost::container::small_vector<char, 64ul * 1024> buffer;
     buffer.reserve(s.size() * 2 + 2);
     char * buf = buffer.data();
     char * p = buf;

lix/libstore/filetransfer.cc

@@ -333,7 +333,7 @@ struct TransferItem
 
             // when the buffer is full (as determined by a historical magic value) we
             // pause the transfer and wait for the receiver to unpause it when ready.
-            if (successfulStatuses.count(getHTTPStatus()) && state->data.size() > 1024 * 1024) {
+            if (successfulStatuses.count(getHTTPStatus()) && state->data.size() > 1024ul * 1024) {
                 return CURL_WRITEFUNC_PAUSE;
             }
 

lix/libstore/path-tree.cc

@@ -265,7 +265,7 @@ static std::map<StorePath, Node> mkGraph(
 
     for (auto & node : graph_data) {
         for (auto & ref : node.second.dependencies) {
-            graph_data.find(ref)->second.dependents.insert(node.first);
+            graph_data.at(ref).dependents.insert(node.first);
         }
     }
 

lix/libstore/profiles.cc

@@ -253,7 +253,7 @@ time_t parseOlderThanTimeSpec(std::string_view timeSpec)
 
     time_t curTime = time(0);
     auto strDays = timeSpec.substr(0, timeSpec.size() - 1);
-    auto days = string2Int<int>(strDays);
+    auto days = string2Int<time_t>(strDays);
 
     if (!days || *days < 1)
         throw UsageError("invalid number of days specifier '%1%'", timeSpec);

lix/libstore/s3-binary-cache-store.cc

@@ -195,8 +195,8 @@ ref<Aws::Client::ClientConfiguration> S3Helper::makeConfig(
     if (!endpoint.empty()) {
         res->endpointOverride = endpoint;
     }
-    res->requestTimeoutMs = 600 * 1000;
-    res->connectTimeoutMs = 5 * 1000;
+    res->requestTimeoutMs = 600l * 1000;
+    res->connectTimeoutMs = 5l * 1000;
     res->retryStrategy = std::make_shared<RetryStrategy>();
     res->caFile = settings.caFile;
     // Use the system proxy env-vars in curl for s3, which is off by default for some reason
@@ -312,8 +312,11 @@ struct S3BinaryCacheStoreConfig final : BinaryCacheStoreConfig
         "Whether to use multi-part uploads."};
 
     const Setting<uint64_t> bufferSize{
-        this, 5 * 1024 * 1024, "buffer-size",
-        "Size (in bytes) of each part in multi-part uploads."};
+        this,
+        5ul * 1024 * 1024,
+        "buffer-size",
+        "Size (in bytes) of each part in multi-part uploads."
+    };
 
     const std::string name() override { return "S3 Binary Cache Store"; }
 

lix/libstore/store-api.cc

@@ -1338,7 +1338,19 @@ try {
             Derivation::nameFromPath(drvPath)
         );
     } catch (FormatError & e) {
-        throw Error("error parsing derivation '%s': %s", store.printStorePath(drvPath), e.msg());
+        auto drvPathS = store.printStorePath(drvPath);
+        throw Error(
+            fmt("error parsing derivation '%1%': %2%\n"
+                "This can occur when the derivation is corrupted.\n"
+                "You can check this with `nix-store --verify-path %1%` and possibly repair with "
+                "`nix-store --repair-path %1%`.\n"
+                "In case the repair cannot be done, `nix-store --delete %1%` may be able "
+                "to remove the broken path.\n"
+                "We would appreciate a bug report at "
+                "https://git.lix.systems/lix-project/lix/issues if you think this is a bug.",
+                drvPathS,
+                e.msg())
+        );
     }
 } catch (...) {
     co_return result::current_exception();

lix/libutil/async-io.hh

@@ -121,7 +121,7 @@ public:
     {
     }
 
-    AsyncBufferedInputStream(AsyncInputStream & inner, size_t bufSize = 32 * 1024)
+    AsyncBufferedInputStream(AsyncInputStream & inner, size_t bufSize = 32ul * 1024)
         : AsyncBufferedInputStream(inner, make_ref<IoBuffer>(bufSize))
     {
     }
@@ -173,7 +173,7 @@ public:
     {
     }
 
-    AsyncBufferedOutputStream(AsyncOutputStream & inner, size_t bufSize = 32 * 1024)
+    AsyncBufferedOutputStream(AsyncOutputStream & inner, size_t bufSize = 32ul * 1024)
         : AsyncBufferedOutputStream(inner, make_ref<IoBuffer>(bufSize))
     {
     }

lix/libutil/compression.cc

@@ -16,7 +16,6 @@
 #include <archive.h>
 #include <archive_entry.h>
 #include <cerrno>
-#include <chrono>
 #include <cstdio>
 #include <cstring>
 
@@ -38,7 +37,7 @@ static const int COMPRESSION_LEVEL_DEFAULT = -1;
 // Don't feed brotli too much at once.
 struct ChunkedCompressionSink : CompressionSink
 {
-    uint8_t outbuf[32 * 1024];
+    uint8_t outbuf[32ul * 1024];
 
     void writeUnbuffered(std::string_view data) override
     {
@@ -63,13 +62,15 @@ struct ArchiveDecompressionSource : Source
         struct archive_entry * ae;
         if (!archive) {
             archive = std::make_unique<TarArchive>(*src, true);
-            this->archive->check(archive_read_next_header(this->archive->archive, &ae),
-                "failed to read header (%s)");
-            if (archive_filter_count(this->archive->archive) < 2) {
+            this->archive->check(
+                archive_read_next_header(this->archive->archive.get(), &ae),
+                "failed to read header (%s)"
+            );
+            if (archive_filter_count(this->archive->archive.get()) < 2) {
                 throw CompressionError("input compression not recognized");
             }
         }
-        ssize_t result = archive_read_data(this->archive->archive, data, len);
+        ssize_t result = archive_read_data(this->archive->archive.get(), data, len);
         if (result > 0) return result;
         if (result == 0) {
             throw EndOfFile("reached end of compressed file");
@@ -167,7 +168,7 @@ struct NoneSink : CompressionSink
 
 struct BrotliDecompressionSource : Source
 {
-    static constexpr size_t BUF_SIZE = 32 * 1024;
+    static constexpr size_t BUF_SIZE = 32ul * 1024;
     std::unique_ptr<char[]> buf;
     size_t avail_in = 0;
     const uint8_t * next_in;

lix/libutil/file-descriptor.cc

@@ -133,7 +133,7 @@ Generator<Bytes> drainFDSource(int fd, bool block)
         }
     });
 
-    std::array<unsigned char, 64 * 1024> buf;
+    std::array<unsigned char, 64ul * 1024> buf;
     while (1) {
         checkInterrupt();
         ssize_t rd = read(fd, buf.data(), buf.size());

lix/libutil/file-system.cc

@@ -443,7 +443,7 @@ void writeFile(const Path & path, Source & source, mode_t mode)
 {
     AutoCloseFD fd = openForWrite(path, mode);
 
-    std::vector<char> buf(64 * 1024);
+    std::vector<char> buf(64ul * 1024);
 
     try {
         while (true) {
@@ -463,7 +463,7 @@ void writeFileExcl(const Path & path, Source & source, mode_t mode)
 {
     AutoCloseFD fd = openForWriteExcl(path, mode);
 
-    std::vector<char> buf(64 * 1024);
+    std::vector<char> buf(64ul * 1024);
 
     try {
         while (true) {
@@ -483,7 +483,7 @@ kj::Promise<Result<void>> writeFile(const Path & path, AsyncInputStream & source
 try {
     AutoCloseFD fd = openForWrite(path, mode);
 
-    std::vector<char> buf(64 * 1024);
+    std::vector<char> buf(64ul * 1024);
 
     try {
         while (true) {

lix/libutil/hash.cc

@@ -191,9 +191,8 @@ Hash::Hash(std::string_view rest, HashType type, bool isSRI)
         };
 
         for (unsigned int i = 0; i < hashSize; i++) {
-            hash[i] =
-                parseHexDigit(rest[i * 2]) << 4
-                | parseHexDigit(rest[i * 2 + 1]);
+            const size_t j = i << 1;
+            hash[i] = parseHexDigit(rest[j]) << 4 | parseHexDigit(rest[j + 1]);
         }
     }
 

lix/libutil/io-buffer.hh

@@ -19,7 +19,7 @@ class IoBuffer
     std::unique_ptr<char[]> buffer;
 
 public:
-    explicit IoBuffer(size_t bufSize = 32 * 1024) : bufSize(bufSize) {}
+    explicit IoBuffer(size_t bufSize = 32ul * 1024) : bufSize(bufSize) {}
 
     size_t size() const
     {

lix/libutil/logging-rpc.cc

@@ -107,8 +107,8 @@ struct RpcLogger : Logger
         }
         buffer->sizeEstimate += sizeof(e) + extraSize;
         buffer->items.emplace_back(std::move(e));
-        return buffer->sizeEstimate >= 1024 * 1024 ? BufferState::NeedsFlush
-                                                   : BufferState::HasSpace;
+        return buffer->sizeEstimate >= static_cast<size_t>(1024 * 1024) ? BufferState::NeedsFlush
+                                                                        : BufferState::HasSpace;
     }
 
     BufferState log(Verbosity lvl, std::string_view s) override

lix/libutil/processes.cc

@@ -214,7 +214,7 @@ Pid startProcess(std::function<void()> fun, const ProcessOptions & options)
         // Not supported, since then we don't know when to free the stack.
         assert(!(options.cloneFlags & CLONE_VM));
 
-        size_t stackSize = 1 * 1024 * 1024;
+        size_t stackSize = 1ul * 1024 * 1024;
         auto stack = static_cast<char *>(mmap(0, stackSize,
             PROT_WRITE | PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0));
         if (stack == MAP_FAILED) throw SysError("allocating stack");

lix/libutil/serialise.hh

@@ -53,7 +53,7 @@ struct BufferedSink : virtual Sink
 {
     ref<IoBuffer> buffer;
 
-    BufferedSink(size_t bufSize = 32 * 1024) : buffer(make_ref<IoBuffer>(bufSize)) {}
+    BufferedSink(size_t bufSize = 32ul * 1024) : buffer(make_ref<IoBuffer>(bufSize)) {}
     explicit BufferedSink(ref<IoBuffer> buffer) : buffer(std::move(buffer)) {}
 
     void operator () (std::string_view data) override;
@@ -105,7 +105,7 @@ struct BufferedSource : Source
 {
     ref<IoBuffer> buffer;
 
-    BufferedSource(size_t bufSize = 32 * 1024) : buffer(make_ref<IoBuffer>(bufSize)) {}
+    BufferedSource(size_t bufSize = 32ul * 1024) : buffer(make_ref<IoBuffer>(bufSize)) {}
     explicit BufferedSource(ref<IoBuffer> buffer) : buffer(std::move(buffer)) {}
 
     size_t read(char * data, size_t len) override;

lix/libutil/tarfile.cc

@@ -1,9 +1,11 @@
 #include <archive.h>
 #include <archive_entry.h>
 #include <kj/async.h>
+#include <map>
 
 #include "async-io.hh"
 #include "file-descriptor.hh"
+#include "lix/libstore/temporary-dir.hh"
 #include "lix/libutil/c-calls.hh"
 #include "lix/libutil/charptr-cast.hh"
 #include "lix/libutil/file-system.hh"
@@ -45,67 +47,110 @@ void TarArchive::check(int err, const std::string & reason)
     if (err == ARCHIVE_EOF)
         throw EndOfFile("reached end of archive");
     else if (err != ARCHIVE_OK)
-        throw Error(reason, archive_error_string(this->archive));
+        throw Error(reason, archive_error_string(this->archive.get()));
 }
 
-TarArchive::TarArchive(Source & source, bool raw) : buffer(65536)
+TarArchive::TarArchive(Source & source, bool raw)
+    : archive{archive_read_new(), archive_read_free}
+    , source(&source)
+    , buffer(65536)
 {
-    this->archive = archive_read_new();
-    this->source = &source;
-
     if (!raw) {
-        archive_read_support_filter_all(archive);
-        archive_read_support_format_all(archive);
+        archive_read_support_filter_all(archive.get());
+        archive_read_support_format_all(archive.get());
     } else {
-        archive_read_support_filter_all(archive);
-        archive_read_support_format_raw(archive);
-        archive_read_support_format_empty(archive);
+        archive_read_support_filter_all(archive.get());
+        archive_read_support_format_raw(archive.get());
+        archive_read_support_format_empty(archive.get());
     }
-    archive_read_set_option(archive, nullptr, "mac-ext", nullptr);
-    check(archive_read_open(archive, (void *)this, callback_open, callback_read, callback_close), "Failed to open archive (%s)");
+    archive_read_set_option(archive.get(), nullptr, "mac-ext", nullptr);
+    check(
+        archive_read_open(
+            archive.get(), (void *) this, callback_open, callback_read, callback_close
+        ),
+        "Failed to open archive (%s)"
+    );
 }
 
-
-TarArchive::TarArchive(const Path & path)
+TarArchive::TarArchive(const Path & path) : archive{archive_read_new(), archive_read_free}
 {
-    this->archive = archive_read_new();
-
-    archive_read_support_filter_all(archive);
-    archive_read_support_format_all(archive);
-    archive_read_set_option(archive, nullptr, "mac-ext", nullptr);
+    archive_read_support_filter_all(archive.get());
+    archive_read_support_format_all(archive.get());
+    archive_read_set_option(archive.get(), nullptr, "mac-ext", nullptr);
     check(
-        archive_read_open_filename(archive, requireCString(path), 16384),
+        archive_read_open_filename(archive.get(), requireCString(path), 16384),
         "failed to open archive: %s"
     );
 }
 
 void TarArchive::close()
 {
-    check(archive_read_close(this->archive), "Failed to close archive (%s)");
+    check(archive_read_close(this->archive.get()), "Failed to close archive (%s)");
 }
 
-TarArchive::~TarArchive()
+namespace {
+
+struct IndexEntry
 {
-    if (this->archive) archive_read_free(this->archive);
+    struct IndexDirectory
+    {
+        std::map<std::string, IndexEntry> entries;
+    };
+
+    struct IndexFile
+    {
+        uint64_t start;
+        uint64_t size;
+    };
+
+    std::variant<IndexDirectory, IndexFile> innerData;
+    int mode;
+};
+
+/** Extract a TAR archive into an in-memory index with data stored on disk,
+ * because fundamentally all we have to do when importing archives to the store
+ * is to sort the entries and then send it into a NAR serializer.
+ *
+ * This deals with macOS performance problems by not materializing the files to
+ * disk in the first place.
+ *
+ * https://git.lix.systems/lix-project/lix/issues/1072
+ */
+struct InMemoryIndex
+{
+    Path tempDir;
+    AutoDelete tempDirDeleter;
+    AutoCloseFD backingFile;
+    std::ofstream writer;
+
+    IndexEntry topLevel;
+
+    InMemoryIndex() : tempDir(createTempDir()), tempDirDeleter(tempDir, true)
+    {
+        backingFile = sys::open(tempDir + "/extracted", O_RDWR | O_TRUNC | O_CLOEXEC);
+    }
+};
+
 }
 
 static void extract_archive(TarArchive & archive, const Path & destDir)
 {
     requireCString(destDir);
 
-    int flags = ARCHIVE_EXTRACT_TIME
-        | ARCHIVE_EXTRACT_SECURE_SYMLINKS
-        | ARCHIVE_EXTRACT_SECURE_NODOTDOT;
+    int flags = ARCHIVE_EXTRACT_TIME | ARCHIVE_EXTRACT_SECURE_SYMLINKS
+        | ARCHIVE_EXTRACT_SECURE_NODOTDOT | ARCHIVE_EXTRACT_NO_HFS_COMPRESSION;
 
     for (;;) {
         struct archive_entry * entry;
-        int r = archive_read_next_header(archive.archive, &entry);
+        int r = archive_read_next_header(archive.archive.get(), &entry);
         if (r == ARCHIVE_EOF) break;
         auto name = archive_entry_pathname(entry);
         if (!name)
-            throw Error("cannot get archive member name: %s", archive_error_string(archive.archive));
+            throw Error(
+                "cannot get archive member name: %s", archive_error_string(archive.archive.get())
+            );
         if (r == ARCHIVE_WARN)
-            printTaggedWarning("%1%", Uncolored(archive_error_string(archive.archive)));
+            printTaggedWarning("%1%", Uncolored(archive_error_string(archive.archive.get())));
         else
             archive.check(r);
 
@@ -125,7 +170,7 @@ static void extract_archive(TarArchive & archive, const Path & destDir)
                 (destDir + "/" + original_hardlink).c_str());
         }
 
-        archive.check(archive_read_extract(archive.archive, entry, flags));
+        archive.check(archive_read_extract(archive.archive.get(), entry, flags));
     }
 
     archive.close();

lix/libutil/tarfile.hh

@@ -8,7 +8,7 @@
 namespace nix {
 
 struct TarArchive {
-    struct archive * archive;
+    std::unique_ptr<struct archive, decltype(&archive_read_free)> archive;
     Source * source;
     std::vector<unsigned char> buffer;
 
@@ -22,8 +22,6 @@ struct TarArchive {
     TarArchive(const TarArchive &) = delete;
 
     void close();
-
-    ~TarArchive();
 };
 
 kj::Promise<Result<void>> unpackTarfile(AsyncInputStream & source, const Path & destDir);

lix/nix/daemon.cc

@@ -55,7 +55,6 @@
 #endif
 
 static constexpr int SUBDAEMON_CONNECTION_FD = 0;
-static constexpr int SUBDAEMON_SETTINGS_FD = 3;
 
 namespace nix {
 
@@ -346,9 +345,6 @@ try {
                 peer.pidKnown ? fmt("pid %1%", peer.pid) : "unknown peer"
             );
 
-            Pipe settings;
-            settings.create();
-
             // Fork a child to handle the connection. make sure it's called with
             // argv0 `nix-daemon` so we don't try to run `nix --for` when called
             // from more modern scripts that assume nix-command being available.
@@ -363,11 +359,7 @@ try {
                         fmt("%1%", int(verbosity)),
                     },
                 .dieWithParent = false,
-                .redirections =
-                    {
-                        {.dup = SUBDAEMON_CONNECTION_FD, .from = remote.get()},
-                        {.dup = SUBDAEMON_SETTINGS_FD, .from = settings.readSide.get()},
-                    }
+                .redirections = {{.dup = SUBDAEMON_CONNECTION_FD, .from = remote.get()}}
             };
             if (forceTrustClientOpt) {
                 options.args.push_back(
@@ -375,15 +367,6 @@ try {
                 );
             }
             runProgram2(options).release();
-
-            FdSink sink(settings.writeSide.get());
-            std::map<std::string, Config::SettingInfo> overriddenSettings;
-            globalConfig.getSettings(overriddenSettings, true);
-            for (auto & setting : overriddenSettings) {
-                sink << 1 << setting.first << setting.second.value;
-            }
-            sink << 0;
-            sink.flush();
         } catch (Error & error) {
             auto ei = error.info();
             // FIXME: add to trace?
@@ -417,21 +400,6 @@ static void daemonInstance(AsyncIoRoot & aio, std::optional<TrustedFlag> forceTr
         forceTrustClientOpt ? " by override" : ""
     );
 
-    {
-        FdSource source(SUBDAEMON_SETTINGS_FD);
-
-        /* Read the parent's settings. */
-        while (readNum<unsigned>(source)) {
-            auto name = readString(source);
-            auto value = readString(source);
-            settings.set(name, value);
-        }
-
-        if (close(SUBDAEMON_SETTINGS_FD) < 0) {
-            throw SysError("preparing subdaemon connection");
-        }
-    }
-
     //  Background the daemon.
     if (setsid() == -1) {
         throw SysError("creating a new session");

lix/nix/develop.cc

@@ -364,6 +364,9 @@ struct Common : InstallableCommand, MixProfile
             out << fmt("%s=\"$%s${nix_saved_%s:+:$nix_saved_%s}\"\n", var, var, var, var);
 
         out << "export NIX_BUILD_TOP=\"$(mktemp -d -t nix-shell.XXXXXX)\"\n";
+        // We re-export similarly to what occurs inside of a derivation goal `NIX_LOG_FD` to stderr.
+        // So that stdenv hooks that logs information can be observed inside this debugging tool.
+        out << "export NIX_LOG_FD=2\n";
         for (auto & i : {"TMP", "TMPDIR", "TEMP", "TEMPDIR"})
             out << fmt("export %s=\"$NIX_BUILD_TOP\"\n", i);
 

lix/nix/diff-closures.cc

@@ -152,7 +152,7 @@ void renderDiffInfo(
     const std::string_view indent)
 {
     for (auto & [name, item] : diff) {
-        auto showDelta = std::abs(item.sizeDelta) >= 8 * 1024;
+        auto showDelta = std::abs(item.sizeDelta) >= 8l * 1024;
 
         std::vector<std::string> line;
         if (!item.removedVersions.empty() || !item.addedVersions.empty())

lix/nix/main.cc

@@ -455,8 +455,6 @@ void registerNixHelp()
 
 int mainWrapped(AsyncIoRoot & aio, int argc, char ** argv)
 {
-    savedArgv = argv;
-
     /* The chroot helper needs to be run before any threads have been
        started. */
     if (argc > 0 && argv[0] == chrootHelperName) {
@@ -493,7 +491,12 @@ int mainWrapped(AsyncIoRoot & aio, int argc, char ** argv)
         registerLegacyCommands();
         auto legacy = (*LegacyCommandRegistry::commands)[programName];
         if (legacy) {
-            return legacy(aio, std::string(baseNameOf(argv[0])), Strings(argv + 1, argv + argc));
+            return legacy(
+                aio,
+                std::string(baseNameOf(argv[0])),
+                Strings(argv + 1, argv + argc),
+                {argv + 1, argv + argc}
+            );
         }
     }
 
@@ -645,7 +648,6 @@ int mainWrapped(AsyncIoRoot & aio, int argc, char ** argv)
 
     return 0;
 }
-
 }
 
 int main(int argc, char * * argv)
@@ -657,7 +659,7 @@ int main(int argc, char * * argv)
 
     // Increase the default stack size for the evaluator and for
     // libstdc++'s std::regex.
-    nix::setStackSize(64 * 1024 * 1024);
+    nix::setStackSize(64ul * 1024 * 1024);
 
     return nix::handleExceptions(argv[0], [&]() {
         nix::AsyncIoRoot aio;

lix/nix/upgrade-nix.cc

@@ -49,6 +49,19 @@ struct CmdUpgradeNix : MixDryRun, EvalCommand
         });
     }
 
+    // NOTE(Raito): we override the store creation
+    // to prevent any store daemon connection.
+    //
+    // An upgrade, by nature, requires a direct store access
+    // to avoid having the daemon die in the middle of changing the binary.
+    //
+    // If more commands needs that, we can move it into a mixin. This was deliberately not done
+    // here.
+    virtual ref<Store> createStore(AsyncIoRoot & aio) override
+    {
+        return aio.blockOn(openStore(settings.storeUri.get(), {}, AllowDaemon::Disallow));
+    }
+
     /**
      * This command is stable before the others
      */

package.nix

@@ -358,6 +358,7 @@ stdenv.mkDerivation (finalAttrs: {
   ++ lib.optional (!officialRelease && buildUnreleasedNotes) build-release-notes
   ++ lib.optional internalApiDocs doxygen
   ++ lib.optionals lintInsteadOfBuild [
+    llvmPackages.libllvm
     # required for a wrapped clang-tidy
     llvmPackages.clang-tools
     # load-bearing order (just as below); the actual stdenv wrapped clang

perl/meson.build

@@ -1,7 +1,7 @@
 project('lix-perl', 'cpp',
   version : run_command('bash', '-c', 'echo -n $(jq -r .version < ../version.json)$VERSION_SUFFIX', check : true).stdout().strip(),
   default_options : [
-    'cpp_std=c++2a',
+    'cpp_std=c++23',
     # TODO(Qyriad): increase the warning level
     'debug=true',
     # FIXME(Qyriad): should this be -O2? The main nix build was switched to -O2 in 3c5234430

subprojects/lix-clang-tidy/CharPtrCast.cc

@@ -22,7 +22,7 @@ void CharPtrCastCheck::check(
   const auto ReinterpretCastExpr =
       Result.Nodes.getNodeAs<CXXReinterpretCastExpr>("reinterpret-cast-expr");
   const auto ToTypeSpan = ReinterpretCastExpr->getAngleBrackets();
-  const auto & SM = Result.Context->getSourceManager();
+  const auto &SM = Result.Context->getSourceManager();
 
   auto Diag =
       diag(ReinterpretCastExpr->getExprLoc(),
@@ -32,14 +32,16 @@ void CharPtrCastCheck::check(
   auto Inside = tooling::getText(*ReinterpretCastExpr->getSubExprAsWritten(),
                                  *Result.Context);
 
-  Diag << Inserter.createIncludeInsertion(SM.getFileID(ReinterpretCastExpr->getExprLoc()), "charptr-cast.hh");
+  Diag << Inserter.createIncludeInsertion(
+      SM.getFileID(ReinterpretCastExpr->getExprLoc()), "charptr-cast.hh");
 
-  llvm::Twine Replacement =
-      "charptr_cast" +
-      tooling::getText(CharSourceRange(ToTypeSpan, true), *Result.Context) +
-      "(" + Inside + ")";
+  auto Replacement =
+      ("charptr_cast" +
+       tooling::getText(CharSourceRange(ToTypeSpan, true), *Result.Context) +
+       "(" + Inside + ")")
+          .str();
   Diag << FixItHint::CreateReplacement(ReinterpretCastExpr->getSourceRange(),
-                                       Replacement.str());
+                                       Replacement);
 }
 
 } // namespace nix::clang_tidy

subprojects/nix-eval-jobs/meson.build

@@ -4,7 +4,7 @@ project('nix-eval-jobs', 'cpp',
   default_options : [
     'debug=true',
     'optimization=2',
-    'cpp_std=c++20',
+    'cpp_std=c++23',
   ],
 )
 

subprojects/nix-eval-jobs/src/buffered-io.cc

@@ -56,3 +56,38 @@ LineReader::LineReader(LineReader &&other) {
     // Remove trailing newline
     return std::string_view(buffer, read - 1);
 }
+
+AsyncLineReader::AsyncLineReader(nix::AutoCloseFD fd)
+    : stream{std::move(fd)}, readBuffer{kj::heapArray<char>(4096)} {}
+
+kj::Promise<nix::Result<std::optional<std::string>>> AsyncLineReader::readLine()
+try {
+    auto pos = buffer.find('\n');
+    if (pos != std::string::npos) {
+        std::string result = buffer.substr(0, pos);
+        memmove(buffer.data(), buffer.data() + pos + 1, buffer.size() - pos - 1);
+        buffer.resize(buffer.size() - pos - 1);
+        co_return result;
+    }
+
+    // No full line was buffered, read until we have one.
+    while (true) {
+        auto nRead = LIX_TRY_AWAIT(stream.read(readBuffer.begin(), readBuffer.size()));
+        if (!nRead && !buffer.empty()) {
+            // File has ended, but not everything has been read out of the buffer yet.
+            co_return std::move(buffer);
+        } else if (!nRead) {
+            co_return std::nullopt;
+        }
+        std::string_view readStr{readBuffer.begin(), *nRead};
+
+        auto pos = readStr.find('\n');
+        if (pos != std::string_view::npos) {
+            buffer.append(readStr.substr(0, pos));
+            co_return std::exchange(buffer, readStr.substr(pos + 1));
+        }
+        buffer.append(readStr);
+    }
+} catch (...) {
+    co_return nix::result::current_exception();
+}

subprojects/nix-eval-jobs/src/buffered-io.hh

@@ -1,4 +1,6 @@
 #pragma once
+#include <lix/libutil/async-io.hh>
+#include <lix/libutil/file-descriptor.hh>
 #include <cstdio>
 #include <string>
 #include <string_view>
@@ -18,3 +20,15 @@ class LineReader {
     char *buffer = nullptr;
     size_t len = 0;
 };
+
+class AsyncLineReader {
+public:
+    AsyncLineReader(nix::AutoCloseFD fd);
+
+    kj::Promise<nix::Result<std::optional<std::string>>> readLine();
+
+private:
+    nix::AsyncFdIoStream stream;
+    std::string buffer;
+    kj::Array<char> readBuffer;
+};

subprojects/nix-eval-jobs/src/nix-eval-jobs.cc

@@ -4,6 +4,8 @@
 #include <lix/libexpr/eval-settings.hh>
 #include <lix/libmain/shared.hh>
 #include <lix/libutil/async.hh>
+#include <lix/libutil/async-collect.hh>
+#include <lix/libutil/async-semaphore.hh>
 #include <lix/libutil/sync.hh>
 #include <lix/libexpr/eval.hh>
 #include <lix/libutil/json.hh>
@@ -25,14 +27,11 @@
 #include <lix/libutil/terminal.hh>
 #include <lix/libutil/ref.hh>
 #include <lix/libstore/store-api.hh>
-#include <condition_variable>
 #include <filesystem>
-#include <exception>
 #include <functional>
 #include <iostream>
 #include <memory>
 #include <optional>
-#include <set>
 #include <string>
 #include <string_view>
 #include <thread>
@@ -75,8 +74,8 @@ class Collector {
     using Response = std::variant<Next, JsonResponse, Restart>;
 
     RunningProgram child;
-    AutoCloseFD to;
-    std::optional<LineReader> from;
+    std::optional<AsyncFdIoStream> to;
+    std::optional<AsyncLineReader> from;
 
     Strings workerCmdline;
 
@@ -105,11 +104,12 @@ class Collector {
         };
 
         child = runProgram2(options);
-        to = std::move(toPipe.writeSide);
-        from.emplace(fromPipe.readSide.release());
+        to.emplace(std::move(toPipe.writeSide));
+        from.emplace(std::move(fromPipe.readSide));
     }
 
-    void waitForWorkerReady() {
+    kj::Promise<Result<void>> waitForWorkerReady()
+    try {
         assert(child);
         std::visit(overloaded{
             [](const Next &) {},
@@ -121,53 +121,66 @@ class Collector {
                 to.reset();
                 from.reset();
             },
-        }, readResponse("checking worker process"));
+        }, LIX_TRY_AWAIT(readResponse("checking worker process")));
+        co_return result::success();
+    } catch (...) {
+        co_return result::current_exception();
     }
 
-    void makeWorkerReady() {
+    kj::Promise<Result<void>> makeWorkerReady()
+    try {
         if (child) {
-            waitForWorkerReady();
+            LIX_TRY_AWAIT(waitForWorkerReady());
         }
         if (!child) {
             startWorker();
-            waitForWorkerReady();
+            LIX_TRY_AWAIT(waitForWorkerReady());
         }
         if (!child) {
             throw Error("worker exited immediately");
         }
+        co_return result::success();
+    } catch (...) {
+        co_return result::current_exception();
     }
 
-    Response readResponse(std::string_view msg) {
+    kj::Promise<Result<Response>> readResponse(std::string_view msg)
+    try {
         assert(from);
-        auto line = from->readLine();
-        if (line.empty()) {
+        auto line = LIX_TRY_AWAIT(from->readLine());
+        if (!line) {
             handleBrokenPipe(msg);
         } else if (line == "next") {
-            return Next{};
+            co_return Next{};
         } else if (line == "restart") {
-            return Restart{};
+            co_return Restart{};
         } else {
             try {
-                return JsonResponse{JSON::parse(line)};
+                co_return JsonResponse{JSON::parse(*line)};
             } catch (const json::ParseError &e) {
                 throw Error(
                     "Received invalid JSON from worker: %s\n json: '%s'",
-                    e.what(), line);
+                    e.what(), *line);
             }
         }
+    } catch (...) {
+        co_return result::current_exception();
     }
 
-    void writeRequest(Request request) {
+    kj::Promise<Result<void>> writeRequest(Request request)
+    try {
         assert(to);
         auto line = std::visit(overloaded{
             [](const Do &request) {
-                return fmt("do %s", request.attrPath.dump());
+                return fmt("do %s\n", request.attrPath.dump());
             },
             [](const Exit &) {
-                return std::string{"exit"};
+                return std::string{"exit\n"};
             },
         }, request);
-        if (tryWriteLine(to.get(), line) < 0) {
+        try {
+            LIX_TRY_AWAIT(to->writeFull(line.data(), line.size()));
+        } catch (SysError &err) {
             auto msg = std::visit(overloaded{
                 [](const Do &request) {
                     return fmt("sending attrPath '%s'", joinAttrPath(request.attrPath));
@@ -178,6 +191,9 @@ class Collector {
             }, request);
             handleBrokenPipe(msg);
         }
+        co_return result::success();
+    } catch (...) {
+        co_return result::current_exception();
     }
 
     [[noreturn]] void handleBrokenPipe(std::string_view msg) {
@@ -240,10 +256,11 @@ public:
         }
     }
 
-    JSON evaluate(JSON attrPath) {
-        makeWorkerReady();
-        writeRequest(Do{attrPath});
-        return std::visit(overloaded{
+    kj::Promise<Result<JSON>> evaluate(JSON attrPath)
+    try {
+        LIX_TRY_AWAIT(makeWorkerReady());
+        LIX_TRY_AWAIT(writeRequest(Do{attrPath}));
+        co_return std::visit(overloaded{
             [](const Next &) -> JSON {
                 throw Error("unexpected response from worker: next");
             },
@@ -253,93 +270,101 @@ public:
             [](const Restart &) -> JSON {
                 throw Error("unexpected response from worker: restart");
             },
-        }, readResponse(fmt("reading result for attrPath '%s'", joinAttrPath(attrPath))));
-    }
-
-    void exit() {
-        if (child) {
-            waitForWorkerReady();
-        }
-        if (child) {
-            writeRequest(Exit{});
-            // The worker will print "restart" when exiting cleanly, even if due to an explicit exit request.
-            waitForWorkerReady();
-        }
-    }
-};
-
-struct State {
-    std::set<JSON> todo = JSON::array({JSON::array()});
-    std::set<JSON> active;
-    std::exception_ptr exc;
-    std::map<std::string, JSON> jobs;
-};
-
-void collectorThread(MyArgs &myArgs, Sync<State> &state_, std::condition_variable &wakeup) {
-    try {
-        Collector collector{myArgs.cmdline};
-
-        while (true) {
-            /* Wait for a job name to become available. */
-            JSON attrPath;
-            while (true) {
-                checkInterrupt();
-                auto state(state_.lock());
-                if ((state->todo.empty() && state->active.empty()) ||
-                    state->exc) {
-                    collector.exit();
-                    return;
-                }
-                if (!state->todo.empty()) {
-                    attrPath = *state->todo.begin();
-                    state->todo.erase(state->todo.begin());
-                    state->active.insert(attrPath);
-                    break;
-                } else
-                    state.wait(wakeup);
-            }
-
-            /* Tell the worker to evaluate it. */
-            auto response = collector.evaluate(attrPath);
-
-            /* Handle the response. */
-            std::vector<JSON> newAttrs;
-            if (response.find("attrs") != response.end()) {
-                for (auto &i : response["attrs"]) {
-                    JSON newAttr = JSON(response["attrPath"]);
-                    newAttr.emplace_back(i);
-                    newAttrs.push_back(newAttr);
-                }
-            } else {
-                auto state(state_.lock());
-                state->jobs.insert_or_assign(response["attr"], response);
-                if (nix::settings.readOnlyMode) {
-                    response.erase("namedConstituents");
-                    response.erase("constituents");
-                }
-                auto named = response.find("namedConstituents");
-                if (named == response.end() || named->empty()) {
-                    response.erase("namedConstituents");
-                    logger->writeToStdout(response.dump());
-                }
-            }
-
-            /* Add newly discovered job names to the queue. */
-            {
-                auto state(state_.lock());
-                state->active.erase(attrPath);
-                for (auto p : newAttrs) {
-                    state->todo.insert(p);
-                }
-                wakeup.notify_all();
-            }
-        }
+        }, LIX_TRY_AWAIT(readResponse(fmt("reading result for attrPath '%s'", joinAttrPath(attrPath)))));
     } catch (...) {
-        auto state(state_.lock());
-        state->exc = std::current_exception();
-        wakeup.notify_all();
+        co_return result::current_exception();
     }
-}
+
+    kj::Promise<Result<void>> exit()
+    try {
+        if (child) {
+            LIX_TRY_AWAIT(waitForWorkerReady());
+        }
+        if (child) {
+            LIX_TRY_AWAIT(writeRequest(Exit{}));
+            // The worker will print "restart" when exiting cleanly, even if due to an explicit exit request.
+            LIX_TRY_AWAIT(waitForWorkerReady());
+        }
+        co_return result::success();
+    } catch (...) {
+        co_return result::current_exception();
+    }
+};
+
+class Coordinator {
+    AsyncSemaphore semaphore;
+    kj::Array<Collector> workers;
+    std::vector<Collector *> idleWorkers;
+
+    kj::Promise<Result<void>> evaluateRecursively(JSON attrPath, std::map<std::string, JSON> &jobs)
+    try {
+        JSON response;
+        {
+            auto _token = co_await semaphore.acquire();
+            auto *worker = idleWorkers.back();
+            idleWorkers.pop_back();
+            Finally _returnWorker{[&]() {
+                idleWorkers.push_back(worker);
+            }};
+            response = LIX_TRY_AWAIT(worker->evaluate(attrPath));
+        }
+
+        std::vector<JSON> newAttrs;
+        if (response.find("attrs") != response.end()) {
+            for (auto &i : response["attrs"]) {
+                JSON newAttr = JSON(response["attrPath"]);
+                newAttr.emplace_back(i);
+                newAttrs.push_back(newAttr);
+            }
+        } else {
+            jobs.insert_or_assign(response["attr"], response);
+            if (nix::settings.readOnlyMode) {
+                response.erase("namedConstituents");
+                response.erase("constituents");
+            }
+            auto named = response.find("namedConstituents");
+            if (named == response.end() || named->empty()) {
+                response.erase("namedConstituents");
+                nix::logger->writeToStdout(response.dump());
+            }
+        }
+
+        LIX_TRY_AWAIT(asyncSpread(newAttrs, [&](const JSON &newAttr) {
+            return evaluateRecursively(newAttr, jobs);
+        }));
+
+        co_return result::success();
+    } catch (...) {
+        co_return result::current_exception();
+    }
+
+public:
+    Coordinator(size_t nrWorkers, const Strings &cmdline)
+        : semaphore{static_cast<unsigned>(nrWorkers)} {
+        if (nrWorkers >= std::numeric_limits<unsigned>::max() - 1) {
+            throw Error("nix-eval-jobs cannot handle %d workers, please choose a reasonable number");
+        }
+
+        auto builder = kj::heapArrayBuilder<Collector>(nrWorkers);
+        for (unsigned i = 0; i < nrWorkers; ++i) {
+            auto worker = &builder.add(cmdline);
+            idleWorkers.push_back(worker);
+        }
+        workers = builder.finish();
+    }
+
+    kj::Promise<Result<std::map<std::string, JSON>>> run()
+    try {
+        std::map<std::string, JSON> jobs;
+        LIX_TRY_AWAIT(evaluateRecursively(JSON::array(), jobs));
+        for (auto &worker : workers) {
+            LIX_TRY_AWAIT(worker.exit());
+        }
+        co_return jobs;
+    } catch (...) {
+        co_return result::current_exception();
+    }
+};
 
 int main(int argc, char **argv) {
     return handleExceptions(argv[0], [&]() {
@@ -390,23 +415,8 @@ int main(int argc, char **argv) {
             return 0;
         }
 
-        Sync<State> state_;
-
-        /* Start a collector thread per worker process. */
-        std::vector<std::thread> threads;
-        std::condition_variable wakeup;
-        for (size_t i = 0; i < myArgs.nrWorkers; i++) {
-            threads.emplace_back(std::bind(collectorThread, std::ref(myArgs),
-                                           std::ref(state_), std::ref(wakeup)));
-        }
-
-        for (auto &thread : threads)
-            thread.join();
-
-        auto state(state_.lock());
-
-        if (state->exc)
-            std::rethrow_exception(state->exc);
+        Coordinator coordinator{myArgs.nrWorkers, myArgs.cmdline};
+        auto jobs = aio.blockOn(coordinator.run());
 
         if (myArgs.constituents) {
             auto store = aio.blockOn(myArgs.evalStoreUrl
@@ -415,28 +425,28 @@ int main(int argc, char **argv) {
             std::visit(
                 nix::overloaded{
                     [&](const std::vector<AggregateJob> &namedConstituents) {
-                        rewriteAggregates(state->jobs, namedConstituents, store,
+                        rewriteAggregates(jobs, namedConstituents, store,
                                           myArgs.gcRootsDir, aio);
                     },
                     [&](const DependencyCycle &e) {
                         printError(
                             "Found dependency cycle between jobs '%s' and '%s'",
                             e.a, e.b);
-                        state->jobs[e.a]["error"] = e.message();
-                        state->jobs[e.b]["error"] = e.message();
+                        jobs[e.a]["error"] = e.message();
+                        jobs[e.b]["error"] = e.message();
 
-                        logger->writeToStdout(state->jobs[e.a].dump());
-                        logger->writeToStdout(state->jobs[e.b].dump());
+                        nix::logger->writeToStdout(jobs[e.a].dump());
+                        nix::logger->writeToStdout(jobs[e.b].dump());
 
                         for (const auto &jobName : e.remainingAggregates) {
-                            state->jobs[jobName]["error"] =
+                            jobs[jobName]["error"] =
                                 "Skipping aggregate because of a dependency "
                                 "cycle";
-                            logger->writeToStdout(state->jobs[jobName].dump());
+                            nix::logger->writeToStdout(jobs[jobName].dump());
                         }
                     },
                 },
-                resolveNamedConstituents(state->jobs));
+                resolveNamedConstituents(jobs));
         }
 
         return 0;

tests/functional/meson.build

@@ -160,7 +160,6 @@ functional_tests_scripts = [
   'regression-reference-checks.sh',
   'redirected-filter-source.sh',
   'daemon-trust.sh',
-  'output-cycles.sh',
 ]
 
 # Plugin tests require shared libraries support.

tests/functional/nix-shell/basic.sh

@@ -75,6 +75,47 @@ output=$(NIX_PATH=nixpkgs="$shellDotNix" nix-shell --pure -p foo --argstr fooCon
     test ! -d "$(dirname $IN_SHELL_TEMPDIR)" && echo "nix-shell deleted the parent directory of \$TEMPDIR: clean up successful" || { echo "nix-shell did not delete the parent directory of \$TEMPDIR: clean up failure"; exit 1; }
 )
 
+# Test whether the added length w.r.t. base directories is reasonable enough.
+# This prevents regressions that can affect negatively things like opening UNIX
+# domain sockets in non-isolated builds.
+(
+    unset TMPDIR
+    BASE_DIR="$(realpath /tmp)"
+
+    LENGTH_RESULT=$(
+      NIX_PATH=nixpkgs="$shellDotNix" nix-shell --pure -p foo --run "
+        BASE_DIR_ENV=\"$BASE_DIR\"
+        ADDED_LENGTH=\$(( \${#NIX_BUILD_TOP} - \${#BASE_DIR_ENV} ))
+        echo \"\$ADDED_LENGTH:\$NIX_BUILD_TOP\"
+      "
+    )
+
+    ADDED_LENGTH="${LENGTH_RESULT%%:*}"
+    IN_SHELL_NIX_BUILD_TOP="${LENGTH_RESULT#*:}"
+
+    # The idea of this value is len("nix-shell-$hash/build-top") + ≤5 chars of margin.
+    MAX_ALLOWED=50
+    # The added length must be minimum 2 chars.
+    MIN_ALLOWED=2
+
+    if (( ADDED_LENGTH < MIN_ALLOWED )); then
+        echo "Added length ($ADDED_LENGTH) is impossibly low. The test is not correct."
+        exit 1
+    fi
+
+    if (( ADDED_LENGTH <= MAX_ALLOWED )); then
+        echo "Added length to \$NIX_BUILD_TOP in shells ($ADDED_LENGTH chars) is within acceptable limit ($MAX_ALLOWED chars)."
+    else
+        echo "ERROR: Added length too large ($ADDED_LENGTH chars > $MAX_ALLOWED chars)."
+        echo "Base: $BASE_DIR"
+        echo "NIX_BUILD_TOP: $IN_SHELL_NIX_BUILD_TOP"
+        exit 1
+    fi
+)
+
+# Test $NIX_LOG_FD
+expectStderr 0 nix-shell --pure $shellDotNix -A shellDrv --run 'echo hello > $NIX_LOG_FD' |& grepQuiet 'hello'
+
 # FIXME: testing that Ctrl-C to a (non-)interactive nix-shell stops it would be appreciated,
 # but it is hard to send accurately the signal to the right process group.
 
@@ -135,6 +176,9 @@ shellDrv=$(nix-instantiate "$shellDotNix" -A shellDrv.out)
 
 nix develop $shellDrv -c bash -c '[[ -n $stdenv ]]'
 
+# Test $NIX_LOG_FD
+expectStderr 0 nix develop $shellDrv -c bash -c 'echo hello > $NIX_LOG_FD' |& grepQuiet 'hello'
+
 nix print-dev-env $shellDrv > $TEST_ROOT/dev-env2.sh
 nix print-dev-env $shellDrv --json > $TEST_ROOT/dev-env2.json
 

tests/functional/output-cycles.sh

@@ -1,18 +0,0 @@
-source common.sh
-
-clearStore
-
-error="$(! nix-build check-outputs.nix -A cycle 2>&1)"
-grepQuiet "cycle detected in build of '.*' in the references of output 'bar' from output 'foo'" <<<"$error"
-
-if [[ "$(uname -s)" = Linux ]]; then
-    <<<"$error" grepQuiet "/store/.*-cycle-bar"
-    <<<"$error" grepQuiet "└───lib/libfoo: ….*cycle-baz.*"
-    <<<"$error" grepQuiet "    →.*/store/.*-cycle-baz"
-    <<<"$error" grepQuiet "    └───share/lalala:.*-cycle-foo.*"
-fi
-
-error="$(! nix-build check-outputs.nix -A as_dependency 2>&1)"
-
-grepQuiet "cycle detected in build of '.*' in the references of output 'bar' from output 'foo'" <<<"$error"
-grepQuiet "error: 1 dependencies of derivation" <<<"$error"

tests/functional2/commands/test_build/assets/test_build/output-cycles.nix

@@ -15,6 +15,18 @@ rec {
     '';
   };
 
+  cycle-with-deps = mkDerivation {
+    name = "cycle-with-deps";
+    inherit dep;
+    outputs = [ "foo" "bar" ];
+    builder = builtins.toFile "builder.sh" ''
+      mkdir -p $foo/bin $bar/lib
+      ln -sf $dep $bar/lib
+      echo $foo > $bar/txt
+      echo $bar > $foo/txt
+    '';
+  };
+
   as_dependency = mkDerivation {
     name = "depends-on-cycle";
     inherit cycle;

tests/functional2/commands/test_build/test_build_output_cycles.py

@@ -0,0 +1,78 @@
+import re
+import sys
+
+from functional2.testlib.fixtures.file_helper import with_files, CopyFile
+from functional2.testlib.fixtures.nix import Nix
+from functional2.testlib.utils import get_global_asset_pack
+
+_files = {
+    "output-cycles.nix": CopyFile("assets/test_build/output-cycles.nix"),
+    **get_global_asset_pack("dependencies"),
+}
+
+
+def _assert_cycle_tree(output: str, regexes: list[str]):
+    lines = output.splitlines()
+    start = next((k for k, v in enumerate(lines) if "Shown below are the files inside" in v), None)
+
+    assert start is not None
+
+    for line, regex in enumerate(regexes, start=start + 1):
+        assert re.search(regex, lines[line])
+
+
+def _assert_cycle_message(err: str):
+    assert (
+        len(
+            re.findall(
+                r"cycle detected in build of '.*' in the references of output 'bar' from output 'foo'",
+                err,
+            )
+        )
+        == 1
+    )
+
+
+@with_files(_files)
+def test_cycle(nix: Nix):
+    res = nix.nix_build(["output-cycles.nix", "-A", "cycle"]).run().expect(1)
+    err = res.stderr_plain
+    _assert_cycle_message(err)
+
+    if sys.platform == "linux":
+        _assert_cycle_tree(
+            err,
+            [
+                r"/store/.*-cycle-bar",
+                r"└───lib/libfoo: ….*cycle-baz.*",
+                r"    →.*/store/.*-cycle-baz",
+                r"    └───share/lalala:.*-cycle-foo.*",
+            ],
+        )
+
+
+@with_files(_files)
+def test_cycle_in_dependency(nix: Nix):
+    res = nix.nix_build(["output-cycles.nix", "-A", "as_dependency"]).run().expect(1)
+    err = res.stderr_plain
+    _assert_cycle_message(err)
+
+    assert "error: 1 dependencies of derivation" in err
+
+
+@with_files(_files)
+def test_cycle_with_deps(nix: Nix):
+    res = nix.nix_build(["output-cycles.nix", "-A", "cycle-with-deps"]).run().expect(1)
+    err = res.stderr_plain
+    _assert_cycle_message(err)
+
+    if sys.platform == "linux":
+        _assert_cycle_tree(
+            err,
+            [
+                r"/store/.*-cycle-with-deps-bar",
+                r"└───txt: ….*cycle-with-deps-foo.*",
+                r"    →.*/store/.*-cycle-with-deps-foo",
+                r"    └───txt:.*-cycle-with-deps-bar.*",
+            ],
+        )

tests/functional2/commands/test_custom_sub_commands.py

@@ -32,7 +32,7 @@ def custom_sub_command(request: pytest.FixtureRequest, custom_sub_command_path:
          import os, sys
          # Start with args[0] set to the actual nix command used for testing
          # as we are not making Lix variants of those.
-         os.execvp("nix-{lix_cmd}", [ "nix-{lix_cmd}" ] + sys.argv[1:])
+         os.execvp("nix", [ "nix-{lix_cmd}" ] + sys.argv[1:])
          """)
     )
     executable.chmod(stat.S_IXUSR | stat.S_IRUSR | stat.S_IWUSR)

tests/functional2/eval/test_eval_cli.py

@@ -117,7 +117,7 @@ def test_valid_restricted_toFile(nix: Nix):  # noqa: N802 # builtin name
 
 @with_files({"cycle.nix": Symlink("cycle.nix")})
 # timeout given in seconds
-@pytest.mark.timeout(1)
+@pytest.mark.timeout(30)
 def test_invalid_no_hang_symlink_cycle(nix: Nix):
     """Check that symlink cycles don't cause a hang."""
     res = nix.nix(["eval", "--file", "cycle.nix"]).run().expect(1)

tests/functional2/meson.build

@@ -14,7 +14,7 @@ xdist_opts = [
 functional2_env = environment()
 # set the bin dir for us to know where to find the lix binaries for our declarative path
 # without leaking all of PATH or env
-functional2_env.set('NIX_BIN_DIR', bindir)
+functional2_env.set('NIX_BIN_DIR', fs.parent(nix.full_path()))
 if build_test_shell != ''
   functional2_env.set('BUILD_TEST_SHELL', build_test_shell)
 endif
@@ -32,6 +32,7 @@ test(
     meson.current_source_dir()
   ],
   env : functional2_env,
+  depends : nix,
   # FIXME: Although we can trivially use TAP here with pytest-tap, due to a meson bug, it is unusable.
   # (failure output does not get displayed to the console. at all. someone should go fix it):
   # https://github.com/mesonbuild/meson/issues/11185

tests/functional2/testlib/fixtures/command.py

@@ -83,6 +83,7 @@ class CommandResult:
 class Command:
     argv: list[str]
     _env: ManagedEnv
+    exe: Path | None = None
     stdin: bytes | None = None
     cwd: Path = dataclasses.field(default=None)
     _logger: logging.Logger = dataclasses.field(default=logger, init=False)
@@ -107,6 +108,7 @@ class Command:
         self._logger.debug("Running Command with args: %s", self.argv)
         proc = subprocess.Popen(
             self.argv,
+            executable=self.exe,
             stdout=subprocess.PIPE,
             stderr=subprocess.PIPE,
             stdin=subprocess.PIPE if self.stdin else subprocess.DEVNULL,

tests/functional2/testlib/fixtures/nix.py

@@ -5,6 +5,7 @@ from pathlib import Path
 from textwrap import dedent
 from typing import Any, Literal
 from collections.abc import Callable, Generator
+import shutil
 
 import pytest
 
@@ -81,7 +82,7 @@ class NixSettings:
         (env.dirs.nix_conf_dir / "nix.conf").write_text(cfg)
         env.set_env("NIX_CONFIG", cfg)
         if self.nix_store_dir:
-            env.dirs.nix_store_dir = str(self.nix_store_dir)
+            env.dirs.nix_store_dir = self.nix_store_dir
 
 
 @dataclasses.dataclass
@@ -89,6 +90,18 @@ class Nix:
     env: ManagedEnv
     _settings: NixSettings | None = dataclasses.field(init=False, default=None)
 
+    @property
+    def _nix_executable(self) -> Path:
+        if nix_bin_dir := self.env.dirs.nix_bin_dir:
+            return Path(nix_bin_dir) / "nix"
+
+        if from_path := shutil.which("nix"):
+            return Path(from_path)
+
+        raise ValueError(
+            "Couldn't find a Nix command to execute! Set NIX_BIN_DIR or fix your environment"
+        )
+
     @property
     def settings(self) -> NixSettings:
         """
@@ -124,7 +137,7 @@ class Nix:
                 settings.nix_store_dir = self.env.dirs.nix_store_dir
 
         settings.to_env_overlay(self.env)
-        return Command(argv=argv, _env=self.env)
+        return Command(argv=argv, exe=self._nix_executable, _env=self.env)
 
     def nix(
         self,
@@ -160,6 +173,45 @@ class Nix:
         self._settings = orig
         return cmd.run()
 
+    @property
+    def store_dir(self) -> Path:
+        """
+        The actual NIX_STORE_DIR this Nix command uses.
+        """
+        assert self.env.dirs.nix_store_dir is not None, "bug in ManagedEnv"
+        return self.env.dirs.nix_store_dir
+
+    def physical_store_path_for(self, path: str | Path) -> Path:
+        """
+        Takes a /nix/store/… path and rewrites it to be relative to this Nix's NIX_STORE_DIR.
+
+        Nix accepts and returns store paths as `/nix/store` even when that's not where `NIX_STORE_DIR`
+        physically is on the filesystem. Since we move the conceptual root for Nix to `test_root`,
+        these "virtual" paths differ from the physical ones. So this function will convert `/nix/store`
+        "virtual" paths to their real, physical location on the system.
+
+        Basically, if you're passing it to `nix build` or `nix-store` or whatever, you want the
+        `/nix/store` version. If you're passing it to a Python API (like pathlib.Path.exists()) or a
+        command that operates on arbitrary files instead of store paths, you want the output of this
+        function.
+
+
+        :param path: a string or Path to convert
+        :return: a Path object holding the rewritten, physical system path to the store entry
+        """
+        return Path(str(path).replace("/nix/store", self.store_dir.as_posix()))
+
+    def hash_path(self, store_path: str | Path, *args: str) -> str:
+        """
+        Shortcut to use `nix hash path {store_path}`, converting "virtual" store paths returned
+        from Nix to their physical system paths including the test root.
+
+        :param store_path: store path of the derivation or entry to hash
+        """
+        actual_path = self.physical_store_path_for(store_path).as_posix()
+        res = self.nix(["hash", "path", actual_path, *args], flake=True).run().ok()
+        return res.stdout_plain
+
 
 @pytest.fixture
 def nix(tmp_path: Path, env: ManagedEnv) -> Generator[Nix, Any, None]:

tests/functional2/testlib/fixtures/test_nix.py

@@ -74,4 +74,4 @@ def test_nix_settings_to_env_overlay_store_dir(tmp_path: Path):
 
     settings.to_env_overlay(env)
     assert "store = " not in env._env["NIX_CONFIG"]
-    assert env.dirs.nix_store_dir == "/some/path"
+    assert str(env.dirs.nix_store_dir) == "/some/path"

tests/unit/libstore/filetransfer.cc

@@ -256,7 +256,7 @@ TEST(FileTransfer, NOT_ON_DARWIN(defersFailures))
         // initial wait for header data will also wait for the the response to
         // complete (the source is only woken when curl returns data, and curl
         // might only do so once its internal buffer has already been filled.)
-        return std::string(1024 * 1024, ' ');
+        return std::string(static_cast<size_t>(1024 * 1024), ' ');
     });
     AsyncIoRoot aio;
     auto ft = makeFileTransfer(0);