From 82ddb130984c7bdc45cdffc14e81bed720089200 Mon Sep 17 00:00:00 2001
From: Graham Bennett <bennettg@pdtpartners.com>
Date: Fri, 29 Apr 2022 15:15:25 -0400
Subject: [PATCH 01/45] Unlock output paths when a derivation is already built

Without this change, nix build processes will not drop the locks for derivation goals
which have already been built by another process when the current process gets
round to building them. This means the locks are held until the process
terminates.

If there are other nix build processes in a similar state, they will also try to
acquire the same locks when they try to build the same derivation, and so will
wait until the lock holder terminates (which might be a very long time if it has
a lot to build). In some pathological cases, those processes might be holding
their own locks on other derivations due to the same issue, and this can lead to
deadlock.

Resolves #6468
---
 src/libstore/build/derivation-goal.cc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 6472ecd99..befbfd10e 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1456,6 +1456,7 @@ void DerivationGoal::done(
     SingleDrvOutputs builtOutputs,
     std::optional<Error> ex)
 {
+    outputLocks.unlock();
     buildResult.status = status;
     if (ex)
         buildResult.errorMsg = fmt("%s", normaltxt(ex->info().msg));

From 60b363936d2fd53ac8741d35ba30ff1e4c405a9f Mon Sep 17 00:00:00 2001
From: r-vdp <ramses@well-founded.dev>
Date: Tue, 31 Oct 2023 17:32:09 +0100
Subject: [PATCH 02/45] libstore/ssh-ng: Fix phase reporting in log files.

When doing local builds, we get phase reporting lines in the log file,
they look like '@nix {"action":"setPhase","phase":"unpackPhase"}'.
With the ssh-ng protocol, we do have access to these messages, but since we
are only including messages of type resBuildLogLine in the logs, the phase
information does not end up in the log file.

The phase reporting could probably be improved altoghether (it looks like it
is kind of accidental that these JSON messages for phase reporting show up
but others don't, just because they are actually emitted by nixpkgs' stdenv),
but as a first step I propose to make ssh-ng behave in the same way as local builds do.
---
 src/libstore/build/derivation-goal.cc |  23 +++++-
 tests/nixos/default.nix               |   2 +
 tests/nixos/remote-builds-ssh-ng.nix  | 108 ++++++++++++++++++++++++++
 3 files changed, 130 insertions(+), 3 deletions(-)
 create mode 100644 tests/nixos/remote-builds-ssh-ng.nix

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 360c6b70b..0cfa9a148 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -1317,9 +1317,26 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data)
                     auto s = handleJSONLogMessage(*json, worker.act, hook->activities, true);
                     // ensure that logs from a builder using `ssh-ng://` as protocol
                     // are also available to `nix log`.
-                    if (s && !isWrittenToLog && logSink && (*json)["type"] == resBuildLogLine) {
-                        auto f = (*json)["fields"];
-                        (*logSink)((f.size() > 0 ? f.at(0).get<std::string>() : "") + "\n");
+                    if (s && !isWrittenToLog && logSink) {
+                        const auto type = (*json)["type"];
+                        const auto fields = (*json)["fields"];
+                        if (type == resBuildLogLine) {
+                            (*logSink)((fields.size() > 0 ? fields[0].get<std::string>() : "") + "\n");
+                        } else if (type == resSetPhase && ! fields.is_null()) {
+                            const auto phase = fields[0];
+                            if (! phase.is_null()) {
+                                // nixpkgs' stdenv produces lines in the log to signal
+                                // phase changes.
+                                // We want to get the same lines in case of remote builds.
+                                // The format is:
+                                //   @nix { "action": "setPhase", "phase": "$curPhase" }
+                                const auto logLine = nlohmann::json::object({
+                                    {"action", "setPhase"},
+                                    {"phase", phase}
+                                });
+                                (*logSink)("@nix " + logLine.dump(-1, ' ', false, nlohmann::json::error_handler_t::replace) + "\n");
+                            }
+                        }
                     }
                 }
                 currentHookLine.clear();
diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix
index b391d7ef2..4459aa664 100644
--- a/tests/nixos/default.nix
+++ b/tests/nixos/default.nix
@@ -21,6 +21,8 @@ in
 
   remoteBuilds = runNixOSTestFor "x86_64-linux" ./remote-builds.nix;
 
+  remoteBuildsSshNg = runNixOSTestFor "x86_64-linux" ./remote-builds-ssh-ng.nix;
+
   nix-copy-closure = runNixOSTestFor "x86_64-linux" ./nix-copy-closure.nix;
 
   nix-copy = runNixOSTestFor "x86_64-linux" ./nix-copy.nix;
diff --git a/tests/nixos/remote-builds-ssh-ng.nix b/tests/nixos/remote-builds-ssh-ng.nix
new file mode 100644
index 000000000..b59dde9bf
--- /dev/null
+++ b/tests/nixos/remote-builds-ssh-ng.nix
@@ -0,0 +1,108 @@
+{ config, lib, hostPkgs, ... }:
+
+let
+  pkgs = config.nodes.client.nixpkgs.pkgs;
+
+  # Trivial Nix expression to build remotely.
+  expr = config: nr: pkgs.writeText "expr.nix"
+    ''
+      let utils = builtins.storePath ${config.system.build.extraUtils}; in
+      derivation {
+        name = "hello-${toString nr}";
+        system = "i686-linux";
+        PATH = "''${utils}/bin";
+        builder = "''${utils}/bin/sh";
+        args = [ "-c" "${
+          lib.concatStringsSep "; " [
+            ''if [[ -n $NIX_LOG_FD ]]''
+            ''then echo '@nix {\"action\":\"setPhase\",\"phase\":\"buildPhase\"}' >&''$NIX_LOG_FD''
+            "fi"
+            "echo Hello"
+            "mkdir $out"
+            "cat /proc/sys/kernel/hostname > $out/host"
+          ]
+        }" ];
+        outputs = [ "out" ];
+      }
+    '';
+in
+
+{
+  name = "remote-builds-ssh-ng";
+
+  nodes =
+    { builder =
+      { config, pkgs, ... }:
+      { services.openssh.enable = true;
+        virtualisation.writableStore = true;
+        nix.settings.sandbox = true;
+        nix.settings.substituters = lib.mkForce [ ];
+      };
+
+      client =
+        { config, lib, pkgs, ... }:
+        { nix.settings.max-jobs = 0; # force remote building
+          nix.distributedBuilds = true;
+          nix.buildMachines =
+            [ { hostName = "builder";
+                sshUser = "root";
+                sshKey = "/root/.ssh/id_ed25519";
+                system = "i686-linux";
+                maxJobs = 1;
+                protocol = "ssh-ng";
+              }
+            ];
+          virtualisation.writableStore = true;
+          virtualisation.additionalPaths = [ config.system.build.extraUtils ];
+          nix.settings.substituters = lib.mkForce [ ];
+          programs.ssh.extraConfig = "ConnectTimeout 30";
+        };
+    };
+
+  testScript = { nodes }: ''
+    # fmt: off
+    import subprocess
+
+    start_all()
+
+    # Create an SSH key on the client.
+    subprocess.run([
+      "${hostPkgs.openssh}/bin/ssh-keygen", "-t", "ed25519", "-f", "key", "-N", ""
+    ], capture_output=True, check=True)
+    client.succeed("mkdir -p -m 700 /root/.ssh")
+    client.copy_from_host("key", "/root/.ssh/id_ed25519")
+    client.succeed("chmod 600 /root/.ssh/id_ed25519")
+
+    # Install the SSH key on the builder.
+    client.wait_for_unit("network.target")
+    builder.succeed("mkdir -p -m 700 /root/.ssh")
+    builder.copy_from_host("key.pub", "/root/.ssh/authorized_keys")
+    builder.wait_for_unit("sshd")
+    client.succeed(f"ssh -o StrictHostKeyChecking=no {builder.name} 'echo hello world'")
+
+    # Perform a build
+    out = client.succeed("nix-build ${expr nodes.client.config 1} 2> build-output")
+
+    # Verify that the build was done on the builder
+    builder.succeed(f"test -e {out.strip()}")
+
+    # Print the build log, prefix the log lines to avoid nix intercepting lines starting with @nix
+    buildOutput = client.succeed("sed -e 's/^/build-output:/' build-output")
+    print(buildOutput)
+
+    # Make sure that we get the expected build output
+    client.succeed("grep -qF Hello build-output")
+
+    # We don't want phase reporting in the build output
+    client.fail("grep -qF '@nix' build-output")
+
+    # Get the log file
+    client.succeed(f"nix-store --read-log {out.strip()} > log-output")
+    # Prefix the log lines to avoid nix intercepting lines starting with @nix
+    logOutput = client.succeed("sed -e 's/^/log-file:/' log-output")
+    print(logOutput)
+
+    # Check that we get phase reporting in the log file
+    client.succeed("grep -q '@nix {\"action\":\"setPhase\",\"phase\":\"buildPhase\"}' log-output")
+  '';
+}

From 1d5a48240cd3c5b81939b0562141772323550d99 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Thu, 9 Nov 2023 23:10:42 -0500
Subject: [PATCH 03/45] `.editorconfig`: Also affect Perl FFI `xs` file

This way `perl/lib/Nix/Store.xs` is affected.
---
 .editorconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.editorconfig b/.editorconfig
index 887ecadba..86360e658 100644
--- a/.editorconfig
+++ b/.editorconfig
@@ -17,7 +17,7 @@ indent_style = space
 indent_size = 2
 
 # Match c++/shell/perl, set indent to spaces with width of four
-[*.{hpp,cc,hh,sh,pl}]
+[*.{hpp,cc,hh,sh,pl,xs}]
 indent_style = space
 indent_size = 4
 

From 3d9d5dc18977d21a04299f4a37b366f9a1d32051 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 3 Nov 2023 00:57:19 -0400
Subject: [PATCH 04/45] Create `MemorySink`

This is for writing to a `MemorySourceAccessor`.
---
 src/libutil/memory-source-accessor.cc | 56 +++++++++++++++++++++++++++
 src/libutil/memory-source-accessor.hh | 25 ++++++++++++
 2 files changed, 81 insertions(+)

diff --git a/src/libutil/memory-source-accessor.cc b/src/libutil/memory-source-accessor.cc
index f34f6c091..78a4dd298 100644
--- a/src/libutil/memory-source-accessor.cc
+++ b/src/libutil/memory-source-accessor.cc
@@ -121,4 +121,60 @@ CanonPath MemorySourceAccessor::addFile(CanonPath path, std::string && contents)
     return path;
 }
 
+
+using File = MemorySourceAccessor::File;
+
+void MemorySink::createDirectory(const Path & path)
+{
+    auto * f = dst.open(CanonPath{path}, File { File::Directory { } });
+    if (!f)
+        throw Error("file '%s' cannot be made because some parent file is not a directory", path);
+
+    if (!std::holds_alternative<File::Directory>(f->raw))
+        throw Error("file '%s' is not a directory", path);
+};
+
+void MemorySink::createRegularFile(const Path & path)
+{
+    auto * f = dst.open(CanonPath{path}, File { File::Regular {} });
+    if (!f)
+        throw Error("file '%s' cannot be made because some parent file is not a directory", path);
+    if (!(r = std::get_if<File::Regular>(&f->raw)))
+        throw Error("file '%s' is not a regular file", path);
+}
+
+void MemorySink::closeRegularFile()
+{
+    r = nullptr;
+}
+
+void MemorySink::isExecutable()
+{
+    assert(r);
+    r->executable = true;
+}
+
+void MemorySink::preallocateContents(uint64_t len)
+{
+    assert(r);
+    r->contents.reserve(len);
+}
+
+void MemorySink::receiveContents(std::string_view data)
+{
+    assert(r);
+    r->contents += data;
+}
+
+void MemorySink::createSymlink(const Path & path, const std::string & target)
+{
+    auto * f = dst.open(CanonPath{path}, File { File::Symlink { } });
+    if (!f)
+        throw Error("file '%s' cannot be made because some parent file is not a directory", path);
+    if (auto * s = std::get_if<File::Symlink>(&f->raw))
+        s->target = target;
+    else
+        throw Error("file '%s' is not a symbolic link", path);
+}
+
 }
diff --git a/src/libutil/memory-source-accessor.hh b/src/libutil/memory-source-accessor.hh
index 014fa8098..b908f3713 100644
--- a/src/libutil/memory-source-accessor.hh
+++ b/src/libutil/memory-source-accessor.hh
@@ -1,4 +1,5 @@
 #include "source-accessor.hh"
+#include "fs-sink.hh"
 #include "variant-wrapper.hh"
 
 namespace nix {
@@ -71,4 +72,28 @@ struct MemorySourceAccessor : virtual SourceAccessor
     CanonPath addFile(CanonPath path, std::string && contents);
 };
 
+/**
+ * Write to a `MemorySourceAccessor` at the given path
+ */
+struct MemorySink : ParseSink
+{
+    MemorySourceAccessor & dst;
+
+    MemorySink(MemorySourceAccessor & dst) : dst(dst) { }
+
+    void createDirectory(const Path & path) override;
+
+    void createRegularFile(const Path & path) override;
+    void receiveContents(std::string_view data) override;
+    void isExecutable() override;
+    void closeRegularFile() override;
+
+    void createSymlink(const Path & path, const std::string & target) override;
+
+    void preallocateContents(uint64_t size) override;
+
+private:
+    MemorySourceAccessor::File::Regular * r;
+};
+
 }

From 9afa697ab61ea6bbbb0d88e629b62606681cc744 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Wed, 8 Nov 2023 00:30:55 -0500
Subject: [PATCH 05/45] Refactor bash test build system a bit

The basic idea here is to separate a few intertwined notions:

1. Not all "run bash tests" are "install tests"

2. Not all "run bash tests" use `tests/functional/init.sh`, or any
   pre-test initialization at all.

This will used in the next commit when we have a test that check unit
test golden master data.

Also, move our custom `PS4` from the test to the test runner, as it is
part of how we want to display the tests, not the test themselves.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
---
 doc/manual/src/contributing/testing.md        | 10 ++++----
 mk/common-test.sh                             | 24 ++++++++++++++-----
 mk/debug-test.sh                              |  5 +++-
 mk/lib.mk                                     |  7 +++---
 mk/run-test.sh                                |  5 +++-
 mk/tests.mk                                   | 21 +++++++++-------
 .../common/vars-and-functions.sh.in           |  2 +-
 7 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md
index 3d75ebe7b..0b45b88a3 100644
--- a/doc/manual/src/contributing/testing.md
+++ b/doc/manual/src/contributing/testing.md
@@ -133,17 +133,17 @@ ran test tests/functional/${testName}.sh... [PASS]
 or without `make`:
 
 ```shell-session
-$ ./mk/run-test.sh tests/functional/${testName}.sh
+$ ./mk/run-test.sh tests/functional/${testName}.sh tests/functional/init.sh
 ran test tests/functional/${testName}.sh... [PASS]
 ```
 
 To see the complete output, one can also run:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/functional/${testName}.sh
-+ foo
+$ ./mk/debug-test.sh tests/functional/${testName}.sh tests/functional/init.sh
++(${testName}.sh:1) foo
 output from foo
-+ bar
++(${testName}.sh:2) bar
 output from bar
 ...
 ```
@@ -175,7 +175,7 @@ edit it like so:
 Then, running the test with `./mk/debug-test.sh` will drop you into GDB once the script reaches that point:
 
 ```shell-session
-$ ./mk/debug-test.sh tests/functional/${testName}.sh
+$ ./mk/debug-test.sh tests/functional/${testName}.sh tests/functional/init.sh
 ...
 + gdb blash blub
 GNU gdb (GDB) 12.1
diff --git a/mk/common-test.sh b/mk/common-test.sh
index 7ab25febf..00ccd1584 100644
--- a/mk/common-test.sh
+++ b/mk/common-test.sh
@@ -1,15 +1,27 @@
-test_dir=tests/functional
+# Remove overall test dir (at most one of the two should match) and
+# remove file extension.
+test_name=$(echo -n "$test" | sed \
+    -e "s|^unit-test-data/||" \
+    -e "s|^tests/functional/||" \
+    -e "s|\.sh$||" \
+    )
 
-test=$(echo -n "$test" | sed -e "s|^$test_dir/||")
-
-TESTS_ENVIRONMENT=("TEST_NAME=${test%.*}" 'NIX_REMOTE=')
+TESTS_ENVIRONMENT=(
+    "TEST_NAME=$test_name"
+    'NIX_REMOTE='
+    'PS4=+(${BASH_SOURCE[0]-$0}:$LINENO) '
+)
 
 : ${BASH:=/usr/bin/env bash}
 
+run () {
+   cd "$(dirname $1)" && env "${TESTS_ENVIRONMENT[@]}" $BASH -x -e -u -o pipefail $(basename $1)
+}
+
 init_test () {
-   cd "$test_dir" && env "${TESTS_ENVIRONMENT[@]}" $BASH -e init.sh 2>/dev/null > /dev/null
+   run "$init" 2>/dev/null > /dev/null
 }
 
 run_test_proper () {
-   cd "$test_dir/$(dirname $test)" && env "${TESTS_ENVIRONMENT[@]}" $BASH -e $(basename $test)
+   run "$test"
 }
