fix(local-store): invalidate phantom referrers at garbage collection time

Sometimes, a path can disappear from the `ValidPaths` table (I have 23
such cases on my 1.4TB Nix store).

When this occurs and you try to run a garbage collection,
`queryReferrers` will report no referrer because it's performing a
*RIGHT JOIN* between `Refs` and `ValidPaths`, finally, when you issue
the deletion SQL statement, this will throw an uncaught exception from
SQLite side regarding a foreign key violation because `reference` in
`Refs` is a foreign key to `ValidPaths` (which we are trying to delete).

Why can this happen?

Two reasons:

* `PRAGMA foreign_keys=off;` will disable deletion on cascade.
* Trigger recursion *limits*, a deletion on cascade is a *trigger*, when
  a delete is issued and it triggers a bunch of deletion on cascade,
  there's a documented limit by SQLite: https://www.sqlite.org/limits.html#max_trigger_depth

  > Recursion limit on foreign key actions. The SQLITE_MAX_TRIGGER_DEPTH
  > and SQLITE_LIMIT_TRIGGER_DEPTH settings determine the maximum
  > allowable depth of trigger program recursion. For the purposes of
  > these limits, foreign key actions are considered trigger programs. The
  > PRAGMA recursive_triggers setting does not affect the operation of
  > foreign key actions. It is not possible to disable recursive foreign
  > key actions.

As I do not see easy ways to solve the root cause, garbage collection
should be self-healing in that regards, so I propose to invalidate
phantom referrers as we go.

As part of a work improving the consistency of the SQLite database, it
would make sense to count how many times this happen and try to find
ways to reproduce this issue.

Change-Id: I055a8a1d8c0e44d4388a411abe8e5a5e385f7b55
Signed-off-by: Raito Bezarius <raito@lix.systems>

.editorconfig

@@ -29,7 +29,3 @@ trim_trailing_whitespace = false
 indent_style = space
 indent_size = 2
 max_line_length = 0
-
-[meson.build]
-indent_style = space
-indent_size = 2

.github/ISSUE_TEMPLATE/missing_documentation.md

@@ -2,7 +2,7 @@
 name: Missing or incorrect documentation
 about: Help us improve the reference manual
 title: ''
-labels: docs
+labels: documentation
 assignees: ''
 
 ---
@@ -19,10 +19,10 @@ assignees: ''
 
 <!-- make sure this issue is not redundant or obsolete -->
 
-- [ ] checked [latest Lix manual] or its [source code]
+- [ ] checked [latest Lix manual] \([source]\)
 - [ ] checked [documentation issues] and [recent documentation changes] for possible duplicates
 
-[latest Lix manual]: https://docs.lix.systems/manual/lix/nightly
+[latest Nix manual]: https://docs.lix.systems/manual/lix/nightly
-[source code]: https://git.lix.systems/lix-project/lix/src/main/doc/manual/src
+[source]: https://git.lix.systems/lix-project/lix/src/main/doc/manual/src
 [documentation issues]: https://git.lix.systems/lix-project/lix/issues?labels=151&state=all
 [recent documentation changes]: https://gerrit.lix.systems/q/p:lix+path:%22%5Edoc/manual/.*%22

doc/internal-api/doxygen.cfg.in

@@ -33,7 +33,32 @@ GENERATE_LATEX         = NO
 # spaces. See also FILE_PATTERNS and EXTENSION_MAPPING
 # Note: If this tag is empty the current directory is searched.
 
-INPUT                  = @INPUT_PATHS@
+# FIXME Make this list more maintainable somehow. We could maybe generate this
+# in the Makefile, but we would need to change how `.in` files are preprocessed
+# so they can expand variables despite configure variables.
+
+INPUT                  = \
+  src/libcmd \
+  src/libexpr \
+  src/libexpr/flake \
+  tests/unit/libexpr \
+  tests/unit/libexpr/value \
+  tests/unit/libexpr/test \
+  tests/unit/libexpr/test/value \
+  src/libexpr/value \
+  src/libfetchers \
+  src/libmain \
+  src/libstore \
+  src/libstore/build \
+  src/libstore/builtins \
+  tests/unit/libstore \
+  tests/unit/libstore/test \
+  src/libutil \
+  tests/unit/libutil \
+  tests/unit/libutil/test \
+  src/nix \
+  src/nix-env \
+  src/nix-store
 
 # If the MACRO_EXPANSION tag is set to YES, doxygen will expand all macro names
 # in the source code. If set to NO, only conditional compilation will be
@@ -72,15 +97,3 @@ EXPAND_AS_DEFINED      = \
   DECLARE_WORKER_SERIALISER \
   DECLARE_SERVE_SERIALISER \
   LENGTH_PREFIXED_PROTO_HELPER
-
-# The STRIP_FROM_PATH tag can be used to strip a user-defined part of the path.
-# Stripping is only done if one of the specified strings matches the left-hand
-# part of the path. The tag can be used to show relative paths in the file list.
-# If left blank the directory from which doxygen is run is used as the path to
-# strip.
-#
-# Note that you can specify absolute paths here, but also relative paths, which
-# will be relative from the directory where doxygen is started.
-# This tag requires that the tag FULL_PATH_NAMES is set to YES.
-
-STRIP_FROM_PATH        = "@PROJECT_SOURCE_ROOT@"

doc/internal-api/meson.build

