Merge pull request #14010 from NixOS/mergify/bp/2.31-maintenance/pr-14009

Revert "tests/nixos: Fix daemon store reference in authorization test" (backport #14009)

.github/workflows/labels.yml

@@ -18,7 +18,7 @@ jobs:
     runs-on: ubuntu-24.04
     if: github.repository_owner == 'NixOS'
     steps:
-    - uses: actions/labeler@v6
+    - uses: actions/labeler@v5
       with:
         repo-token: ${{ secrets.GITHUB_TOKEN }}
         sync-labels: false

.mergify.yml

@@ -161,14 +161,3 @@ pull_request_rules:
         labels:
           - automatic backport
           - merge-queue
-
-  - name: backport patches to 2.31
-    conditions:
-      - label=backport 2.31-maintenance
-    actions:
-      backport:
-        branches:
-          - "2.31-maintenance"
-        labels:
-          - automatic backport
-          - merge-queue

.version

@@ -1 +1 @@
-2.32.0
+2.31.2

doc/manual/rl-next/c-api-recoverable-errors.md

@@ -1,23 +0,0 @@
----
-synopsis: "C API: Errors returned from your primops are not treated as recoverable by default"
-prs: [13930]
----
-
-Nix 2.32 by default remembers the error in the thunk that triggered it.
-
-Previously the following sequence of events worked:
-
-1. Have a thunk that invokes a primop that's defined through the C API
-2. The primop returns an error
-3. Force the thunk again
-4. The primop returns a value
-5. The thunk evaluated successfully
-
-**Resolution**
-
-C API consumers that rely on this must change their recoverable error calls:
-
-```diff
--nix_set_err_msg(context, NIX_ERR_*, msg);
-+nix_set_err_msg(context, NIX_ERR_RECOVERABLE, msg);
-```

doc/manual/rl-next/dropped-compat.md

@@ -1,6 +0,0 @@
----
-synopsis: "Removed support for daemons and clients older than Nix 2.0"
-prs: [13951]
----
-
-We have dropped support in the daemon worker protocol for daemons and clients that don't speak at least version 18 of the protocol. This first Nix release that supports this version is Nix 2.0, released in February 2018.

doc/manual/source/development/building.md

@@ -34,7 +34,7 @@ $ nix-shell --attr devShells.x86_64-linux.native-clangStdenvPackages
 To build Nix itself in this shell:
 
 ```console
-[nix-shell]$ out="$(pwd)/outputs/out" dev=$out debug=$out mesonFlags+=" --prefix=${out}"
+[nix-shell]$ mesonFlags+=" --prefix=$(pwd)/outputs/out"
 [nix-shell]$ dontAddPrefix=1 configurePhase
 [nix-shell]$ buildPhase
 ```

docker.nix

@@ -281,10 +281,7 @@ let
 
           # may get replaced by pkgs.dockerTools.caCertificates
           mkdir -p $out/etc/ssl/certs
-          # Old NixOS compatibility.
           ln -s /nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt $out/etc/ssl/certs
-          # NixOS canonical location
-          ln -s /nix/var/nix/profiles/default/etc/ssl/certs/ca-bundle.crt $out/etc/ssl/certs/ca-certificates.crt
 
           cat $passwdContentsPath > $out/etc/passwd
           echo "" >> $out/etc/passwd

flake.nix

@@ -32,7 +32,7 @@
     let
       inherit (nixpkgs) lib;
 
-      officialRelease = false;
+      officialRelease = true;
 
       linux32BitSystems = [ "i686-linux" ];
       linux64BitSystems = [

maintainers/README.md

@@ -46,7 +46,7 @@ The team meets twice a week (times are denoted in the [Europe/Amsterdam](https:/
        - mark it as draft if it is blocked on the contributor
        - escalate it back to the team by moving it to To discuss, and leaving a comment as to why the issue needs to be discussed again.
 
-- Work meeting: Mondays 18:00-20:00 Europe/Amsterdam; see [calendar](https://calendar.google.com/calendar/u/0/embed?src=b9o52fobqjak8oq8lfkhg3t0qg@group.calendar.google.com).
+- Work meeting: Mondays 14:00-16:00 Europe/Amsterdam see [calendar](https://calendar.google.com/calendar/u/0/embed?src=b9o52fobqjak8oq8lfkhg3t0qg@group.calendar.google.com).
 
   1. Code review on pull requests from [In review](#in-review).
   2. Other chores and tasks.

maintainers/release-notes-todo

@@ -1,58 +0,0 @@
-#!/usr/bin/env bash
-
-set -euo pipefail
-# debug:
-# set -x
-
-START_REF="${1}"
-END_REF="${2:-upstream/master}"
-
-# Get the merge base
-MERGE_BASE=$(git merge-base "$START_REF" "$END_REF")
-unset START_REF
-
-# Get date range
-START_DATE=$(git show -s --format=%cI "$MERGE_BASE")
-END_DATE=$(git show -s --format=%cI "$END_REF")
-
-echo "Checking PRs merged between $START_DATE and $END_DATE" >&2
-
-# Get all commits between merge base and HEAD
-COMMITS=$(git rev-list "$MERGE_BASE..$END_REF")
-
-# Convert to set for fast lookup
-declare -A commit_set
-for commit in $COMMITS; do
-    commit_set["$commit"]=1
-done
-
-# Get the current changelog
-LOG_DONE="$(changelog-d doc/manual/rl-next)"
-is_done(){
-    local nr="$1"
-    echo "$LOG_DONE" | grep -E "^- .*/pull/$nr)"
-}
-
-# Query merged PRs in date range
-gh pr list \
-    --repo NixOS/nix \
-    --state merged \
-    --limit 1000 \
-    --json number,title,author,mergeCommit \
-    --search "merged:$START_DATE..$END_DATE" | \
-jq -r '.[] | [.number, .mergeCommit.oid, .title, .author.login] | @tsv' | \
-while IFS=$'\t' read -r pr_num merge_commit _title author; do
-    # Check if this PR's merge commit is in our branch
-    if [[ -n "${commit_set[$merge_commit]:-}" ]]; then
-        # Full detail, not suitable for comment due to mass ping and duplicate title
-        # echo "- #$pr_num $_title (@$author)"
-        echo "- #$pr_num ($author)"
-        if is_done "$pr_num"
-        then
-            echo "  - [x] has note"
-        else
-            echo "  - [ ] has note"
-        fi
-        echo "  - [ ] skip"
-    fi
-done

maintainers/release-process.md

@@ -24,12 +24,6 @@ release:
 * In a checkout of the Nix repo, make sure you're on `master` and run
   `git pull`.
 
-* Compile a release notes to-do list by running
-
-  ```console
-  $ ./maintainers/release-notes-todo PREV_RELEASE HEAD
-  ```
-
 * Compile the release notes by running
 
   ```console
@@ -133,8 +127,6 @@ release:
 
   Commit and push this to the maintenance branch.
 
-* Create a backport label.
-
 * Bump the version of `master`:
 
   ```console
@@ -142,7 +134,6 @@ release:
   $ git pull
   $ NEW_VERSION=2.13.0
   $ echo $NEW_VERSION > .version
-  $ ... edit .mergify.yml to add the previous version ...
   $ git checkout -b bump-$NEW_VERSION
   $ git commit -a -m 'Bump version'
   $ git push --set-upstream origin bump-$NEW_VERSION
@@ -150,6 +141,10 @@ release:
 
   Make a pull request and auto-merge it.
 
+* Create a backport label.
+
+* Add the new backport label to `.mergify.yml`.
+
 * Post an [announcement on Discourse](https://discourse.nixos.org/c/announcements/8), including the contents of
   `rl-$VERSION.md`.
 

packaging/dependencies.nix

@@ -76,16 +76,6 @@ scope: {
         prevAttrs.postInstall;
   });
 
-  toml11 = pkgs.toml11.overrideAttrs rec {
-    version = "4.4.0";
-    src = pkgs.fetchFromGitHub {
-      owner = "ToruNiina";
-      repo = "toml11";
-      tag = "v${version}";
-      hash = "sha256-sgWKYxNT22nw376ttGsTdg0AMzOwp8QH3E8mx0BZJTQ=";
-    };
-  };
-
   # TODO Hack until https://github.com/NixOS/nixpkgs/issues/45462 is fixed.
   boost =
     (pkgs.boost.override {

src/libcmd/installable-flake.cc

@@ -105,8 +105,8 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
 
     std::optional<NixInt::Inner> priority;
 
-    if (attr->maybeGetAttr(state->s.outputSpecified)) {
-    } else if (auto aMeta = attr->maybeGetAttr(state->s.meta)) {
+    if (attr->maybeGetAttr(state->sOutputSpecified)) {
+    } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
         if (auto aPriority = aMeta->maybeGetAttr("priority"))
             priority = aPriority->getInt().value;
     }
@@ -119,12 +119,12 @@ DerivedPathsWithInfo InstallableFlake::toDerivedPaths()
                     overloaded{
                         [&](const ExtendedOutputsSpec::Default & d) -> OutputsSpec {
                             StringSet outputsToInstall;
-                            if (auto aOutputSpecified = attr->maybeGetAttr(state->s.outputSpecified)) {
+                            if (auto aOutputSpecified = attr->maybeGetAttr(state->sOutputSpecified)) {
                                 if (aOutputSpecified->getBool()) {
                                     if (auto aOutputName = attr->maybeGetAttr("outputName"))
                                         outputsToInstall = {aOutputName->getString()};
                                 }
-                            } else if (auto aMeta = attr->maybeGetAttr(state->s.meta)) {
+                            } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
                                 if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall"))
                                     for (auto & s : aOutputsToInstall->getListOfStrings())
                                         outputsToInstall.insert(s);

src/libexpr-c/nix_api_expr.cc

@@ -40,8 +40,6 @@ static T * unsafe_new_with_self(F && init)
     return new (p) T(init(static_cast<T *>(p)));
 }
 
-extern "C" {
-
 nix_err nix_libexpr_init(nix_c_context * context)
 {
     if (context)
@@ -289,5 +287,3 @@ void nix_gc_register_finalizer(void * obj, void * cd, void (*finalizer)(void * o
     GC_REGISTER_FINALIZER(obj, finalizer, cd, 0, 0);
 #endif
 }
-
-} // extern "C"

src/libexpr-c/nix_api_expr_internal.h

@@ -8,8 +8,6 @@
 #include "nix_api_value.h"
 #include "nix/expr/search-path.hh"
 
-extern "C" {
-
 struct nix_eval_state_builder
 {
     nix::ref<nix::Store> store;
@@ -63,6 +61,4 @@ struct nix_realised_string
     std::vector<StorePath> storePaths;
 };
 
-} // extern "C"
-
 #endif // NIX_API_EXPR_INTERNAL_H

src/libexpr-c/nix_api_external.cc

@@ -14,8 +14,6 @@
 
 #include <nlohmann/json.hpp>
 
-extern "C" {
-
 void nix_set_string_return(nix_string_return * str, const char * c)
 {
     str->str = c;
@@ -42,8 +40,6 @@ nix_err nix_external_add_string_context(nix_c_context * context, nix_string_cont
     NIXC_CATCH_ERRS
 }
 
-} // extern "C"
-
 class NixCExternalValue : public nix::ExternalValueBase
 {
     NixCExternalValueDesc & desc;
@@ -174,8 +170,6 @@ public:
     virtual ~NixCExternalValue() override {};
 };
 
-extern "C" {
-
 ExternalValue * nix_create_external_value(nix_c_context * context, NixCExternalValueDesc * desc, void * v)
 {
     if (context)
@@ -204,5 +198,3 @@ void * nix_get_external_value_content(nix_c_context * context, ExternalValue * b
     }
     NIXC_CATCH_ERRS_NULL
 }
-
-} // extern "C"

src/libexpr-c/nix_api_value.cc

@@ -1,5 +1,4 @@
 #include "nix/expr/attr-set.hh"
-#include "nix/expr/eval-error.hh"
 #include "nix/util/configuration.hh"
 #include "nix/expr/eval.hh"
 #include "nix/store/globals.hh"
@@ -90,13 +89,8 @@ static void nix_c_primop_wrapper(
     f(userdata, &ctx, (EvalState *) &state, (nix_value **) args, (nix_value *) &vTmp);
 
     if (ctx.last_err_code != NIX_OK) {
-        if (ctx.last_err_code == NIX_ERR_RECOVERABLE) {
-            state.error<nix::RecoverableEvalError>("Recoverable error from custom function: %s", *ctx.last_err)
-                .atPos(pos)
-                .debugThrow();
-        } else {
-            state.error<nix::EvalError>("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow();
-        }
+        /* TODO: Throw different errors depending on the error code */
+        state.error<nix::EvalError>("Error from custom function: %s", *ctx.last_err).atPos(pos).debugThrow();
     }
 
     if (!vTmp.isValid()) {
@@ -117,8 +111,6 @@ static void nix_c_primop_wrapper(
     v = vTmp;
 }
 
-extern "C" {
-
 PrimOp * nix_alloc_primop(
     nix_c_context * context,
     PrimOpFun fun,
@@ -183,8 +175,6 @@ ValueType nix_get_type(nix_c_context * context, const nix_value * value)
         switch (v.type()) {
         case nThunk:
             return NIX_TYPE_THUNK;
-        case nFailed:
-            return NIX_TYPE_FAILED;
         case nInt:
             return NIX_TYPE_INT;
         case nFloat:
@@ -602,7 +592,7 @@ nix_err nix_bindings_builder_insert(nix_c_context * context, BindingsBuilder * b
         context->last_err_code = NIX_OK;
     try {
         auto & v = check_value_not_null(value);
-        nix::Symbol s = bb->builder.state.get().symbols.create(name);
+        nix::Symbol s = bb->builder.state.symbols.create(name);
         bb->builder.insert(s, &v);
     }
     NIXC_CATCH_ERRS
@@ -661,5 +651,3 @@ const StorePath * nix_realised_string_get_store_path(nix_realised_string * s, si
 {
     return &s->storePaths[i];
 }
-
-} // extern "C"

src/libexpr-c/nix_api_value.h

@@ -32,8 +32,7 @@ typedef enum {
     NIX_TYPE_ATTRS,
     NIX_TYPE_LIST,
     NIX_TYPE_FUNCTION,
-    NIX_TYPE_EXTERNAL,
-    NIX_TYPE_FAILED,
+    NIX_TYPE_EXTERNAL
 } ValueType;
 
 // forward declarations

src/libexpr-tests/json.cc

@@ -54,7 +54,7 @@ TEST_F(JSONValueTest, IntNegative)
 TEST_F(JSONValueTest, String)
 {
     Value v;
-    v.mkStringNoCopy("test");
+    v.mkString("test");
     ASSERT_EQ(getJSONValue(v), "\"test\"");
 }
 
@@ -62,7 +62,7 @@ TEST_F(JSONValueTest, StringQuotes)
 {
     Value v;
 
-    v.mkStringNoCopy("test\"");
+    v.mkString("test\"");
     ASSERT_EQ(getJSONValue(v), "\"test\\\"\"");
 }
 

src/libexpr-tests/meson.build

@@ -55,7 +55,6 @@ sources = files(
   'nix_api_expr.cc',
   'nix_api_external.cc',
   'nix_api_value.cc',
-  'nix_api_value_internal.cc',
   'primops.cc',
   'search-path.cc',
   'trivial.cc',

src/libexpr-tests/nix_api_expr.cc

@@ -1,5 +1,7 @@
 #include "nix_api_store.h"
+#include "nix_api_store_internal.h"
 #include "nix_api_util.h"
+#include "nix_api_util_internal.h"
 #include "nix_api_expr.h"
 #include "nix_api_value.h"
 
@@ -149,8 +151,8 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_value)
     assert_ctx_ok();
     auto r = nix_string_realise(ctx, state, value, false);
     ASSERT_EQ(nullptr, r);
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
-    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("cannot coerce"));
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("cannot coerce")));
 }
 
 TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build)
@@ -166,8 +168,8 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build)
     assert_ctx_ok();
     auto r = nix_string_realise(ctx, state, value, false);
     ASSERT_EQ(nullptr, r);
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
-    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("failed with exit code 1"));
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("failed with exit code 1")));
 }
 
 TEST_F(nix_api_expr_test, nix_expr_realise_context)
@@ -379,11 +381,12 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_no_return)
     nix_value * result = nix_alloc_value(ctx, state);
     assert_ctx_ok();
     nix_value_call(ctx, state, primopValue, three, result);
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
     ASSERT_THAT(
-        nix_err_msg(nullptr, ctx, nullptr),
-        testing::HasSubstr("Implementation error in custom function: return value was not initialized"));
-    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badNoReturn"));
+        ctx->last_err,
+        testing::Optional(
+            testing::HasSubstr("Implementation error in custom function: return value was not initialized")));
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badNoReturn")));
 }
 
 static void primop_bad_return_thunk(
@@ -416,11 +419,12 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_return_thunk)
     assert_ctx_ok();
     NIX_VALUE_CALL(ctx, state, result, primopValue, toString, four);
 
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
     ASSERT_THAT(
-        nix_err_msg(nullptr, ctx, nullptr),
-        testing::HasSubstr("Implementation error in custom function: return value must not be a thunk"));
-    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badReturnThunk"));
+        ctx->last_err,
+        testing::Optional(
+            testing::HasSubstr("Implementation error in custom function: return value must not be a thunk")));
+    ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badReturnThunk")));
 }
 
 TEST_F(nix_api_expr_test, nix_value_call_multi_no_args)
@@ -437,105 +441,4 @@ TEST_F(nix_api_expr_test, nix_value_call_multi_no_args)
     assert_ctx_ok();
     ASSERT_EQ(3, rInt);
 }
-
-// The following is a test case for retryable thunks.
-// This is a requirement for the current way in which NixOps4 evaluates its deployment expressions.
-// An alternative strategy could be implemented, but unwinding the stack may be a more efficient way to deal with many
-// suspensions/resumptions, compared to e.g. using a thread or coroutine stack for each suspended dependency. This test
-// models the essential bits of a deployment tool that uses such a strategy.
-
-// State for the retryable primop - simulates deployment resource availability
-struct DeploymentResourceState
-{
-    bool vm_created = false;
-};
-
-static void primop_load_resource_input(
-    void * user_data, nix_c_context * context, EvalState * state, nix_value ** args, nix_value * ret)
-{
-    assert(context);
-    assert(state);
-    auto * resource_state = static_cast<DeploymentResourceState *>(user_data);
-
-    // Get the resource input name argument
-    std::string input_name;
-    if (nix_get_string(context, args[0], OBSERVE_STRING(input_name)) != NIX_OK)
-        return;
-
-    // Only handle "vm_id" input - throw for anything else
-    if (input_name != "vm_id") {
-        std::string error_msg = "unknown resource input: " + input_name;
-        nix_set_err_msg(context, NIX_ERR_NIX_ERROR, error_msg.c_str());
-        return;
-    }
-
-    if (resource_state->vm_created) {
-        // VM has been created, return the ID
-        nix_init_string(context, ret, "vm-12345");
-    } else {
-        // VM not created yet, fail with dependency error
-        nix_set_err_msg(context, NIX_ERR_RECOVERABLE, "VM not yet created");
-    }
-}
-
-TEST_F(nix_api_expr_test, nix_expr_thunk_re_evaluation_after_deployment)
-{
-    // This test demonstrates NixOps4's requirement: a thunk calling a primop should be
-    // re-evaluable when deployment resources become available that were not available initially.
-
-    DeploymentResourceState resource_state;
-
-    PrimOp * primop = nix_alloc_primop(
-        ctx,
-        primop_load_resource_input,
-        1,
-        "loadResourceInput",
-        nullptr,
-        "load a deployment resource input",
-        &resource_state);
-    assert_ctx_ok();
-
-    nix_value * primopValue = nix_alloc_value(ctx, state);
-    assert_ctx_ok();
-    nix_init_primop(ctx, primopValue, primop);
-    assert_ctx_ok();
-
-    nix_value * inputName = nix_alloc_value(ctx, state);
-    assert_ctx_ok();
-    nix_init_string(ctx, inputName, "vm_id");
-    assert_ctx_ok();
-
-    // Create a single thunk by using nix_init_apply instead of nix_value_call
-    // This creates a lazy application that can be forced multiple times
-    nix_value * thunk = nix_alloc_value(ctx, state);
-    assert_ctx_ok();
-    nix_init_apply(ctx, thunk, primopValue, inputName);
-    assert_ctx_ok();
-
-    // First force: VM not created yet, should fail
-    nix_value_force(ctx, state, thunk);
-    ASSERT_EQ(NIX_ERR_NIX_ERROR, nix_err_code(ctx));
-    ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("VM not yet created"));
-
-    // Clear the error context for the next attempt
-    nix_c_context_free(ctx);
-    ctx = nix_c_context_create();
-
-    // Simulate deployment process: VM gets created
-    resource_state.vm_created = true;
-
-    // Second force of the SAME thunk: this is where the "failed" value issue appears
-    // With failed value caching, this should fail because the thunk is marked as permanently failed
-    // Without failed value caching (or with retryable failures), this should succeed
-    nix_value_force(ctx, state, thunk);
-
-    // If we get here without error, the thunk was successfully re-evaluated
-    assert_ctx_ok();
-
-    std::string result;
-    nix_get_string(ctx, thunk, OBSERVE_STRING(result));
-    assert_ctx_ok();
-    ASSERT_STREQ("vm-12345", result.c_str());
-}
-
 } // namespace nixC

src/libexpr-tests/nix_api_external.cc

@@ -1,6 +1,9 @@
 #include "nix_api_store.h"
+#include "nix_api_store_internal.h"
 #include "nix_api_util.h"
+#include "nix_api_util_internal.h"
 #include "nix_api_expr.h"
+#include "nix_api_expr_internal.h"
 #include "nix_api_value.h"
 #include "nix_api_external.h"
 
@@ -36,7 +39,7 @@ private:
         std::string type_string = "nix-external<MyExternalValueDesc( ";
         type_string += std::to_string(obj->_x);
         type_string += " )>";
-        nix_set_string_return(res, &*type_string.begin());
+        res->str = &*type_string.begin();
     }
 };
 

src/libexpr-tests/nix_api_value.cc

@@ -1,7 +1,10 @@
 #include "nix_api_store.h"
+#include "nix_api_store_internal.h"
 #include "nix_api_util.h"
+#include "nix_api_util_internal.h"
 #include "nix_api_expr.h"
 #include "nix_api_value.h"
+#include "nix_api_expr_internal.h"
 
 #include "nix/expr/tests/nix_api_expr.hh"
 #include "nix/util/tests/string_callback.hh"
@@ -13,6 +16,14 @@
 
 namespace nixC {
 
+TEST_F(nix_api_expr_test, as_nix_value_ptr)
+{
+    // nix_alloc_value casts nix::Value to nix_value
+    // It should be obvious from the decl that that works, but if it doesn't,
+    // the whole implementation would be utterly broken.
+    ASSERT_EQ(sizeof(nix::Value), sizeof(nix_value));
+}
+
 TEST_F(nix_api_expr_test, nix_value_get_int_invalid)
 {
     ASSERT_EQ(0, nix_get_int(ctx, nullptr));
@@ -309,10 +320,8 @@ TEST_F(nix_api_expr_test, nix_value_init_apply_error)
 
     // Evaluate it
     nix_value_force(ctx, state, v);
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
-    ASSERT_THAT(
-        nix_err_msg(nullptr, ctx, nullptr),
-        testing::HasSubstr("attempt to call something which is not a function but"));
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
+    ASSERT_THAT(ctx->last_err.value(), testing::HasSubstr("attempt to call something which is not a function but"));
 
     // Clean up
     nix_gc_decref(ctx, some_string);
@@ -371,9 +380,7 @@ TEST_F(nix_api_expr_test, nix_value_init_apply_lazy_arg)
     // nix_get_attr_byname isn't lazy (it could have been) so it will throw the exception
     nix_value * foo = nix_get_attr_byname(ctx, r, state, "foo");
     ASSERT_EQ(nullptr, foo);
-    ASSERT_THAT(
-        nix_err_msg(nullptr, ctx, nullptr),
-        testing::HasSubstr("error message for test case nix_value_init_apply_lazy_arg"));
+    ASSERT_THAT(ctx->last_err.value(), testing::HasSubstr("error message for test case nix_value_init_apply_lazy_arg"));
 
     // Clean up
     nix_gc_decref(ctx, f);

src/libexpr-tests/nix_api_value_internal.cc

@@ -1,25 +0,0 @@
-#include "nix_api_store.h"
-#include "nix_api_util.h"
-#include "nix_api_expr.h"
-#include "nix_api_value.h"
-#include "nix_api_expr_internal.h"
-
-#include "nix/expr/tests/nix_api_expr.hh"
-#include "nix/util/tests/string_callback.hh"
-
-#include <gmock/gmock.h>
-#include <cstddef>
-#include <cstdlib>
-#include <gtest/gtest.h>
-
-namespace nixC {
-
-TEST_F(nix_api_expr_test, as_nix_value_ptr)
-{
-    // nix_alloc_value casts nix::Value to nix_value
-    // It should be obvious from the decl that that works, but if it doesn't,
-    // the whole implementation would be utterly broken.
-    ASSERT_EQ(sizeof(nix::Value), sizeof(nix_value));
-}
-
-} // namespace nixC

src/libexpr-tests/value/print.cc

@@ -35,14 +35,14 @@ TEST_F(ValuePrintingTests, tBool)
 TEST_F(ValuePrintingTests, tString)
 {
     Value vString;
-    vString.mkStringNoCopy("some-string");
+    vString.mkString("some-string");
     test(vString, "\"some-string\"");
 }
 
 TEST_F(ValuePrintingTests, tPath)
 {
     Value vPath;
-    vPath.mkStringNoCopy("/foo");
+    vPath.mkString("/foo");
     test(vPath, "\"/foo\"");
 }
 
@@ -61,7 +61,7 @@ TEST_F(ValuePrintingTests, tAttrs)
     Value vTwo;
     vTwo.mkInt(2);
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("one"), &vOne);
     builder.insert(state.symbols.create("two"), &vTwo);
 
@@ -196,11 +196,11 @@ TEST_F(ValuePrintingTests, depthAttrs)
     Value vTwo;
     vTwo.mkInt(2);
 
-    BindingsBuilder builderEmpty = state.buildBindings(0);
+    BindingsBuilder builderEmpty(state, state.allocBindings(0));
     Value vAttrsEmpty;
     vAttrsEmpty.mkAttrs(builderEmpty.finish());
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("one"), &vOne);
     builder.insert(state.symbols.create("two"), &vTwo);
     builder.insert(state.symbols.create("nested"), &vAttrsEmpty);
@@ -208,7 +208,7 @@ TEST_F(ValuePrintingTests, depthAttrs)
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());
 