diff --git a/mk/debug-test.sh b/mk/debug-test.sh
index b5b628ecd..52482c01e 100755
--- a/mk/debug-test.sh
+++ b/mk/debug-test.sh
@@ -3,9 +3,12 @@
 set -eu -o pipefail
 
 test=$1
+init=${2-}
 
 dir="$(dirname "${BASH_SOURCE[0]}")"
 source "$dir/common-test.sh"
 
-(init_test)
+if [ -n "$init" ]; then
+    (init_test)
+fi
 run_test_proper
diff --git a/mk/lib.mk b/mk/lib.mk
index e86a7f1a4..49abe9862 100644
--- a/mk/lib.mk
+++ b/mk/lib.mk
@@ -122,14 +122,15 @@ $(foreach script, $(bin-scripts), $(eval $(call install-program-in,$(script),$(b
 $(foreach script, $(bin-scripts), $(eval programs-list += $(script)))
 $(foreach script, $(noinst-scripts), $(eval programs-list += $(script)))
 $(foreach template, $(template-files), $(eval $(call instantiate-template,$(template))))
+install_test_init=tests/functional/init.sh
 $(foreach test, $(install-tests), \
-  $(eval $(call run-install-test,$(test))) \
+  $(eval $(call run-test,$(test),$(install_test_init))) \
   $(eval installcheck: $(test).test))
 $(foreach test-group, $(install-tests-groups), \
-  $(eval $(call run-install-test-group,$(test-group))) \
+  $(eval $(call run-test-group,$(test-group),$(install_test_init))) \
   $(eval installcheck: $(test-group).test-group) \
   $(foreach test, $($(test-group)-tests), \
-    $(eval $(call run-install-test,$(test))) \
+    $(eval $(call run-test,$(test),$(install_test_init))) \
     $(eval $(test-group).test-group: $(test).test)))
 
 $(foreach file, $(man-pages), $(eval $(call install-data-in, $(file), $(mandir)/man$(patsubst .%,%,$(suffix $(file))))))
diff --git a/mk/run-test.sh b/mk/run-test.sh
index 1a1d65930..da9c5a473 100755
--- a/mk/run-test.sh
+++ b/mk/run-test.sh
@@ -8,6 +8,7 @@ yellow=""
 normal=""
 
 test=$1
+init=${2-}
 
 dir="$(dirname "${BASH_SOURCE[0]}")"
 source "$dir/common-test.sh"
@@ -21,7 +22,9 @@ if [ -t 1 ]; then
 fi
 
 run_test () {
-    (init_test 2>/dev/null > /dev/null)
+    if [ -n "$init" ]; then
+        (init_test 2>/dev/null > /dev/null)
+    fi
     log="$(run_test_proper 2>&1)" && status=0 || status=$?
 }
 
diff --git a/mk/tests.mk b/mk/tests.mk
index ec8128bdf..bac9b704a 100644
--- a/mk/tests.mk
+++ b/mk/tests.mk
@@ -2,19 +2,22 @@
 
 test-deps =
 
-define run-install-test
+define run-bash
 
-  .PHONY: $1.test
-  $1.test: $1 $(test-deps)
-	@env BASH=$(bash) $(bash) mk/run-test.sh $1 < /dev/null
-
-  .PHONY: $1.test-debug
-  $1.test-debug: $1 $(test-deps)
-	@env BASH=$(bash) $(bash) mk/debug-test.sh $1 < /dev/null
+  .PHONY: $1
+  $1: $2
+	@env BASH=$(bash) $(bash) $3 < /dev/null
 
 endef
 
-define run-install-test-group
+define run-test
+
+  $(eval $(call run-bash,$1.test,$1 $(test-deps),mk/run-test.sh $1 $2))
+  $(eval $(call run-bash,$1.test-debug,$1 $(test-deps),mk/debug-test.sh $1 $2))
+
+endef
+
+define run-test-group
 
   .PHONY: $1.test-group
 
diff --git a/tests/functional/common/vars-and-functions.sh.in b/tests/functional/common/vars-and-functions.sh.in
index 967d6be54..848988af9 100644
--- a/tests/functional/common/vars-and-functions.sh.in
+++ b/tests/functional/common/vars-and-functions.sh.in
@@ -4,7 +4,7 @@ if [[ -z "${COMMON_VARS_AND_FUNCTIONS_SH_SOURCED-}" ]]; then
 
 COMMON_VARS_AND_FUNCTIONS_SH_SOURCED=1
 
-export PS4='+(${BASH_SOURCE[0]-$0}:$LINENO) '
+set +x
 
 export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/${TEST_NAME:-default/tests\/functional//}
 export NIX_STORE_DIR

From 20b95d622336cf982082d7daf3075339f6edce70 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Sat, 4 Nov 2023 15:35:38 -0400
Subject: [PATCH 06/45] Git object hashing in libutil

This is the core functionality but just unit-tested and not yet made
part of the store layer. This is because there is some tech debt around
(a) repeated boilerplate hashing objects (b) better integration of the
new `SourceAccessor` type that needs to be cleaned up first.

Part of RFC 133

Co-Authored-By: Matthew Bauer <mjbauer95@gmail.com>
Co-Authored-By: Carlo Nucera <carlo.nucera@protonmail.com>
Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Florian Klink <flokli@flokli.de>
---
 src/libutil/experimental-features.cc          |   8 +
 src/libutil/experimental-features.hh          |   1 +
 src/libutil/git.cc                            | 263 +++++++++++++++++-
 src/libutil/git.hh                            | 141 +++++++++-
 src/libutil/serialise.cc                      |   4 +
 src/libutil/serialise.hh                      |   1 +
 src/libutil/tests/git.cc                      | 249 +++++++++++++++--
 src/libutil/tests/local.mk                    |   4 +
 unit-test-data/libutil/git/check-data.sh      |  31 +++
 .../libutil/git/hello-world-blob.bin          | Bin 0 -> 24 bytes
 unit-test-data/libutil/git/hello-world.bin    | Bin 0 -> 16 bytes
 unit-test-data/libutil/git/tree.bin           | Bin 0 -> 100 bytes
 unit-test-data/libutil/git/tree.txt           |   3 +
 13 files changed, 667 insertions(+), 38 deletions(-)
 create mode 100644 unit-test-data/libutil/git/check-data.sh
 create mode 100644 unit-test-data/libutil/git/hello-world-blob.bin
 create mode 100644 unit-test-data/libutil/git/hello-world.bin
 create mode 100644 unit-test-data/libutil/git/tree.bin
 create mode 100644 unit-test-data/libutil/git/tree.txt

diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc
index 88fb55713..ac4d189e1 100644
--- a/src/libutil/experimental-features.cc
+++ b/src/libutil/experimental-features.cc
@@ -96,6 +96,14 @@ constexpr std::array<ExperimentalFeatureDetails, numXpFeatures> xpFeatureDetails
             [`nix`](@docroot@/command-ref/new-cli/nix.md) for details.
         )",
     },
+    {
+        .tag = Xp::GitHashing,
+        .name = "git-hashing",
+        .description = R"(
+            Allow creating (content-addressed) store objects which are hashed via Git's hashing algorithm.
+            These store objects will not be understandable by older versions of Nix.
+        )",
+    },
     {
         .tag = Xp::RecursiveNix,
         .name = "recursive-nix",
diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh
index f580fd030..c355b8081 100644
--- a/src/libutil/experimental-features.hh
+++ b/src/libutil/experimental-features.hh
@@ -22,6 +22,7 @@ enum struct ExperimentalFeature
     Flakes,
     FetchTree,
     NixCommand,
+    GitHashing,
     RecursiveNix,
     NoUrlLiterals,
     FetchClosure,
diff --git a/src/libutil/git.cc b/src/libutil/git.cc
index f35c2fdb7..a4bd60096 100644
--- a/src/libutil/git.cc
+++ b/src/libutil/git.cc
@@ -1,9 +1,263 @@
-#include "git.hh"
-
+#include <cerrno>
+#include <algorithm>
+#include <vector>
+#include <map>
 #include <regex>
+#include <strings.h> // for strcasecmp
+
+#include "signals.hh"
+#include "config.hh"
+#include "hash.hh"
+#include "posix-source-accessor.hh"
+
+#include "git.hh"
+#include "serialise.hh"
+
+namespace nix::git {
+
+using namespace nix;
+using namespace std::string_literals;
+
+std::optional<Mode> decodeMode(RawMode m) {
+    switch (m) {
+        case (RawMode) Mode::Directory:
+        case (RawMode) Mode::Executable:
+        case (RawMode) Mode::Regular:
+        case (RawMode) Mode::Symlink:
+            return (Mode) m;
+        default:
+            return std::nullopt;
+    }
+}
+
+
+static std::string getStringUntil(Source & source, char byte)
+{
+    std::string s;
+    char n[1];
+    source(std::string_view { n, 1 });
+    while (*n != byte) {
+        s += *n;
+        source(std::string_view { n, 1 });
+    }
+    return s;
+}
+
+
+static std::string getString(Source & source, int n)
+{
+    std::string v;
+    v.resize(n);
+    source(v);
+    return v;
+}
+
+
+void parse(
+    ParseSink & sink,
+    const Path & sinkPath,
+    Source & source,
+    std::function<SinkHook> hook,
+    const ExperimentalFeatureSettings & xpSettings)
+{
+    xpSettings.require(Xp::GitHashing);
+
+    auto type = getString(source, 5);
+
+    if (type == "blob ") {
+        sink.createRegularFile(sinkPath);
+
+        unsigned long long size = std::stoi(getStringUntil(source, 0));
+
+        sink.preallocateContents(size);
+
+        unsigned long long left = size;
+        std::string buf;
+        buf.reserve(65536);
+
+        while (left) {
+            checkInterrupt();
+            buf.resize(std::min((unsigned long long)buf.capacity(), left));
+            source(buf);
+            sink.receiveContents(buf);
+            left -= buf.size();
+        }
+    } else if (type == "tree ") {
+        unsigned long long size = std::stoi(getStringUntil(source, 0));
+        unsigned long long left = size;
+
+        sink.createDirectory(sinkPath);
+
+        while (left) {
+            std::string perms = getStringUntil(source, ' ');
+            left -= perms.size();
+            left -= 1;
+
+            RawMode rawMode = std::stoi(perms, 0, 8);
+            auto modeOpt = decodeMode(rawMode);
+            if (!modeOpt)
+                throw Error("Unknown Git permission: %o", perms);
+            auto mode = std::move(*modeOpt);
+
+            std::string name = getStringUntil(source, '\0');
+            left -= name.size();
+            left -= 1;
+
+            std::string hashs = getString(source, 20);
+            left -= 20;
+
+            Hash hash(htSHA1);
+            std::copy(hashs.begin(), hashs.end(), hash.hash);
+
+            hook(name, TreeEntry {
+                .mode = mode,
+                .hash = hash,
+            });
+
+            if (mode == Mode::Executable)
+                sink.isExecutable();
+        }
+    } else throw Error("input doesn't look like a Git object");
+}
+
+
+std::optional<Mode> convertMode(SourceAccessor::Type type)
+{
+    switch (type) {
+    case SourceAccessor::tSymlink:   return Mode::Symlink;
+    case SourceAccessor::tRegular:   return Mode::Regular;
+    case SourceAccessor::tDirectory: return Mode::Directory;
+    case SourceAccessor::tMisc:      return std::nullopt;
+    default: abort();
+    }
+}
+
+
+void restore(ParseSink & sink, Source & source, std::function<RestoreHook> hook)
+{
+    parse(sink, "", source, [&](Path name, TreeEntry entry) {
+        auto [accessor, from] = hook(entry.hash);
+        auto stat = accessor->lstat(from);
+        auto gotOpt = convertMode(stat.type);
+        if (!gotOpt)
+            throw Error("file '%s' (git hash %s) has an unsupported type",
+                from,
+                entry.hash.to_string(HashFormat::Base16, false));
+        auto & got = *gotOpt;
+        if (got != entry.mode)
+            throw Error("git mode of file '%s' (git hash %s) is %o but expected %o",
+                from,
+                entry.hash.to_string(HashFormat::Base16, false),
+                (RawMode) got,
+                (RawMode) entry.mode);
+        copyRecursive(
+            *accessor, from,
+            sink, name);
+    });
+}
+
+
+void dumpBlobPrefix(
+    uint64_t size, Sink & sink,
+    const ExperimentalFeatureSettings & xpSettings)
+{
+    xpSettings.require(Xp::GitHashing);
+    auto s = fmt("blob %d\0"s, std::to_string(size));
+    sink(s);
+}
+
+
+void dumpTree(const Tree & entries, Sink & sink,
+    const ExperimentalFeatureSettings & xpSettings)
+{
+    xpSettings.require(Xp::GitHashing);
+
+    std::string v1;
+
+    for (auto & [name, entry] : entries) {
+        auto name2 = name;
+        if (entry.mode == Mode::Directory) {
+            assert(name2.back() == '/');
+            name2.pop_back();
+        }
+        v1 += fmt("%o %s\0"s, static_cast<RawMode>(entry.mode), name2);
+        std::copy(entry.hash.hash, entry.hash.hash + entry.hash.hashSize, std::back_inserter(v1));
+    }
+
+    {
+        auto s = fmt("tree %d\0"s, v1.size());
+        sink(s);
+    }
+
+    sink(v1);
+}
+
+
+Mode dump(
+    SourceAccessor & accessor, const CanonPath & path,
+    Sink & sink,
+    std::function<DumpHook> hook,
+    PathFilter & filter,
+    const ExperimentalFeatureSettings & xpSettings)
+{
+    auto st = accessor.lstat(path);
+
+    switch (st.type) {
+    case SourceAccessor::tRegular:
+    {
+        accessor.readFile(path, sink, [&](uint64_t size) {
+            dumpBlobPrefix(size, sink, xpSettings);
+        });
+        return st.isExecutable
+            ? Mode::Executable
+            : Mode::Regular;
+    }
+
+    case SourceAccessor::tDirectory:
+    {
+        Tree entries;
+        for (auto & [name, _] : accessor.readDirectory(path)) {
+            auto child = path + name;
+            if (!filter(child.abs())) continue;
+
+            auto entry = hook(child);
+
+            auto name2 = name;
+            if (entry.mode == Mode::Directory)
+                name2 += "/";
+
+            entries.insert_or_assign(std::move(name2), std::move(entry));
+        }
+        dumpTree(entries, sink, xpSettings);
+        return Mode::Directory;
+    }
+
+    case SourceAccessor::tSymlink:
+    case SourceAccessor::tMisc:
+    default:
+        throw Error("file '%1%' has an unsupported type", path);
+    }
+}
+
+
+TreeEntry dumpHash(
+    HashType ht,
+    SourceAccessor & accessor, const CanonPath & path, PathFilter & filter)
+{
+    std::function<DumpHook> hook;
+    hook = [&](const CanonPath & path) -> TreeEntry {
+        auto hashSink = HashSink(ht);
+        auto mode = dump(accessor, path, hashSink, hook, filter);
+        auto hash = hashSink.finish().first;
+        return {
+            .mode = mode,
+            .hash = hash,
+        };
+    };
+
+    return hook(path);
+}
 
-namespace nix {
-namespace git {
 
 std::optional<LsRemoteRefLine> parseLsRemoteLine(std::string_view line)
 {
@@ -22,4 +276,3 @@ std::optional<LsRemoteRefLine> parseLsRemoteLine(std::string_view line)
 }
 
 }
-}
diff --git a/src/libutil/git.hh b/src/libutil/git.hh
index bf2b9a286..303460072 100644
--- a/src/libutil/git.hh
+++ b/src/libutil/git.hh
@@ -5,9 +5,127 @@
 #include <string_view>
 #include <optional>
 
-namespace nix {
+#include "types.hh"
+#include "serialise.hh"
+#include "hash.hh"
+#include "source-accessor.hh"
+#include "fs-sink.hh"
 
-namespace git {
+namespace nix::git {
+
+using RawMode = uint32_t;
+
+enum struct Mode : RawMode {
+    Directory = 0040000,
+    Executable = 0100755,
+    Regular = 0100644,
+    Symlink = 0120000,
+};
+
+std::optional<Mode> decodeMode(RawMode m);
+
+/**
+ * An anonymous Git tree object entry (no name part).
+ */
+struct TreeEntry
+{
+    Mode mode;
+    Hash hash;
+
+    GENERATE_CMP(TreeEntry, me->mode, me->hash);
+};
+
+/**
+ * A Git tree object, fully decoded and stored in memory.
+ *
+ * Directory names must end in a `/` for sake of sorting. See
+ * https://github.com/mirage/irmin/issues/352
+ */
+using Tree = std::map<std::string, TreeEntry>;
+
+/**
+ * Callback for processing a child hash with `parse`
+ *
+ * The function should
+ *
+ * 1. Obtain the file system objects denoted by `gitHash`
+ *
+ * 2. Ensure they match `mode`
+ *
+ * 3. Feed them into the same sink `parse` was called with
+ *
+ * Implementations may seek to memoize resources (bandwidth, storage,
+ * etc.) for the same Git hash.
+ */
+using SinkHook = void(const Path & name, TreeEntry entry);
+
+void parse(
+    ParseSink & sink, const Path & sinkPath,
+    Source & source,
+    std::function<SinkHook> hook,
+    const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
+
+/**
+ * Assists with writing a `SinkHook` step (2).
+ */
+std::optional<Mode> convertMode(SourceAccessor::Type type);
+
+/**
+ * Simplified version of `SinkHook` for `restore`.
+ *
+ * Given a `Hash`, return a `SourceAccessor` and `CanonPath` pointing to
+ * the file system object with that path.
+ */
+using RestoreHook = std::pair<SourceAccessor *, CanonPath>(Hash);
+
+/**
+ * Wrapper around `parse` and `RestoreSink`
+ */
+void restore(ParseSink & sink, Source & source, std::function<RestoreHook> hook);
+
+/**
+ * Dumps a single file to a sink
+ *
+ * @param xpSettings for testing purposes
+ */
+void dumpBlobPrefix(
+    uint64_t size, Sink & sink,
+    const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
+
+/**
+ * Dumps a representation of a git tree to a sink
+ */
+void dumpTree(
+    const Tree & entries, Sink & sink,
+    const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
+
+/**
+ * Callback for processing a child with `dump`
+ *
+ * The function should return the Git hash and mode of the file at the
+ * given path in the accessor passed to `dump`.
+ *
+ * Note that if the child is a directory, its child in must also be so
+ * processed in order to compute this information.
+ */
+using DumpHook = TreeEntry(const CanonPath & path);
+
+Mode dump(
+    SourceAccessor & accessor, const CanonPath & path,
+    Sink & sink,
+    std::function<DumpHook> hook,
+    PathFilter & filter = defaultPathFilter,
+    const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
+
+/**
+ * Recursively dumps path, hashing as we go.
+ *
+ * A smaller wrapper around `dump`.
+ */
+TreeEntry dumpHash(
+    HashType ht,
+    SourceAccessor & accessor, const CanonPath & path,
+    PathFilter & filter = defaultPathFilter);
 
 /**
  * A line from the output of `git ls-remote --symref`.
@@ -16,15 +134,17 @@ namespace git {
  *
  * - Symbolic references of the form
  *
- *      ref: {target}	{reference}
- *
- *    where {target} is itself a reference and {reference} is optional
+ *   ```
+ *   ref: {target} {reference}
+ *   ```
+ *   where {target} is itself a reference and {reference} is optional
  *
  * - Object references of the form
  *
- *      {target}	{reference}
- *
- *    where {target} is a commit id and {reference} is mandatory
+ *   ```
+ *   {target}  {reference}
+ *   ```
+ *   where {target} is a commit id and {reference} is mandatory
  */
 struct LsRemoteRefLine {
     enum struct Kind {
@@ -36,8 +156,9 @@ struct LsRemoteRefLine {
     std::optional<std::string> reference;
 };
 
+/**
+ * Parse an `LsRemoteRefLine`
+ */
 std::optional<LsRemoteRefLine> parseLsRemoteLine(std::string_view line);
 
 }
-
-}
diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc
index 725ddbb8d..d7950b11b 100644
--- a/src/libutil/serialise.cc
+++ b/src/libutil/serialise.cc
@@ -74,6 +74,10 @@ void Source::operator () (char * data, size_t len)
     }
 }
 
+void Source::operator () (std::string_view data)
+{
+    (*this)((char *)data.data(), data.size());
+}
 
 void Source::drainInto(Sink & sink)
 {
diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh
index 9e07226bf..3f57ce88b 100644
--- a/src/libutil/serialise.hh
+++ b/src/libutil/serialise.hh
@@ -73,6 +73,7 @@ struct Source
      * an error if it is not going to be available.
      */
     void operator () (char * data, size_t len);
+    void operator () (std::string_view data);
 
     /**
      * Store up to ‘len’ in the buffer pointed to by ‘data’, and
diff --git a/src/libutil/tests/git.cc b/src/libutil/tests/git.cc
index 5b5715fc2..2842ea4d0 100644
--- a/src/libutil/tests/git.cc
+++ b/src/libutil/tests/git.cc
@@ -1,33 +1,236 @@
-#include "git.hh"
 #include <gtest/gtest.h>
 
+#include "git.hh"
+#include "memory-source-accessor.hh"
+
+#include "tests/characterization.hh"
+
 namespace nix {
 
-    TEST(GitLsRemote, parseSymrefLineWithReference) {
-        auto line = "ref: refs/head/main	HEAD";
-        auto res = git::parseLsRemoteLine(line);
-        ASSERT_TRUE(res.has_value());
-        ASSERT_EQ(res->kind, git::LsRemoteRefLine::Kind::Symbolic);
-        ASSERT_EQ(res->target, "refs/head/main");
-        ASSERT_EQ(res->reference, "HEAD");
+using namespace git;
+
+class GitTest : public CharacterizationTest
+{
+    Path unitTestData = getUnitTestData() + "/libutil/git";
+
+public:
+
+    Path goldenMaster(std::string_view testStem) const override {
+        return unitTestData + "/" + testStem;
     }
 
-    TEST(GitLsRemote, parseSymrefLineWithNoReference) {
-        auto line = "ref: refs/head/main";
-        auto res = git::parseLsRemoteLine(line);
-        ASSERT_TRUE(res.has_value());
-        ASSERT_EQ(res->kind, git::LsRemoteRefLine::Kind::Symbolic);
-        ASSERT_EQ(res->target, "refs/head/main");
-        ASSERT_EQ(res->reference, std::nullopt);
-    }
+    /**
+     * We set these in tests rather than the regular globals so we don't have
+     * to worry about race conditions if the tests run concurrently.
+     */
+    ExperimentalFeatureSettings mockXpSettings;
 
-    TEST(GitLsRemote, parseObjectRefLine) {
-        auto line = "abc123	refs/head/main";
-        auto res = git::parseLsRemoteLine(line);
-        ASSERT_TRUE(res.has_value());
-        ASSERT_EQ(res->kind, git::LsRemoteRefLine::Kind::Object);
-        ASSERT_EQ(res->target, "abc123");
-        ASSERT_EQ(res->reference, "refs/head/main");
+private:
+
+    void SetUp() override
+    {
+        mockXpSettings.set("experimental-features", "git-hashing");
     }
+};
+
+TEST(GitMode, gitMode_directory) {
+    Mode m = Mode::Directory;
+    RawMode r = 0040000;
+    ASSERT_EQ(static_cast<RawMode>(m), r);
+    ASSERT_EQ(decodeMode(r), std::optional { m });
+};
+
+TEST(GitMode, gitMode_executable) {
+    Mode m = Mode::Executable;
+    RawMode r = 0100755;
+    ASSERT_EQ(static_cast<RawMode>(m), r);
+    ASSERT_EQ(decodeMode(r), std::optional { m });
+};
+
+TEST(GitMode, gitMode_regular) {
+    Mode m = Mode::Regular;
+    RawMode r = 0100644;
+    ASSERT_EQ(static_cast<RawMode>(m), r);
+    ASSERT_EQ(decodeMode(r), std::optional { m });
+};
+
+TEST(GitMode, gitMode_symlink) {
+    Mode m = Mode::Symlink;
+    RawMode r = 0120000;
+    ASSERT_EQ(static_cast<RawMode>(m), r);
+    ASSERT_EQ(decodeMode(r), std::optional { m });
+};
+
+TEST_F(GitTest, blob_read) {
+    readTest("hello-world-blob.bin", [&](const auto & encoded) {
+        StringSource in { encoded };
+        StringSink out;
+        RegularFileSink out2 { out };
+        parse(out2, "", in, [](auto &, auto) {}, mockXpSettings);
+
+        auto expected = readFile(goldenMaster("hello-world.bin"));
+
+        ASSERT_EQ(out.s, expected);
+    });
 }
 
+TEST_F(GitTest, blob_write) {
+    writeTest("hello-world-blob.bin", [&]() {
+        auto decoded = readFile(goldenMaster("hello-world.bin"));
+        StringSink s;
+        dumpBlobPrefix(decoded.size(), s, mockXpSettings);
+        s(decoded);
+        return s.s;
+    });
+}
+
+/**
+ * This data is for "shallow" tree tests. However, we use "real" hashes
+ * so that we can check our test data in the corresponding functional
+ * test (`git-hashing/unit-test-data`).
+ */
+const static Tree tree = {
+    {
+        "Foo",
+        {
+            .mode = Mode::Regular,
+            // hello world with special chars from above
+            .hash = Hash::parseAny("63ddb340119baf8492d2da53af47e8c7cfcd5eb2", htSHA1),
+        },
+    },
+    {
+        "bAr",
+        {
+            .mode = Mode::Executable,
+            // ditto
+            .hash = Hash::parseAny("63ddb340119baf8492d2da53af47e8c7cfcd5eb2", htSHA1),
+        },
+    },
+    {
+        "baZ/",
+        {
+            .mode = Mode::Directory,
+            // Empty directory hash
+            .hash = Hash::parseAny("4b825dc642cb6eb9a060e54bf8d69288fbee4904", htSHA1),
+        },
+    },
+};
+
+TEST_F(GitTest, tree_read) {
+    readTest("tree.bin", [&](const auto & encoded) {
+        StringSource in { encoded };
+        NullParseSink out;
+        Tree got;
+        parse(out, "", in, [&](auto & name, auto entry) {
+            auto name2 = name;
+            if (entry.mode == Mode::Directory)
+                name2 += '/';
+            got.insert_or_assign(name2, std::move(entry));
+        }, mockXpSettings);
+
+        ASSERT_EQ(got, tree);
+    });
+}
+
+TEST_F(GitTest, tree_write) {
+    writeTest("tree.bin", [&]() {
+        StringSink s;
+        dumpTree(tree, s, mockXpSettings);
+        return s.s;
+    });
+}
+
+TEST_F(GitTest, both_roundrip) {
+    using File = MemorySourceAccessor::File;
+
+    MemorySourceAccessor files;
+    files.root = File::Directory {
+        .contents {
+            {
+                "foo",
+                File::Regular {
+                    .contents = "hello\n\0\n\tworld!",
+                },
+            },
+            {
+                "bar",
+                File::Directory {
+                    .contents = {
+                        {
+                            "baz",
+                            File::Regular {
+                                .executable = true,
+                                .contents = "good day,\n\0\n\tworld!",
+                            },
+                        },
+                    },
+                },
+            },
+        },
+    };
+
+    std::map<Hash, std::string> cas;
+
+    std::function<DumpHook> dumpHook;
+    dumpHook = [&](const CanonPath & path) {
+        StringSink s;
+        HashSink hashSink { htSHA1 };
+        TeeSink s2 { s, hashSink };
+        auto mode = dump(
+            files, path, s2, dumpHook,
+            defaultPathFilter, mockXpSettings);
+        auto hash = hashSink.finish().first;
+        cas.insert_or_assign(hash, std::move(s.s));
+        return TreeEntry {
+            .mode = mode,
+            .hash = hash,
+        };
+    };
+
+    auto root = dumpHook(CanonPath::root);
+
+    MemorySourceAccessor files2;
+
+    MemorySink sinkFiles2 { files2 };
+
+    std::function<void(const Path, const Hash &)> mkSinkHook;
+    mkSinkHook = [&](const Path prefix, const Hash & hash) {
+        StringSource in { cas[hash] };
+        parse(sinkFiles2, prefix, in, [&](const Path & name, const auto & entry) {
+            mkSinkHook(prefix + "/" + name, entry.hash);
+        }, mockXpSettings);
+    };
+
+    mkSinkHook("", root.hash);
+
+    ASSERT_EQ(files, files2);
+}
+
+TEST(GitLsRemote, parseSymrefLineWithReference) {
+    auto line = "ref: refs/head/main	HEAD";
+    auto res = parseLsRemoteLine(line);
+    ASSERT_TRUE(res.has_value());
+    ASSERT_EQ(res->kind, LsRemoteRefLine::Kind::Symbolic);
+    ASSERT_EQ(res->target, "refs/head/main");
+    ASSERT_EQ(res->reference, "HEAD");
+}
+
+TEST(GitLsRemote, parseSymrefLineWithNoReference) {
+    auto line = "ref: refs/head/main";
+    auto res = parseLsRemoteLine(line);
+    ASSERT_TRUE(res.has_value());
+    ASSERT_EQ(res->kind, LsRemoteRefLine::Kind::Symbolic);
+    ASSERT_EQ(res->target, "refs/head/main");
+    ASSERT_EQ(res->reference, std::nullopt);
+}
+
+TEST(GitLsRemote, parseObjectRefLine) {
+    auto line = "abc123	refs/head/main";
+    auto res = parseLsRemoteLine(line);
+    ASSERT_TRUE(res.has_value());
+    ASSERT_EQ(res->kind, LsRemoteRefLine::Kind::Object);
+    ASSERT_EQ(res->target, "abc123");
+    ASSERT_EQ(res->reference, "refs/head/main");
+}
+
+}
diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk
index 167915439..5a970c0f2 100644
--- a/src/libutil/tests/local.mk
+++ b/src/libutil/tests/local.mk
@@ -27,3 +27,7 @@ libutil-tests_CXXFLAGS += -I src/libutil
 libutil-tests_LIBS = libutil
 
 libutil-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)
+
+check: unit-test-data/libutil/git/check-data.sh.test
+
+$(eval $(call run-test,unit-test-data/libutil/git/check-data.sh))
diff --git a/unit-test-data/libutil/git/check-data.sh b/unit-test-data/libutil/git/check-data.sh
new file mode 100644
index 000000000..68b705c95
--- /dev/null
+++ b/unit-test-data/libutil/git/check-data.sh
@@ -0,0 +1,31 @@
+#!/usr/bin/env bash
+
+set -eu -o pipefail
+
+export TEST_ROOT=$(realpath ${TMPDIR:-/tmp}/nix-test)/git-hashing/unit-test-data
+mkdir -p $TEST_ROOT
+
+repo="$TEST_ROOT/scratch"
+git init "$repo"
+
+git -C "$repo" config user.email "you@example.com"
+git -C "$repo" config user.name "Your Name"
+
+# `-w` to write for tree test
+freshlyAddedHash=$(git -C "$repo" hash-object -w -t blob --stdin < "./hello-world.bin")
+encodingHash=$(sha1sum -b < "./hello-world-blob.bin" | head -c 40)
+
+# If the hashes match, then `hello-world-blob.bin` must be the encoding
+# of `hello-world.bin`.
+[[ "$encodingHash" == "$freshlyAddedHash" ]]
+
+# Create empty directory object for tree test
+echo -n | git -C "$repo" hash-object -w -t tree --stdin
+
+# Relies on both child hashes already existing in the git store
+freshlyAddedHash=$(git -C "$repo" mktree < "./tree.txt")
+encodingHash=$(sha1sum -b < "./tree.bin" | head -c 40)
+
+# If the hashes match, then `tree.bin` must be the encoding of the
+# directory denoted by `tree.txt` interpreted as git directory listing.
+[[ "$encodingHash" == "$freshlyAddedHash" ]]
diff --git a/unit-test-data/libutil/git/hello-world-blob.bin b/unit-test-data/libutil/git/hello-world-blob.bin
new file mode 100644
index 0000000000000000000000000000000000000000..255f5df55ccedb2dae5f541d516896ffffcdb526
GIT binary patch
literal 24
fcmYew$xl)+G-L2c&B@7E;9}t7R0z*6%1HqLQkDjQ

literal 0
HcmV?d00001

diff --git a/unit-test-data/libutil/git/hello-world.bin b/unit-test-data/libutil/git/hello-world.bin
new file mode 100644
index 0000000000000000000000000000000000000000..63ddb340119baf8492d2da53af47e8c7cfcd5eb2
GIT binary patch
literal 16
XcmeZB&B@7E;9}t7R0z*6%1HqLBqsz~

literal 0
HcmV?d00001

diff --git a/unit-test-data/libutil/git/tree.bin b/unit-test-data/libutil/git/tree.bin
new file mode 100644
index 0000000000000000000000000000000000000000..5256ec140702fef5f88bd5750caf7cd57c03e5ac
GIT binary patch
literal 100
zcmXRZN=;R;G-5C`FfcPQQE<!8XGp%g*+Fpj`j$zTZUwJ*e{uZ$*|<$WX>(Ikg(Sx!
ktkNb1K%kJ67{%b-6no6+bl%Pd2~WL$T$|MK`<*8X01f;sp#T5?

literal 0
HcmV?d00001

diff --git a/unit-test-data/libutil/git/tree.txt b/unit-test-data/libutil/git/tree.txt
new file mode 100644
index 000000000..be3d02920
--- /dev/null
+++ b/unit-test-data/libutil/git/tree.txt
@@ -0,0 +1,3 @@
+100644 blob 63ddb340119baf8492d2da53af47e8c7cfcd5eb2	Foo
+100755 blob 63ddb340119baf8492d2da53af47e8c7cfcd5eb2	bAr
+040000 tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904	baZ

From fd5a4a846752873331b6549f0778181dc4ecc2f3 Mon Sep 17 00:00:00 2001
From: Graham Christensen <graham@grahamc.com>
Date: Fri, 10 Nov 2023 12:12:13 -0500
Subject: [PATCH 07/45] nix upgrade-nix: make the source URL an option

This new option enables organizations to more easily manage their Nix
fleet's deployment, and ensure a consistent and planned rollout of Nix
upgrades.
---
 src/libstore/globals.hh | 10 ++++++++++
 src/nix/upgrade-nix.cc  |  7 +++----
 src/nix/upgrade-nix.md  |  6 ++++--
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh
index 8e034f5a9..27caf42c4 100644
--- a/src/libstore/globals.hh
+++ b/src/libstore/globals.hh
@@ -1084,6 +1084,16 @@ public:
         true, // document default
         Xp::ConfigurableImpureEnv
     };
+
+    Setting<std::string> upgradeNixStorePathUrl{
+        this,
+        "https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/installer/tools/nix-fallback-paths.nix",
+        "upgrade-nix-store-path-url",
+        R"(
+          Used by `nix upgrade-nix`, the URL of the file that contains the
+          store paths of the latest Nix release.
+        )"
+    };
 };
 
 
diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc
index c529c2363..4c7a74e16 100644
--- a/src/nix/upgrade-nix.cc
+++ b/src/nix/upgrade-nix.cc
@@ -14,7 +14,6 @@ using namespace nix;
 struct CmdUpgradeNix : MixDryRun, StoreCommand
 {
     Path profileDir;
-    std::string storePathsUrl = "https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/installer/tools/nix-fallback-paths.nix";
 
     CmdUpgradeNix()
     {
@@ -30,7 +29,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
             .longName = "nix-store-paths-url",
             .description = "The URL of the file that contains the store paths of the latest Nix release.",
             .labels = {"url"},
-            .handler = {&storePathsUrl}
+            .handler = {&(std::string&) settings.upgradeNixStorePathUrl}
         });
     }
 
@@ -44,7 +43,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
 
     std::string description() override
     {
-        return "upgrade Nix to the stable version declared in Nixpkgs";
+        return "upgrade Nix to the latest stable version";
     }
 
     std::string doc() override
@@ -145,7 +144,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
         Activity act(*logger, lvlInfo, actUnknown, "querying latest Nix version");
 
         // FIXME: use nixos.org?
-        auto req = FileTransferRequest(storePathsUrl);
+        auto req = FileTransferRequest((std::string&) settings.upgradeNixStorePathUrl);
         auto res = getFileTransfer()->download(req);
 
         auto state = std::make_unique<EvalState>(SearchPath{}, store);
diff --git a/src/nix/upgrade-nix.md b/src/nix/upgrade-nix.md
index cce88c397..3a3bf61b9 100644
--- a/src/nix/upgrade-nix.md
+++ b/src/nix/upgrade-nix.md
@@ -16,8 +16,10 @@ R""(
 
 # Description
 
-This command upgrades Nix to the stable version declared in Nixpkgs.
-This stable version is defined in [nix-fallback-paths.nix](https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/installer/tools/nix-fallback-paths.nix)
+This command upgrades Nix to the stable version.
+
+By default, the latest stable version is defined by Nixpkgs, in
+[nix-fallback-paths.nix](https://github.com/NixOS/nixpkgs/raw/master/nixos/modules/installer/tools/nix-fallback-paths.nix)
 and updated manually. It may not always be the latest tagged release.
 
 By default, it locates the directory containing the `nix` binary in the `$PATH`

From 0be84c83b242b6e6a22400727752072b298e7cab Mon Sep 17 00:00:00 2001
From: Tom Bereknyei <tomberek@gmail.com>
Date: Mon, 6 Nov 2023 10:29:37 -0500
Subject: [PATCH 08/45] key and cat: no need for progressBar

otherwise the output will be invisible in common terminal configurations
---
 src/nix/cat.cc  | 2 ++
 src/nix/sigs.cc | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/src/nix/cat.cc b/src/nix/cat.cc
index 6e5a736f2..4df086d4f 100644
--- a/src/nix/cat.cc
+++ b/src/nix/cat.cc
@@ -1,6 +1,7 @@
 #include "command.hh"
 #include "store-api.hh"
 #include "nar-accessor.hh"
+#include "progress-bar.hh"
 
 using namespace nix;
 
@@ -13,6 +14,7 @@ struct MixCat : virtual Args
         auto st = accessor->lstat(CanonPath(path));
         if (st.type != SourceAccessor::Type::tRegular)
             throw Error("path '%1%' is not a regular file", path);
+        stopProgressBar();
         writeFull(STDOUT_FILENO, accessor->readFile(CanonPath(path)));
     }
 };
diff --git a/src/nix/sigs.cc b/src/nix/sigs.cc
index a68616355..39555c9ea 100644
--- a/src/nix/sigs.cc
+++ b/src/nix/sigs.cc
@@ -3,6 +3,7 @@
 #include "shared.hh"
 #include "store-api.hh"
 #include "thread-pool.hh"
+#include "progress-bar.hh"
 
 #include <atomic>
 
@@ -174,6 +175,7 @@ struct CmdKeyGenerateSecret : Command
         if (!keyName)
             throw UsageError("required argument '--key-name' is missing");
 
+        stopProgressBar();
         writeFull(STDOUT_FILENO, SecretKey::generate(*keyName).to_string());
     }
 };
@@ -195,6 +197,7 @@ struct CmdKeyConvertSecretToPublic : Command
     void run() override
     {
         SecretKey secretKey(drainFD(STDIN_FILENO));
+        stopProgressBar();
         writeFull(STDOUT_FILENO, secretKey.toPublicKey().to_string());
     }
 };

From 742a63b98f2008161fd00bdbbd39b8f1b14f6443 Mon Sep 17 00:00:00 2001
From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com>
Date: Mon, 13 Nov 2023 22:01:50 +0000
Subject: [PATCH 09/45] build(deps): bump zeebe-io/backport-action from 2.1.0
 to 2.1.1

Bumps [zeebe-io/backport-action](https://github.com/zeebe-io/backport-action) from 2.1.0 to 2.1.1.
- [Release notes](https://github.com/zeebe-io/backport-action/releases)
- [Commits](https://github.com/zeebe-io/backport-action/compare/v2.1.0...v2.1.1)

---
updated-dependencies:
- dependency-name: zeebe-io/backport-action
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
---
 .github/workflows/backport.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/backport.yml b/.github/workflows/backport.yml
index 893f4a56f..975c90b91 100644
--- a/.github/workflows/backport.yml
+++ b/.github/workflows/backport.yml
@@ -21,7 +21,7 @@ jobs:
           fetch-depth: 0
       - name: Create backport PRs
         # should be kept in sync with `version`
-        uses: zeebe-io/backport-action@v2.1.0
+        uses: zeebe-io/backport-action@v2.1.1
         with:
           # Config README: https://github.com/zeebe-io/backport-action#backport-action
           github_token: ${{ secrets.GITHUB_TOKEN }}

From ad99c8950b86b8f354f5c72efe690d3cba045d99 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andreas=20St=C3=BChrk?= <andy@hammerhartes.de>
Date: Mon, 13 Nov 2023 23:19:27 +0100
Subject: [PATCH 10/45] Update comment to reflect bind mounts are now used for
 store in chroot

---
 src/libstore/build/local-derivation-goal.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index adb011e30..a9f930773 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -652,8 +652,8 @@ void LocalDerivationGoal::startBuilder()
 #if __linux__
         /* Create a temporary directory in which we set up the chroot
            environment using bind-mounts.  We put it in the Nix store
-           to ensure that we can create hard-links to non-directory
-           inputs in the fake Nix store in the chroot (see below). */
+           so that the build outputs can be moved efficiently from the
+           chroot to their final location. */
         chrootRootDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
         deletePath(chrootRootDir);
 

From 4944cdb94d03742176cc7881f126e981c0e7e21c Mon Sep 17 00:00:00 2001
From: vicky1999 <ravivignesh2000@gmail.com>
Date: Tue, 14 Nov 2023 19:59:48 +0530
Subject: [PATCH 11/45] nar dump-path command renamed to nar pack

---
 src/nix/dump-path.cc     | 10 +++++++++-
 src/nix/nar-dump-path.md |  3 ++-
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/nix/dump-path.cc b/src/nix/dump-path.cc
index c4edc894b..0850d4c1c 100644
--- a/src/nix/dump-path.cc
+++ b/src/nix/dump-path.cc
@@ -61,4 +61,12 @@ struct CmdDumpPath2 : Command
     }
 };
 
-static auto rDumpPath2 = registerCommand2<CmdDumpPath2>({"nar", "dump-path"});
+struct CmdNarDumpPath : CmdDumpPath2 {
+    void run() override {
+        warn("'nix nar dump-path' is a deprecated alias for 'nix nar pack'");
+        CmdDumpPath2::run();
+    }
+};
+
+static auto rCmdNarPack = registerCommand2<CmdDumpPath2>({"nar", "pack"});
+static auto rCmdNarDumpPath = registerCommand2<CmdNarDumpPath>({"nar", "dump-path"});
diff --git a/src/nix/nar-dump-path.md b/src/nix/nar-dump-path.md
index 26191ad25..29eaacfdb 100644
--- a/src/nix/nar-dump-path.md
+++ b/src/nix/nar-dump-path.md
@@ -5,7 +5,7 @@ R""(
 * To serialise directory `foo` as a NAR:
 
   ```console
-  # nix nar dump-path ./foo > foo.nar
+  # nix nar pack ./foo > foo.nar
   ```
 
 # Description
@@ -15,3 +15,4 @@ This command generates a NAR file containing the serialisation of
 symbolic links. The NAR is written to standard output.
 
 )""
+

From e07e3c106a9ac0537210e62286c4e696573e9f6f Mon Sep 17 00:00:00 2001
From: vicky1999 <ravivignesh2000@gmail.com>
Date: Tue, 14 Nov 2023 20:02:33 +0530
Subject: [PATCH 12/45] code cleanup

---
 src/nix/nar-dump-path.md | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/nix/nar-dump-path.md b/src/nix/nar-dump-path.md
index 29eaacfdb..de82202de 100644
--- a/src/nix/nar-dump-path.md
+++ b/src/nix/nar-dump-path.md
@@ -15,4 +15,3 @@ This command generates a NAR file containing the serialisation of
 symbolic links. The NAR is written to standard output.
 
 )""
-

From 9c7749e13508996eb9df83b1692664cc8cdbf952 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 14 Nov 2023 11:42:25 -0500
Subject: [PATCH 13/45] Fix makefile bug confusing `libnixutil-test` exe vs lib

The `-exe` variant is the program, the unsuffixed variant is the
library.

The corrected usage matches `libnixstore-test`.
---
 src/libutil/tests/local.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk
index 5a970c0f2..c8b8557cb 100644
--- a/src/libutil/tests/local.mk
+++ b/src/libutil/tests/local.mk
@@ -1,6 +1,6 @@
 check: libutil-tests_RUN
 
-programs += libutil-tests
+programs += libutil-tests-exe
 
 libutil-tests-exe_NAME = libnixutil-tests
 

From 70b396649c127760e4b123da41451aa7456bc68d Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Wed, 15 Nov 2023 00:03:44 +0100
Subject: [PATCH 14/45] doc: logical implication is right-associative

    nix-repl> bools = [ false true ]

    nix-repl> combinations = builtins.concatMap (a: builtins.concatMap (b: map (c: { inherit a b c; }) bools) bools) bools

    nix-repl> builtins.all ({ a, b, c }: (a -> b -> c) == (a -> (b -> c))) combinations
    true

    nix-repl> builtins.all ({ a, b, c }: (a -> b -> c) == ((a -> b) -> c)) combinations
    false
---
 doc/manual/src/language/operators.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/manual/src/language/operators.md b/doc/manual/src/language/operators.md
index cc825b4cf..e9cbb5c92 100644
--- a/doc/manual/src/language/operators.md
+++ b/doc/manual/src/language/operators.md
@@ -25,7 +25,7 @@
 | Inequality                             | *expr* `!=` *expr*                         | none          | 11         |
 | Logical conjunction (`AND`)            | *bool* `&&` *bool*                         | left          | 12         |
 | Logical disjunction (`OR`)             | *bool* <code>\|\|</code> *bool*            | left          | 13         |
-| [Logical implication]                  | *bool* `->` *bool*                         | none          | 14         |
+| [Logical implication]                  | *bool* `->` *bool*                         | right         | 14         |
 
 [string]: ./values.md#type-string
 [path]: ./values.md#type-path

From 84128461b68f6274f1cbf309fd019959132f3c2a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Wed, 15 Nov 2023 09:23:26 +0100
Subject: [PATCH 15/45] Add a new `nix store add` command

Deprecate `nix store add-file` and `nix store add-path`, and replace
them with a single `nix store add` command.
---
 doc/manual/src/release-notes/rl-next.md |  2 +
 src/nix/add-file.md                     | 28 ----------
 src/nix/add-to-store.cc                 | 70 +++++++++++++++++--------
 src/nix/{add-path.md => add.md}         |  2 +-
 tests/functional/add.sh                 | 17 ++++++
 5 files changed, 68 insertions(+), 51 deletions(-)
 delete mode 100644 src/nix/add-file.md
 rename src/nix/{add-path.md => add.md} (94%)

diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md
index 1e6ad6922..422f1fce8 100644
--- a/doc/manual/src/release-notes/rl-next.md
+++ b/doc/manual/src/release-notes/rl-next.md
@@ -73,3 +73,5 @@
   [`XDG_DATA_DIRS`](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html#variables) is now populated with the path to the `/share` subdirectory of the current profile.
   This means that command completion scripts, `.desktop` files, and similar artifacts installed via [`nix-env`](@docroot@/command-ref/nix-env.md) or [`nix profile`](@docroot@/command-ref/new-cli/nix3-profile.md)
   (experimental) can be found by any program that follows the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html).
+
+- A new command `nix store add` has been added. It replaces `nix store add-file` and `nix store add-path` which are now deprecated.
diff --git a/src/nix/add-file.md b/src/nix/add-file.md
deleted file mode 100644
index ed237a035..000000000
--- a/src/nix/add-file.md
+++ /dev/null
@@ -1,28 +0,0 @@
-R""(
-
-# Description
-
-Copy the regular file *path* to the Nix store, and print the resulting
-store path on standard output.
-
-> **Warning**
->
-> The resulting store path is not registered as a garbage
-> collector root, so it could be deleted before you have a
-> chance to register it.
-
-# Examples
-
-Add a regular file to the store:
-
-```console
-# echo foo > bar
-
-# nix store add-file ./bar
-/nix/store/cbv2s4bsvzjri77s2gb8g8bpcb6dpa8w-bar
-
-# cat /nix/store/cbv2s4bsvzjri77s2gb8g8bpcb6dpa8w-bar
-foo
-```
-
-)""
diff --git a/src/nix/add-to-store.cc b/src/nix/add-to-store.cc
index 39e5cc99d..f9d487ada 100644
--- a/src/nix/add-to-store.cc
+++ b/src/nix/add-to-store.cc
@@ -5,11 +5,22 @@
 
 using namespace nix;
 
+static FileIngestionMethod parseIngestionMethod(std::string_view input)
+{
+    if (input == "flat") {
+        return FileIngestionMethod::Flat;
+    } else if (input == "nar") {
+        return FileIngestionMethod::Recursive;
+    } else {
+        throw UsageError("Unknown hash mode '%s', expect `flat` or `nar`");
+    }
+}
+
 struct CmdAddToStore : MixDryRun, StoreCommand
 {
     Path path;
     std::optional<std::string> namePart;
-    FileIngestionMethod ingestionMethod;
+    FileIngestionMethod ingestionMethod = FileIngestionMethod::Recursive;
 
     CmdAddToStore()
     {
@@ -23,6 +34,23 @@ struct CmdAddToStore : MixDryRun, StoreCommand
             .labels = {"name"},
             .handler = {&namePart},
         });
+
+        addFlag({
+            .longName  = "mode",
+            .shortName = 'n',
+            .description = R"(
+    How to compute the hash of the input.
+    One of:
+
+    - `nar` (the default): Serialises the input as an archive (following the [_Nix Archive Format_](https://edolstra.github.io/pubs/phd-thesis.pdf#page=101)) and passes that to the hash function.
+
+    - `flat`: Assumes that the input is a single file and directly passes it to the hash function;
+            )",
+            .labels = {"hash-mode"},
+            .handler = {[this](std::string s) {
+                this->ingestionMethod = parseIngestionMethod(s);
+            }},
+        });
     }
 
     void run(ref<Store> store) override
@@ -62,6 +90,22 @@ struct CmdAddToStore : MixDryRun, StoreCommand
     }
 };
 
+struct CmdAdd : CmdAddToStore
+{
+
+    std::string description() override
+    {
+        return "Add a file or directory to the Nix store";
+    }
+
+    std::string doc() override
+    {
+        return
+          #include "add.md"
+          ;
+    }
+};
+
 struct CmdAddFile : CmdAddToStore
 {
     CmdAddFile()
@@ -71,36 +115,18 @@ struct CmdAddFile : CmdAddToStore
 
     std::string description() override
     {
-        return "add a regular file to the Nix store";
-    }
-
-    std::string doc() override
-    {
-        return
-          #include "add-file.md"
-          ;
+        return "Deprecated. Use [`nix store add --mode flat`](@docroot@/command-ref/new-cli/nix3-store-add.md) instead.";
     }
 };
 
 struct CmdAddPath : CmdAddToStore
 {
-    CmdAddPath()
-    {
-        ingestionMethod = FileIngestionMethod::Recursive;
-    }
-
     std::string description() override
     {
-        return "add a path to the Nix store";
-    }
-
-    std::string doc() override
-    {
-        return
-          #include "add-path.md"
-          ;
+        return "Deprecated alias to [`nix store add`](@docroot@/command-ref/new-cli/nix3-store-add.md).";
     }
 };
 
 static auto rCmdAddFile = registerCommand2<CmdAddFile>({"store", "add-file"});
 static auto rCmdAddPath = registerCommand2<CmdAddPath>({"store", "add-path"});
+static auto rCmdAdd = registerCommand2<CmdAdd>({"store", "add"});
diff --git a/src/nix/add-path.md b/src/nix/add.md
similarity index 94%
rename from src/nix/add-path.md
rename to src/nix/add.md
index 87473611d..d38cd21d8 100644
--- a/src/nix/add-path.md
+++ b/src/nix/add.md
@@ -19,7 +19,7 @@ Add a directory to the store:
 # mkdir dir
 # echo foo > dir/bar
 
-# nix store add-path ./dir
+# nix store add ./dir
 /nix/store/6pmjx56pm94n66n4qw1nff0y1crm8nqg-dir
 
 # cat /nix/store/6pmjx56pm94n66n4qw1nff0y1crm8nqg-dir/bar
diff --git a/tests/functional/add.sh b/tests/functional/add.sh
index 5c3eed793..d0fedcb25 100644
--- a/tests/functional/add.sh
+++ b/tests/functional/add.sh
@@ -26,3 +26,20 @@ hash2=$(nix-hash --type sha256 --base32 ./dummy)
 echo $hash2
 
 test "$hash1" = "sha256:$hash2"
+
+#### New style commands
+
+clearStore
+
+(
+    path1=$(nix store add ./dummy)
+    path2=$(nix store add --mode nar ./dummy)
+    path3=$(nix store add-path ./dummy)
+    [[ "$path1" == "$path2" ]]
+    [[ "$path1" == "$path3" ]]
+)
+(
+    path1=$(nix store add --mode flat ./dummy)
+    path2=$(nix store add-file ./dummy)
+    [[ "$path1" == "$path2" ]]
+)

From 5196613e8290a9ee81f1b9d88e7bc61cc3f64d2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Fri, 18 Nov 2022 11:13:32 +0100
Subject: [PATCH 16/45] Use boost small vectors instead of VLAs in the primops

VLAs are a dangerous feature, and their usage triggers an undefined
behavior since theire size can be zero in some cases.
So replace them with `boost::small_vector`s which fit the same goal but
are safer.

It's also incidentally consistently 1% faster on the benchmarks.
---
 src/libexpr/primops.cc | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 8d3a18526..e7587506a 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -29,7 +29,6 @@
 
 #include <cmath>
 
-
 namespace nix {
 
 
@@ -2729,8 +2728,8 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V
     auto attrName = state.symbols.create(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.catAttrs"));
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs");
 
-    Value * res[args[1]->listSize()];
-    unsigned int found = 0;
+    boost::container::small_vector<Value *, 256> res(args[1]->listSize());
+    size_t found = 0;
 
     for (auto v2 : args[1]->listItems()) {
         state.forceAttrs(*v2, pos, "while evaluating an element in the list passed as second argument to builtins.catAttrs");
@@ -3064,9 +3063,8 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val
 
     state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filter");
 
-    // FIXME: putting this on the stack is risky.
-    Value * vs[args[1]->listSize()];
-    unsigned int k = 0;
+    boost::container::small_vector<Value *, 256> vs(args[1]->listSize());
+    size_t k = 0;
 
     bool same = true;
     for (unsigned int n = 0; n < args[1]->listSize(); ++n) {
@@ -3450,7 +3448,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap");
     auto nrLists = args[1]->listSize();
 
-    Value lists[nrLists];
+    boost::container::small_vector<Value, 64> lists(nrLists);
     size_t len = 0;
 
     for (unsigned int n = 0; n < nrLists; ++n) {

From ba3cb4a04949e043669299da5497bea27b944598 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?=
 <theophane.hufschmitt@tweag.io>
Date: Fri, 18 Nov 2022 13:02:06 +0100
Subject: [PATCH 17/45] Remove all the occurences of VLAs

There's generally no strict reason for using them, and they are somewhat
fishy, so let's avoid them.
---
 src/libexpr/eval.cc         | 13 ++++++++-----
 src/libstore/derivations.cc |  9 ++++-----
 src/libstore/gc.cc          |  9 ++-------
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index dfe81cbf7..d853b104b 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -41,6 +41,7 @@
 #include <boost/coroutine2/coroutine.hpp>
 #include <boost/coroutine2/protected_fixedsize_stack.hpp>
 #include <boost/context/stack_context.hpp>
+#include <boost/container/small_vector.hpp>
 
 #endif
 
@@ -1691,7 +1692,8 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
                 /* We have all the arguments, so call the primop with
                    the previous and new arguments. */
 
-                Value * vArgs[arity];
+                assert(arity < 64);
+                Value * vArgs[64];
                 auto n = argsDone;
                 for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left)
                     vArgs[--n] = arg->primOpApp.right;
@@ -1748,11 +1750,12 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
     Value vFun;
     fun->eval(state, env, vFun);
 
-    Value * vArgs[args.size()];
+
+    boost::container::small_vector<Value *, 64> vArgs(args.size());
     for (size_t i = 0; i < args.size(); ++i)
         vArgs[i] = args[i]->maybeThunk(state, env);
 
-    state.callFunction(vFun, args.size(), vArgs, v, pos);
+    state.callFunction(vFun, args.size(), vArgs.data(), v, pos);
 }
 
 
@@ -1991,8 +1994,8 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
         return result;
     };
 
-    Value values[es->size()];
-    Value * vTmpP = values;
+    boost::container::small_vector<Value, 64> values(es->size());
+    Value * vTmpP = values.data();
 
     for (auto & [i_pos, i] : *es) {
         Value & vTmp = *vTmpP++;
diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
index 1fecd1c97..6d9c8b9d6 100644
--- a/src/libstore/derivations.cc
+++ b/src/libstore/derivations.cc
@@ -151,11 +151,10 @@ StorePath writeDerivation(Store & store,
 /* Read string `s' from stream `str'. */
 static void expect(std::istream & str, std::string_view s)
 {
-    char s2[s.size()];
-    str.read(s2, s.size());
-    std::string_view s2View { s2, s.size() };
-    if (s2View != s)
-        throw FormatError("expected string '%s', got '%s'", s, s2View);
+    for (auto & c : s) {
+        if (str.get() != c)
+            throw FormatError("expected string '%1%'", s);
+    }
 }
 
 
diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index 8d05ae4bd..ddec43fdc 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -330,9 +330,7 @@ typedef std::unordered_map<Path, std::unordered_set<std::string>> UncheckedRoots
 
 static void readProcLink(const std::string & file, UncheckedRoots & roots)
 {
-    /* 64 is the starting buffer size gnu readlink uses... */
-    auto bufsiz = ssize_t{64};
-try_again:
+    constexpr auto bufsiz = PATH_MAX;
     char buf[bufsiz];
     auto res = readlink(file.c_str(), buf, bufsiz);
     if (res == -1) {
@@ -341,10 +339,7 @@ try_again:
         throw SysError("reading symlink");
     }
     if (res == bufsiz) {
-        if (SSIZE_MAX / 2 < bufsiz)
-            throw Error("stupidly long symlink");
-        bufsiz *= 2;
-        goto try_again;
+        throw Error("stupidly long symlink");
     }
     if (res > 0 && buf[0] == '/')
         roots[std::string(static_cast<char *>(buf), res)]

From 0daccb1121dfd5e98db3e41ba992b1b2c413dfc8 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 11:10:25 +0100
Subject: [PATCH 18/45] libexpr: Check primop arity earlier

---
 src/libexpr/eval.cc              | 20 ++++++++++++++++++--
 src/libexpr/eval.hh              | 12 ++++++++++++
 src/libexpr/tests/value/print.cc |  3 ++-
 src/libexpr/value.hh             |  8 +-------
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index d853b104b..1425eab97 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -723,6 +723,23 @@ void EvalState::addConstant(const std::string & name, Value * v, Constant info)
 }
 
 
+void PrimOp::check()
+{
+    if (arity > maxPrimOpArity) {
+        throw Error("primop arity must not exceed %1%", maxPrimOpArity);
+    }
+}
+
+
+void Value::mkPrimOp(PrimOp * p)
+{
+    p->check();
+    clearValue();
+    internalType = tPrimOp;
+    primOp = p;
+}
+
+
 Value * EvalState::addPrimOp(PrimOp && primOp)
 {
     /* Hack to make constants lazy: turn them into a application of
@@ -1692,8 +1709,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
                 /* We have all the arguments, so call the primop with
                    the previous and new arguments. */
 
-                assert(arity < 64);
-                Value * vArgs[64];
+                Value * vArgs[maxPrimOpArity];
                 auto n = argsDone;
                 for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left)
                     vArgs[--n] = arg->primOpApp.right;
diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 048dff42b..5ee6359a8 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -18,6 +18,12 @@
 
 namespace nix {
 
+/**
+ * We put a limit on primop arity because it lets us use a fixed size array on
+ * the stack. 16 is already an impractical number of arguments. Use an attrset
+ * argument for such overly complicated functions.
+ */
+constexpr size_t maxPrimOpArity = 64;
 
 class Store;
 class EvalState;
@@ -71,6 +77,12 @@ struct PrimOp
      * Optional experimental for this to be gated on.
      */
     std::optional<ExperimentalFeature> experimentalFeature;
+
+    /**
+     * Validity check to be performed by functions that introduce primops,
+     * such as RegisterPrimOp() and Value::mkPrimOp().
+     */
+    void check();
 };
 
 /**
diff --git a/src/libexpr/tests/value/print.cc b/src/libexpr/tests/value/print.cc
index 5e96e12ec..a4f6fc014 100644
--- a/src/libexpr/tests/value/print.cc
+++ b/src/libexpr/tests/value/print.cc
@@ -114,7 +114,8 @@ TEST_F(ValuePrintingTests, vLambda)
 TEST_F(ValuePrintingTests, vPrimOp)
 {
     Value vPrimOp;
-    vPrimOp.mkPrimOp(nullptr);
+    PrimOp primOp{};
+    vPrimOp.mkPrimOp(&primOp);
 
     test(vPrimOp, "<PRIMOP>");
 }
diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 622e613ea..191cc30ba 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -354,13 +354,7 @@ public:
         // Value will be overridden anyways
     }
 
-    inline void mkPrimOp(PrimOp * p)
-    {
-        clearValue();
-        internalType = tPrimOp;
-        primOp = p;
-    }
-
+    void mkPrimOp(PrimOp * p);
 
     inline void mkPrimOpApp(Value * l, Value * r)
     {

From 12c91a823e80b5e0a14a0abb0f34a6633b14bbfe Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 11:27:31 +0100
Subject: [PATCH 19/45] maxPrimOpArity: 64 -> 8

This makes stack usage significantly more compact, allowing larger
amounts of data to be processed on the same stack.

PrimOp functions with more than 8 positional (curried) arguments
should use an attrset instead.
---
 src/libexpr/eval.hh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 5ee6359a8..ce798ed96 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -20,10 +20,10 @@ namespace nix {
 
 /**
  * We put a limit on primop arity because it lets us use a fixed size array on
- * the stack. 16 is already an impractical number of arguments. Use an attrset
+ * the stack. 8 is already an impractical number of arguments. Use an attrset
  * argument for such overly complicated functions.
  */
-constexpr size_t maxPrimOpArity = 64;
+constexpr size_t maxPrimOpArity = 8;
 
 class Store;
 class EvalState;

From 9fa133dde5610dfb0605399ffea83081bda1c6fc Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 11:33:10 +0100
Subject: [PATCH 20/45] readProcLink: Replace unnecessary value judgement by
 actual info

---
 src/libstore/gc.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc
index ddec43fdc..93fa60682 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -339,7 +339,7 @@ static void readProcLink(const std::string & file, UncheckedRoots & roots)
         throw SysError("reading symlink");
     }
     if (res == bufsiz) {
-        throw Error("stupidly long symlink");
+        throw Error("overly long symlink starting with '%1%'", std::string_view(buf, bufsiz));
     }
     if (res > 0 && buf[0] == '/')
         roots[std::string(static_cast<char *>(buf), res)]

From 206ece0f41142536a856c62c49bd202282f12db8 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 12:18:37 +0100
Subject: [PATCH 21/45] builtins.{any,all}: Use constant errorCtx

Clang warned that the expanded code used to have a buffer overflow.
Very strange, but also very avoidable.
---
 src/libexpr/primops.cc | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index e7587506a..d104b7180 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -3189,10 +3189,14 @@ static void anyOrAll(bool any, EvalState & state, const PosIdx pos, Value * * ar
     state.forceFunction(*args[0], pos, std::string("while evaluating the first argument passed to builtins.") + (any ? "any" : "all"));
     state.forceList(*args[1], pos, std::string("while evaluating the second argument passed to builtins.") + (any ? "any" : "all"));
 
+    std::string_view errorCtx = any
+        ? "while evaluating the return value of the function passed to builtins.any"
+        : "while evaluating the return value of the function passed to builtins.all";
+
     Value vTmp;
     for (auto elem : args[1]->listItems()) {
         state.callFunction(*args[0], *elem, vTmp, pos);
-        bool res = state.forceBool(vTmp, pos, std::string("while evaluating the return value of the function passed to builtins.") + (any ? "any" : "all"));
+        bool res = state.forceBool(vTmp, pos, errorCtx);
         if (res == any) {
             v.mkBool(any);
             return;

From 91114a6fa48e2eb9399c23938eb12fdbd4fcda42 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 12:44:10 +0100
Subject: [PATCH 22/45] ExprCall::eval: Heap allocate at arity 5+

---
 src/libexpr/eval.cc | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 1425eab97..bfbda52ef 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1766,8 +1766,13 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
     Value vFun;
     fun->eval(state, env, vFun);
 
-
-    boost::container::small_vector<Value *, 64> vArgs(args.size());
+    // Empirical arity of Nixpkgs lambdas by regex e.g. ([a-zA-Z]+:(\s|(/\*.*\/)|(#.*\n))*){5}
+    // 2: over 4000
+    // 3: about 300
+    // 4: about 60
+    // 5: under 10
+    // This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total.
+    boost::container::small_vector<Value *, 4> vArgs(args.size());
     for (size_t i = 0; i < args.size(); ++i)
         vArgs[i] = args[i]->maybeThunk(state, env);
 

From 898c47384f651f51b3e4b63c271da274db8fca2e Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 12:48:37 +0100
Subject: [PATCH 23/45] primops: Err on the side of less stack usage

Try to stay away from stack overflows.

These small vectors use stack space. Most instances will not need
to allocate because in general most things are small, and large
things are worth heap allocating.

16 * 3 * word = 384 bytes is still quite a bit, but these functions
tend not to be part of deep recursions.
---
 src/libexpr/eval.cc    | 2 +-
 src/libexpr/primops.cc | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index bfbda52ef..2fcbf3311 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -2015,7 +2015,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
         return result;
     };
 
-    boost::container::small_vector<Value, 64> values(es->size());
+    boost::container::small_vector<Value, 16> values(es->size());
     Value * vTmpP = values.data();
 
     for (auto & [i_pos, i] : *es) {
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index d104b7180..7aa212281 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2549,7 +2549,7 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
     /* Get the attribute names to be removed.
        We keep them as Attrs instead of Symbols so std::set_difference
        can be used to remove them from attrs[0]. */
-    boost::container::small_vector<Attr, 64> names;
+    boost::container::small_vector<Attr, 16> names;
     names.reserve(args[1]->listSize());
     for (auto elem : args[1]->listItems()) {
         state.forceStringNoCtx(*elem, pos, "while evaluating the values of the second argument passed to builtins.removeAttrs");
@@ -3452,7 +3452,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap");
     auto nrLists = args[1]->listSize();
 
-    boost::container::small_vector<Value, 64> lists(nrLists);
+    boost::container::small_vector<Value, 16> lists(nrLists);
     size_t len = 0;
 
     for (unsigned int n = 0; n < nrLists; ++n) {

From 1b9813e4e60836ddb1467efd50c572e7579ac945 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 13:04:27 +0100
Subject: [PATCH 24/45] primops: Name stack reservation limits

---
 src/libexpr/eval.cc    |  3 ++-
 src/libexpr/primops.cc |  6 +++---
 src/libexpr/primops.hh | 16 ++++++++++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 2fcbf3311..8b0ada517 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -1,6 +1,7 @@
 #include "eval.hh"
 #include "eval-settings.hh"
 #include "hash.hh"
+#include "primops.hh"
 #include "types.hh"
 #include "util.hh"
 #include "store-api.hh"
@@ -2015,7 +2016,7 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
         return result;
     };
 
-    boost::container::small_vector<Value, 16> values(es->size());
+    boost::container::small_vector<Value, conservativeStackReservation> values(es->size());
     Value * vTmpP = values.data();
 
     for (auto & [i_pos, i] : *es) {
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 7aa212281..adce95bed 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2728,7 +2728,7 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V
     auto attrName = state.symbols.create(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.catAttrs"));
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs");
 
-    boost::container::small_vector<Value *, 256> res(args[1]->listSize());
+    boost::container::small_vector<Value *, nonRecursiveStackReservation> res(args[1]->listSize());
     size_t found = 0;
 
     for (auto v2 : args[1]->listItems()) {
@@ -3063,7 +3063,7 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val
 
     state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filter");
 
-    boost::container::small_vector<Value *, 256> vs(args[1]->listSize());
+    boost::container::small_vector<Value *, nonRecursiveStackReservation> vs(args[1]->listSize());
     size_t k = 0;
 
     bool same = true;
@@ -3452,7 +3452,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap");
     auto nrLists = args[1]->listSize();
 
-    boost::container::small_vector<Value, 16> lists(nrLists);
+    boost::container::small_vector<Value, conservativeStackReservation> lists(nrLists);
     size_t len = 0;
 
     for (unsigned int n = 0; n < nrLists; ++n) {
diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh
index 930e7f32a..1d5d5710d 100644
--- a/src/libexpr/primops.hh
+++ b/src/libexpr/primops.hh
@@ -8,6 +8,22 @@
 
 namespace nix {
 
+/**
+ * For functions where we do not expect deep recursion, we can use a sizable
+ * part of the stack a free allocation space.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t nonRecursiveStackReservation = 256;
+
+/**
+ * Functions that maybe applied to self-similar inputs, such as concatMap on a
+ * tree, should reserve a smaller part of the stack for allocation.
+ *
+ * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
+ */
+constexpr size_t conservativeStackReservation = 16;
+
 struct RegisterPrimOp
 {
     typedef std::vector<PrimOp> PrimOps;

From a96be29db536177fdc284b51a3b2af44a70496e0 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 13:13:01 +0100
Subject: [PATCH 25/45] removeAttrs: increase stack reservation to 64

---
 src/libexpr/primops.cc | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index adce95bed..e274c3c0c 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2549,7 +2549,8 @@ static void prim_removeAttrs(EvalState & state, const PosIdx pos, Value * * args
     /* Get the attribute names to be removed.
        We keep them as Attrs instead of Symbols so std::set_difference
        can be used to remove them from attrs[0]. */
-    boost::container::small_vector<Attr, 16> names;
+    // 64: large enough to fit the attributes of a derivation
+    boost::container::small_vector<Attr, 64> names;
     names.reserve(args[1]->listSize());
     for (auto elem : args[1]->listItems()) {
         state.forceStringNoCtx(*elem, pos, "while evaluating the values of the second argument passed to builtins.removeAttrs");

From 4e27f1947a444a36d6a85f41cbf1afdc70ac6c4c Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Thu, 16 Nov 2023 13:23:17 +0100
Subject: [PATCH 26/45] libexpr: Reduce nonRecursiveStackReservation

128 is still beyond the point where the allocation overhead is
insignificant, but we don't anticipate to overflow for these
use cases, so it's fine.
---
 src/libexpr/primops.hh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh
index 1d5d5710d..45486608f 100644
--- a/src/libexpr/primops.hh
+++ b/src/libexpr/primops.hh
@@ -14,7 +14,7 @@ namespace nix {
  *
  * Note: this is expected to be multiplied by sizeof(Value), or about 24 bytes.
  */
-constexpr size_t nonRecursiveStackReservation = 256;
+constexpr size_t nonRecursiveStackReservation = 128;
 
 /**
  * Functions that maybe applied to self-similar inputs, such as concatMap on a

From 6c8f4ef3502aa214557541ec00538e41aeced6e3 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Tue, 14 Nov 2023 10:52:57 -0500
Subject: [PATCH 27/45] Allow installing unit tests

Closes #9343

See that issue for motivation.

Installing these is disabled by default, but we enable it (and the
additional output we want isntall these too so as not to clutter the
existing ones) to use in cross builds and dev shells.
---
 Makefile.config.in          |  3 +++
 configure.ac                | 12 ++++++++++++
 flake.nix                   | 10 ++++++++--
 src/libexpr/tests/local.mk  |  6 +++++-
 src/libstore/tests/local.mk | 12 ++++++++++--
 src/libutil/tests/local.mk  | 12 ++++++++++--
 6 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/Makefile.config.in b/Makefile.config.in
index 19992fa20..1482db81f 100644
--- a/Makefile.config.in
+++ b/Makefile.config.in
@@ -28,6 +28,8 @@ SODIUM_LIBS = @SODIUM_LIBS@
 SQLITE3_LIBS = @SQLITE3_LIBS@
 bash = @bash@
 bindir = @bindir@
+checkbindir = @checkbindir@
+checklibdir = @checklibdir@
 datadir = @datadir@
 datarootdir = @datarootdir@
 doc_generate = @doc_generate@
@@ -48,4 +50,5 @@ sysconfdir = @sysconfdir@
 system = @system@
 ENABLE_BUILD = @ENABLE_BUILD@
 ENABLE_TESTS = @ENABLE_TESTS@
+INSTALL_UNIT_TESTS = @INSTALL_UNIT_TESTS@
 internal_api_docs = @internal_api_docs@
diff --git a/configure.ac b/configure.ac
index 75ce7d01d..281ba2c32 100644
--- a/configure.ac
+++ b/configure.ac
@@ -167,6 +167,18 @@ AC_ARG_ENABLE(tests, AS_HELP_STRING([--disable-tests],[Do not build the tests]),
   ENABLE_TESTS=$enableval, ENABLE_TESTS=yes)
 AC_SUBST(ENABLE_TESTS)
 
+AC_ARG_ENABLE(install-unit-tests, AS_HELP_STRING([--enable-install-unit-tests],[Install the unit tests for running later (default no)]),
+  INSTALL_UNIT_TESTS=$enableval, INSTALL_UNIT_TESTS=no)
+AC_SUBST(INSTALL_UNIT_TESTS)
+
+AC_ARG_WITH(check-bin-dir, AS_HELP_STRING([--with-check-bin-dir=PATH],[path to install unit tests for running later (defaults to $libexecdir/nix)]),
+  checkbindir=$withval, checkbindir=$libexecdir/nix)
+AC_SUBST(checkbindir)
+
+AC_ARG_WITH(check-lib-dir, AS_HELP_STRING([--with-check-lib-dir=PATH],[path to install unit tests for running later (defaults to $libdir)]),
+  checklibdir=$withval, checklibdir=$libdir)
+AC_SUBST(checklibdir)
+
 # Building without API docs is the default as Nix' C++ interfaces are internal and unstable.
 AC_ARG_ENABLE(internal_api_docs, AS_HELP_STRING([--enable-internal-api-docs],[Build API docs for Nix's internal unstable C++ interfaces]),
   internal_api_docs=$enableval, internal_api_docs=no)
diff --git a/flake.nix b/flake.nix
index 51d818423..05ab7b06d 100644
--- a/flake.nix
+++ b/flake.nix
@@ -164,6 +164,10 @@
 
         testConfigureFlags = [
           "RAPIDCHECK_HEADERS=${lib.getDev rapidcheck}/extras/gtest/include"
+        ] ++ lib.optionals (stdenv.hostPlatform != stdenv.buildPlatform) [
+          "--enable-install-unit-tests"
+          "--with-check-bin-dir=${builtins.placeholder "check"}/bin"
+          "--with-check-lib-dir=${builtins.placeholder "check"}/lib"
         ];
 
         internalApiDocsConfigureFlags = [
@@ -404,7 +408,8 @@
             src = nixSrc;
             VERSION_SUFFIX = versionSuffix;
 
-            outputs = [ "out" "dev" "doc" ];
+            outputs = [ "out" "dev" "doc" ]
+              ++ lib.optional (currentStdenv.hostPlatform != currentStdenv.buildPlatform) "check";
 
             nativeBuildInputs = nativeBuildDeps;
             buildInputs = buildDeps
@@ -710,7 +715,8 @@
           stdenv.mkDerivation {
             name = "nix";
 
-            outputs = [ "out" "dev" "doc" ];
+            outputs = [ "out" "dev" "doc" ]
+              ++ lib.optional (stdenv.hostPlatform != stdenv.buildPlatform) "check";
 
             nativeBuildInputs = nativeBuildDeps
               ++ lib.optional stdenv.cc.isClang pkgs.buildPackages.bear
diff --git a/src/libexpr/tests/local.mk b/src/libexpr/tests/local.mk
index 331a5ead6..6d2a04aaf 100644
--- a/src/libexpr/tests/local.mk
+++ b/src/libexpr/tests/local.mk
@@ -6,7 +6,11 @@ libexpr-tests_NAME := libnixexpr-tests
 
 libexpr-tests_DIR := $(d)
 
-libexpr-tests_INSTALL_DIR :=
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libexpr-tests_INSTALL_DIR := $(checkbindir)
+else
+  libexpr-tests_INSTALL_DIR :=
+endif
 
 libexpr-tests_SOURCES := \
     $(wildcard $(d)/*.cc) \
diff --git a/src/libstore/tests/local.mk b/src/libstore/tests/local.mk
index 03becc7d1..e9b8b4f99 100644
--- a/src/libstore/tests/local.mk
+++ b/src/libstore/tests/local.mk
@@ -6,7 +6,11 @@ libstore-tests-exe_NAME = libnixstore-tests
 
 libstore-tests-exe_DIR := $(d)
 
-libstore-tests-exe_INSTALL_DIR :=
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libstore-tests-exe_INSTALL_DIR := $(checkbindir)
+else
+  libstore-tests-exe_INSTALL_DIR :=
+endif
 
 libstore-tests-exe_LIBS = libstore-tests
 
@@ -18,7 +22,11 @@ libstore-tests_NAME = libnixstore-tests
 
 libstore-tests_DIR := $(d)
 
-libstore-tests_INSTALL_DIR :=
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libstore-tests_INSTALL_DIR := $(checklibdir)
+else
+  libstore-tests_INSTALL_DIR :=
+endif
 
 libstore-tests_SOURCES := $(wildcard $(d)/*.cc)
 
diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk
index c8b8557cb..e6fc4e364 100644
--- a/src/libutil/tests/local.mk
+++ b/src/libutil/tests/local.mk
@@ -6,7 +6,11 @@ libutil-tests-exe_NAME = libnixutil-tests
 
 libutil-tests-exe_DIR := $(d)
 
-libutil-tests-exe_INSTALL_DIR :=
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libutil-tests-exe_INSTALL_DIR := $(checkbindir)
+else
+  libutil-tests-exe_INSTALL_DIR :=
+endif
 
 libutil-tests-exe_LIBS = libutil-tests
 
@@ -18,7 +22,11 @@ libutil-tests_NAME = libnixutil-tests
 
 libutil-tests_DIR := $(d)
 
-libutil-tests_INSTALL_DIR :=
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libutil-tests_INSTALL_DIR := $(checklibdir)
+else
+  libutil-tests_INSTALL_DIR :=
+endif
 
 libutil-tests_SOURCES := $(wildcard $(d)/*.cc)
 

From 31ebc6028b3682969d86a7b39ae87131c41cc604 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 16 Nov 2023 16:45:14 +0100
Subject: [PATCH 28/45] Fix symlink handling

This restores the symlink handling behaviour prior to
94812cca98fbb157e5f64a15a85a2b852d289feb.

Fixes #9298.
---
 src/libexpr/eval.hh                            |  2 +-
 src/libexpr/parser.y                           | 18 +++++++++++++-----
 .../lang/eval-okay-symlink-resolution.exp      |  1 +
 .../lang/eval-okay-symlink-resolution.nix      |  1 +
 .../symlink-resolution/foo/lib/default.nix     |  1 +
 .../lang/symlink-resolution/foo/overlays       |  1 +
 .../symlink-resolution/overlays/overlay.nix    |  1 +
 7 files changed, 19 insertions(+), 6 deletions(-)
 create mode 100644 tests/functional/lang/eval-okay-symlink-resolution.exp
 create mode 100644 tests/functional/lang/eval-okay-symlink-resolution.nix
 create mode 100644 tests/functional/lang/symlink-resolution/foo/lib/default.nix
 create mode 120000 tests/functional/lang/symlink-resolution/foo/overlays
 create mode 100644 tests/functional/lang/symlink-resolution/overlays/overlay.nix

diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh
index 048dff42b..9257a0e48 100644
--- a/src/libexpr/eval.hh
+++ b/src/libexpr/eval.hh
@@ -827,7 +827,7 @@ std::string showType(const Value & v);
 /**
  * If `path` refers to a directory, then append "/default.nix".
  */
-SourcePath resolveExprPath(const SourcePath & path);
+SourcePath resolveExprPath(SourcePath path);
 
 struct InvalidPathError : EvalError
 {
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index b86cef217..f6cf1f689 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -686,17 +686,25 @@ Expr * EvalState::parse(
 }
 
 
-SourcePath resolveExprPath(const SourcePath & path)
+SourcePath resolveExprPath(SourcePath path)
 {
+    unsigned int followCount = 0, maxFollow = 1024;
+
     /* If `path' is a symlink, follow it.  This is so that relative
        path references work. */
-    auto path2 = path.resolveSymlinks();
+    while (true) {
+        // Basic cycle/depth limit to avoid infinite loops.
+        if (++followCount >= maxFollow)
+            throw Error("too many symbolic links encountered while traversing the path '%s'", path);
+        if (path.lstat().type != InputAccessor::tSymlink) break;
+        path = {path.accessor, CanonPath(path.readLink(), path.path.parent().value_or(CanonPath::root))};
+    }
 
     /* If `path' refers to a directory, append `/default.nix'. */
-    if (path2.lstat().type == InputAccessor::tDirectory)
-        return path2 + "default.nix";
+    if (path.lstat().type == InputAccessor::tDirectory)
+        return path + "default.nix";
 
-    return path2;
+    return path;
 }
 
 
diff --git a/tests/functional/lang/eval-okay-symlink-resolution.exp b/tests/functional/lang/eval-okay-symlink-resolution.exp
new file mode 100644
index 000000000..8b8441b91
--- /dev/null
+++ b/tests/functional/lang/eval-okay-symlink-resolution.exp
@@ -0,0 +1 @@
+"test"
diff --git a/tests/functional/lang/eval-okay-symlink-resolution.nix b/tests/functional/lang/eval-okay-symlink-resolution.nix
new file mode 100644
index 000000000..ffb1818bd
--- /dev/null
+++ b/tests/functional/lang/eval-okay-symlink-resolution.nix
@@ -0,0 +1 @@
+import symlink-resolution/foo/overlays/overlay.nix
diff --git a/tests/functional/lang/symlink-resolution/foo/lib/default.nix b/tests/functional/lang/symlink-resolution/foo/lib/default.nix
new file mode 100644
index 000000000..8b8441b91
--- /dev/null
+++ b/tests/functional/lang/symlink-resolution/foo/lib/default.nix
@@ -0,0 +1 @@
+"test"
diff --git a/tests/functional/lang/symlink-resolution/foo/overlays b/tests/functional/lang/symlink-resolution/foo/overlays
new file mode 120000
index 000000000..0d44a21c5
--- /dev/null
+++ b/tests/functional/lang/symlink-resolution/foo/overlays
@@ -0,0 +1 @@
+../overlays
\ No newline at end of file
diff --git a/tests/functional/lang/symlink-resolution/overlays/overlay.nix b/tests/functional/lang/symlink-resolution/overlays/overlay.nix
new file mode 100644
index 000000000..b0368308e
--- /dev/null
+++ b/tests/functional/lang/symlink-resolution/overlays/overlay.nix
@@ -0,0 +1 @@
+import ../lib

From 96d67620d551c7143b6682cfff74a2ee2edbe863 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Thu, 16 Nov 2023 17:12:06 +0100
Subject: [PATCH 29/45] Fix a broken generated header file dependency

https://hydra.nixos.org/build/240882042
---
 src/libexpr/local.mk | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/libexpr/local.mk b/src/libexpr/local.mk
index d243b9cec..ed7bf9490 100644
--- a/src/libexpr/local.mk
+++ b/src/libexpr/local.mk
@@ -43,7 +43,9 @@ $(foreach i, $(wildcard src/libexpr/value/*.hh), \
 $(foreach i, $(wildcard src/libexpr/flake/*.hh), \
   $(eval $(call install-file-in, $(i), $(includedir)/nix/flake, 0644)))
 
-$(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh
+$(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh
+
+$(d)/eval.cc: $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh
 
 $(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh
 

From c81937576928ca494af0be6e7c61f2070be5d353 Mon Sep 17 00:00:00 2001
From: Dominic Shelton <frogamic@protonmail.com>
Date: Fri, 17 Nov 2023 17:50:17 +1100
Subject: [PATCH 30/45] doc: Add example of inherit in a let expression

---
 doc/manual/src/language/constructs.md | 54 ++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/doc/manual/src/language/constructs.md b/doc/manual/src/language/constructs.md
index a3590f55d..bd7bd92a5 100644
--- a/doc/manual/src/language/constructs.md
+++ b/doc/manual/src/language/constructs.md
@@ -132,6 +132,32 @@ a = src-set.a; b = src-set.b; c = src-set.c;
 when used while defining local variables in a let-expression or while
 defining a set.
 
+in a let expression, inherit can be used to selectively bring specific attributes of a set into scope. For example
+
+
+```nix
+let
+  x = { a = 1; b = 2; };
+  inherit (builtins) attrNames;
+in
+{
+  names = attrNames x;
+}
+```
+
+is equivalent to
+
+```nix
+let
+  x = { a = 1; b = 2; };
+in
+{
+  names = builtins.attrNames x;
+}
+```
+
+both resolve to `{ names = [ "a" "b" ]; }`.
+
 ## Functions
 
 Functions have the following form:
@@ -146,65 +172,65 @@ three kinds of patterns:
 
   - If a pattern is a single identifier, then the function matches any
     argument. Example:
-    
+
     ```nix
     let negate = x: !x;
         concat = x: y: x + y;
     in if negate true then concat "foo" "bar" else ""
     ```
-    
+
     Note that `concat` is a function that takes one argument and returns
     a function that takes another argument. This allows partial
     parameterisation (i.e., only filling some of the arguments of a
     function); e.g.,
-    
+
     ```nix
     map (concat "foo") [ "bar" "bla" "abc" ]
     ```
-    
+
     evaluates to `[ "foobar" "foobla" "fooabc" ]`.
 
   - A *set pattern* of the form `{ name1, name2, …, nameN }` matches a
     set containing the listed attributes, and binds the values of those
     attributes to variables in the function body. For example, the
     function
-    
+
     ```nix
     { x, y, z }: z + y + x
     ```
-    
+
     can only be called with a set containing exactly the attributes `x`,
     `y` and `z`. No other attributes are allowed. If you want to allow
     additional arguments, you can use an ellipsis (`...`):
-    
+
     ```nix
     { x, y, z, ... }: z + y + x
     ```
-    
+
     This works on any set that contains at least the three named
     attributes.
-    
+
     It is possible to provide *default values* for attributes, in
     which case they are allowed to be missing. A default value is
     specified by writing `name ?  e`, where *e* is an arbitrary
     expression. For example,
-    
+
     ```nix
     { x, y ? "foo", z ? "bar" }: z + y + x
     ```
-    
+
     specifies a function that only requires an attribute named `x`, but
     optionally accepts `y` and `z`.
 
   - An `@`-pattern provides a means of referring to the whole value
     being matched:
-    
+
     ```nix
     args@{ x, y, z, ... }: z + y + x + args.a
     ```
-    
+
     but can also be written as:
-    
+
     ```nix
     { x, y, z, ... } @ args: z + y + x + args.a
     ```

From 2eb59c34b531f03a85f67b9246ccaf0ff5fcad23 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:02:02 +0100
Subject: [PATCH 31/45] Value: extract Value::StringWithContext

---
 src/libexpr/value.hh | 54 +++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 26 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 191cc30ba..0f8cef418 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -158,37 +158,39 @@ public:
     inline bool isPrimOp() const { return internalType == tPrimOp; };
     inline bool isPrimOpApp() const { return internalType == tPrimOpApp; };
 
+    /**
+     * Strings in the evaluator carry a so-called `context` which
+     * is a list of strings representing store paths.  This is to
+     * allow users to write things like
+     *
+     *   "--with-freetype2-library=" + freetype + "/lib"
+     *
+     * where `freetype` is a derivation (or a source to be copied
+     * to the store).  If we just concatenated the strings without
+     * keeping track of the referenced store paths, then if the
+     * string is used as a derivation attribute, the derivation
+     * will not have the correct dependencies in its inputDrvs and
+     * inputSrcs.
+
+     * The semantics of the context is as follows: when a string
+     * with context C is used as a derivation attribute, then the
+     * derivations in C will be added to the inputDrvs of the
+     * derivation, and the other store paths in C will be added to
+     * the inputSrcs of the derivations.
+
+     * For canonicity, the store paths should be in sorted order.
+     */
+    struct StringWithContext {
+        const char * c_str;
+        const char * * context; // must be in sorted order
+    };
+
     union
     {
         NixInt integer;
         bool boolean;
 
-        /**
-         * Strings in the evaluator carry a so-called `context` which
-         * is a list of strings representing store paths.  This is to
-         * allow users to write things like
-
-         *   "--with-freetype2-library=" + freetype + "/lib"
-
-         * where `freetype` is a derivation (or a source to be copied
-         * to the store).  If we just concatenated the strings without
-         * keeping track of the referenced store paths, then if the
-         * string is used as a derivation attribute, the derivation
-         * will not have the correct dependencies in its inputDrvs and
-         * inputSrcs.
-
-         * The semantics of the context is as follows: when a string
-         * with context C is used as a derivation attribute, then the
-         * derivations in C will be added to the inputDrvs of the
-         * derivation, and the other store paths in C will be added to
-         * the inputSrcs of the derivations.
-
-         * For canonicity, the store paths should be in sorted order.
-         */
-        struct {
-            const char * c_str;
-            const char * * context; // must be in sorted order
-        } string;
+        StringWithContext string;
 
         struct {
             InputAccessor * accessor;

From d8ff5cfe8eba34c8b4b5cc53f3b40cd3dfd84224 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:03:37 +0100
Subject: [PATCH 32/45] Value: extract Value::Path

---
 src/libexpr/value.hh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 0f8cef418..34f994997 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -185,6 +185,11 @@ public:
         const char * * context; // must be in sorted order
     };
 
+    struct Path {
+        InputAccessor * accessor;
+        const char * path;
+    };
+
     union
     {
         NixInt integer;
@@ -192,10 +197,7 @@ public:
 
         StringWithContext string;
 
-        struct {
-            InputAccessor * accessor;
-            const char * path;
-        } _path;
+        Path _path;
 
         Bindings * attrs;
         struct {

From b55203e874f8e4b2fc5289129efba791937c23d0 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:06:04 +0100
Subject: [PATCH 33/45] Value: extract Value::ClosureThunk

---
 src/libexpr/value.hh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 34f994997..4c51c52e4 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -190,6 +190,11 @@ public:
         const char * path;
     };
 
+    struct ClosureThunk {
+        Env * env;
+        Expr * expr;
+    };
+
     union
     {
         NixInt integer;
@@ -205,10 +210,7 @@ public:
             Value * * elems;
         } bigList;
         Value * smallList[2];
-        struct {
-            Env * env;
-            Expr * expr;
-        } thunk;
+        ClosureThunk thunk;
         struct {
             Value * left, * right;
         } app;

From 6af1d9f7b94da454252b62f0cfff4ce800c5a46b Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:06:55 +0100
Subject: [PATCH 34/45] Value: extract Value::FunctionApplicationThunk

---
 src/libexpr/value.hh | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 4c51c52e4..cfb3f5276 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -195,6 +195,10 @@ public:
         Expr * expr;
     };
 
+    struct FunctionApplicationThunk {
+        Value * left, * right;
+    };
+
     union
     {
         NixInt integer;
@@ -211,17 +215,13 @@ public:
         } bigList;
         Value * smallList[2];
         ClosureThunk thunk;
-        struct {
-            Value * left, * right;
-        } app;
+        FunctionApplicationThunk app;
         struct {
             Env * env;
             ExprLambda * fun;
         } lambda;
         PrimOp * primOp;
-        struct {
-            Value * left, * right;
-        } primOpApp;
+        FunctionApplicationThunk primOpApp;
         ExternalValueBase * external;
         NixFloat fpoint;
     };

From 7055c6528532fdd3b0ce9b8f5282b002fc011470 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:07:32 +0100
Subject: [PATCH 35/45] Value: extract Value::Lambda

---
 src/libexpr/value.hh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index cfb3f5276..93ccdbc2e 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -199,6 +199,11 @@ public:
         Value * left, * right;
     };
 
+    struct Lambda {
+        Env * env;
+        ExprLambda * fun;
+    };
+
     union
     {
         NixInt integer;
@@ -216,10 +221,7 @@ public:
         Value * smallList[2];
         ClosureThunk thunk;
         FunctionApplicationThunk app;
-        struct {
-            Env * env;
-            ExprLambda * fun;
-        } lambda;
+        Lambda lambda;
         PrimOp * primOp;
         FunctionApplicationThunk primOpApp;
         ExternalValueBase * external;

From 260c6147625e95e4772ccdee80d6463d242c7b64 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 03:31:45 +0100
Subject: [PATCH 36/45] Value: use std::span, change use of const

**`Value` and `const`**

These two deserve some explanation. We'll get to lists later.

Values can normally be thought of as immutable, except they are
are also the vehicle for call by need, which must be implemented
using mutation.

This circumstance makes a `const Value` a rather useless thing:

 - If it's a thunk, you can't evaluate it, except by copying, but
   that would not be call by need.

 - If it's not a thunk, you know the type, so the method that
   acquired it for you should have returned something more specific,
   such as a `const Bindings &` (which actually does make sense
   because that's an immutable span of pointers to mutable `Value`s.

 - If you don't care about the type yet, you might establish the
   convention that `const Value` means `deepSeq`-ed data, but
   this is hardly useful and not actually as safe as you would
   supposedly want to trust it to be - just convention.

**Lists**

`std::span` is a tuple of pointer and size - just what we need.

We don't return them as `const Value`, because considering the
first bullet point we discussed before, we'd have to force all
the list values, which isn't what we want.

So what we end up with is a nice representation of a list in
weak head normal form: the spine is immutable, but the
items may need some evaluation later.
---
 src/libexpr/value.hh | 37 ++++++++-----------------------------
 1 file changed, 8 insertions(+), 29 deletions(-)

diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh
index 93ccdbc2e..bcff8ae55 100644
--- a/src/libexpr/value.hh
+++ b/src/libexpr/value.hh
@@ -3,6 +3,7 @@
 
 #include <cassert>
 #include <climits>
+#include <span>
 
 #include "symbol-table.hh"
 #include "value/context.hh"
@@ -395,7 +396,13 @@ public:
         return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems;
     }
 
-    const Value * const * listElems() const
+    std::span<Value * const> listItems() const
+    {
+        assert(isList());
+        return std::span<Value * const>(listElems(), listSize());
+    }
+
+    Value * const * listElems() const
     {
         return internalType == tList1 || internalType == tList2 ? smallList : bigList.elems;
     }
@@ -414,34 +421,6 @@ public:
      */
     bool isTrivial() const;
 
-    auto listItems()
-    {
-        struct ListIterable
-        {
-            typedef Value * const * iterator;
-            iterator _begin, _end;
-            iterator begin() const { return _begin; }
-            iterator end() const { return _end; }
-        };
-        assert(isList());
-        auto begin = listElems();
-        return ListIterable { begin, begin + listSize() };
-    }
-
-    auto listItems() const
-    {
-        struct ConstListIterable
-        {
-            typedef const Value * const * iterator;
-            iterator _begin, _end;
-            iterator begin() const { return _begin; }
-            iterator end() const { return _end; }
-        };
-        assert(isList());
-        auto begin = listElems();
-        return ConstListIterable { begin, begin + listSize() };
-    }
-
     SourcePath path() const
     {
         assert(internalType == tPath);

From 121665f3773bc46ca6df0dda6f66b1a86e7d9e72 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 12 Nov 2023 17:51:09 +0100
Subject: [PATCH 37/45] nix-env: Use state.mkList, required for correct stats

---
 src/nix-env/nix-env.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc
index 213a20d93..ab1d8f713 100644
--- a/src/nix-env/nix-env.cc
+++ b/src/nix-env/nix-env.cc
@@ -172,7 +172,7 @@ static void loadSourceExpr(EvalState & state, const SourcePath & path, Value & v
        directory). */
     else if (st.type == InputAccessor::tDirectory) {
         auto attrs = state.buildBindings(maxAttrs);
-        attrs.alloc("_combineChannels").mkList(0);
+        state.mkList(attrs.alloc("_combineChannels"), 0);
         StringSet seen;
         getAllExprs(state, path, seen, attrs);
         v.mkAttrs(attrs);

From 7b0e8c5c2c09146722349d3fd2dd69211d8b8945 Mon Sep 17 00:00:00 2001
From: Robert Hensing <roberth@users.noreply.github.com>
Date: Fri, 17 Nov 2023 10:56:23 +0100
Subject: [PATCH 38/45] Apply suggestions from code review

Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
---
 doc/manual/src/language/constructs.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/manual/src/language/constructs.md b/doc/manual/src/language/constructs.md
index bd7bd92a5..a82ec5960 100644
--- a/doc/manual/src/language/constructs.md
+++ b/doc/manual/src/language/constructs.md
@@ -132,7 +132,7 @@ a = src-set.a; b = src-set.b; c = src-set.c;
 when used while defining local variables in a let-expression or while
 defining a set.
 
-in a let expression, inherit can be used to selectively bring specific attributes of a set into scope. For example
+In a `let` expression, `inherit` can be used to selectively bring specific attributes of a set into scope. For example
 
 
 ```nix
@@ -156,7 +156,7 @@ in
 }
 ```
 
-both resolve to `{ names = [ "a" "b" ]; }`.
+both evaluate to `{ names = [ "a" "b" ]; }`.
 
 ## Functions
 

From f7d59d0dda5e4a793e701bc8fb9136b3ef22948c Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Fri, 17 Nov 2023 14:21:17 +0100
Subject: [PATCH 39/45] Release notes

---
 doc/manual/src/SUMMARY.md.in            |  1 +
 doc/manual/src/release-notes/rl-2.19.md | 77 +++++++++++++++++++++++++
 doc/manual/src/release-notes/rl-next.md | 75 ------------------------
 3 files changed, 78 insertions(+), 75 deletions(-)
 create mode 100644 doc/manual/src/release-notes/rl-2.19.md

diff --git a/doc/manual/src/SUMMARY.md.in b/doc/manual/src/SUMMARY.md.in
index 794f78a07..8dc464abd 100644
--- a/doc/manual/src/SUMMARY.md.in
+++ b/doc/manual/src/SUMMARY.md.in
@@ -115,6 +115,7 @@
   - [C++ style guide](contributing/cxx.md)
 - [Release Notes](release-notes/release-notes.md)
   - [Release X.Y (202?-??-??)](release-notes/rl-next.md)
+  - [Release 2.19 (2023-11-17)](release-notes/rl-2.19.md)
   - [Release 2.18 (2023-09-20)](release-notes/rl-2.18.md)
   - [Release 2.17 (2023-07-24)](release-notes/rl-2.17.md)
   - [Release 2.16 (2023-05-31)](release-notes/rl-2.16.md)
diff --git a/doc/manual/src/release-notes/rl-2.19.md b/doc/manual/src/release-notes/rl-2.19.md
new file mode 100644
index 000000000..4eecaf929
--- /dev/null
+++ b/doc/manual/src/release-notes/rl-2.19.md
@@ -0,0 +1,77 @@
+# Release 2.19 (2023-11-17)
+
+- The experimental `nix` command can now act as a [shebang interpreter](@docroot@/command-ref/new-cli/nix.md#shebang-interpreter)
+  by appending the contents of any `#! nix` lines and the script's location into a single call.
+
+- [URL flake references](@docroot@/command-ref/new-cli/nix3-flake.md#flake-references) now support [percent-encoded](https://datatracker.ietf.org/doc/html/rfc3986#section-2.1) characters.
+
+- [Path-like flake references](@docroot@/command-ref/new-cli/nix3-flake.md#path-like-syntax) now accept arbitrary unicode characters (except `#` and `?`).
+
+- The experimental feature `repl-flake` is no longer needed, as its functionality is now part of the `flakes` experimental feature. To get the previous behavior, use the `--file/--expr` flags accordingly.
+
+- There is a new flake installable syntax `flakeref#.attrPath` where the "." prefix specifies that `attrPath` is interpreted from the root of the flake outputs, with no searching of default attribute prefixes like `packages.<SYSTEM>` or `legacyPackages.<SYSTEM>`.
+
+- Nix adds `apple-virt` to the default system features on macOS systems that support virtualization. This is similar to what's done for the `kvm` system feature on Linux hosts.
+
+- Add a new built-in function [`builtins.convertHash`](@docroot@/language/builtins.md#builtins-convertHash).
+
+- `nix-shell` shebang lines now support single-quoted arguments.
+
+- `builtins.fetchTree` is now its own experimental feature, [`fetch-tree`](@docroot@/contributing/experimental-features.md#xp-fetch-tree).
+  As described in the documentation for that feature, this is because we anticipate polishing it and then stabilizing it before the rest of flakes.
+
+- The interface for creating and updating lock files has been overhauled:
+
+  - [`nix flake lock`](@docroot@/command-ref/new-cli/nix3-flake-lock.md) only creates lock files and adds missing inputs now.
+    It will *never* update existing inputs.
+
+  - [`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) does the same, but *will* update inputs.
+    - Passing no arguments will update all inputs of the current flake, just like it already did.
+    - Passing input names as arguments will ensure only those are updated. This replaces the functionality of `nix flake lock --update-input`
+    - To operate on a flake outside the current directory, you must now pass `--flake path/to/flake`.
+
+  - The flake-specific flags `--recreate-lock-file` and `--update-input` have been removed from all commands operating on installables.
+    They are superceded by `nix flake update`.
+
+- Commit signature verification for the [`builtins.fetchGit`](@docroot@/language/builtins.md#builtins-fetchGit) is added as the new [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches).
+
+- [`nix path-info --json`](@docroot@/command-ref/new-cli/nix3-path-info.md)
+  (experimental) now returns a JSON map rather than JSON list.
+  The `path` field of each object has instead become the key in the outer map, since it is unique.
+  The `valid` field also goes away because we just use `null` instead.
+
+  - Old way:
+
+    ```json5
+    [
+      {
+        "path": "/nix/store/8fv91097mbh5049i9rglc73dx6kjg3qk-bash-5.2-p15",
+        "valid": true,
+        // ...
+      },
+      {
+        "path": "/nix/store/wffw7l0alvs3iw94cbgi1gmmbmw99sqb-home-manager-path",
+        "valid": false
+      }
+    ]
+    ```
+
+  - New way
+
+    ```json5
+    {
+      "/nix/store/8fv91097mbh5049i9rglc73dx6kjg3qk-bash-5.2-p15": {
+        // ...
+      },
+      "/nix/store/wffw7l0alvs3iw94cbgi1gmmbmw99sqb-home-manager-path": null,
+    }
+    ```
+
+  This makes it match `nix derivation show`, which also maps store paths to information.
+
+- When Nix is installed using the [binary installer](@docroot@/installation/installing-binary.md), in supported shells (Bash, Zsh, Fish)
+  [`XDG_DATA_DIRS`](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html#variables) is now populated with the path to the `/share` subdirectory of the current profile.
+  This means that command completion scripts, `.desktop` files, and similar artifacts installed via [`nix-env`](@docroot@/command-ref/nix-env.md) or [`nix profile`](@docroot@/command-ref/new-cli/nix3-profile.md)
+  (experimental) can be found by any program that follows the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html).
+
+- A new command `nix store add` has been added. It replaces `nix store add-file` and `nix store add-path` which are now deprecated.
diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md
index 422f1fce8..78ae99f4b 100644
--- a/doc/manual/src/release-notes/rl-next.md
+++ b/doc/manual/src/release-notes/rl-next.md
@@ -1,77 +1,2 @@
 # Release X.Y (202?-??-??)
 
-- The experimental nix command can now act as a [shebang interpreter](@docroot@/command-ref/new-cli/nix.md#shebang-interpreter)
-  by appending the contents of any `#! nix` lines and the script's location to a single call.
-
-- [URL flake references](@docroot@/command-ref/new-cli/nix3-flake.md#flake-references) now support [percent-encoded](https://datatracker.ietf.org/doc/html/rfc3986#section-2.1) characters.
-
-- [Path-like flake references](@docroot@/command-ref/new-cli/nix3-flake.md#path-like-syntax) now accept arbitrary unicode characters (except `#` and `?`).
-
-- The experimental feature `repl-flake` is no longer needed, as its functionality is now part of the `flakes` experimental feature. To get the previous behavior, use the `--file/--expr` flags accordingly.
-
-- Introduce new flake installable syntax `flakeref#.attrPath` where the "." prefix denotes no searching of default attribute prefixes like `packages.<SYSTEM>` or `legacyPackages.<SYSTEM>`.
-
-- Nix adds `apple-virt` to the default system features on macOS systems that support virtualization. This is similar to what's done for the `kvm` system feature on Linux hosts.
-
-- Introduce a new built-in function [`builtins.convertHash`](@docroot@/language/builtins.md#builtins-convertHash).
-
-- `nix-shell` shebang lines now support single-quoted arguments.
-
-- `builtins.fetchTree` is now its own experimental feature, [`fetch-tree`](@docroot@/contributing/experimental-features.md#xp-fetch-tree).
-  As described in the document for that feature, this is because we anticipate polishing it and then stabilizing it before the rest of Flakes.
-
-- The interface for creating and updating lock files has been overhauled:
-
-  - [`nix flake lock`](@docroot@/command-ref/new-cli/nix3-flake-lock.md) only creates lock files and adds missing inputs now.
-    It will *never* update existing inputs.
-
-  - [`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) does the same, but *will* update inputs.
-    - Passing no arguments will update all inputs of the current flake, just like it already did.
-    - Passing input names as arguments will ensure only those are updated. This replaces the functionality of `nix flake lock --update-input`
-    - To operate on a flake outside the current directory, you must now pass `--flake path/to/flake`.
-
-  - The flake-specific flags `--recreate-lock-file` and `--update-input` have been removed from all commands operating on installables.
-    They are superceded by `nix flake update`.
-
-- Commit signature verification for the [`builtins.fetchGit`](@docroot@/language/builtins.md#builtins-fetchGit) is added as the new [`verified-fetches` experimental feature](@docroot@/contributing/experimental-features.md#xp-feature-verified-fetches).
-
-- [`nix path-info --json`](@docroot@/command-ref/new-cli/nix3-path-info.md)
-  (experimental) now returns a JSON map rather than JSON list.
-  The `path` field of each object has instead become the key in th outer map, since it is unique.
-  The `valid` field also goes away because we just use null instead.
-
-  - Old way:
-
-    ```json5
-    [
-      {
-        "path": "/nix/store/8fv91097mbh5049i9rglc73dx6kjg3qk-bash-5.2-p15",
-        "valid": true,
-        // ...
-      },
-      {
-        "path": "/nix/store/wffw7l0alvs3iw94cbgi1gmmbmw99sqb-home-manager-path",
-        "valid": false
-      }
-    ]
-    ```
-
-  - New way
-
-    ```json5
-    {
-      "/nix/store/8fv91097mbh5049i9rglc73dx6kjg3qk-bash-5.2-p15": {
-        // ...
-      },
-      "/nix/store/wffw7l0alvs3iw94cbgi1gmmbmw99sqb-home-manager-path": null,
-    }
-    ```
-
-  This makes it match `nix derivation show`, which also maps store paths to information.
-
-- When Nix is installed using the [binary installer](@docroot@/installation/installing-binary.md), in supported shells (Bash, Zsh, Fish)
-  [`XDG_DATA_DIRS`](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html#variables) is now populated with the path to the `/share` subdirectory of the current profile.
-  This means that command completion scripts, `.desktop` files, and similar artifacts installed via [`nix-env`](@docroot@/command-ref/nix-env.md) or [`nix profile`](@docroot@/command-ref/new-cli/nix3-profile.md)
-  (experimental) can be found by any program that follows the [XDG Base Directory Specification](https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html).
-
-- A new command `nix store add` has been added. It replaces `nix store add-file` and `nix store add-path` which are now deprecated.

From 293ae592576bb9c48975466613fcba6a30d06f5e Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 17 Nov 2023 11:26:45 -0500
Subject: [PATCH 40/45] Fix `make check`

After 9c7749e13508996eb9df83b1692664cc8cdbf952, `libutil-tests_RUN`
doesn't exist. It needs to become `libutil-tests-exe_RUN`.
---
 src/libutil/tests/local.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk
index e6fc4e364..66886c45f 100644
--- a/src/libutil/tests/local.mk
+++ b/src/libutil/tests/local.mk
@@ -1,4 +1,4 @@
-check: libutil-tests_RUN
+check: libutil-tests-exe_RUN
 
 programs += libutil-tests-exe
 

From 4a539ac3eac90b2c2f839cae885df89a03240348 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Fri, 17 Nov 2023 17:38:08 +0100
Subject: [PATCH 41/45] Fix buildNoGc

Fixes https://hydra.nixos.org/build/241067941/nixlog/1

    src/libexpr/eval.cc:1776:54: error: variable 'boost::container::small_vector<nix::Value*, 4> vArgs' has initializer but incomplete type
---
 src/libexpr/eval.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index 8b0ada517..e9b8cacfd 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -31,6 +31,7 @@
 
 #include <sys/resource.h>
 #include <nlohmann/json.hpp>
+#include <boost/container/small_vector.hpp>
 
 #if HAVE_BOEHMGC
 
@@ -42,7 +43,6 @@
 #include <boost/coroutine2/coroutine.hpp>
 #include <boost/coroutine2/protected_fixedsize_stack.hpp>
 #include <boost/context/stack_context.hpp>
-#include <boost/container/small_vector.hpp>
 
 #endif
 

From 251fb23aeab8f85afb4f8376c2e6fc3d8b229d23 Mon Sep 17 00:00:00 2001
From: Robert Hensing <robert@roberthensing.nl>
Date: Sun, 19 Nov 2023 01:46:48 +0100
Subject: [PATCH 42/45] Shebang parser: add virtual destructor

Fixes:

    warning: destructor called on non-final 'nix::ParseUnquoted' that has virtual functions but non-virtual destructor [-Wdelete-non-abstract-non-virtual-dtor]
---
 src/libutil/args.cc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/libutil/args.cc b/src/libutil/args.cc
index 4359c5e8e..4480a03f5 100644
--- a/src/libutil/args.cc
+++ b/src/libutil/args.cc
@@ -97,6 +97,8 @@ struct Parser {
     virtual void operator()(std::shared_ptr<Parser> & state, Strings & r) = 0;
 
     Parser(std::string_view s) : remaining(s) {};
+
+    virtual ~Parser() { };
 };
 
 struct ParseQuoted : public Parser {

From 70ddf298e0882075dcb1cf69562c629a195718f7 Mon Sep 17 00:00:00 2001
From: Silvan Mosberger <silvan.mosberger@tweag.io>
Date: Sun, 19 Nov 2023 04:09:14 +0100
Subject: [PATCH 43/45] doc: Add link to filterSource from path

---
 src/libexpr/primops.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index e274c3c0c..dda409955 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2374,7 +2374,7 @@ static RegisterPrimOp primop_path({
           like `@`.
 
         - filter\
-          A function of the type expected by `builtins.filterSource`,
+          A function of the type expected by [`builtins.filterSource`](#builtins-filterSource),
           with the same semantics.
 
         - recursive\

From d5928085d5a542b19cc21b1f299392ee7a0c960b Mon Sep 17 00:00:00 2001
From: nicoo <nicoo@mur.at>
Date: Sun, 19 Nov 2023 19:57:07 +0100
Subject: [PATCH 44/45] builtins.concatMap: Fix typo in error message

---
 src/libexpr/primops.cc            | 2 +-
 src/libexpr/tests/error_traces.cc | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index dda409955..27f502830 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -3459,7 +3459,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
     for (unsigned int n = 0; n < nrLists; ++n) {
         Value * vElem = args[1]->listElems()[n];
         state.callFunction(*args[0], *vElem, lists[n], pos);
-        state.forceList(lists[n], lists[n].determinePos(args[0]->determinePos(pos)), "while evaluating the return value of the function passed to buitlins.concatMap");
+        state.forceList(lists[n], lists[n].determinePos(args[0]->determinePos(pos)), "while evaluating the return value of the function passed to builtins.concatMap");
         len += lists[n].listSize();
     }
 
diff --git a/src/libexpr/tests/error_traces.cc b/src/libexpr/tests/error_traces.cc
index 139366bcd..81498f65a 100644
--- a/src/libexpr/tests/error_traces.cc
+++ b/src/libexpr/tests/error_traces.cc
@@ -906,12 +906,12 @@ namespace nix {
         ASSERT_TRACE2("concatMap (x: 1) [ \"foo\" ] # TODO",
                       TypeError,
                       hintfmt("value is %s while a list was expected", "an integer"),
-                      hintfmt("while evaluating the return value of the function passed to buitlins.concatMap"));
+                      hintfmt("while evaluating the return value of the function passed to builtins.concatMap"));
 
         ASSERT_TRACE2("concatMap (x: \"foo\") [ 1 2 ] # TODO",
                       TypeError,
                       hintfmt("value is %s while a list was expected", "a string"),
-                      hintfmt("while evaluating the return value of the function passed to buitlins.concatMap"));
+                      hintfmt("while evaluating the return value of the function passed to builtins.concatMap"));
 
     }
 

From 1d6abec993a371091459d5e23f985c6d69621ce7 Mon Sep 17 00:00:00 2001
From: Eelco Dolstra <edolstra@gmail.com>
Date: Mon, 20 Nov 2023 12:35:35 +0100
Subject: [PATCH 45/45] Revert use of boost::container::small_vector in the
 evaluator

It caused random crashes (https://hydra.nixos.org/build/241514506,
https://hydra.nixos.org/build/241443330) because the heap allocation
done by small_vector in the not-small case is not scanned for GC
roots.
---
 src/libexpr/eval.cc    | 11 +++++------
 src/libexpr/primops.cc |  7 ++++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc
index e9b8cacfd..46a49c891 100644
--- a/src/libexpr/eval.cc
+++ b/src/libexpr/eval.cc
@@ -31,7 +31,6 @@
 
 #include <sys/resource.h>
 #include <nlohmann/json.hpp>
-#include <boost/container/small_vector.hpp>
 
 #if HAVE_BOEHMGC
 
@@ -1710,7 +1709,7 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value &
                 /* We have all the arguments, so call the primop with
                    the previous and new arguments. */
 
-                Value * vArgs[maxPrimOpArity];
+                Value * vArgs[arity];
                 auto n = argsDone;
                 for (Value * arg = &vCur; arg->isPrimOpApp(); arg = arg->primOpApp.left)
                     vArgs[--n] = arg->primOpApp.right;
@@ -1773,11 +1772,11 @@ void ExprCall::eval(EvalState & state, Env & env, Value & v)
     // 4: about 60
     // 5: under 10
     // This excluded attrset lambdas (`{...}:`). Contributions of mixed lambdas appears insignificant at ~150 total.
-    boost::container::small_vector<Value *, 4> vArgs(args.size());
+    Value * vArgs[args.size()];
     for (size_t i = 0; i < args.size(); ++i)
         vArgs[i] = args[i]->maybeThunk(state, env);
 
-    state.callFunction(vFun, args.size(), vArgs.data(), v, pos);
+    state.callFunction(vFun, args.size(), vArgs, v, pos);
 }
 
 
@@ -2016,8 +2015,8 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
         return result;
     };
 
-    boost::container::small_vector<Value, conservativeStackReservation> values(es->size());
-    Value * vTmpP = values.data();
+    Value values[es->size()];
+    Value * vTmpP = values;
 
     for (auto & [i_pos, i] : *es) {
         Value & vTmp = *vTmpP++;
diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc
index 27f502830..a8d44d8b7 100644
--- a/src/libexpr/primops.cc
+++ b/src/libexpr/primops.cc
@@ -2729,7 +2729,7 @@ static void prim_catAttrs(EvalState & state, const PosIdx pos, Value * * args, V
     auto attrName = state.symbols.create(state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.catAttrs"));
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.catAttrs");
 
-    boost::container::small_vector<Value *, nonRecursiveStackReservation> res(args[1]->listSize());
+    Value * res[args[1]->listSize()];
     size_t found = 0;
 
     for (auto v2 : args[1]->listItems()) {
@@ -3064,7 +3064,8 @@ static void prim_filter(EvalState & state, const PosIdx pos, Value * * args, Val
 
     state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filter");
 
-    boost::container::small_vector<Value *, nonRecursiveStackReservation> vs(args[1]->listSize());
+    // FIXME: putting this on the stack is risky.
+    Value * vs[args[1]->listSize()];
     size_t k = 0;
 
     bool same = true;
@@ -3453,7 +3454,7 @@ static void prim_concatMap(EvalState & state, const PosIdx pos, Value * * args,
     state.forceList(*args[1], pos, "while evaluating the second argument passed to builtins.concatMap");
     auto nrLists = args[1]->listSize();
 
-    boost::container::small_vector<Value, conservativeStackReservation> lists(nrLists);
+    Value lists[nrLists];
     size_t len = 0;
 
     for (unsigned int n = 0; n < nrLists; ++n) {