@@ -1,35 +1,3 @@
-internal_api_sources = [
-  'src/libcmd',
-  'src/libexpr',
-  'src/libexpr/flake',
-  'tests/unit/libexpr',
-  'tests/unit/libexpr/value',
-  'tests/unit/libexpr/test',
-  'tests/unit/libexpr/test/value',
-  'src/libexpr/value',
-  'src/libfetchers',
-  'src/libmain',
-  'src/libstore',
-  'src/libstore/build',
-  'src/libstore/builtins',
-  'tests/unit/libstore',
-  'tests/unit/libstore/test',
-  'src/libutil',
-  'tests/unit/libutil',
-  'tests/unit/libutil/test',
-  'src/nix',
-  'src/nix-env',
-  'src/nix-store',
-]
-
-# We feed Doxygen absolute paths so it can be invoked from any working directory.
-internal_api_sources_absolute = []
-foreach src : internal_api_sources
-  internal_api_sources_absolute += '"' + (meson.project_source_root() / src) + '"'
-endforeach
-
-internal_api_sources_oneline = ' \\\n  '.join(internal_api_sources_absolute)
-
 doxygen_cfg = configure_file(
   input : 'doxygen.cfg.in',
   output : 'doxygen.cfg',
@@ -37,16 +5,22 @@ doxygen_cfg = configure_file(
     'PACKAGE_VERSION': meson.project_version(),
     'RAPIDCHECK_HEADERS': rapidcheck_meson.get_variable('includedir'),
     'docdir' : meson.current_build_dir(),
-    'INPUT_PATHS' : internal_api_sources_oneline,
-    'PROJECT_SOURCE_ROOT' : meson.project_source_root(),
   },
 )
 
 internal_api_docs = custom_target(
   'internal-api-docs',
   command : [
-    doxygen.full_path(),
+    bash,
-    '@INPUT0@',
+    # Meson can you please just give us a `workdir` argument to custom targets...
+    '-c',
+    # We have to prefix the doxygen_cfg path with the project build root
+    # because of the cd in front.
+    'cd @0@ && @1@ @2@/@INPUT0@'.format(
+      meson.project_source_root(),
+      doxygen.full_path(),
+      meson.project_build_root(),
+    ),
   ],
   input : [
     doxygen_cfg,

doc/manual/change-authors.yml

@@ -147,6 +147,3 @@ winter:
 
 yshui:
   github: yshui
-
-zimbatm:
-  github: zimbatm

doc/manual/meson.build

@@ -126,19 +126,20 @@ manual = custom_target(
     'manual',
     'markdown',
   ],
-  install : true,
-  install_dir : [
-    datadir / 'doc/nix',
-    false,
-  ],
   depfile : 'manual.d',
   env : {
     'RUST_LOG': 'info',
     'MDBOOK_SUBSTITUTE_SEARCH': meson.current_build_dir() / 'src',
   },
 )
+manual_html = manual[0]
 manual_md = manual[1]
 
+install_subdir(
+  manual_html.full_path(),
+  install_dir : datadir / 'doc/nix',
+)
+
 nix_nested_manpages = [
   [ 'nix-env',
     [

doc/manual/rl-next/fetchGit-regression.md

@@ -1,23 +0,0 @@
----
-synopsis: restore backwards-compatibility of `builtins.fetchGit` with Nix 2.3
-issues: [5291, 5128]
-credits: [ma27]
-category: Fixes
----
-
-Compatibility with `builtins.fetchGit` from Nix 2.3 has been restored as follows:
-
-* Until now, each `ref` was prefixed with `refs/heads` unless it starts with `refs/` itself.
-
-  Now, this is not done if the `ref` looks like a commit hash.
-
-* Specifying `builtins.fetchGit { ref = "a-tag"; /* … */ }` was broken because `refs/heads` was appended.
-
-  Now, the fetcher doesn't turn a ref into `refs/heads/ref`, but into `refs/*/ref`. That way,
-  the value in `ref` can be either a tag or a branch.
-
-* The ref resolution happens the same way as in git:
-
-  * If `refs/ref` exists, it's used.
-  * If a tag `refs/tags/ref` exists, it's used.
-  * If a branch `refs/heads/ref` exists, it's used.

doc/manual/rl-next/nix-fmt-default-argument.md

@@ -1,38 +0,0 @@
----
-synopsis: Removing the `.` default argument passed to the `nix fmt` formatter
-issues: []
-prs: [11438]
-cls: [1902]
-category: Breaking Changes
-credits: zimbatm
----
-
-The underlying formatter no longer receives the ". " default argument when `nix fmt` is called with no arguments.
-
-This change was necessary as the formatter wasn't able to distinguish between
-a user wanting to format the current folder with `nix fmt .` or the generic
-`nix fmt`.
-
-The default behaviour is now the responsibility of the formatter itself, and
-allows tools such as treefmt to format the whole tree instead of only the
-current directory and below.
-
-This may cause issues with some formatters: nixfmt, nixpkgs-fmt and alejandra currently format stdin when no arguments are passed.
-
-Here is a small wrapper example that will restore the previous behaviour for such a formatter:
-
-```nix
-{
-  outputs = { self, nixpkgs, systems }:
-    let
-      eachSystem = nixpkgs.lib.genAttrs (import systems) (system: nixpkgs.legacyPackages.${system});
-    in
-    {
-      formatter = eachSystem (pkgs:
-        pkgs.writeShellScriptBin "formatter" ''
-          if [[ $# = 0 ]]; set -- .; fi
-          exec "${pkgs.nixfmt-rfc-style}/bin/nixfmt "$@"
-        '');
-    };
-}
-```

doc/manual/rl-next/readline-support-removed.md

@@ -1,17 +0,0 @@
----
-synopsis: readline support removed
-cls: [1885]
-category: Packaging
-credits: [9999years]
----
-
-Support for building Lix with [`readline`][readline] instead of
-[`editline`][editline] has been removed. `readline` support hasn't worked for a
-long time (attempting to use it would lead to build errors) and would make Lix
-subject to the GPL if it did work. In the future, we're hoping to replace
-`editline` with [`rustyline`][rustyline] for improved ergonomics in the `nix
-repl`.
-
-[readline]: https://en.wikipedia.org/wiki/GNU_Readline
-[editline]: https://github.com/troglobit/editline
-[rustyline]: https://github.com/kkawakam/rustyline

doc/manual/rl-next/stack-traces.md

@@ -1,26 +0,0 @@
----
-synopsis: "Some Lix crashes now produce reporting instructions and a stack trace, then abort"
-cls: [1854]
-category: Improvements
-credits: jade
----
-
-Lix, being a C++ program, can crash in a few kinds of ways.
-It can obviously do a memory access violation, which will generate a core dump and thus be relatively debuggable.
-But, worse, it could throw an unhandled exception, and, in the past, we would just show the message but not where it comes from, in spite of this always being a bug, since we expect all such errors to be translated to a Lix specific error.
-Now the latter kind of bug should print reporting instructions, a rudimentary stack trace and (depending on system configuration) generate a core dump.
-
-Sample output:
-
-```
-Lix crashed. This is a bug. We would appreciate if you report it along with what caused it at https://git.lix.systems/lix-project/lix/issues with the following information included:
-
-Exception: std::runtime_error: test exception
-Stack trace:
- 0# nix::printStackTrace() in /home/jade/lix/lix3/build/src/nix/../libutil/liblixutil.so
- 1# 0x000073C9862331F2 in /home/jade/lix/lix3/build/src/nix/../libmain/liblixmain.so
- 2# 0x000073C985F2E21A in /nix/store/p44qan69linp3ii0xrviypsw2j4qdcp2-gcc-13.2.0-lib/lib/libstdc++.so.6
- 3# 0x000073C985F2E285 in /nix/store/p44qan69linp3ii0xrviypsw2j4qdcp2-gcc-13.2.0-lib/lib/libstdc++.so.6
- 4# nix::handleExceptions(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<void ()>) in /home/jade/lix/lix3/build/src/nix/../libmain/liblixmain.so
- ...
-```

doc/manual/rl-next/verify-tls.md

@@ -1,10 +0,0 @@
----
-synopsis: "`<nix/fetchurl.nix>` now uses TLS verification"
-category: Fixes
-prs: [11585]
-credits: edolstra
----
-
-Previously `<nix/fetchurl.nix>` did not do TLS verification. This was because the Nix sandbox in the past did not have access to TLS certificates, and Nix checks the hash of the fetched file anyway. However, this can expose authentication data from `netrc` and URLs to man-in-the-middle attackers. In addition, Nix now in some cases (such as when using impure derivations) does *not* check the hash. Therefore we have now enabled TLS verification. This means that downloads by `<nix/fetchurl.nix>` will now fail if you're fetching from a HTTPS server that does not have a valid certificate.
-
-`<nix/fetchurl.nix>` is also known as the builtin derivation builder `builtin:fetchurl`. It's not to be confused with the evaluation-time function `builtins.fetchurl`, which was not affected by this issue.

flake.nix

@@ -217,7 +217,7 @@
 
       # A Nixpkgs overlay that overrides the 'nix' and
       # 'nix.perl-bindings' packages.
-      overlays.default = overlayFor (p: p.clangStdenv);
+      overlays.default = overlayFor (p: p.stdenv);
 
       hydraJobs = {
         # Binary package for various platforms.

meson.build

@@ -47,7 +47,6 @@
 # in the build directory.
 
 project('lix', 'cpp', 'rust',
-  meson_version : '>=1.4.0',
   version : run_command('bash', '-c', 'echo -n $(jq -r .version < ./version.json)$VERSION_SUFFIX', check : true).stdout().strip(),
   default_options : [
     'cpp_std=c++2a',
@@ -493,6 +492,12 @@ add_project_arguments(
   '-Wdeprecated-copy',
   '-Wignored-qualifiers',
   '-Werror=suggest-override',
+  # Enable assertions in libstdc++ by default. Harmless on libc++. Benchmarked
+  # at ~1% overhead in `nix search`.
+  #
+  # FIXME: remove when we get meson 1.4.0 which will default this to on for us:
+  # https://mesonbuild.com/Release-notes-for-1-4-0.html#ndebug-setting-now-controls-c-stdlib-assertions
+  '-D_GLIBCXX_ASSERTIONS=1',
   language : 'cpp',
 )
 
@@ -588,10 +593,10 @@ run_command(
 )
 
 if is_darwin
-  fs.copyfile(
+  configure_file(
-    'misc/launchd/org.nixos.nix-daemon.plist.in',
+    input : 'misc/launchd/org.nixos.nix-daemon.plist.in',
-    'org.nixos.nix-daemon.plist',
+    output : 'org.nixos.nix-daemon.plist',
-    install : true,
+    copy : true,
     install_dir : prefix / 'Library/LaunchDaemons',
   )
 endif

misc/bash/meson.build

@@ -1,7 +1,8 @@
-fs.copyfile(
+configure_file(
-  'completion.sh',
+  input : 'completion.sh',
-  'nix',
+  output : 'nix',
   install : true,
   install_dir : datadir / 'bash-completion/completions',
   install_mode : 'rw-r--r--',
+  copy : true,
 )

misc/fish/meson.build

@@ -1,7 +1,8 @@
-fs.copyfile(
+configure_file(
-  'completion.fish',
+  input : 'completion.fish',
-  'nix.fish',
+  output : 'nix.fish',
   install : true,
   install_dir : datadir / 'fish/vendor_completions.d',
   install_mode : 'rw-r--r--',
+  copy : true,
 )

misc/meson.build

@@ -5,4 +5,8 @@ subdir('zsh')
 subdir('systemd')
 subdir('flake-registry')
 
-runinpty = fs.copyfile('runinpty.py')
+runinpty = configure_file(
+  copy : true,
+  input : meson.current_source_dir() / 'runinpty.py',
+  output : 'runinpty.py',
+)

misc/zsh/meson.build

@@ -1,9 +1,10 @@
 foreach script : [ [ 'completion.zsh', '_nix' ], [ 'run-help-nix' ] ]
-  fs.copyfile(
+  configure_file(
-    script[0],
+    input : script[0],
-    script.get(1, script[0]),
+    output : script.get(1, script[0]),
     install : true,
     install_dir : datadir / 'zsh/site-functions',
     install_mode : 'rw-r--r--',
+    copy : true,
   )
 endforeach

package.nix

@@ -22,6 +22,7 @@
   doxygen,
   editline-lix ? __forDefaults.editline-lix,
   editline,
+  fetchpatch,
   git,
   gtest,
   jq,
@@ -99,7 +100,7 @@
         (lib.enableFeature (ncurses != null) "termcap")
       ];
 
-      buildInputs = (prev.buildInputs or [ ]) ++ [ ncurses ];
+      nativeBuildInputs = (prev.nativeBuildInputs or [ ]) ++ [ ncurses ];
     });
 
     build-release-notes = callPackage ./maintainers/build-release-notes.nix { };
@@ -110,7 +111,7 @@
 }:
 
 # gcc miscompiles coroutines at least until 13.2, possibly longer
-assert stdenv.cc.isClang || lintInsteadOfBuild || internalApiDocs;
+assert stdenv.cc.isClang || lintInsteadOfBuild;
 
 let
   inherit (__forDefaults) canRunInstalled;

scripts/meson.build

@@ -8,7 +8,12 @@ configure_file(
   }
 )
 
-fs.copyfile('nix-profile.sh.in')
+# https://github.com/mesonbuild/meson/issues/860
+configure_file(
+  input : 'nix-profile.sh.in',
+  output : 'nix-profile.sh.in',
+  copy : true,
+)
 
 foreach rc : [ '.sh', '.fish', '-daemon.sh', '-daemon.fish' ]
   configure_file(

src/libcmd/common-eval-args.cc

@@ -9,24 +9,8 @@
 #include "store-api.hh"
 #include "command.hh"
 
-#include <regex>
-
 namespace nix {
 
-static std::regex const identifierRegex("^[A-Za-z_][A-Za-z0-9_'-]*$");
-static void warnInvalidNixIdentifier(const std::string & name)
-{
-    std::smatch match;
-    if (!std::regex_match(name, match, identifierRegex)) {
-        warn("This Nix invocation specifies a value for argument '%s' which isn't a valid \
-Nix identifier. The project is considering to drop support for this \
-or to require quotes around args that aren't valid Nix identifiers. \
-If you depend on this behvior, please reach out in \
-https://git.lix.systems/lix-project/lix/issues/496 so we can discuss \
-your use-case.", name);
-    }
-}
-
 MixEvalArgs::MixEvalArgs()
 {
     addFlag({
@@ -34,10 +18,7 @@ MixEvalArgs::MixEvalArgs()
         .description = "Pass the value *expr* as the argument *name* to Nix functions.",
         .category = category,
         .labels = {"name", "expr"},
-        .handler = {[&](std::string name, std::string expr) {
+        .handler = {[&](std::string name, std::string expr) { autoArgs[name] = 'E' + expr; }}
-            warnInvalidNixIdentifier(name);
-            autoArgs[name] = 'E' + expr;
-        }}
     });
 
     addFlag({
@@ -45,10 +26,7 @@ MixEvalArgs::MixEvalArgs()
         .description = "Pass the string *string* as the argument *name* to Nix functions.",
         .category = category,
         .labels = {"name", "string"},
-        .handler = {[&](std::string name, std::string s) {
+        .handler = {[&](std::string name, std::string s) { autoArgs[name] = 'S' + s; }},
-            warnInvalidNixIdentifier(name);
-            autoArgs[name] = 'S' + s;
-        }},
     });
 
     addFlag({

src/libcmd/repl-interacter.cc

@@ -8,6 +8,10 @@
 #include <string_view>
 #include <cerrno>
 
+#ifdef READLINE
+#include <readline/history.h>
+#include <readline/readline.h>
+#else
 // editline < 1.15.2 don't wrap their API for C++ usage
 // (added in https://github.com/troglobit/editline/commit/91398ceb3427b730995357e9d120539fb9bb7461).
 // This results in linker errors due to to name-mangling of editline C symbols.
@@ -16,6 +20,7 @@
 extern "C" {
 #include <editline.h>
 }
+#endif
 
 #include "finally.hh"
 #include "repl-interacter.hh"
@@ -110,13 +115,17 @@ ReadlineLikeInteracter::Guard ReadlineLikeInteracter::init(detail::ReplCompleter
     } catch (SysError & e) {
         logWarning(e.info());
     }
+#ifndef READLINE
     el_hist_size = 1000;
+#endif
     read_history(historyFile.c_str());
     auto oldRepl = curRepl;
     curRepl = repl;
     Guard restoreRepl([oldRepl] { curRepl = oldRepl; });
+#ifndef READLINE
     rl_set_complete_func(completionCallback);
     rl_set_list_possib_func(listPossibleCallback);
+#endif
     return restoreRepl;
 }
 

src/libexpr/eval-cache.cc

@@ -79,7 +79,7 @@ struct AttrDb
                 state->txn->commit();
             state->txn.reset();
         } catch (...) {
-            ignoreExceptionInDestructor();
+            ignoreException();
         }
     }
 
@@ -90,7 +90,7 @@ struct AttrDb
         try {
             return fun();
         } catch (SQLiteError &) {
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
             failed = true;
             return 0;
         }
@@ -329,7 +329,7 @@ static std::shared_ptr<AttrDb> makeAttrDb(
     try {
         return std::make_shared<AttrDb>(cfg, fingerprint, symbols);
     } catch (SQLiteError &) {
-        ignoreExceptionExceptInterrupt();
+        ignoreException();
         return nullptr;
     }
 }

src/libexpr/primops/fetchTree.cc

@@ -394,8 +394,7 @@ static RegisterPrimOp primop_fetchGit({
         [Git reference]: https://git-scm.com/book/en/v2/Git-Internals-Git-References
 
         By default, the `ref` value is prefixed with `refs/heads/`.
-        As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/` or
+        As of 2.3.0, Nix will not prefix `refs/heads/` if `ref` starts with `refs/`.
-        if `ref` looks like a commit hash for backwards compatibility with CppNix 2.3.
 
       - `submodules` (default: `false`)
 

src/libfetchers/git.cc

@@ -1,4 +1,3 @@
-#include "error.hh"
 #include "fetchers.hh"
 #include "cache.hh"
 #include "globals.hh"
@@ -258,28 +257,6 @@ std::pair<StorePath, Input> fetchFromWorkdir(ref<Store> store, Input & input, co
 }
 }  // end namespace
 
-static std::optional<Path> resolveRefToCachePath(
-    Input & input,
-    const Path & cacheDir,
-    std::vector<Path> & gitRefFileCandidates,
-    std::function<bool(const Path&)> condition)
-{
-    if (input.getRef()->starts_with("refs/")) {
-        Path fullpath = cacheDir + "/" + *input.getRef();
-        if (condition(fullpath)) {
-            return fullpath;
-        }
-    }
-
-    for (auto & candidate : gitRefFileCandidates) {
-        if (condition(candidate)) {
-            return candidate;
-        }
-    }
-
-    return std::nullopt;
-}
-
 struct GitInputScheme : InputScheme
 {
     std::optional<Input> inputFromURL(const ParsedURL & url, bool requireTree) const override
@@ -562,13 +539,10 @@ struct GitInputScheme : InputScheme
                 runProgram("git", true, { "-c", "init.defaultBranch=" + gitInitialBranch, "init", "--bare", repoDir });
             }
 
-            std::vector<Path> gitRefFileCandidates;
+            Path localRefFile =
-            for (auto & infix : {"", "tags/", "heads/"}) {
+                input.getRef()->compare(0, 5, "refs/") == 0
-                Path p = cacheDir + "/refs/" + infix + *input.getRef();
+                ? cacheDir + "/" + *input.getRef()
-                gitRefFileCandidates.push_back(p);
+                : cacheDir + "/refs/heads/" + *input.getRef();
-            }
-
-            Path localRefFile;
 
             bool doFetch;
             time_t now = time(0);
@@ -590,70 +564,29 @@ struct GitInputScheme : InputScheme
                 if (allRefs) {
                     doFetch = true;
                 } else {
-                    std::function<bool(const Path&)> condition;
+                    /* If the local ref is older than ‘tarball-ttl’ seconds, do a
-                    condition = [&now](const Path & path) {
+                       git fetch to update the local ref to the remote ref. */
-                        /* If the local ref is older than ‘tarball-ttl’ seconds, do a
+                    struct stat st;
-                           git fetch to update the local ref to the remote ref. */
+                    doFetch = stat(localRefFile.c_str(), &st) != 0 ||
-                        struct stat st;
+                        !isCacheFileWithinTtl(now, st);
-                        return stat(path.c_str(), &st) == 0 &&
-                            isCacheFileWithinTtl(now, st);
-                    };
-                    if (auto result = resolveRefToCachePath(
-                        input,
-                        cacheDir,
-                        gitRefFileCandidates,
-                        condition
-                    )) {
-                        localRefFile = *result;
-                        doFetch = false;
-                    } else {
-                        doFetch = true;
-                    }
                 }
             }
 
-            // When having to fetch, we don't know `localRefFile` yet.
-            // Because git needs to figure out what we're fetching
-            // (i.e. is it a rev? a branch? a tag?)
             if (doFetch) {
                 Activity act(*logger, lvlTalkative, actUnknown, fmt("fetching Git repository '%s'", actualUrl));
 
-                auto ref = input.getRef();
+                // FIXME: git stderr messes up our progress indicator, so
-                std::string fetchRef;
+                // we're using --quiet for now. Should process its stderr.
-                if (allRefs) {
-                    fetchRef = "refs/*";
-                } else if (
-                    ref->starts_with("refs/")
-                    || *ref == "HEAD"
-                    || std::regex_match(*ref, revRegex))
-                {
-                    fetchRef = *ref;
-                } else {
-                    fetchRef = "refs/*/" + *ref;
-                }
-
                 try {
-                    Finally finally([&]() {
+                    auto ref = input.getRef();
-                        if (auto p = resolveRefToCachePath(
+                    auto fetchRef = allRefs
-                            input,
+                        ? "refs/*"
-                            cacheDir,
+                        : ref->compare(0, 5, "refs/") == 0
-                            gitRefFileCandidates,
+                            ? *ref
-                            pathExists
+                            : ref == "HEAD"
-                        )) {
+                                ? *ref
-                            localRefFile = *p;
+                                : "refs/heads/" + *ref;
-                        }
+                    runProgram("git", true, { "-C", repoDir, "--git-dir", gitDir, "fetch", "--quiet", "--force", "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef) }, true);
-                    });
-
-                    // FIXME: git stderr messes up our progress indicator, so
-                    // we're using --quiet for now. Should process its stderr.
-                    runProgram("git", true, {
-                        "-C", repoDir,
-                        "--git-dir", gitDir,
-                        "fetch",
-                        "--quiet",
-                        "--force",
-                        "--", actualUrl, fmt("%s:%s", fetchRef, fetchRef)
-                    }, true);
                 } catch (Error & e) {
                     if (!pathExists(localRefFile)) throw;
                     warn("could not update local clone of Git repository '%s'; continuing with the most recent version", actualUrl);

src/libmain/crash-handler.cc

@@ -1,41 +0,0 @@
-#include "crash-handler.hh"
-#include "fmt.hh"
-
-#include <boost/core/demangle.hpp>
-#include <exception>
-
-namespace nix {
-
-namespace {
-void onTerminate()
-{
-    std::cerr << "Lix crashed. This is a bug. We would appreciate if you report it along with what caused it at https://git.lix.systems/lix-project/lix/issues with the following information included:\n\n";
-    try {
-        std::exception_ptr eptr = std::current_exception();
-        if (eptr) {
-            std::rethrow_exception(eptr);
-        } else {
-            std::cerr << "std::terminate() called without exception\n";
-        }
-    } catch (const std::exception & ex) {
-        std::cerr << "Exception: " << boost::core::demangle(typeid(ex).name()) << ": " << ex.what() << "\n";
-    } catch (...) {
-        std::cerr << "Unknown exception! Spooky.\n";
-    }
-
-    std::cerr << "Stack trace:\n";
-    nix::printStackTrace();
-
-    std::abort();
-}
-}
-
-void registerCrashHandler()
-{
-    // DO NOT use this for signals. Boost stacktrace is very much not
-    // async-signal-safe, and in a world with ASLR, addr2line is pointless.
-    //
-    // If you want signals, set up a minidump system and do it out-of-process.
-    std::set_terminate(onTerminate);
-}
-}

src/libmain/crash-handler.hh

@@ -1,21 +0,0 @@
-#pragma once
-/// @file Crash handler for Lix that prints back traces (hopefully in instances where it is not just going to crash the process itself).
-/*
- * Author's note: This will probably be partially/fully supplanted by a
- * minidump writer like the following once we get our act together on crashes a
- * little bit more:
- * https://github.com/rust-minidump/minidump-writer
- * https://github.com/EmbarkStudios/crash-handling
- * (out of process implementation *should* be able to be done on-demand)
- *
- * Such an out-of-process implementation could then both make minidumps and
- * print stack traces for arbitrarily messed-up process states such that we can
- * safely give out backtraces for SIGSEGV and other deadly signals.
- */
-
-namespace nix {
-
-/** Registers the Lix crash handler for std::terminate (currently; will support more crashes later). See also detectStackOverflow().  */
-void registerCrashHandler();
-
-}

src/libmain/meson.build

@@ -1,6 +1,5 @@
 libmain_sources = files(
   'common-args.cc',
-  'crash-handler.cc',
   'loggers.cc',
   'progress-bar.cc',
   'shared.cc',
@@ -9,7 +8,6 @@ libmain_sources = files(
 
 libmain_headers = files(
   'common-args.hh',
-  'crash-handler.hh',
   'loggers.hh',
   'progress-bar.hh',
   'shared.hh',

src/libmain/shared.cc

@@ -1,4 +1,3 @@
-#include "crash-handler.hh"
 #include "globals.hh"
 #include "shared.hh"
 #include "store-api.hh"
@@ -119,8 +118,6 @@ static void sigHandler(int signo) { }
 
 void initNix()
 {
-    registerCrashHandler();
-
     /* Turn on buffering for cerr. */
     static char buf[1024];
     std::cerr.rdbuf()->pubsetbuf(buf, sizeof(buf));
@@ -338,15 +335,12 @@ int handleExceptions(const std::string & programName, std::function<void()> fun)
     } catch (BaseError & e) {
         logError(e.info());
         return e.info().status;
-    } catch (const std::bad_alloc & e) {
+    } catch (std::bad_alloc & e) {
         printError(error + "out of memory");
         return 1;
-    } catch (const std::exception & e) {
+    } catch (std::exception & e) {
-        // Random exceptions bubbling into main are cause for bug reports, crash
+        printError(error + e.what());
-        std::terminate();
+        return 1;
-    } catch (...) {
-        // Explicitly do not tolerate non-std exceptions escaping.
-        std::terminate();
     }
 
     return 0;
@@ -395,7 +389,7 @@ RunPager::~RunPager()
             pid.wait();
         }
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libmain/shared.hh

@@ -111,7 +111,7 @@ struct PrintFreed
 
 
 /**
- * Install a SIGSEGV handler to detect stack overflows. See also registerCrashHandler().
+ * Install a SIGSEGV handler to detect stack overflows.
  */
 void detectStackOverflow();
 

src/libstore/build/derivation-goal.cc

@@ -11,13 +11,7 @@
 #include "drv-output-substitution-goal.hh"
 #include "strings.hh"
 
-#include <boost/outcome/try.hpp>
 #include <fstream>
-#include <kj/array.h>
-#include <kj/async-unix.h>
-#include <kj/async.h>
-#include <kj/debug.h>
-#include <kj/vector.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <sys/un.h>
@@ -71,6 +65,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath,
     , wantedOutputs(wantedOutputs)
     , buildMode(buildMode)
 {
+    state = &DerivationGoal::getDerivation;
     name = fmt(
         "building of '%s' from .drv file",
         DerivedPath::Built { makeConstantStorePathRef(drvPath), wantedOutputs }.to_string(worker.store));
@@ -90,6 +85,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation
 {
     this->drv = std::make_unique<Derivation>(drv);
 
+    state = &DerivationGoal::haveDerivation;
     name = fmt(
         "building of '%s' from in-memory derivation",
         DerivedPath::Built { makeConstantStorePathRef(drvPath), drv.outputNames() }.to_string(worker.store));
@@ -107,7 +103,17 @@ DerivationGoal::~DerivationGoal() noexcept(false)
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try { closeLogFile(); } catch (...) { ignoreExceptionInDestructor(); }
+    try { closeLogFile(); } catch (...) { ignoreException(); }
+}
+
+
+std::string DerivationGoal::key()
+{
+    /* Ensure that derivations get built in order of their name,
+       i.e. a derivation named "aardvark" always comes before
+       "baboon". And substitution goals always happen before
+       derivation goals (due to "b$"). */
+    return "b$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath);
 }
 
 
@@ -118,24 +124,20 @@ void DerivationGoal::killChild()
 }
 
 
-Goal::WorkResult DerivationGoal::timedOut(Error && ex)
+Goal::Finished DerivationGoal::timedOut(Error && ex)
 {
     killChild();
     return done(BuildResult::TimedOut, {}, std::move(ex));
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::workImpl() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::work(bool inBuildSlot) noexcept
 {
-    return useDerivation ? getDerivation() : haveDerivation();
+    return (this->*state)(inBuildSlot);
 }
 
-bool DerivationGoal::addWantedOutputs(const OutputsSpec & outputs)
+void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs)
 {
-    if (isDone) {
-        return false;
-    }
-
     auto newWanted = wantedOutputs.union_(outputs);
     switch (needRestart) {
     case NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed:
@@ -152,11 +154,10 @@ bool DerivationGoal::addWantedOutputs(const OutputsSpec & outputs)
         break;
     };
     wantedOutputs = newWanted;
-    return true;
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::getDerivation() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::getDerivation(bool inBuildSlot) noexcept
 try {
     trace("init");
 
@@ -164,17 +165,18 @@ try {
        exists.  If it doesn't, it may be created through a
        substitute. */
     if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) {
-        co_return co_await loadDerivation();
+        return loadDerivation(inBuildSlot);
     }
 
-    (co_await waitForGoals(worker.goalFactory().makePathSubstitutionGoal(drvPath))).value();
+
-    co_return co_await loadDerivation();
+    state = &DerivationGoal::loadDerivation;
+    return {WaitForGoals{{worker.goalFactory().makePathSubstitutionGoal(drvPath)}}};
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::loadDerivation() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::loadDerivation(bool inBuildSlot) noexcept
 try {
     trace("loading derivation");
 
@@ -205,13 +207,13 @@ try {
     }
     assert(drv);
 
-    return haveDerivation();
+    return haveDerivation(inBuildSlot);
 } catch (...) {
     return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::haveDerivation() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::haveDerivation(bool inBuildSlot) noexcept
 try {
     trace("have derivation");
 
@@ -239,7 +241,7 @@ try {
             });
         }
 
-        co_return co_await gaveUpOnSubstitution();
+        return gaveUpOnSubstitution(inBuildSlot);
     }
 
     for (auto & i : drv->outputsAndOptPaths(worker.store))
@@ -261,19 +263,19 @@ try {
 
     /* If they are all valid, then we're done. */
     if (allValid && buildMode == bmNormal) {
-        co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
+        return {done(BuildResult::AlreadyValid, std::move(validOutputs))};
     }
 
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
        them. */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
+    WaitForGoals result;
     if (settings.useSubstitutes) {
         if (parsedDrv->substitutesAllowed()) {
             for (auto & [outputName, status] : initialOutputs) {
                 if (!status.wanted) continue;
                 if (!status.known)
-                    dependencies.add(
+                    result.goals.insert(
                         worker.goalFactory().makeDrvOutputSubstitutionGoal(
                             DrvOutput{status.outputHash, outputName},
                             buildMode == bmRepair ? Repair : NoRepair
@@ -281,7 +283,7 @@ try {
                     );
                 else {
                     auto * cap = getDerivationCA(*drv);
-                    dependencies.add(worker.goalFactory().makePathSubstitutionGoal(
+                    result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(
                         status.known->path,
                         buildMode == bmRepair ? Repair : NoRepair,
                         cap ? std::optional { *cap } : std::nullopt));
@@ -292,15 +294,17 @@ try {
         }
     }
 
-    if (!dependencies.empty()) { /* to prevent hang (no wake-up event) */
+    if (result.goals.empty()) { /* to prevent hang (no wake-up event) */
-        (co_await waitForGoals(dependencies.releaseAsArray())).value();
+        return outputsSubstitutionTried(inBuildSlot);
+    } else {
+        state = &DerivationGoal::outputsSubstitutionTried;
+        return {std::move(result)};
     }
-    co_return co_await outputsSubstitutionTried();
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::outputsSubstitutionTried() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::outputsSubstitutionTried(bool inBuildSlot) noexcept
 try {
     trace("all outputs substituted (maybe)");
 
@@ -350,7 +354,7 @@ try {
 
     if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) {
         needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
-        return haveDerivation();
+        return haveDerivation(inBuildSlot);
     }
 
     auto [allValid, validOutputs] = checkPathValidity();
@@ -366,7 +370,7 @@ try {
             worker.store.printStorePath(drvPath));
 
     /* Nothing to wait for; tail call */
-    return gaveUpOnSubstitution();
+    return gaveUpOnSubstitution(inBuildSlot);
 } catch (...) {
     return {std::current_exception()};
 }
@@ -374,9 +378,9 @@ try {
 
 /* At least one of the output paths could not be
    produced using a substitute.  So we have to build instead. */
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::gaveUpOnSubstitution() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot) noexcept
 try {
-    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
+    WaitForGoals result;
 
     /* At this point we are building all outputs, so if more are wanted there
        is no need to restart. */
@@ -389,7 +393,7 @@ try {
 
         addWaiteeDerivedPath = [&](ref<SingleDerivedPath> inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
             if (!inputNode.value.empty())
-                dependencies.add(worker.goalFactory().makeGoal(
+                result.goals.insert(worker.goalFactory().makeGoal(
                     DerivedPath::Built {
                         .drvPath = inputDrv,
                         .outputs = inputNode.value,
@@ -434,15 +438,17 @@ try {
         if (!settings.useSubstitutes)
             throw Error("dependency '%s' of '%s' does not exist, and substitution is disabled",
                 worker.store.printStorePath(i), worker.store.printStorePath(drvPath));
-        dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i));
+        result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i));
     }
 
-    if (!dependencies.empty()) {/* to prevent hang (no wake-up event) */
+    if (result.goals.empty()) {/* to prevent hang (no wake-up event) */
-        (co_await waitForGoals(dependencies.releaseAsArray())).value();
+        return inputsRealised(inBuildSlot);
+    } else {
+        state = &DerivationGoal::inputsRealised;
+        return {result};
     }
-    co_return co_await inputsRealised();
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
@@ -482,7 +488,7 @@ try {
         }
 
     /* Check each path (slow!). */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
+    WaitForGoals result;
     for (auto & i : outputClosure) {
         if (worker.pathContentsGood(i)) continue;
         printError(
@@ -490,9 +496,9 @@ try {
             worker.store.printStorePath(i), worker.store.printStorePath(drvPath));
         auto drvPath2 = outputsToDrv.find(i);
         if (drvPath2 == outputsToDrv.end())
-            dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i, Repair));
+            result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i, Repair));
         else
-            dependencies.add(worker.goalFactory().makeGoal(
+            result.goals.insert(worker.goalFactory().makeGoal(
                 DerivedPath::Built {
                     .drvPath = makeConstantStorePathRef(drvPath2->second),
                     .outputs = OutputsSpec::All { },
@@ -500,18 +506,18 @@ try {
                 bmRepair));
     }
 
-    if (dependencies.empty()) {
+    if (result.goals.empty()) {
-        co_return done(BuildResult::AlreadyValid, assertPathValidity());
+        return {done(BuildResult::AlreadyValid, assertPathValidity())};
     }
 
-    (co_await waitForGoals(dependencies.releaseAsArray())).value();
+    state = &DerivationGoal::closureRepaired;
-    co_return co_await closureRepaired();
+    return {result};
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::closureRepaired() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::closureRepaired(bool inBuildSlot) noexcept
 try {
     trace("closure repaired");
     if (nrFailed > 0)
@@ -523,14 +529,14 @@ try {
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::inputsRealised() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::inputsRealised(bool inBuildSlot) noexcept
 try {
     trace("all inputs realised");
 
     if (nrFailed != 0) {
         if (!useDerivation)
             throw Error("some dependencies of '%s' are missing", worker.store.printStorePath(drvPath));
-        co_return done(
+        return {done(
             BuildResult::DependencyFailed,
             {},
             Error(
@@ -538,12 +544,12 @@ try {
                 nrFailed,
                 worker.store.printStorePath(drvPath)
             )
-        );
+        )};
     }
 
     if (retrySubstitution == RetrySubstitution::YesNeed) {
         retrySubstitution = RetrySubstitution::AlreadyRetried;
-        co_return co_await haveDerivation();
+        return haveDerivation(inBuildSlot);
     }
 
     /* Gather information necessary for computing the closure and/or
@@ -605,12 +611,11 @@ try {
                        worker.store.printStorePath(pathResolved),
                    });
 
-            auto dependency = worker.goalFactory().makeDerivationGoal(
+            resolvedDrvGoal = worker.goalFactory().makeDerivationGoal(
                 pathResolved, wantedOutputs, buildMode);
-            resolvedDrvGoal = dependency.first;
 
-            (co_await waitForGoals(std::move(dependency))).value();
+            state = &DerivationGoal::resolvedFinished;
-            co_return co_await resolvedFinished();
+            return {WaitForGoals{{resolvedDrvGoal}}};
         }
 
         std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumInputPaths;
@@ -674,9 +679,10 @@ try {
     /* Okay, try to build.  Note that here we don't wait for a build
        slot to become available, since we don't need one if there is a
        build hook. */
-    co_return co_await tryToBuild();
+    state = &DerivationGoal::tryToBuild;
+    return tryToBuild(inBuildSlot);
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 void DerivationGoal::started()
@@ -692,9 +698,8 @@ void DerivationGoal::started()
     mcRunningBuilds = worker.runningBuilds.addTemporarily(1);
 }
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::tryToBuild() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::tryToBuild(bool inBuildSlot) noexcept
 try {
-retry:
     trace("trying to build");
 
     /* Obtain locks on all output paths, if the paths are known a priori.
@@ -728,9 +733,7 @@ retry:
         if (!actLock)
             actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting,
                 fmt("waiting for lock on %s", Magenta(showPaths(lockFiles))));
-        co_await waitForAWhile();
+        return {WaitForAWhile{}};
-        // we can loop very often, and `co_return co_await` always allocates a new frame
-        goto retry;
     }
 
     actLock.reset();
@@ -747,7 +750,7 @@ retry:
     if (buildMode != bmCheck && allValid) {
         debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath));
         outputLocks.setDeletion(true);
-        co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
+        return {done(BuildResult::AlreadyValid, std::move(validOutputs))};
     }
 
     /* If any of the outputs already exist but are not valid, delete
@@ -767,56 +770,47 @@ retry:
         && settings.maxBuildJobs.get() != 0;
 
     if (!buildLocally) {
-        auto hookReply = tryBuildHook();
+        auto hookReply = tryBuildHook(inBuildSlot);
-        switch (hookReply.index()) {
+        auto result = std::visit(
-        case 0: {
+            overloaded{
-            HookReply::Accept & a = std::get<0>(hookReply);
+                [&](HookReply::Accept & a) -> std::optional<WorkResult> {
-            /* Yes, it has started doing so.  Wait until we get
+                    /* Yes, it has started doing so.  Wait until we get
-                EOF from the hook. */
+                       EOF from the hook. */
-            actLock.reset();
+                    actLock.reset();
-            buildResult.startTime = time(0); // inexact
+                    buildResult.startTime = time(0); // inexact
-            started();
+                    state = &DerivationGoal::buildDone;
-            auto r = co_await a.promise;
+                    started();
-            if (r.has_value()) {
+                    return WaitForWorld{std::move(a.fds), false};
-                co_return co_await buildDone();
+                },
-            } else if (r.has_error()) {
+                [&](HookReply::Postpone) -> std::optional<WorkResult> {
-                co_return r.assume_error();
+                    /* Not now; wait until at least one child finishes or
-            } else {
+                       the wake-up timeout expires. */
-                co_return r.assume_exception();
+                    if (!actLock)
-            }
+                        actLock = std::make_unique<Activity>(*logger, lvlTalkative, actBuildWaiting,
-        }
+                            fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath))));
-
+                    outputLocks.unlock();
-        case 1: {
+                    return WaitForAWhile{};
-            HookReply::Decline _ [[gnu::unused]] = std::get<1>(hookReply);
+                },
-            break;
+                [&](HookReply::Decline) -> std::optional<WorkResult> {
-        }
+                    /* We should do it ourselves. */
-
+                    return std::nullopt;
-        case 2: {
+                },
-            HookReply::Postpone _ [[gnu::unused]] = std::get<2>(hookReply);
+            },
-            /* Not now; wait until at least one child finishes or
+            hookReply);
-                the wake-up timeout expires. */
+        if (result) {
-            if (!actLock)
+            return {std::move(*result)};
-                actLock = std::make_unique<Activity>(*logger, lvlTalkative, actBuildWaiting,
-                    fmt("waiting for a machine to build '%s'", Magenta(worker.store.printStorePath(drvPath))));
-            outputLocks.unlock();
-            co_await waitForAWhile();
-            goto retry;
-        }
-
-        default:
-            // can't static_assert this because HookReply *subclasses* variant and std::variant_size breaks
-            assert(false && "unexpected hook reply");
         }
     }
 
     actLock.reset();
 
-    co_return co_await tryLocalBuild();
+    state = &DerivationGoal::tryLocalBuild;
+    return tryLocalBuild(inBuildSlot);
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::tryLocalBuild() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::tryLocalBuild(bool inBuildSlot) noexcept
 try {
     throw Error(
         "unable to build with a primary store that isn't a local store; "
@@ -863,7 +857,7 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath)
             // attempt to recover
             movePath(oldPath, storePath);
         } catch (...) {
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
         }
         throw;
     }
@@ -979,11 +973,10 @@ void runPostBuildHook(
     proc.getStdout()->drainInto(sink);
 }
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::buildDone() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::buildDone(bool inBuildSlot) noexcept
 try {
     trace("build done");
 
-    slotToken = {};
     Finally releaseBuildUser([&](){ this->cleanupHookFinally(); });
 
     cleanupPreChildKill();
@@ -999,6 +992,9 @@ try {
     buildResult.timesBuilt++;
     buildResult.stopTime = time(0);
 
+    /* So the child is gone now. */
+    worker.childTerminated(this);
+
     /* Close the read side of the logger pipe. */
     closeReadPipes();
 
@@ -1099,7 +1095,7 @@ try {
     return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DerivationGoal::resolvedFinished() noexcept
+kj::Promise<Result<Goal::WorkResult>> DerivationGoal::resolvedFinished(bool inBuildSlot) noexcept
 try {
     trace("resolved derivation finished");
 
@@ -1172,7 +1168,7 @@ try {
     return {std::current_exception()};
 }
 
-HookReply DerivationGoal::tryBuildHook()
+HookReply DerivationGoal::tryBuildHook(bool inBuildSlot)
 {
     if (!worker.hook.available || !useDerivation) return HookReply::Decline{};
 
@@ -1184,7 +1180,7 @@ HookReply DerivationGoal::tryBuildHook()
         /* Send the request to the hook. */
         worker.hook.instance->sink
             << "try"
-            << (slotToken.valid() ? 1 : 0)
+            << (inBuildSlot ? 1 : 0)
             << drv->platform
             << worker.store.printStorePath(drvPath)
             << parsedDrv->getRequiredSystemFeatures();
@@ -1270,8 +1266,12 @@ HookReply DerivationGoal::tryBuildHook()
     /* Create the log file and pipe. */
     Path logFile = openLogFile();
 
+    std::set<int> fds;
+    fds.insert(hook->fromHook.get());
+    fds.insert(hook->builderOut.get());
     builderOutFD = &hook->builderOut;
-    return HookReply::Accept{handleChildOutput()};
+
+    return HookReply::Accept{std::move(fds)};
 }
 
 
@@ -1331,69 +1331,23 @@ void DerivationGoal::closeLogFile()
 }
 
 
-Goal::WorkResult DerivationGoal::tooMuchLogs()
+Goal::WorkResult DerivationGoal::handleChildOutput(int fd, std::string_view data)
 {
-    killChild();
+    assert(builderOutFD);
-    return done(
-        BuildResult::LogLimitExceeded, {},
-        Error("%s killed after writing more than %d bytes of log output",
-            getName(), settings.maxLogSize));
-}
 
-struct DerivationGoal::InputStream final : private kj::AsyncObject
+    auto tooMuchLogs = [&] {
-{
+        killChild();
-    int fd;
+        return done(
-    kj::UnixEventPort::FdObserver observer;
+            BuildResult::LogLimitExceeded, {},
-
+            Error("%s killed after writing more than %d bytes of log output",
-    InputStream(kj::UnixEventPort & ep, int fd)
+                getName(), settings.maxLogSize));
-        : fd(fd)
+    };
-        , observer(ep, fd, kj::UnixEventPort::FdObserver::OBSERVE_READ)
-    {
-        int flags = fcntl(fd, F_GETFL);
-        if (flags < 0) {
-            throw SysError("fcntl(F_GETFL) failed on fd %i", fd);
-        }
-        if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0) {
-            throw SysError("fcntl(F_SETFL) failed on fd %i", fd);
-        }
-    }
-
-    kj::Promise<std::string_view> read(kj::ArrayPtr<char> buffer)
-    {
-        const auto res = ::read(fd, buffer.begin(), buffer.size());
-        // closing a pty endpoint causes EIO on the other endpoint. stock kj streams
-        // do not handle this and throw exceptions we can't ask for errno instead :(
-        // (we can't use `errno` either because kj may well have mangled it by now.)
-        if (res == 0 || (res == -1 && errno == EIO)) {
-            return std::string_view{};
-        }
-
-        KJ_NONBLOCKING_SYSCALL(res) {}
-
-        if (res > 0) {
-            return std::string_view{buffer.begin(), static_cast<size_t>(res)};
-        }
-
-        return observer.whenBecomesReadable().then([this, buffer] {
-            return read(buffer);
-        });
-    }
-};
-
-kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleBuilderOutput(InputStream & in) noexcept
-try {
-    auto buf = kj::heapArray<char>(4096);
-    while (true) {
-        auto data = co_await in.read(buf);
-        lastChildActivity = worker.aio.provider->getTimer().now();
-
-        if (data.empty()) {
-            co_return result::success();
-        }
 
+    // local & `ssh://`-builds are dealt with here.
+    if (fd == builderOutFD->get()) {
         logSize += data.size();
         if (settings.maxLogSize && logSize > settings.maxLogSize) {
-            co_return tooMuchLogs();
+            return tooMuchLogs();
         }
 
         for (auto c : data)
@@ -1408,22 +1362,10 @@ try {
             }
 
         if (logSink) (*logSink)(data);
+        return StillAlive{};
     }
-} catch (...) {
-    co_return std::current_exception();
-}
-
-kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleHookOutput(InputStream & in) noexcept
-try {
-    auto buf = kj::heapArray<char>(4096);
-    while (true) {
-        auto data = co_await in.read(buf);
-        lastChildActivity = worker.aio.provider->getTimer().now();
-
-        if (data.empty()) {
-            co_return result::success();
-        }
 
+    if (hook && fd == hook->fromHook.get()) {
         for (auto c : data)
             if (c == '\n') {
                 auto json = parseJSONMessage(currentHookLine);
@@ -1439,7 +1381,7 @@ try {
                                 (fields.size() > 0 ? fields[0].get<std::string>() : "") + "\n";
                             logSize += logLine.size();
                             if (settings.maxLogSize && logSize > settings.maxLogSize) {
-                                co_return tooMuchLogs();
+                                return tooMuchLogs();
                             }
                             (*logSink)(logLine);
                         } else if (type == resSetPhase && ! fields.is_null()) {
@@ -1463,83 +1405,16 @@ try {
             } else
                 currentHookLine += c;
     }
-} catch (...) {
+
-    co_return std::current_exception();
+    return StillAlive{};
 }
 
-kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::handleChildOutput() noexcept
-try {
-    assert(builderOutFD);
 
-    auto builderIn = kj::heap<InputStream>(worker.aio.unixEventPort, builderOutFD->get());
+void DerivationGoal::handleEOF(int fd)
-    kj::Own<InputStream> hookIn;
-    if (hook) {
-        hookIn = kj::heap<InputStream>(worker.aio.unixEventPort, hook->fromHook.get());
-    }
-
-    auto handlers = handleChildStreams(*builderIn, hookIn.get()).attach(std::move(builderIn), std::move(hookIn));
-
-    if (respectsTimeouts() && settings.buildTimeout != 0) {
-        handlers = handlers.exclusiveJoin(
-            worker.aio.provider->getTimer()
-                .afterDelay(settings.buildTimeout.get() * kj::SECONDS)
-                .then([this]() -> Outcome<void, WorkResult> {
-                    return timedOut(
-                        Error("%1% timed out after %2% seconds", name, settings.buildTimeout)
-                    );
-                })
-        );
-    }
-
-    return handlers.then([this](auto r) -> Outcome<void, WorkResult> {
-        if (!currentLogLine.empty()) flushLine();
-        return r;
-    });
-} catch (...) {
-    return {std::current_exception()};
-}
-
-kj::Promise<Outcome<void, Goal::WorkResult>> DerivationGoal::monitorForSilence() noexcept
 {
-    while (true) {
+    if (!currentLogLine.empty()) flushLine();
-        const auto stash = lastChildActivity;
-        auto waitUntil = lastChildActivity + settings.maxSilentTime.get() * kj::SECONDS;
-        co_await worker.aio.provider->getTimer().atTime(waitUntil);
-        if (lastChildActivity == stash) {
-            co_return timedOut(
-                Error("%1% timed out after %2% seconds of silence", name, settings.maxSilentTime)
-            );
-        }
-    }
 }
 
-kj::Promise<Outcome<void, Goal::WorkResult>>
-DerivationGoal::handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept
-{
-    lastChildActivity = worker.aio.provider->getTimer().now();
-
-    auto handlers = kj::joinPromisesFailFast([&] {
-        kj::Vector<kj::Promise<Outcome<void, WorkResult>>> parts{2};
-
-        parts.add(handleBuilderOutput(builderIn));
-        if (hookIn) {
-            parts.add(handleHookOutput(*hookIn));
-        }
-
-        return parts.releaseAsArray();
-    }());
-
-    if (respectsTimeouts() && settings.maxSilentTime != 0) {
-        handlers = handlers.exclusiveJoin(monitorForSilence().then([](auto r) {
-            return kj::arr(std::move(r));
-        }));
-    }
-
-    for (auto r : co_await handlers) {
-        BOOST_OUTCOME_CO_TRYV(r);
-    }
-    co_return result::success();
-}
 
 void DerivationGoal::flushLine()
 {
@@ -1680,13 +1555,11 @@ SingleDrvOutputs DerivationGoal::assertPathValidity()
 }
 
 
-Goal::WorkResult DerivationGoal::done(
+Goal::Finished DerivationGoal::done(
     BuildResult::Status status,
     SingleDrvOutputs builtOutputs,
     std::optional<Error> ex)
 {
-    isDone = true;
-
     outputLocks.unlock();
     buildResult.status = status;
     if (ex)
@@ -1717,7 +1590,7 @@ Goal::WorkResult DerivationGoal::done(
         logError(ex->info());
     }
 
-    return WorkResult{
+    return Finished{
         .exitCode = buildResult.success() ? ecSuccess : ecFailed,
         .result = buildResult,
         .ex = ex ? std::make_shared<Error>(std::move(*ex)) : nullptr,
@@ -1756,4 +1629,5 @@ void DerivationGoal::waiteeDone(GoalPtr waitee)
         }
     }
 }
+
 }

src/libstore/build/derivation-goal.hh

@@ -8,7 +8,6 @@
 #include "store-api.hh"
 #include "pathlocks.hh"
 #include "goal.hh"
-#include <kj/time.h>
 
 namespace nix {
 
@@ -18,7 +17,7 @@ struct HookInstance;
 
 struct HookReplyBase {
     struct [[nodiscard]] Accept {
-        kj::Promise<Outcome<void, Goal::WorkResult>> promise;
+        std::set<int> fds;
     };
     struct [[nodiscard]] Decline {};
     struct [[nodiscard]] Postpone {};
@@ -71,14 +70,6 @@ struct InitialOutput {
  */
 struct DerivationGoal : public Goal
 {
-    struct InputStream;
-
-    /**
-      * Whether this goal has completed. Completed goals can not be
-      * asked for more outputs, a new goal must be created instead.
-      */
-    bool isDone = false;
-
     /**
      * Whether to use an on-disk .drv file.
      */
@@ -184,11 +175,6 @@ struct DerivationGoal : public Goal
 
     std::map<std::string, InitialOutput> initialOutputs;
 
-    /**
-     * Build result.
-     */
-    BuildResult buildResult;
-
     /**
      * File descriptor for the log file.
      */
@@ -227,6 +213,9 @@ struct DerivationGoal : public Goal
      */
     std::optional<DerivationType> derivationType;
 
+    typedef kj::Promise<Result<WorkResult>> (DerivationGoal::*GoalState)(bool inBuildSlot) noexcept;
+    GoalState state;
+
     BuildMode buildMode;
 
     NotifyingCounter<uint64_t>::Bump mcExpectedBuilds, mcRunningBuilds;
@@ -253,35 +242,37 @@ struct DerivationGoal : public Goal
         BuildMode buildMode = bmNormal);
     virtual ~DerivationGoal() noexcept(false);
 
-    WorkResult timedOut(Error && ex);
+    Finished timedOut(Error && ex) override;
 
-    kj::Promise<Result<WorkResult>> workImpl() noexcept override;
+    std::string key() override;
+
+    kj::Promise<Result<WorkResult>> work(bool inBuildSlot) noexcept override;
 
     /**
      * Add wanted outputs to an already existing derivation goal.
      */
-    bool addWantedOutputs(const OutputsSpec & outputs);
+    void addWantedOutputs(const OutputsSpec & outputs);
 
     /**
      * The states.
      */
-    kj::Promise<Result<WorkResult>> getDerivation() noexcept;
+    kj::Promise<Result<WorkResult>> getDerivation(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> loadDerivation() noexcept;
+    kj::Promise<Result<WorkResult>> loadDerivation(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> haveDerivation() noexcept;
+    kj::Promise<Result<WorkResult>> haveDerivation(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> outputsSubstitutionTried() noexcept;
+    kj::Promise<Result<WorkResult>> outputsSubstitutionTried(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> gaveUpOnSubstitution() noexcept;
+    kj::Promise<Result<WorkResult>> gaveUpOnSubstitution(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> closureRepaired() noexcept;
+    kj::Promise<Result<WorkResult>> closureRepaired(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> inputsRealised() noexcept;
+    kj::Promise<Result<WorkResult>> inputsRealised(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> tryToBuild() noexcept;
+    kj::Promise<Result<WorkResult>> tryToBuild(bool inBuildSlot) noexcept;
-    virtual kj::Promise<Result<WorkResult>> tryLocalBuild() noexcept;
+    virtual kj::Promise<Result<WorkResult>> tryLocalBuild(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> buildDone() noexcept;
+    kj::Promise<Result<WorkResult>> buildDone(bool inBuildSlot) noexcept;
 
-    kj::Promise<Result<WorkResult>> resolvedFinished() noexcept;
+    kj::Promise<Result<WorkResult>> resolvedFinished(bool inBuildSlot) noexcept;
 
     /**
      * Is the build hook willing to perform the build?
      */
-    HookReply tryBuildHook();
+    HookReply tryBuildHook(bool inBuildSlot);
 
     virtual int getChildStatus();
 
@@ -321,19 +312,13 @@ struct DerivationGoal : public Goal
     virtual void cleanupPostOutputsRegisteredModeCheck();
     virtual void cleanupPostOutputsRegisteredModeNonCheck();
 
-protected:
+    /**
-    kj::TimePoint lastChildActivity = kj::minValue;
+     * Callback used by the worker to write to the log.
-
+     */
-    kj::Promise<Outcome<void, WorkResult>> handleChildOutput() noexcept;
+    WorkResult handleChildOutput(int fd, std::string_view data) override;
-    kj::Promise<Outcome<void, WorkResult>>
+    void handleEOF(int fd) override;
-    handleChildStreams(InputStream & builderIn, InputStream * hookIn) noexcept;
-    kj::Promise<Outcome<void, WorkResult>> handleBuilderOutput(InputStream & in) noexcept;
-    kj::Promise<Outcome<void, WorkResult>> handleHookOutput(InputStream & in) noexcept;
-    kj::Promise<Outcome<void, WorkResult>> monitorForSilence() noexcept;
-    WorkResult tooMuchLogs();
     void flushLine();
 
-public:
     /**
      * Wrappers around the corresponding Store methods that first consult the
      * derivation.  This is currently needed because when there is no drv file
@@ -365,18 +350,13 @@ public:
 
     void started();
 
-    WorkResult done(
+    Finished done(
         BuildResult::Status status,
         SingleDrvOutputs builtOutputs = {},
         std::optional<Error> ex = {});
 
     void waiteeDone(GoalPtr waitee) override;
 
-    virtual bool respectsTimeouts()
-    {
-        return false;
-    }
-
     StorePathSet exportReferences(const StorePathSet & storePaths);
 
     JobCategory jobCategory() const override {

src/libstore/build/drv-output-substitution-goal.cc

@@ -4,9 +4,6 @@
 #include "worker.hh"
 #include "substitution-goal.hh"
 #include "signals.hh"
-#include <kj/array.h>
-#include <kj/async.h>
-#include <kj/vector.h>
 
 namespace nix {
 
@@ -19,32 +16,33 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(
     : Goal(worker, isDependency)
     , id(id)
 {
+    state = &DrvOutputSubstitutionGoal::init;
     name = fmt("substitution of '%s'", id.to_string());
     trace("created");
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::workImpl() noexcept
+kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::init(bool inBuildSlot) noexcept
 try {
     trace("init");
 
     /* If the derivation already exists, we’re done */
     if (worker.store.queryRealisation(id)) {
-        co_return WorkResult{ecSuccess};
+        return {Finished{ecSuccess, std::move(buildResult)}};
     }
 
     subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>();
-    co_return co_await tryNext();
+    return tryNext(inBuildSlot);
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::tryNext() noexcept
+kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::tryNext(bool inBuildSlot) noexcept
 try {
     trace("trying next substituter");
 
-    if (!slotToken.valid()) {
+    if (!inBuildSlot) {
-        slotToken = co_await worker.substitutions.acquire();
+        return {WaitForSlot{}};
     }
 
     maintainRunningSubstitutions = worker.runningSubstitutions.addTemporarily(1);
@@ -61,7 +59,7 @@ try {
         /* Hack: don't indicate failure if there were no substituters.
            In that case the calling derivation should just do a
            build. */
-        co_return WorkResult{substituterFailed ? ecFailed : ecNoSubstituters};
+        return {Finished{substituterFailed ? ecFailed : ecNoSubstituters, std::move(buildResult)}};
     }
 
     sub = subs.front();
@@ -71,26 +69,25 @@ try {
        some other error occurs), so it must not touch `this`. So put
        the shared state in a separate refcounted object. */
     downloadState = std::make_shared<DownloadState>();
-    auto pipe = kj::newPromiseAndCrossThreadFulfiller<void>();
+    downloadState->outPipe.create();
-    downloadState->outPipe = kj::mv(pipe.fulfiller);
 
     downloadState->result =
         std::async(std::launch::async, [downloadState{downloadState}, id{id}, sub{sub}] {
-            Finally updateStats([&]() { downloadState->outPipe->fulfill(); });
             ReceiveInterrupts receiveInterrupts;
+            Finally updateStats([&]() { downloadState->outPipe.writeSide.close(); });
             return sub->queryRealisation(id);
         });
 
-    co_await pipe.promise;
+    state = &DrvOutputSubstitutionGoal::realisationFetched;
-    co_return co_await realisationFetched();
+    return {WaitForWorld{{downloadState->outPipe.readSide.get()}, true}};
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::realisationFetched() noexcept
+kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::realisationFetched(bool inBuildSlot) noexcept
 try {
+    worker.childTerminated(this);
     maintainRunningSubstitutions.reset();
-    slotToken = {};
 
     try {
         outputInfo = downloadState->result.get();
@@ -100,10 +97,10 @@ try {
     }
 
     if (!outputInfo) {
-        co_return co_await tryNext();
+        return tryNext(inBuildSlot);
     }
 
-    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
+    WaitForGoals result;
     for (const auto & [depId, depPath] : outputInfo->dependentRealisations) {
         if (depId != id) {
             if (auto localOutputInfo = worker.store.queryRealisation(depId);
@@ -117,31 +114,34 @@ try {
                     worker.store.printStorePath(localOutputInfo->outPath),
                     worker.store.printStorePath(depPath)
                 );
-                co_return co_await tryNext();
+                return tryNext(inBuildSlot);
             }
-            dependencies.add(worker.goalFactory().makeDrvOutputSubstitutionGoal(depId));
+            result.goals.insert(worker.goalFactory().makeDrvOutputSubstitutionGoal(depId));
         }
     }
 
-    dependencies.add(worker.goalFactory().makePathSubstitutionGoal(outputInfo->outPath));
+    result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(outputInfo->outPath));
 
-    if (!dependencies.empty()) {
+    if (result.goals.empty()) {
-        (co_await waitForGoals(dependencies.releaseAsArray())).value();
+        return outPathValid(inBuildSlot);
+    } else {
+        state = &DrvOutputSubstitutionGoal::outPathValid;
+        return {std::move(result)};
     }
-    co_return co_await outPathValid();
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
-kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::outPathValid() noexcept
+kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::outPathValid(bool inBuildSlot) noexcept
 try {
     assert(outputInfo);
     trace("output path substituted");
 
     if (nrFailed > 0) {
         debug("The output path of the derivation output '%s' could not be substituted", id.to_string());
-        return {WorkResult{
+        return {Finished{
             nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed,
+            std::move(buildResult),
         }};
     }
 
@@ -154,9 +154,22 @@ try {
 kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::finished() noexcept
 try {
     trace("finished");
-    return {WorkResult{ecSuccess}};
+    return {Finished{ecSuccess, std::move(buildResult)}};
 } catch (...) {
     return {std::current_exception()};
 }
 
+std::string DrvOutputSubstitutionGoal::key()
+{
+    /* "a$" ensures substitution goals happen before derivation
+       goals. */
+    return "a$" + std::string(id.to_string());
+}
+
+kj::Promise<Result<Goal::WorkResult>> DrvOutputSubstitutionGoal::work(bool inBuildSlot) noexcept
+{
+    return (this->*state)(inBuildSlot);
+}
+
+
 }

src/libstore/build/drv-output-substitution-goal.hh

@@ -45,7 +45,7 @@ class DrvOutputSubstitutionGoal : public Goal {
 
     struct DownloadState
     {
-        kj::Own<kj::CrossThreadPromiseFulfiller<void>> outPipe;
+        Pipe outPipe;
         std::future<std::shared_ptr<const Realisation>> result;
     };
 
@@ -65,12 +65,20 @@ public:
         std::optional<ContentAddress> ca = std::nullopt
     );
 
-    kj::Promise<Result<WorkResult>> tryNext() noexcept;
+    typedef kj::Promise<Result<WorkResult>> (DrvOutputSubstitutionGoal::*GoalState)(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> realisationFetched() noexcept;
+    GoalState state;
-    kj::Promise<Result<WorkResult>> outPathValid() noexcept;
+
+    kj::Promise<Result<WorkResult>> init(bool inBuildSlot) noexcept;
+    kj::Promise<Result<WorkResult>> tryNext(bool inBuildSlot) noexcept;
+    kj::Promise<Result<WorkResult>> realisationFetched(bool inBuildSlot) noexcept;
+    kj::Promise<Result<WorkResult>> outPathValid(bool inBuildSlot) noexcept;
     kj::Promise<Result<WorkResult>> finished() noexcept;
 
-    kj::Promise<Result<WorkResult>> workImpl() noexcept override;
+    Finished timedOut(Error && ex) override { abort(); };
+
+    std::string key() override;
+
+    kj::Promise<Result<WorkResult>> work(bool inBuildSlot) noexcept override;
 
     JobCategory jobCategory() const override {
         return JobCategory::Substitution;

src/libstore/build/entry-points.cc

@@ -17,22 +17,22 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
     Worker worker(*this, evalStore ? *evalStore : *this, aio);
 
     auto goals = runWorker(worker, [&](GoalFactory & gf) {
-        Worker::Targets goals;
+        Goals goals;
         for (auto & br : reqs)
-            goals.emplace(gf.makeGoal(br, buildMode));
+            goals.insert(gf.makeGoal(br, buildMode));
         return goals;
     });
 
     StringSet failed;
     std::shared_ptr<Error> ex;
-    for (auto & [i, result] : goals) {
+    for (auto & i : goals) {
-        if (result.ex) {
+        if (i->ex) {
             if (ex)
-                logError(result.ex->info());
+                logError(i->ex->info());
             else
-                ex = result.ex;
+                ex = i->ex;
         }
-        if (result.exitCode != Goal::ecSuccess) {
+        if (i->exitCode != Goal::ecSuccess) {
             if (auto i2 = dynamic_cast<DerivationGoal *>(i.get()))
                 failed.insert(printStorePath(i2->drvPath));
             else if (auto i2 = dynamic_cast<PathSubstitutionGoal *>(i.get()))
@@ -60,11 +60,11 @@ std::vector<KeyedBuildResult> Store::buildPathsWithResults(
     std::vector<std::pair<const DerivedPath &, GoalPtr>> state;
 
     auto goals = runWorker(worker, [&](GoalFactory & gf) {
-        Worker::Targets goals;
+        Goals goals;
         for (const auto & req : reqs) {
             auto goal = gf.makeGoal(req, buildMode);
-            state.push_back({req, goal.first});
+            goals.insert(goal);
-            goals.emplace(std::move(goal));
+            state.push_back({req, goal});
         }
         return goals;
     });
@@ -72,7 +72,7 @@ std::vector<KeyedBuildResult> Store::buildPathsWithResults(
     std::vector<KeyedBuildResult> results;
 
     for (auto & [req, goalPtr] : state)
-        results.emplace_back(goals[goalPtr].result.restrictTo(req));
+        results.emplace_back(goalPtr->buildResult.restrictTo(req));
 
     return results;
 }
@@ -84,13 +84,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
     Worker worker(*this, *this, aio);
 
     try {
-        auto goals = runWorker(worker, [&](GoalFactory & gf) {
+        auto goals = runWorker(worker, [&](GoalFactory & gf) -> Goals {
-            Worker::Targets goals;
+            return Goals{gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)};
-            goals.emplace(gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode));
-            return goals;
         });
-        auto [goal, result] = *goals.begin();
+        auto goal = *goals.begin();
-        return result.result.restrictTo(DerivedPath::Built {
+        return goal->buildResult.restrictTo(DerivedPath::Built {
             .drvPath = makeConstantStorePathRef(drvPath),
             .outputs = OutputsSpec::All {},
         });
@@ -112,16 +110,14 @@ void Store::ensurePath(const StorePath & path)
     Worker worker(*this, *this, aio);
 
     auto goals = runWorker(worker, [&](GoalFactory & gf) {
-        Worker::Targets goals;
+        return Goals{gf.makePathSubstitutionGoal(path)};
-        goals.emplace(gf.makePathSubstitutionGoal(path));
-        return goals;
     });
-    auto [goal, result] = *goals.begin();
+    auto goal = *goals.begin();
 
-    if (result.exitCode != Goal::ecSuccess) {
+    if (goal->exitCode != Goal::ecSuccess) {
-        if (result.ex) {
+        if (goal->ex) {
-            result.ex->withExitStatus(worker.failingExitStatus());
+            goal->ex->withExitStatus(worker.failingExitStatus());
-            throw std::move(*result.ex);
+            throw std::move(*goal->ex);
         } else
             throw Error(worker.failingExitStatus(), "path '%s' does not exist and cannot be created", printStorePath(path));
     }
@@ -134,28 +130,24 @@ void Store::repairPath(const StorePath & path)
     Worker worker(*this, *this, aio);
 
     auto goals = runWorker(worker, [&](GoalFactory & gf) {
-        Worker::Targets goals;
+        return Goals{gf.makePathSubstitutionGoal(path, Repair)};
-        goals.emplace(gf.makePathSubstitutionGoal(path, Repair));
-        return goals;
     });
-    auto [goal, result] = *goals.begin();
+    auto goal = *goals.begin();
 
-    if (result.exitCode != Goal::ecSuccess) {
+    if (goal->exitCode != Goal::ecSuccess) {
         /* Since substituting the path didn't work, if we have a valid
            deriver, then rebuild the deriver. */
         auto info = queryPathInfo(path);
         if (info->deriver && isValidPath(*info->deriver)) {
             worker.run([&](GoalFactory & gf) {
-                Worker::Targets goals;
+                return Goals{gf.makeGoal(
-                goals.emplace(gf.makeGoal(
                     DerivedPath::Built{
                         .drvPath = makeConstantStorePathRef(*info->deriver),
                         // FIXME: Should just build the specific output we need.
                         .outputs = OutputsSpec::All{},
                     },
                     bmRepair
-                ));
+                )};
-                return goals;
             });
         } else
             throw Error(worker.failingExitStatus(), "cannot repair path '%s'", printStorePath(path));

src/libstore/build/goal.cc

@@ -1,73 +1,18 @@
 #include "goal.hh"
-#include "async-collect.hh"
-#include "worker.hh"
-#include <boost/outcome/try.hpp>
-#include <kj/time.h>
 
 namespace nix {
 
 
+bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const {
+    std::string s1 = a->key();
+    std::string s2 = b->key();
+    return s1 < s2;
+}
+
+
 void Goal::trace(std::string_view s)
 {
     debug("%1%: %2%", name, s);
 }
 
-kj::Promise<void> Goal::waitForAWhile()
-{
-    trace("wait for a while");
-    /* If we are polling goals that are waiting for a lock, then wake
-       up after a few seconds at most. */
-    return worker.aio.provider->getTimer().afterDelay(settings.pollInterval.get() * kj::SECONDS);
-}
-
-kj::Promise<Result<Goal::WorkResult>> Goal::work() noexcept
-try {
-    BOOST_OUTCOME_CO_TRY(auto result, co_await workImpl());
-
-    trace("done");
-
-    cleanup();
-
-    co_return std::move(result);
-} catch (...) {
-    co_return result::failure(std::current_exception());
-}
-
-kj::Promise<Result<void>>
-Goal::waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept
-try {
-    auto left = dependencies.size();
-    for (auto & [dep, p] : dependencies) {
-        p = p.then([this, dep, &left](auto _result) -> Result<WorkResult> {
-            BOOST_OUTCOME_TRY(auto result, _result);
-
-            left--;
-            trace(fmt("waitee '%s' done; %d left", dep->name, left));
-
-            if (result.exitCode != Goal::ecSuccess) ++nrFailed;
-            if (result.exitCode == Goal::ecNoSubstituters) ++nrNoSubstituters;
-            if (result.exitCode == Goal::ecIncompleteClosure) ++nrIncompleteClosure;
-
-            return std::move(result);
-        }).eagerlyEvaluate(nullptr);
-    }
-
-    auto collectDeps = asyncCollect(std::move(dependencies));
-
-    while (auto item = co_await collectDeps.next()) {
-        auto & [dep, _result] = *item;
-        BOOST_OUTCOME_CO_TRY(auto result, _result);
-
-        waiteeDone(dep);
-
-        if (result.exitCode == ecFailed && !settings.keepGoing) {
-            co_return result::success();
-        }
-    }
-
-    co_return result::success();
-} catch (...) {
-    co_return result::failure(std::current_exception());
-}
-
 }

src/libstore/build/goal.hh

@@ -1,12 +1,10 @@
 #pragma once
 ///@file
 
-#include "async-semaphore.hh"
 #include "result.hh"
 #include "types.hh"
 #include "store-api.hh"
 #include "build-result.hh"
-#include <concepts> // IWYU pragma: keep
 #include <kj/async.h>
 
 namespace nix {
@@ -21,11 +19,22 @@ class Worker;
  * A pointer to a goal.
  */
 typedef std::shared_ptr<Goal> GoalPtr;
+typedef std::weak_ptr<Goal> WeakGoalPtr;
+
+struct CompareGoalPtrs {
+    bool operator() (const GoalPtr & a, const GoalPtr & b) const;
+};
 
 /**
  * Set of goals.
  */
-typedef std::set<GoalPtr> Goals;
+typedef std::set<GoalPtr, CompareGoalPtrs> Goals;
+typedef std::set<WeakGoalPtr, std::owner_less<WeakGoalPtr>> WeakGoals;
+
+/**
+ * A map of paths to goals (and the other way around).
+ */
+typedef std::map<StorePath, WeakGoalPtr> WeakGoalMap;
 
 /**
  * Used as a hint to the worker on how to schedule a particular goal. For example,
@@ -60,6 +69,17 @@ struct Goal
       */
     const bool isDependency;
 
+    /**
+     * Goals that this goal is waiting for.
+     */
+    Goals waitees;
+
+    /**
+     * Goals waiting for this one to finish.  Must use weak pointers
+     * here to prevent cycles.
+     */
+    WeakGoals waiters;
+
     /**
      * Number of goals we are/were waiting for that have failed.
      */
@@ -82,11 +102,30 @@ struct Goal
      */
     std::string name;
 
-protected:
+    /**
-    AsyncSemaphore::Token slotToken;
+     * Whether the goal is finished.
+     */
+    std::optional<ExitCode> exitCode;
+
+    /**
+     * Build result.
+     */
+    BuildResult buildResult;
 
 public:
-    struct [[nodiscard]] WorkResult {
+
+    struct [[nodiscard]] StillAlive {};
+    struct [[nodiscard]] WaitForSlot {};
+    struct [[nodiscard]] WaitForAWhile {};
+    struct [[nodiscard]] ContinueImmediately {};
+    struct [[nodiscard]] WaitForGoals {
+        Goals goals;
+    };
+    struct [[nodiscard]] WaitForWorld {
+        std::set<int> fds;
+        bool inBuildSlot;
+    };
+    struct [[nodiscard]] Finished {
         ExitCode exitCode;
         BuildResult result;
         std::shared_ptr<Error> ex;
@@ -96,23 +135,24 @@ public:
         bool checkMismatch = false;
     };
 
-protected:
+    struct [[nodiscard]] WorkResult : std::variant<
-    kj::Promise<void> waitForAWhile();
+                                          StillAlive,
-    kj::Promise<Result<void>>
+                                          WaitForSlot,
-    waitForGoals(kj::Array<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies) noexcept;
+                                          WaitForAWhile,
-
+                                          ContinueImmediately,
-    template<std::derived_from<Goal>... G>
+                                          WaitForGoals,
-    kj::Promise<Result<void>>
+                                          WaitForWorld,
-    waitForGoals(std::pair<std::shared_ptr<G>, kj::Promise<Result<WorkResult>>>... goals) noexcept
+                                          Finished>
     {
-        return waitForGoals(
+        WorkResult() = delete;
-            kj::arrOf<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>>(std::move(goals)...)
+        using variant::variant;
-        );
+    };
-    }
 
-    virtual kj::Promise<Result<WorkResult>> workImpl() noexcept = 0;
+    /**
+     * Exception containing an error message, if any.
+     */
+    std::shared_ptr<Error> ex;
 
-public:
     explicit Goal(Worker & worker, bool isDependency)
         : worker(worker)
         , isDependency(isDependency)
@@ -123,10 +163,24 @@ public:
         trace("goal destroyed");
     }
 
-    kj::Promise<Result<WorkResult>> work() noexcept;
+    virtual kj::Promise<Result<WorkResult>> work(bool inBuildSlot) noexcept = 0;
 
     virtual void waiteeDone(GoalPtr waitee) { }
 
+    virtual WorkResult handleChildOutput(int fd, std::string_view data)
+    {
+        abort();
+    }
+
+    virtual void handleEOF(int fd)
+    {
+    }
+
+    virtual bool respectsTimeouts()
+    {
+        return false;
+    }
+
     void trace(std::string_view s);
 
     std::string getName() const
@@ -134,6 +188,15 @@ public:
         return name;
     }
 
+    /**
+     * Callback in case of a timeout.  It should wake up its waiters,
+     * get rid of any running child processes that are being monitored
+     * by the worker (important!), etc.
+     */
+    virtual Finished timedOut(Error && ex) = 0;
+
+    virtual std::string key() = 0;
+
     virtual void cleanup() { }
 
     /**

src/libstore/build/hook-instance.cc

@@ -1,5 +1,4 @@
 #include "child.hh"
-#include "error.hh"
 #include "file-system.hh"
 #include "globals.hh"
 #include "hook-instance.hh"
@@ -87,7 +86,7 @@ HookInstance::~HookInstance()
         toHook.reset();
         if (pid) pid.kill();
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

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

@@ -1,5 +1,4 @@
 #include "local-derivation-goal.hh"
-#include "error.hh"
 #include "indirect-root-store.hh"
 #include "machines.hh"
 #include "store-api.hh"
@@ -99,9 +98,9 @@ LocalDerivationGoal::~LocalDerivationGoal() noexcept(false)
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try { deleteTmpDir(false); } catch (...) { ignoreExceptionInDestructor(); }
+    try { deleteTmpDir(false); } catch (...) { ignoreException(); }
-    try { killChild(); } catch (...) { ignoreExceptionInDestructor(); }
+    try { killChild(); } catch (...) { ignoreException(); }
-    try { stopDaemon(); } catch (...) { ignoreExceptionInDestructor(); }
+    try { stopDaemon(); } catch (...) { ignoreException(); }
 }
 
 
@@ -122,6 +121,8 @@ LocalStore & LocalDerivationGoal::getLocalStore()
 void LocalDerivationGoal::killChild()
 {
     if (pid) {
+        worker.childTerminated(this);
+
         /* If we're using a build user, then there is a tricky race
            condition: if we kill the build user before the child has
            done its setuid() to the build user uid, then it won't be
@@ -148,18 +149,17 @@ void LocalDerivationGoal::killSandbox(bool getStats)
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> LocalDerivationGoal::tryLocalBuild() noexcept
+kj::Promise<Result<Goal::WorkResult>> LocalDerivationGoal::tryLocalBuild(bool inBuildSlot) noexcept
 try {
-retry:
 #if __APPLE__
     additionalSandboxProfile = parsedDrv->getStringAttr("__sandboxProfile").value_or("");
 #endif
 
-    if (!slotToken.valid()) {
+    if (!inBuildSlot) {
+        state = &DerivationGoal::tryToBuild;
         outputLocks.unlock();
-        if (worker.localBuilds.capacity() > 0) {
+        if (0U != settings.maxBuildJobs) {
-            slotToken = co_await worker.localBuilds.acquire();
+            return {WaitForSlot{}};
-            co_return co_await tryToBuild();
         }
         if (getMachines().empty()) {
             throw Error(
@@ -214,9 +214,7 @@ retry:
             if (!actLock)
                 actLock = std::make_unique<Activity>(*logger, lvlWarn, actBuildWaiting,
                     fmt("waiting for a free build user ID for '%s'", Magenta(worker.store.printStorePath(drvPath))));
-            co_await waitForAWhile();
+            return {WaitForAWhile{}};
-            // we can loop very often, and `co_return co_await` always allocates a new frame
-            goto retry;
         }
     }
 
@@ -245,29 +243,24 @@ retry:
     try {
 
         /* Okay, we have to build. */
-        auto promise = startBuilder();
+        auto fds = startBuilder();
+
+        /* This state will be reached when we get EOF on the child's
+           log pipe. */
+        state = &DerivationGoal::buildDone;
 
         started();
-        auto r = co_await promise;
+        return {WaitForWorld{std::move(fds), true}};
-        if (r.has_value()) {
-            // all good so far
-        } else if (r.has_error()) {
-            co_return r.assume_error();
-        } else {
-            co_return r.assume_exception();
-        }
 
     } catch (BuildError & e) {
         outputLocks.unlock();
         buildUser.reset();
         auto report = done(BuildResult::InputRejected, {}, std::move(e));
         report.permanentFailure = true;
-        co_return report;
+        return {std::move(report)};
     }
-
-    co_return co_await buildDone();
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
@@ -397,9 +390,7 @@ void LocalDerivationGoal::cleanupPostOutputsRegisteredModeNonCheck()
     cleanupPostOutputsRegisteredModeCheck();
 }
 
-// NOTE this one isn't noexcept because it's called from places that expect
+std::set<int> LocalDerivationGoal::startBuilder()
-// exceptions to signal failure to launch. we should change this some time.
-kj::Promise<Outcome<void, Goal::WorkResult>> LocalDerivationGoal::startBuilder()
 {
     if ((buildUser && buildUser->getUIDCount() != 1)
         #if __linux__
@@ -788,7 +779,7 @@ kj::Promise<Outcome<void, Goal::WorkResult>> LocalDerivationGoal::startBuilder()
         msgs.push_back(std::move(msg));
     }
 
-    return handleChildOutput();
+    return {builderOutPTY.get()};
 }
 
 
@@ -1250,7 +1241,7 @@ void LocalDerivationGoal::startDaemon()
                         NotTrusted, daemon::Recursive);
                     debug("terminated daemon connection");
                 } catch (SysError &) {
-                    ignoreExceptionExceptInterrupt();
+                    ignoreException();
                 }
             });
 
@@ -1370,20 +1361,13 @@ void LocalDerivationGoal::runChild()
 
         bool setUser = true;
 
-        /* Make the contents of netrc and the CA certificate bundle
+        /* Make the contents of netrc available to builtin:fetchurl
-           available to builtin:fetchurl (which may run under a
+           (which may run under a different uid and/or in a sandbox). */
-           different uid and/or in a sandbox). */
         std::string netrcData;
-        std::string caFileData;
+        try {
-        if (drv->isBuiltin() && drv->builder == "builtin:fetchurl" && !derivationType->isSandboxed()) {
+            if (drv->isBuiltin() && drv->builder == "builtin:fetchurl" && !derivationType->isSandboxed())
-            try {
                 netrcData = readFile(settings.netrcFile);
-            } catch (SysError &) { }
+        } catch (SysError &) { }
-
-            try {
-                caFileData = readFile(settings.caFile);
-            } catch (SysError &) { }
-        }
 
 #if __linux__
         if (useChroot) {
@@ -1818,7 +1802,7 @@ void LocalDerivationGoal::runChild()
                     e.second = rewriteStrings(e.second, inputRewrites);
 
                 if (drv->builder == "builtin:fetchurl")
-                    builtinFetchurl(drv2, netrcData, caFileData);
+                    builtinFetchurl(drv2, netrcData);
                 else if (drv->builder == "builtin:buildenv")
                     builtinBuildenv(drv2);
                 else if (drv->builder == "builtin:unpack-channel")

src/libstore/build/local-derivation-goal.hh

@@ -182,7 +182,7 @@ struct LocalDerivationGoal : public DerivationGoal
      * Create a LocalDerivationGoal without an on-disk .drv file,
      * possibly a platform-specific subclass
      */
-    static std::unique_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
+    static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
         const StorePath & drvPath,
         const OutputsSpec & wantedOutputs,
         Worker & worker,
@@ -194,7 +194,7 @@ struct LocalDerivationGoal : public DerivationGoal
      * Create a LocalDerivationGoal for an on-disk .drv file,
      * possibly a platform-specific subclass
      */
-    static std::unique_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
+    static std::shared_ptr<LocalDerivationGoal> makeLocalDerivationGoal(
         const StorePath & drvPath,
         const BasicDerivation & drv,
         const OutputsSpec & wantedOutputs,
@@ -213,12 +213,12 @@ struct LocalDerivationGoal : public DerivationGoal
     /**
      * The additional states.
      */
-    kj::Promise<Result<WorkResult>> tryLocalBuild() noexcept override;
+    kj::Promise<Result<WorkResult>> tryLocalBuild(bool inBuildSlot) noexcept override;
 
     /**
      * Start building a derivation.
      */
-    kj::Promise<Outcome<void, WorkResult>> startBuilder();
+    std::set<int> startBuilder();
 
     /**
      * Fill in the environment for the builder.

src/libstore/build/substitution-goal.cc

@@ -3,8 +3,6 @@
 #include "nar-info.hh"
 #include "signals.hh"
 #include "finally.hh"
-#include <kj/array.h>
-#include <kj/vector.h>
 
 namespace nix {
 
@@ -20,6 +18,7 @@ PathSubstitutionGoal::PathSubstitutionGoal(
     , repair(repair)
     , ca(ca)
 {
+    state = &PathSubstitutionGoal::init;
     name = fmt("substitution of '%s'", worker.store.printStorePath(this->storePath));
     trace("created");
     maintainExpectedSubstitutions = worker.expectedSubstitutions.addTemporarily(1);
@@ -32,21 +31,27 @@ PathSubstitutionGoal::~PathSubstitutionGoal()
 }
 
 
-Goal::WorkResult PathSubstitutionGoal::done(
+Goal::Finished PathSubstitutionGoal::done(
     ExitCode result,
     BuildResult::Status status,
     std::optional<std::string> errorMsg)
 {
-    BuildResult buildResult{.status = status};
+    buildResult.status = status;
     if (errorMsg) {
         debug(*errorMsg);
         buildResult.errorMsg = *errorMsg;
     }
-    return WorkResult{result, std::move(buildResult)};
+    return Finished{result, std::move(buildResult)};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::workImpl() noexcept
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::work(bool inBuildSlot) noexcept
+{
+    return (this->*state)(inBuildSlot);
+}
+
+
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::init(bool inBuildSlot) noexcept
 try {
     trace("init");
 
@@ -62,13 +67,13 @@ try {
 
     subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>();
 
-    return tryNext();
+    return tryNext(inBuildSlot);
 } catch (...) {
     return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::tryNext() noexcept
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::tryNext(bool inBuildSlot) noexcept
 try {
     trace("trying next substituter");
 
@@ -84,10 +89,10 @@ try {
         /* Hack: don't indicate failure if there were no substituters.
            In that case the calling derivation should just do a
            build. */
-        co_return done(
+        return {done(
             substituterFailed ? ecFailed : ecNoSubstituters,
             BuildResult::NoSubstituters,
-            fmt("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath)));
+            fmt("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath)))};
     }
 
     sub = subs.front();
@@ -100,28 +105,26 @@ try {
         if (sub->storeDir == worker.store.storeDir)
             assert(subPath == storePath);
     } else if (sub->storeDir != worker.store.storeDir) {
-        co_return co_await tryNext();
+        return tryNext(inBuildSlot);
     }
 
-    do {
+    try {
-        try {
+        // FIXME: make async
-            // FIXME: make async
+        info = sub->queryPathInfo(subPath ? *subPath : storePath);
-            info = sub->queryPathInfo(subPath ? *subPath : storePath);
+    } catch (InvalidPath &) {
-            break;
+        return tryNext(inBuildSlot);
-        } catch (InvalidPath &) {
+    } catch (SubstituterDisabled &) {
-        } catch (SubstituterDisabled &) {
+        if (settings.tryFallback) {
-            if (!settings.tryFallback) {
+            return tryNext(inBuildSlot);
-                throw;
-            }
-        } catch (Error & e) {
-            if (settings.tryFallback) {
-                logError(e.info());
-            } else {
-                throw;
-            }
         }
-        co_return co_await tryNext();
+        throw;
-    } while (false);
+    } catch (Error & e) {
+        if (settings.tryFallback) {
+            logError(e.info());
+            return tryNext(inBuildSlot);
+        }
+        throw;
+    }
 
     if (info->path != storePath) {
         if (info->isContentAddressed(*sub) && info->references.empty()) {
@@ -131,7 +134,7 @@ try {
         } else {
             printError("asked '%s' for '%s' but got '%s'",
                 sub->getUri(), worker.store.printStorePath(storePath), sub->printStorePath(info->path));
-            co_return co_await tryNext();
+            return tryNext(inBuildSlot);
         }
     }
 
@@ -152,26 +155,28 @@ try {
     {
         warn("ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'",
             worker.store.printStorePath(storePath), sub->getUri());
-        co_return co_await tryNext();
+        return tryNext(inBuildSlot);
     }
 
     /* To maintain the closure invariant, we first have to realise the
        paths referenced by this one. */
-    kj::Vector<std::pair<GoalPtr, kj::Promise<Result<WorkResult>>>> dependencies;
+    WaitForGoals result;
     for (auto & i : info->references)
         if (i != storePath) /* ignore self-references */
-            dependencies.add(worker.goalFactory().makePathSubstitutionGoal(i));
+            result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i));
 
-    if (!dependencies.empty()) {/* to prevent hang (no wake-up event) */
+    if (result.goals.empty()) {/* to prevent hang (no wake-up event) */
-        (co_await waitForGoals(dependencies.releaseAsArray())).value();
+        return referencesValid(inBuildSlot);
+    } else {
+        state = &PathSubstitutionGoal::referencesValid;
+        return {std::move(result)};
     }
-    co_return co_await referencesValid();
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::referencesValid() noexcept
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::referencesValid(bool inBuildSlot) noexcept
 try {
     trace("all references realised");
 
@@ -186,33 +191,33 @@ try {
         if (i != storePath) /* ignore self-references */
             assert(worker.store.isValidPath(i));
 
-    return tryToRun();
+    state = &PathSubstitutionGoal::tryToRun;
+    return tryToRun(inBuildSlot);
 } catch (...) {
     return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::tryToRun() noexcept
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::tryToRun(bool inBuildSlot) noexcept
 try {
     trace("trying to run");
 
-    if (!slotToken.valid()) {
+    if (!inBuildSlot) {
-        slotToken = co_await worker.substitutions.acquire();
+        return {WaitForSlot{}};
     }
 
     maintainRunningSubstitutions = worker.runningSubstitutions.addTemporarily(1);
 
-    auto pipe = kj::newPromiseAndCrossThreadFulfiller<void>();
+    outPipe.create();
-    outPipe = kj::mv(pipe.fulfiller);
 
     thr = std::async(std::launch::async, [this]() {
-        /* Wake up the worker loop when we're done. */
-        Finally updateStats([this]() { outPipe->fulfill(); });
-
         auto & fetchPath = subPath ? *subPath : storePath;
         try {
             ReceiveInterrupts receiveInterrupts;
 
+            /* Wake up the worker loop when we're done. */
+            Finally updateStats([this]() { outPipe.writeSide.close(); });
+
             Activity act(*logger, actSubstitute, Logger::Fields{worker.store.printStorePath(storePath), sub->getUri()});
             PushActivity pact(act.id);
 
@@ -228,39 +233,39 @@ try {
         }
     });
 
-    co_await pipe.promise;
+    state = &PathSubstitutionGoal::finished;
-    co_return co_await finished();
+    return {WaitForWorld{{outPipe.readSide.get()}, true}};
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
 }
 
 
-kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::finished() noexcept
+kj::Promise<Result<Goal::WorkResult>> PathSubstitutionGoal::finished(bool inBuildSlot) noexcept
 try {
     trace("substitute finished");
 
-    do {
+    worker.childTerminated(this);
-        try {
-            slotToken = {};
-            thr.get();
-            break;
-        } catch (std::exception & e) {
-            printError(e.what());
 
-            /* Cause the parent build to fail unless --fallback is given,
+    try {
-               or the substitute has disappeared. The latter case behaves
+        thr.get();
-               the same as the substitute never having existed in the
+    } catch (std::exception & e) {
-               first place. */
+        printError(e.what());
-            try {
+
-                throw;
+        /* Cause the parent build to fail unless --fallback is given,
-            } catch (SubstituteGone &) {
+           or the substitute has disappeared. The latter case behaves
-            } catch (...) {
+           the same as the substitute never having existed in the
-                substituterFailed = true;
+           first place. */
-            }
+        try {
+            throw;
+        } catch (SubstituteGone &) {
+        } catch (...) {
+            substituterFailed = true;
         }
+
         /* Try the next substitute. */
-        co_return co_await tryNext();
+        state = &PathSubstitutionGoal::tryNext;
-    } while (false);
+        return tryNext(inBuildSlot);
+    }
 
     worker.markContentsGood(storePath);
 
@@ -277,9 +282,15 @@ try {
     worker.doneNarSize += maintainExpectedNar.delta();
     maintainExpectedNar.reset();
 
-    co_return done(ecSuccess, BuildResult::Substituted);
+    return {done(ecSuccess, BuildResult::Substituted)};
 } catch (...) {
-    co_return result::failure(std::current_exception());
+    return {std::current_exception()};
+}
+
+
+Goal::WorkResult PathSubstitutionGoal::handleChildOutput(int fd, std::string_view data)
+{
+    return StillAlive{};
 }
 
 
@@ -289,9 +300,12 @@ void PathSubstitutionGoal::cleanup()
         if (thr.valid()) {
             // FIXME: signal worker thread to quit.
             thr.get();
+            worker.childTerminated(this);
         }
+
+        outPipe.close();
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libstore/build/substitution-goal.hh

@@ -46,7 +46,7 @@ struct PathSubstitutionGoal : public Goal
     /**
      * Pipe for the substituter's standard output.
      */
-    kj::Own<kj::CrossThreadPromiseFulfiller<void>> outPipe;
+    Pipe outPipe;
 
     /**
      * The substituter thread.
@@ -67,12 +67,15 @@ struct PathSubstitutionGoal : public Goal
     NotifyingCounter<uint64_t>::Bump maintainExpectedSubstitutions,
         maintainRunningSubstitutions, maintainExpectedNar, maintainExpectedDownload;
 
+    typedef kj::Promise<Result<WorkResult>> (PathSubstitutionGoal::*GoalState)(bool inBuildSlot) noexcept;
+    GoalState state;
+
     /**
      * Content address for recomputing store path
      */
     std::optional<ContentAddress> ca;
 
-    WorkResult done(
+    Finished done(
         ExitCode result,
         BuildResult::Status status,
         std::optional<std::string> errorMsg = {});
@@ -87,15 +90,32 @@ public:
     );
     ~PathSubstitutionGoal();
 
-    kj::Promise<Result<WorkResult>> workImpl() noexcept override;
+    Finished timedOut(Error && ex) override { abort(); };
+
+    /**
+     * We prepend "a$" to the key name to ensure substitution goals
+     * happen before derivation goals.
+     */
+    std::string key() override
+    {
+        return "a$" + std::string(storePath.name()) + "$" + worker.store.printStorePath(storePath);
+    }
+
+    kj::Promise<Result<WorkResult>> work(bool inBuildSlot) noexcept override;
 
     /**
      * The states.
      */
-    kj::Promise<Result<WorkResult>> tryNext() noexcept;
+    kj::Promise<Result<WorkResult>> init(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> referencesValid() noexcept;
+    kj::Promise<Result<WorkResult>> tryNext(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> tryToRun() noexcept;
+    kj::Promise<Result<WorkResult>> referencesValid(bool inBuildSlot) noexcept;
-    kj::Promise<Result<WorkResult>> finished() noexcept;
+    kj::Promise<Result<WorkResult>> tryToRun(bool inBuildSlot) noexcept;
+    kj::Promise<Result<WorkResult>> finished(bool inBuildSlot) noexcept;
+
+    /**
+     * Callback used by the worker to write to the log.
+     */
+    WorkResult handleChildOutput(int fd, std::string_view data) override;
 
     /* Called by destructor, can't be overridden */
     void cleanup() override final;

src/libstore/build/worker.cc

@@ -1,4 +1,3 @@
-#include "async-collect.hh"
 #include "charptr-cast.hh"
 #include "worker.hh"
 #include "finally.hh"
@@ -7,22 +6,11 @@
 #include "local-derivation-goal.hh"
 #include "signals.hh"
 #include "hook-instance.hh" // IWYU pragma: keep
-#include <boost/outcome/try.hpp>
+
-#include <kj/vector.h>
+#include <poll.h>
 
 namespace nix {
 
-namespace {
-struct ErrorHandler : kj::TaskSet::ErrorHandler
-{
-    void taskFailed(kj::Exception && e) override
-    {
-        printError("unexpected async failure in Worker: %s", kj::str(e).cStr());
-        abort();
-    }
-} errorHandler;
-}
-
 Worker::Worker(Store & store, Store & evalStore, kj::AsyncIoContext & aio)
     : act(*logger, actRealise)
     , actDerivations(*logger, actBuilds)
@@ -30,13 +18,11 @@ Worker::Worker(Store & store, Store & evalStore, kj::AsyncIoContext & aio)
     , store(store)
     , evalStore(evalStore)
     , aio(aio)
-      /* Make sure that we are always allowed to run at least one substitution.
-         This prevents infinite waiting. */
-    , substitutions(std::max<unsigned>(1, settings.maxSubstitutionJobs))
-    , localBuilds(settings.maxBuildJobs)
-    , children(errorHandler)
 {
     /* Debugging: prevent recursive workers. */
+    nrLocalBuilds = 0;
+    nrSubstitutions = 0;
+    lastWokenUp = steady_time_point::min();
 }
 
 
@@ -46,11 +32,7 @@ Worker::~Worker()
        goals that refer to this worker should be gone.  (Otherwise we
        are in trouble, since goals may call childTerminated() etc. in
        their destructors). */
-    children.clear();
+    topGoals.clear();
-
-    derivationGoals.clear();
-    drvOutputSubstitutionGoals.clear();
-    substitutionGoals.clear();
 
     assert(expectedSubstitutions == 0);
     assert(expectedDownloadSize == 0);
@@ -58,158 +40,292 @@ Worker::~Worker()
 }
 
 
-template<typename ID, std::derived_from<Goal> G>
+std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
-std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoalCommon(
-    std::map<ID, CachedGoal<G>> & map,
-    const ID & key,
-    InvocableR<std::unique_ptr<G>> auto create,
-    InvocableR<bool, G &> auto modify
-)
-{
-    auto [it, _inserted] = map.try_emplace(key);
-    // try twice to create the goal. we can only loop if we hit the continue,
-    // and then we only want to recreate the goal *once*. concurrent accesses
-    // to the worker are not sound, we want to catch them if at all possible.
-    for ([[maybe_unused]] auto _attempt : {1, 2}) {
-        auto & cachedGoal = it->second;
-        auto & goal = cachedGoal.goal;
-        if (!goal) {
-            goal = create();
-            // do not start working immediately. if we are not yet running we
-            // may create dependencies as though they were toplevel goals, in
-            // which case the dependencies will not report build errors. when
-            // we are running we may be called for this same goal more times,
-            // and then we want to modify rather than recreate when possible.
-            auto removeWhenDone = [goal, &map, it] {
-                // c++ lambda coroutine capture semantics are *so* fucked up.
-                return [](auto goal, auto & map, auto it) -> kj::Promise<Result<Goal::WorkResult>> {
-                    auto result = co_await goal->work();
-                    // a concurrent call to makeGoalCommon may have reset our
-                    // cached goal and replaced it with a new instance. don't
-                    // remove the goal in this case, otherwise we will crash.
-                    if (goal == it->second.goal) {
-                        map.erase(it);
-                    }
-                    co_return result;
-                }(goal, map, it);
-            };
-            cachedGoal.promise = kj::evalLater(std::move(removeWhenDone)).fork();
-            children.add(cachedGoal.promise.addBranch().then([this](auto _result) {
-                if (_result.has_value()) {
-                    auto & result = _result.value();
-                    permanentFailure |= result.permanentFailure;
-                    timedOut |= result.timedOut;
-                    hashMismatch |= result.hashMismatch;
-                    checkMismatch |= result.checkMismatch;
-                }
-            }));
-        } else {
-            if (!modify(*goal)) {
-                cachedGoal = {};
-                continue;
-            }
-        }
-        return {goal, cachedGoal.promise.addBranch()};
-    }
-    assert(false && "could not make a goal. possible concurrent worker access");
-}
-
-
-std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeDerivationGoal(
-    const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode
-)
-{
-    return makeGoalCommon(
-        derivationGoals,
-        drvPath,
-        [&]() -> std::unique_ptr<DerivationGoal> {
-            return !dynamic_cast<LocalStore *>(&store)
-                ? std::make_unique<DerivationGoal>(
-                    drvPath, wantedOutputs, *this, running, buildMode
-                )
-                : LocalDerivationGoal::makeLocalDerivationGoal(
-                    drvPath, wantedOutputs, *this, running, buildMode
-                );
-        },
-        [&](DerivationGoal & g) { return g.addWantedOutputs(wantedOutputs); }
-    );
-}
-
-
-std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> Worker::makeBasicDerivationGoal(
     const StorePath & drvPath,
-    const BasicDerivation & drv,
     const OutputsSpec & wantedOutputs,
-    BuildMode buildMode
+    std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal)
-)
 {
-    return makeGoalCommon(
+    std::weak_ptr<DerivationGoal> & goal_weak = derivationGoals[drvPath];
-        derivationGoals,
+    std::shared_ptr<DerivationGoal> goal = goal_weak.lock();
+    if (!goal) {
+        goal = mkDrvGoal();
+        goal_weak = goal;
+        wakeUp(goal);
+    } else {
+        goal->addWantedOutputs(wantedOutputs);
+    }
+    return goal;
+}
+
+
+std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath,
+    const OutputsSpec & wantedOutputs, BuildMode buildMode)
+{
+    return makeDerivationGoalCommon(
         drvPath,
-        [&]() -> std::unique_ptr<DerivationGoal> {
+        wantedOutputs,
+        [&]() -> std::shared_ptr<DerivationGoal> {
             return !dynamic_cast<LocalStore *>(&store)
-                ? std::make_unique<DerivationGoal>(
+                ? std::make_shared<DerivationGoal>(
+                    drvPath, wantedOutputs, *this, running, buildMode
+                )
+                : LocalDerivationGoal::makeLocalDerivationGoal(
+                    drvPath, wantedOutputs, *this, running, buildMode
+                );
+        }
+    );
+}
+
+
+std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath,
+    const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode)
+{
+    return makeDerivationGoalCommon(
+        drvPath,
+        wantedOutputs,
+        [&]() -> std::shared_ptr<DerivationGoal> {
+            return !dynamic_cast<LocalStore *>(&store)
+                ? std::make_shared<DerivationGoal>(
                     drvPath, drv, wantedOutputs, *this, running, buildMode
                 )
                 : LocalDerivationGoal::makeLocalDerivationGoal(
                     drvPath, drv, wantedOutputs, *this, running, buildMode
                 );
-        },
+        }
-        [&](DerivationGoal & g) { return g.addWantedOutputs(wantedOutputs); }
     );
 }
 
 
-std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
+std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca)
-Worker::makePathSubstitutionGoal(
-    const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca
-)
 {
-    return makeGoalCommon(
+    std::weak_ptr<PathSubstitutionGoal> & goal_weak = substitutionGoals[path];
-        substitutionGoals,
+    auto goal = goal_weak.lock(); // FIXME
-        path,
+    if (!goal) {
-        [&] { return std::make_unique<PathSubstitutionGoal>(path, *this, running, repair, ca); },
+        goal = std::make_shared<PathSubstitutionGoal>(path, *this, running, repair, ca);
-        [&](auto &) { return true; }
+        goal_weak = goal;
-    );
+        wakeUp(goal);
+    }
+    return goal;
 }
 
 
-std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
+std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal(const DrvOutput& id, RepairFlag repair, std::optional<ContentAddress> ca)
-Worker::makeDrvOutputSubstitutionGoal(
-    const DrvOutput & id, RepairFlag repair, std::optional<ContentAddress> ca
-)
 {
-    return makeGoalCommon(
+    std::weak_ptr<DrvOutputSubstitutionGoal> & goal_weak = drvOutputSubstitutionGoals[id];
-        drvOutputSubstitutionGoals,
+    auto goal = goal_weak.lock(); // FIXME
-        id,
+    if (!goal) {
-        [&] { return std::make_unique<DrvOutputSubstitutionGoal>(id, *this, running, repair, ca); },
+        goal = std::make_shared<DrvOutputSubstitutionGoal>(id, *this, running, repair, ca);
-        [&](auto &) { return true; }
+        goal_weak = goal;
-    );
+        wakeUp(goal);
+    }
+    return goal;
 }
 
 
-std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
+GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
 {
     return std::visit(overloaded {
-        [&](const DerivedPath::Built & bfd) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> {
+        [&](const DerivedPath::Built & bfd) -> GoalPtr {
             if (auto bop = std::get_if<DerivedPath::Opaque>(&*bfd.drvPath))
                 return makeDerivationGoal(bop->path, bfd.outputs, buildMode);
             else
                 throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented.");
         },
-        [&](const DerivedPath::Opaque & bo) -> std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>> {
+        [&](const DerivedPath::Opaque & bo) -> GoalPtr {
             return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair);
         },
     }, req.raw());
 }
 
-kj::Promise<Result<Worker::Results>> Worker::updateStatistics()
-try {
-    while (true) {
-        statisticsUpdateInhibitor = co_await statisticsUpdateSignal.acquire();
 
-        // only update progress info while running. this notably excludes updating
+template<typename K, typename G>
-        // progress info while destroying, which causes the progress bar to assert
+static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> & goalMap)
+{
+    /* !!! inefficient */
+    for (auto i = goalMap.begin();
+         i != goalMap.end(); )
+        if (i->second.lock() == goal) {
+            auto j = i; ++j;
+            goalMap.erase(i);
+            i = j;
+        }
+        else ++i;
+}
+
+
+void Worker::goalFinished(GoalPtr goal, Goal::Finished & f)
+{
+    goal->trace("done");
+    assert(!goal->exitCode.has_value());
+    goal->exitCode = f.exitCode;
+    goal->ex = f.ex;
+
+    permanentFailure |= f.permanentFailure;
+    timedOut |= f.timedOut;
+    hashMismatch |= f.hashMismatch;
+    checkMismatch |= f.checkMismatch;
+
+    for (auto & i : goal->waiters) {
+        if (GoalPtr waiting = i.lock()) {
+            assert(waiting->waitees.count(goal));
+            waiting->waitees.erase(goal);
+
+            waiting->trace(fmt("waitee '%s' done; %d left", goal->name, waiting->waitees.size()));
+
+            if (f.exitCode != Goal::ecSuccess) ++waiting->nrFailed;
+            if (f.exitCode == Goal::ecNoSubstituters) ++waiting->nrNoSubstituters;
+            if (f.exitCode == Goal::ecIncompleteClosure) ++waiting->nrIncompleteClosure;
+
+            if (waiting->waitees.empty() || (f.exitCode == Goal::ecFailed && !settings.keepGoing)) {
+                /* If we failed and keepGoing is not set, we remove all
+                   remaining waitees. */
+                for (auto & i : waiting->waitees) {
+                    i->waiters.extract(waiting);
+                }
+                waiting->waitees.clear();
+
+                wakeUp(waiting);
+            }
+
+            waiting->waiteeDone(goal);
+        }
+    }
+    goal->waiters.clear();
+    removeGoal(goal);
+    goal->cleanup();
+}
+
+void Worker::handleWorkResult(GoalPtr goal, Goal::WorkResult how)
+{
+    std::visit(
+        overloaded{
+            [&](Goal::StillAlive) {},
+            [&](Goal::WaitForSlot) { waitForBuildSlot(goal); },
+            [&](Goal::WaitForAWhile) { waitForAWhile(goal); },
+            [&](Goal::ContinueImmediately) { wakeUp(goal); },
+            [&](Goal::WaitForGoals & w) {
+                for (auto & dep : w.goals) {
+                    goal->waitees.insert(dep);
+                    dep->waiters.insert(goal);
+                }
+            },
+            [&](Goal::WaitForWorld & w) { childStarted(goal, w.fds, w.inBuildSlot); },
+            [&](Goal::Finished & f) { goalFinished(goal, f); },
+        },
+        how
+    );
+}
+
+void Worker::removeGoal(GoalPtr goal)
+{
+    if (auto drvGoal = std::dynamic_pointer_cast<DerivationGoal>(goal))
+        nix::removeGoal(drvGoal, derivationGoals);
+    else if (auto subGoal = std::dynamic_pointer_cast<PathSubstitutionGoal>(goal))
+        nix::removeGoal(subGoal, substitutionGoals);
+    else if (auto subGoal = std::dynamic_pointer_cast<DrvOutputSubstitutionGoal>(goal))
+        nix::removeGoal(subGoal, drvOutputSubstitutionGoals);
+    else
+        assert(false);
+
+    if (topGoals.find(goal) != topGoals.end()) {
+        topGoals.erase(goal);
+        /* If a top-level goal failed, then kill all other goals
+           (unless keepGoing was set). */
+        if (goal->exitCode == Goal::ecFailed && !settings.keepGoing)
+            topGoals.clear();
+    }
+}
+
+
+void Worker::wakeUp(GoalPtr goal)
+{
+    goal->trace("woken up");
+    awake.insert(goal);
+}
+
+
+void Worker::childStarted(GoalPtr goal, const std::set<int> & fds,
+    bool inBuildSlot)
+{
+    Child child;
+    child.goal = goal;
+    child.goal2 = goal.get();
+    child.fds = fds;
+    child.timeStarted = child.lastOutput = steady_time_point::clock::now();
+    child.inBuildSlot = inBuildSlot;
+    children.emplace_back(child);
+    if (inBuildSlot) {
+        switch (goal->jobCategory()) {
+        case JobCategory::Substitution:
+            nrSubstitutions++;
+            break;
+        case JobCategory::Build:
+            nrLocalBuilds++;
+            break;
+        default:
+            abort();
+        }
+    }
+}
+
+
+void Worker::childTerminated(Goal * goal)
+{
+    auto i = std::find_if(children.begin(), children.end(),
+        [&](const Child & child) { return child.goal2 == goal; });
+    if (i == children.end()) return;
+
+    if (i->inBuildSlot) {
+        switch (goal->jobCategory()) {
+        case JobCategory::Substitution:
+            assert(nrSubstitutions > 0);
+            nrSubstitutions--;
+            break;
+        case JobCategory::Build:
+            assert(nrLocalBuilds > 0);
+            nrLocalBuilds--;
+            break;
+        default:
+            abort();
+        }
+    }
+
+    children.erase(i);
+
+    /* Wake up goals waiting for a build slot. */
+    for (auto & j : wantingToBuild) {
+        GoalPtr goal = j.lock();
+        if (goal) wakeUp(goal);
+    }
+
+    wantingToBuild.clear();
+}
+
+
+void Worker::waitForBuildSlot(GoalPtr goal)
+{
+    goal->trace("wait for build slot");
+    bool isSubstitutionGoal = goal->jobCategory() == JobCategory::Substitution;
+    if ((!isSubstitutionGoal && nrLocalBuilds < settings.maxBuildJobs) ||
+        (isSubstitutionGoal && nrSubstitutions < settings.maxSubstitutionJobs))
+        wakeUp(goal); /* we can do it right away */
+    else
+        wantingToBuild.insert(goal);
+}
+
+
+void Worker::waitForAWhile(GoalPtr goal)
+{
+    debug("wait for a while");
+    waitingForAWhile.insert(goal);
+}
+
+
+void Worker::updateStatistics()
+{
+    // only update progress info while running. this notably excludes updating
+    // progress info while destroying, which causes the progress bar to assert
+    if (running && statisticsOutdated) {
         actDerivations.progress(
             doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds
         );
@@ -222,82 +338,221 @@ try {
         act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize);
         act.setExpected(actCopyPath, expectedNarSize + doneNarSize);
 
-        // limit to 50fps. that should be more than good enough for anything we do
+        statisticsOutdated = false;
-        co_await aio.provider->getTimer().afterDelay(20 * kj::MILLISECONDS);
     }
-} catch (...) {
-    co_return result::failure(std::current_exception());
 }
 
-Worker::Results Worker::run(std::function<Targets (GoalFactory &)> req)
+Goals Worker::run(std::function<Goals (GoalFactory &)> req)
 {
-    auto topGoals = req(goalFactory());
+    auto _topGoals = req(goalFactory());
 
     assert(!running);
     running = true;
     Finally const _stop([&] { running = false; });
 
-    auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller<Result<Results>>();
+    updateStatistics();
-    auto interruptCallback = createInterruptCallback([&] {
-        return result::failure(std::make_exception_ptr(makeInterrupted()));
-    });
 
-    auto promise = runImpl(std::move(topGoals))
+    topGoals = _topGoals;
-                       .exclusiveJoin(updateStatistics())
-                       .exclusiveJoin(std::move(onInterrupt.promise));
 
-    // TODO GC interface?
-    if (auto localStore = dynamic_cast<LocalStore *>(&store); localStore && settings.minFree != 0) {
-        // Periodically wake up to see if we need to run the garbage collector.
-        promise = promise.exclusiveJoin(boopGC(*localStore));
-    }
-
-    return promise.wait(aio.waitScope).value();
-}
-
-kj::Promise<Result<Worker::Results>> Worker::runImpl(Targets topGoals)
-try {
     debug("entered goal loop");
 
-    kj::Vector<Targets::value_type> promises(topGoals.size());
+    while (1) {
-    for (auto & gp : topGoals) {
-        promises.add(std::move(gp));
-    }
 
-    Results results;
+        checkInterrupt();
 
-    auto collect = AsyncCollect(promises.releaseAsArray());
+        // TODO GC interface?
-    while (auto done = co_await collect.next()) {
+        if (auto localStore = dynamic_cast<LocalStore *>(&store))
-        // propagate goal exceptions outward
+            localStore->autoGC(false);
-        BOOST_OUTCOME_CO_TRY(auto result, done->second);
-        results.emplace(done->first, result);
 
-        /* If a top-level goal failed, then kill all other goals
+        /* Call every wake goal (in the ordering established by
-           (unless keepGoing was set). */
+           CompareGoalPtrs). */
-        if (result.exitCode == Goal::ecFailed && !settings.keepGoing) {
+        while (!awake.empty() && !topGoals.empty()) {
-            children.clear();
+            Goals awake2;
-            break;
+            for (auto & i : awake) {
+                GoalPtr goal = i.lock();
+                if (goal) awake2.insert(goal);
+            }
+            awake.clear();
+            for (auto & goal : awake2) {
+                checkInterrupt();
+                /* Make sure that we are always allowed to run at least one substitution.
+                   This prevents infinite waiting. */
+                const bool inSlot = goal->jobCategory() == JobCategory::Substitution
+                    ? nrSubstitutions < std::max(1U, (unsigned int) settings.maxSubstitutionJobs)
+                    : nrLocalBuilds < settings.maxBuildJobs;
+                handleWorkResult(goal, goal->work(inSlot).wait(aio.waitScope).value());
+                updateStatistics();
+
+                if (topGoals.empty()) break; // stuff may have been cancelled
+            }
+        }
+
+        if (topGoals.empty()) break;
+
+        /* Wait for input. */
+        if (!children.empty() || !waitingForAWhile.empty())
+            waitForInput();
+        else {
+            assert(!awake.empty());
         }
     }
 
     /* If --keep-going is not set, it's possible that the main goal
        exited while some of its subgoals were still active.  But if
        --keep-going *is* set, then they must all be finished now. */
-    assert(!settings.keepGoing || children.isEmpty());
+    assert(!settings.keepGoing || awake.empty());
+    assert(!settings.keepGoing || wantingToBuild.empty());
+    assert(!settings.keepGoing || children.empty());
 
-    co_return std::move(results);
+    return _topGoals;
-} catch (...) {
-    co_return result::failure(std::current_exception());
 }
 
-kj::Promise<Result<Worker::Results>> Worker::boopGC(LocalStore & localStore)
+void Worker::waitForInput()
-try {
+{
-    while (true) {
+    printMsg(lvlVomit, "waiting for children");
-        co_await aio.provider->getTimer().afterDelay(10 * kj::SECONDS);
+
-        localStore.autoGC(false);
+    /* Process output from the file descriptors attached to the
+       children, namely log output and output path creation commands.
+       We also use this to detect child termination: if we get EOF on
+       the logger pipe of a build, we assume that the builder has
+       terminated. */
+
+    bool useTimeout = false;
+    long timeout = 0;
+    auto before = steady_time_point::clock::now();
+
+    /* If we're monitoring for silence on stdout/stderr, or if there
+       is a build timeout, then wait for input until the first
+       deadline for any child. */
+    auto nearest = steady_time_point::max(); // nearest deadline
+    if (settings.minFree.get() != 0)
+        // Periodicallty wake up to see if we need to run the garbage collector.
+        nearest = before + std::chrono::seconds(10);
+    for (auto & i : children) {
+        if (auto goal = i.goal.lock()) {
+            if (!goal->respectsTimeouts()) continue;
+            if (0 != settings.maxSilentTime)
+                nearest = std::min(nearest, i.lastOutput + std::chrono::seconds(settings.maxSilentTime));
+            if (0 != settings.buildTimeout)
+                nearest = std::min(nearest, i.timeStarted + std::chrono::seconds(settings.buildTimeout));
+        }
+    }
+    if (nearest != steady_time_point::max()) {
+        timeout = std::max(1L, (long) std::chrono::duration_cast<std::chrono::seconds>(nearest - before).count());
+        useTimeout = true;
+    }
+
+    /* If we are polling goals that are waiting for a lock, then wake
+       up after a few seconds at most. */
+    if (!waitingForAWhile.empty()) {
+        useTimeout = true;
+        if (lastWokenUp == steady_time_point::min() || lastWokenUp > before) lastWokenUp = before;
+        timeout = std::max(1L,
+            (long) std::chrono::duration_cast<std::chrono::seconds>(
+                lastWokenUp + std::chrono::seconds(settings.pollInterval) - before).count());
+    } else lastWokenUp = steady_time_point::min();
+
+    if (useTimeout)
+        vomit("sleeping %d seconds", timeout);
+
+    /* Use select() to wait for the input side of any logger pipe to
+       become `available'.  Note that `available' (i.e., non-blocking)
+       includes EOF. */
+    std::vector<struct pollfd> pollStatus;
+    std::map<int, size_t> fdToPollStatus;
+    for (auto & i : children) {
+        for (auto & j : i.fds) {
+            pollStatus.push_back((struct pollfd) { .fd = j, .events = POLLIN });
+            fdToPollStatus[j] = pollStatus.size() - 1;
+        }
+    }
+
+    if (poll(pollStatus.data(), pollStatus.size(),
+            useTimeout ? timeout * 1000 : -1) == -1) {
+        if (errno == EINTR) return;
+        throw SysError("waiting for input");
+    }
+
+    auto after = steady_time_point::clock::now();
+
+    /* Process all available file descriptors. FIXME: this is
+       O(children * fds). */
+    decltype(children)::iterator i;
+    for (auto j = children.begin(); j != children.end(); j = i) {
+        i = std::next(j);
+
+        checkInterrupt();
+
+        GoalPtr goal = j->goal.lock();
+        assert(goal);
+
+        if (!goal->exitCode.has_value() &&
+            0 != settings.maxSilentTime &&
+            goal->respectsTimeouts() &&
+            after - j->lastOutput >= std::chrono::seconds(settings.maxSilentTime))
+        {
+            handleWorkResult(
+                goal,
+                goal->timedOut(Error(
+                    "%1% timed out after %2% seconds of silence",
+                    goal->getName(),
+                    settings.maxSilentTime
+                ))
+            );
+            continue;
+        }
+
+        else if (!goal->exitCode.has_value() &&
+            0 != settings.buildTimeout &&
+            goal->respectsTimeouts() &&
+            after - j->timeStarted >= std::chrono::seconds(settings.buildTimeout))
+        {
+            handleWorkResult(
+                goal,
+                goal->timedOut(
+                    Error("%1% timed out after %2% seconds", goal->getName(), settings.buildTimeout)
+                )
+            );
+            continue;
+        }
+
+        std::set<int> fds2(j->fds);
+        std::vector<unsigned char> buffer(4096);
+        for (auto & k : fds2) {
+            const auto fdPollStatusId = get(fdToPollStatus, k);
+            assert(fdPollStatusId);
+            assert(*fdPollStatusId < pollStatus.size());
+            if (pollStatus.at(*fdPollStatusId).revents) {
+                ssize_t rd = ::read(k, buffer.data(), buffer.size());
+                // FIXME: is there a cleaner way to handle pt close
+                // than EIO? Is this even standard?
+                if (rd == 0 || (rd == -1 && errno == EIO)) {
+                    debug("%1%: got EOF", goal->getName());
+                    goal->handleEOF(k);
+                    handleWorkResult(goal, Goal::ContinueImmediately{});
+                    j->fds.erase(k);
+                } else if (rd == -1) {
+                    if (errno != EINTR)
+                        throw SysError("%s: read failed", goal->getName());
+                } else {
+                    printMsg(lvlVomit, "%1%: read %2% bytes",
+                        goal->getName(), rd);
+                    std::string_view data(charptr_cast<char *>(buffer.data()), rd);
+                    j->lastOutput = after;
+                    handleWorkResult(goal, goal->handleChildOutput(k, data));
+                }
+            }
+        }
+    }
+
+    if (!waitingForAWhile.empty() && lastWokenUp + std::chrono::seconds(settings.pollInterval) <= after) {
+        lastWokenUp = after;
+        for (auto & i : waitingForAWhile) {
+            GoalPtr goal = i.lock();
+            if (goal) wakeUp(goal);
+        }
+        waitingForAWhile.clear();
     }
-} catch (...) {
-    co_return result::failure(std::current_exception());
 }
 
 

src/libstore/build/worker.hh

@@ -1,8 +1,6 @@
 #pragma once
 ///@file
 
-#include "async-semaphore.hh"
-#include "concepts.hh"
 #include "notifying-counter.hh"
 #include "types.hh"
 #include "lock.hh"
@@ -20,22 +18,37 @@ namespace nix {
 struct DerivationGoal;
 struct PathSubstitutionGoal;
 class DrvOutputSubstitutionGoal;
-class LocalStore;
 
 typedef std::chrono::time_point<std::chrono::steady_clock> steady_time_point;
 
+/**
+ * A mapping used to remember for each child process to what goal it
+ * belongs, and file descriptors for receiving log data and output
+ * path creation commands.
+ */
+struct Child
+{
+    WeakGoalPtr goal;
+    Goal * goal2; // ugly hackery
+    std::set<int> fds;
+    bool inBuildSlot;
+    /**
+     * Time we last got output on stdout/stderr
+     */
+    steady_time_point lastOutput;
+    steady_time_point timeStarted;
+};
+
 /* Forward definition. */
 struct HookInstance;
 
 class GoalFactory
 {
 public:
-    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    virtual std::shared_ptr<DerivationGoal> makeDerivationGoal(
-    makeDerivationGoal(
         const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal
     ) = 0;
-    virtual std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    virtual std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(
-    makeBasicDerivationGoal(
         const StorePath & drvPath,
         const BasicDerivation & drv,
         const OutputsSpec & wantedOutputs,
@@ -45,14 +58,12 @@ public:
     /**
      * @ref SubstitutionGoal "substitution goal"
      */
-    virtual std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    virtual std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(
-    makePathSubstitutionGoal(
         const StorePath & storePath,
         RepairFlag repair = NoRepair,
         std::optional<ContentAddress> ca = std::nullopt
     ) = 0;
-    virtual std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    virtual std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(
-    makeDrvOutputSubstitutionGoal(
         const DrvOutput & id,
         RepairFlag repair = NoRepair,
         std::optional<ContentAddress> ca = std::nullopt
@@ -64,8 +75,7 @@ public:
      * It will be a `DerivationGoal` for a `DerivedPath::Built` or
      * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
      */
-    virtual std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>
+    virtual GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0;
-    makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0;
 };
 
 // elaborate hoax to let goals access factory methods while hiding them from the public
@@ -84,27 +94,61 @@ protected:
  */
 class Worker : public WorkerBase
 {
-public:
-    using Targets = std::map<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>;
-    using Results = std::map<GoalPtr, Goal::WorkResult>;
-
 private:
 
     bool running = false;
 
-    template<typename G>
+    /* Note: the worker should only have strong pointers to the
-    struct CachedGoal
+       top-level goals. */
-    {
+
-        std::shared_ptr<G> goal;
+    /**
-        kj::ForkedPromise<Result<Goal::WorkResult>> promise{nullptr};
+     * The top-level goals of the worker.
-    };
+     */
+    Goals topGoals;
+
+    /**
+     * Goals that are ready to do some work.
+     */
+    WeakGoals awake;
+
+    /**
+     * Goals waiting for a build slot.
+     */
+    WeakGoals wantingToBuild;
+
+    /**
+     * Child processes currently running.
+     */
+    std::list<Child> children;
+
+    /**
+     * Number of build slots occupied.  This includes local builds but does not
+     * include substitutions or remote builds via the build hook.
+     */
+    unsigned int nrLocalBuilds;
+
+    /**
+     * Number of substitution slots occupied.
+     */
+    unsigned int nrSubstitutions;
+
     /**
      * Maps used to prevent multiple instantiations of a goal for the
      * same derivation / path.
      */
-    std::map<StorePath, CachedGoal<DerivationGoal>> derivationGoals;
+    std::map<StorePath, std::weak_ptr<DerivationGoal>> derivationGoals;
-    std::map<StorePath, CachedGoal<PathSubstitutionGoal>> substitutionGoals;
+    std::map<StorePath, std::weak_ptr<PathSubstitutionGoal>> substitutionGoals;
-    std::map<DrvOutput, CachedGoal<DrvOutputSubstitutionGoal>> drvOutputSubstitutionGoals;
+    std::map<DrvOutput, std::weak_ptr<DrvOutputSubstitutionGoal>> drvOutputSubstitutionGoals;
+
+    /**
+     * Goals sleeping for a few seconds (polling a lock).
+     */
+    WeakGoals waitingForAWhile;
+
+    /**
+     * Last time the goals in `waitingForAWhile` where woken up.
+     */
+    steady_time_point lastWokenUp;
 
     /**
      * Cache for pathContentsGood().
@@ -132,25 +176,60 @@ private:
      */
     bool checkMismatch = false;
 
+    void goalFinished(GoalPtr goal, Goal::Finished & f);
+    void handleWorkResult(GoalPtr goal, Goal::WorkResult how);
+
+    /**
+     * Put `goal` to sleep until a build slot becomes available (which
+     * might be right away).
+     */
+    void waitForBuildSlot(GoalPtr goal);
+
+    /**
+     * Wait for a few seconds and then retry this goal.  Used when
+     * waiting for a lock held by another process.  This kind of
+     * polling is inefficient, but POSIX doesn't really provide a way
+     * to wait for multiple locks in the main select() loop.
+     */
+    void waitForAWhile(GoalPtr goal);
+
+    /**
+     * Wake up a goal (i.e., there is something for it to do).
+     */
+    void wakeUp(GoalPtr goal);
+
+    /**
+     * Wait for input to become available.
+     */
+    void waitForInput();
+
+    /**
+     * Remove a dead goal.
+     */
+    void removeGoal(GoalPtr goal);
+
+    /**
+     * Registers a running child process.  `inBuildSlot` means that
+     * the process counts towards the jobs limit.
+     */
+    void childStarted(GoalPtr goal, const std::set<int> & fds,
+        bool inBuildSlot);
+
     /**
       * Pass current stats counters to the logger for progress bar updates.
       */
-    kj::Promise<Result<Results>> updateStatistics();
+    void updateStatistics();
 
-    AsyncSemaphore statisticsUpdateSignal{1};
+    bool statisticsOutdated = true;
-    std::optional<AsyncSemaphore::Token> statisticsUpdateInhibitor;
 
     /**
       * Mark statistics as outdated, such that `updateStatistics` will be called.
       */
     void updateStatisticsLater()
     {
-        statisticsUpdateInhibitor = {};
+        statisticsOutdated = true;
     }
 
-    kj::Promise<Result<Results>> runImpl(Targets topGoals);
-    kj::Promise<Result<Results>> boopGC(LocalStore & localStore);
-
 public:
 
     const Activity act;
@@ -160,12 +239,7 @@ public:
     Store & store;
     Store & evalStore;
     kj::AsyncIoContext & aio;
-    AsyncSemaphore substitutions, localBuilds;
 
-private:
-    kj::TaskSet children;
-
-public:
     struct HookState {
         std::unique_ptr<HookInstance> instance;
 
@@ -203,35 +277,21 @@ public:
      * @ref DerivationGoal "derivation goal"
      */
 private:
-    template<typename ID, std::derived_from<Goal> G>
+    std::shared_ptr<DerivationGoal> makeDerivationGoalCommon(
-    std::pair<std::shared_ptr<G>, kj::Promise<Result<Goal::WorkResult>>> makeGoalCommon(
+        const StorePath & drvPath, const OutputsSpec & wantedOutputs,
-        std::map<ID, CachedGoal<G>> & map,
+        std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal);
-        const ID & key,
+    std::shared_ptr<DerivationGoal> makeDerivationGoal(
-        InvocableR<std::unique_ptr<G>> auto create,
-        InvocableR<bool, G &> auto modify
-    );
-    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeDerivationGoal(
         const StorePath & drvPath,
         const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
-    std::pair<std::shared_ptr<DerivationGoal>, kj::Promise<Result<Goal::WorkResult>>> makeBasicDerivationGoal(
+    std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(
         const StorePath & drvPath, const BasicDerivation & drv,
         const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
 
     /**
      * @ref SubstitutionGoal "substitution goal"
      */
-    std::pair<std::shared_ptr<PathSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
+    std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override;
-    makePathSubstitutionGoal(
+    std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override;
-        const StorePath & storePath,
-        RepairFlag repair = NoRepair,
-        std::optional<ContentAddress> ca = std::nullopt
-    ) override;
-    std::pair<std::shared_ptr<DrvOutputSubstitutionGoal>, kj::Promise<Result<Goal::WorkResult>>>
-    makeDrvOutputSubstitutionGoal(
-        const DrvOutput & id,
-        RepairFlag repair = NoRepair,
-        std::optional<ContentAddress> ca = std::nullopt
-    ) override;
 
     /**
      * Make a goal corresponding to the `DerivedPath`.
@@ -239,14 +299,18 @@ private:
      * It will be a `DerivationGoal` for a `DerivedPath::Built` or
      * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
      */
-    std::pair<GoalPtr, kj::Promise<Result<Goal::WorkResult>>>
+    GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override;
-    makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override;
 
 public:
+    /**
+     * Unregisters a running child process.
+     */
+    void childTerminated(Goal * goal);
+
     /**
      * Loop until the specified top-level goals have finished.
      */
-    Results run(std::function<Targets (GoalFactory &)> req);
+    Goals run(std::function<Goals (GoalFactory &)> req);
 
     /***
      * The exit status in case of failure.

src/libstore/builtins.hh

@@ -6,7 +6,7 @@
 namespace nix {
 
 // TODO: make pluggable.
-void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData, const std::string & caFileData);
+void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData);
 void builtinUnpackChannel(const BasicDerivation & drv);
 
 }

src/libstore/builtins/fetchurl.cc

@@ -7,7 +7,7 @@
 
 namespace nix {
 
-void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData, const std::string & caFileData)
+void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)
 {
     /* Make the host's netrc data available. Too bad curl requires
        this to be stored in a file. It would be nice if we could just
@@ -17,9 +17,6 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData,
         writeFile(settings.netrcFile, netrcData, 0600);
     }
 
-    settings.caFile = "ca-certificates.crt";
-    writeFile(settings.caFile, caFileData, 0600);
-
     auto getAttr = [&](const std::string & name) {
         auto i = drv.env.find(name);
         if (i == drv.env.end()) throw Error("attribute '%s' missing", name);
@@ -36,7 +33,10 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData,
 
     auto fetch = [&](const std::string & url) {
 
+        /* No need to do TLS verification, because we check the hash of
+           the result anyway. */
         FileTransferRequest request(url);
+        request.verifyTLS = false;
 
         auto raw = fileTransfer->download(std::move(request));
         auto decompressor = makeDecompressionSource(

src/libstore/filetransfer.cc

@@ -115,7 +115,7 @@ struct curlFileTransfer : public FileTransfer
                 if (!done)
                     fail(FileTransferError(Interrupted, {}, "download of '%s' was interrupted", request.uri));
             } catch (...) {
-                ignoreExceptionInDestructor();
+                ignoreException();
             }
         }
 
@@ -337,7 +337,7 @@ struct curlFileTransfer : public FileTransfer
             // wrapping user `callback`s instead is not possible because the
             // Callback api expects std::functions, and copying Callbacks is
             // not possible due the promises they hold.
-            if (code == CURLE_OK && !dataCallback && result.data.length() > 0) {
+            if (code == CURLE_OK && !dataCallback) {
                 result.data = decompress(encoding, result.data);
             }
 

src/libstore/gc.cc

@@ -923,8 +923,8 @@ void LocalStore::autoGC(bool sync)
 
             } catch (...) {
                 // FIXME: we could propagate the exception to the
-                // future, but we don't really care. (what??)
+                // future, but we don't really care.
-                ignoreExceptionInDestructor();
+                ignoreException();
             }
 
         }).detach();

src/libstore/globals.cc

@@ -443,7 +443,7 @@ static bool initLibStoreDone = false;
 void assertLibStoreInitialized() {
     if (!initLibStoreDone) {
         printError("The program must call nix::initNix() before calling any libstore library functions.");
-        std::terminate();
+        abort();
     };
 }
 

src/libstore/local-store.cc

@@ -62,6 +62,8 @@ struct LocalStore::State::Stmts {
     SQLiteStmt QueryReferences;
     SQLiteStmt QueryReferrers;
     SQLiteStmt InvalidatePath;
+    SQLiteStmt InvalidatePhantomReferrers;
+    SQLiteStmt QueryPhantomReferrers;
     SQLiteStmt AddDerivationOutput;
     SQLiteStmt RegisterRealisedOutput;
     SQLiteStmt UpdateRealisedOutput;
@@ -384,6 +386,10 @@ LocalStore::LocalStore(const Params & params)
         "select path from Refs join ValidPaths on referrer = id where reference = (select id from ValidPaths where path = ?);");
     state->stmts->InvalidatePath.create(state->db,
         "delete from ValidPaths where path = ?;");
+    state->stmts->InvalidatePhantomReferrers.create(state->db,
+        "delete from Refs where referrer IN (select referrer from Refs left join ValidPaths on referrer = id where reference = (select id from ValidPaths where path = ?));");
+    state->stmts->QueryPhantomReferrers.create(state->db,
+        "select referrer from Refs left join ValidPaths on referrer = id where reference = (select id from ValidPaths where path = ?);");
     state->stmts->AddDerivationOutput.create(state->db,
         "insert or replace into DerivationOutputs (drv, id, path) values (?, ?, ?);");
     state->stmts->QueryValidDerivers.create(state->db,
@@ -481,7 +487,7 @@ LocalStore::~LocalStore()
             unlink(fnTempRoots.c_str());
         }
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 
@@ -664,20 +670,6 @@ static void canonicalisePathMetaData_(
     if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)))
         throw Error("file '%1%' has an unsupported type", path);
 
-    /* Fail if the file is not owned by the build user.  This prevents
-       us from messing up the ownership/permissions of files
-       hard-linked into the output (e.g. "ln /etc/shadow $out/foo").
-       However, ignore files that we chown'ed ourselves previously to
-       ensure that we don't fail on hard links within the same build
-       (i.e. "touch $out/foo; ln $out/foo $out/bar"). */
-    if (uidRange && (st.st_uid < uidRange->first || st.st_uid > uidRange->second)) {
-        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));
-        return;
-    }
-
 #if __linux__
     /* Remove extended attributes / ACLs. */
     ssize_t eaSize = llistxattr(path.c_str(), nullptr, 0);
@@ -691,8 +683,6 @@ static void canonicalisePathMetaData_(
         if ((eaSize = llistxattr(path.c_str(), eaBuf.data(), eaBuf.size())) < 0)
             throw SysError("querying extended attributes of '%s'", path);
 
-        if (S_ISREG(st.st_mode) || S_ISDIR(st.st_mode))
-            chmod(path.c_str(), st.st_mode | S_IWUSR);
         for (auto & eaName: tokenizeString<Strings>(std::string(eaBuf.data(), eaSize), std::string("\000", 1))) {
             if (settings.ignoredAcls.get().count(eaName)) continue;
             if (lremovexattr(path.c_str(), eaName.c_str()) == -1)
@@ -701,6 +691,20 @@ static void canonicalisePathMetaData_(
      }
 #endif
 
+    /* Fail if the file is not owned by the build user.  This prevents
+       us from messing up the ownership/permissions of files
+       hard-linked into the output (e.g. "ln /etc/shadow $out/foo").
+       However, ignore files that we chown'ed ourselves previously to
+       ensure that we don't fail on hard links within the same build
+       (i.e. "touch $out/foo; ln $out/foo $out/bar"). */
+    if (uidRange && (st.st_uid < uidRange->first || st.st_uid > uidRange->second)) {
+        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));
+        return;
+    }
+
     inodesSeen.insert(Inode(st.st_dev, st.st_ino));
 
     canonicaliseTimestampAndPermissions(path, st);
@@ -1218,11 +1222,11 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
     bool narRead = false;
     Finally cleanup = [&]() {
         if (!narRead) {
-            NARParseVisitor sink;
+            ParseSink sink;
             try {
                 parseDump(sink, source);
             } catch (...) {
-                ignoreExceptionExceptInterrupt();
+                ignoreException();
             }
         }
     };
@@ -1524,6 +1528,18 @@ void LocalStore::invalidatePathChecked(const StorePath & path)
             if (!referrers.empty())
                 throw PathInUse("cannot delete path '%s' because it is in use by %s",
                     printStorePath(path), showPaths(referrers));
+
+            // Note: `queryReferrers` will only return *valid* referrers.
+            // i.e. referrer for which there is a *ValidPath* row in the SQLite database.
+            // In the unfortunate situation where a valid path is removed but its corresponding `Refs` are not removed (*), we better just invalidate all these phantom referrers,
+            // otherwise we will create a foreign key violation when we actually try to invalidate paths.
+            //
+            // (*) : yes, there's a "ON DELETE CASCADE" on the referrer foreign key.
+            // Unfortunately, in practice, it doesn't ensure integrity over large SQLite databases.
+            if (hasPhantomReferrers(*state, path)) {
+                warn("'%s' has phantom referrers (disappeared referrers from the valid path table)", printStorePath(path));
+                invalidatePhantomReferrers(*state, path);
+            }
             invalidatePath(*state, path);
         }
 
@@ -1531,6 +1547,24 @@ void LocalStore::invalidatePathChecked(const StorePath & path)
     });
 }
 
+bool LocalStore::hasPhantomReferrers(State & state, const StorePath & path)
+{
+    return retrySQLite<bool>([&]() -> bool {
+        debug("checking for phantom referrers for '%s'", printStorePath(path));
+        auto useQueryPhantomReferrers(state.stmts->QueryPhantomReferrers.use()(printStorePath(path)));
+
+        return useQueryPhantomReferrers.next();
+    });
+}
+
+void LocalStore::invalidatePhantomReferrers(State & state, const StorePath & path)
+{
+    retrySQLite<void>([&]() {
+        debug("invalidating phantom referrers to '%s'", printStorePath(path));
+        state.stmts->InvalidatePhantomReferrers.use()(printStorePath(path)).exec();
+    });
+}
+
 
 bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)
 {

src/libstore/local-store.hh

@@ -322,6 +322,14 @@ private:
      * Delete a path from the Nix store.
      */
     void invalidatePathChecked(const StorePath & path);
+    /**
+     * Check if there's phantom referrers for a certain path in the Nix SQLite database
+     */
+    bool hasPhantomReferrers(State & state, const StorePath & path);
+    /**
+     * Invalidate all phantom referrers from the Nix SQLite database.
+     */
+    void invalidatePhantomReferrers(State & state, const StorePath & path);
 
     void verifyPath(const StorePath & path, const StorePathSet & store,
         StorePathSet & done, StorePathSet & validPaths, RepairFlag repair, bool & errors);

src/libstore/lock.cc

@@ -73,16 +73,8 @@ struct SimpleUserLock : UserLock
             debug("trying user '%s'", i);
 
             struct passwd * pw = getpwnam(i.c_str());
-            if (!pw) {
+            if (!pw)
-#ifdef __APPLE__
+                throw Error("the user '%s' in the group '%s' does not exist", i, settings.buildUsersGroup);
-#define APPLE_HINT "\n\nhint: this may be caused by an update to macOS Sequoia breaking existing Lix installations.\n" \
-                "See the macOS Sequoia page on the Lix wiki for detailed repair instructions: https://wiki.lix.systems/link/81"
-#else
-#define APPLE_HINT
-#endif
-                throw Error("the user '%s' in the group '%s' does not exist" APPLE_HINT, i, settings.buildUsersGroup);
-#undef APPLE_HINT
-            }
 
             auto fnUserLock = fmt("%s/userpool/%s", settings.nixStateDir,pw->pw_uid);
 

src/libstore/nar-accessor.cc

@@ -2,7 +2,6 @@
 #include "archive.hh"
 
 #include <map>
-#include <memory>
 #include <stack>
 #include <algorithm>
 
@@ -34,7 +33,7 @@ struct NarAccessor : public FSAccessor
 
     NarMember root;
 
-    struct NarIndexer : NARParseVisitor, Source
+    struct NarIndexer : ParseSink, Source
     {
         NarAccessor & acc;
         Source & source;
@@ -45,12 +44,11 @@ struct NarAccessor : public FSAccessor
 
         uint64_t pos = 0;
 
-    public:
         NarIndexer(NarAccessor & acc, Source & source)
             : acc(acc), source(source)
         { }
 
-        NarMember & createMember(const Path & path, NarMember member)
+        void createMember(const Path & path, NarMember member)
         {
             size_t level = std::count(path.begin(), path.end(), '/');
             while (parents.size() > level) parents.pop();
@@ -64,8 +62,6 @@ struct NarAccessor : public FSAccessor
                 auto result = parents.top()->children.emplace(baseNameOf(path), std::move(member));
                 parents.push(&result.first->second);
             }
-
-            return *parents.top();
         }
 
         void createDirectory(const Path & path) override
@@ -73,18 +69,29 @@ struct NarAccessor : public FSAccessor
             createMember(path, {FSAccessor::Type::tDirectory, false, 0, 0});
         }
 
-        std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
+        void createRegularFile(const Path & path) override
         {
-            auto & memb = createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
+            createMember(path, {FSAccessor::Type::tRegular, false, 0, 0});
-
-            assert(size <= std::numeric_limits<uint64_t>::max());
-            memb.size = (uint64_t) size;
-            memb.start = pos;
-            memb.isExecutable = executable;
-
-            return std::make_unique<FileHandle>();
         }
 
+        void closeRegularFile() override
+        { }
+
+        void isExecutable() override
+        {
+            parents.top()->isExecutable = true;
+        }
+
+        void preallocateContents(uint64_t size) override
+        {
+            assert(size <= std::numeric_limits<uint64_t>::max());
+            parents.top()->size = (uint64_t) size;
+            parents.top()->start = pos;
+        }
+
+        void receiveContents(std::string_view data) override
+        { }
+
         void createSymlink(const Path & path, const std::string & target) override
         {
             createMember(path,

src/libstore/optimise-store.cc

@@ -31,7 +31,7 @@ struct MakeReadOnly
             /* This will make the path read-only. */
             if (path != "") canonicaliseTimestampAndPermissions(path);
         } catch (...) {
-            ignoreExceptionInDestructor();
+            ignoreException();
         }
     }
 };

src/libstore/pathlocks.cc

@@ -145,7 +145,7 @@ PathLocks::~PathLocks()
     try {
         unlock();
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libstore/pathlocks.hh

@@ -1,7 +1,6 @@
 #pragma once
 ///@file
 
-#include "error.hh"
 #include "file-descriptor.hh"
 
 namespace nix {
@@ -54,7 +53,7 @@ struct FdLock
             if (acquired)
                 lockFile(fd, ltNone, false);
         } catch (SysError &) {
-            ignoreExceptionInDestructor();
+            ignoreException();
         }
     }
 };

src/libstore/platform.cc

@@ -25,7 +25,7 @@ std::shared_ptr<LocalStore> LocalStore::makeLocalStore(const Params & params)
 #endif
 }
 
-std::unique_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
+std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
     const StorePath & drvPath,
     const OutputsSpec & wantedOutputs,
     Worker & worker,
@@ -34,17 +34,17 @@ std::unique_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoa
 )
 {
 #if __linux__
-    return std::make_unique<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
+    return std::make_shared<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
 #elif __APPLE__
-    return std::make_unique<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
+    return std::make_shared<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
 #elif __FreeBSD__
-    return std::make_unique<FreeBSDLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
+    return std::make_shared<FreeBSDLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
 #else
-    return std::make_unique<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
+    return std::make_shared<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode);
 #endif
 }
 
-std::unique_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
+std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoal(
     const StorePath & drvPath,
     const BasicDerivation & drv,
     const OutputsSpec & wantedOutputs,
@@ -54,19 +54,19 @@ std::unique_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoa
 )
 {
 #if __linux__
-    return std::make_unique<LinuxLocalDerivationGoal>(
+    return std::make_shared<LinuxLocalDerivationGoal>(
         drvPath, drv, wantedOutputs, worker, isDependency, buildMode
     );
 #elif __APPLE__
-    return std::make_unique<DarwinLocalDerivationGoal>(
+    return std::make_shared<DarwinLocalDerivationGoal>(
         drvPath, drv, wantedOutputs, worker, isDependency, buildMode
     );
 #elif __FreeBSD__
-    return std::make_unique<FreeBSDLocalDerivationGoal>(
+    return std::make_shared<FreeBSDLocalDerivationGoal>(
         drvPath, drv, wantedOutputs, worker, isDependency, buildMode
     );
 #else
-    return std::make_unique<FallbackLocalDerivationGoal>(
+    return std::make_shared<FallbackLocalDerivationGoal>(
         drvPath, drv, wantedOutputs, worker, isDependency, buildMode
     );
 #endif

src/libstore/remote-fs-accessor.cc

@@ -29,7 +29,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str
             /* FIXME: do this asynchronously. */
             writeFile(makeCacheFile(hashPart, "nar"), nar);
         } catch (...) {
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
         }
     }
 
@@ -41,7 +41,7 @@ ref<FSAccessor> RemoteFSAccessor::addToCache(std::string_view hashPart, std::str
             nlohmann::json j = listNar(narAccessor, "", true);
             writeFile(makeCacheFile(hashPart, "ls"), j.dump());
         } catch (...) {
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
         }
     }
 

src/libstore/remote-store.cc

@@ -1,4 +1,3 @@
-#include "error.hh"
 #include "serialise.hh"
 #include "signals.hh"
 #include "path-with-outputs.hh"
@@ -856,7 +855,7 @@ RemoteStore::Connection::~Connection()
     try {
         to.flush();
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 
@@ -986,7 +985,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sin
                 try {
                     std::rethrow_exception(ex);
                 } catch (...) {
-                    ignoreExceptionExceptInterrupt();
+                    ignoreException();
                 }
             }
         }

src/libstore/sqlite.cc

@@ -85,7 +85,7 @@ SQLite::~SQLite()
         if (db && sqlite3_close(db) != SQLITE_OK)
             SQLiteError::throw_(db, "closing database");
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 
@@ -124,7 +124,7 @@ SQLiteStmt::~SQLiteStmt()
         if (stmt && sqlite3_finalize(stmt) != SQLITE_OK)
             SQLiteError::throw_(db, "finalizing statement '%s'", sql);
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 
@@ -248,7 +248,7 @@ SQLiteTxn::~SQLiteTxn()
         if (active && sqlite3_exec(db, "rollback;", 0, 0, 0) != SQLITE_OK)
             SQLiteError::throw_(db, "aborting transaction");
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libstore/store-api.cc

@@ -379,48 +379,6 @@ void Store::addMultipleToStore(
     }
 }
 
-namespace {
-/**
- * If the NAR archive contains a single file at top-level, then save
- * the contents of the file to `s`.  Otherwise assert.
- */
-struct RetrieveRegularNARVisitor : NARParseVisitor
-{
-    struct MyFileHandle : public FileHandle
-    {
-        Sink & sink;
-
-        void receiveContents(std::string_view data) override
-        {
-            sink(data);
-        }
-
-    private:
-        MyFileHandle(Sink & sink) : sink(sink) {}
-
-        friend struct RetrieveRegularNARVisitor;
-    };
-
-    Sink & sink;
-
-    RetrieveRegularNARVisitor(Sink & sink) : sink(sink) { }
-
-    std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
-    {
-        return std::unique_ptr<MyFileHandle>(new MyFileHandle{sink});
-    }
-
-    void createDirectory(const Path & path) override
-    {
-        assert(false && "RetrieveRegularNARVisitor::createDirectory must not be called");
-    }
-
-    void createSymlink(const Path & path, const std::string & target) override
-    {
-        assert(false && "RetrieveRegularNARVisitor::createSymlink must not be called");
-    }
-};
-}
 
 /*
 The aim of this function is to compute in one pass the correct ValidPathInfo for
@@ -455,7 +413,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
     /* Note that fileSink and unusualHashTee must be mutually exclusive, since
        they both write to caHashSink. Note that that requisite is currently true
        because the former is only used in the flat case. */
-    RetrieveRegularNARVisitor fileSink { caHashSink };
+    RetrieveRegularNARSink fileSink { caHashSink };
     TeeSink unusualHashTee { narHashSink, caHashSink };
 
     auto & narSink = method == FileIngestionMethod::Recursive && hashAlgo != HashType::SHA256
@@ -471,7 +429,7 @@ ValidPathInfo Store::addToStoreSlow(std::string_view name, const Path & srcPath,
        information to narSink. */
     TeeSource tapped { fileSource, narSink };
 
-    NARParseVisitor blank;
+    ParseSink blank;
     auto & parseSink = method == FileIngestionMethod::Flat
         ? fileSink
         : blank;
@@ -1163,7 +1121,7 @@ std::map<StorePath, StorePath> copyPaths(
         // not be within our control to change that, and we might still want
         // to at least copy the output paths.
         if (e.missingFeature == Xp::CaDerivations)
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
         else
             throw;
     }

src/libutil/archive.cc

@@ -334,7 +334,7 @@ Generator<Entry> parse(Source & source)
 }
 
 
-static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry> nar)
+static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar)
 {
     while (auto entry = nar.next()) {
         co_yield std::visit(
@@ -347,13 +347,16 @@ static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry>
                 },
                 [&](nar::File f) {
                     return [](auto f, auto & sink) -> WireFormatGenerator {
-                        auto handle = sink.createRegularFile(f.path, f.size, f.executable);
+                        sink.createRegularFile(f.path);
-
+                        sink.preallocateContents(f.size);
+                        if (f.executable) {
+                            sink.isExecutable();
+                        }
                         while (auto block = f.contents.next()) {
-                            handle->receiveContents(std::string_view{block->data(), block->size()});
+                            sink.receiveContents(std::string_view{block->data(), block->size()});
                             co_yield *block;
                         }
-                        handle->close();
+                        sink.closeRegularFile();
                     }(std::move(f), sink);
                 },
                 [&](nar::Symlink sl) {
@@ -374,12 +377,12 @@ static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry>
     }
 }
 
-WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source)
+WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source)
 {
     return restore(sink, nar::parse(source));
 }
 
-void parseDump(NARParseVisitor & sink, Source & source)
+void parseDump(ParseSink & sink, Source & source)
 {
     auto parser = parseAndCopyDump(sink, source);
     while (parser.next()) {
@@ -387,99 +390,11 @@ void parseDump(NARParseVisitor & sink, Source & source)
     }
 }
 
-/*
+struct RestoreSink : ParseSink
- * Note [NAR restoration security]:
- * It's *critical* that NAR restoration will never overwrite anything even if
- * duplicate filenames are passed in. It is inevitable that not all NARs are
- * fit to actually successfully restore to the target filesystem; errors may
- * occur due to collisions, and this *must* cause the NAR to be rejected.
- *
- * Although the filenames are blocked from being *the same bytes* by a higher
- * layer, filesystems have other ideas on every platform:
- * - The store may be on a case-insensitive filesystem like APFS, ext4 with
- *   casefold directories, zfs with casesensitivity=insensitive
- * - The store may be on a Unicode normalizing (or normalization-insensitive)
- *   filesystem like APFS (where files are looked up by
- *   hash(normalize(fname))), HFS+ (where file names are always normalized to
- *   approximately NFD), or zfs with normalization=formC, etc.
- *
- * It is impossible to know the version of Unicode being used by the underlying
- * filesystem, thus it is *impossible* to stop these collisions.
- *
- * Overwriting files as a result of invalid NARs will cause a security bug like
- * CppNix's CVE-2024-45593 (GHSA-h4vv-h3jq-v493)
- */
-
-/**
- * This code restores NARs from disk.
- *
- * See Note [NAR restoration security] for security invariants in this procedure.
- *
- */
-struct NARRestoreVisitor : NARParseVisitor
 {
     Path dstPath;
+    AutoCloseFD fd;
 
-private:
-    class MyFileHandle : public FileHandle
-    {
-        AutoCloseFD fd;
-
-        MyFileHandle(AutoCloseFD && fd, uint64_t size, bool executable) : FileHandle(), fd(std::move(fd))
-        {
-            if (executable) {
-                makeExecutable();
-            }
-
-            maybePreallocateContents(size);
-        }
-
-        void makeExecutable()
-        {
-            struct stat st;
-            if (fstat(fd.get(), &st) == -1)
-                throw SysError("fstat");
-            if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
-                throw SysError("fchmod");
-        }
-
-        void maybePreallocateContents(uint64_t len)
-        {
-            if (!archiveSettings.preallocateContents)
-                return;
-
-#if HAVE_POSIX_FALLOCATE
-            if (len) {
-                errno = posix_fallocate(fd.get(), 0, len);
-                /* Note that EINVAL may indicate that the underlying
-                   filesystem doesn't support preallocation (e.g. on
-                   OpenSolaris).  Since preallocation is just an
-                   optimisation, ignore it. */
-                if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
-                    throw SysError("preallocating file of %1% bytes", len);
-            }
-#endif
-        }
-
-    public:
-
-        ~MyFileHandle() = default;
-
-        virtual void close() override
-        {
-            /* Call close explicitly to make sure the error is checked */
-            fd.close();
-        }
-
-        void receiveContents(std::string_view data) override
-        {
-            writeFull(fd.get(), data);
-        }
-
-        friend struct NARRestoreVisitor;
-    };
-
-public:
     void createDirectory(const Path & path) override
     {
         Path p = dstPath + path;
@@ -487,13 +402,49 @@ public:
             throw SysError("creating directory '%1%'", p);
     };
 
-    std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override
+    void createRegularFile(const Path & path) override
     {
         Path p = dstPath + path;
-        AutoCloseFD fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
+        fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)};
         if (!fd) throw SysError("creating file '%1%'", p);
+    }
 
-        return std::unique_ptr<MyFileHandle>(new MyFileHandle(std::move(fd), size, executable));
+    void closeRegularFile() override
+    {
+        /* Call close explicitly to make sure the error is checked */
+        fd.close();
+    }
+
+    void isExecutable() override
+    {
+        struct stat st;
+        if (fstat(fd.get(), &st) == -1)
+            throw SysError("fstat");
+        if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
+            throw SysError("fchmod");
+    }
+
+    void preallocateContents(uint64_t len) override
+    {
+        if (!archiveSettings.preallocateContents)
+            return;
+
+#if HAVE_POSIX_FALLOCATE
+        if (len) {
+            errno = posix_fallocate(fd.get(), 0, len);
+            /* Note that EINVAL may indicate that the underlying
+               filesystem doesn't support preallocation (e.g. on
+               OpenSolaris).  Since preallocation is just an
+               optimisation, ignore it. */
+            if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
+                throw SysError("preallocating file of %1% bytes", len);
+        }
+#endif
+    }
+
+    void receiveContents(std::string_view data) override
+    {
+        writeFull(fd.get(), data);
     }
 
     void createSymlink(const Path & path, const std::string & target) override
@@ -506,7 +457,7 @@ public:
 
 void restorePath(const Path & path, Source & source)
 {
-    NARRestoreVisitor sink;
+    RestoreSink sink;
     sink.dstPath = path;
     parseDump(sink, source);
 }
@@ -517,9 +468,10 @@ WireFormatGenerator copyNAR(Source & source)
     // FIXME: if 'source' is the output of dumpPath() followed by EOF,
     // we should just forward all data directly without parsing.
 
-    static NARParseVisitor parseSink; /* null sink; just parse the NAR */
+    static ParseSink parseSink; /* null sink; just parse the NAR */
 
     return parseAndCopyDump(parseSink, source);
 }
 
+
 }

src/libutil/archive.hh

@@ -76,47 +76,45 @@ WireFormatGenerator dumpString(std::string_view s);
 
 /**
  * \todo Fix this API, it sucks.
- * A visitor for NAR parsing that performs filesystem (or virtual-filesystem)
- * actions to restore a NAR.
- *
- * Methods of this may arbitrarily fail due to filename collisions.
  */
-struct NARParseVisitor
+struct ParseSink
 {
-    /**
+    virtual void createDirectory(const Path & path) { };
-     * A type-erased file handle specific to this particular NARParseVisitor.
+
-     */
+    virtual void createRegularFile(const Path & path) { };
-    struct FileHandle
+    virtual void closeRegularFile() { };
+    virtual void isExecutable() { };
+    virtual void preallocateContents(uint64_t size) { };
+    virtual void receiveContents(std::string_view data) { };
+
+    virtual void createSymlink(const Path & path, const std::string & target) { };
+};
+
+/**
+ * If the NAR archive contains a single file at top-level, then save
+ * the contents of the file to `s`.  Otherwise barf.
+ */
+struct RetrieveRegularNARSink : ParseSink
+{
+    bool regular = true;
+    Sink & sink;
+
+    RetrieveRegularNARSink(Sink & sink) : sink(sink) { }
+
+    void createDirectory(const Path & path) override
     {
-        FileHandle() {}
+        regular = false;
-        FileHandle(FileHandle const &) = delete;
-        FileHandle & operator=(FileHandle &) = delete;
-
-        /** Puts one block of data into the file */
-        virtual void receiveContents(std::string_view data) { }
-
-        /**
-         * Explicitly closes the file. Further operations may throw an assert.
-         * This exists so that closing can fail and throw an exception without doing so in a destructor.
-         */
-        virtual void close() { }
-
-        virtual ~FileHandle() = default;
-    };
-
-    virtual void createDirectory(const Path & path) { }
-
-    /**
-     * Creates a regular file in the extraction output with the given size and executable flag.
-     * The size is guaranteed to be the true size of the file.
-     */
-    [[nodiscard]]
-    virtual std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable)
-    {
-        return std::make_unique<FileHandle>();
     }
 
-    virtual void createSymlink(const Path & path, const std::string & target) { }
+    void receiveContents(std::string_view data) override
+    {
+        sink(data);
+    }
+
+    void createSymlink(const Path & path, const std::string & target) override
+    {
+        regular = false;
+    }
 };
 
 namespace nar {
@@ -162,8 +160,8 @@ Generator<Entry> parse(Source & source);
 
 }
 
-WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source);
+WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source);
-void parseDump(NARParseVisitor & sink, Source & source);
+void parseDump(ParseSink & sink, Source & source);
 
 void restorePath(const Path & path, Source & source);
 

src/libutil/async-collect.hh

@@ -1,101 +0,0 @@
-#pragma once
-/// @file
-
-#include <kj/async.h>
-#include <kj/common.h>
-#include <kj/vector.h>
-#include <list>
-#include <optional>
-#include <type_traits>
-
-namespace nix {
-
-template<typename K, typename V>
-class AsyncCollect
-{
-public:
-    using Item = std::conditional_t<std::is_void_v<V>, K, std::pair<K, V>>;
-
-private:
-    kj::ForkedPromise<void> allPromises;
-    std::list<Item> results;
-    size_t remaining;
-
-    kj::ForkedPromise<void> signal;
-    kj::Maybe<kj::Own<kj::PromiseFulfiller<void>>> notify;
-
-    void oneDone(Item item)
-    {
-        results.emplace_back(std::move(item));
-        remaining -= 1;
-        KJ_IF_MAYBE (n, notify) {
-            (*n)->fulfill();
-            notify = nullptr;
-        }
-    }
-
-    kj::Promise<void> collectorFor(K key, kj::Promise<V> promise)
-    {
-        if constexpr (std::is_void_v<V>) {
-            return promise.then([this, key{std::move(key)}] { oneDone(std::move(key)); });
-        } else {
-            return promise.then([this, key{std::move(key)}](V v) {
-                oneDone(Item{std::move(key), std::move(v)});
-            });
-        }
-    }
-
-    kj::ForkedPromise<void> waitForAll(kj::Array<std::pair<K, kj::Promise<V>>> & promises)
-    {
-        kj::Vector<kj::Promise<void>> wrappers;
-        for (auto & [key, promise] : promises) {
-            wrappers.add(collectorFor(std::move(key), std::move(promise)));
-        }
-
-        return kj::joinPromisesFailFast(wrappers.releaseAsArray()).fork();
-    }
-
-public:
-    AsyncCollect(kj::Array<std::pair<K, kj::Promise<V>>> && promises)
-        : allPromises(waitForAll(promises))
-        , remaining(promises.size())
-        , signal{nullptr}
-    {
-    }
-
-    kj::Promise<std::optional<Item>> next()
-    {
-        if (remaining == 0 && results.empty()) {
-            return {std::nullopt};
-        }
-
-        if (!results.empty()) {
-            auto result = std::move(results.front());
-            results.pop_front();
-            return {{std::move(result)}};
-        }
-
-        if (notify == nullptr) {
-            auto pair = kj::newPromiseAndFulfiller<void>();
-            notify = std::move(pair.fulfiller);
-            signal = pair.promise.fork();
-        }
-
-        return signal.addBranch().exclusiveJoin(allPromises.addBranch()).then([this] {
-            return next();
-        });
-    }
-};
-
-/**
-  * Collect the results of a list of promises, in order of completion.
-  * Once any input promise is rejected all promises that have not been
-  * resolved or rejected will be cancelled and the exception rethrown.
-  */
-template<typename K, typename V>
-AsyncCollect<K, V> asyncCollect(kj::Array<std::pair<K, kj::Promise<V>>> promises)
-{
-    return AsyncCollect<K, V>(std::move(promises));
-}
-
-}

src/libutil/async-semaphore.hh

@@ -1,122 +0,0 @@
-#pragma once
-/// @file
-/// @brief A semaphore implementation usable from within a KJ event loop.
-
-#include <cassert>
-#include <kj/async.h>
-#include <kj/common.h>
-#include <kj/exception.h>
-#include <kj/list.h>
-#include <kj/source-location.h>
-#include <memory>
-#include <optional>
-
-namespace nix {
-
-class AsyncSemaphore
-{
-public:
-    class [[nodiscard("destroying a semaphore guard releases the semaphore immediately")]] Token
-    {
-        struct Release
-        {
-            void operator()(AsyncSemaphore * sem) const
-            {
-                sem->unsafeRelease();
-            }
-        };
-
-        std::unique_ptr<AsyncSemaphore, Release> parent;
-
-    public:
-        Token() = default;
-        Token(AsyncSemaphore & parent, kj::Badge<AsyncSemaphore>) : parent(&parent) {}
-
-        bool valid() const
-        {
-            return parent != nullptr;
-        }
-    };
-
-private:
-    struct Waiter
-    {
-        kj::PromiseFulfiller<Token> & fulfiller;
-        kj::ListLink<Waiter> link;
-        kj::List<Waiter, &Waiter::link> & list;
-
-        Waiter(kj::PromiseFulfiller<Token> & fulfiller, kj::List<Waiter, &Waiter::link> & list)
-            : fulfiller(fulfiller)
-            , list(list)
-        {
-            list.add(*this);
-        }
-
-        ~Waiter()
-        {
-            if (link.isLinked()) {
-                list.remove(*this);
-            }
-        }
-    };
-
-    const unsigned capacity_;
-    unsigned used_ = 0;
-    kj::List<Waiter, &Waiter::link> waiters;
-
-    void unsafeRelease()
-    {
-        used_ -= 1;
-        while (used_ < capacity_ && !waiters.empty()) {
-            used_ += 1;
-            auto & w = waiters.front();
-            w.fulfiller.fulfill(Token{*this, {}});
-            waiters.remove(w);
-        }
-    }
-
-public:
-    explicit AsyncSemaphore(unsigned capacity) : capacity_(capacity) {}
-
-    KJ_DISALLOW_COPY_AND_MOVE(AsyncSemaphore);
-
-    ~AsyncSemaphore()
-    {
-        assert(waiters.empty() && "destroyed a semaphore with active waiters");
-    }
-
-    std::optional<Token> tryAcquire()
-    {
-        if (used_ < capacity_) {
-            used_ += 1;
-            return Token{*this, {}};
-        } else {
-            return {};
-        }
-    }
-
-    kj::Promise<Token> acquire()
-    {
-        if (auto t = tryAcquire()) {
-            return std::move(*t);
-        } else {
-            return kj::newAdaptedPromise<Token, Waiter>(waiters);
-        }
-    }
-
-    unsigned capacity() const
-    {
-        return capacity_;
-    }
-
-    unsigned used() const
-    {
-        return used_;
-    }
-
-    unsigned available() const
-    {
-        return capacity_ - used_;
-    }
-};
-}

src/libutil/compression.cc

@@ -144,7 +144,6 @@ struct BrotliDecompressionSource : Source
     std::unique_ptr<char[]> buf;
     size_t avail_in = 0;
     const uint8_t * next_in;
-    std::exception_ptr inputEofException = nullptr;
 
     Source * inner;
     std::unique_ptr<BrotliDecoderState, void (*)(BrotliDecoderState *)> state;
@@ -168,42 +167,23 @@ struct BrotliDecompressionSource : Source
         while (len && !BrotliDecoderIsFinished(state.get())) {
             checkInterrupt();
 
-            while (avail_in == 0 && inputEofException == nullptr) {
+            while (avail_in == 0) {
                 try {
                     avail_in = inner->read(buf.get(), BUF_SIZE);
                 } catch (EndOfFile &) {
-                    // No more data, but brotli may still have output remaining
-                    // from the last call.
-                    inputEofException = std::current_exception();
                     break;
                 }
                 next_in = charptr_cast<const uint8_t *>(buf.get());
             }
 
-            BrotliDecoderResult res = BrotliDecoderDecompressStream(
+            if (!BrotliDecoderDecompressStream(
-                state.get(), &avail_in, &next_in, &len, &out, nullptr
+                    state.get(), &avail_in, &next_in, &len, &out, nullptr
-            );
+                ))
-
+            {
-            switch (res) {
-            case BROTLI_DECODER_RESULT_SUCCESS:
-                // We're done here!
-                goto finish;
-            case BROTLI_DECODER_RESULT_NEEDS_MORE_INPUT:
-                // Grab more input. Don't try if we already have exhausted our input stream.
-                if (inputEofException != nullptr) {
-                    std::rethrow_exception(inputEofException);
-                } else {
-                    continue;
-                }
-            case BROTLI_DECODER_RESULT_NEEDS_MORE_OUTPUT:
-                // Need more output space: we can only get another buffer by someone calling us again, so get out.
-                goto finish;
-            case BROTLI_DECODER_RESULT_ERROR:
                 throw CompressionError("error while decompressing brotli file");
             }
         }
 
-finish:
         if (begin != out) {
             return out - begin;
         } else {

src/libutil/current-process.cc

@@ -49,7 +49,7 @@ unsigned int getMaxCPU()
         auto period = cpuMaxParts[1];
         if (quota != "max")
                 return std::ceil(std::stoi(quota) / std::stof(period));
-    } catch (Error &) { ignoreExceptionInDestructor(lvlDebug); }
+    } catch (Error &) { ignoreException(lvlDebug); }
     #endif
 
     return 0;

src/libutil/error.cc

@@ -4,7 +4,6 @@
 #include "position.hh"
 #include "terminal.hh"
 #include "strings.hh"
-#include "signals.hh"
 
 #include <iostream>
 #include <optional>
@@ -417,7 +416,7 @@ std::ostream & showErrorInfo(std::ostream & out, const ErrorInfo & einfo, bool s
     return out;
 }
 
-void ignoreExceptionInDestructor(Verbosity lvl)
+void ignoreException(Verbosity lvl)
 {
     /* Make sure no exceptions leave this function.
        printError() also throws when remote is closed. */
@@ -430,15 +429,4 @@ void ignoreExceptionInDestructor(Verbosity lvl)
     } catch (...) { }
 }
 
-void ignoreExceptionExceptInterrupt(Verbosity lvl)
-{
-    try {
-        throw;
-    } catch (const Interrupted & e) {
-        throw;
-    } catch (std::exception & e) {
-        printMsg(lvl, "error (ignored): %1%", e.what());
-    }
-}
-
 }

src/libutil/error.hh

@@ -204,22 +204,7 @@ public:
 /**
  * Exception handling in destructors: print an error message, then
  * ignore the exception.
- *
- * If you're not in a destructor, you usually want to use `ignoreExceptionExceptInterrupt()`.
- *
- * This function might also be used in callbacks whose caller may not handle exceptions,
- * but ideally we propagate the exception using an exception_ptr in such cases.
- * See e.g. `PackBuilderContext`
  */
-void ignoreExceptionInDestructor(Verbosity lvl = lvlError);
+void ignoreException(Verbosity lvl = lvlError);
-
-/**
- * Not destructor-safe.
- * Print an error message, then ignore the exception.
- * If the exception is an `Interrupted` exception, rethrow it.
- *
- * This may be used in a few places where Interrupt can't happen, but that's ok.
- */
-void ignoreExceptionExceptInterrupt(Verbosity lvl = lvlError);
 
 }

src/libutil/experimental-features.cc

@@ -247,7 +247,7 @@ constexpr std::array<ExperimentalFeatureDetails, numXpFeatures> xpFeatureDetails
         .tag = Xp::ReplAutomation,
         .name = "repl-automation",
         .description = R"(
-            Makes the repl not use editline, print ENQ (U+0005) when ready for a command, and take commands followed by newline.
+            Makes the repl not use readline/editline, print ENQ (U+0005) when ready for a command, and take commands followed by newline.
         )",
     },
 }};

src/libutil/file-descriptor.cc

@@ -146,7 +146,7 @@ AutoCloseFD::~AutoCloseFD()
     try {
         close();
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libutil/file-system.cc

@@ -522,7 +522,7 @@ AutoDelete::~AutoDelete()
             }
         }
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libutil/file-system.hh

@@ -210,7 +210,7 @@ inline Paths createDirs(PathView path)
 }
 
 /**
- * Create a symlink. Throws if the symlink exists.
+ * Create a symlink.
  */
 void createSymlink(const Path & target, const Path & link);
 

src/libutil/fmt.hh

@@ -136,17 +136,11 @@ inline std::string fmt(const char * s)
 
 template<typename... Args>
 inline std::string fmt(const std::string & fs, const Args &... args)
-try {
+{
     boost::format f(fs);
     fmt_internal::setExceptions(f);
     (f % ... % args);
     return f.str();
-} catch (boost::io::format_error & fe) {
-    // I don't care who catches this, we do not put up with boost format errors
-    // Give me a stack trace and a core dump
-    std::cerr << "nix::fmt threw format error. Original format string: '";
-    std::cerr << fs << "'; number of arguments: " << sizeof...(args) << "\n";
-    std::terminate();
 }
 
 /**
@@ -180,13 +174,15 @@ public:
             std::cerr << "HintFmt received incorrect number of format args. Original format string: '";
             std::cerr << format << "'; number of arguments: " << sizeof...(args) << "\n";
             // And regardless of the coredump give me a damn stacktrace.
-            std::terminate();
+            printStackTrace();
+            abort();
         }
     } catch (boost::io::format_error & ex) {
         // Same thing, but for anything that happens in the member initializers.
         std::cerr << "HintFmt received incorrect format string. Original format string: '";
         std::cerr << format << "'; number of arguments: " << sizeof...(args) << "\n";
-        std::terminate();
+        printStackTrace();
+        abort();
     }
 
     HintFmt(const HintFmt & hf) : fmt(hf.fmt) {}

src/libutil/logging.cc

@@ -352,7 +352,7 @@ Activity::~Activity()
     try {
         logger.stopActivity(id);
     } catch (...) {
-        ignoreExceptionInDestructor();
+        ignoreException();
     }
 }
 

src/libutil/meson.build

@@ -53,8 +53,6 @@ libutil_headers = files(
   'archive.hh',
   'args/root.hh',
   'args.hh',
-  'async-collect.hh',
-  'async-semaphore.hh',
   'backed-string-view.hh',
   'box_ptr.hh',
   'canon-path.hh',

src/libutil/serialise.cc

@@ -83,7 +83,7 @@ void BufferedSink::flush()
 
 FdSink::~FdSink()
 {
-    try { flush(); } catch (...) { ignoreExceptionInDestructor(); }
+    try { flush(); } catch (...) { ignoreException(); }
 }
 
 

src/libutil/serialise.hh

@@ -77,11 +77,6 @@ struct Source
      * Store up to ‘len’ in the buffer pointed to by ‘data’, and
      * return the number of bytes stored.  It blocks until at least
      * one byte is available.
-     *
-     * Should not return 0 (generally you want to throw EndOfFile), but nothing
-     * stops that.
-     *
-     * \throws EndOfFile if there is no more data.
      */
     virtual size_t read(char * data, size_t len) = 0;
 
@@ -549,7 +544,7 @@ struct FramedSource : Source
                 }
             }
         } catch (...) {
-            ignoreExceptionInDestructor();
+            ignoreException();
         }
     }
 
@@ -595,7 +590,7 @@ struct FramedSink : nix::BufferedSink
             to << 0;
             to.flush();
         } catch (...) {
-            ignoreExceptionInDestructor();
+            ignoreException();
         }
     }
 

src/libutil/signals.cc

@@ -12,18 +12,13 @@ std::atomic<bool> _isInterrupted = false;
 
 thread_local std::function<bool()> interruptCheck;
 
-Interrupted makeInterrupted()
-{
-    return Interrupted("interrupted by the user");
-}
-
 void _interrupted()
 {
     /* Block user interrupts while an exception is being handled.
        Throwing an exception while another exception is being handled
        kills the program! */
     if (!std::uncaught_exceptions()) {
-        throw makeInterrupted();
+        throw Interrupted("interrupted by the user");
     }
 }
 
@@ -83,7 +78,7 @@ void triggerInterrupt()
             try {
                 callback();
             } catch (...) {
-                ignoreExceptionInDestructor();
+                ignoreException();
             }
         }
     }

src/libutil/signals.hh

@@ -16,13 +16,10 @@ namespace nix {
 
 /* User interruption. */
 
-class Interrupted;
-
 extern std::atomic<bool> _isInterrupted;
 
 extern thread_local std::function<bool()> interruptCheck;
 
-Interrupted makeInterrupted();
 void _interrupted();
 
 void inline checkInterrupt()

src/libutil/thread-pool.cc

@@ -109,8 +109,9 @@ void ThreadPool::doWork(bool mainThread)
                         try {
                             std::rethrow_exception(exc);
                         } catch (std::exception & e) {
-                            if (!dynamic_cast<ThreadPoolShutDown*>(&e))
+                            if (!dynamic_cast<Interrupted*>(&e) &&
-                                ignoreExceptionExceptInterrupt();
+                                !dynamic_cast<ThreadPoolShutDown*>(&e))
+                                ignoreException();
                         } catch (...) {
                         }
                     }

src/nix-channel/meson.build

@@ -1 +1,5 @@
-fs.copyfile('unpack-channel.nix')
+configure_file(
+  input : 'unpack-channel.nix',
+  output : 'unpack-channel.nix',
+  copy : true,
+)

src/nix/develop.cc

@@ -639,7 +639,7 @@ struct CmdDevelop : Common, MixEnvironment
                 throw Error("package 'nixpkgs#bashInteractive' does not provide a 'bin/bash'");
 
         } catch (Error &) {
-            ignoreExceptionExceptInterrupt();
+            ignoreException();
         }
 
         // Override SHELL with the one chosen for this environment.

src/nix/flake.cc

@@ -16,7 +16,6 @@
 #include "eval-cache.hh"
 #include "markdown.hh"
 #include "terminal.hh"
-#include "signals.hh"
 
 #include <limits>
 #include <nlohmann/json.hpp>
@@ -368,11 +367,9 @@ struct CmdFlakeCheck : FlakeCommand
         auto reportError = [&](const Error & e) {
             try {
                 throw e;
-            } catch (Interrupted & e) {
-                throw;
             } catch (Error & e) {
                 if (settings.keepGoing) {
-                    ignoreExceptionExceptInterrupt();
+                    ignoreException();
                     hasErrors = true;
                 }
                 else

src/nix/fmt.cc

@@ -39,8 +39,14 @@ struct CmdFmt : SourceExprCommand {
         Strings programArgs{app.program};
 
         // Propagate arguments from the CLI
-        for (auto &i : args) {
+        if (args.empty()) {
-            programArgs.push_back(i);
+            // Format the current flake out of the box
+            programArgs.push_back(".");
+        } else {
+            // User wants more power, let them decide which paths to include/exclude
+            for (auto &i : args) {
+                programArgs.push_back(i);
+            }
         }
 
         runProgramInStore(store, UseSearchPath::DontUse, app.program, programArgs);

src/nix/path-info.cc

@@ -82,10 +82,6 @@ struct CmdPathInfo : StorePathsCommand, MixJSON
 
     void run(ref<Store> store, StorePaths && storePaths) override
     {
-        // Wipe the progress bar to prevent interference with the output.
-        // It's not needed any more because expensive evaluation or builds are already done here.
-        logger->pause();
-
         size_t pathLen = 0;
         for (auto & storePath : storePaths)
             pathLen = std::max(pathLen, store->printStorePath(storePath).size());

tests/functional/build.sh

@@ -144,10 +144,13 @@ test "$(<<<"$out" grep -E '^error:' | wc -l)" = 1
 # --keep-going and FOD
 out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$?
 test "$status" = 1
-# at least one "hash mismatch" error, one "build of ... failed"
+# one "hash mismatch" error, one "build of ... failed"
-test "$(<<<"$out" grep -E '^error:' | wc -l)" -ge 2
+test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2
-<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x.\\.drv'"
+<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'"
-<<<"$out" grepQuiet -E "likely URL: "
+<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'"
+<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
+<<<"$out" grepQuiet -E "likely URL: https://meow.puppy.forge/puppy.tar.gz"
+<<<"$out" grepQuiet -vE "likely URL: https://kitty.forge/cat.tar.gz"
 <<<"$out" grepQuiet -E "error: build of '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out' failed"
 
 out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$?
@@ -164,9 +167,9 @@ test "$(<<<"$out" grep -E '^error:' | wc -l)" = 4
 
 out="$(nix build -f fod-failing.nix -L x4 2>&1)" && status=0 || status=$?
 test "$status" = 1
-test "$(<<<"$out" grep -E '^error:' | wc -l)" -ge 2
+test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2
-<<<"$out" grepQuiet -E "error: [12] dependencies of derivation '.*-x4\\.drv' failed to build"
+<<<"$out" grepQuiet -E "error: 1 dependencies of derivation '.*-x4\\.drv' failed to build"
-<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x[23]\\.drv'"
+<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x2\\.drv'"
 
 out="$(nix build -f fod-failing.nix -L x4 --keep-going 2>&1)" && status=0 || status=$?
 test "$status" = 1

tests/functional/common/vars-and-functions.sh.in

@@ -16,6 +16,7 @@ fi
 export NIX_LOCALSTATE_DIR=$TEST_ROOT/var
 export NIX_LOG_DIR=$TEST_ROOT/var/log/nix
 export NIX_STATE_DIR=$TEST_ROOT/var/nix
+export NIX_SQLITE_DATABASE=$NIX_STATE_DIR/db/db.sqlite
 export NIX_CONF_DIR=$TEST_ROOT/etc
 export NIX_DAEMON_SOCKET_PATH=$TEST_ROOT/dSocket
 unset NIX_USER_CONF_FILES
@@ -164,6 +165,10 @@ requireDaemonNewerThan () {
     isDaemonNewer "$1" || skipTest "Daemon is too old"
 }
 
+requireSqliteDatabase() {
+    [[ -f "$NIX_SQLITE_DATABASE" ]] || skipTest "SQLite database is not used for this store implementation"
+}
+
 canUseSandbox() {
     [[ ${_canUseSandbox-} ]]
 }

tests/functional/fetchGit.sh

@@ -53,17 +53,8 @@ out=$(nix eval --impure --raw --expr "builtins.fetchGit { url = \"file://$repo\"
 [[ $status == 1 ]]
 [[ $out =~ 'Cannot find Git revision' ]]
 
-# allow revs as refs (for 2.3 compat)
 [[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]]
 
-rm -rf "$TEST_ROOT/test-home"
-[[ $(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; allRefs = true; } + \"/differentbranch\")") = 'different file' ]]
-
-rm -rf "$TEST_ROOT/test-home"
-out=$(nix eval --raw --expr "builtins.readFile (builtins.fetchGit { url = \"file://$repo\"; rev = \"$devrev\"; ref = \"lolkek\"; } + \"/differentbranch\")" 2>&1) || status=$?
-[[ $status == 1 ]]
-[[ $out =~ 'Cannot find Git revision' ]]
-
 # In pure eval mode, fetchGit without a revision should fail.
 [[ $(nix eval --impure --raw --expr "builtins.readFile (fetchGit \"file://$repo\" + \"/hello\")") = world ]]
 (! nix eval --raw --expr "builtins.readFile (fetchGit \"file://$repo\" + \"/hello\")")
@@ -237,12 +228,6 @@ export _NIX_FORCE_HTTP=1
 rev_tag1_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag1\"; }).rev")
 rev_tag1=$(git -C $repo rev-parse refs/tags/tag1)
 [[ $rev_tag1_nix = $rev_tag1 ]]
-
-# Allow fetching tags w/o specifying refs/tags
-rm -rf "$TEST_ROOT/test-home"
-rev_tag1_nix_alt=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"tag1\"; }).rev")
-[[ $rev_tag1_nix_alt = $rev_tag1 ]]
-
 rev_tag2_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/tag2\"; }).rev")
 rev_tag2=$(git -C $repo rev-parse refs/tags/tag2)
 [[ $rev_tag2_nix = $rev_tag2 ]]
@@ -269,33 +254,3 @@ git -C "$repo" add hello .gitignore
 git -C "$repo" commit -m 'Bla1'
 cd "$repo"
 path11=$(nix eval --impure --raw --expr "(builtins.fetchGit ./.).outPath")
-
-# test behavior if both branch and tag with same name exist
-repo="$TEST_ROOT/git"
-rm -rf "$repo"/.git
-git init "$repo"
-git -C "$repo" config user.email "foobar@example.com"
-git -C "$repo" config user.name "Foobar"
-
-touch "$repo"/test
-echo "hello world" > "$repo"/test
-git -C "$repo" checkout -b branch
-git -C "$repo" add test
-
-git -C "$repo" commit -m "Init"
-
-git -C "$repo" tag branch
-
-echo "goodbye world" > "$repo"/test
-git -C "$repo" add test
-git -C "$repo" commit -m "Update test"
-
-path12=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"branch\"; }).outPath")
-[[ "$(cat "$path12"/test)" =~ 'hello world' ]]
-[[ "$(cat "$repo"/test)" =~ 'goodbye world' ]]
-
-path13=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/heads/branch\"; }).outPath")
-[[ "$(cat "$path13"/test)" =~ 'goodbye world' ]]
-
-path14=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"refs/tags/branch\"; }).outPath")
-[[ "$path14" = "$path12" ]]

tests/functional/fmt.sh

@@ -26,10 +26,7 @@ cat << EOF > flake.nix
   };
 }
 EOF
-# No arguments check
+nix fmt ./file ./folder | grep 'Formatting: ./file ./folder'
-[[ "$(nix fmt)" = "Formatting(0):" ]]
-# Argument forwarding check
-nix fmt ./file ./folder | grep 'Formatting(2): ./file ./folder'
 nix flake check
 nix flake show | grep -P "package 'formatter'"
 

tests/functional/fmt.simple.sh

@@ -1,3 +1,3 @@
 #!/usr/bin/env bash
 
-echo "Formatting(${#}):" "${@}"
+echo Formatting: "${@}"

tests/functional/meson.build

@@ -76,6 +76,7 @@ functional_tests_scripts = [
   'flakes/flake-registry.sh',
   'flakes/subdir-flake.sh',
   'gc.sh',
+  'phantom-referrers-gc.sh',
   'nix-collect-garbage-d.sh',
   'nix-collect-garbage-dry-run.sh',
   'remote-store.sh',

tests/functional/phantom-referrers-gc.sh

@@ -0,0 +1,62 @@
+source common.sh
+
+startDaemon
+
+requireDaemonNewerThan "2.92.0"
+requireSqliteDatabase
+
+clearStore
+
+depOutPath=$(nix-build --no-out-link -E '
+  with import ./config.nix;
+
+  mkDerivation {
+    name = "phantom";
+    outputs = [ "out" ];
+    buildCommand = "
+      echo i will become a phantom soon > $out
+    ";
+  }
+')
+
+finalOutPath=$(nix-build --no-out-link -E '
+  with import ./config.nix;
+
+  let dep = mkDerivation {
+    name = "phantom";
+    outputs = [ "out" ];
+    buildCommand = "
+      echo i will become a phantom soon > $out
+    ";
+  }; in
+
+  mkDerivation {
+    name = "phantom-gc";
+    outputs = [ "out" ];
+    buildCommand = "
+      echo UNUSED: ${dep} > $out
+    ";
+  }
+')
+
+echo "displaying all valid paths"
+sqlite3 "$NIX_SQLITE_DATABASE" <<EOF
+  select * from validpaths;
+EOF
+
+
+echo "displaying the relevant IDs..."
+sqlite3 "$NIX_SQLITE_DATABASE" <<EOF
+  select r.referrer, r.reference from Refs r join ValidPaths vp on r.referrer = vp.id where path = '$finalOutPath';
+EOF
+
+echo "corrupting the SQLite database manually..."
+sqlite3 "$NIX_SQLITE_DATABASE" <<EOF
+  pragma foreign_keys = off;
+  delete from ValidPaths where path = '$finalOutPath';
+  select * from Refs;
+EOF
+
+restartDaemon
+# expect this to work and maybe warn about phantom referrers
+expectStderr 0 nix-collect-garbage -vvvv | grepQuiet 'phantom referrers'

tests/nixos/default.nix

@@ -157,6 +157,4 @@ in
   coredumps = runNixOSTestFor "x86_64-linux" ./coredumps;
 
   io_uring = runNixOSTestFor "x86_64-linux" ./io_uring;
-
-  fetchurl = runNixOSTestFor "x86_64-linux" ./fetchurl.nix;
 }

tests/nixos/fetchurl.nix

@@ -1,84 +0,0 @@
-# Test whether builtin:fetchurl properly performs TLS certificate
-# checks on HTTPS servers.
-
-{ lib, config, pkgs, ... }:
-
-let
-
-  makeTlsCert = name: pkgs.runCommand name {
-    nativeBuildInputs = with pkgs; [ openssl ];
-  } ''
-    mkdir -p $out
-    openssl req -x509 \
-      -subj '/CN=${name}/' -days 49710 \
-      -addext 'subjectAltName = DNS:${name}' \
-      -keyout "$out/key.pem" -newkey ed25519 \
-      -out "$out/cert.pem" -noenc
-  '';
-
-  goodCert = makeTlsCert "good";
-  badCert = makeTlsCert "bad";
-
-in
-
-{
-  name = "fetchurl";
-
-  nodes = {
-    machine = { lib, pkgs, ... }: {
-      services.nginx = {
-        enable = true;
-
-        virtualHosts."good" = {
-          addSSL = true;
-          sslCertificate = "${goodCert}/cert.pem";
-          sslCertificateKey = "${goodCert}/key.pem";
-          root = pkgs.runCommand "nginx-root" {} ''
-            mkdir "$out"
-            echo 'hello world' > "$out/index.html"
-          '';
-        };
-
-        virtualHosts."bad" = {
-          addSSL = true;
-          sslCertificate = "${badCert}/cert.pem";
-          sslCertificateKey = "${badCert}/key.pem";
-          root = pkgs.runCommand "nginx-root" {} ''
-            mkdir "$out"
-            echo 'foobar' > "$out/index.html"
-          '';
-        };
-      };
-
-      security.pki.certificateFiles = [ "${goodCert}/cert.pem" ];
-
-      networking.hosts."127.0.0.1" = [ "good" "bad" ];
-
-      virtualisation.writableStore = true;
-
-      nix.settings.experimental-features = "nix-command";
-    };
-  };
-
-  testScript = { nodes, ... }: ''
-    machine.wait_for_unit("nginx")
-    machine.wait_for_open_port(443)
-
-    out = machine.succeed("curl https://good/index.html")
-    assert out == "hello world\n"
-
-    out = machine.succeed("cat ${badCert}/cert.pem > /tmp/cafile.pem; curl --cacert /tmp/cafile.pem https://bad/index.html")
-    assert out == "foobar\n"
-
-    # Fetching from a server with a trusted cert should work.
-    machine.succeed("nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://good/index.html\"; hash = \"sha256-qUiQTy8PR5uPgZdpSzAYSw0u0cHNKh7A+4XSmaGSpEc=\"; }'")
-
-    # Fetching from a server with an untrusted cert should fail.
-    err = machine.fail("nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://bad/index.html\"; hash = \"sha256-rsBwZF/lPuOzdjBZN2E08FjMM3JHyXit0Xi2zN+wAZ8=\"; }' 2>&1")
-    print(err)
-    assert "SSL certificate problem: self-signed certificate" in err or "SSL peer certificate or SSH remote key was not OK" in err
-
-    # Fetching from a server with a trusted cert should work via environment variable override.
-    machine.succeed("NIX_SSL_CERT_FILE=/tmp/cafile.pem nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://bad/index.html\"; hash = \"sha256-rsBwZF/lPuOzdjBZN2E08FjMM3JHyXit0Xi2zN+wAZ8=\"; }'")
-  '';
-}

tests/unit/libmain/crash.cc

@@ -1,56 +0,0 @@
-#include <gtest/gtest.h>
-#include "crash-handler.hh"
-
-namespace nix {
-
-class OopsException : public std::exception
-{
-    const char * msg;
-
-public:
-    OopsException(const char * msg) : msg(msg) {}
-    const char * what() const noexcept override
-    {
-        return msg;
-    }
-};
-
-void causeCrashForTesting(std::function<void()> fixture)
-{
-    registerCrashHandler();
-    std::cerr << "time to crash\n";
-    try {
-        fixture();
-    } catch (...) {
-        std::terminate();
-    }
-}
-
-TEST(CrashHandler, exceptionName)
-{
-    ASSERT_DEATH(
-        causeCrashForTesting([]() { throw OopsException{"lol oops"}; }),
-        "time to crash\nLix crashed.*OopsException: lol oops"
-    );
-}
-
-TEST(CrashHandler, unknownTerminate)
-{
-    ASSERT_DEATH(
-        causeCrashForTesting([]() { std::terminate(); }),
-        "time to crash\nLix crashed.*std::terminate\\(\\) called without exception"
-    );
-}
-
-TEST(CrashHandler, nonStdException)
-{
-    ASSERT_DEATH(
-        causeCrashForTesting([]() {
-            // NOLINTNEXTLINE(hicpp-exception-baseclass): intentional
-            throw 4;
-        }),
-        "time to crash\nLix crashed.*Unknown exception! Spooky\\."
-    );
-}
-
-}

tests/unit/libutil/async-collect.cc

@@ -1,104 +0,0 @@
-#include "async-collect.hh"
-
-#include <gtest/gtest.h>
-#include <kj/array.h>
-#include <kj/async.h>
-#include <kj/exception.h>
-#include <stdexcept>
-
-namespace nix {
-
-TEST(AsyncCollect, void)
-{
-    kj::EventLoop loop;
-    kj::WaitScope waitScope(loop);
-
-    auto a = kj::newPromiseAndFulfiller<void>();
-    auto b = kj::newPromiseAndFulfiller<void>();
-    auto c = kj::newPromiseAndFulfiller<void>();
-    auto d = kj::newPromiseAndFulfiller<void>();
-
-    auto collect = asyncCollect(kj::arr(
-        std::pair(1, std::move(a.promise)),
-        std::pair(2, std::move(b.promise)),
-        std::pair(3, std::move(c.promise)),
-        std::pair(4, std::move(d.promise))
-    ));
-
-    auto p = collect.next();
-    ASSERT_FALSE(p.poll(waitScope));
-
-    // collection is ordered
-    c.fulfiller->fulfill();
-    b.fulfiller->fulfill();
-
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_EQ(p.wait(waitScope), 3);
-
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_EQ(p.wait(waitScope), 2);
-
-    p = collect.next();
-    ASSERT_FALSE(p.poll(waitScope));
-
-    // exceptions propagate
-    a.fulfiller->rejectIfThrows([] { throw std::runtime_error("test"); });
-
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_THROW(p.wait(waitScope), kj::Exception);
-
-    // first exception aborts collection
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_THROW(p.wait(waitScope), kj::Exception);
-}
-
-TEST(AsyncCollect, nonVoid)
-{
-    kj::EventLoop loop;
-    kj::WaitScope waitScope(loop);
-
-    auto a = kj::newPromiseAndFulfiller<int>();
-    auto b = kj::newPromiseAndFulfiller<int>();
-    auto c = kj::newPromiseAndFulfiller<int>();
-    auto d = kj::newPromiseAndFulfiller<int>();
-
-    auto collect = asyncCollect(kj::arr(
-        std::pair(1, std::move(a.promise)),
-        std::pair(2, std::move(b.promise)),
-        std::pair(3, std::move(c.promise)),
-        std::pair(4, std::move(d.promise))
-    ));
-
-    auto p = collect.next();
-    ASSERT_FALSE(p.poll(waitScope));
-
-    // collection is ordered
-    c.fulfiller->fulfill(1);
-    b.fulfiller->fulfill(2);
-
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_EQ(p.wait(waitScope), std::pair(3, 1));
-
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_EQ(p.wait(waitScope), std::pair(2, 2));
-
-    p = collect.next();
-    ASSERT_FALSE(p.poll(waitScope));
-
-    // exceptions propagate
-    a.fulfiller->rejectIfThrows([] { throw std::runtime_error("test"); });
-
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_THROW(p.wait(waitScope), kj::Exception);
-
-    // first exception aborts collection
-    p = collect.next();
-    ASSERT_TRUE(p.poll(waitScope));
-    ASSERT_THROW(p.wait(waitScope), kj::Exception);
-}
-}

tests/unit/libutil/async-semaphore.cc

@@ -1,74 +0,0 @@
-#include "async-semaphore.hh"
-
-#include <gtest/gtest.h>
-#include <kj/async.h>
-
-namespace nix {
-
-TEST(AsyncSemaphore, counting)
-{
-    kj::EventLoop loop;
-    kj::WaitScope waitScope(loop);
-
-    AsyncSemaphore sem(2);
-
-    ASSERT_EQ(sem.available(), 2);
-    ASSERT_EQ(sem.used(), 0);
-
-    auto a = kj::evalNow([&] { return sem.acquire(); });
-    ASSERT_EQ(sem.available(), 1);
-    ASSERT_EQ(sem.used(), 1);
-    auto b = kj::evalNow([&] { return sem.acquire(); });
-    ASSERT_EQ(sem.available(), 0);
-    ASSERT_EQ(sem.used(), 2);
-
-    auto c = kj::evalNow([&] { return sem.acquire(); });
-    auto d = kj::evalNow([&] { return sem.acquire(); });
-
-    ASSERT_TRUE(a.poll(waitScope));
-    ASSERT_TRUE(b.poll(waitScope));
-    ASSERT_FALSE(c.poll(waitScope));
-    ASSERT_FALSE(d.poll(waitScope));
-
-    a = nullptr;
-    ASSERT_TRUE(c.poll(waitScope));
-    ASSERT_FALSE(d.poll(waitScope));
-
-    {
-        auto lock = b.wait(waitScope);
-        ASSERT_FALSE(d.poll(waitScope));
-    }
-
-    ASSERT_TRUE(d.poll(waitScope));
-
-    ASSERT_EQ(sem.available(), 0);
-    ASSERT_EQ(sem.used(), 2);
-    c = nullptr;
-    ASSERT_EQ(sem.available(), 1);
-    ASSERT_EQ(sem.used(), 1);
-    d = nullptr;
-    ASSERT_EQ(sem.available(), 2);
-    ASSERT_EQ(sem.used(), 0);
-}
-
-TEST(AsyncSemaphore, cancelledWaiter)
-{
-    kj::EventLoop loop;
-    kj::WaitScope waitScope(loop);
-
-    AsyncSemaphore sem(1);
-
-    auto a = kj::evalNow([&] { return sem.acquire(); });
-    auto b = kj::evalNow([&] { return sem.acquire(); });
-    auto c = kj::evalNow([&] { return sem.acquire(); });
-
-    ASSERT_TRUE(a.poll(waitScope));
-    ASSERT_FALSE(b.poll(waitScope));
-
-    b = nullptr;
-    a = nullptr;
-
-    ASSERT_TRUE(c.poll(waitScope));
-}
-
-}

tests/unit/libutil/compression.cc

@@ -3,161 +3,105 @@
 
 namespace nix {
 
-/* ----------------------------------------------------------------------------
+    /* ----------------------------------------------------------------------------
- * compress / decompress
+     * compress / decompress
- * --------------------------------------------------------------------------*/
+     * --------------------------------------------------------------------------*/
 
-TEST(compress, compressWithUnknownMethod)
+    TEST(compress, compressWithUnknownMethod) {
-{
+        ASSERT_THROW(compress("invalid-method", "something-to-compress"), UnknownCompressionMethod);
-    ASSERT_THROW(compress("invalid-method", "something-to-compress"), UnknownCompressionMethod);
+    }
-}
+
-
+    TEST(compress, noneMethodDoesNothingToTheInput) {
-TEST(compress, noneMethodDoesNothingToTheInput)
+        auto o = compress("none", "this-is-a-test");
-{
+
-    auto o = compress("none", "this-is-a-test");
+        ASSERT_EQ(o, "this-is-a-test");
-
+    }
-    ASSERT_EQ(o, "this-is-a-test");
+
-}
+    TEST(decompress, decompressNoneCompressed) {
-
+        auto method = "none";
-TEST(decompress, decompressEmptyString)
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-{
+        auto o = decompress(method, str);
-    // Empty-method decompression used e.g. by S3 store
+
-    // (Content-Encoding == "").
+        ASSERT_EQ(o, str);
-    auto o = decompress("", "this-is-a-test");
+    }
-
+
-    ASSERT_EQ(o, "this-is-a-test");
+    TEST(decompress, decompressEmptyCompressed) {
-}
+        // Empty-method decompression used e.g. by S3 store
-
+        // (Content-Encoding == "").
-/* ----------------------------------------------------------------------------
+        auto method = "";
- * compression sinks
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
- * --------------------------------------------------------------------------*/
+        auto o = decompress(method, str);
-
+
-TEST(makeCompressionSink, noneSinkDoesNothingToInput)
+        ASSERT_EQ(o, str);
-{
+    }
-    auto method = "none";
+
-    StringSink strSink;
+    TEST(decompress, decompressXzCompressed) {
-    auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+        auto method = "xz";
-    auto sink = makeCompressionSink(method, strSink);
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-    (*sink)(inputString);
+        auto o = decompress(method, compress(method, str));
-    sink->finish();
+
-
+        ASSERT_EQ(o, str);
-    ASSERT_STREQ(strSink.s.c_str(), inputString);
+    }
-}
+
-
+    TEST(decompress, decompressBzip2Compressed) {
-/** Tests applied to all compression types */
+        auto method = "bzip2";
-class PerTypeCompressionTest : public testing::TestWithParam<const char *>
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-{};
+        auto o = decompress(method, compress(method, str));
-
+
-/** Tests applied to non-passthrough compression types */
+        ASSERT_EQ(o, str);
-class PerTypeNonNullCompressionTest : public testing::TestWithParam<const char *>
+    }
-{};
+
-
+    TEST(decompress, decompressBrCompressed) {
-constexpr const char * COMPRESSION_TYPES_NONNULL[] = {
+        auto method = "br";
-    // libarchive
+        auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-    "bzip2",
+        auto o = decompress(method, compress(method, str));
-    "compress",
+
-    "gzip",
+        ASSERT_EQ(o, str);
-    "lzip",
+    }
-    "lzma",
+
-    "xz",
+    TEST(decompress, decompressInvalidInputThrowsCompressionError) {
-    "zstd",
+        auto method = "bzip2";
-    // Uses external program via libarchive so cannot be used :(
+        auto str = "this is a string that does not qualify as valid bzip2 data";
-    /*
+
-    "grzip",
+        ASSERT_THROW(decompress(method, str), CompressionError);
-    "lrzip",
+    }
-    "lzop",
+
-    "lz4",
+    TEST(decompress, veryLongBrotli) {
-    */
+        auto method = "br";
-    // custom
+        auto str = std::string(65536, 'a');
-    "br",
+        auto o = decompress(method, compress(method, str));
-};
+
-
+        // This is just to not print 64k of "a" for most failures
-INSTANTIATE_TEST_SUITE_P(
+        ASSERT_EQ(o.length(), str.length());
-    compressionNonNull, PerTypeNonNullCompressionTest, testing::ValuesIn(COMPRESSION_TYPES_NONNULL)
+        ASSERT_EQ(o, str);
-);
+    }
-INSTANTIATE_TEST_SUITE_P(
+
-    compressionNonNull, PerTypeCompressionTest, testing::ValuesIn(COMPRESSION_TYPES_NONNULL)
+    /* ----------------------------------------------------------------------------
-);
+     * compression sinks
-
+     * --------------------------------------------------------------------------*/
-INSTANTIATE_TEST_SUITE_P(
+
-    compressionNull, PerTypeCompressionTest, testing::Values("none")
+    TEST(makeCompressionSink, noneSinkDoesNothingToInput) {
-);
+        StringSink strSink;
-
+        auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-/* ---------------------------------------
+        auto sink = makeCompressionSink("none", strSink);
- * All compression types
+        (*sink)(inputString);
- * --------------------------------------- */
+        sink->finish();
-
+
-TEST_P(PerTypeCompressionTest, roundTrips)
+        ASSERT_STREQ(strSink.s.c_str(), inputString);
-{
+    }
-    auto method = GetParam();
+
-    auto str = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
+    TEST(makeCompressionSink, compressAndDecompress) {
-    auto o = decompress(method, compress(method, str));
+        auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-
+
-    ASSERT_EQ(o, str);
+        StringSink strSink;
-}
+        auto sink = makeCompressionSink("bzip2", strSink);
-
+        (*sink)(inputString);
-TEST_P(PerTypeCompressionTest, longerThanBuffer)
+        sink->finish();
-{
+
-    // This is targeted originally at regression testing a brotli bug, but we might as well do it to
+        StringSource strSource{strSink.s};
-    // everything
+        auto decompressionSource = makeDecompressionSource("bzip2", strSource);
-    auto method = GetParam();
+
-    auto str = std::string(65536, 'a');
+        ASSERT_STREQ(decompressionSource->drain().c_str(), inputString);
-    auto o = decompress(method, compress(method, str));
-
-    // This is just to not print 64k of "a" for most failures
-    ASSERT_EQ(o.length(), str.length());
-    ASSERT_EQ(o, str);
-}
-
-TEST_P(PerTypeCompressionTest, sinkAndSource)
-{
-    auto method = GetParam();
-    auto inputString = "slfja;sljfklsa;jfklsjfkl;sdjfkl;sadjfkl;sdjf;lsdfjsadlf";
-
-    StringSink strSink;
-    auto sink = makeCompressionSink(method, strSink);
-    (*sink)(inputString);
-    sink->finish();
-
-    StringSource strSource{strSink.s};
-    auto decompressionSource = makeDecompressionSource(method, strSource);
-
-    ASSERT_STREQ(decompressionSource->drain().c_str(), inputString);
-}
-
-/* ---------------------------------------
- * Non null compression types
- * --------------------------------------- */
-
-TEST_P(PerTypeNonNullCompressionTest, bogusInputDecompression)
-{
-    auto param = GetParam();
-
-    auto bogus = "this data is bogus and should throw when decompressing";
-    ASSERT_THROW(decompress(param, bogus), CompressionError);
-}
-
-TEST_P(PerTypeNonNullCompressionTest, truncatedValidInput)
-{
-    auto method = GetParam();
-
-    auto inputString = "the quick brown fox jumps over the lazy doggos";
-    auto compressed = compress(method, inputString);
-
-    /* n.b. This also tests zero-length input, which is also invalid.
-     * As of the writing of this comment, it returns empty output, but is
-     * allowed to throw a compression error instead. */
-    for (int i = 0; i < compressed.length(); ++i) {
-        auto newCompressed = compressed.substr(compressed.length() - i);
-        try {
-            decompress(method, newCompressed);
-            // Success is acceptable as well, even though it is corrupt data.
-            // The compression method is not expected to provide integrity,
-            // just, not break explosively on bad input.
-        } catch (CompressionError &) {
-            // Acceptable
-        }
     }
-}
 
 }

tests/unit/meson.build

@@ -39,8 +39,6 @@ liblixutil_test_support = declare_dependency(
 )
 
 libutil_tests_sources = files(
-  'libutil/async-collect.cc',
-  'libutil/async-semaphore.cc',
   'libutil/canon-path.cc',
   'libutil/checked-arithmetic.cc',
   'libutil/chunked-vector.cc',
@@ -78,7 +76,6 @@ libutil_tester = executable(
     liblixexpr_mstatic,
     liblixutil_test_support,
     nlohmann_json,
-    kj,
   ],
   cpp_pch : cpp_pch,
 )
@@ -265,14 +262,9 @@ test(
   protocol : 'gtest',
 )
 
-libmain_tests_sources = files(
-  'libmain/crash.cc',
-  'libmain/progress-bar.cc',
-)
-
 libmain_tester = executable(
   'liblixmain-tests',
-  libmain_tests_sources,
+  files('libmain/progress-bar.cc'),
   dependencies : [
     liblixmain,
     liblixexpr,