-    BindingsBuilder builder2 = state.buildBindings(10);
+    BindingsBuilder builder2(state, state.allocBindings(10));
     builder2.insert(state.symbols.create("one"), &vOne);
     builder2.insert(state.symbols.create("two"), &vTwo);
     builder2.insert(state.symbols.create("nested"), &vAttrs);
@@ -233,14 +233,14 @@ TEST_F(ValuePrintingTests, depthList)
     Value vTwo;
     vTwo.mkInt(2);
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("one"), &vOne);
     builder.insert(state.symbols.create("two"), &vTwo);
 
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());
 
-    BindingsBuilder builder2 = state.buildBindings(10);
+    BindingsBuilder builder2(state, state.allocBindings(10));
     builder2.insert(state.symbols.create("one"), &vOne);
     builder2.insert(state.symbols.create("two"), &vTwo);
     builder2.insert(state.symbols.create("nested"), &vAttrs);
@@ -290,12 +290,12 @@ TEST_F(StringPrintingTests, maxLengthTruncation)
 TEST_F(ValuePrintingTests, attrsTypeFirst)
 {
     Value vType;
-    vType.mkStringNoCopy("puppy");
+    vType.mkString("puppy");
 
     Value vApple;
-    vApple.mkStringNoCopy("apple");
+    vApple.mkString("apple");
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("type"), &vType);
     builder.insert(state.symbols.create("apple"), &vApple);
 
@@ -334,7 +334,7 @@ TEST_F(ValuePrintingTests, ansiColorsBool)
 TEST_F(ValuePrintingTests, ansiColorsString)
 {
     Value v;
-    v.mkStringNoCopy("puppy");
+    v.mkString("puppy");
 
     test(v, ANSI_MAGENTA "\"puppy\"" ANSI_NORMAL, PrintOptions{.ansiColors = true});
 }
@@ -342,7 +342,7 @@ TEST_F(ValuePrintingTests, ansiColorsString)
 TEST_F(ValuePrintingTests, ansiColorsStringElided)
 {
     Value v;
-    v.mkStringNoCopy("puppy");
+    v.mkString("puppy");
 
     test(
         v,
@@ -374,7 +374,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs)
     Value vTwo;
     vTwo.mkInt(2);
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("one"), &vOne);
     builder.insert(state.symbols.create("two"), &vTwo);
 
@@ -390,10 +390,10 @@ TEST_F(ValuePrintingTests, ansiColorsAttrs)
 TEST_F(ValuePrintingTests, ansiColorsDerivation)
 {
     Value vDerivation;
-    vDerivation.mkStringNoCopy("derivation");
+    vDerivation.mkString("derivation");
 
-    BindingsBuilder builder = state.buildBindings(10);
-    builder.insert(state.s.type, &vDerivation);
+    BindingsBuilder builder(state, state.allocBindings(10));
+    builder.insert(state.sType, &vDerivation);
 
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());
@@ -413,7 +413,7 @@ TEST_F(ValuePrintingTests, ansiColorsError)
 {
     Value throw_ = state.getBuiltin("throw");
     Value message;
-    message.mkStringNoCopy("uh oh!");
+    message.mkString("uh oh!");
     Value vError;
     vError.mkApp(&throw_, &message);
 
@@ -430,16 +430,16 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError)
 {
     Value throw_ = state.getBuiltin("throw");
     Value message;
-    message.mkStringNoCopy("uh oh!");
+    message.mkString("uh oh!");
     Value vError;
     vError.mkApp(&throw_, &message);
 
     Value vDerivation;
-    vDerivation.mkStringNoCopy("derivation");
+    vDerivation.mkString("derivation");
 
-    BindingsBuilder builder = state.buildBindings(10);
-    builder.insert(state.s.type, &vDerivation);
-    builder.insert(state.s.drvPath, &vError);
+    BindingsBuilder builder(state, state.allocBindings(10));
+    builder.insert(state.sType, &vDerivation);
+    builder.insert(state.sDrvPath, &vError);
 
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());
@@ -553,12 +553,12 @@ TEST_F(ValuePrintingTests, ansiColorsBlackhole)
 
 TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated)
 {
-    BindingsBuilder emptyBuilder = state.buildBindings(1);
+    BindingsBuilder emptyBuilder(state, state.allocBindings(1));
 
     Value vEmpty;
     vEmpty.mkAttrs(emptyBuilder.finish());
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("a"), &vEmpty);
     builder.insert(state.symbols.create("b"), &vEmpty);
 
@@ -570,7 +570,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsRepeated)
 
 TEST_F(ValuePrintingTests, ansiColorsListRepeated)
 {
-    BindingsBuilder emptyBuilder = state.buildBindings(1);
+    BindingsBuilder emptyBuilder(state, state.allocBindings(1));
 
     Value vEmpty;
     vEmpty.mkAttrs(emptyBuilder.finish());
@@ -586,7 +586,7 @@ TEST_F(ValuePrintingTests, ansiColorsListRepeated)
 
 TEST_F(ValuePrintingTests, listRepeated)
 {
-    BindingsBuilder emptyBuilder = state.buildBindings(1);
+    BindingsBuilder emptyBuilder(state, state.allocBindings(1));
 
     Value vEmpty;
     vEmpty.mkAttrs(emptyBuilder.finish());
@@ -609,7 +609,7 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided)
     Value vTwo;
     vTwo.mkInt(2);
 
-    BindingsBuilder builder = state.buildBindings(10);
+    BindingsBuilder builder(state, state.allocBindings(10));
     builder.insert(state.symbols.create("one"), &vOne);
     builder.insert(state.symbols.create("two"), &vTwo);
 
@@ -635,6 +635,8 @@ TEST_F(ValuePrintingTests, ansiColorsAttrsElided)
 
 TEST_F(ValuePrintingTests, ansiColorsListElided)
 {
+    BindingsBuilder emptyBuilder(state, state.allocBindings(1));
+
     Value vOne;
     vOne.mkInt(1);
 

src/libexpr/attr-set.cc

@@ -16,19 +16,19 @@ Bindings * EvalState::allocBindings(size_t capacity)
         throw Error("attribute set of size %d is too big", capacity);
     nrAttrsets++;
     nrAttrsInAttrsets += capacity;
-    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings();
+    return new (allocBytes(sizeof(Bindings) + sizeof(Attr) * capacity)) Bindings((Bindings::size_t) capacity);
 }
 
 Value & BindingsBuilder::alloc(Symbol name, PosIdx pos)
 {
-    auto value = state.get().allocValue();
+    auto value = state.allocValue();
     bindings->push_back(Attr(name, value, pos));
     return *value;
 }
 
 Value & BindingsBuilder::alloc(std::string_view name, PosIdx pos)
 {
-    return alloc(state.get().symbols.create(name), pos);
+    return alloc(state.symbols.create(name), pos);
 }
 
 void Bindings::sort()

src/libexpr/eval-cache.cc

@@ -330,7 +330,7 @@ AttrCursor::AttrCursor(
 AttrKey AttrCursor::getKey()
 {
     if (!parent)
-        return {0, root->state.s.epsilon};
+        return {0, root->state.sEpsilon};
     if (!parent->first->cachedValue) {
         parent->first->cachedValue = root->db->getAttr(parent->first->getKey());
         assert(parent->first->cachedValue);
@@ -702,7 +702,7 @@ bool AttrCursor::isDerivation()
 
 StorePath AttrCursor::forceDerivation()
 {
-    auto aDrvPath = getAttr(root->state.s.drvPath);
+    auto aDrvPath = getAttr(root->state.sDrvPath);
     auto drvPath = root->state.store->parseStorePath(aDrvPath->getString());
     drvPath.requireDerivation();
     if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) {

src/libexpr/eval-error.cc

@@ -113,6 +113,5 @@ template class EvalErrorBuilder<MissingArgumentError>;
 template class EvalErrorBuilder<InfiniteRecursionError>;
 template class EvalErrorBuilder<InvalidPathError>;
 template class EvalErrorBuilder<IFDError>;
-template class EvalErrorBuilder<RecoverableEvalError>;
 
 } // namespace nix

src/libexpr/eval-gc.cc

@@ -16,7 +16,6 @@
 #  endif
 
 #  include <gc/gc_allocator.h>
-#  include <gc/gc_tiny_fl.h> // For GC_GRANULE_BYTES
 
 #  include <boost/coroutine2/coroutine.hpp>
 #  include <boost/coroutine2/protected_fixedsize_stack.hpp>
@@ -24,17 +23,6 @@
 
 #endif
 
-/*
- * Ensure that Boehm satisfies our alignment requirements. This is the default configuration [^]
- * and this assertion should never break for any platform. Let's assert it just in case.
- *
- * This alignment is particularly useful to be able to use aligned
- * load/store instructions for loading/writing Values.
- *
- * [^]: https://github.com/bdwgc/bdwgc/blob/54ac18ccbc5a833dd7edaff94a10ab9b65044d61/include/gc/gc_tiny_fl.h#L31-L33
- */
-static_assert(sizeof(void *) * 2 == GC_GRANULE_BYTES, "Boehm GC must use GC_GRANULE_WORDS = 2");
-
 namespace nix {
 
 #if NIX_USE_BOEHMGC

src/libexpr/eval-profiler.cc

@@ -185,7 +185,7 @@ FrameInfo SampleStack::getPrimOpFrameInfo(const PrimOp & primOp, std::span<Value
                 /* Error context strings don't actually matter, since we ignore all eval errors. */
                 state.forceAttrs(*args[0], pos, "");
                 auto attrs = args[0]->attrs();
-                auto nameAttr = state.getAttr(state.s.name, attrs, "");
+                auto nameAttr = state.getAttr(state.sName, attrs, "");
                 auto drvName = std::string(state.forceStringNoCtx(*nameAttr->value, pos, ""));
                 return DerivationStrictFrameInfo{.callPos = pos, .drvName = std::move(drvName)};
             } catch (...) {
@@ -211,7 +211,7 @@ FrameInfo SampleStack::getFrameInfoFromValueAndPos(const Value & v, std::span<Va
         /* Resolve primOp eagerly. Must not hold on to a reference to a Value. */
         return PrimOpFrameInfo{.expr = v.primOpAppPrimOp(), .callPos = pos};
     else if (state.isFunctor(v)) {
-        const auto functor = v.attrs()->get(state.s.functor);
+        const auto functor = v.attrs()->get(state.sFunctor);
         if (auto pos_ = posCache.lookup(pos); std::holds_alternative<std::monostate>(pos_.origin))
             /* HACK: In case callsite position is unresolved. */
             return FunctorFrameInfo{.pos = functor->pos};

src/libexpr/eval.cc

@@ -1,5 +1,4 @@
 #include "nix/expr/eval.hh"
-#include "nix/expr/eval-error.hh"
 #include "nix/expr/eval-settings.hh"
 #include "nix/expr/primops.hh"
 #include "nix/expr/print-options.hh"
@@ -23,12 +22,10 @@
 #include "nix/fetchers/fetch-to-store.hh"
 #include "nix/fetchers/tarball.hh"
 #include "nix/fetchers/input-cache.hh"
-#include "nix/util/current-process.hh"
 
 #include "parser-tab.hh"
 
 #include <algorithm>
-#include <exception>
 #include <iostream>
 #include <sstream>
 #include <cstring>
@@ -41,6 +38,10 @@
 #include <nlohmann/json.hpp>
 #include <boost/container/small_vector.hpp>
 
+#ifndef _WIN32 // TODO use portable implementation
+#  include <sys/resource.h>
+#endif
+
 #include "nix/util/strings-inline.hh"
 
 using json = nlohmann::json;
@@ -126,8 +127,6 @@ std::string_view showType(ValueType type, bool withArticle)
         return WA("a", "float");
     case nThunk:
         return WA("a", "thunk");
-    case nFailed:
-        return WA("a", "failure");
     }
     unreachable();
 }
@@ -204,65 +203,124 @@ EvalState::EvalState(
     std::shared_ptr<Store> buildStore)
     : fetchSettings{fetchSettings}
     , settings{settings}
-    , symbols(StaticEvalSymbols::staticSymbolTable())
+    , sWith(symbols.create("<with>"))
+    , sOutPath(symbols.create("outPath"))
+    , sDrvPath(symbols.create("drvPath"))
+    , sType(symbols.create("type"))
+    , sMeta(symbols.create("meta"))
+    , sName(symbols.create("name"))
+    , sValue(symbols.create("value"))
+    , sSystem(symbols.create("system"))
+    , sOverrides(symbols.create("__overrides"))
+    , sOutputs(symbols.create("outputs"))
+    , sOutputName(symbols.create("outputName"))
+    , sIgnoreNulls(symbols.create("__ignoreNulls"))
+    , sFile(symbols.create("file"))
+    , sLine(symbols.create("line"))
+    , sColumn(symbols.create("column"))
+    , sFunctor(symbols.create("__functor"))
+    , sToString(symbols.create("__toString"))
+    , sRight(symbols.create("right"))
+    , sWrong(symbols.create("wrong"))
+    , sStructuredAttrs(symbols.create("__structuredAttrs"))
+    , sJson(symbols.create("__json"))
+    , sAllowedReferences(symbols.create("allowedReferences"))
+    , sAllowedRequisites(symbols.create("allowedRequisites"))
+    , sDisallowedReferences(symbols.create("disallowedReferences"))
+    , sDisallowedRequisites(symbols.create("disallowedRequisites"))
+    , sMaxSize(symbols.create("maxSize"))
+    , sMaxClosureSize(symbols.create("maxClosureSize"))
+    , sBuilder(symbols.create("builder"))
+    , sArgs(symbols.create("args"))
+    , sContentAddressed(symbols.create("__contentAddressed"))
+    , sImpure(symbols.create("__impure"))
+    , sOutputHash(symbols.create("outputHash"))
+    , sOutputHashAlgo(symbols.create("outputHashAlgo"))
+    , sOutputHashMode(symbols.create("outputHashMode"))
+    , sRecurseForDerivations(symbols.create("recurseForDerivations"))
+    , sDescription(symbols.create("description"))
+    , sSelf(symbols.create("self"))
+    , sEpsilon(symbols.create(""))
+    , sStartSet(symbols.create("startSet"))
+    , sOperator(symbols.create("operator"))
+    , sKey(symbols.create("key"))
+    , sPath(symbols.create("path"))
+    , sPrefix(symbols.create("prefix"))
+    , sOutputSpecified(symbols.create("outputSpecified"))
+    , exprSymbols{
+        .sub = symbols.create("__sub"),
+        .lessThan = symbols.create("__lessThan"),
+        .mul = symbols.create("__mul"),
+        .div = symbols.create("__div"),
+        .or_ = symbols.create("or"),
+        .findFile = symbols.create("__findFile"),
+        .nixPath = symbols.create("__nixPath"),
+        .body = symbols.create("body"),
+    }
     , repair(NoRepair)
-    , emptyBindings(Bindings())
-    , storeFS(makeMountedSourceAccessor({
-          {CanonPath::root, makeEmptySourceAccessor()},
-          /* In the pure eval case, we can simply require
-             valid paths. However, in the *impure* eval
-             case this gets in the way of the union
-             mechanism, because an invalid access in the
-             upper layer will *not* be caught by the union
-             source accessor, but instead abort the entire
-             lookup.
+    , emptyBindings(0)
+    , storeFS(
+        makeMountedSourceAccessor(
+            {
+                {CanonPath::root, makeEmptySourceAccessor()},
+                /* In the pure eval case, we can simply require
+                   valid paths. However, in the *impure* eval
+                   case this gets in the way of the union
+                   mechanism, because an invalid access in the
+                   upper layer will *not* be caught by the union
+                   source accessor, but instead abort the entire
+                   lookup.
 
-             This happens when the store dir in the
-             ambient file system has a path (e.g. because
-             another Nix store there), but the relocated
-             store does not.
+                   This happens when the store dir in the
+                   ambient file system has a path (e.g. because
+                   another Nix store there), but the relocated
+                   store does not.
 
-             TODO make the various source accessors doing
-             access control all throw the same type of
-             exception, and make union source accessor
-             catch it, so we don't need to do this hack.
-           */
-          {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)},
-      }))
-    , rootFS(({
-        /* In pure eval mode, we provide a filesystem that only
-           contains the Nix store.
+                   TODO make the various source accessors doing
+                   access control all throw the same type of
+                   exception, and make union source accessor
+                   catch it, so we don't need to do this hack.
+                 */
+                {CanonPath(store->storeDir), store->getFSAccessor(settings.pureEval)},
+            }))
+    , rootFS(
+        ({
+            /* In pure eval mode, we provide a filesystem that only
+               contains the Nix store.
 
-           If we have a chroot store and pure eval is not enabled,
-           use a union accessor to make the chroot store available
-           at its logical location while still having the
-           underlying directory available. This is necessary for
-           instance if we're evaluating a file from the physical
-           /nix/store while using a chroot store. */
-        auto accessor = getFSSourceAccessor();
+               If we have a chroot store and pure eval is not enabled,
+               use a union accessor to make the chroot store available
+               at its logical location while still having the
+               underlying directory available. This is necessary for
+               instance if we're evaluating a file from the physical
+               /nix/store while using a chroot store. */
+            auto accessor = getFSSourceAccessor();
 
-        auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
-        if (settings.pureEval || store->storeDir != realStoreDir) {
-            accessor = settings.pureEval ? storeFS : makeUnionSourceAccessor({accessor, storeFS});
-        }
+            auto realStoreDir = dirOf(store->toRealPath(StorePath::dummy));
+            if (settings.pureEval || store->storeDir != realStoreDir) {
+                accessor = settings.pureEval
+                    ? storeFS
+                    : makeUnionSourceAccessor({accessor, storeFS});
+            }
 
-        /* Apply access control if needed. */
-        if (settings.restrictEval || settings.pureEval)
-            accessor = AllowListSourceAccessor::create(
-                accessor, {}, {}, [&settings](const CanonPath & path) -> RestrictedPathError {
-                    auto modeInformation = settings.pureEval ? "in pure evaluation mode (use '--impure' to override)"
-                                                             : "in restricted mode";
-                    throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
-                });
+            /* Apply access control if needed. */
+            if (settings.restrictEval || settings.pureEval)
+                accessor = AllowListSourceAccessor::create(accessor, {}, {},
+                    [&settings](const CanonPath & path) -> RestrictedPathError {
+                        auto modeInformation = settings.pureEval
+                            ? "in pure evaluation mode (use '--impure' to override)"
+                            : "in restricted mode";
+                        throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation);
+                    });
 
-        accessor;
-    }))
+            accessor;
+        }))
     , corepkgsFS(make_ref<MemorySourceAccessor>())
     , internalFS(make_ref<MemorySourceAccessor>())
     , derivationInternal{corepkgsFS->addFile(
-          CanonPath("derivation-internal.nix"),
+        CanonPath("derivation-internal.nix"),
 #include "primops/derivation.nix.gen.hh"
-          )}
+    )}
     , store(store)
     , buildStore(buildStore ? buildStore : store)
     , inputCache(fetchers::InputCache::create())
@@ -293,10 +351,10 @@ EvalState::EvalState(
     vNull.mkNull();
     vTrue.mkBool(true);
     vFalse.mkBool(false);
-    vStringRegular.mkStringNoCopy("regular");
-    vStringDirectory.mkStringNoCopy("directory");
-    vStringSymlink.mkStringNoCopy("symlink");
-    vStringUnknown.mkStringNoCopy("unknown");
+    vStringRegular.mkString("regular");
+    vStringDirectory.mkString("directory");
+    vStringSymlink.mkString("symlink");
+    vStringUnknown.mkString("unknown");
 
     /* Construct the Nix expression search path. */
     assert(lookupPath.elements.empty());
@@ -596,7 +654,7 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
     }
     if (isFunctor(v)) {
         try {
-            Value & functor = *v.attrs()->find(s.functor)->value;
+            Value & functor = *v.attrs()->find(sFunctor)->value;
             Value * vp[] = {&v};
             Value partiallyApplied;
             // The first parameter is not user-provided, and may be
@@ -825,7 +883,7 @@ DebugTraceStacker::DebugTraceStacker(EvalState & evalState, DebugTrace t)
 
 void Value::mkString(std::string_view s)
 {
-    mkStringNoCopy(makeImmutableString(s));
+    mkString(makeImmutableString(s));
 }
 
 static const char ** encodeContext(const NixStringContext & context)
@@ -844,12 +902,12 @@ static const char ** encodeContext(const NixStringContext & context)
 
 void Value::mkString(std::string_view s, const NixStringContext & context)
 {
-    mkStringNoCopy(makeImmutableString(s), encodeContext(context));
+    mkString(makeImmutableString(s), encodeContext(context));
 }
 
 void Value::mkStringMove(const char * s, const NixStringContext & context)
 {
-    mkStringNoCopy(s, encodeContext(context));
+    mkString(s, encodeContext(context));
 }
 
 void Value::mkPath(const SourcePath & path)
@@ -920,8 +978,8 @@ void EvalState::mkPos(Value & v, PosIdx p)
     auto origin = positions.originOf(p);
     if (auto path = std::get_if<SourcePath>(&origin)) {
         auto attrs = buildBindings(3);
-        attrs.alloc(s.file).mkString(path->path.abs());
-        makePositionThunks(*this, p, attrs.alloc(s.line), attrs.alloc(s.column));
+        attrs.alloc(sFile).mkString(path->path.abs());
+        makePositionThunks(*this, p, attrs.alloc(sLine), attrs.alloc(sColumn));
         v.mkAttrs(attrs);
     } else
         v.mkNull();
@@ -1187,7 +1245,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         dynamicEnv = &env2;
         Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
 
-        AttrDefs::iterator overrides = attrs.find(state.s.overrides);
+        AttrDefs::iterator overrides = attrs.find(state.sOverrides);
         bool hasOverrides = overrides != attrs.end();
 
         /* The recursive attributes are evaluated in the new
@@ -1219,7 +1277,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
                 *vOverrides,
                 [&]() { return vOverrides->determinePos(noPos); },
                 "while evaluating the `__overrides` attribute");
-            bindings.grow(state.buildBindings(bindings.capacity() + vOverrides->attrs()->size()));
+            bindings.grow(state.allocBindings(bindings.capacity() + vOverrides->attrs()->size()));
             for (auto & i : *vOverrides->attrs()) {
                 AttrDefs::iterator j = attrs.find(i.name);
                 if (j != attrs.end()) {
@@ -1659,7 +1717,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             }
         }
 
-        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(s.functor))) {
+        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(sFunctor))) {
             /* 'vCur' may be allocated on the stack of the calling
                function, but for functors we may keep a reference, so
                heap-allocate a copy and use that instead. */
@@ -1721,7 +1779,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
     forceValue(fun, pos);
 
     if (fun.type() == nAttrs) {
-        auto found = fun.attrs()->find(s.functor);
+        auto found = fun.attrs()->find(sFunctor);
         if (found != fun.attrs()->end()) {
             Value * v = allocValue();
             callFunction(*found->value, fun, *v, pos);
@@ -2064,54 +2122,6 @@ void ExprBlackHole::eval(EvalState & state, [[maybe_unused]] Env & env, Value &
 // always force this to be separate, otherwise forceValue may inline it and take
 // a massive perf hit
 [[gnu::noinline]]
-void EvalState::handleEvalExceptionForThunk(Env * env, Expr * expr, Value & v, const PosIdx pos)
-{
-    v.mkThunk(env, expr);
-    tryFixupBlackHolePos(v, pos);
-
-    auto e = std::current_exception();
-    Value * recovery = nullptr;
-    try {
-        std::rethrow_exception(e);
-    } catch (const RecoverableEvalError & e) {
-        recovery = allocValue();
-    } catch (...) {
-    }
-    if (recovery) {
-        recovery->mkThunk(env, expr);
-    }
-    v.mkFailed(e, recovery);
-}
-
-[[gnu::noinline]]
-void EvalState::handleEvalExceptionForApp(Value & v)
-{
-    auto e = std::current_exception();
-    Value * recovery = nullptr;
-    try {
-        std::rethrow_exception(e);
-    } catch (const RecoverableEvalError & e) {
-        recovery = allocValue();
-    } catch (...) {
-    }
-    if (recovery) {
-        *recovery = v;
-    }
-    v.mkFailed(e, recovery);
-}
-
-[[gnu::noinline]]
-void EvalState::handleEvalFailed(Value & v, const PosIdx pos)
-{
-    assert(v.isFailed());
-    if (auto recoveryValue = v.failed()->recoveryValue) {
-        v = *recoveryValue;
-        forceValue(v, pos);
-    } else {
-        std::rethrow_exception(v.failed()->ex);
-    }
-}
-
 void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos)
 {
     if (!v.isBlackhole())
@@ -2120,8 +2130,7 @@ void EvalState::tryFixupBlackHolePos(Value & v, PosIdx pos)
     try {
         std::rethrow_exception(e);
     } catch (InfiniteRecursionError & e) {
-        if (!e.hasPos())
-            e.atPos(positions[pos]);
+        e.atPos(positions[pos]);
     } catch (...) {
     }
 }
@@ -2232,7 +2241,7 @@ Bindings::const_iterator EvalState::getAttr(Symbol attrSym, const Bindings * att
 
 bool EvalState::isFunctor(const Value & fun) const
 {
-    return fun.type() == nAttrs && fun.attrs()->find(s.functor) != fun.attrs()->end();
+    return fun.type() == nAttrs && fun.attrs()->find(sFunctor) != fun.attrs()->end();
 }
 
 void EvalState::forceFunction(Value & v, const PosIdx pos, std::string_view errorCtx)
@@ -2301,7 +2310,7 @@ bool EvalState::isDerivation(Value & v)
 {
     if (v.type() != nAttrs)
         return false;
-    auto i = v.attrs()->get(s.type);
+    auto i = v.attrs()->get(sType);
     if (!i)
         return false;
     forceValue(*i->value, i->pos);
@@ -2313,7 +2322,7 @@ bool EvalState::isDerivation(Value & v)
 std::optional<std::string>
 EvalState::tryAttrsToString(const PosIdx pos, Value & v, NixStringContext & context, bool coerceMore, bool copyToStore)
 {
-    auto i = v.attrs()->find(s.toString);
+    auto i = v.attrs()->find(sToString);
     if (i != v.attrs()->end()) {
         Value v1;
         callFunction(*i->value, v, v1, pos);
@@ -2359,7 +2368,7 @@ BackedStringView EvalState::coerceToString(
         auto maybeString = tryAttrsToString(pos, v, context, coerceMore, copyToStore);
         if (maybeString)
             return std::move(*maybeString);
-        auto i = v.attrs()->find(s.outPath);
+        auto i = v.attrs()->find(sOutPath);
         if (i == v.attrs()->end()) {
             error<TypeError>(
                 "cannot coerce %1% to a string: %2%", showType(v), ValuePrinter(*this, v, errorPrintOptions))
@@ -2466,7 +2475,7 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext
     /* Similarly, handle __toString where the result may be a path
        value. */
     if (v.type() == nAttrs) {
-        auto i = v.attrs()->find(s.toString);
+        auto i = v.attrs()->find(sToString);
         if (i != v.attrs()->end()) {
             Value v1;
             callFunction(*i->value, v, v1, pos);
@@ -2656,8 +2665,8 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
 
     case nAttrs: {
         if (isDerivation(v1) && isDerivation(v2)) {
-            auto i = v1.attrs()->get(s.outPath);
-            auto j = v2.attrs()->get(s.outPath);
+            auto i = v1.attrs()->get(sOutPath);
+            auto j = v2.attrs()->get(sOutPath);
             if (i && j) {
                 try {
                     assertEqValues(*i->value, *j->value, pos, errorCtx);
@@ -2746,11 +2755,8 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
         }
         return;
 
-    // Cannot be returned by forceValue().
-    case nThunk:
-    case nFailed:
-        unreachable();
-
+    case nThunk: // Must not be left by forceValue
+        assert(false);
     default: // Note that we pass compiler flags that should make `default:` unreachable.
         // Also note that this probably ran after `eqValues`, which implements
         // the same logic more efficiently (without having to unwind stacks),
@@ -2813,8 +2819,8 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         /* If both sets denote a derivation (type = "derivation"),
            then compare their outPaths. */
         if (isDerivation(v1) && isDerivation(v2)) {
-            auto i = v1.attrs()->get(s.outPath);
-            auto j = v2.attrs()->get(s.outPath);
+            auto i = v1.attrs()->get(sOutPath);
+            auto j = v2.attrs()->get(sOutPath);
             if (i && j)
                 return eqValues(*i->value, *j->value, pos, errorCtx);
         }
@@ -2842,11 +2848,8 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         // !!!
         return v1.fpoint() == v2.fpoint();
 
-    // Cannot be returned by forceValue().
-    case nThunk:
-    case nFailed:
-        unreachable();
-
+    case nThunk: // Must not be left by forceValue
+        assert(false);
     default: // Note that we pass compiler flags that should make `default:` unreachable.
         error<EvalError>("eqValues: cannot compare %1% with %2%", showType(v1), showType(v2))
             .withTrace(pos, errorCtx)
@@ -2886,8 +2889,11 @@ void EvalState::maybePrintStats()
 
 void EvalState::printStatistics()
 {
-    std::chrono::microseconds cpuTimeDuration = getCpuUserTime();
-    float cpuTime = std::chrono::duration_cast<std::chrono::duration<float>>(cpuTimeDuration).count();
+#ifndef _WIN32 // TODO use portable implementation
+    struct rusage buf;
+    getrusage(RUSAGE_SELF, &buf);
+    float cpuTime = buf.ru_utime.tv_sec + ((float) buf.ru_utime.tv_usec / 1000000);
+#endif
 
     uint64_t bEnvs = nrEnvs * sizeof(Env) + nrValuesInEnvs * sizeof(Value *);
     uint64_t bLists = nrListElems * sizeof(Value *);
@@ -2909,12 +2915,18 @@ void EvalState::printStatistics()
     if (outPath != "-")
         fs.open(outPath, std::fstream::out);
     json topObj = json::object();
+#ifndef _WIN32 // TODO implement
     topObj["cpuTime"] = cpuTime;
+#endif
     topObj["time"] = {
+#ifndef _WIN32 // TODO implement
         {"cpu", cpuTime},
+#endif
 #if NIX_USE_BOEHMGC
         {GC_is_incremental_mode() ? "gcNonIncremental" : "gc", gcFullOnlyTime},
+#  ifndef _WIN32 // TODO implement
         {GC_is_incremental_mode() ? "gcNonIncrementalFraction" : "gcFraction", gcFullOnlyTime / cpuTime},
+#  endif
 #endif
     };
     topObj["envs"] = {
@@ -3184,7 +3196,8 @@ Expr * EvalState::parse(
         docComments = &it->second;
     }
 
-    auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS);
+    auto result = parseExprFromBuf(
+        text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS, exprSymbols);
 
     result->bindVars(*this, staticEnv);
 

src/libexpr/get-drvs.cc

@@ -45,7 +45,7 @@ PackageInfo::PackageInfo(EvalState & state, ref<Store> store, const std::string
 std::string PackageInfo::queryName() const
 {
     if (name == "" && attrs) {
-        auto i = attrs->find(state->s.name);
+        auto i = attrs->find(state->sName);
         if (i == attrs->end())
             state->error<TypeError>("derivation name missing").debugThrow();
         name = state->forceStringNoCtx(*i->value, noPos, "while evaluating the 'name' attribute of a derivation");
@@ -56,7 +56,7 @@ std::string PackageInfo::queryName() const
 std::string PackageInfo::querySystem() const
 {
     if (system == "" && attrs) {
-        auto i = attrs->find(state->s.system);
+        auto i = attrs->find(state->sSystem);
         system =
             i == attrs->end()
                 ? "unknown"
@@ -68,7 +68,7 @@ std::string PackageInfo::querySystem() const
 std::optional<StorePath> PackageInfo::queryDrvPath() const
 {
     if (!drvPath && attrs) {
-        if (auto i = attrs->get(state->s.drvPath)) {
+        if (auto i = attrs->get(state->sDrvPath)) {
             NixStringContext context;
             auto found = state->coerceToStorePath(
                 i->pos, *i->value, context, "while evaluating the 'drvPath' attribute of a derivation");
@@ -95,7 +95,7 @@ StorePath PackageInfo::requireDrvPath() const
 StorePath PackageInfo::queryOutPath() const
 {
     if (!outPath && attrs) {
-        auto i = attrs->find(state->s.outPath);
+        auto i = attrs->find(state->sOutPath);
         NixStringContext context;
         if (i != attrs->end())
             outPath = state->coerceToStorePath(
@@ -111,7 +111,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
     if (outputs.empty()) {
         /* Get the ‘outputs’ list. */
         const Attr * i;
-        if (attrs && (i = attrs->get(state->s.outputs))) {
+        if (attrs && (i = attrs->get(state->sOutputs))) {
             state->forceList(*i->value, i->pos, "while evaluating the 'outputs' attribute of a derivation");
 
             /* For each output... */
@@ -127,7 +127,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
                     state->forceAttrs(*out->value, i->pos, "while evaluating an output of a derivation");
 
                     /* And evaluate its ‘outPath’ attribute. */
-                    auto outPath = out->value->attrs()->get(state->s.outPath);
+                    auto outPath = out->value->attrs()->get(state->sOutPath);
                     if (!outPath)
                         continue; // FIXME: throw error?
                     NixStringContext context;
@@ -146,7 +146,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
         return outputs;
 
     const Attr * i;
-    if (attrs && (i = attrs->get(state->s.outputSpecified))
+    if (attrs && (i = attrs->get(state->sOutputSpecified))
         && state->forceBool(*i->value, i->pos, "while evaluating the 'outputSpecified' attribute of a derivation")) {
         Outputs result;
         auto out = outputs.find(queryOutputName());
@@ -181,7 +181,7 @@ PackageInfo::Outputs PackageInfo::queryOutputs(bool withPaths, bool onlyOutputsT
 std::string PackageInfo::queryOutputName() const
 {
     if (outputName == "" && attrs) {
-        auto i = attrs->get(state->s.outputName);
+        auto i = attrs->get(state->sOutputName);
         outputName =
             i ? state->forceStringNoCtx(*i->value, noPos, "while evaluating the output name of a derivation") : "";
     }
@@ -194,7 +194,7 @@ const Bindings * PackageInfo::getMeta()
         return meta;
     if (!attrs)
         return 0;
-    auto a = attrs->get(state->s.meta);
+    auto a = attrs->get(state->sMeta);
     if (!a)
         return 0;
     state->forceAttrs(*a->value, a->pos, "while evaluating the 'meta' attribute of a derivation");
@@ -221,7 +221,7 @@ bool PackageInfo::checkMeta(Value & v)
                 return false;
         return true;
     } else if (v.type() == nAttrs) {
-        if (v.attrs()->get(state->s.outPath))
+        if (v.attrs()->get(state->sOutPath))
             return false;
         for (auto & i : *v.attrs())
             if (!checkMeta(*i.value))
@@ -411,7 +411,7 @@ static void getDerivations(
                     should we recurse into it?  => Only if it has a
                     `recurseForDerivations = true' attribute. */
                     if (i->value->type() == nAttrs) {
-                        auto j = i->value->attrs()->get(state.s.recurseForDerivations);
+                        auto j = i->value->attrs()->get(state.sRecurseForDerivations);
                         if (j
                             && state.forceBool(
                                 *j->value, j->pos, "while evaluating the attribute `recurseForDerivations`"))

src/libexpr/include/nix/expr/attr-set.hh

@@ -5,7 +5,6 @@
 #include "nix/expr/symbol-table.hh"
 
 #include <algorithm>
-#include <functional>
 
 namespace nix {
 
@@ -55,14 +54,16 @@ public:
     PosIdx pos;
 
 private:
-    size_t size_ = 0;
+    size_t size_, capacity_;
     Attr attrs[0];
 
-    Bindings() = default;
-    Bindings(const Bindings &) = delete;
-    Bindings(Bindings &&) = delete;
-    Bindings & operator=(const Bindings &) = delete;
-    Bindings & operator=(Bindings &&) = delete;
+    Bindings(size_t capacity)
+        : size_(0)
+        , capacity_(capacity)
+    {
+    }
+
+    Bindings(const Bindings & bindings) = delete;
 
 public:
     size_t size() const
@@ -81,6 +82,7 @@ public:
 
     void push_back(const Attr & attr)
     {
+        assert(size_ < capacity_);
         attrs[size_++] = attr;
     }
 
@@ -134,6 +136,11 @@ public:
 
     void sort();
 
+    size_t capacity() const
+    {
+        return capacity_;
+    }
+
     /**
      * Returns the attributes in lexicographically sorted order.
      */
@@ -158,29 +165,22 @@ public:
  * order at the end. The only way to consume a BindingsBuilder is to
  * call finish(), which sorts the bindings.
  */
-class BindingsBuilder final
+class BindingsBuilder
 {
+    Bindings * bindings;
+
 public:
     // needed by std::back_inserter
     using value_type = Attr;
-    using size_type = Bindings::size_t;
 
-private:
-    Bindings * bindings;
-    Bindings::size_t capacity_;
+    EvalState & state;
 
-    friend class EvalState;
-
-    BindingsBuilder(EvalState & state, Bindings * bindings, size_type capacity)
+    BindingsBuilder(EvalState & state, Bindings * bindings)
         : bindings(bindings)
-        , capacity_(capacity)
         , state(state)
     {
     }
 
-public:
-    std::reference_wrapper<EvalState> state;
-
     void insert(Symbol name, Value * value, PosIdx pos = noPos)
     {
         insert(Attr(name, value, pos));
@@ -193,7 +193,6 @@ public:
 
     void push_back(const Attr & attr)
     {
-        assert(bindings->size() < capacity_);
         bindings->push_back(attr);
     }
 
@@ -212,16 +211,16 @@ public:
         return bindings;
     }
 
-    size_t capacity() const noexcept
+    size_t capacity()
     {
-        return capacity_;
+        return bindings->capacity();
     }
 
-    void grow(BindingsBuilder newBindings)
+    void grow(Bindings * newBindings)
     {
         for (auto & i : *bindings)
-            newBindings.push_back(i);
-        std::swap(*this, newBindings);
+            newBindings->push_back(i);
+        bindings = newBindings;
     }
 
     friend struct ExprAttrs;

src/libexpr/include/nix/expr/eval-error.hh

@@ -56,14 +56,6 @@ MakeError(MissingArgumentError, EvalError);
 MakeError(InfiniteRecursionError, EvalError);
 MakeError(IFDError, EvalBaseError);
 
-/**
- * An evaluation error which should be retried instead of rethrown
- *
- * A RecoverableEvalError is not an EvalError, because we shouldn't cache it in the eval cache, as it should be retried
- * anyway.
- */
-MakeError(RecoverableEvalError, EvalBaseError);
-
 struct InvalidPathError : public EvalError
 {
 public:

src/libexpr/include/nix/expr/eval-inline.hh

@@ -5,7 +5,6 @@
 #include "nix/expr/eval.hh"
 #include "nix/expr/eval-error.hh"
 #include "nix/expr/eval-settings.hh"
-#include <exception>
 
 namespace nix {
 
@@ -98,19 +97,12 @@ void EvalState::forceValue(Value & v, const PosIdx pos)
             else
                 ExprBlackHole::throwInfiniteRecursionError(*this, v);
         } catch (...) {
-            handleEvalExceptionForThunk(env, expr, v, pos);
+            v.mkThunk(env, expr);
+            tryFixupBlackHolePos(v, pos);
             throw;
         }
-    } else if (v.isApp()) {
-        try {
-            callFunction(*v.app().left, *v.app().right, v, pos);
-        } catch (...) {
-            handleEvalExceptionForApp(v);
-            throw;
-        }
-    } else if (v.isFailed()) {
-        handleEvalFailed(v, pos);
-    }
+    } else if (v.isApp())
+        callFunction(*v.app().left, *v.app().right, v, pos);
 }
 
 [[gnu::always_inline]]

src/libexpr/include/nix/expr/eval.hh

@@ -213,100 +213,23 @@ struct DebugTrace
     }
 };
 
-struct StaticEvalSymbols
-{
-    Symbol with, outPath, drvPath, type, meta, name, value, system, overrides, outputs, outputName, ignoreNulls, file,
-        line, column, functor, toString, right, wrong, structuredAttrs, json, allowedReferences, allowedRequisites,
-        disallowedReferences, disallowedRequisites, maxSize, maxClosureSize, builder, args, contentAddressed, impure,
-        outputHash, outputHashAlgo, outputHashMode, recurseForDerivations, description, self, epsilon, startSet,
-        operator_, key, path, prefix, outputSpecified;
-
-    Expr::AstSymbols exprSymbols;
-
-    static constexpr auto preallocate()
-    {
-        StaticSymbolTable alloc;
-
-        StaticEvalSymbols staticSymbols = {
-            .with = alloc.create("<with>"),
-            .outPath = alloc.create("outPath"),
-            .drvPath = alloc.create("drvPath"),
-            .type = alloc.create("type"),
-            .meta = alloc.create("meta"),
-            .name = alloc.create("name"),
-            .value = alloc.create("value"),
-            .system = alloc.create("system"),
-            .overrides = alloc.create("__overrides"),
-            .outputs = alloc.create("outputs"),
-            .outputName = alloc.create("outputName"),
-            .ignoreNulls = alloc.create("__ignoreNulls"),
-            .file = alloc.create("file"),
-            .line = alloc.create("line"),
-            .column = alloc.create("column"),
-            .functor = alloc.create("__functor"),
-            .toString = alloc.create("__toString"),
-            .right = alloc.create("right"),
-            .wrong = alloc.create("wrong"),
-            .structuredAttrs = alloc.create("__structuredAttrs"),
-            .json = alloc.create("__json"),
-            .allowedReferences = alloc.create("allowedReferences"),
-            .allowedRequisites = alloc.create("allowedRequisites"),
-            .disallowedReferences = alloc.create("disallowedReferences"),
-            .disallowedRequisites = alloc.create("disallowedRequisites"),
-            .maxSize = alloc.create("maxSize"),
-            .maxClosureSize = alloc.create("maxClosureSize"),
-            .builder = alloc.create("builder"),
-            .args = alloc.create("args"),
-            .contentAddressed = alloc.create("__contentAddressed"),
-            .impure = alloc.create("__impure"),
-            .outputHash = alloc.create("outputHash"),
-            .outputHashAlgo = alloc.create("outputHashAlgo"),
-            .outputHashMode = alloc.create("outputHashMode"),
-            .recurseForDerivations = alloc.create("recurseForDerivations"),
-            .description = alloc.create("description"),
-            .self = alloc.create("self"),
-            .epsilon = alloc.create(""),
-            .startSet = alloc.create("startSet"),
-            .operator_ = alloc.create("operator"),
-            .key = alloc.create("key"),
-            .path = alloc.create("path"),
-            .prefix = alloc.create("prefix"),
-            .outputSpecified = alloc.create("outputSpecified"),
-            .exprSymbols = {
-                .sub = alloc.create("__sub"),
-                .lessThan = alloc.create("__lessThan"),
-                .mul = alloc.create("__mul"),
-                .div = alloc.create("__div"),
-                .or_ = alloc.create("or"),
-                .findFile = alloc.create("__findFile"),
-                .nixPath = alloc.create("__nixPath"),
-                .body = alloc.create("body"),
-            }};
-
-        return std::pair{staticSymbols, alloc};
-    }
-
-    static consteval StaticEvalSymbols create()
-    {
-        return preallocate().first;
-    }
-
-    static constexpr StaticSymbolTable staticSymbolTable()
-    {
-        return preallocate().second;
-    }
-};
-
 class EvalState : public std::enable_shared_from_this<EvalState>
 {
 public:
-    static constexpr StaticEvalSymbols s = StaticEvalSymbols::create();
-
     const fetchers::Settings & fetchSettings;
     const EvalSettings & settings;
     SymbolTable symbols;
     PosTable positions;
 
+    const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue, sSystem, sOverrides, sOutputs, sOutputName,
+        sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, sRight, sWrong, sStructuredAttrs, sJson,
+        sAllowedReferences, sAllowedRequisites, sDisallowedReferences, sDisallowedRequisites, sMaxSize, sMaxClosureSize,
+        sBuilder, sArgs, sContentAddressed, sImpure, sOutputHash, sOutputHashAlgo, sOutputHashMode,
+        sRecurseForDerivations, sDescription, sSelf, sEpsilon, sStartSet, sOperator, sKey, sPath, sPrefix,
+        sOutputSpecified;
+
+    const Expr::AstSymbols exprSymbols;
+
     /**
      * If set, force copying files to the Nix store even if they
      * already exist there.
@@ -610,28 +533,8 @@ public:
      */
     inline void forceValue(Value & v, const PosIdx pos);
 
-private:
-
-    /**
-     * Internal support function for forceValue
-     *
-     * This code is factored out so that it's not in the heavily inlined hot path.
-     */
-    void handleEvalExceptionForThunk(Env * env, Expr * expr, Value & v, const PosIdx pos);
-
-    /**
-     * Internal support function for forceValue
-     *
-     * This code is factored out so that it's not in the heavily inlined hot path.
-     */
-    void handleEvalExceptionForApp(Value & v);
-
-    void handleEvalFailed(Value & v, PosIdx pos);
-
     void tryFixupBlackHolePos(Value & v, PosIdx pos);
 
-public:
-
     /**
      * Force a value, then recursively force list elements and
      * attributes.
@@ -899,7 +802,7 @@ public:
 
     BindingsBuilder buildBindings(size_t capacity)
     {
-        return BindingsBuilder(*this, allocBindings(capacity), capacity);
+        return BindingsBuilder(*this, allocBindings(capacity));
     }
 
     ListBuilder buildList(size_t size)

src/libexpr/include/nix/expr/nixexpr.hh

@@ -158,7 +158,7 @@ struct ExprString : Expr
     ExprString(std::string && s)
         : s(std::move(s))
     {
-        v.mkStringNoCopy(this->s.data());
+        v.mkString(this->s.data());
     };
 
     Value * maybeThunk(EvalState & state, Env & env) override;
@@ -595,17 +595,12 @@ struct ExprOpNot : Expr
         {                                                                                    \
             return pos;                                                                      \
         }                                                                                    \
-    }
+    };
 
-MakeBinOp(ExprOpEq, "==");
-MakeBinOp(ExprOpNEq, "!=");
-MakeBinOp(ExprOpAnd, "&&");
-MakeBinOp(ExprOpOr, "||");
-MakeBinOp(ExprOpImpl, "->");
-MakeBinOp(ExprOpUpdate, "//");
-MakeBinOp(ExprOpConcatLists, "++");
+MakeBinOp(ExprOpEq, "==") MakeBinOp(ExprOpNEq, "!=") MakeBinOp(ExprOpAnd, "&&") MakeBinOp(ExprOpOr, "||")
+    MakeBinOp(ExprOpImpl, "->") MakeBinOp(ExprOpUpdate, "//") MakeBinOp(ExprOpConcatLists, "++")
 
-struct ExprConcatStrings : Expr
+        struct ExprConcatStrings : Expr
 {
     PosIdx pos;
     bool forceString;

src/libexpr/include/nix/expr/parser-state.hh

@@ -88,7 +88,7 @@ struct ParserState
     SourcePath basePath;
     PosTable::Origin origin;
     const ref<SourceAccessor> rootFS;
-    static constexpr Expr::AstSymbols s = StaticEvalSymbols::create().exprSymbols;
+    const Expr::AstSymbols & s;
     const EvalSettings & settings;
 
     void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos);

src/libexpr/include/nix/expr/symbol-table.hh

@@ -28,8 +28,6 @@ public:
     }
 };
 
-class StaticSymbolTable;
-
 /**
  * Symbols have the property that they can be compared efficiently
  * (using an equality test), because the symbol table stores only one
@@ -39,38 +37,36 @@ class Symbol
 {
     friend class SymbolStr;
     friend class SymbolTable;
-    friend class StaticSymbolTable;
 
 private:
     uint32_t id;
 
-    explicit constexpr Symbol(uint32_t id) noexcept
+    explicit Symbol(uint32_t id) noexcept
         : id(id)
     {
     }
 
 public:
-    constexpr Symbol() noexcept
+    Symbol() noexcept
         : id(0)
     {
     }
 
     [[gnu::always_inline]]
-    constexpr explicit operator bool() const noexcept
+    explicit operator bool() const noexcept
     {
         return id > 0;
     }
 
-    /**
-     * The ID is a private implementation detail that should generally not be observed. However, we expose here just for
-     * sake of `switch...case`, which needs to dispatch on numbers. */
-    [[gnu::always_inline]]
-    constexpr uint32_t getId() const noexcept
+    auto operator<=>(const Symbol other) const noexcept
     {
-        return id;
+        return id <=> other.id;
     }
 
-    constexpr auto operator<=>(const Symbol & other) const noexcept = default;
+    bool operator==(const Symbol other) const noexcept
+    {
+        return id == other.id;
+    }
 
     friend class std::hash<Symbol>;
 };
@@ -122,12 +118,12 @@ public:
         // for multi-threaded implementations: lock store and allocator here
         const auto & [v, idx] = key.store.add(SymbolValue{});
         if (size == 0) {
-            v.mkStringNoCopy("", nullptr);
+            v.mkString("", nullptr);
         } else {
             auto s = key.alloc.allocate(size + 1);
             memcpy(s, key.s.data(), size);
             s[size] = '\0';
-            v.mkStringNoCopy(s, nullptr);
+            v.mkString(s, nullptr);
         }
         v.size_ = size;
         v.idx = idx;
@@ -214,39 +210,6 @@ public:
     };
 };
 
-class SymbolTable;
-
-/**
- * Convenience class to statically assign symbol identifiers at compile-time.
- */
-class StaticSymbolTable
-{
-    static constexpr std::size_t maxSize = 1024;
-
-    struct StaticSymbolInfo
-    {
-        std::string_view str;
-        Symbol sym;
-    };
-
-    std::array<StaticSymbolInfo, maxSize> symbols;
-    std::size_t size = 0;
-
-public:
-    constexpr StaticSymbolTable() = default;
-
-    constexpr Symbol create(std::string_view str)
-    {
-        /* No need to check bounds because out of bounds access is
-           a compilation error. */
-        auto sym = Symbol(size + 1); //< +1 because Symbol with id = 0 is reserved
-        symbols[size++] = {str, sym};
-        return sym;
-    }
-
-    void copyIntoSymbolTable(SymbolTable & symtab) const;
-};
-
 /**
  * Symbol table used by the parser and evaluator to represent and look
  * up identifiers and attributes efficiently.
@@ -269,10 +232,6 @@ private:
     boost::unordered_flat_set<SymbolStr, SymbolStr::Hash, SymbolStr::Equal> symbols{SymbolStr::chunkSize};
 
 public:
-    SymbolTable(const StaticSymbolTable & staticSymtab)
-    {
-        staticSymtab.copyIntoSymbolTable(*this);
-    }
 
     /**
      * Converts a string into a symbol.
@@ -317,16 +276,6 @@ public:
     }
 };
 
-inline void StaticSymbolTable::copyIntoSymbolTable(SymbolTable & symtab) const
-{
-    for (std::size_t i = 0; i < size; ++i) {
-        auto [str, staticSym] = symbols[i];
-        auto sym = symtab.create(str);
-        if (sym != staticSym) [[unlikely]]
-            unreachable();
-    }
-}
-
 } // namespace nix
 
 template<>

src/libexpr/include/nix/expr/value.hh

@@ -2,7 +2,6 @@
 ///@file
 
 #include <cassert>
-#include <exception>
 #include <span>
 #include <type_traits>
 #include <concepts>
@@ -36,7 +35,6 @@ typedef enum {
     tBool,
     tNull,
     tFloat,
-    tFailed,
     tExternal,
     tPrimOp,
     tAttrs,
@@ -59,7 +57,6 @@ typedef enum {
  */
 typedef enum {
     nThunk,
-    nFailed,
     nInt,
     nFloat,
     nBool,
@@ -268,30 +265,6 @@ struct ValueBase
         size_t size;
         Value * const * elems;
     };
-
-    /**
-      Representation of an evaluation that previously failed.
-
-      `Value` references `Failed` by packed pointer, and its `new` is GC managed.
-
-      @see gc_cleanup std::exception_ptr
-     */
-    class Failed : public gc_cleanup
-    {
-    public:
-        std::exception_ptr ex;
-        /**
-         * Optional value for recovering `RecoverableEvalError`
-         * Must be set iff `ex` is an instance of `RecoverableEvalError`.
-         */
-        Value * recoveryValue;
-
-        Failed(std::exception_ptr ex, Value * recoveryValue)
-            : ex(ex)
-            , recoveryValue(recoveryValue)
-        {
-        }
-    };
 };
 
 template<typename T>
@@ -318,7 +291,6 @@ struct PayloadTypeToInternalType
     MACRO(PrimOp *, primOp, tPrimOp)                                \
     MACRO(ValueBase::PrimOpApplicationThunk, primOpApp, tPrimOpApp) \
     MACRO(ExternalValueBase *, external, tExternal)                 \
-    MACRO(ValueBase::Failed *, failed, tFailed)                     \
     MACRO(NixFloat, fpoint, tFloat)
 
 #define NIX_VALUE_PAYLOAD_TYPE(T, FIELD_NAME, DISCRIMINATOR) \
@@ -397,7 +369,7 @@ namespace detail {
 /* Whether to use a specialization of ValueStorage that does bitpacking into
    alignment niches. */
 template<std::size_t ptrSize>
-inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 16);
+inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEFAULT_NEW_ALIGNMENT__ >= 8);
 
 } // namespace detail
 
@@ -406,8 +378,7 @@ inline constexpr bool useBitPackedValueStorage = (ptrSize == 8) && (__STDCPP_DEF
  * Packs discriminator bits into the pointer alignment niches.
  */
 template<std::size_t ptrSize>
-class alignas(16) ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>>
-    : public detail::ValueBase
+class ValueStorage<ptrSize, std::enable_if_t<detail::useBitPackedValueStorage<ptrSize>>> : public detail::ValueBase
 {
     /* Needs a dependent type name in order for member functions (and
      * potentially ill-formed bit casts) to be SFINAE'd out.
@@ -623,11 +594,6 @@ protected:
         path.path = std::bit_cast<const char *>(payload[1]);
     }
 
-    void getStorage(Failed *& failed) const noexcept
-    {
-        failed = std::bit_cast<Failed *>(payload[1]);
-    }
-
     void setStorage(NixInt integer) noexcept
     {
         setSingleDWordPayload<tInt>(integer.value);
@@ -677,11 +643,6 @@ protected:
     {
         setUntaggablePayload<pdPath>(path.accessor, path.path);
     }
-
-    void setStorage(Failed * failed) noexcept
-    {
-        setSingleDWordPayload<tFailed>(std::bit_cast<PackedPointer>(failed));
-    }
 };
 
 /**
@@ -904,12 +865,12 @@ public:
     inline bool isThunk() const
     {
         return isa<tThunk>();
-    }
+    };
 
     inline bool isApp() const
     {
         return isa<tApp>();
-    }
+    };
 
     inline bool isBlackhole() const;
 
@@ -917,22 +878,17 @@ public:
     inline bool isLambda() const
     {
         return isa<tLambda>();
-    }
+    };
 
     inline bool isPrimOp() const
     {
         return isa<tPrimOp>();
-    }
+    };
 
     inline bool isPrimOpApp() const
     {
         return isa<tPrimOpApp>();
-    }
-
-    inline bool isFailed() const
-    {
-        return isa<tFailed>();
-    }
+    };
 
     /**
      * Returns the normal type of a Value. This only returns nThunk if
@@ -969,8 +925,6 @@ public:
             return nExternal;
         case tFloat:
             return nFloat;
-        case tFailed:
-            return nFailed;
         case tThunk:
         case tApp:
             return nThunk;
@@ -1006,7 +960,7 @@ public:
         setStorage(b);
     }
 
-    void mkStringNoCopy(const char * s, const char ** context = 0) noexcept
+    inline void mkString(const char * s, const char ** context = 0) noexcept
     {
         setStorage(StringWithContext{.c_str = s, .context = context});
     }
@@ -1018,6 +972,7 @@ public:
     void mkStringMove(const char * s, const NixStringContext & context);
 
     void mkPath(const SourcePath & path);
+    void mkPath(std::string_view path);
 
     inline void mkPath(SourceAccessor * accessor, const char * path) noexcept
     {
@@ -1038,20 +993,12 @@ public:
 
     void mkList(const ListBuilder & builder) noexcept
     {
-        switch (builder.size) {
-        case 0:
-            setStorage(List{.size = 0, .elems = nullptr});
-            break;
-        case 1:
+        if (builder.size == 1)
             setStorage(std::array<Value *, 2>{builder.inlineElems[0], nullptr});
-            break;
-        case 2:
+        else if (builder.size == 2)
             setStorage(std::array<Value *, 2>{builder.inlineElems[0], builder.inlineElems[1]});
-            break;
-        default:
+        else
             setStorage(List{.size = builder.size, .elems = builder.elems});
-            break;
-        }
     }
 
     inline void mkThunk(Env * e, Expr * ex) noexcept
@@ -1093,11 +1040,6 @@ public:
         setStorage(n);
     }
 
-    inline void mkFailed(std::exception_ptr e, Value * recovery) noexcept
-    {
-        setStorage(new Value::Failed(e, recovery));
-    }
-
     bool isList() const noexcept
     {
         return isa<tListSmall, tListN>();
@@ -1201,11 +1143,6 @@ public:
     {
         return getStorage<Path>().accessor;
     }
-
-    Failed * failed() const noexcept
-    {
-        return getStorage<Failed *>();
-    }
 };
 
 extern ExprBlackHole eBlackHole;

src/libexpr/parser.y

@@ -68,7 +68,8 @@ Expr * parseExprFromBuf(
     const EvalSettings & settings,
     PosTable & positions,
     DocCommentMap & docComments,
-    const ref<SourceAccessor> rootFS);
+    const ref<SourceAccessor> rootFS,
+    const Expr::AstSymbols & astSymbols);
 
 }
 
@@ -541,7 +542,8 @@ Expr * parseExprFromBuf(
     const EvalSettings & settings,
     PosTable & positions,
     DocCommentMap & docComments,
-    const ref<SourceAccessor> rootFS)
+    const ref<SourceAccessor> rootFS,
+    const Expr::AstSymbols & astSymbols)
 {
     yyscan_t scanner;
     LexerState lexerState {
@@ -556,6 +558,7 @@ Expr * parseExprFromBuf(
         .basePath = basePath,
         .origin = lexerState.origin,
         .rootFS = rootFS,
+        .s = astSymbols,
         .settings = settings,
     };
 

src/libexpr/primops.cc

@@ -214,20 +214,20 @@ void derivationToValue(
     auto path2 = path.path.abs();
     Derivation drv = state.store->readDerivation(storePath);
     auto attrs = state.buildBindings(3 + drv.outputs.size());
-    attrs.alloc(state.s.drvPath)
+    attrs.alloc(state.sDrvPath)
         .mkString(
             path2,
             {
                 NixStringContextElem::DrvDeep{.drvPath = storePath},
             });
-    attrs.alloc(state.s.name).mkString(drv.env["name"]);
+    attrs.alloc(state.sName).mkString(drv.env["name"]);
 
     auto list = state.buildList(drv.outputs.size());
     for (const auto & [i, o] : enumerate(drv.outputs)) {
         mkOutputString(state, attrs, storePath, o);
         (list[i] = state.allocValue())->mkString(o.first);
     }
-    attrs.alloc(state.s.outputs).mkList(list);
+    attrs.alloc(state.sOutputs).mkList(list);
 
     auto w = state.allocValue();
     w->mkAttrs(attrs);
@@ -483,41 +483,42 @@ void prim_exec(EvalState & state, const PosIdx pos, Value ** args, Value & v)
 static void prim_typeOf(EvalState & state, const PosIdx pos, Value ** args, Value & v)
 {
     state.forceValue(*args[0], pos);
+    std::string t;
     switch (args[0]->type()) {
     case nInt:
-        v.mkStringNoCopy("int");
+        t = "int";
         break;
     case nBool:
-        v.mkStringNoCopy("bool");
+        t = "bool";
         break;
     case nString:
-        v.mkStringNoCopy("string");
+        t = "string";
         break;
     case nPath:
-        v.mkStringNoCopy("path");
+        t = "path";
         break;
     case nNull:
-        v.mkStringNoCopy("null");
+        t = "null";
         break;
     case nAttrs:
-        v.mkStringNoCopy("set");
+        t = "set";
         break;
     case nList:
-        v.mkStringNoCopy("list");
+        t = "list";
         break;
     case nFunction:
-        v.mkStringNoCopy("lambda");
+        t = "lambda";
         break;
     case nExternal:
-        v.mkString(args[0]->external()->typeOf());
+        t = args[0]->external()->typeOf();
         break;
     case nFloat:
-        v.mkStringNoCopy("float");
+        t = "float";
         break;
     case nThunk:
-    case nFailed:
         unreachable();
     }
+    v.mkString(t);
 }
 
 static RegisterPrimOp primop_typeOf({
@@ -730,7 +731,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the start set. */
     auto startSet = state.getAttr(
-        state.s.startSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.sStartSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
 
     state.forceList(
         *startSet->value,
@@ -748,7 +749,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the operator. */
     auto op = state.getAttr(
-        state.s.operator_, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.sOperator, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
     state.forceFunction(
         *op->value, noPos, "while evaluating the 'operator' attribute passed as argument to builtins.genericClosure");
 
@@ -770,7 +771,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
             "while evaluating one of the elements generated by (or initially passed to) builtins.genericClosure");
 
         auto key = state.getAttr(
-            state.s.key,
+            state.sKey,
             e->attrs(),
             "in one of the attrsets generated by (or initially passed to) builtins.genericClosure");
         state.forceValue(*key->value, noPos);
@@ -1075,11 +1076,11 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value ** args, Val
 
     try {
         state.forceValue(*args[0], pos);
-        attrs.insert(state.s.value, args[0]);
+        attrs.insert(state.sValue, args[0]);
         attrs.insert(state.symbols.create("success"), &state.vTrue);
     } catch (AssertionError & e) {
         // `value = false;` is unfortunate but removing it is a breaking change.
-        attrs.insert(state.s.value, &state.vFalse);
+        attrs.insert(state.sValue, &state.vFalse);
         attrs.insert(state.symbols.create("success"), &state.vFalse);
     }
 
@@ -1291,8 +1292,7 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value **
     auto attrs = args[0]->attrs();
 
     /* Figure out the name first (for stack backtraces). */
-    auto nameAttr =
-        state.getAttr(state.s.name, attrs, "in the attrset passed as argument to builtins.derivationStrict");
+    auto nameAttr = state.getAttr(state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict");
 
     std::string_view drvName;
     try {
@@ -1366,7 +1366,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     using nlohmann::json;
     std::optional<StructuredAttrs> jsonObject;
     auto pos = v.determinePos(noPos);
-    auto attr = attrs->find(state.s.structuredAttrs);
+    auto attr = attrs->find(state.sStructuredAttrs);
     if (attr != attrs->end()
         && state.forceBool(
             *attr->value,
@@ -1377,7 +1377,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
 
     /* Check whether null attributes should be ignored. */
     bool ignoreNulls = false;
-    attr = attrs->find(state.s.ignoreNulls);
+    attr = attrs->find(state.sIgnoreNulls);
     if (attr != attrs->end())
         ignoreNulls = state.forceBool(
             *attr->value,
@@ -1401,7 +1401,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     outputs.insert("out");
 
     for (auto & i : attrs->lexicographicOrder(state.symbols)) {
-        if (i->name == state.s.ignoreNulls)
+        if (i->name == state.sIgnoreNulls)
             continue;
         auto key = state.symbols[i->name];
         vomit("processing attribute '%1%'", key);
@@ -1453,22 +1453,19 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                     continue;
             }
 
-            switch (i->name.getId()) {
-            case EvalState::s.contentAddressed.getId():
-                if (state.forceBool(*i->value, pos, context_below)) {
-                    contentAddressed = true;
-                    experimentalFeatureSettings.require(Xp::CaDerivations);
-                }
-                break;
-            case EvalState::s.impure.getId():
-                if (state.forceBool(*i->value, pos, context_below)) {
-                    isImpure = true;
-                    experimentalFeatureSettings.require(Xp::ImpureDerivations);
-                }
-                break;
+            if (i->name == state.sContentAddressed && state.forceBool(*i->value, pos, context_below)) {
+                contentAddressed = true;
+                experimentalFeatureSettings.require(Xp::CaDerivations);
+            }
+
+            else if (i->name == state.sImpure && state.forceBool(*i->value, pos, context_below)) {
+                isImpure = true;
+                experimentalFeatureSettings.require(Xp::ImpureDerivations);
+            }
+
             /* The `args' attribute is special: it supplies the
                command-line arguments to the builder. */
-            case EvalState::s.args.getId():
+            else if (i->name == state.sArgs) {
                 state.forceList(*i->value, pos, context_below);
                 for (auto elem : i->value->listView()) {
                     auto s = state
@@ -1477,116 +1474,86 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                                  .toOwned();
                     drv.args.push_back(s);
                 }
-                break;
+            }
+
             /* All other attributes are passed to the builder through
                the environment. */
-            default:
+            else {
 
                 if (jsonObject) {
 
-                    if (i->name == state.s.structuredAttrs)
+                    if (i->name == state.sStructuredAttrs)
                         continue;
 
                     jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
 
-                    switch (i->name.getId()) {
-                    case EvalState::s.builder.getId():
+                    if (i->name == state.sBuilder)
                         drv.builder = state.forceString(*i->value, context, pos, context_below);
-                        break;
-                    case EvalState::s.system.getId():
+                    else if (i->name == state.sSystem)
                         drv.platform = state.forceStringNoCtx(*i->value, pos, context_below);
-                        break;
-                    case EvalState::s.outputHash.getId():
+                    else if (i->name == state.sOutputHash)
                         outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
-                        break;
-                    case EvalState::s.outputHashAlgo.getId():
+                    else if (i->name == state.sOutputHashAlgo)
                         outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
-                        break;
-                    case EvalState::s.outputHashMode.getId():
+                    else if (i->name == state.sOutputHashMode)
                         handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
-                        break;
-                    case EvalState::s.outputs.getId(): {
-                        /* Require 'outputs' to be a list of strings. */
+                    else if (i->name == state.sOutputs) {
+                        /* Require ‘outputs’ to be a list of strings. */
                         state.forceList(*i->value, pos, context_below);
                         Strings ss;
                         for (auto elem : i->value->listView())
                             ss.emplace_back(state.forceStringNoCtx(*elem, pos, context_below));
                         handleOutputs(ss);
-                        break;
-                    }
-                    default:
-                        break;
                     }
 
-                    switch (i->name.getId()) {
-                    case EvalState::s.allowedReferences.getId():
+                    if (i->name == state.sAllowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks.<output>.allowedReferences' instead",
                             drvName);
-                        break;
-                    case EvalState::s.allowedRequisites.getId():
+                    if (i->name == state.sAllowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks.<output>.allowedRequisites' instead",
                             drvName);
-                        break;
-                    case EvalState::s.disallowedReferences.getId():
+                    if (i->name == state.sDisallowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead",
                             drvName);
-                        break;
-                    case EvalState::s.disallowedRequisites.getId():
+                    if (i->name == state.sDisallowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks.<output>.disallowedRequisites' instead",
                             drvName);
-                        break;
-                    case EvalState::s.maxSize.getId():
+                    if (i->name == state.sMaxSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks.<output>.maxSize' instead",
                             drvName);
-                        break;
-                    case EvalState::s.maxClosureSize.getId():
+                    if (i->name == state.sMaxClosureSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks.<output>.maxClosureSize' instead",
                             drvName);
-                        break;
-                    default:
-                        break;
-                    }
 
                 } else {
                     auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned();
-                    if (i->name == state.s.json) {
+                    if (i->name == state.sJson) {
                         warn(
                             "In derivation '%s': setting structured attributes via '__json' is deprecated, and may be disallowed in future versions of Nix. Set '__structuredAttrs = true' instead.",
                             drvName);
                         drv.structuredAttrs = StructuredAttrs::parse(s);
                     } else {
                         drv.env.emplace(key, s);
-                        switch (i->name.getId()) {
-                        case EvalState::s.builder.getId():
+                        if (i->name == state.sBuilder)
                             drv.builder = std::move(s);
-                            break;
-                        case EvalState::s.system.getId():
+                        else if (i->name == state.sSystem)
                             drv.platform = std::move(s);
-                            break;
-                        case EvalState::s.outputHash.getId():
+                        else if (i->name == state.sOutputHash)
                             outputHash = std::move(s);
-                            break;
-                        case EvalState::s.outputHashAlgo.getId():
+                        else if (i->name == state.sOutputHashAlgo)
                             outputHashAlgo = parseHashAlgoOpt(s);
-                            break;
-                        case EvalState::s.outputHashMode.getId():
+                        else if (i->name == state.sOutputHashMode)
                             handleHashMode(s);
-                            break;
-                        case EvalState::s.outputs.getId():
+                        else if (i->name == state.sOutputs)
                             handleOutputs(tokenizeString<Strings>(s));
-                            break;
-                        default:
-                            break;
-                        }
                     }
                 }
-                break;
             }
 
         } catch (Error & e) {
@@ -1755,7 +1722,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     }
 
     auto result = state.buildBindings(1 + drv.outputs.size());
-    result.alloc(state.s.drvPath)
+    result.alloc(state.sDrvPath)
         .mkString(
             drvPathS,
             {
@@ -2039,14 +2006,14 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value ** args, Va
         state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.findFile");
 
         std::string prefix;
-        auto i = v2->attrs()->find(state.s.prefix);
+        auto i = v2->attrs()->find(state.sPrefix);
         if (i != v2->attrs()->end())
             prefix = state.forceStringNoCtx(
                 *i->value,
                 pos,
                 "while evaluating the `prefix` attribute of an element of the list passed to builtins.findFile");
 
-        i = state.getAttr(state.s.path, v2->attrs(), "in an element of the __nixPath");
+        i = state.getAttr(state.sPath, v2->attrs(), "in an element of the __nixPath");
 
         NixStringContext context;
         auto path =
@@ -2819,7 +2786,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value ** args, Value
         if (n == "path")
             path.emplace(state.coerceToPath(
                 attr.pos, *attr.value, context, "while evaluating the 'path' attribute passed to 'builtins.path'"));
-        else if (attr.name == state.s.name)
+        else if (attr.name == state.sName)
             name = state.forceStringNoCtx(
                 *attr.value, attr.pos, "while evaluating the `name` attribute passed to builtins.path");
         else if (n == "filter")
@@ -3138,7 +3105,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args,
     for (const auto & [n, v2] : enumerate(listView)) {
         state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.listToAttrs");
 
-        auto j = state.getAttr(state.s.name, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.sName, v2->attrs(), "in a {name=...; value=...;} pair");
 
         auto name = state.forceStringNoCtx(
             *j->value,
@@ -3165,7 +3132,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args,
         // Note that .value is actually a Value * *; see earlier comments
         Value * v2 = *std::bit_cast<ElemPtr>(attr.value);
 
-        auto j = state.getAttr(state.s.value, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.sValue, v2->attrs(), "in a {name=...; value=...;} pair");
         prev = attr.name;
         bindings.push_back({prev, j->value, j->pos});
     }
@@ -3981,13 +3948,13 @@ static void prim_partition(EvalState & state, const PosIdx pos, Value ** args, V
     auto rlist = state.buildList(rsize);
     if (rsize)
         memcpy(rlist.elems, right.data(), sizeof(Value *) * rsize);
-    attrs.alloc(state.s.right).mkList(rlist);
+    attrs.alloc(state.sRight).mkList(rlist);
 
     auto wsize = wrong.size();
     auto wlist = state.buildList(wsize);
     if (wsize)
         memcpy(wlist.elems, wrong.data(), sizeof(Value *) * wsize);
-    attrs.alloc(state.s.wrong).mkList(wlist);
+    attrs.alloc(state.sWrong).mkList(wlist);
 
     v.mkAttrs(attrs);
 }
@@ -4381,7 +4348,7 @@ static void prim_substring(EvalState & state, const PosIdx pos, Value ** args, V
     if (len == 0) {
         state.forceValue(*args[2], pos);
         if (args[2]->type() == nString) {
-            v.mkStringNoCopy("", args[2]->context());
+            v.mkString("", args[2]->context());
             return;
         }
     }
@@ -4906,7 +4873,7 @@ static void prim_parseDrvName(EvalState & state, const PosIdx pos, Value ** args
         state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.parseDrvName");
     DrvName parsed(name);
     auto attrs = state.buildBindings(2);
-    attrs.alloc(state.s.name).mkString(parsed.name);
+    attrs.alloc(state.sName).mkString(parsed.name);
     attrs.alloc("version").mkString(parsed.version);
     v.mkAttrs(attrs);
 }

src/libexpr/primops/context.cc

@@ -219,7 +219,7 @@ static void prim_getContext(EvalState & state, const PosIdx pos, Value ** args,
             auto list = state.buildList(info.second.outputs.size());
             for (const auto & [i, output] : enumerate(info.second.outputs))
                 (list[i] = state.allocValue())->mkString(output);
-            infoAttrs.alloc(state.s.outputs).mkList(list);
+            infoAttrs.alloc(state.sOutputs).mkList(list);
         }
         attrs.alloc(state.store->printStorePath(info.first)).mkAttrs(infoAttrs);
     }
@@ -300,7 +300,7 @@ static void prim_appendContext(EvalState & state, const PosIdx pos, Value ** arg
             }
         }
 
-        if (auto attr = i.value->attrs()->get(state.s.outputs)) {
+        if (auto attr = i.value->attrs()->get(state.sOutputs)) {
             state.forceList(*attr->value, attr->pos, "while evaluating the `outputs` attribute of a string context");
             if (attr->value->listSize() && !isDerivation(name)) {
                 state

src/libexpr/primops/fetchMercurial.cc

@@ -84,7 +84,7 @@ static void prim_fetchMercurial(EvalState & state, const PosIdx pos, Value ** ar
     auto [storePath, input2] = input.fetchToStore(state.store);
 
     auto attrs2 = state.buildBindings(8);
-    state.mkStorePathString(storePath, attrs2.alloc(state.s.outPath));
+    state.mkStorePathString(storePath, attrs2.alloc(state.sOutPath));
     if (input2.getRef())
         attrs2.alloc("branch").mkString(*input2.getRef());
     // Backward compatibility: set 'rev' to

src/libexpr/primops/fetchTree.cc

@@ -29,7 +29,7 @@ void emitTreeAttrs(
 {
     auto attrs = state.buildBindings(100);
 
-    state.mkStorePathString(storePath, attrs.alloc(state.s.outPath));
+    state.mkStorePathString(storePath, attrs.alloc(state.sOutPath));
 
     // FIXME: support arbitrary input attributes.
 
@@ -95,7 +95,7 @@ static void fetchTree(
 
         fetchers::Attrs attrs;
 
-        if (auto aType = args[0]->attrs()->get(state.s.type)) {
+        if (auto aType = args[0]->attrs()->get(state.sType)) {
             if (type)
                 state.error<EvalError>("unexpected argument 'type'").atPos(pos).debugThrow();
             type = state.forceStringNoCtx(
@@ -106,14 +106,14 @@ static void fetchTree(
         attrs.emplace("type", type.value());
 
         for (auto & attr : *args[0]->attrs()) {
-            if (attr.name == state.s.type)
+            if (attr.name == state.sType)
                 continue;
             state.forceValue(*attr.value, attr.pos);
             if (attr.value->type() == nPath || attr.value->type() == nString) {
                 auto s = state.coerceToString(attr.pos, *attr.value, context, "", false, false).toOwned();
                 attrs.emplace(
                     state.symbols[attr.name],
-                    params.isFetchGit && state.symbols[attr.name] == "url" ? fixGitURL(s).to_string() : s);
+                    params.isFetchGit && state.symbols[attr.name] == "url" ? fixGitURL(s) : s);
             } else if (attr.value->type() == nBool)
                 attrs.emplace(state.symbols[attr.name], Explicit<bool>{attr.value->boolean()});
             else if (attr.value->type() == nInt) {
@@ -175,7 +175,7 @@ static void fetchTree(
         if (params.isFetchGit) {
             fetchers::Attrs attrs;
             attrs.emplace("type", "git");
-            attrs.emplace("url", fixGitURL(url).to_string());
+            attrs.emplace("url", fixGitURL(url));
             if (!attrs.contains("exportIgnore")
                 && (!attrs.contains("submodules") || !*fetchers::maybeGetBoolAttr(attrs, "submodules"))) {
                 attrs.emplace("exportIgnore", Explicit<bool>{true});

src/libexpr/primops/fromTOML.cc

@@ -136,7 +136,7 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Va
                 normalizeDatetimeFormat(t);
 #endif
                 auto attrs = state.buildBindings(2);
-                attrs.alloc("_type").mkStringNoCopy("timestamp");
+                attrs.alloc("_type").mkString("timestamp");
                 std::ostringstream s;
                 s << t;
                 auto str = toView(s);

src/libexpr/print-ambiguous.cc

@@ -75,9 +75,6 @@ void printAmbiguous(
             str << "«potential infinite recursion»";
         }
         break;
-    case nFailed:
-        str << "«failed»";
-        break;
     case nFunction:
         if (v.isLambda()) {
             str << "<LAMBDA>";

src/libexpr/print.cc

@@ -272,7 +272,7 @@ private:
     void printDerivation(Value & v)
     {
         std::optional<StorePath> storePath;
-        if (auto i = v.attrs()->get(state.s.drvPath)) {
+        if (auto i = v.attrs()->get(state.sDrvPath)) {
             NixStringContext context;
             storePath =
                 state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation");
@@ -508,11 +508,6 @@ private:
         }
     }
 
-    void printFailed(Value & v)
-    {
-        output << "«failed»";
-    }
-
     void printExternal(Value & v)
     {
         v.external()->print(output);
@@ -588,10 +583,6 @@ private:
                 printThunk(v);
                 break;
 
-            case nFailed:
-                printFailed(v);
-                break;
-
             case nExternal:
                 printExternal(v);
                 break;

src/libexpr/value-to-json.cc

@@ -53,7 +53,7 @@ json printValueAsJSON(
             out = *maybeString;
             break;
         }
-        if (auto i = v.attrs()->get(state.s.outPath))
+        if (auto i = v.attrs()->get(state.sOutPath))
             return printValueAsJSON(state, strict, *i->value, i->pos, context, copyToStore);
         else {
             out = json::object();
@@ -96,7 +96,6 @@ json printValueAsJSON(
         break;
 
     case nThunk:
-    case nFailed:
     case nFunction:
         state.error<TypeError>("cannot convert %1% to JSON", showType(v)).atPos(v.determinePos(pos)).debugThrow();
     }

src/libexpr/value-to-xml.cc

@@ -98,14 +98,14 @@ static void printValueAsXML(
             XMLAttrs xmlAttrs;
 
             Path drvPath;
-            if (auto a = v.attrs()->get(state.s.drvPath)) {
+            if (auto a = v.attrs()->get(state.sDrvPath)) {
                 if (strict)
                     state.forceValue(*a->value, a->pos);
                 if (a->value->type() == nString)
                     xmlAttrs["drvPath"] = drvPath = a->value->c_str();
             }
 
-            if (auto a = v.attrs()->get(state.s.outPath)) {
+            if (auto a = v.attrs()->get(state.sOutPath)) {
                 if (strict)
                     state.forceValue(*a->value, a->pos);
                 if (a->value->type() == nString)
@@ -170,11 +170,6 @@ static void printValueAsXML(
 
     case nThunk:
         doc.writeEmptyElement("unevaluated");
-        break;
-
-    case nFailed:
-        doc.writeEmptyElement("failed");
-        break;
     }
 }
 

src/libfetchers-c/nix_api_fetchers.cc

@@ -2,8 +2,6 @@
 #include "nix_api_fetchers_internal.hh"
 #include "nix_api_util_internal.h"
 
-extern "C" {
-
 nix_fetchers_settings * nix_fetchers_settings_new(nix_c_context * context)
 {
     try {
@@ -19,5 +17,3 @@ void nix_fetchers_settings_free(nix_fetchers_settings * settings)
 {
     delete settings;
 }
-
-} // extern "C"

src/libfetchers/git-lfs-fetch.cc

@@ -25,7 +25,7 @@ static void downloadToSink(
     std::string sha256Expected,
     size_t sizeExpected)
 {
-    FileTransferRequest request(parseURL(url));
+    FileTransferRequest request(url);
     Headers headers;
     if (authHeader.has_value())
         headers.push_back({"Authorization", *authHeader});
@@ -69,8 +69,7 @@ static LfsApiInfo getLfsApi(const ParsedURL & url)
 
         args.push_back("--");
         args.push_back("git-lfs-authenticate");
-        // FIXME %2F encode slashes? Does this command take/accept percent encoding?
-        args.push_back(url.renderPath(/*encode=*/false));
+        args.push_back(url.path);
         args.push_back("download");
 
         auto [status, output] = runProgram({.program = "ssh", .args = args});
@@ -180,7 +179,7 @@ Fetch::Fetch(git_repository * repo, git_oid rev)
 
     const auto remoteUrl = lfs::getLfsEndpointUrl(repo);
 
-    this->url = nix::fixGitURL(remoteUrl).canonicalise();
+    this->url = nix::parseURL(nix::fixGitURL(remoteUrl)).canonicalise();
 }
 
 bool Fetch::shouldFetch(const CanonPath & path) const
@@ -208,7 +207,7 @@ std::vector<nlohmann::json> Fetch::fetchUrls(const std::vector<Pointer> & pointe
     auto api = lfs::getLfsApi(this->url);
     auto url = api.endpoint + "/objects/batch";
     const auto & authHeader = api.authHeader;
-    FileTransferRequest request(parseURL(url));
+    FileTransferRequest request(url);
     request.post = true;
     Headers headers;
     if (authHeader.has_value())

src/libfetchers/git-utils.cc

@@ -568,34 +568,23 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this<GitRepoImpl>
 
     void verifyCommit(const Hash & rev, const std::vector<fetchers::PublicKey> & publicKeys) override
     {
-        // Map of SSH key types to their internal OpenSSH representations
-        static const std::unordered_map<std::string_view, std::string_view> keyTypeMap = {
-            {"ssh-dsa", "ssh-dsa"},
-            {"ssh-ecdsa", "ssh-ecdsa"},
-            {"ssh-ecdsa-sk", "sk-ecdsa-sha2-nistp256@openssh.com"},
-            {"ssh-ed25519", "ssh-ed25519"},
-            {"ssh-ed25519-sk", "sk-ssh-ed25519@openssh.com"},
-            {"ssh-rsa", "ssh-rsa"}};
-
         // Create ad-hoc allowedSignersFile and populate it with publicKeys
         auto allowedSignersFile = createTempFile().second;
         std::string allowedSigners;
-
         for (const fetchers::PublicKey & k : publicKeys) {
-            auto it = keyTypeMap.find(k.type);
-            if (it == keyTypeMap.end()) {
-                std::string supportedTypes;
-                for (const auto & [type, _] : keyTypeMap) {
-                    supportedTypes += fmt("  %s\n", type);
-                }
+            if (k.type != "ssh-dsa" && k.type != "ssh-ecdsa" && k.type != "ssh-ecdsa-sk" && k.type != "ssh-ed25519"
+                && k.type != "ssh-ed25519-sk" && k.type != "ssh-rsa")
                 throw Error(
-                    "Invalid SSH key type '%s' in publicKeys.\n"
-                    "Please use one of:\n%s",
-                    k.type,
-                    supportedTypes);
-            }
-
-            allowedSigners += fmt("* %s %s\n", it->second, k.key);
+                    "Unknown key type '%s'.\n"
+                    "Please use one of\n"
+                    "- ssh-dsa\n"
+                    "  ssh-ecdsa\n"
+                    "  ssh-ecdsa-sk\n"
+                    "  ssh-ed25519\n"
+                    "  ssh-ed25519-sk\n"
+                    "  ssh-rsa",
+                    k.type);
+            allowedSigners += "* " + k.type + " " + k.key + "\n";
         }
         writeFile(allowedSignersFile, allowedSigners);
 

src/libfetchers/git.cc

@@ -163,8 +163,8 @@ struct GitInputScheme : InputScheme
 {
     std::optional<Input> inputFromURL(const Settings & settings, const ParsedURL & url, bool requireTree) const override
     {
-        auto parsedScheme = parseUrlScheme(url.scheme);
-        if (parsedScheme.application != "git")
+        if (url.scheme != "git" && url.scheme != "git+http" && url.scheme != "git+https" && url.scheme != "git+ssh"
+            && url.scheme != "git+file")
             return {};
 
         auto url2(url);
@@ -233,7 +233,9 @@ struct GitInputScheme : InputScheme
 
         Input input{settings};
         input.attrs = attrs;
-        input.attrs["url"] = fixGitURL(getStrAttr(attrs, "url")).to_string();
+        auto url = fixGitURL(getStrAttr(attrs, "url"));
+        parseURL(url);
+        input.attrs["url"] = url;
         getShallowAttr(input);
         getSubmodulesAttr(input);
         getAllRefsAttr(input);
@@ -462,8 +464,8 @@ struct GitInputScheme : InputScheme
 
         // Why are we checking for bare repository?
         // well if it's a bare repository we want to force a git fetch rather than copying the folder
-        auto isBareRepository = [](PathView path) { return pathExists(path) && !pathExists(path + "/.git"); };
-
+        bool isBareRepository = url.scheme == "file" && pathExists(url.path) && !pathExists(url.path + "/.git");
+        //
         // FIXME: here we turn a possibly relative path into an absolute path.
         // This allows relative git flake inputs to be resolved against the
         // **current working directory** (as in POSIX), which tends to work out
@@ -472,10 +474,8 @@ struct GitInputScheme : InputScheme
         //
         // See: https://discourse.nixos.org/t/57783 and #9708
         //
-        if (url.scheme == "file" && !forceHttp && !isBareRepository(renderUrlPathEnsureLegal(url.path))) {
-            auto path = renderUrlPathEnsureLegal(url.path);
-
-            if (!isAbsolute(path)) {
+        if (url.scheme == "file" && !forceHttp && !isBareRepository) {
+            if (!isAbsolute(url.path)) {
                 warn(
                     "Fetching Git repository '%s', which uses a path relative to the current directory. "
                     "This is not supported and will stop working in a future release. "
@@ -485,10 +485,10 @@ struct GitInputScheme : InputScheme
 
             // If we don't check here for the path existence, then we can give libgit2 any directory
             // and it will initialize them as git directories.
-            if (!pathExists(path)) {
-                throw Error("The path '%s' does not exist.", path);
+            if (!pathExists(url.path)) {
+                throw Error("The path '%s' does not exist.", url.path);
             }
-            repoInfo.location = std::filesystem::absolute(path);
+            repoInfo.location = std::filesystem::absolute(url.path);
         } else {
             if (url.scheme == "file")
                 /* Query parameters are meaningless for file://, but

src/libfetchers/github.cc

@@ -19,7 +19,7 @@ namespace nix::fetchers {
 
 struct DownloadUrl
 {
-    ParsedURL url;
+    std::string url;
     Headers headers;
 };
 
@@ -38,8 +38,7 @@ struct GitArchiveInputScheme : InputScheme
         if (url.scheme != schemeName())
             return {};
 
-        /* This ignores empty path segments for back-compat. Older versions used a tokenizeString here. */
-        auto path = url.pathSegments(/*skipEmpty=*/true) | std::ranges::to<std::vector<std::string>>();
+        auto path = tokenizeString<std::vector<std::string>>(url.path, "/");
 
         std::optional<Hash> rev;
         std::optional<std::string> ref;
@@ -140,12 +139,12 @@ struct GitArchiveInputScheme : InputScheme
         auto repo = getStrAttr(input.attrs, "repo");
         auto ref = input.getRef();
         auto rev = input.getRev();
-        std::vector<std::string> path{owner, repo};
+        auto path = owner + "/" + repo;
         assert(!(ref && rev));
         if (ref)
-            path.push_back(*ref);
+            path += "/" + *ref;
         if (rev)
-            path.push_back(rev->to_string(HashFormat::Base16, false));
+            path += "/" + rev->to_string(HashFormat::Base16, false);
         auto url = ParsedURL{
             .scheme = std::string{schemeName()},
             .path = path,
@@ -421,7 +420,7 @@ struct GitHubInputScheme : GitArchiveInputScheme
         const auto url =
             fmt(urlFmt, host, getOwner(input), getRepo(input), input.getRev()->to_string(HashFormat::Base16, false));
 
-        return DownloadUrl{parseURL(url), headers};
+        return DownloadUrl{url, headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override
@@ -501,7 +500,7 @@ struct GitLabInputScheme : GitArchiveInputScheme
                 input.getRev()->to_string(HashFormat::Base16, false));
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
-        return DownloadUrl{parseURL(url), headers};
+        return DownloadUrl{url, headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override
@@ -593,7 +592,7 @@ struct SourceHutInputScheme : GitArchiveInputScheme
                 input.getRev()->to_string(HashFormat::Base16, false));
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
-        return DownloadUrl{parseURL(url), headers};
+        return DownloadUrl{url, headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override

src/libfetchers/indirect.cc

@@ -14,8 +14,7 @@ struct IndirectInputScheme : InputScheme
         if (url.scheme != "flake")
             return {};
 
-        /* This ignores empty path segments for back-compat. Older versions used a tokenizeString here. */
-        auto path = url.pathSegments(/*skipEmpty=*/true) | std::ranges::to<std::vector<std::string>>();
+        auto path = tokenizeString<std::vector<std::string>>(url.path, "/");
 
         std::optional<Hash> rev;
         std::optional<std::string> ref;
@@ -83,15 +82,16 @@ struct IndirectInputScheme : InputScheme
 
     ParsedURL toURL(const Input & input) const override
     {
-        ParsedURL url{
-            .scheme = "flake",
-            .path = {getStrAttr(input.attrs, "id")},
-        };
+        ParsedURL url;
+        url.scheme = "flake";
+        url.path = getStrAttr(input.attrs, "id");
         if (auto ref = input.getRef()) {
-            url.path.push_back(*ref);
+            url.path += '/';
+            url.path += *ref;
         };
         if (auto rev = input.getRev()) {
-            url.path.push_back(rev->gitRev());
+            url.path += '/';
+            url.path += rev->gitRev();
         };
         return url;
     }

src/libfetchers/mercurial.cc

@@ -120,7 +120,7 @@ struct MercurialInputScheme : InputScheme
     {
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         if (url.scheme == "file" && !input.getRef() && !input.getRev())
-            return renderUrlPathEnsureLegal(url.path);
+            return url.path;
         return {};
     }
 
@@ -152,7 +152,7 @@ struct MercurialInputScheme : InputScheme
     {
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         bool isLocal = url.scheme == "file";
-        return {isLocal, isLocal ? renderUrlPathEnsureLegal(url.path) : url.to_string()};
+        return {isLocal, isLocal ? url.path : url.to_string()};
     }
 
     StorePath fetchToStore(ref<Store> store, Input & input) const

src/libfetchers/path.cc

@@ -20,7 +20,7 @@ struct PathInputScheme : InputScheme
 
         Input input{settings};
         input.attrs.insert_or_assign("type", "path");
-        input.attrs.insert_or_assign("path", renderUrlPathEnsureLegal(url.path));
+        input.attrs.insert_or_assign("path", url.path);
 
         for (auto & [name, value] : url.query)
             if (name == "rev" || name == "narHash")
@@ -74,7 +74,7 @@ struct PathInputScheme : InputScheme
         query.erase("__final");
         return ParsedURL{
             .scheme = "path",
-            .path = splitString<std::vector<std::string>>(getStrAttr(input.attrs, "path"), "/"),
+            .path = getStrAttr(input.attrs, "path"),
             .query = query,
         };
     }

src/libfetchers/tarball.cc

@@ -43,7 +43,7 @@ DownloadFileResult downloadFile(
     if (cached && !cached->expired)
         return useCached();
 
-    FileTransferRequest request(ValidURL{url});
+    FileTransferRequest request(url);
     request.headers = headers;
     if (cached)
         request.expectedETag = getStrAttr(cached->value, "etag");
@@ -107,20 +107,20 @@ DownloadFileResult downloadFile(
 }
 
 static DownloadTarballResult downloadTarball_(
-    const Settings & settings, const std::string & urlS, const Headers & headers, const std::string & displayPrefix)
+    const Settings & settings, const std::string & url, const Headers & headers, const std::string & displayPrefix)
 {
-    ValidURL url = urlS;
 
     // Some friendly error messages for common mistakes.
     // Namely lets catch when the url is a local file path, but
     // it is not in fact a tarball.
-    if (url.scheme() == "file") {
-        std::filesystem::path localPath = renderUrlPathEnsureLegal(url.path());
-        if (!exists(localPath)) {
+    if (url.rfind("file://", 0) == 0) {
+        // Remove "file://" prefix to get the local file path
+        std::string localPath = url.substr(7);
+        if (!std::filesystem::exists(localPath)) {
             throw Error("tarball '%s' does not exist.", localPath);
         }
-        if (is_directory(localPath)) {
-            if (exists(localPath / ".git")) {
+        if (std::filesystem::is_directory(localPath)) {
+            if (std::filesystem::exists(localPath + "/.git")) {
                 throw Error(
                     "tarball '%s' is a git repository, not a tarball. Please use `git+file` as the scheme.", localPath);
             }
@@ -128,7 +128,7 @@ static DownloadTarballResult downloadTarball_(
         }
     }
 
-    Cache::Key cacheKey{"tarball", {{"url", urlS}}};
+    Cache::Key cacheKey{"tarball", {{"url", url}}};
 
     auto cached = settings.getCache()->lookupExpired(cacheKey);
 
@@ -166,7 +166,7 @@ static DownloadTarballResult downloadTarball_(
 
     /* Note: if the download is cached, `importTarball()` will receive
        no data, which causes it to import an empty tarball. */
-    auto archive = !url.path().empty() && hasSuffix(toLower(url.path().back()), ".zip") ? ({
+    auto archive = hasSuffix(toLower(parseURL(url).path), ".zip") ? ({
         /* In streaming mode, libarchive doesn't handle
            symlinks in zip files correctly (#10649). So write
            the entire file to disk so libarchive can access it
@@ -180,7 +180,7 @@ static DownloadTarballResult downloadTarball_(
         }
         TarArchive{path};
     })
-                                                                                        : TarArchive{*source};
+                                                                  : TarArchive{*source};
     auto tarballCache = getTarballCache();
     auto parseSink = tarballCache->getFileSystemObjectSink();
     auto lastModified = unpackTarfileToSink(archive, *parseSink);
@@ -234,11 +234,8 @@ struct CurlInputScheme : InputScheme
 {
     const StringSet transportUrlSchemes = {"file", "http", "https"};
 
-    bool hasTarballExtension(const ParsedURL & url) const
+    bool hasTarballExtension(std::string_view path) const
     {
-        if (url.path.empty())
-            return false;
-        const auto & path = url.path.back();
         return hasSuffix(path, ".zip") || hasSuffix(path, ".tar") || hasSuffix(path, ".tgz")
                || hasSuffix(path, ".tar.gz") || hasSuffix(path, ".tar.xz") || hasSuffix(path, ".tar.bz2")
                || hasSuffix(path, ".tar.zst");
@@ -339,7 +336,7 @@ struct FileInputScheme : CurlInputScheme
         auto parsedUrlScheme = parseUrlScheme(url.scheme);
         return transportUrlSchemes.count(std::string(parsedUrlScheme.transport))
                && (parsedUrlScheme.application ? parsedUrlScheme.application.value() == schemeName()
-                                               : (!requireTree && !hasTarballExtension(url)));
+                                               : (!requireTree && !hasTarballExtension(url.path)));
     }
 
     std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override
@@ -376,7 +373,7 @@ struct TarballInputScheme : CurlInputScheme
 
         return transportUrlSchemes.count(std::string(parsedUrlScheme.transport))
                && (parsedUrlScheme.application ? parsedUrlScheme.application.value() == schemeName()
-                                               : (requireTree || hasTarballExtension(url)));
+                                               : (requireTree || hasTarballExtension(url.path)));
     }
 
     std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override

src/libflake-c/nix_api_flake.cc

@@ -10,8 +10,6 @@
 
 #include "nix/flake/flake.hh"
 
-extern "C" {
-
 nix_flake_settings * nix_flake_settings_new(nix_c_context * context)
 {
     nix_clear_err(context);
@@ -205,5 +203,3 @@ nix_value * nix_locked_flake_get_output_attrs(
     }
     NIXC_CATCH_ERRS_NULL
 }
-
-} // extern "C"

src/libflake-tests/flakeref.cc

@@ -2,7 +2,6 @@
 
 #include "nix/fetchers/fetch-settings.hh"
 #include "nix/flake/flakeref.hh"
-#include "nix/fetchers/attrs.hh"
 
 namespace nix {
 
@@ -91,158 +90,6 @@ TEST(parseFlakeRef, GitArchiveInput)
     }
 }
 
-struct InputFromURLTestCase
-{
-    std::string url;
-    fetchers::Attrs attrs;
-    std::string description;
-    std::string expectedUrl = url;
-};
-
-class InputFromURLTest : public ::testing::WithParamInterface<InputFromURLTestCase>, public ::testing::Test
-{};
-
-TEST_P(InputFromURLTest, attrsAreCorrectAndRoundTrips)
-{
-    experimentalFeatureSettings.experimentalFeatures.get().insert(Xp::Flakes);
-    fetchers::Settings fetchSettings;
-
-    const auto & testCase = GetParam();
-
-    auto flakeref = parseFlakeRef(fetchSettings, testCase.url);
-
-    EXPECT_EQ(flakeref.toAttrs(), testCase.attrs);
-    EXPECT_EQ(flakeref.to_string(), testCase.expectedUrl);
-
-    auto input = fetchers::Input::fromURL(fetchSettings, flakeref.to_string());
-
-    EXPECT_EQ(input.toURLString(), testCase.expectedUrl);
-    EXPECT_EQ(input.toAttrs(), testCase.attrs);
-
-    // Round-trip check.
-    auto input2 = fetchers::Input::fromURL(fetchSettings, input.toURLString());
-    EXPECT_EQ(input, input2);
-    EXPECT_EQ(input.toURLString(), input2.toURLString());
-}
-
-using fetchers::Attr;
-
-INSTANTIATE_TEST_SUITE_P(
-    InputFromURL,
-    InputFromURLTest,
-    ::testing::Values(
-        InputFromURLTestCase{
-            .url = "flake:nixpkgs",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                },
-            .description = "basic_indirect",
-        },
-        InputFromURLTestCase{
-            .url = "flake:nixpkgs/branch",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                },
-            .description = "basic_indirect_branch",
-        },
-        InputFromURLTestCase{
-            .url = "nixpkgs/branch",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                },
-            .description = "flake_id_ref_branch",
-            .expectedUrl = "flake:nixpkgs/branch",
-        },
-        InputFromURLTestCase{
-            .url = "nixpkgs/branch/2aae6c35c94fcfb415dbe95f408b9ce91ee846ed",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                    {"rev", Attr("2aae6c35c94fcfb415dbe95f408b9ce91ee846ed")},
-                },
-            .description = "flake_id_ref_branch_trailing_slash",
-            .expectedUrl = "flake:nixpkgs/branch/2aae6c35c94fcfb415dbe95f408b9ce91ee846ed",
-        },
-        // The following tests are for back-compat with lax parsers in older versions
-        // that used `tokenizeString` for splitting path segments, which ignores empty
-        // strings.
-        InputFromURLTestCase{
-            .url = "nixpkgs/branch////",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                },
-            .description = "flake_id_ref_branch_ignore_empty_trailing_segments",
-            .expectedUrl = "flake:nixpkgs/branch",
-        },
-        InputFromURLTestCase{
-            .url = "nixpkgs/branch///2aae6c35c94fcfb415dbe95f408b9ce91ee846ed///",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                    {"rev", Attr("2aae6c35c94fcfb415dbe95f408b9ce91ee846ed")},
-                },
-            .description = "flake_id_ref_branch_ignore_empty_segments_ref_rev",
-            .expectedUrl = "flake:nixpkgs/branch/2aae6c35c94fcfb415dbe95f408b9ce91ee846ed",
-        },
-        InputFromURLTestCase{
-            // Note that this is different from above because the "flake id" shorthand
-            // doesn't allow this.
-            .url = "flake:/nixpkgs///branch////",
-            .attrs =
-                {
-                    {"id", Attr("nixpkgs")},
-                    {"type", Attr("indirect")},
-                    {"ref", Attr("branch")},
-                },
-            .description = "indirect_branch_empty_segments_everywhere",
-            .expectedUrl = "flake:nixpkgs/branch",
-        },
-        InputFromURLTestCase{
-            // TODO: Technically this has an empty authority, but it's ignored
-            // for now. Yes, this is what all versions going back to at least
-            // 2.18 did and yes, this should not be allowed.
-            .url = "github://////owner%42/////repo%41///branch%43////",
-            .attrs =
-                {
-                    {"type", Attr("github")},
-                    {"owner", Attr("ownerB")},
-                    {"repo", Attr("repoA")},
-                    {"ref", Attr("branchC")},
-                },
-            .description = "github_ref_slashes_in_path_everywhere",
-            .expectedUrl = "github:ownerB/repoA/branchC",
-        },
-        InputFromURLTestCase{
-            // FIXME: Subgroups in gitlab URLs are busted. This double-encoding
-            // behavior exists since 2.18. See issue #9161 and PR #8845.
-            .url = "gitlab:/owner%252Fsubgroup/////repo%41///branch%43////",
-            .attrs =
-                {
-                    {"type", Attr("gitlab")},
-                    {"owner", Attr("owner%2Fsubgroup")},
-                    {"repo", Attr("repoA")},
-                    {"ref", Attr("branchC")},
-                },
-            .description = "gitlab_ref_slashes_in_path_everywhere_with_pct_encoding",
-            .expectedUrl = "gitlab:owner%252Fsubgroup/repoA/branchC",
-        }),
-    [](const ::testing::TestParamInfo<InputFromURLTestCase> & info) { return info.param.description; });
-
 TEST(to_string, doesntReencodeUrl)
 {
     fetchers::Settings fetchSettings;

src/libflake/flake.cc

@@ -232,7 +232,7 @@ static Flake readFlake(
         .path = flakePath,
     };
 
-    if (auto description = vInfo.attrs()->get(state.s.description)) {
+    if (auto description = vInfo.attrs()->get(state.sDescription)) {
         expectType(state, nString, *description->value, description->pos);
         flake.description = description->value->c_str();
     }
@@ -253,7 +253,7 @@ static Flake readFlake(
 
         if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals()) {
             for (auto & formal : outputs->value->lambda().fun->formals->formals) {
-                if (formal.name != state.s.self)
+                if (formal.name != state.sSelf)
                     flake.inputs.emplace(
                         state.symbols[formal.name],
                         FlakeInput{.ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name]))});
@@ -305,8 +305,7 @@ static Flake readFlake(
     }
 
     for (auto & attr : *vInfo.attrs()) {
-        if (attr.name != state.s.description && attr.name != sInputs && attr.name != sOutputs
-            && attr.name != sNixConfig)
+        if (attr.name != state.sDescription && attr.name != sInputs && attr.name != sOutputs && attr.name != sNixConfig)
             throw Error(
                 "flake '%s' has an unsupported attribute '%s', at %s",
                 resolvedRef,

src/libflake/flakeref.cc

@@ -143,7 +143,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
                     auto parsedURL = ParsedURL{
                         .scheme = "git+file",
                         .authority = ParsedURL::Authority{},
-                        .path = splitString<std::vector<std::string>>(flakeRoot, "/"),
+                        .path = flakeRoot,
                         .query = query,
                         .fragment = fragment,
                     };
@@ -172,13 +172,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
 
     return fromParsedURL(
         fetchSettings,
-        {
-            .scheme = "path",
-            .authority = ParsedURL::Authority{},
-            .path = splitString<std::vector<std::string>>(path, "/"),
-            .query = query,
-            .fragment = fragment,
-        },
+        {.scheme = "path", .authority = ParsedURL::Authority{}, .path = path, .query = query, .fragment = fragment},
         isFlake);
 }
 
@@ -198,8 +192,8 @@ parseFlakeIdRef(const fetchers::Settings & fetchSettings, const std::string & ur
     if (std::regex_match(url, match, flakeRegex)) {
         auto parsedURL = ParsedURL{
             .scheme = "flake",
-            .authority = std::nullopt,
-            .path = splitString<std::vector<std::string>>(match[1].str(), "/"),
+            .authority = ParsedURL::Authority{},
+            .path = match[1],
         };
 
         return std::make_pair(
@@ -217,12 +211,8 @@ std::optional<std::pair<FlakeRef, std::string>> parseURLFlakeRef(
 {
     try {
         auto parsed = parseURL(url, /*lenient=*/true);
-        if (baseDir && (parsed.scheme == "path" || parsed.scheme == "git+file")) {
-            /* Here we know that the path must not contain encoded '/' or NUL bytes. */
-            auto path = renderUrlPathEnsureLegal(parsed.path);
-            if (!isAbsolute(path))
-                parsed.path = splitString<std::vector<std::string>>(absPath(path, *baseDir), "/");
-        }
+        if (baseDir && (parsed.scheme == "path" || parsed.scheme == "git+file") && !isAbsolute(parsed.path))
+            parsed.path = absPath(parsed.path, *baseDir);
         return fromParsedURL(fetchSettings, std::move(parsed), isFlake);
     } catch (BadURL &) {
         return std::nullopt;

src/libflake/url-name.cc

@@ -27,21 +27,16 @@ std::optional<std::string> getNameFromURL(const ParsedURL & url)
         return match.str(2);
     }
 
-    /* This is not right, because special chars like slashes within the
-       path fragments should be percent encoded, but I don't think any
-       of the regexes above care. */
-    auto path = concatStringsSep("/", url.path);
-
     /* If this is a github/gitlab/sourcehut flake, use the repo name */
-    if (std::regex_match(url.scheme, gitProviderRegex) && std::regex_match(path, match, secondPathSegmentRegex))
+    if (std::regex_match(url.scheme, gitProviderRegex) && std::regex_match(url.path, match, secondPathSegmentRegex))
         return match.str(1);
 
     /* If it is a regular git flake, use the directory name */
-    if (std::regex_match(url.scheme, gitSchemeRegex) && std::regex_match(path, match, lastPathSegmentRegex))
+    if (std::regex_match(url.scheme, gitSchemeRegex) && std::regex_match(url.path, match, lastPathSegmentRegex))
         return match.str(1);
 
     /* If there is no fragment, take the last element of the path */
-    if (std::regex_match(path, match, lastPathSegmentRegex))
+    if (std::regex_match(url.path, match, lastPathSegmentRegex))
         return match.str(1);
 
     /* If even that didn't work, the URL does not contain enough info to determine a useful name */

src/libmain-c/nix_api_main.cc

@@ -5,8 +5,6 @@
 
 #include "nix/main/plugin.hh"
 
-extern "C" {
-
 nix_err nix_init_plugins(nix_c_context * context)
 {
     if (context)
@@ -16,5 +14,3 @@ nix_err nix_init_plugins(nix_c_context * context)
     }
     NIXC_CATCH_ERRS
 }
-
-} // extern "C"

src/libstore-c/nix_api_store.cc

@@ -10,8 +10,6 @@
 
 #include "nix/store/globals.hh"
 
-extern "C" {
-
 nix_err nix_libstore_init(nix_c_context * context)
 {
     if (context)
@@ -93,7 +91,7 @@ nix_store_get_version(nix_c_context * context, Store * store, nix_get_string_cal
     NIXC_CATCH_ERRS
 }
 
-bool nix_store_is_valid_path(nix_c_context * context, Store * store, const StorePath * path)
+bool nix_store_is_valid_path(nix_c_context * context, Store * store, StorePath * path)
 {
     if (context)
         context->last_err_code = NIX_OK;
@@ -131,7 +129,7 @@ nix_err nix_store_realise(
     Store * store,
     StorePath * path,
     void * userdata,
-    void (*callback)(void * userdata, const char *, const StorePath *))
+    void (*callback)(void * userdata, const char *, const char *))
 {
     if (context)
         context->last_err_code = NIX_OK;
@@ -146,8 +144,8 @@ nix_err nix_store_realise(
         if (callback) {
             for (const auto & result : results) {
                 for (const auto & [outputName, realisation] : result.builtOutputs) {
-                    StorePath p{realisation.outPath};
-                    callback(userdata, outputName.c_str(), &p);
+                    auto op = store->ptr->printStorePath(realisation.outPath);
+                    callback(userdata, outputName.c_str(), op.c_str());
                 }
             }
         }
@@ -182,5 +180,3 @@ nix_err nix_store_copy_closure(nix_c_context * context, Store * srcStore, Store
     }
     NIXC_CATCH_ERRS
 }
-
-} // extern "C"

src/libstore-c/nix_api_store.h

@@ -148,7 +148,7 @@ void nix_store_path_free(StorePath * p);
  * @param[in] path Path to check
  * @return true or false, error info in context
  */
-bool nix_store_is_valid_path(nix_c_context * context, Store * store, const StorePath * path);
+bool nix_store_is_valid_path(nix_c_context * context, Store * store, StorePath * path);
 
 /**
  * @brief Get the physical location of a store path
@@ -190,7 +190,7 @@ nix_err nix_store_realise(
     Store * store,
     StorePath * path,
     void * userdata,
-    void (*callback)(void * userdata, const char * outname, const StorePath * out));
+    void (*callback)(void * userdata, const char * outname, const char * out));
 
 /**
  * @brief get the version of a nix store.

src/libstore-c/nix_api_store_internal.h

@@ -2,8 +2,6 @@
 #define NIX_API_STORE_INTERNAL_H
 #include "nix/store/store-api.hh"
 
-extern "C" {
-
 struct Store
 {
     nix::ref<nix::Store> ptr;
@@ -14,6 +12,4 @@ struct StorePath
     nix::StorePath path;
 };
 
-} // extern "C"
-
 #endif

src/libstore-tests/derivation-parser-bench.cc

@@ -28,33 +28,12 @@ static void BM_ParseRealDerivationFile(benchmark::State & state, const std::stri
     state.SetBytesProcessed(state.iterations() * content.size());
 }
 
-// Benchmark unparsing real derivation files
-static void BM_UnparseRealDerivationFile(benchmark::State & state, const std::string & filename)
-{
-    // Read the file once
-    std::ifstream file(filename);
-    std::stringstream buffer;
-    buffer << file.rdbuf();
-    std::string content = buffer.str();
-
-    auto store = openStore("dummy://");
-    ExperimentalFeatureSettings xpSettings;
-    auto drv = parseDerivation(*store, std::string(content), "test", xpSettings);
-
-    for (auto _ : state) {
-        auto unparsed = drv.unparse(*store, /*maskOutputs=*/false);
-        benchmark::DoNotOptimize(unparsed);
-        assert(unparsed.size() == content.size());
-    }
-    state.SetBytesProcessed(state.iterations() * content.size());
-}
-
 // Register benchmarks for actual test derivation files if they exist
 BENCHMARK_CAPTURE(
-    BM_ParseRealDerivationFile, hello, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/hello.drv");
+    BM_ParseRealDerivationFile,
+    hello,
+    getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value_or(NIX_UNIT_TEST_DATA) + "/derivation/hello.drv");
 BENCHMARK_CAPTURE(
-    BM_ParseRealDerivationFile, firefox, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/firefox.drv");
-BENCHMARK_CAPTURE(
-    BM_UnparseRealDerivationFile, hello, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/hello.drv");
-BENCHMARK_CAPTURE(
-    BM_UnparseRealDerivationFile, firefox, getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value() + "/derivation/firefox.drv");
+    BM_ParseRealDerivationFile,
+    firefox,
+    getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value_or(NIX_UNIT_TEST_DATA) + "/derivation/firefox.drv");

src/libstore-tests/local-store.cc

@@ -36,7 +36,7 @@ TEST(LocalStore, constructConfig_rootPath)
 TEST(LocalStore, constructConfig_to_string)
 {
     LocalStoreConfig config{"local", "", {}};
-    EXPECT_EQ(config.getReference().to_string(), "local");
+    EXPECT_EQ(config.getReference().render(), "local");
 }
 
 } // namespace nix

src/libstore-tests/meson.build

@@ -130,13 +130,10 @@ if get_option('benchmarks')
     link_args : linker_export_flags,
     install : true,
     cpp_pch : do_pch ? [ 'pch/precompiled-headers.hh' ] : [],
+    cpp_args : [
+      '-DNIX_UNIT_TEST_DATA="' + meson.current_source_dir() + '/data"',
+    ],
   )
 
-  benchmark(
-    'nix-store-benchmarks',
-    benchmark_exe,
-    env : {
-      '_NIX_TEST_UNIT_DATA' : meson.current_source_dir() / 'data',
-    },
-  )
+  benchmark('nix-store-benchmarks', benchmark_exe)
 endif

src/libstore-tests/nix_api_store.cc

@@ -1,5 +1,7 @@
 #include "nix_api_util.h"
+#include "nix_api_util_internal.h"
 #include "nix_api_store.h"
+#include "nix_api_store_internal.h"
 
 #include "nix/store/tests/nix_api_store.hh"
 #include "nix/util/tests/string_callback.hh"
@@ -63,7 +65,7 @@ TEST_F(nix_api_store_test, nix_store_get_storedir)
 TEST_F(nix_api_store_test, InvalidPathFails)
 {
     nix_store_parse_path(ctx, store, "invalid-path");
-    ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR);
+    ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR);
 }
 
 TEST_F(nix_api_store_test, ReturnsValidStorePath)
@@ -78,7 +80,7 @@ TEST_F(nix_api_store_test, ReturnsValidStorePath)
 TEST_F(nix_api_store_test, SetsLastErrCodeToNixOk)
 {
     StorePath * path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str());
-    ASSERT_EQ(nix_err_code(ctx), NIX_OK);
+    ASSERT_EQ(ctx->last_err_code, NIX_OK);
     nix_store_path_free(path);
 }
 
@@ -101,7 +103,7 @@ TEST_F(nix_api_util_context, nix_store_open_dummy)
 {
     nix_libstore_init(ctx);
     Store * store = nix_store_open(ctx, "dummy://", nullptr);
-    ASSERT_EQ(NIX_OK, nix_err_code(ctx));
+    ASSERT_EQ(NIX_OK, ctx->last_err_code);
     ASSERT_STREQ("dummy://", store->ptr->config.getReference().render(/*withParams=*/true).c_str());
 
     std::string str;
@@ -115,7 +117,7 @@ TEST_F(nix_api_util_context, nix_store_open_invalid)
 {
     nix_libstore_init(ctx);
     Store * store = nix_store_open(ctx, "invalid://", nullptr);
-    ASSERT_EQ(NIX_ERR_NIX_ERROR, nix_err_code(ctx));
+    ASSERT_EQ(NIX_ERR_NIX_ERROR, ctx->last_err_code);
     ASSERT_EQ(nullptr, store);
     nix_store_free(store);
 }

src/libstore-tests/s3.cc

@@ -21,7 +21,7 @@ class ParsedS3URLTest : public ::testing::WithParamInterface<ParsedS3URLTestCase
 TEST_P(ParsedS3URLTest, parseS3URLSuccessfully)
 {
     const auto & testCase = GetParam();
-    auto parsed = ParsedS3URL::parse(parseURL(testCase.url));
+    auto parsed = ParsedS3URL::parse(testCase.url);
     ASSERT_EQ(parsed, testCase.expected);
 }
 
@@ -33,57 +33,51 @@ INSTANTIATE_TEST_SUITE_P(
             "s3://my-bucket/my-key.txt",
             {
                 .bucket = "my-bucket",
-                .key = {"my-key.txt"},
+                .key = "my-key.txt",
             },
-            "basic_s3_bucket",
-        },
+            "basic_s3_bucket"},
         ParsedS3URLTestCase{
             "s3://prod-cache/nix/store/abc123.nar.xz?region=eu-west-1",
             {
                 .bucket = "prod-cache",
-                .key = {"nix", "store", "abc123.nar.xz"},
+                .key = "nix/store/abc123.nar.xz",
                 .region = "eu-west-1",
             },
-            "with_region",
-        },
+            "with_region"},
         ParsedS3URLTestCase{
             "s3://bucket/key?region=us-west-2&profile=prod&endpoint=custom.s3.com&scheme=https&region=us-east-1",
             {
                 .bucket = "bucket",
-                .key = {"key"},
+                .key = "key",
                 .profile = "prod",
                 .region = "us-west-2", //< using the first parameter (decodeQuery ignores dupicates)
                 .scheme = "https",
                 .endpoint = ParsedURL::Authority{.host = "custom.s3.com"},
             },
-            "complex",
-        },
+            "complex"},
         ParsedS3URLTestCase{
             "s3://cache/file.txt?profile=production&region=ap-southeast-2",
             {
                 .bucket = "cache",
-                .key = {"file.txt"},
+                .key = "file.txt",
                 .profile = "production",
                 .region = "ap-southeast-2",
             },
-            "with_profile_and_region",
-        },
+            "with_profile_and_region"},
         ParsedS3URLTestCase{
             "s3://bucket/key?endpoint=https://minio.local&scheme=http",
             {
                 .bucket = "bucket",
-                .key = {"key"},
+                .key = "key",
                 /* TODO: Figure out what AWS SDK is doing when both endpointOverride and scheme are set. */
                 .scheme = "http",
                 .endpoint =
                     ParsedURL{
                         .scheme = "https",
                         .authority = ParsedURL::Authority{.host = "minio.local"},
-                        .path = {""},
                     },
             },
-            "with_absolute_endpoint_uri",
-        }),
+            "with_absolute_endpoint_uri"}),
     [](const ::testing::TestParamInfo<ParsedS3URLTestCase> & info) { return info.param.description; });
 
 TEST(InvalidParsedS3URLTest, parseS3URLErrors)
@@ -92,114 +86,11 @@ TEST(InvalidParsedS3URLTest, parseS3URLErrors)
         testing::HasSubstrIgnoreANSIMatcher("error: URI has a missing or invalid bucket name"));
 
     /* Empty bucket (authority) */
-    ASSERT_THAT([]() { ParsedS3URL::parse(parseURL("s3:///key")); }, invalidBucketMatcher);
+    ASSERT_THAT([]() { ParsedS3URL::parse("s3:///key"); }, invalidBucketMatcher);
     /* Invalid bucket name */
-    ASSERT_THAT([]() { ParsedS3URL::parse(parseURL("s3://127.0.0.1")); }, invalidBucketMatcher);
+    ASSERT_THAT([]() { ParsedS3URL::parse("s3://127.0.0.1"); }, invalidBucketMatcher);
 }
 
-// Parameterized test for s3ToHttpsUrl conversion
-struct S3ToHttpsConversionTestCase
-{
-    ParsedS3URL input;
-    ParsedURL expected;
-    std::string expectedRendered;
-    std::string description;
-};
-
-class S3ToHttpsConversionTest : public ::testing::WithParamInterface<S3ToHttpsConversionTestCase>,
-                                public ::testing::Test
-{};
-
-TEST_P(S3ToHttpsConversionTest, ConvertsCorrectly)
-{
-    const auto & testCase = GetParam();
-    auto result = testCase.input.toHttpsUrl();
-    EXPECT_EQ(result, testCase.expected) << "Failed for: " << testCase.description;
-    EXPECT_EQ(result.to_string(), testCase.expectedRendered);
-}
-
-INSTANTIATE_TEST_SUITE_P(
-    S3ToHttpsConversion,
-    S3ToHttpsConversionTest,
-    ::testing::Values(
-        S3ToHttpsConversionTestCase{
-            ParsedS3URL{
-                .bucket = "my-bucket",
-                .key = {"my-key.txt"},
-            },
-            ParsedURL{
-                .scheme = "https",
-                .authority = ParsedURL::Authority{.host = "s3.us-east-1.amazonaws.com"},
-                .path = {"", "my-bucket", "my-key.txt"},
-            },
-            "https://s3.us-east-1.amazonaws.com/my-bucket/my-key.txt",
-            "basic_s3_default_region",
-        },
-        S3ToHttpsConversionTestCase{
-            ParsedS3URL{
-                .bucket = "prod-cache",
-                .key = {"nix", "store", "abc123.nar.xz"},
-                .region = "eu-west-1",
-            },
-            ParsedURL{
-                .scheme = "https",
-                .authority = ParsedURL::Authority{.host = "s3.eu-west-1.amazonaws.com"},
-                .path = {"", "prod-cache", "nix", "store", "abc123.nar.xz"},
-            },
-            "https://s3.eu-west-1.amazonaws.com/prod-cache/nix/store/abc123.nar.xz",
-            "with_eu_west_1_region",
-        },
-        S3ToHttpsConversionTestCase{
-            ParsedS3URL{
-                .bucket = "bucket",
-                .key = {"key"},
-                .scheme = "http",
-                .endpoint = ParsedURL::Authority{.host = "custom.s3.com"},
-            },
-            ParsedURL{
-                .scheme = "http",
-                .authority = ParsedURL::Authority{.host = "custom.s3.com"},
-                .path = {"", "bucket", "key"},
-            },
-            "http://custom.s3.com/bucket/key",
-            "custom_endpoint_authority",
-        },
-        S3ToHttpsConversionTestCase{
-            ParsedS3URL{
-                .bucket = "bucket",
-                .key = {"key"},
-                .endpoint =
-                    ParsedURL{
-                        .scheme = "http",
-                        .authority = ParsedURL::Authority{.host = "server", .port = 9000},
-                        .path = {""},
-                    },
-            },
-            ParsedURL{
-                .scheme = "http",
-                .authority = ParsedURL::Authority{.host = "server", .port = 9000},
-                .path = {"", "bucket", "key"},
-            },
-            "http://server:9000/bucket/key",
-            "custom_endpoint_with_port",
-        },
-        S3ToHttpsConversionTestCase{
-            ParsedS3URL{
-                .bucket = "bucket",
-                .key = {"path", "to", "file.txt"},
-                .region = "ap-southeast-2",
-                .scheme = "https",
-            },
-            ParsedURL{
-                .scheme = "https",
-                .authority = ParsedURL::Authority{.host = "s3.ap-southeast-2.amazonaws.com"},
-                .path = {"", "bucket", "path", "to", "file.txt"},
-            },
-            "https://s3.ap-southeast-2.amazonaws.com/bucket/path/to/file.txt",
-            "complex_path_and_region",
-        }),
-    [](const ::testing::TestParamInfo<S3ToHttpsConversionTestCase> & info) { return info.param.description; });
-
 } // namespace nix
 
 #endif

src/libstore-tests/uds-remote-store.cc

@@ -19,7 +19,7 @@ TEST(UDSRemoteStore, constructConfigWrongScheme)
 TEST(UDSRemoteStore, constructConfig_to_string)
 {
     UDSRemoteStoreConfig config{"unix", "", {}};
-    EXPECT_EQ(config.getReference().to_string(), "daemon");
+    EXPECT_EQ(config.getReference().render(), "daemon");
 }
 
 } // namespace nix

src/libstore/binary-cache-store.cc

@@ -125,8 +125,11 @@ void BinaryCacheStore::writeNarInfo(ref<NarInfo> narInfo)
 
     upsertFile(narInfoFile, narInfo->to_string(*this), "text/x-nix-narinfo");
 
-    pathInfoCache->lock()->upsert(
-        std::string(narInfo->path.to_string()), PathInfoCacheValue{.value = std::shared_ptr<NarInfo>(narInfo)});
+    {
+        auto state_(state.lock());
+        state_->pathInfoCache.upsert(
+            std::string(narInfo->path.to_string()), PathInfoCacheValue{.value = std::shared_ptr<NarInfo>(narInfo)});
+    }
 
     if (diskCache)
         diskCache->upsertNarInfo(

src/libstore/build/derivation-check.cc

@@ -1,190 +0,0 @@
-#include <queue>
-
-#include "nix/store/store-api.hh"
-#include "nix/store/build-result.hh"
-
-#include "derivation-check.hh"
-
-namespace nix {
-
-void checkOutputs(
-    Store & store,
-    const StorePath & drvPath,
-    const decltype(Derivation::outputs) & drvOutputs,
-    const decltype(DerivationOptions::outputChecks) & outputChecks,
-    const std::map<std::string, ValidPathInfo> & outputs)
-{
-    std::map<Path, const ValidPathInfo &> outputsByPath;
-    for (auto & output : outputs)
-        outputsByPath.emplace(store.printStorePath(output.second.path), output.second);
-
-    for (auto & [outputName, info] : outputs) {
-
-        auto * outputSpec = get(drvOutputs, outputName);
-        assert(outputSpec);
-
-        if (const auto * dof = std::get_if<DerivationOutput::CAFixed>(&outputSpec->raw)) {
-            auto & wanted = dof->ca.hash;
-
-            /* Check wanted hash */
-            assert(info.ca);
-            auto & got = info.ca->hash;
-            if (wanted != got) {
-                /* Throw an error after registering the path as
-                   valid. */
-                throw BuildError(
-                    BuildResult::HashMismatch,
-                    "hash mismatch in fixed-output derivation '%s':\n  specified: %s\n     got:    %s",
-                    store.printStorePath(drvPath),
-                    wanted.to_string(HashFormat::SRI, true),
-                    got.to_string(HashFormat::SRI, true));
-            }
-            if (!info.references.empty()) {
-                auto numViolations = info.references.size();
-                throw BuildError(
-                    BuildResult::HashMismatch,
-                    "fixed-output derivations must not reference store paths: '%s' references %d distinct paths, e.g. '%s'",
-                    store.printStorePath(drvPath),
-                    numViolations,
-                    store.printStorePath(*info.references.begin()));
-            }
-        }
-
-        /* Compute the closure and closure size of some output. This
-           is slightly tricky because some of its references (namely
-           other outputs) may not be valid yet. */
-        auto getClosure = [&](const StorePath & path) {
-            uint64_t closureSize = 0;
-            StorePathSet pathsDone;
-            std::queue<StorePath> pathsLeft;
-            pathsLeft.push(path);
-
-            while (!pathsLeft.empty()) {
-                auto path = pathsLeft.front();
-                pathsLeft.pop();
-                if (!pathsDone.insert(path).second)
-                    continue;
-
-                auto i = outputsByPath.find(store.printStorePath(path));
-                if (i != outputsByPath.end()) {
-                    closureSize += i->second.narSize;
-                    for (auto & ref : i->second.references)
-                        pathsLeft.push(ref);
-                } else {
-                    auto info = store.queryPathInfo(path);
-                    closureSize += info->narSize;
-                    for (auto & ref : info->references)
-                        pathsLeft.push(ref);
-                }
-            }
-
-            return std::make_pair(std::move(pathsDone), closureSize);
-        };
-
-        auto applyChecks = [&](const DerivationOptions::OutputChecks & checks) {
-            if (checks.maxSize && info.narSize > *checks.maxSize)
-                throw BuildError(
-                    BuildResult::OutputRejected,
-                    "path '%s' is too large at %d bytes; limit is %d bytes",
-                    store.printStorePath(info.path),
-                    info.narSize,
-                    *checks.maxSize);
-
-            if (checks.maxClosureSize) {
-                uint64_t closureSize = getClosure(info.path).second;
-                if (closureSize > *checks.maxClosureSize)
-                    throw BuildError(
-                        BuildResult::OutputRejected,
-                        "closure of path '%s' is too large at %d bytes; limit is %d bytes",
-                        store.printStorePath(info.path),
-                        closureSize,
-                        *checks.maxClosureSize);
-            }
-
-            auto checkRefs = [&](const StringSet & value, bool allowed, bool recursive) {
-                /* Parse a list of reference specifiers.  Each element must
-                   either be a store path, or the symbolic name of the output
-                   of the derivation (such as `out'). */
-                StorePathSet spec;
-                for (auto & i : value) {
-                    if (store.isStorePath(i))
-                        spec.insert(store.parseStorePath(i));
-                    else if (auto output = get(outputs, i))
-                        spec.insert(output->path);
-                    else {
-                        std::string outputsListing =
-                            concatMapStringsSep(", ", outputs, [](auto & o) { return o.first; });
-                        throw BuildError(
-                            BuildResult::OutputRejected,
-                            "derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
-                            " expected store path or output name (one of [%s])",
-                            store.printStorePath(drvPath),
-                            outputName,
-                            i,
-                            outputsListing);
-                    }
-                }
-
-                auto used = recursive ? getClosure(info.path).first : info.references;
-
-                if (recursive && checks.ignoreSelfRefs)
-                    used.erase(info.path);
-
-                StorePathSet badPaths;
-
-                for (auto & i : used)
-                    if (allowed) {
-                        if (!spec.count(i))
-                            badPaths.insert(i);
-                    } else {
-                        if (spec.count(i))
-                            badPaths.insert(i);
-                    }
-
-                if (!badPaths.empty()) {
-                    std::string badPathsStr;
-                    for (auto & i : badPaths) {
-                        badPathsStr += "\n  ";
-                        badPathsStr += store.printStorePath(i);
-                    }
-                    throw BuildError(
-                        BuildResult::OutputRejected,
-                        "output '%s' is not allowed to refer to the following paths:%s",
-                        store.printStorePath(info.path),
-                        badPathsStr);
-                }
-            };
-
-            /* Mandatory check: absent whitelist, and present but empty
-               whitelist mean very different things. */
-            if (auto & refs = checks.allowedReferences) {
-                checkRefs(*refs, true, false);
-            }
-            if (auto & refs = checks.allowedRequisites) {
-                checkRefs(*refs, true, true);
-            }
-
-            /* Optimization: don't need to do anything when
-               disallowed and empty set. */
-            if (!checks.disallowedReferences.empty()) {
-                checkRefs(checks.disallowedReferences, false, false);
-            }
-            if (!checks.disallowedRequisites.empty()) {
-                checkRefs(checks.disallowedRequisites, false, true);
-            }
-        };
-
-        std::visit(
-            overloaded{
-                [&](const DerivationOptions::OutputChecks & checks) { applyChecks(checks); },
-                [&](const std::map<std::string, DerivationOptions::OutputChecks> & checksPerOutput) {
-                    if (auto outputChecks = get(checksPerOutput, outputName))
-
-                        applyChecks(*outputChecks);
-                },
-            },
-            outputChecks);
-    }
-}
-
-} // namespace nix

src/libstore/build/derivation-check.hh

@@ -1,27 +0,0 @@
-#pragma once
-///@file
-
-#include "nix/store/derivations.hh"
-#include "nix/store/derivation-options.hh"
-#include "nix/store/path-info.hh"
-
-namespace nix {
-
-/**
- * Check that outputs meets the requirements specified by the
- * 'outputChecks' attribute (or the legacy
- * '{allowed,disallowed}{References,Requisites}' attributes).
- *
- * The outputs may not be valid yet, hence outputs needs to contain all
- * needed info like the NAR size. However, the external (not other
- * output) references of the output must be valid, so we can compute the
- * closure size.
- */
-void checkOutputs(
-    Store & store,
-    const StorePath & drvPath,
-    const decltype(Derivation::outputs) & drvOutputs,
-    const decltype(DerivationOptions::outputChecks) & drvOptions,
-    const std::map<std::string, ValidPathInfo> & outputs);
-
-} // namespace nix

src/libstore/build/derivation-env-desugar.cc

@@ -1,59 +0,0 @@
-#include "nix/store/build/derivation-env-desugar.hh"
-#include "nix/store/store-api.hh"
-#include "nix/store/derivations.hh"
-#include "nix/store/derivation-options.hh"
-
-namespace nix {
-
-std::string & DesugaredEnv::atFileEnvPair(std::string_view name, std::string fileName)
-{
-    auto & ret = extraFiles[fileName];
-    variables.insert_or_assign(
-        std::string{name},
-        EnvEntry{
-            .prependBuildDirectory = true,
-            .value = std::move(fileName),
-        });
-    return ret;
-}
-
-DesugaredEnv DesugaredEnv::create(
-    Store & store, const Derivation & drv, const DerivationOptions & drvOptions, const StorePathSet & inputPaths)
-{
-    DesugaredEnv res;
-
-    if (drv.structuredAttrs) {
-        auto json = drv.structuredAttrs->prepareStructuredAttrs(store, drvOptions, inputPaths, drv.outputs);
-        res.atFileEnvPair("NIX_ATTRS_SH_FILE", ".attrs.sh") = StructuredAttrs::writeShell(json);
-        res.atFileEnvPair("NIX_ATTRS_JSON_FILE", ".attrs.json") = json.dump();
-    } else {
-        /* In non-structured mode, set all bindings either directory in the
-           environment or via a file, as specified by
-           `DerivationOptions::passAsFile`. */
-        for (auto & [envName, envValue] : drv.env) {
-            if (!drvOptions.passAsFile.contains(envName)) {
-                res.variables.insert_or_assign(
-                    envName,
-                    EnvEntry{
-                        .value = envValue,
-                    });
-            } else {
-                res.atFileEnvPair(
-                    envName + "Path",
-                    ".attr-" + hashString(HashAlgorithm::SHA256, envName).to_string(HashFormat::Nix32, false)) =
-                    envValue;
-            }
-        }
-
-        /* Handle exportReferencesGraph(), if set. */
-        for (auto & [fileName, storePaths] : drvOptions.getParsedExportReferencesGraph(store)) {
-            /* Write closure info to <fileName>. */
-            res.extraFiles.insert_or_assign(
-                fileName, store.makeValidityRegistration(store.exportReferences(storePaths, inputPaths), false, false));
-        }
-    }
-
-    return res;
-}
-
-} // namespace nix

src/libstore/build/derivation-goal.cc

@@ -94,7 +94,7 @@ Goal::Co DerivationGoal::haveDerivation()
 
         /* If they are all valid, then we're done. */
         if (checkResult && checkResult->second == PathStatus::Valid && buildMode == bmNormal) {
-            co_return doneSuccess(BuildResult::AlreadyValid, checkResult->first);
+            co_return done(BuildResult::AlreadyValid, checkResult->first);
         }
 
         Goals waitees;
@@ -122,10 +122,12 @@ Goal::Co DerivationGoal::haveDerivation()
         assert(!drv->type().isImpure());
 
         if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) {
-            co_return doneFailure(BuildError(
+            co_return done(
                 BuildResult::TransientFailure,
-                "some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
-                worker.store.printStorePath(drvPath)));
+                {},
+                Error(
+                    "some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
+                    worker.store.printStorePath(drvPath)));
         }
 
         nrFailed = nrNoSubstituters = 0;
@@ -135,7 +137,7 @@ Goal::Co DerivationGoal::haveDerivation()
         bool allValid = checkResult && checkResult->second == PathStatus::Valid;
 
         if (buildMode == bmNormal && allValid) {
-            co_return doneSuccess(BuildResult::Substituted, checkResult->first);
+            co_return done(BuildResult::Substituted, checkResult->first);
         }
         if (buildMode == bmRepair && allValid) {
             co_return repairClosure();
@@ -279,7 +281,7 @@ Goal::Co DerivationGoal::repairClosure()
                 "some paths in the output closure of derivation '%s' could not be repaired",
                 worker.store.printStorePath(drvPath));
     }
-    co_return doneSuccess(BuildResult::AlreadyValid, assertPathValidity());
+    co_return done(BuildResult::AlreadyValid, assertPathValidity());
 }
 
 std::optional<std::pair<Realisation, PathStatus>> DerivationGoal::checkPathValidity()
@@ -337,27 +339,12 @@ Realisation DerivationGoal::assertPathValidity()
     return checkResult->first;
 }
 
-Goal::Done DerivationGoal::doneSuccess(BuildResult::Status status, Realisation builtOutput)
+Goal::Done
+DerivationGoal::done(BuildResult::Status status, std::optional<Realisation> builtOutput, std::optional<Error> ex)
 {
     buildResult.status = status;
-
-    assert(buildResult.success());
-
-    mcExpectedBuilds.reset();
-
-    buildResult.builtOutputs = {{wantedOutput, std::move(builtOutput)}};
-    if (status == BuildResult::Built)
-        worker.doneBuilds++;
-
-    worker.updateProgress();
-
-    return amDone(ecSuccess, std::nullopt);
-}
-
-Goal::Done DerivationGoal::doneFailure(BuildError ex)
-{
-    buildResult.status = ex.status;
-    buildResult.errorMsg = fmt("%s", Uncolored(ex.info().msg));
+    if (ex)
+        buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
     if (buildResult.status == BuildResult::TimedOut)
         worker.timedOut = true;
     if (buildResult.status == BuildResult::PermanentFailure)
@@ -365,12 +352,26 @@ Goal::Done DerivationGoal::doneFailure(BuildError ex)
 
     mcExpectedBuilds.reset();
 
-    if (ex.status != BuildResult::DependencyFailed)
-        worker.failedBuilds++;
+    if (buildResult.success()) {
+        assert(builtOutput);
+        buildResult.builtOutputs = {{wantedOutput, std::move(*builtOutput)}};
+        if (status == BuildResult::Built)
+            worker.doneBuilds++;
+    } else {
+        if (status != BuildResult::DependencyFailed)
+            worker.failedBuilds++;
+    }
 
     worker.updateProgress();
 
-    return amDone(ecFailed, {std::move(ex)});
+    auto traceBuiltOutputsFile = getEnv("_NIX_TRACE_BUILT_OUTPUTS").value_or("");
+    if (traceBuiltOutputsFile != "") {
+        std::fstream fs;
+        fs.open(traceBuiltOutputsFile, std::fstream::out);
+        fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl;
+    }
+
+    return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
 }
 
 } // namespace nix

src/libstore/builtins/fetchurl.cc

@@ -37,7 +37,7 @@ static void builtinFetchurl(const BuiltinBuilderContext & ctx)
 
     auto fetch = [&](const std::string & url) {
         auto source = sinkToSource([&](Sink & sink) {
-            FileTransferRequest request(ValidURL{url});
+            FileTransferRequest request(url);
             request.decompress = false;
 
             auto decompressor = makeDecompressionSink(unpack && hasSuffix(mainUrl, ".xz") ? "xz" : "none", sink);

src/libstore/daemon.cc

@@ -546,22 +546,47 @@ static void performOp(
         break;
     }
 
+    case WorkerProto::Op::ExportPath: {
+        auto path = store->parseStorePath(readString(conn.from));
+        readInt(conn.from); // obsolete
+        logger->startWork();
+        TunnelSink sink(conn.to);
+        store->exportPath(path, sink);
+        logger->stopWork();
+        conn.to << 1;
+        break;
+    }
+
+    case WorkerProto::Op::ImportPaths: {
+        logger->startWork();
+        TunnelSource source(conn.from, conn.to);
+        auto paths = store->importPaths(source, trusted ? NoCheckSigs : CheckSigs);
+        logger->stopWork();
+        Strings paths2;
+        for (auto & i : paths)
+            paths2.push_back(store->printStorePath(i));
+        conn.to << paths2;
+        break;
+    }
+
     case WorkerProto::Op::BuildPaths: {
         auto drvs = WorkerProto::Serialise<DerivedPaths>::read(*store, rconn);
         BuildMode mode = bmNormal;
-        mode = WorkerProto::Serialise<BuildMode>::read(*store, rconn);
+        if (GET_PROTOCOL_MINOR(conn.protoVersion) >= 15) {
+            mode = WorkerProto::Serialise<BuildMode>::read(*store, rconn);
 
-        /* Repairing is not atomic, so disallowed for "untrusted"
-           clients.
+            /* Repairing is not atomic, so disallowed for "untrusted"
+               clients.
 
-           FIXME: layer violation in this message: the daemon code (i.e.
-           this file) knows whether a client/connection is trusted, but it
-           does not how how the client was authenticated. The mechanism
-           need not be getting the UID of the other end of a Unix Domain
-           Socket.
-          */
-        if (mode == bmRepair && !trusted)
-            throw Error("repairing is not allowed because you are not in 'trusted-users'");
+               FIXME: layer violation in this message: the daemon code (i.e.
+               this file) knows whether a client/connection is trusted, but it
+               does not how how the client was authenticated. The mechanism
+               need not be getting the UID of the other end of a Unix Domain
+               Socket.
+              */
+            if (mode == bmRepair && !trusted)
+                throw Error("repairing is not allowed because you are not in 'trusted-users'");
+        }
         logger->startWork();
         store->buildPaths(drvs, mode);
         logger->stopWork();
@@ -780,11 +805,13 @@ static void performOp(
         clientSettings.buildCores = readInt(conn.from);
         clientSettings.useSubstitutes = readInt(conn.from);
 
-        unsigned int n = readInt(conn.from);
-        for (unsigned int i = 0; i < n; i++) {
-            auto name = readString(conn.from);
-            auto value = readString(conn.from);
-            clientSettings.overrides.emplace(name, value);
+        if (GET_PROTOCOL_MINOR(conn.protoVersion) >= 12) {
+            unsigned int n = readInt(conn.from);
+            for (unsigned int i = 0; i < n; i++) {
+                auto name = readString(conn.from);
+                auto value = readString(conn.from);
+                clientSettings.overrides.emplace(name, value);
+            }
         }
 
         logger->startWork();
@@ -849,12 +876,19 @@ static void performOp(
         auto path = store->parseStorePath(readString(conn.from));
         std::shared_ptr<const ValidPathInfo> info;
         logger->startWork();
-        info = store->queryPathInfo(path);
+        try {
+            info = store->queryPathInfo(path);
+        } catch (InvalidPath &) {
+            if (GET_PROTOCOL_MINOR(conn.protoVersion) < 17)
+                throw;
+        }
         logger->stopWork();
         if (info) {
-            conn.to << 1;
+            if (GET_PROTOCOL_MINOR(conn.protoVersion) >= 17)
+                conn.to << 1;
             WorkerProto::write(*store, wconn, static_cast<const UnkeyedValidPathInfo &>(*info));
         } else {
+            assert(GET_PROTOCOL_MINOR(conn.protoVersion) >= 17);
             conn.to << 0;
         }
         break;
@@ -1029,7 +1063,7 @@ void processConnection(ref<Store> store, FdSource && from, FdSink && to, Trusted
     auto [protoVersion, features] =
         WorkerProto::BasicServerConnection::handshake(to, from, PROTOCOL_VERSION, WorkerProto::allFeatures);
 
-    if (protoVersion < 256 + 18)
+    if (protoVersion < 0x10a)
         throw Error("the Nix client version is too old");
 
     WorkerProto::BasicServerConnection conn;

src/libstore/derivation-options.cc

@@ -265,8 +265,7 @@ DerivationOptions::getParsedExportReferencesGraph(const StoreDirConfig & store)
         StorePathSet storePaths;
         for (auto & storePathS : ss) {
             if (!store.isInStore(storePathS))
-                throw BuildError(
-                    BuildResult::InputRejected, "'exportReferencesGraph' contains a non-store path '%1%'", storePathS);
+                throw BuildError("'exportReferencesGraph' contains a non-store path '%1%'", storePathS);
             storePaths.insert(store.toStorePath(storePathS).first);
         }
         res.insert_or_assign(fileName, storePaths);

src/libstore/derivations.cc

@@ -498,33 +498,28 @@ Derivation parseDerivation(
  */
 static void printString(std::string & res, std::string_view s)
 {
-    res.reserve(res.size() + s.size() * 2 + 2);
-    res += '"';
-    static constexpr auto chunkSize = 1024;
-    std::array<char, 2 * chunkSize + 2> buffer;
-    while (!s.empty()) {
-        auto chunk = s.substr(0, /*n=*/chunkSize);
-        s.remove_prefix(chunk.size());
-        char * buf = buffer.data();
-        char * p = buf;
-        for (auto c : chunk)
-            if (c == '\"' || c == '\\') {
-                *p++ = '\\';
-                *p++ = c;
-            } else if (c == '\n') {
-                *p++ = '\\';
-                *p++ = 'n';
-            } else if (c == '\r') {
-                *p++ = '\\';
-                *p++ = 'r';
-            } else if (c == '\t') {
-                *p++ = '\\';
-                *p++ = 't';
-            } else
-                *p++ = c;
-        res.append(buf, p - buf);
-    }
-    res += '"';
+    boost::container::small_vector<char, 64 * 1024> buffer;
+    buffer.reserve(s.size() * 2 + 2);
+    char * buf = buffer.data();
+    char * p = buf;
+    *p++ = '"';
+    for (auto c : s)
+        if (c == '\"' || c == '\\') {
+            *p++ = '\\';
+            *p++ = c;
+        } else if (c == '\n') {
+            *p++ = '\\';
+            *p++ = 'n';
+        } else if (c == '\r') {
+            *p++ = '\\';
+            *p++ = 'r';
+        } else if (c == '\t') {
+            *p++ = '\\';
+            *p++ = 't';
+        } else
+            *p++ = c;
+    *p++ = '"';
+    res.append(buf, p - buf);
 }
 
 static void printUnquotedString(std::string & res, std::string_view s)

src/libstore/filetransfer.cc

@@ -100,7 +100,7 @@ struct curlFileTransfer : public FileTransfer
                   lvlTalkative,
                   actFileTransfer,
                   fmt("%sing '%s'", request.verb(), request.uri),
-                  {request.uri.to_string()},
+                  {request.uri},
                   request.parentAct)
             , callback(std::move(callback))
             , finalSink([this](std::string_view data) {
@@ -121,7 +121,7 @@ struct curlFileTransfer : public FileTransfer
                     this->result.data.append(data);
             })
         {
-            result.urls.push_back(request.uri.to_string());
+            result.urls.push_back(request.uri);
 
             requestHeaders = curl_slist_append(requestHeaders, "Accept-Encoding: zstd, br, gzip, deflate, bzip2, xz");
             if (!request.expectedETag.empty())
@@ -350,7 +350,7 @@ struct curlFileTransfer : public FileTransfer
                 curl_easy_setopt(req, CURLOPT_DEBUGFUNCTION, TransferItem::debugCallback);
             }
 
-            curl_easy_setopt(req, CURLOPT_URL, request.uri.to_string().c_str());
+            curl_easy_setopt(req, CURLOPT_URL, request.uri.c_str());
             curl_easy_setopt(req, CURLOPT_FOLLOWLOCATION, 1L);
             curl_easy_setopt(req, CURLOPT_MAXREDIRS, 10);
             curl_easy_setopt(req, CURLOPT_NOSIGNAL, 1);
@@ -784,8 +784,8 @@ struct curlFileTransfer : public FileTransfer
 
     void enqueueItem(std::shared_ptr<TransferItem> item)
     {
-        if (item->request.data && item->request.uri.scheme() != "http" && item->request.uri.scheme() != "https")
-            throw nix::Error("uploading to '%s' is not supported", item->request.uri.to_string());
+        if (item->request.data && !hasPrefix(item->request.uri, "http://") && !hasPrefix(item->request.uri, "https://"))
+            throw nix::Error("uploading to '%s' is not supported", item->request.uri);
 
         {
             auto state(state_.lock());
@@ -801,11 +801,11 @@ struct curlFileTransfer : public FileTransfer
     void enqueueFileTransfer(const FileTransferRequest & request, Callback<FileTransferResult> callback) override
     {
         /* Ugly hack to support s3:// URIs. */
-        if (request.uri.scheme() == "s3") {
+        if (hasPrefix(request.uri, "s3://")) {
             // FIXME: do this on a worker thread
             try {
 #if NIX_WITH_S3_SUPPORT
-                auto parsed = ParsedS3URL::parse(request.uri.parsed());
+                auto parsed = ParsedS3URL::parse(request.uri);
 
                 std::string profile = parsed.profile.value_or("");
                 std::string region = parsed.region.value_or(Aws::Region::US_EAST_1);
@@ -815,16 +815,15 @@ struct curlFileTransfer : public FileTransfer
                 S3Helper s3Helper(profile, region, scheme, endpoint);
 
                 // FIXME: implement ETag
-                auto s3Res = s3Helper.getObject(parsed.bucket, encodeUrlPath(parsed.key));
+                auto s3Res = s3Helper.getObject(parsed.bucket, parsed.key);
                 FileTransferResult res;
                 if (!s3Res.data)
                     throw FileTransferError(NotFound, {}, "S3 object '%s' does not exist", request.uri);
                 res.data = std::move(*s3Res.data);
-                res.urls.push_back(request.uri.to_string());
+                res.urls.push_back(request.uri);
                 callback(std::move(res));
 #else
-                throw nix::Error(
-                    "cannot download '%s' because Nix is not built with S3 support", request.uri.to_string());
+                throw nix::Error("cannot download '%s' because Nix is not built with S3 support", request.uri);
 #endif
             } catch (...) {
                 callback.rethrow();

src/libstore/gc.cc

@@ -931,7 +931,7 @@ void LocalStore::autoGC(bool sync)
     std::shared_future<void> future;
 
     {
-        auto state(_state->lock());
+        auto state(_state.lock());
 
         if (state->gcRunning) {
             future = state->gcFuture;
@@ -964,7 +964,7 @@ void LocalStore::autoGC(bool sync)
 
                 /* Wake up any threads waiting for the auto-GC to finish. */
                 Finally wakeup([&]() {
-                    auto state(_state->lock());
+                    auto state(_state.lock());
                     state->gcRunning = false;
                     state->lastGCCheck = std::chrono::steady_clock::now();
                     promise.set_value();
@@ -979,7 +979,7 @@ void LocalStore::autoGC(bool sync)
 
                 collectGarbage(options, results);
 
-                _state->lock()->availAfterGC = getAvail();
+                _state.lock()->availAfterGC = getAvail();
 
             } catch (...) {
                 // FIXME: we could propagate the exception to the

src/libstore/http-binary-cache-store.cc

@@ -27,7 +27,7 @@ HttpBinaryCacheStoreConfig::HttpBinaryCacheStoreConfig(
           + (!_cacheUri.empty() ? _cacheUri
                                 : throw UsageError("`%s` Store requires a non-empty authority in Store URL", scheme))))
 {
-    while (!cacheUri.path.empty() && cacheUri.path.back() == "")
+    while (!cacheUri.path.empty() && cacheUri.path.back() == '/')
         cacheUri.path.pop_back();
 }
 
@@ -37,7 +37,7 @@ StoreReference HttpBinaryCacheStoreConfig::getReference() const
         .variant =
             StoreReference::Specified{
                 .scheme = cacheUri.scheme,
-                .authority = cacheUri.renderAuthorityAndPath(),
+                .authority = (cacheUri.authority ? cacheUri.authority->to_string() : "") + cacheUri.path,
             },
         .params = cacheUri.query,
     };
@@ -154,17 +154,22 @@ protected:
 
     FileTransferRequest makeRequest(const std::string & path)
     {
-        /* Otherwise the last path fragment will get discarded. */
-        auto cacheUriWithTrailingSlash = config->cacheUri;
-        if (!cacheUriWithTrailingSlash.path.empty())
-            cacheUriWithTrailingSlash.path.push_back("");
-
-        /* path is not a path, but a full relative or absolute
+        /* FIXME path is not a path, but a full relative or absolute
            URL, e.g. we've seen in the wild NARINFO files have a URL
            field which is
            `nar/15f99rdaf26k39knmzry4xd0d97wp6yfpnfk1z9avakis7ipb9yg.nar?hash=zphkqn2wg8mnvbkixnl2aadkbn0rcnfj`
-           (note the query param) and that gets passed here. */
-        return FileTransferRequest(parseURLRelative(path, cacheUriWithTrailingSlash));
+           (note the query param) and that gets passed here.
+
+           What should actually happen is that we have two parsed URLs
+           (if we support relative URLs), and then we combined them with
+           a URL `operator/` which would be like
+           `std::filesystem::path`'s equivalent operator, which properly
+           combines the the URLs, whether the right is relative or
+           absolute. */
+        return FileTransferRequest(
+            hasPrefix(path, "https://") || hasPrefix(path, "http://") || hasPrefix(path, "file://")
+                ? path
+                : config->cacheUri.to_string() + "/" + path);
     }
 
     void getFile(const std::string & path, Sink & sink) override

src/libstore/include/nix/store/build-result.hh

@@ -1,13 +1,13 @@
 #pragma once
 ///@file
 
+#include "nix/store/realisation.hh"
+#include "nix/store/derived-path.hh"
+
 #include <string>
 #include <chrono>
 #include <optional>
 
-#include "nix/store/derived-path.hh"
-#include "nix/store/realisation.hh"
-
 namespace nix {
 
 struct BuildResult
@@ -36,10 +36,6 @@ struct BuildResult
         NotDeterministic,
         ResolvesToAlreadyValid,
         NoSubstituters,
-        /// A certain type of `OutputRejected`. The protocols do not yet
-        /// know about this one, so change it back to `OutputRejected`
-        /// before serialization.
-        HashMismatch,
     } status = MiscFailure;
 
     /**
@@ -50,6 +46,47 @@ struct BuildResult
      */
     std::string errorMsg;
 
+    std::string toString() const
+    {
+        auto strStatus = [&]() {
+            switch (status) {
+            case Built:
+                return "Built";
+            case Substituted:
+                return "Substituted";
+            case AlreadyValid:
+                return "AlreadyValid";
+            case PermanentFailure:
+                return "PermanentFailure";
+            case InputRejected:
+                return "InputRejected";
+            case OutputRejected:
+                return "OutputRejected";
+            case TransientFailure:
+                return "TransientFailure";
+            case CachedFailure:
+                return "CachedFailure";
+            case TimedOut:
+                return "TimedOut";
+            case MiscFailure:
+                return "MiscFailure";
+            case DependencyFailed:
+                return "DependencyFailed";
+            case LogLimitExceeded:
+                return "LogLimitExceeded";
+            case NotDeterministic:
+                return "NotDeterministic";
+            case ResolvesToAlreadyValid:
+                return "ResolvesToAlreadyValid";
+            case NoSubstituters:
+                return "NoSubstituters";
+            default:
+                return "Unknown";
+            };
+        }();
+        return strStatus + ((errorMsg == "") ? "" : " : " + errorMsg);
+    }
+
     /**
      * How many times this build was performed.
      */
@@ -94,26 +131,6 @@ struct BuildResult
     }
 };
 
-/**
- * denotes a permanent build failure
- */
-struct BuildError : public Error
-{
-    BuildResult::Status status;
-
-    BuildError(BuildResult::Status status, BuildError && error)
-        : Error{std::move(error)}
-        , status{status}
-    {
-    }
-
-    BuildError(BuildResult::Status status, auto &&... args)
-        : Error{args...}
-        , status{status}
-    {
-    }
-};
-
 /**
  * A `BuildResult` together with its "primary key".
  */

src/libstore/include/nix/store/build/derivation-builder.hh

@@ -8,33 +8,9 @@
 #include "nix/store/parsed-derivations.hh"
 #include "nix/util/processes.hh"
 #include "nix/store/restricted-store.hh"
-#include "nix/store/build/derivation-env-desugar.hh"
 
 namespace nix {
 
-/**
- * Denotes a build failure that stemmed from the builder exiting with a
- * failing exist status.
- */
-struct BuilderFailureError : BuildError
-{
-    int builderStatus;
-
-    std::string extraMsgAfter;
-
-    BuilderFailureError(BuildResult::Status status, int builderStatus, std::string extraMsgAfter)
-        : BuildError{
-            status,
-              /* No message for now, because the caller will make for
-                 us, with extra context */
-              "",
-          }
-        , builderStatus{std::move(builderStatus)}
-        , extraMsgAfter{std::move(extraMsgAfter)}
-    {
-    }
-};
-
 /**
  * Stuff we need to pass to initChild().
  */
@@ -76,7 +52,10 @@ struct DerivationBuilderParams
      */
     const StorePathSet & inputPaths;
 
-    const std::map<std::string, InitialOutput> & initialOutputs;
+    /**
+     * @note we do in fact mutate this
+     */
+    std::map<std::string, InitialOutput> & initialOutputs;
 
     const BuildMode & buildMode;
 
@@ -86,15 +65,60 @@ struct DerivationBuilderParams
      */
     PathsInChroot defaultPathsInChroot;
 
-    /**
-     * May be used to control various platform-specific functionality.
-     *
-     * For example, on Linux, the `kvm` system feature controls whether
-     * `/dev/kvm` should be exposed to the builder within the sandbox.
-     */
-    StringSet systemFeatures;
+    struct EnvEntry
+    {
+        /**
+         * Actually, this should be passed as a file, but with a custom
+         * name (rather than hash-derived name for usual "pass as file").
+         */
+        std::optional<std::string> nameOfPassAsFile;
 
-    DesugaredEnv desugaredEnv;
+        /**
+         * String value of env var, or contents of the file
+         */
+        std::string value;
+    };
+
+    /**
+     * The final environment variables to additionally set, possibly
+     * indirectly via a file.
+     *
+     * This is used by the caller to desugar the "structured attrs"
+     * mechanism, so `DerivationBuilder` doesn't need to know about it.
+     */
+    std::map<std::string, EnvEntry, std::less<>> finalEnv;
+
+    /**
+     * Inserted in the temp dir, but no file names placed in env, unlike
+     * `EnvEntry::nameOfPassAsFile` above.
+     */
+    StringMap extraFiles;
+
+    DerivationBuilderParams(
+        const StorePath & drvPath,
+        const BuildMode & buildMode,
+        BuildResult & buildResult,
+        const Derivation & drv,
+        const DerivationOptions & drvOptions,
+        const StorePathSet & inputPaths,
+        std::map<std::string, InitialOutput> & initialOutputs,
+        PathsInChroot defaultPathsInChroot,
+        std::map<std::string, EnvEntry, std::less<>> finalEnv,
+        StringMap extraFiles)
+        : drvPath{drvPath}
+        , buildResult{buildResult}
+        , drv{drv}
+        , drvOptions{drvOptions}
+        , inputPaths{inputPaths}
+        , initialOutputs{initialOutputs}
+        , buildMode{buildMode}
+        , defaultPathsInChroot{std::move(defaultPathsInChroot)}
+        , finalEnv{std::move(finalEnv)}
+        , extraFiles{std::move(extraFiles)}
+    {
+    }
+
+    DerivationBuilderParams(DerivationBuilderParams &&) = default;
 };
 
 /**
@@ -114,10 +138,24 @@ struct DerivationBuilderCallbacks
      */
     virtual void closeLogFile() = 0;
 
+    virtual void appendLogTailErrorMsg(std::string & msg) = 0;
+
+    /**
+     * Hook up `builderOut` to some mechanism to ingest the log
+     *
+     * @todo this should be reworked
+     */
+    virtual void childStarted(Descriptor builderOut) = 0;
+
     /**
      * @todo this should be reworked
      */
     virtual void childTerminated() = 0;
+
+    virtual void noteHashMismatch(void) = 0;
+    virtual void noteCheckMismatch(void) = 0;
+
+    virtual void markContentsGood(const StorePath & path) = 0;
 };
 
 /**
@@ -133,6 +171,11 @@ struct DerivationBuilderCallbacks
  */
 struct DerivationBuilder : RestrictionContext
 {
+    /**
+     * The process ID of the builder.
+     */
+    Pid pid;
+
     DerivationBuilder() = default;
     virtual ~DerivationBuilder() = default;
 
@@ -147,15 +190,15 @@ struct DerivationBuilder : RestrictionContext
      * locks as needed). After this is run, the builder should be
      * started.
      *
-     * @returns logging pipe if successful, `std::nullopt` if we could
-     * not acquire a build user. In that case, the caller must wait and
-     * then try again.
-     *
-     * @note "success" just means that we were able to set up the environment
-     * and start the build. The builder could have immediately exited with
-     * failure, and that would still be considered a successful start.
+     * @returns true if successful, false if we could not acquire a build
+     * user. In that case, the caller must wait and then try again.
      */
-    virtual std::optional<Descriptor> startBuild() = 0;
+    virtual bool prepareBuild() = 0;
+
+    /**
+     * Start building a derivation.
+     */
+    virtual void startBuilder() = 0;
 
     /**
      * Tear down build environment after the builder exits (either on
@@ -165,18 +208,25 @@ struct DerivationBuilder : RestrictionContext
      * processing. A status code and exception are returned, providing
      * more information. The second case indicates success, and
      * realisations for each output of the derivation are returned.
-     *
-     * @throws BuildError
      */
-    virtual SingleDrvOutputs unprepareBuild() = 0;
+    virtual std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() = 0;
 
     /**
-     * Forcibly kill the child process, if any.
-     *
-     * @returns whether the child was still alive and needed to be
-     * killed.
+     * Stop the in-process nix daemon thread.
+     * @see startDaemon
      */
-    virtual bool killChild() = 0;
+    virtual void stopDaemon() = 0;
+
+    /**
+     * Delete the temporary directory, if we have one.
+     */
+    virtual void deleteTmpDir(bool force) = 0;
+
+    /**
+     * Kill any processes running under the build user UID or in the
+     * cgroup of the build.
+     */
+    virtual void killSandbox(bool getStats) = 0;
 };
 
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows

src/libstore/include/nix/store/build/derivation-building-goal.hh

@@ -14,7 +14,6 @@ namespace nix {
 
 using std::map;
 
-struct BuilderFailureError;
 #ifndef _WIN32 // TODO enable build hook on Windows
 struct HookInstance;
 struct DerivationBuilder;
@@ -29,12 +28,6 @@ typedef enum { rpAccept, rpDecline, rpPostpone } HookReply;
  */
 struct DerivationBuildingGoal : public Goal
 {
-    DerivationBuildingGoal(
-        const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal);
-    ~DerivationBuildingGoal();
-
-private:
-
     /** The path of the derivation. */
     StorePath drvPath;
 
@@ -49,12 +42,19 @@ private:
      * The remainder is state held during the build.
      */
 
+    /**
+     * Locks on (fixed) output paths.
+     */
+    PathLocks outputLocks;
+
     /**
      * All input paths (that is, the union of FS closures of the
      * immediate input paths).
      */
     StorePathSet inputPaths;
 
+    std::map<std::string, InitialOutput> initialOutputs;
+
     /**
      * File descriptor for the log file.
      */
@@ -91,8 +91,22 @@ private:
 
     std::unique_ptr<Activity> act;
 
+    /**
+     * Activity that denotes waiting for a lock.
+     */
+    std::unique_ptr<Activity> actLock;
+
     std::map<ActivityId, Activity> builderActivities;
 
+    /**
+     * The remote machine on which we're building.
+     */
+    std::string machineName;
+
+    DerivationBuildingGoal(
+        const StorePath & drvPath, const Derivation & drv, Worker & worker, BuildMode buildMode = bmNormal);
+    ~DerivationBuildingGoal();
+
     void timedOut(Error && ex) override;
 
     std::string key() override;
@@ -102,11 +116,12 @@ private:
      */
     Co gaveUpOnSubstitution();
     Co tryToBuild();
+    Co hookDone();
 
     /**
      * Is the build hook willing to perform the build?
      */
-    HookReply tryBuildHook(const std::map<std::string, InitialOutput> & initialOutputs);
+    HookReply tryBuildHook();
 
     /**
      * Open a log file and a pipe to it.
@@ -140,18 +155,24 @@ private:
      * whether all outputs are valid and non-corrupt, and a
      * 'SingleDrvOutputs' structure containing the valid outputs.
      */
-    std::pair<bool, SingleDrvOutputs> checkPathValidity(std::map<std::string, InitialOutput> & initialOutputs);
+    std::pair<bool, SingleDrvOutputs> checkPathValidity();
+
+    /**
+     * Aborts if any output is not valid or corrupt, and otherwise
+     * returns a 'SingleDrvOutputs' structure containing all outputs.
+     */
+    SingleDrvOutputs assertPathValidity();
 
     /**
      * Forcibly kill the child process, if any.
      */
     void killChild();
 
-    Done doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs);
+    void started();
 
-    Done doneFailure(BuildError ex);
+    Done done(BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {});
 
-    BuildError fixupBuilderFailureErrorMessage(BuilderFailureError msg);
+    void appendLogTailErrorMsg(std::string & msg);
 
     JobCategory jobCategory() const override
     {

src/libstore/include/nix/store/build/derivation-building-misc.hh

@@ -49,6 +49,9 @@ struct InitialOutput
     std::optional<InitialOutputStatus> known;
 };
 
+void runPostBuildHook(
+    const StoreDirConfig & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths);
+
 /**
  * Format the known outputs of a derivation for use in error messages.
  */

src/libstore/include/nix/store/build/derivation-env-desugar.hh

@@ -1,83 +0,0 @@
-#pragma once
-///@file
-
-#include "nix/util/types.hh"
-#include "nix/store/path.hh"
-
-namespace nix {
-
-class Store;
-struct Derivation;
-struct DerivationOptions;
-
-/**
- * Derivations claim to "just" specify their environment variables, but
- * actually do a number of different features, such as "structured
- * attrs", "pass as file", and "export references graph", things are
- * more complicated then they appear.
- *
- * The good news is that we can simplify all that to the following view,
- * where environment variables and extra files are specified exactly,
- * with no special cases.
- *
- * Because we have `DesugaredEnv`, `DerivationBuilder` doesn't need to
- * know about any of those above features, and their special case.
- */
-struct DesugaredEnv
-{
-    struct EnvEntry
-    {
-        /**
-         * Whether to prepend the (inside via) path to the sandbox build
-         * directory to `value`. This is useful for when the env var
-         * should point to a file visible to the builder.
-         */
-        bool prependBuildDirectory = false;
-
-        /**
-         * String value of env var, or contents of the file.
-         */
-        std::string value;
-    };
-
-    /**
-     * The final environment variables to set.
-     */
-    std::map<std::string, EnvEntry, std::less<>> variables;
-
-    /**
-     * Extra file to be placed in the build directory.
-     *
-     * @note `EnvEntry::prependBuildDirectory` can be used to refer to
-     * those files without knowing what the build directory is.
-     */
-    StringMap extraFiles;
-
-    /**
-     * A common case is to define an environment variable that points to
-     * a file, which contains some contents.
-     *
-     * In base:
-     * ```
-     * export VAR=FILE_NAME
-     * echo CONTENTS >FILE_NAME
-     * ```
-     *
-     * This function assists in doing both parts, so the file name is
-     * kept in sync.
-     */
-    std::string & atFileEnvPair(std::string_view name, std::string fileName);
-
-    /**
-     * Given a (resolved) derivation, its options, and the closure of
-     * its inputs (which we can get since the derivation is resolved),
-     * desugar the environment to create a `DesguaredEnv`.
-     *
-     * @todo drvOptions will go away as a separate argument when it is
-     * just part of `Derivation`.
-     */
-    static DesugaredEnv create(
-        Store & store, const Derivation & drv, const DerivationOptions & drvOptions, const StorePathSet & inputPaths);
-};
-
-} // namespace nix

src/libstore/include/nix/store/build/derivation-goal.hh

@@ -14,6 +14,9 @@ namespace nix {
 
 using std::map;
 
+/** Used internally */
+void runPostBuildHook(Store & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths);
+
 /**
  * A goal for realising a single output of a derivation. Various sorts of
  * fetching (which will be done by other goal types) is tried, and if none of
@@ -99,9 +102,13 @@ private:
 
     Co repairClosure();
 
-    Done doneSuccess(BuildResult::Status status, Realisation builtOutput);
-
-    Done doneFailure(BuildError ex);
+    /**
+     * @param builtOutput Must be set if `status` is successful.
+     */
+    Done done(
+        BuildResult::Status status,
+        std::optional<Realisation> builtOutput = std::nullopt,
+        std::optional<Error> ex = {});
 };
 
 } // namespace nix

src/libstore/include/nix/store/filetransfer.hh

@@ -9,7 +9,6 @@
 #include "nix/util/ref.hh"
 #include "nix/util/configuration.hh"
 #include "nix/util/serialise.hh"
-#include "nix/util/url.hh"
 
 namespace nix {
 
@@ -31,9 +30,17 @@ struct FileTransferSettings : Config
         )",
         {"binary-caches-parallel-connections"}};
 
+    /* Do not set this too low. On glibc, getaddrinfo() contains fallback code
+       paths that deal with ill-behaved DNS servers. Setting this too low
+       prevents some fallbacks from occurring.
+
+       See description of options timeout, single-request, single-request-reopen
+       in resolv.conf(5). Also see https://github.com/NixOS/nix/pull/13985 for
+       details on the interaction between getaddrinfo(3) behavior and libcurl
+       CURLOPT_CONNECTTIMEOUT. */
     Setting<unsigned long> connectTimeout{
         this,
-        5,
+        15,
         "connect-timeout",
         R"(
           The timeout (in seconds) for establishing connections in the
@@ -71,7 +78,7 @@ extern const unsigned int RETRY_TIME_MS_DEFAULT;
 
 struct FileTransferRequest
 {
-    ValidURL uri;
+    std::string uri;
     Headers headers;
     std::string expectedETag;
     bool verifyTLS = true;
@@ -85,8 +92,8 @@ struct FileTransferRequest
     std::string mimeType;
     std::function<void(std::string_view data)> dataCallback;
 
-    FileTransferRequest(ValidURL uri)
-        : uri(std::move(uri))
+    FileTransferRequest(std::string_view uri)
+        : uri(uri)
         , parentAct(getCurActivity())
     {
     }
@@ -112,9 +119,6 @@ struct FileTransferResult
 
     /**
      * All URLs visited in the redirect chain.
-     *
-     * @note Intentionally strings and not `ParsedURL`s so we faithfully
-     * return what cURL gave us.
      */
     std::vector<std::string> urls;
 

src/libstore/include/nix/store/http-binary-cache-store.hh

@@ -1,6 +1,3 @@
-#pragma once
-///@file
-
 #include "nix/util/url.hh"
 #include "nix/store/binary-cache-store.hh"
 

src/libstore/include/nix/store/legacy-ssh-store.hh

@@ -179,6 +179,12 @@ public:
      */
     StorePathSet queryValidPaths(const StorePathSet & paths, bool lock, SubstituteFlag maybeSubstitute = NoSubstitute);
 
+    /**
+     * Just exists because this is exactly what Hydra was doing, and we
+     * don't yet want an algorithmic change.
+     */
+    void addMultipleToStoreLegacy(Store & srcStore, const StorePathSet & paths);
+
     void connect() override;
 
     unsigned int getProtocol() override;

src/libstore/include/nix/store/local-store.hh

@@ -174,11 +174,7 @@ private:
         std::unique_ptr<PublicKeys> publicKeys;
     };
 
-    /**
-     * Mutable state. It's behind a `ref` to reduce false sharing
-     * between immutable and mutable fields.
-     */
-    ref<Sync<State>> _state;
+    Sync<State> _state;
 
 public:
 

src/libstore/include/nix/store/meson.build

@@ -15,7 +15,6 @@ headers = [ config_pub_h ] + files(
   'build/derivation-builder.hh',
   'build/derivation-building-goal.hh',
   'build/derivation-building-misc.hh',
-  'build/derivation-env-desugar.hh',
   'build/derivation-goal.hh',
   'build/derivation-trampoline-goal.hh',
   'build/drv-output-substitution-goal.hh',

src/libstore/include/nix/store/s3.hh

@@ -54,12 +54,7 @@ struct S3Helper
 struct ParsedS3URL
 {
     std::string bucket;
-    /**
-     * @see ParsedURL::path. This is a vector for the same reason.
-     * Unlike ParsedURL::path this doesn't include the leading empty segment,
-     * since the bucket name is necessary.
-     */
-    std::vector<std::string> key;
+    std::string key;
     std::optional<std::string> profile;
     std::optional<std::string> region;
     std::optional<std::string> scheme;
@@ -79,13 +74,7 @@ struct ParsedS3URL
             endpoint);
     }
 
-    static ParsedS3URL parse(const ParsedURL & uri);
-
-    /**
-     * Convert this ParsedS3URL to HTTPS ParsedURL for use with curl's AWS SigV4 authentication
-     */
-    ParsedURL toHttpsUrl() const;
-
+    static ParsedS3URL parse(std::string_view uri);
     auto operator<=>(const ParsedS3URL & other) const = default;
 };
 

src/libstore/include/nix/store/ssh.hh

@@ -1,7 +1,6 @@
 #pragma once
 ///@file
 
-#include "nix/util/ref.hh"
 #include "nix/util/sync.hh"
 #include "nix/util/url.hh"
 #include "nix/util/processes.hh"
@@ -27,13 +26,12 @@ private:
     const bool compress;
     const Descriptor logFD;
 
-    const ref<const AutoDelete> tmpDir;
-
     struct State
     {
 #ifndef _WIN32 // TODO re-enable on Windows, once we can start processes.
         Pid sshMaster;
 #endif
+        std::unique_ptr<AutoDelete> tmpDir;
         Path socketPath;
     };