Improve Git URI handling

Git URI can also support scp style links similar to git itself.

This change augments the function fixGitURL to better handle the scp
style urls through a minimal parser rather than regex which has been
found to be brittle.

* Support for IPV6 added
* New test cases added for fixGitURL
* Clearer documentation on purpose and goal of function
* More `std::string_view` for performance
* Update URL tests

Fixes #5958

Mostly undoes revert 4757487110599bbe9a287ead75741bba5436d52f
Adapted from commit 04ad66af5f

.mergify.yml

@@ -161,3 +161,14 @@ 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.31.0
+2.32.0

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]$ mesonFlags+=" --prefix=$(pwd)/outputs/out"
+[nix-shell]$ out="$(pwd)/outputs/out" dev=$out debug=$out mesonFlags+=" --prefix=${out}"
 [nix-shell]$ dontAddPrefix=1 configurePhase
 [nix-shell]$ buildPhase
 ```

docker.nix

@@ -281,7 +281,10 @@ 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.lock

@@ -63,11 +63,11 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1755442223,
-        "narHash": "sha256-VtMQg02B3kt1oejwwrGn50U9Xbjgzfbb5TV5Wtx8dKI=",
+        "lastModified": 1756178832,
+        "narHash": "sha256-O2CIn7HjZwEGqBrwu9EU76zlmA5dbmna7jL1XUmAId8=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "cd32a774ac52caaa03bcfc9e7591ac8c18617ced",
+        "rev": "d98ce345cdab58477ca61855540999c86577d19d",
         "type": "github"
       },
       "original": {

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 14:00-16:00 Europe/Amsterdam see [calendar](https://calendar.google.com/calendar/u/0/embed?src=b9o52fobqjak8oq8lfkhg3t0qg@group.calendar.google.com).
+- 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).
 
   1. Code review on pull requests from [In review](#in-review).
   2. Other chores and tasks.

maintainers/release-notes-todo

@@ -0,0 +1,58 @@
+#!/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,6 +24,12 @@ 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
@@ -127,6 +133,8 @@ release:
 
   Commit and push this to the maintenance branch.
 
+* Create a backport label.
+
 * Bump the version of `master`:
 
   ```console
@@ -134,6 +142,7 @@ 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
@@ -141,10 +150,6 @@ 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,6 +76,16 @@ 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->sOutputSpecified)) {
-    } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
+    if (attr->maybeGetAttr(state->s.outputSpecified)) {
+    } else if (auto aMeta = attr->maybeGetAttr(state->s.meta)) {
         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->sOutputSpecified)) {
+                            if (auto aOutputSpecified = attr->maybeGetAttr(state->s.outputSpecified)) {
                                 if (aOutputSpecified->getBool()) {
                                     if (auto aOutputName = attr->maybeGetAttr("outputName"))
                                         outputsToInstall = {aOutputName->getString()};
                                 }
-                            } else if (auto aMeta = attr->maybeGetAttr(state->sMeta)) {
+                            } else if (auto aMeta = attr->maybeGetAttr(state->s.meta)) {
                                 if (auto aOutputsToInstall = aMeta->maybeGetAttr("outputsToInstall"))
                                     for (auto & s : aOutputsToInstall->getListOfStrings())
                                         outputsToInstall.insert(s);

src/libexpr-tests/value/print.cc

@@ -393,7 +393,7 @@ TEST_F(ValuePrintingTests, ansiColorsDerivation)
     vDerivation.mkString("derivation");
 
     BindingsBuilder builder(state, state.allocBindings(10));
-    builder.insert(state.sType, &vDerivation);
+    builder.insert(state.s.type, &vDerivation);
 
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());
@@ -438,8 +438,8 @@ TEST_F(ValuePrintingTests, ansiColorsDerivationError)
     vDerivation.mkString("derivation");
 
     BindingsBuilder builder(state, state.allocBindings(10));
-    builder.insert(state.sType, &vDerivation);
-    builder.insert(state.sDrvPath, &vError);
+    builder.insert(state.s.type, &vDerivation);
+    builder.insert(state.s.drvPath, &vError);
 
     Value vAttrs;
     vAttrs.mkAttrs(builder.finish());

src/libexpr/eval-cache.cc

@@ -330,7 +330,7 @@ AttrCursor::AttrCursor(
 AttrKey AttrCursor::getKey()
 {
     if (!parent)
-        return {0, root->state.sEpsilon};
+        return {0, root->state.s.epsilon};
     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.sDrvPath);
+    auto aDrvPath = getAttr(root->state.s.drvPath);
     auto drvPath = root->state.store->parseStorePath(aDrvPath->getString());
     drvPath.requireDerivation();
     if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) {

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.sName, attrs, "");
+                auto nameAttr = state.getAttr(state.s.name, 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.sFunctor);
+        const auto functor = v.attrs()->get(state.s.functor);
         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

@@ -203,124 +203,65 @@ EvalState::EvalState(
     std::shared_ptr<Store> buildStore)
     : fetchSettings{fetchSettings}
     , settings{settings}
-    , 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"),
-    }
+    , symbols(StaticEvalSymbols::staticSymbolTable())
     , repair(NoRepair)
     , 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.
+    , 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())
@@ -654,7 +595,7 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
     }
     if (isFunctor(v)) {
         try {
-            Value & functor = *v.attrs()->find(sFunctor)->value;
+            Value & functor = *v.attrs()->find(s.functor)->value;
             Value * vp[] = {&v};
             Value partiallyApplied;
             // The first parameter is not user-provided, and may be
@@ -978,8 +919,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(sFile).mkString(path->path.abs());
-        makePositionThunks(*this, p, attrs.alloc(sLine), attrs.alloc(sColumn));
+        attrs.alloc(s.file).mkString(path->path.abs());
+        makePositionThunks(*this, p, attrs.alloc(s.line), attrs.alloc(s.column));
         v.mkAttrs(attrs);
     } else
         v.mkNull();
@@ -1245,7 +1186,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.sOverrides);
+        AttrDefs::iterator overrides = attrs.find(state.s.overrides);
         bool hasOverrides = overrides != attrs.end();
 
         /* The recursive attributes are evaluated in the new
@@ -1717,7 +1658,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             }
         }
 
-        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(sFunctor))) {
+        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(s.functor))) {
             /* '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. */
@@ -1779,7 +1720,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
     forceValue(fun, pos);
 
     if (fun.type() == nAttrs) {
-        auto found = fun.attrs()->find(sFunctor);
+        auto found = fun.attrs()->find(s.functor);
         if (found != fun.attrs()->end()) {
             Value * v = allocValue();
             callFunction(*found->value, fun, *v, pos);
@@ -2241,7 +2182,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(sFunctor) != fun.attrs()->end();
+    return fun.type() == nAttrs && fun.attrs()->find(s.functor) != fun.attrs()->end();
 }
 
 void EvalState::forceFunction(Value & v, const PosIdx pos, std::string_view errorCtx)
@@ -2310,7 +2251,7 @@ bool EvalState::isDerivation(Value & v)
 {
     if (v.type() != nAttrs)
         return false;
-    auto i = v.attrs()->get(sType);
+    auto i = v.attrs()->get(s.type);
     if (!i)
         return false;
     forceValue(*i->value, i->pos);
@@ -2322,7 +2263,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(sToString);
+    auto i = v.attrs()->find(s.toString);
     if (i != v.attrs()->end()) {
         Value v1;
         callFunction(*i->value, v, v1, pos);
@@ -2368,7 +2309,7 @@ BackedStringView EvalState::coerceToString(
         auto maybeString = tryAttrsToString(pos, v, context, coerceMore, copyToStore);
         if (maybeString)
             return std::move(*maybeString);
-        auto i = v.attrs()->find(sOutPath);
+        auto i = v.attrs()->find(s.outPath);
         if (i == v.attrs()->end()) {
             error<TypeError>(
                 "cannot coerce %1% to a string: %2%", showType(v), ValuePrinter(*this, v, errorPrintOptions))
@@ -2475,7 +2416,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(sToString);
+        auto i = v.attrs()->find(s.toString);
         if (i != v.attrs()->end()) {
             Value v1;
             callFunction(*i->value, v, v1, pos);
@@ -2665,8 +2606,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(sOutPath);
-            auto j = v2.attrs()->get(sOutPath);
+            auto i = v1.attrs()->get(s.outPath);
+            auto j = v2.attrs()->get(s.outPath);
             if (i && j) {
                 try {
                     assertEqValues(*i->value, *j->value, pos, errorCtx);
@@ -2819,8 +2760,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(sOutPath);
-            auto j = v2.attrs()->get(sOutPath);
+            auto i = v1.attrs()->get(s.outPath);
+            auto j = v2.attrs()->get(s.outPath);
             if (i && j)
                 return eqValues(*i->value, *j->value, pos, errorCtx);
         }
@@ -3196,8 +3137,7 @@ Expr * EvalState::parse(
         docComments = &it->second;
     }
 
-    auto result = parseExprFromBuf(
-        text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS, exprSymbols);
+    auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS);
 
     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->sName);
+        auto i = attrs->find(state->s.name);
         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->sSystem);
+        auto i = attrs->find(state->s.system);
         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->sDrvPath)) {
+        if (auto i = attrs->get(state->s.drvPath)) {
             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->sOutPath);
+        auto i = attrs->find(state->s.outPath);
         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->sOutputs))) {
+        if (attrs && (i = attrs->get(state->s.outputs))) {
             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->sOutPath);
+                    auto outPath = out->value->attrs()->get(state->s.outPath);
                     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->sOutputSpecified))
+    if (attrs && (i = attrs->get(state->s.outputSpecified))
         && 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->sOutputName);
+        auto i = attrs->get(state->s.outputName);
         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->sMeta);
+    auto a = attrs->get(state->s.meta);
     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->sOutPath))
+        if (v.attrs()->get(state->s.outPath))
             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.sRecurseForDerivations);
+                        auto j = i->value->attrs()->get(state.s.recurseForDerivations);
                         if (j
                             && state.forceBool(
                                 *j->value, j->pos, "while evaluating the attribute `recurseForDerivations`"))

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

@@ -213,23 +213,100 @@ 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.

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

@@ -595,12 +595,17 @@ 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;
-    const Expr::AstSymbols & s;
+    static constexpr Expr::AstSymbols s = StaticEvalSymbols::create().exprSymbols;
     const EvalSettings & settings;
 
     void dupAttr(const AttrPath & attrPath, const PosIdx pos, const PosIdx prevPos);

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

@@ -28,6 +28,8 @@ public:
     }
 };
 
+class StaticSymbolTable;
+
 /**
  * Symbols have the property that they can be compared efficiently
  * (using an equality test), because the symbol table stores only one
@@ -37,36 +39,29 @@ class Symbol
 {
     friend class SymbolStr;
     friend class SymbolTable;
+    friend class StaticSymbolTable;
 
 private:
     uint32_t id;
 
-    explicit Symbol(uint32_t id) noexcept
+    explicit constexpr Symbol(uint32_t id) noexcept
         : id(id)
     {
     }
 
 public:
-    Symbol() noexcept
+    constexpr Symbol() noexcept
         : id(0)
     {
     }
 
     [[gnu::always_inline]]
-    explicit operator bool() const noexcept
+    constexpr explicit operator bool() const noexcept
     {
         return id > 0;
     }
 
-    auto operator<=>(const Symbol other) const noexcept
-    {
-        return id <=> other.id;
-    }
-
-    bool operator==(const Symbol other) const noexcept
-    {
-        return id == other.id;
-    }
+    constexpr auto operator<=>(const Symbol & other) const noexcept = default;
 
     friend class std::hash<Symbol>;
 };
@@ -210,6 +205,39 @@ 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.
@@ -232,6 +260,10 @@ 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.
@@ -276,6 +308,16 @@ 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/meson.build

@@ -71,6 +71,12 @@ toml11 = dependency(
   method : 'cmake',
   include_type : 'system',
 )
+
+configdata_priv.set(
+  'HAVE_TOML11_4',
+  toml11.version().version_compare('>= 4.0.0').to_int(),
+)
+
 deps_other += toml11
 
 config_priv_h = configure_file(

src/libexpr/parser.y

@@ -68,8 +68,7 @@ Expr * parseExprFromBuf(
     const EvalSettings & settings,
     PosTable & positions,
     DocCommentMap & docComments,
-    const ref<SourceAccessor> rootFS,
-    const Expr::AstSymbols & astSymbols);
+    const ref<SourceAccessor> rootFS);
 
 }
 
@@ -542,8 +541,7 @@ Expr * parseExprFromBuf(
     const EvalSettings & settings,
     PosTable & positions,
     DocCommentMap & docComments,
-    const ref<SourceAccessor> rootFS,
-    const Expr::AstSymbols & astSymbols)
+    const ref<SourceAccessor> rootFS)
 {
     yyscan_t scanner;
     LexerState lexerState {
@@ -558,7 +556,6 @@ 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.sDrvPath)
+    attrs.alloc(state.s.drvPath)
         .mkString(
             path2,
             {
                 NixStringContextElem::DrvDeep{.drvPath = storePath},
             });
-    attrs.alloc(state.sName).mkString(drv.env["name"]);
+    attrs.alloc(state.s.name).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.sOutputs).mkList(list);
+    attrs.alloc(state.s.outputs).mkList(list);
 
     auto w = state.allocValue();
     w->mkAttrs(attrs);
@@ -731,7 +731,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the start set. */
     auto startSet = state.getAttr(
-        state.sStartSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.s.startSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
 
     state.forceList(
         *startSet->value,
@@ -749,7 +749,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the operator. */
     auto op = state.getAttr(
-        state.sOperator, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.s.operator_, 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");
 
@@ -771,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.sKey,
+            state.s.key,
             e->attrs(),
             "in one of the attrsets generated by (or initially passed to) builtins.genericClosure");
         state.forceValue(*key->value, noPos);
@@ -1076,11 +1076,11 @@ static void prim_tryEval(EvalState & state, const PosIdx pos, Value ** args, Val
 
     try {
         state.forceValue(*args[0], pos);
-        attrs.insert(state.sValue, args[0]);
+        attrs.insert(state.s.value, 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.sValue, &state.vFalse);
+        attrs.insert(state.s.value, &state.vFalse);
         attrs.insert(state.symbols.create("success"), &state.vFalse);
     }
 
@@ -1292,7 +1292,8 @@ 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.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict");
+    auto nameAttr =
+        state.getAttr(state.s.name, attrs, "in the attrset passed as argument to builtins.derivationStrict");
 
     std::string_view drvName;
     try {
@@ -1366,7 +1367,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.sStructuredAttrs);
+    auto attr = attrs->find(state.s.structuredAttrs);
     if (attr != attrs->end()
         && state.forceBool(
             *attr->value,
@@ -1377,7 +1378,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
 
     /* Check whether null attributes should be ignored. */
     bool ignoreNulls = false;
-    attr = attrs->find(state.sIgnoreNulls);
+    attr = attrs->find(state.s.ignoreNulls);
     if (attr != attrs->end())
         ignoreNulls = state.forceBool(
             *attr->value,
@@ -1401,7 +1402,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     outputs.insert("out");
 
     for (auto & i : attrs->lexicographicOrder(state.symbols)) {
-        if (i->name == state.sIgnoreNulls)
+        if (i->name == state.s.ignoreNulls)
             continue;
         auto key = state.symbols[i->name];
         vomit("processing attribute '%1%'", key);
@@ -1453,19 +1454,19 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                     continue;
             }
 
-            if (i->name == state.sContentAddressed && state.forceBool(*i->value, pos, context_below)) {
+            if (i->name == state.s.contentAddressed && 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)) {
+            else if (i->name == state.s.impure && 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. */
-            else if (i->name == state.sArgs) {
+            else if (i->name == state.s.args) {
                 state.forceList(*i->value, pos, context_below);
                 for (auto elem : i->value->listView()) {
                     auto s = state
@@ -1482,22 +1483,22 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
 
                 if (jsonObject) {
 
-                    if (i->name == state.sStructuredAttrs)
+                    if (i->name == state.s.structuredAttrs)
                         continue;
 
                     jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
 
-                    if (i->name == state.sBuilder)
+                    if (i->name == state.s.builder)
                         drv.builder = state.forceString(*i->value, context, pos, context_below);
-                    else if (i->name == state.sSystem)
+                    else if (i->name == state.s.system)
                         drv.platform = state.forceStringNoCtx(*i->value, pos, context_below);
-                    else if (i->name == state.sOutputHash)
+                    else if (i->name == state.s.outputHash)
                         outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
-                    else if (i->name == state.sOutputHashAlgo)
+                    else if (i->name == state.s.outputHashAlgo)
                         outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
-                    else if (i->name == state.sOutputHashMode)
+                    else if (i->name == state.s.outputHashMode)
                         handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
-                    else if (i->name == state.sOutputs) {
+                    else if (i->name == state.s.outputs) {
                         /* Require ‘outputs’ to be a list of strings. */
                         state.forceList(*i->value, pos, context_below);
                         Strings ss;
@@ -1506,51 +1507,51 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                         handleOutputs(ss);
                     }
 
-                    if (i->name == state.sAllowedReferences)
+                    if (i->name == state.s.allowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks.<output>.allowedReferences' instead",
                             drvName);
-                    if (i->name == state.sAllowedRequisites)
+                    if (i->name == state.s.allowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks.<output>.allowedRequisites' instead",
                             drvName);
-                    if (i->name == state.sDisallowedReferences)
+                    if (i->name == state.s.disallowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead",
                             drvName);
-                    if (i->name == state.sDisallowedRequisites)
+                    if (i->name == state.s.disallowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks.<output>.disallowedRequisites' instead",
                             drvName);
-                    if (i->name == state.sMaxSize)
+                    if (i->name == state.s.maxSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks.<output>.maxSize' instead",
                             drvName);
-                    if (i->name == state.sMaxClosureSize)
+                    if (i->name == state.s.maxClosureSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks.<output>.maxClosureSize' instead",
                             drvName);
 
                 } else {
                     auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned();
-                    if (i->name == state.sJson) {
+                    if (i->name == state.s.json) {
                         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);
-                        if (i->name == state.sBuilder)
+                        if (i->name == state.s.builder)
                             drv.builder = std::move(s);
-                        else if (i->name == state.sSystem)
+                        else if (i->name == state.s.system)
                             drv.platform = std::move(s);
-                        else if (i->name == state.sOutputHash)
+                        else if (i->name == state.s.outputHash)
                             outputHash = std::move(s);
-                        else if (i->name == state.sOutputHashAlgo)
+                        else if (i->name == state.s.outputHashAlgo)
                             outputHashAlgo = parseHashAlgoOpt(s);
-                        else if (i->name == state.sOutputHashMode)
+                        else if (i->name == state.s.outputHashMode)
                             handleHashMode(s);
-                        else if (i->name == state.sOutputs)
+                        else if (i->name == state.s.outputs)
                             handleOutputs(tokenizeString<Strings>(s));
                     }
                 }
@@ -1722,7 +1723,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     }
 
     auto result = state.buildBindings(1 + drv.outputs.size());
-    result.alloc(state.sDrvPath)
+    result.alloc(state.s.drvPath)
         .mkString(
             drvPathS,
             {
@@ -2006,14 +2007,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.sPrefix);
+        auto i = v2->attrs()->find(state.s.prefix);
         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.sPath, v2->attrs(), "in an element of the __nixPath");
+        i = state.getAttr(state.s.path, v2->attrs(), "in an element of the __nixPath");
 
         NixStringContext context;
         auto path =
@@ -2786,7 +2787,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.sName)
+        else if (attr.name == state.s.name)
             name = state.forceStringNoCtx(
                 *attr.value, attr.pos, "while evaluating the `name` attribute passed to builtins.path");
         else if (n == "filter")
@@ -3105,7 +3106,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.sName, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.s.name, v2->attrs(), "in a {name=...; value=...;} pair");
 
         auto name = state.forceStringNoCtx(
             *j->value,
@@ -3132,7 +3133,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.sValue, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.s.value, v2->attrs(), "in a {name=...; value=...;} pair");
         prev = attr.name;
         bindings.push_back({prev, j->value, j->pos});
     }
@@ -3948,13 +3949,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.sRight).mkList(rlist);
+    attrs.alloc(state.s.right).mkList(rlist);
 
     auto wsize = wrong.size();
     auto wlist = state.buildList(wsize);
     if (wsize)
         memcpy(wlist.elems, wrong.data(), sizeof(Value *) * wsize);
-    attrs.alloc(state.sWrong).mkList(wlist);
+    attrs.alloc(state.s.wrong).mkList(wlist);
 
     v.mkAttrs(attrs);
 }
@@ -4873,7 +4874,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.sName).mkString(parsed.name);
+    attrs.alloc(state.s.name).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.sOutputs).mkList(list);
+            infoAttrs.alloc(state.s.outputs).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.sOutputs)) {
+        if (auto attr = i.value->attrs()->get(state.s.outputs)) {
             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/fetchClosure.cc

@@ -185,7 +185,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value ** args
             {.msg = HintFmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"),
              .pos = state.positions[pos]});
 
-    auto parsedURL = parseURL(*fromStoreUrl);
+    auto parsedURL = parseURL(*fromStoreUrl, /*lenient=*/true);
 
     if (parsedURL.scheme != "http" && parsedURL.scheme != "https"
         && !(getEnv("_NIX_IN_TEST").has_value() && parsedURL.scheme == "file"))

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.sOutPath));
+    state.mkStorePathString(storePath, attrs2.alloc(state.s.outPath));
     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.sOutPath));
+    state.mkStorePathString(storePath, attrs.alloc(state.s.outPath));
 
     // FIXME: support arbitrary input attributes.
 
@@ -95,7 +95,7 @@ static void fetchTree(
 
         fetchers::Attrs attrs;
 
-        if (auto aType = args[0]->attrs()->get(state.sType)) {
+        if (auto aType = args[0]->attrs()->get(state.s.type)) {
             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.sType)
+            if (attr.name == state.s.type)
                 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) : s);
+                    params.isFetchGit && state.symbols[attr.name] == "url" ? fixGitURL(s).to_string() : 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));
+            attrs.emplace("url", fixGitURL(url).to_string());
             if (!attrs.contains("exportIgnore")
                 && (!attrs.contains("submodules") || !*fetchers::maybeGetBoolAttr(attrs, "submodules"))) {
                 attrs.emplace("exportIgnore", Explicit<bool>{true});

src/libexpr/primops/fromTOML.cc

@@ -1,73 +1,140 @@
 #include "nix/expr/primops.hh"
 #include "nix/expr/eval-inline.hh"
 
+#include "expr-config-private.hh"
+
 #include <sstream>
 
 #include <toml.hpp>
 
 namespace nix {
 
+#if HAVE_TOML11_4
+
+/**
+ * This is what toml11 < 4.0 did when choosing the subsecond precision.
+ * TOML 1.0.0 spec doesn't define how sub-millisecond ranges should be handled and calls it
+ * implementation defined behavior. For a lack of a better choice we stick with what older versions
+ * of toml11 did [1].
+ *
+ * [1]: https://github.com/ToruNiina/toml11/blob/dcfe39a783a94e8d52c885e5883a6fbb21529019/toml/datetime.hpp#L282
+ */
+static size_t normalizeSubsecondPrecision(toml::local_time lt)
+{
+    auto millis = lt.millisecond;
+    auto micros = lt.microsecond;
+    auto nanos = lt.nanosecond;
+    if (millis != 0 || micros != 0 || nanos != 0) {
+        if (micros != 0 || nanos != 0) {
+            if (nanos != 0)
+                return 9;
+            return 6;
+        }
+        return 3;
+    }
+    return 0;
+}
+
+/**
+ * Normalize date/time formats to serialize to the same strings as versions prior to toml11 4.0.
+ *
+ * Several things to consider:
+ *
+ * 1. Sub-millisecond range is represented the same way as in toml11 versions prior to 4.0. Precisioun is rounded
+ *    towards the next multiple of 3 or capped at 9 digits.
+ * 2. Seconds must be specified. This may become optional in (yet unreleased) TOML 1.1.0, but 1.0.0 defined local time
+ *    in terms of RFC3339 [1].
+ * 3. date-time separator (`t`, `T` or space ` `) is canonicalized to an upper T. This is compliant with RFC3339
+ *    [1] 5.6:
+ *    > Applications that generate this format SHOULD use upper case letters.
+ *
+ * [1]: https://datatracker.ietf.org/doc/html/rfc3339#section-5.6
+ */
+static void normalizeDatetimeFormat(toml::value & t)
+{
+    if (t.is_local_datetime()) {
+        auto & ldt = t.as_local_datetime();
+        t.as_local_datetime_fmt() = {
+            .delimiter = toml::datetime_delimiter_kind::upper_T,
+            // https://datatracker.ietf.org/doc/html/rfc3339#section-5.6
+            .has_seconds = true, // Mandated by TOML 1.0.0
+            .subsecond_precision = normalizeSubsecondPrecision(ldt.time),
+        };
+        return;
+    }
+
+    if (t.is_offset_datetime()) {
+        auto & odt = t.as_offset_datetime();
+        t.as_offset_datetime_fmt() = {
+            .delimiter = toml::datetime_delimiter_kind::upper_T,
+            // https://datatracker.ietf.org/doc/html/rfc3339#section-5.6
+            .has_seconds = true, // Mandated by TOML 1.0.0
+            .subsecond_precision = normalizeSubsecondPrecision(odt.time),
+        };
+        return;
+    }
+
+    if (t.is_local_time()) {
+        auto & lt = t.as_local_time();
+        t.as_local_time_fmt() = {
+            .has_seconds = true, // Mandated by TOML 1.0.0
+            .subsecond_precision = normalizeSubsecondPrecision(lt),
+        };
+        return;
+    }
+}
+
+#endif
+
 static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Value & val)
 {
     auto toml = state.forceStringNoCtx(*args[0], pos, "while evaluating the argument passed to builtins.fromTOML");
 
     std::istringstream tomlStream(std::string{toml});
 
-    std::function<void(Value &, toml::value)> visit;
-
-    visit = [&](Value & v, toml::value t) {
+    auto visit = [&](auto & self, Value & v, toml::value t) -> void {
         switch (t.type()) {
         case toml::value_t::table: {
             auto table = toml::get<toml::table>(t);
-
-            size_t size = 0;
-            for (auto & i : table) {
-                (void) i;
-                size++;
-            }
-
-            auto attrs = state.buildBindings(size);
+            auto attrs = state.buildBindings(table.size());
 
             for (auto & elem : table) {
                 forceNoNullByte(elem.first);
-                visit(attrs.alloc(elem.first), elem.second);
+                self(self, attrs.alloc(elem.first), elem.second);
             }
 
             v.mkAttrs(attrs);
         } break;
-            ;
         case toml::value_t::array: {
             auto array = toml::get<std::vector<toml::value>>(t);
 
             auto list = state.buildList(array.size());
             for (const auto & [n, v] : enumerate(list))
-                visit(*(v = state.allocValue()), array[n]);
+                self(self, *(v = state.allocValue()), array[n]);
             v.mkList(list);
         } break;
-            ;
         case toml::value_t::boolean:
             v.mkBool(toml::get<bool>(t));
             break;
-            ;
         case toml::value_t::integer:
             v.mkInt(toml::get<int64_t>(t));
             break;
-            ;
         case toml::value_t::floating:
             v.mkFloat(toml::get<NixFloat>(t));
             break;
-            ;
         case toml::value_t::string: {
             auto s = toml::get<std::string_view>(t);
             forceNoNullByte(s);
             v.mkString(s);
         } break;
-            ;
         case toml::value_t::local_datetime:
         case toml::value_t::offset_datetime:
         case toml::value_t::local_date:
         case toml::value_t::local_time: {
             if (experimentalFeatureSettings.isEnabled(Xp::ParseTomlTimestamps)) {
+#if HAVE_TOML11_4
+                normalizeDatetimeFormat(t);
+#endif
                 auto attrs = state.buildBindings(2);
                 attrs.alloc("_type").mkString("timestamp");
                 std::ostringstream s;
@@ -80,16 +147,24 @@ static void prim_fromTOML(EvalState & state, const PosIdx pos, Value ** args, Va
                 throw std::runtime_error("Dates and times are not supported");
             }
         } break;
-            ;
         case toml::value_t::empty:
             v.mkNull();
             break;
-            ;
         }
     };
 
     try {
-        visit(val, toml::parse(tomlStream, "fromTOML" /* the "filename" */));
+        visit(
+            visit,
+            val,
+            toml::parse(
+                tomlStream,
+                "fromTOML" /* the "filename" */
+#if HAVE_TOML11_4
+                ,
+                toml::spec::v(1, 0, 0) // Be explicit that we are parsing TOML 1.0.0 without extensions
+#endif
+                ));
     } catch (std::exception & e) { // TODO: toml::syntax_error
         state.error<EvalError>("while parsing TOML: %s", e.what()).atPos(pos).debugThrow();
     }

src/libexpr/print.cc

@@ -272,7 +272,7 @@ private:
     void printDerivation(Value & v)
     {
         std::optional<StorePath> storePath;
-        if (auto i = v.attrs()->get(state.sDrvPath)) {
+        if (auto i = v.attrs()->get(state.s.drvPath)) {
             NixStringContext context;
             storePath =
                 state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation");

src/libexpr/value-to-json.cc

@@ -53,7 +53,7 @@ json printValueAsJSON(
             out = *maybeString;
             break;
         }
-        if (auto i = v.attrs()->get(state.sOutPath))
+        if (auto i = v.attrs()->get(state.s.outPath))
             return printValueAsJSON(state, strict, *i->value, i->pos, context, copyToStore);
         else {
             out = json::object();

src/libexpr/value-to-xml.cc

@@ -98,14 +98,14 @@ static void printValueAsXML(
             XMLAttrs xmlAttrs;
 
             Path drvPath;
-            if (auto a = v.attrs()->get(state.sDrvPath)) {
+            if (auto a = v.attrs()->get(state.s.drvPath)) {
                 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.sOutPath)) {
+            if (auto a = v.attrs()->get(state.s.outPath)) {
                 if (strict)
                     state.forceValue(*a->value, a->pos);
                 if (a->value->type() == nString)

src/libfetchers/git-lfs-fetch.cc

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

src/libfetchers/git.cc

@@ -233,9 +233,7 @@ struct GitInputScheme : InputScheme
 
         Input input{settings};
         input.attrs = attrs;
-        auto url = fixGitURL(getStrAttr(attrs, "url"));
-        parseURL(url);
-        input.attrs["url"] = url;
+        input.attrs["url"] = fixGitURL(getStrAttr(attrs, "url")).to_string();
         getShallowAttr(input);
         getSubmodulesAttr(input);
         getAllRefsAttr(input);
@@ -464,8 +462,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
-        bool isBareRepository = url.scheme == "file" && pathExists(url.path) && !pathExists(url.path + "/.git");
-        //
+        auto isBareRepository = [](PathView path) { return pathExists(path) && !pathExists(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
@@ -474,8 +472,10 @@ struct GitInputScheme : InputScheme
         //
         // See: https://discourse.nixos.org/t/57783 and #9708
         //
-        if (url.scheme == "file" && !forceHttp && !isBareRepository) {
-            if (!isAbsolute(url.path)) {
+        if (url.scheme == "file" && !forceHttp && !isBareRepository(renderUrlPathEnsureLegal(url.path))) {
+            auto path = renderUrlPathEnsureLegal(url.path);
+
+            if (!isAbsolute(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(url.path)) {
-                throw Error("The path '%s' does not exist.", url.path);
+            if (!pathExists(path)) {
+                throw Error("The path '%s' does not exist.", path);
             }
-            repoInfo.location = std::filesystem::absolute(url.path);
+            repoInfo.location = std::filesystem::absolute(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
 {
-    std::string url;
+    ParsedURL url;
     Headers headers;
 };
 
@@ -38,7 +38,8 @@ struct GitArchiveInputScheme : InputScheme
         if (url.scheme != schemeName())
             return {};
 
-        auto path = tokenizeString<std::vector<std::string>>(url.path, "/");
+        /* 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>>();
 
         std::optional<Hash> rev;
         std::optional<std::string> ref;
@@ -139,12 +140,12 @@ struct GitArchiveInputScheme : InputScheme
         auto repo = getStrAttr(input.attrs, "repo");
         auto ref = input.getRef();
         auto rev = input.getRev();
-        auto path = owner + "/" + repo;
+        std::vector<std::string> path{owner, repo};
         assert(!(ref && rev));
         if (ref)
-            path += "/" + *ref;
+            path.push_back(*ref);
         if (rev)
-            path += "/" + rev->to_string(HashFormat::Base16, false);
+            path.push_back(rev->to_string(HashFormat::Base16, false));
         auto url = ParsedURL{
             .scheme = std::string{schemeName()},
             .path = path,
@@ -420,7 +421,7 @@ struct GitHubInputScheme : GitArchiveInputScheme
         const auto url =
             fmt(urlFmt, host, getOwner(input), getRepo(input), input.getRev()->to_string(HashFormat::Base16, false));
 
-        return DownloadUrl{url, headers};
+        return DownloadUrl{parseURL(url), headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override
@@ -500,7 +501,7 @@ struct GitLabInputScheme : GitArchiveInputScheme
                 input.getRev()->to_string(HashFormat::Base16, false));
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
-        return DownloadUrl{url, headers};
+        return DownloadUrl{parseURL(url), headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override
@@ -592,7 +593,7 @@ struct SourceHutInputScheme : GitArchiveInputScheme
                 input.getRev()->to_string(HashFormat::Base16, false));
 
         Headers headers = makeHeadersWithAuthTokens(*input.settings, host, input);
-        return DownloadUrl{url, headers};
+        return DownloadUrl{parseURL(url), headers};
     }
 
     void clone(const Input & input, const Path & destDir) const override

src/libfetchers/indirect.cc

@@ -14,7 +14,8 @@ struct IndirectInputScheme : InputScheme
         if (url.scheme != "flake")
             return {};
 
-        auto path = tokenizeString<std::vector<std::string>>(url.path, "/");
+        /* 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>>();
 
         std::optional<Hash> rev;
         std::optional<std::string> ref;
@@ -82,16 +83,15 @@ struct IndirectInputScheme : InputScheme
 
     ParsedURL toURL(const Input & input) const override
     {
-        ParsedURL url;
-        url.scheme = "flake";
-        url.path = getStrAttr(input.attrs, "id");
+        ParsedURL url{
+            .scheme = "flake",
+            .path = {getStrAttr(input.attrs, "id")},
+        };
         if (auto ref = input.getRef()) {
-            url.path += '/';
-            url.path += *ref;
+            url.path.push_back(*ref);
         };
         if (auto rev = input.getRev()) {
-            url.path += '/';
-            url.path += rev->gitRev();
+            url.path.push_back(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 url.path;
+            return renderUrlPathEnsureLegal(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 ? url.path : url.to_string()};
+        return {isLocal, isLocal ? renderUrlPathEnsureLegal(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", url.path);
+        input.attrs.insert_or_assign("path", renderUrlPathEnsureLegal(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 = getStrAttr(input.attrs, "path"),
+            .path = splitString<std::vector<std::string>>(getStrAttr(input.attrs, "path"), "/"),
             .query = query,
         };
     }

src/libfetchers/tarball.cc

@@ -43,7 +43,7 @@ DownloadFileResult downloadFile(
     if (cached && !cached->expired)
         return useCached();
 
-    FileTransferRequest request(url);
+    FileTransferRequest request(ValidURL{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 & url, const Headers & headers, const std::string & displayPrefix)
+    const Settings & settings, const std::string & urlS, 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.rfind("file://", 0) == 0) {
-        // Remove "file://" prefix to get the local file path
-        std::string localPath = url.substr(7);
-        if (!std::filesystem::exists(localPath)) {
+    if (url.scheme() == "file") {
+        std::filesystem::path localPath = renderUrlPathEnsureLegal(url.path());
+        if (!exists(localPath)) {
             throw Error("tarball '%s' does not exist.", localPath);
         }
-        if (std::filesystem::is_directory(localPath)) {
-            if (std::filesystem::exists(localPath + "/.git")) {
+        if (is_directory(localPath)) {
+            if (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", url}}};
+    Cache::Key cacheKey{"tarball", {{"url", urlS}}};
 
     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 = hasSuffix(toLower(parseURL(url).path), ".zip") ? ({
+    auto archive = !url.path().empty() && hasSuffix(toLower(url.path().back()), ".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,8 +234,11 @@ struct CurlInputScheme : InputScheme
 {
     const StringSet transportUrlSchemes = {"file", "http", "https"};
 
-    bool hasTarballExtension(std::string_view path) const
+    bool hasTarballExtension(const ParsedURL & url) 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");
@@ -336,7 +339,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.path)));
+                                               : (!requireTree && !hasTarballExtension(url)));
     }
 
     std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override
@@ -373,7 +376,7 @@ struct TarballInputScheme : CurlInputScheme
 
         return transportUrlSchemes.count(std::string(parsedUrlScheme.transport))
                && (parsedUrlScheme.application ? parsedUrlScheme.application.value() == schemeName()
-                                               : (requireTree || hasTarballExtension(url.path)));
+                                               : (requireTree || hasTarballExtension(url)));
     }
 
     std::pair<ref<SourceAccessor>, Input> getAccessor(ref<Store> store, const Input & _input) const override

src/libflake-tests/flakeref.cc

@@ -2,6 +2,7 @@
 
 #include "nix/fetchers/fetch-settings.hh"
 #include "nix/flake/flakeref.hh"
+#include "nix/fetchers/attrs.hh"
 
 namespace nix {
 
@@ -90,6 +91,158 @@ 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-tests/url-name.cc

@@ -13,8 +13,9 @@ TEST(getNameFromURL, getNameFromURL)
     ASSERT_EQ(getNameFromURL(parseURL("path:~/repos/nixpkgs#packages.x86_64-linux.Hello")), "Hello");
     ASSERT_EQ(getNameFromURL(parseURL("path:.#nonStandardAttr.mylaptop")), "mylaptop");
     ASSERT_EQ(getNameFromURL(parseURL("path:./repos/myflake#nonStandardAttr.mylaptop")), "mylaptop");
-    ASSERT_EQ(getNameFromURL(parseURL("path:./nixpkgs#packages.x86_64-linux.complex^bin,man")), "complex");
-    ASSERT_EQ(getNameFromURL(parseURL("path:./myproj#packages.x86_64-linux.default^*")), "myproj");
+    ASSERT_EQ(
+        getNameFromURL(parseURL("path:./nixpkgs#packages.x86_64-linux.complex^bin,man", /*lenient=*/true)), "complex");
+    ASSERT_EQ(getNameFromURL(parseURL("path:./myproj#packages.x86_64-linux.default^*", /*lenient=*/true)), "myproj");
     ASSERT_EQ(getNameFromURL(parseURL("path:./myproj#defaultPackage.x86_64-linux")), "myproj");
 
     ASSERT_EQ(getNameFromURL(parseURL("github:NixOS/nixpkgs#packages.x86_64-linux.hello")), "hello");
@@ -80,6 +81,6 @@ TEST(getNameFromURL, getNameFromURL)
     ASSERT_EQ(getNameFromURL(parseURL("path:.")), std::nullopt);
     ASSERT_EQ(getNameFromURL(parseURL("file:.#")), std::nullopt);
     ASSERT_EQ(getNameFromURL(parseURL("path:.#packages.x86_64-linux.default")), std::nullopt);
-    ASSERT_EQ(getNameFromURL(parseURL("path:.#packages.x86_64-linux.default^*")), std::nullopt);
+    ASSERT_EQ(getNameFromURL(parseURL("path:.#packages.x86_64-linux.default^*", /*lenient=*/true)), std::nullopt);
 }
 } // namespace nix

src/libflake/flake.cc

@@ -232,7 +232,7 @@ static Flake readFlake(
         .path = flakePath,
     };
 
-    if (auto description = vInfo.attrs()->get(state.sDescription)) {
+    if (auto description = vInfo.attrs()->get(state.s.description)) {
         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.sSelf)
+                if (formal.name != state.s.self)
                     flake.inputs.emplace(
                         state.symbols[formal.name],
                         FlakeInput{.ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name]))});
@@ -305,7 +305,8 @@ static Flake readFlake(
     }
 
     for (auto & attr : *vInfo.attrs()) {
-        if (attr.name != state.sDescription && attr.name != sInputs && attr.name != sOutputs && attr.name != sNixConfig)
+        if (attr.name != state.s.description && 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

@@ -82,7 +82,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
     auto succeeds = std::regex_match(url, match, pathFlakeRegex);
     assert(succeeds);
     auto path = match[1].str();
-    auto query = decodeQuery(match[3]);
+    auto query = decodeQuery(match[3].str(), /*lenient=*/true);
     auto fragment = percentDecode(match[5].str());
 
     if (baseDir) {
@@ -143,7 +143,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
                     auto parsedURL = ParsedURL{
                         .scheme = "git+file",
                         .authority = ParsedURL::Authority{},
-                        .path = flakeRoot,
+                        .path = splitString<std::vector<std::string>>(flakeRoot, "/"),
                         .query = query,
                         .fragment = fragment,
                     };
@@ -172,7 +172,13 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
 
     return fromParsedURL(
         fetchSettings,
-        {.scheme = "path", .authority = ParsedURL::Authority{}, .path = path, .query = query, .fragment = fragment},
+        {
+            .scheme = "path",
+            .authority = ParsedURL::Authority{},
+            .path = splitString<std::vector<std::string>>(path, "/"),
+            .query = query,
+            .fragment = fragment,
+        },
         isFlake);
 }
 
@@ -192,8 +198,8 @@ parseFlakeIdRef(const fetchers::Settings & fetchSettings, const std::string & ur
     if (std::regex_match(url, match, flakeRegex)) {
         auto parsedURL = ParsedURL{
             .scheme = "flake",
-            .authority = ParsedURL::Authority{},
-            .path = match[1],
+            .authority = std::nullopt,
+            .path = splitString<std::vector<std::string>>(match[1].str(), "/"),
         };
 
         return std::make_pair(
@@ -210,9 +216,13 @@ std::optional<std::pair<FlakeRef, std::string>> parseURLFlakeRef(
     bool isFlake)
 {
     try {
-        auto parsed = parseURL(url);
-        if (baseDir && (parsed.scheme == "path" || parsed.scheme == "git+file") && !isAbsolute(parsed.path))
-            parsed.path = absPath(parsed.path, *baseDir);
+        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), "/");
+        }
         return fromParsedURL(fetchSettings, std::move(parsed), isFlake);
     } catch (BadURL &) {
         return std::nullopt;
@@ -289,7 +299,7 @@ FlakeRef FlakeRef::canonicalize() const
        filtering the `dir` query parameter from the URL. */
     if (auto url = fetchers::maybeGetStrAttr(flakeRef.input.attrs, "url")) {
         try {
-            auto parsed = parseURL(*url);
+            auto parsed = parseURL(*url, /*lenient=*/true);
             if (auto dir2 = get(parsed.query, "dir")) {
                 if (flakeRef.subdir != "" && flakeRef.subdir == *dir2)
                     parsed.query.erase("dir");

src/libflake/url-name.cc

@@ -27,16 +27,21 @@ 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(url.path, match, secondPathSegmentRegex))
+    if (std::regex_match(url.scheme, gitProviderRegex) && std::regex_match(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(url.path, match, lastPathSegmentRegex))
+    if (std::regex_match(url.scheme, gitSchemeRegex) && std::regex_match(path, match, lastPathSegmentRegex))
         return match.str(1);
 
     /* If there is no fragment, take the last element of the path */
-    if (std::regex_match(url.path, match, lastPathSegmentRegex))
+    if (std::regex_match(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/libstore-tests/derivation-parser-bench.cc

@@ -28,6 +28,27 @@ 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,
@@ -37,3 +58,11 @@ BENCHMARK_CAPTURE(
     BM_ParseRealDerivationFile,
     firefox,
     getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value_or(NIX_UNIT_TEST_DATA) + "/derivation/firefox.drv");
+BENCHMARK_CAPTURE(
+    BM_UnparseRealDerivationFile,
+    hello,
+    getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value_or(NIX_UNIT_TEST_DATA) + "/derivation/hello.drv");
+BENCHMARK_CAPTURE(
+    BM_UnparseRealDerivationFile,
+    firefox,
+    getEnvNonEmpty("_NIX_TEST_UNIT_DATA").value_or(NIX_UNIT_TEST_DATA) + "/derivation/firefox.drv");

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(testCase.url);
+    auto parsed = ParsedS3URL::parse(parseURL(testCase.url));
     ASSERT_EQ(parsed, testCase.expected);
 }
 
@@ -33,51 +33,57 @@ 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)
@@ -86,11 +92,114 @@ TEST(InvalidParsedS3URLTest, parseS3URLErrors)
         testing::HasSubstrIgnoreANSIMatcher("error: URI has a missing or invalid bucket name"));
 
     /* Empty bucket (authority) */
-    ASSERT_THAT([]() { ParsedS3URL::parse("s3:///key"); }, invalidBucketMatcher);
+    ASSERT_THAT([]() { ParsedS3URL::parse(parseURL("s3:///key")); }, invalidBucketMatcher);
     /* Invalid bucket name */
-    ASSERT_THAT([]() { ParsedS3URL::parse("s3://127.0.0.1"); }, invalidBucketMatcher);
+    ASSERT_THAT([]() { ParsedS3URL::parse(parseURL("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/build/derivation-building-goal.cc

@@ -1,4 +1,5 @@
 #include "nix/store/build/derivation-building-goal.hh"
+#include "nix/store/build/derivation-env-desugar.hh"
 #include "nix/store/build/derivation-trampoline-goal.hh"
 #ifndef _WIN32 // TODO enable build hook on Windows
 #  include "nix/store/build/hook-instance.hh"
@@ -53,24 +54,9 @@ DerivationBuildingGoal::~DerivationBuildingGoal()
 {
     /* Careful: we should never ever throw an exception from a
        destructor. */
-    try {
-        killChild();
-    } catch (...) {
-        ignoreExceptionInDestructor();
-    }
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows
-    if (builder) {
-        try {
-            builder->stopDaemon();
-        } catch (...) {
-            ignoreExceptionInDestructor();
-        }
-        try {
-            builder->deleteTmpDir(false);
-        } catch (...) {
-            ignoreExceptionInDestructor();
-        }
-    }
+    if (builder)
+        builder.reset();
 #endif
     try {
         closeLogFile();
@@ -94,22 +80,8 @@ void DerivationBuildingGoal::killChild()
     hook.reset();
 #endif
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows
-    if (builder && builder->pid != -1) {
+    if (builder && builder->killChild())
         worker.childTerminated(this);
-
-        // FIXME: move this into DerivationBuilder.
-
-        /* If we're using a build user, then there is a tricky race
-           condition: if we kill the build user before the child has
-           done its setuid() to the build user uid, then it won't be
-           killed, and we'll potentially lock up in pid.wait().  So
-           also send a conventional kill to the child. */
-        ::kill(-builder->pid, SIGKILL); /* ignore the result */
-
-        builder->killSandbox(true);
-
-        builder->pid.wait();
-    }
 #endif
 }
 
@@ -118,7 +90,7 @@ void DerivationBuildingGoal::timedOut(Error && ex)
     killChild();
     // We're not inside a coroutine, hence we can't use co_return here.
     // Thus we ignore the return value.
-    [[maybe_unused]] Done _ = done(BuildResult::TimedOut, {}, std::move(ex));
+    [[maybe_unused]] Done _ = doneFailure({BuildResult::TimedOut, std::move(ex)});
 }
 
 /**
@@ -148,6 +120,9 @@ std::string showKnownOutputs(const StoreDirConfig & store, const Derivation & dr
     return msg;
 }
 
+static void runPostBuildHook(
+    const StoreDirConfig & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths);
+
 /* At least one of the output paths could not be
    produced using a substitute.  So we have to build instead. */
 Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
@@ -255,7 +230,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
                 nrFailed,
                 nrFailed == 1 ? "dependency" : "dependencies");
         msg += showKnownOutputs(worker.store, *drv);
-        co_return done(BuildResult::DependencyFailed, {}, Error(msg));
+        co_return doneFailure(BuildError(BuildResult::DependencyFailed, msg));
     }
 
     /* Gather information necessary for computing the closure and/or
@@ -356,9 +331,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 
             auto resolvedResult = resolvedDrvGoal->buildResult;
 
-            SingleDrvOutputs builtOutputs;
-
             if (resolvedResult.success()) {
+                SingleDrvOutputs builtOutputs;
+
                 auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
 
                 StorePathSet outputPaths;
@@ -408,13 +383,19 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
                 }
 
                 runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
+
+                auto status = resolvedResult.status;
+                if (status == BuildResult::AlreadyValid)
+                    status = BuildResult::ResolvesToAlreadyValid;
+
+                co_return doneSuccess(status, std::move(builtOutputs));
+            } else {
+                co_return doneFailure({
+                    BuildResult::DependencyFailed,
+                    "build of resolved derivation '%s' failed",
+                    worker.store.printStorePath(pathResolved),
+                });
             }
-
-            auto status = resolvedResult.status;
-            if (status == BuildResult::AlreadyValid)
-                status = BuildResult::ResolvesToAlreadyValid;
-
-            co_return done(status, std::move(builtOutputs));
         }
 
         /* If we get this far, we know no dynamic drvs inputs */
@@ -539,7 +520,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
         debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath));
         outputLocks.setDeletion(true);
         outputLocks.unlock();
-        co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
+        co_return doneSuccess(BuildResult::AlreadyValid, std::move(validOutputs));
     }
 
     /* If any of the outputs already exist but are not valid, delete
@@ -643,21 +624,6 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                     goal.worker.childTerminated(&goal);
                 }
 
-                void noteHashMismatch() override
-                {
-                    goal.worker.hashMismatch = true;
-                }
-
-                void noteCheckMismatch() override
-                {
-                    goal.worker.checkMismatch = true;
-                }
-
-                void markContentsGood(const StorePath & path) override
-                {
-                    goal.worker.markContentsGood(path);
-                }
-
                 Path openLogFile() override
                 {
                     return goal.openLogFile();
@@ -667,19 +633,13 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 {
                     goal.closeLogFile();
                 }
-
-                void appendLogTailErrorMsg(std::string & msg) override
-                {
-                    goal.appendLogTailErrorMsg(msg);
-                }
             };
 
             auto * localStoreP = dynamic_cast<LocalStore *>(&worker.store);
             assert(localStoreP);
 
             decltype(DerivationBuilderParams::defaultPathsInChroot) defaultPathsInChroot = settings.sandboxPaths.get();
-            decltype(DerivationBuilderParams::finalEnv) finalEnv;
-            decltype(DerivationBuilderParams::extraFiles) extraFiles;
+            DesugaredEnv desugaredEnv;
 
             /* Add the closure of store paths to the chroot. */
             StorePathSet closure;
@@ -698,58 +658,11 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             }
 
             try {
-                if (drv->structuredAttrs) {
-                    auto json = drv->structuredAttrs->prepareStructuredAttrs(
-                        worker.store, *drvOptions, inputPaths, drv->outputs);
-
-                    finalEnv.insert_or_assign(
-                        "NIX_ATTRS_SH_FILE",
-                        DerivationBuilderParams::EnvEntry{
-                            .nameOfPassAsFile = ".attrs.sh",
-                            .value = StructuredAttrs::writeShell(json),
-                        });
-                    finalEnv.insert_or_assign(
-                        "NIX_ATTRS_JSON_FILE",
-                        DerivationBuilderParams::EnvEntry{
-                            .nameOfPassAsFile = ".attrs.json",
-                            .value = 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.find(envName) == drvOptions->passAsFile.end()) {
-                            finalEnv.insert_or_assign(
-                                envName,
-                                DerivationBuilderParams::EnvEntry{
-                                    .nameOfPassAsFile = std::nullopt,
-                                    .value = envValue,
-                                });
-                        } else {
-                            auto hash = hashString(HashAlgorithm::SHA256, envName);
-                            finalEnv.insert_or_assign(
-                                envName + "Path",
-                                DerivationBuilderParams::EnvEntry{
-                                    .nameOfPassAsFile = ".attr-" + hash.to_string(HashFormat::Nix32, false),
-                                    .value = envValue,
-                                });
-                        }
-                    }
-
-                    /* Handle exportReferencesGraph(), if set. */
-                    for (auto & [fileName, storePaths] : drvOptions->getParsedExportReferencesGraph(worker.store)) {
-                        /* Write closure info to <fileName>. */
-                        extraFiles.insert_or_assign(
-                            fileName,
-                            worker.store.makeValidityRegistration(
-                                worker.store.exportReferences(storePaths, inputPaths), false, false));
-                    }
-                }
+                desugaredEnv = DesugaredEnv::create(worker.store, *drv, *drvOptions, inputPaths);
             } catch (BuildError & e) {
                 outputLocks.unlock();
                 worker.permanentFailure = true;
-                co_return done(BuildResult::InputRejected, {}, std::move(e));
+                co_return doneFailure(std::move(e));
             }
 
             /* If we have to wait and retry (see below), then `builder` will
@@ -758,16 +671,16 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 *localStoreP,
                 std::make_unique<DerivationBuildingGoalCallbacks>(*this, builder),
                 DerivationBuilderParams{
-                    drvPath,
-                    buildMode,
-                    buildResult,
-                    *drv,
-                    *drvOptions,
-                    inputPaths,
-                    initialOutputs,
-                    std::move(defaultPathsInChroot),
-                    std::move(finalEnv),
-                    std::move(extraFiles),
+                    .drvPath = drvPath,
+                    .buildResult = buildResult,
+                    .drv = *drv,
+                    .drvOptions = *drvOptions,
+                    .inputPaths = inputPaths,
+                    .initialOutputs = initialOutputs,
+                    .buildMode = buildMode,
+                    .defaultPathsInChroot = std::move(defaultPathsInChroot),
+                    .systemFeatures = worker.store.config.systemFeatures.get(),
+                    .desugaredEnv = std::move(desugaredEnv),
                 });
         }
 
@@ -796,7 +709,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
         builder.reset();
         outputLocks.unlock();
         worker.permanentFailure = true;
-        co_return done(BuildResult::InputRejected, {}, std::move(e));
+        co_return doneFailure(std::move(e)); // InputRejected
     }
 
     started();
@@ -804,26 +717,60 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
 
     trace("build done");
 
-    auto res = builder->unprepareBuild();
-    // N.B. cannot use `std::visit` with co-routine return
-    if (auto * ste = std::get_if<0>(&res)) {
+    SingleDrvOutputs builtOutputs;
+    try {
+        builtOutputs = builder->unprepareBuild();
+    } catch (BuilderFailureError & e) {
+        builder.reset();
         outputLocks.unlock();
-        co_return done(std::move(ste->first), {}, std::move(ste->second));
-    } else if (auto * builtOutputs = std::get_if<1>(&res)) {
+        co_return doneFailure(fixupBuilderFailureErrorMessage(std::move(e)));
+    } catch (BuildError & e) {
+        builder.reset();
+        outputLocks.unlock();
+// Allow selecting a subset of enum values
+#  pragma GCC diagnostic push
+#  pragma GCC diagnostic ignored "-Wswitch-enum"
+        switch (e.status) {
+        case BuildResult::HashMismatch:
+            worker.hashMismatch = true;
+            /* See header, the protocols don't know about `HashMismatch`
+               yet, so change it to `OutputRejected`, which they expect
+               for this case (hash mismatch is a type of output
+               rejection). */
+            e.status = BuildResult::OutputRejected;
+            break;
+        case BuildResult::NotDeterministic:
+            worker.checkMismatch = true;
+            break;
+        default:
+            /* Other statuses need no adjusting */
+            break;
+        }
+#  pragma GCC diagnostic pop
+        co_return doneFailure(std::move(e));
+    }
+    {
+        builder.reset();
+        StorePathSet outputPaths;
+        for (auto & [_, output] : builtOutputs) {
+            // for sake of `bmRepair`
+            worker.markContentsGood(output.outPath);
+            outputPaths.insert(output.outPath);
+        }
+        runPostBuildHook(worker.store, *logger, drvPath, outputPaths);
+
         /* It is now safe to delete the lock files, since all future
            lockers will see that the output paths are valid; they will
            not create new lock files with the same names as the old
            (unlinked) lock files. */
         outputLocks.setDeletion(true);
         outputLocks.unlock();
-        co_return done(BuildResult::Built, std::move(*builtOutputs));
-    } else {
-        unreachable();
+        co_return doneSuccess(BuildResult::Built, std::move(builtOutputs));
     }
 #endif
 }
 
-void runPostBuildHook(
+static void runPostBuildHook(
     const StoreDirConfig & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths)
 {
     auto hook = settings.postBuildHook;
@@ -889,8 +836,16 @@ void runPostBuildHook(
     });
 }
 
-void DerivationBuildingGoal::appendLogTailErrorMsg(std::string & msg)
+BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailureError e)
 {
+    auto msg =
+        fmt("Cannot build '%s'.\n"
+            "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".",
+            Magenta(worker.store.printStorePath(drvPath)),
+            statusToString(e.builderStatus));
+
+    msg += showKnownOutputs(worker.store, *drv);
+
     if (!logger->isVerbose() && !logTail.empty()) {
         msg += fmt("\nLast %d log lines:\n", logTail.size());
         for (auto & line : logTail) {
@@ -907,6 +862,10 @@ void DerivationBuildingGoal::appendLogTailErrorMsg(std::string & msg)
                 nixLogCommand,
                 worker.store.printStorePath(drvPath));
     }
+
+    msg += e.extraMsgAfter;
+
+    return BuildError{e.status, msg};
 }
 
 Goal::Co DerivationBuildingGoal::hookDone()
@@ -947,21 +906,13 @@ Goal::Co DerivationBuildingGoal::hookDone()
 
     /* Check the exit status. */
     if (!statusOk(status)) {
-        auto msg =
-            fmt("Cannot build '%s'.\n"
-                "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".",
-                Magenta(worker.store.printStorePath(drvPath)),
-                statusToString(status));
-
-        msg += showKnownOutputs(worker.store, *drv);
-
-        appendLogTailErrorMsg(msg);
+        auto e = fixupBuilderFailureErrorMessage({BuildResult::MiscFailure, status, ""});
 
         outputLocks.unlock();
 
         /* TODO (once again) support fine-grained error codes, see issue #12641. */
 
-        co_return done(BuildResult::MiscFailure, {}, BuildError(msg));
+        co_return doneFailure(std::move(e));
     }
 
     /* Compute the FS closure of the outputs and register them as
@@ -988,7 +939,7 @@ Goal::Co DerivationBuildingGoal::hookDone()
     outputLocks.setDeletion(true);
     outputLocks.unlock();
 
-    co_return done(BuildResult::Built, std::move(builtOutputs));
+    co_return doneSuccess(BuildResult::Built, std::move(builtOutputs));
 }
 
 HookReply DerivationBuildingGoal::tryBuildHook()
@@ -1170,10 +1121,11 @@ void DerivationBuildingGoal::handleChildOutput(Descriptor fd, std::string_view d
             killChild();
             // We're not inside a coroutine, hence we can't use co_return here.
             // Thus we ignore the return value.
-            [[maybe_unused]] Done _ = done(
+            [[maybe_unused]] Done _ = doneFailure(BuildError(
                 BuildResult::LogLimitExceeded,
-                {},
-                Error("%s killed after writing more than %d bytes of log output", getName(), settings.maxLogSize));
+                "%s killed after writing more than %d bytes of log output",
+                getName(),
+                settings.maxLogSize));
             return;
         }
 
@@ -1334,13 +1286,29 @@ SingleDrvOutputs DerivationBuildingGoal::assertPathValidity()
     return validOutputs;
 }
 
-Goal::Done
-DerivationBuildingGoal::done(BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional<Error> ex)
+Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs)
 {
     outputLocks.unlock();
     buildResult.status = status;
-    if (ex)
-        buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
+
+    assert(buildResult.success());
+
+    mcRunningBuilds.reset();
+
+    buildResult.builtOutputs = std::move(builtOutputs);
+    if (status == BuildResult::Built)
+        worker.doneBuilds++;
+
+    worker.updateProgress();
+
+    return amDone(ecSuccess, std::nullopt);
+}
+
+Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex)
+{
+    outputLocks.unlock();
+    buildResult.status = ex.status;
+    buildResult.errorMsg = fmt("%s", Uncolored(ex.info().msg));
     if (buildResult.status == BuildResult::TimedOut)
         worker.timedOut = true;
     if (buildResult.status == BuildResult::PermanentFailure)
@@ -1348,25 +1316,12 @@ DerivationBuildingGoal::done(BuildResult::Status status, SingleDrvOutputs builtO
 
     mcRunningBuilds.reset();
 
-    if (buildResult.success()) {
-        buildResult.builtOutputs = std::move(builtOutputs);
-        if (status == BuildResult::Built)
-            worker.doneBuilds++;
-    } else {
-        if (status != BuildResult::DependencyFailed)
-            worker.failedBuilds++;
-    }
+    if (ex.status != BuildResult::DependencyFailed)
+        worker.failedBuilds++;
 
     worker.updateProgress();
 
-    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));
+    return amDone(ecFailed, {std::move(ex)});
 }
 
 } // namespace nix

src/libstore/build/derivation-check.cc

@@ -0,0 +1,190 @@
+#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

@@ -0,0 +1,27 @@
+#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

@@ -0,0 +1,59 @@
+#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 done(BuildResult::AlreadyValid, checkResult->first);
+            co_return doneSuccess(BuildResult::AlreadyValid, checkResult->first);
         }
 
         Goals waitees;
@@ -122,12 +122,10 @@ Goal::Co DerivationGoal::haveDerivation()
         assert(!drv->type().isImpure());
 
         if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) {
-            co_return done(
+            co_return doneFailure(BuildError(
                 BuildResult::TransientFailure,
-                {},
-                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)));
+                "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;
@@ -137,7 +135,7 @@ Goal::Co DerivationGoal::haveDerivation()
         bool allValid = checkResult && checkResult->second == PathStatus::Valid;
 
         if (buildMode == bmNormal && allValid) {
-            co_return done(BuildResult::Substituted, checkResult->first);
+            co_return doneSuccess(BuildResult::Substituted, checkResult->first);
         }
         if (buildMode == bmRepair && allValid) {
             co_return repairClosure();
@@ -281,7 +279,7 @@ Goal::Co DerivationGoal::repairClosure()
                 "some paths in the output closure of derivation '%s' could not be repaired",
                 worker.store.printStorePath(drvPath));
     }
-    co_return done(BuildResult::AlreadyValid, assertPathValidity());
+    co_return doneSuccess(BuildResult::AlreadyValid, assertPathValidity());
 }
 
 std::optional<std::pair<Realisation, PathStatus>> DerivationGoal::checkPathValidity()
@@ -339,12 +337,27 @@ Realisation DerivationGoal::assertPathValidity()
     return checkResult->first;
 }
 
-Goal::Done
-DerivationGoal::done(BuildResult::Status status, std::optional<Realisation> builtOutput, std::optional<Error> ex)
+Goal::Done DerivationGoal::doneSuccess(BuildResult::Status status, Realisation builtOutput)
 {
     buildResult.status = status;
-    if (ex)
-        buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
+
+    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 (buildResult.status == BuildResult::TimedOut)
         worker.timedOut = true;
     if (buildResult.status == BuildResult::PermanentFailure)
@@ -352,26 +365,12 @@ DerivationGoal::done(BuildResult::Status status, std::optional<Realisation> buil
 
     mcExpectedBuilds.reset();
 
-    if (buildResult.success()) {
-        assert(builtOutput);
-        buildResult.builtOutputs = {{wantedOutput, std::move(*builtOutput)}};
-        if (status == BuildResult::Built)
-            worker.doneBuilds++;
-    } else {
-        if (status != BuildResult::DependencyFailed)
-            worker.failedBuilds++;
-    }
+    if (ex.status != BuildResult::DependencyFailed)
+        worker.failedBuilds++;
 
     worker.updateProgress();
 
-    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));
+    return amDone(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(url);
+            FileTransferRequest request(ValidURL{url});
             request.decompress = false;
 
             auto decompressor = makeDecompressionSink(unpack && hasSuffix(mainUrl, ".xz") ? "xz" : "none", sink);

src/libstore/derivation-options.cc

@@ -265,7 +265,8 @@ DerivationOptions::getParsedExportReferencesGraph(const StoreDirConfig & store)
         StorePathSet storePaths;
         for (auto & storePathS : ss) {
             if (!store.isInStore(storePathS))
-                throw BuildError("'exportReferencesGraph' contains a non-store path '%1%'", storePathS);
+                throw BuildError(
+                    BuildResult::InputRejected, "'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,28 +498,33 @@ Derivation parseDerivation(
  */
 static void printString(std::string & res, std::string_view s)
 {
-    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);
+    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 += '"';
 }
 
 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},
+                  {request.uri.to_string()},
                   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);
+            result.urls.push_back(request.uri.to_string());
 
             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.c_str());
+            curl_easy_setopt(req, CURLOPT_URL, request.uri.to_string().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 && !hasPrefix(item->request.uri, "http://") && !hasPrefix(item->request.uri, "https://"))
-            throw nix::Error("uploading to '%s' is not supported", item->request.uri);
+        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());
 
         {
             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 (hasPrefix(request.uri, "s3://")) {
+        if (request.uri.scheme() == "s3") {
             // FIXME: do this on a worker thread
             try {
 #if NIX_WITH_S3_SUPPORT
-                auto parsed = ParsedS3URL::parse(request.uri);
+                auto parsed = ParsedS3URL::parse(request.uri.parsed());
 
                 std::string profile = parsed.profile.value_or("");
                 std::string region = parsed.region.value_or(Aws::Region::US_EAST_1);
@@ -815,15 +815,16 @@ struct curlFileTransfer : public FileTransfer
                 S3Helper s3Helper(profile, region, scheme, endpoint);
 
                 // FIXME: implement ETag
-                auto s3Res = s3Helper.getObject(parsed.bucket, parsed.key);
+                auto s3Res = s3Helper.getObject(parsed.bucket, encodeUrlPath(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);
+                res.urls.push_back(request.uri.to_string());
                 callback(std::move(res));
 #else
-                throw nix::Error("cannot download '%s' because Nix is not built with S3 support", request.uri);
+                throw nix::Error(
+                    "cannot download '%s' because Nix is not built with S3 support", request.uri.to_string());
 #endif
             } catch (...) {
                 callback.rethrow();

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.authority ? cacheUri.authority->to_string() : "") + cacheUri.path,
+                .authority = cacheUri.renderAuthorityAndPath(),
             },
         .params = cacheUri.query,
     };
@@ -154,22 +154,17 @@ protected:
 
     FileTransferRequest makeRequest(const std::string & path)
     {
-        /* FIXME path is not a path, but a full relative or absolute
+        /* 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
            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.
-
-           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);
+           (note the query param) and that gets passed here. */
+        return FileTransferRequest(parseURLRelative(path, cacheUriWithTrailingSlash));
     }
 
     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,6 +36,10 @@ 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;
 
     /**
@@ -46,47 +50,6 @@ 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.
      */
@@ -131,6 +94,26 @@ 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,9 +8,33 @@
 #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().
  */
@@ -65,60 +89,15 @@ struct DerivationBuilderParams
      */
     PathsInChroot defaultPathsInChroot;
 
-    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;
-
-        /**
-         * 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.
+     * May be used to control various platform-specific functionality.
      *
-     * This is used by the caller to desugar the "structured attrs"
-     * mechanism, so `DerivationBuilder` doesn't need to know about it.
+     * For example, on Linux, the `kvm` system feature controls whether
+     * `/dev/kvm` should be exposed to the builder within the sandbox.
      */
-    std::map<std::string, EnvEntry, std::less<>> finalEnv;
+    StringSet systemFeatures;
 
-    /**
-     * 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;
+    DesugaredEnv desugaredEnv;
 };
 
 /**
@@ -138,8 +117,6 @@ struct DerivationBuilderCallbacks
      */
     virtual void closeLogFile() = 0;
 
-    virtual void appendLogTailErrorMsg(std::string & msg) = 0;
-
     /**
      * Hook up `builderOut` to some mechanism to ingest the log
      *
@@ -151,11 +128,6 @@ struct DerivationBuilderCallbacks
      * @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;
 };
 
 /**
@@ -171,11 +143,6 @@ struct DerivationBuilderCallbacks
  */
 struct DerivationBuilder : RestrictionContext
 {
-    /**
-     * The process ID of the builder.
-     */
-    Pid pid;
-
     DerivationBuilder() = default;
     virtual ~DerivationBuilder() = default;
 
@@ -208,25 +175,18 @@ 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 std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() = 0;
+    virtual SingleDrvOutputs unprepareBuild() = 0;
 
     /**
-     * Stop the in-process nix daemon thread.
-     * @see startDaemon
+     * Forcibly kill the child process, if any.
+     *
+     * @returns whether the child was still alive and needed to be
+     * killed.
      */
-    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;
+    virtual bool killChild() = 0;
 };
 
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows

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

@@ -14,6 +14,7 @@ namespace nix {
 
 using std::map;
 
+struct BuilderFailureError;
 #ifndef _WIN32 // TODO enable build hook on Windows
 struct HookInstance;
 struct DerivationBuilder;
@@ -170,9 +171,11 @@ struct DerivationBuildingGoal : public Goal
 
     void started();
 
-    Done done(BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional<Error> ex = {});
+    Done doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs);
 
-    void appendLogTailErrorMsg(std::string & msg);
+    Done doneFailure(BuildError ex);
+
+    BuildError fixupBuilderFailureErrorMessage(BuilderFailureError msg);
 
     JobCategory jobCategory() const override
     {

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

@@ -49,9 +49,6 @@ 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

@@ -0,0 +1,83 @@
+#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,9 +14,6 @@ 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
@@ -102,13 +99,9 @@ private:
 
     Co repairClosure();
 
-    /**
-     * @param builtOutput Must be set if `status` is successful.
-     */
-    Done done(
-        BuildResult::Status status,
-        std::optional<Realisation> builtOutput = std::nullopt,
-        std::optional<Error> ex = {});
+    Done doneSuccess(BuildResult::Status status, Realisation builtOutput);
+
+    Done doneFailure(BuildError ex);
 };
 
 } // namespace nix

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

@@ -9,6 +9,7 @@
 #include "nix/util/ref.hh"
 #include "nix/util/configuration.hh"
 #include "nix/util/serialise.hh"
+#include "nix/util/url.hh"
 
 namespace nix {
 
@@ -70,7 +71,7 @@ extern const unsigned int RETRY_TIME_MS_DEFAULT;
 
 struct FileTransferRequest
 {
-    std::string uri;
+    ValidURL uri;
     Headers headers;
     std::string expectedETag;
     bool verifyTLS = true;
@@ -84,8 +85,8 @@ struct FileTransferRequest
     std::string mimeType;
     std::function<void(std::string_view data)> dataCallback;
 
-    FileTransferRequest(std::string_view uri)
-        : uri(uri)
+    FileTransferRequest(ValidURL uri)
+        : uri(std::move(uri))
         , parentAct(getCurActivity())
     {
     }
@@ -111,6 +112,9 @@ 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,3 +1,6 @@
+#pragma once
+///@file
+
 #include "nix/util/url.hh"
 #include "nix/store/binary-cache-store.hh"
 

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

@@ -15,6 +15,7 @@ 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,7 +54,12 @@ struct S3Helper
 struct ParsedS3URL
 {
     std::string bucket;
-    std::string key;
+    /**
+     * @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::optional<std::string> profile;
     std::optional<std::string> region;
     std::optional<std::string> scheme;
@@ -74,7 +79,13 @@ struct ParsedS3URL
             endpoint);
     }
 
-    static ParsedS3URL parse(std::string_view uri);
+    static ParsedS3URL parse(const ParsedURL & uri);
+
+    /**
+     * Convert this ParsedS3URL to HTTPS ParsedURL for use with curl's AWS SigV4 authentication
+     */
+    ParsedURL toHttpsUrl() const;
+
     auto operator<=>(const ParsedS3URL & other) const = default;
 };
 

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

@@ -24,11 +24,6 @@
 
 namespace nix {
 
-MakeError(SubstError, Error);
-/**
- * denotes a permanent build failure
- */
-MakeError(BuildError, Error);
 MakeError(InvalidPath, Error);
 MakeError(Unsupported, Error);
 MakeError(SubstituteGone, Error);

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

@@ -77,12 +77,22 @@ struct StoreReference
      */
     std::string render(bool withParams = true) const;
 
+    std::string to_string() const
+    {
+        return render();
+    }
+
     /**
      * Parse a URI into a store reference.
      */
     static StoreReference parse(const std::string & uri, const Params & extraParams = Params{});
 };
 
+static inline std::ostream & operator<<(std::ostream & os, const StoreReference & ref)
+{
+    return os << ref.render();
+}
+
 /**
  * Split URI into protocol+hierarchy part and its parameter set.
  */

src/libstore/local-store.cc

@@ -1002,7 +1002,10 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
             }},
             {[&](const StorePath & path, const StorePath & parent) {
                 return BuildError(
-                    "cycle detected in the references of '%s' from '%s'", printStorePath(path), printStorePath(parent));
+                    BuildResult::OutputRejected,
+                    "cycle detected in the references of '%s' from '%s'",
+                    printStorePath(path),
+                    printStorePath(parent));
             }});
 
         txn.commit();

src/libstore/meson.build

@@ -265,6 +265,8 @@ sources = files(
   'binary-cache-store.cc',
   'build-result.cc',
   'build/derivation-building-goal.cc',
+  'build/derivation-check.cc',
+  'build/derivation-env-desugar.cc',
   'build/derivation-goal.cc',
   'build/derivation-trampoline-goal.cc',
   'build/drv-output-substitution-goal.cc',

src/libstore/misc.cc

@@ -322,7 +322,10 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths)
         }},
         {[&](const StorePath & path, const StorePath & parent) {
             return BuildError(
-                "cycle detected in the references of '%s' from '%s'", printStorePath(path), printStorePath(parent));
+                BuildResult::OutputRejected,
+                "cycle detected in the references of '%s' from '%s'",
+                printStorePath(path),
+                printStorePath(parent));
         }});
 }
 

src/libstore/posix-fs-canonicalise.cc

@@ -98,7 +98,7 @@ static void canonicalisePathMetaData_(
        (i.e. "touch $out/foo; ln $out/foo $out/bar"). */
     if (uidRange && (st.st_uid < uidRange->first || st.st_uid > uidRange->second)) {
         if (S_ISDIR(st.st_mode) || !inodesSeen.count(Inode(st.st_dev, st.st_ino)))
-            throw BuildError("invalid ownership on file '%1%'", path);
+            throw BuildError(BuildResult::OutputRejected, "invalid ownership on file '%1%'", path);
         mode_t mode = st.st_mode & ~S_IFMT;
         assert(
             S_ISLNK(st.st_mode)

src/libstore/s3.cc

@@ -1,6 +1,11 @@
 #include "nix/store/s3.hh"
 #include "nix/util/split.hh"
 #include "nix/util/url.hh"
+#include "nix/util/util.hh"
+#include "nix/util/canon-path.hh"
+#include "nix/util/strings-inline.hh"
+
+#include <ranges>
 
 namespace nix {
 
@@ -8,10 +13,8 @@ using namespace std::string_view_literals;
 
 #if NIX_WITH_S3_SUPPORT
 
-ParsedS3URL ParsedS3URL::parse(std::string_view uri)
+ParsedS3URL ParsedS3URL::parse(const ParsedURL & parsed)
 try {
-    auto parsed = parseURL(uri);
-
     if (parsed.scheme != "s3"sv)
         throw BadURL("URI scheme '%s' is not 's3'", parsed.scheme);
 
@@ -24,10 +27,6 @@ try {
         || parsed.authority->hostType != ParsedURL::Authority::HostType::Name)
         throw BadURL("URI has a missing or invalid bucket name");
 
-    std::string_view key = parsed.path;
-    /* Make the key a relative path. */
-    splitPrefix(key, "/");
-
     /* TODO: Validate the key against:
      * https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines
      */
@@ -41,10 +40,14 @@ try {
     };
 
     auto endpoint = getOptionalParam("endpoint");
+    if (parsed.path.size() <= 1 || !parsed.path.front().empty())
+        throw BadURL("URI has a missing or invalid key");
+
+    auto path = std::views::drop(parsed.path, 1) | std::ranges::to<std::vector<std::string>>();
 
     return ParsedS3URL{
-        .bucket = std::move(parsed.authority->host),
-        .key = std::string{key},
+        .bucket = parsed.authority->host,
+        .key = std::move(path),
         .profile = getOptionalParam("profile"),
         .region = getOptionalParam("region"),
         .scheme = getOptionalParam("scheme"),
@@ -62,10 +65,57 @@ try {
         }(),
     };
 } catch (BadURL & e) {
-    e.addTrace({}, "while parsing S3 URI: '%s'", uri);
+    e.addTrace({}, "while parsing S3 URI: '%s'", parsed.to_string());
     throw;
 }
 
+ParsedURL ParsedS3URL::toHttpsUrl() const
+{
+    auto toView = [](const auto & x) { return std::string_view{x}; };
+
+    auto regionStr = region.transform(toView).value_or("us-east-1");
+    auto schemeStr = scheme.transform(toView).value_or("https");
+
+    // Handle endpoint configuration using std::visit
+    return std::visit(
+        overloaded{
+            [&](const std::monostate &) {
+                // No custom endpoint, use standard AWS S3 endpoint
+                std::vector<std::string> path{""};
+                path.push_back(bucket);
+                path.insert(path.end(), key.begin(), key.end());
+                return ParsedURL{
+                    .scheme = std::string{schemeStr},
+                    .authority = ParsedURL::Authority{.host = "s3." + regionStr + ".amazonaws.com"},
+                    .path = std::move(path),
+                };
+            },
+            [&](const ParsedURL::Authority & auth) {
+                // Endpoint is just an authority (hostname/port)
+                std::vector<std::string> path{""};
+                path.push_back(bucket);
+                path.insert(path.end(), key.begin(), key.end());
+                return ParsedURL{
+                    .scheme = std::string{schemeStr},
+                    .authority = auth,
+                    .path = std::move(path),
+                };
+            },
+            [&](const ParsedURL & endpointUrl) {
+                // Endpoint is already a ParsedURL (e.g., http://server:9000)
+                auto path = endpointUrl.path;
+                path.push_back(bucket);
+                path.insert(path.end(), key.begin(), key.end());
+                return ParsedURL{
+                    .scheme = endpointUrl.scheme,
+                    .authority = endpointUrl.authority,
+                    .path = std::move(path),
+                };
+            },
+        },
+        endpoint);
+}
+
 #endif
 
 } // namespace nix

src/libstore/sqlite.cc

@@ -4,6 +4,10 @@
 #include "nix/util/url.hh"
 #include "nix/util/signals.hh"
 
+#ifdef __linux__
+#  include <sys/vfs.h>
+#endif
+
 #include <sqlite3.h>
 
 #include <atomic>
@@ -60,6 +64,28 @@ static void traceSQL(void * x, const char * sql)
 
 SQLite::SQLite(const std::filesystem::path & path, SQLiteOpenMode mode)
 {
+    // Work around a ZFS issue where SQLite's truncate() call on
+    // db.sqlite-shm can randomly take up to a few seconds. See
+    // https://github.com/openzfs/zfs/issues/14290#issuecomment-3074672917.
+    // Remove this workaround when a fix is widely installed, perhaps 2027? Candidate:
+    // https://github.com/search?q=repo%3Aopenzfs%2Fzfs+%22Linux%3A+zfs_putpage%3A+complete+async+page+writeback+immediately%22&type=commits
+#ifdef __linux__
+    try {
+        auto shmFile = path;
+        shmFile += "-shm";
+        AutoCloseFD fd = open(shmFile.string().c_str(), O_RDWR | O_CLOEXEC);
+        if (fd) {
+            struct statfs fs;
+            if (fstatfs(fd.get(), &fs))
+                throw SysError("statfs() on '%s'", shmFile);
+            if (fs.f_type == /* ZFS_SUPER_MAGIC */ 801189825 && fdatasync(fd.get()) != 0)
+                throw SysError("fsync() on '%s'", shmFile);
+        }
+    } catch (...) {
+        throw;
+    }
+#endif
+
     // useSQLiteWAL also indicates what virtual file system we need.  Using
     // `unix-dotfile` is needed on NFS file systems and on Windows' Subsystem
     // for Linux (WSL) where useSQLiteWAL should be false by default.

src/libstore/store-api.cc

@@ -770,6 +770,7 @@ StorePathSet Store::exportReferences(const StorePathSet & storePaths, const Stor
     for (auto & storePath : storePaths) {
         if (!inputPaths.count(storePath))
             throw BuildError(
+                BuildResult::InputRejected,
                 "cannot export references of path '%s' because it is not in the input closure of the derivation",
                 printStorePath(storePath));
 

src/libstore/store-reference.cc

@@ -45,16 +45,14 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen
 {
     auto params = extraParams;
     try {
-        auto parsedUri = parseURL(uri);
+        auto parsedUri = parseURL(uri, /*lenient=*/true);
         params.insert(parsedUri.query.begin(), parsedUri.query.end());
 
-        auto baseURI = parsedUri.authority.value_or(ParsedURL::Authority{}).to_string() + parsedUri.path;
-
         return {
             .variant =
                 Specified{
                     .scheme = std::move(parsedUri.scheme),
-                    .authority = std::move(baseURI),
+                    .authority = parsedUri.renderAuthorityAndPath(),
                 },
             .params = std::move(params),
         };
@@ -107,7 +105,7 @@ std::pair<std::string, StoreReference::Params> splitUriAndParams(const std::stri
     StoreReference::Params params;
     auto q = uri.find('?');
     if (q != std::string::npos) {
-        params = decodeQuery(uri.substr(q + 1));
+        params = decodeQuery(uri.substr(q + 1), /*lenient=*/true);
         uri = uri_.substr(0, q);
     }
     return {uri, params};

src/libstore/unix/build/chroot-derivation-builder.cc

@@ -22,13 +22,6 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
 
     PathsInChroot pathsInChroot;
 
-    void deleteTmpDir(bool force) override
-    {
-        autoDelChroot.reset(); /* this runs the destructor */
-
-        DerivationBuilderImpl::deleteTmpDir(force);
-    }
-
     bool needsHashRewrite() override
     {
         return false;
@@ -166,13 +159,13 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
         return !needsHashRewrite() ? chrootRootDir + p : store.toRealPath(p);
     }
 
-    void cleanupBuild() override
+    void cleanupBuild(bool force) override
     {
-        DerivationBuilderImpl::cleanupBuild();
+        DerivationBuilderImpl::cleanupBuild(force);
 
         /* Move paths out of the chroot for easier debugging of
            build failures. */
-        if (buildMode == bmNormal)
+        if (!force && buildMode == bmNormal)
             for (auto & [_, status] : initialOutputs) {
                 if (!status.known)
                     continue;
@@ -182,6 +175,8 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
                 if (pathExists(chrootRootDir + p))
                     std::filesystem::rename((chrootRootDir + p), p);
             }
+
+        autoDelChroot.reset(); /* this runs the destructor */
     }
 
     std::pair<Path, Path> addDependencyPrep(const StorePath & path)

src/libstore/unix/build/derivation-builder.cc

@@ -17,6 +17,7 @@
 #include "nix/store/restricted-store.hh"
 #include "nix/store/user-lock.hh"
 #include "nix/store/globals.hh"
+#include "nix/store/build/derivation-env-desugar.hh"
 
 #include <queue>
 
@@ -42,10 +43,17 @@
 #include "nix/util/signals.hh"
 
 #include "store-config-private.hh"
+#include "build/derivation-check.hh"
 
 namespace nix {
 
-MakeError(NotDeterministic, BuildError);
+struct NotDeterministic : BuildError
+{
+    NotDeterministic(auto &&... args)
+        : BuildError(BuildResult::NotDeterministic, args...)
+    {
+    }
+};
 
 /**
  * This class represents the state for building locally.
@@ -63,6 +71,11 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder
 {
 protected:
 
+    /**
+     * The process ID of the builder.
+     */
+    Pid pid;
+
     LocalStore & store;
 
     std::unique_ptr<DerivationBuilderCallbacks> miscMethods;
@@ -78,6 +91,27 @@ public:
     {
     }
 
+    ~DerivationBuilderImpl()
+    {
+        /* Careful: we should never ever throw an exception from a
+           destructor. */
+        try {
+            killChild();
+        } catch (...) {
+            ignoreExceptionInDestructor();
+        }
+        try {
+            stopDaemon();
+        } catch (...) {
+            ignoreExceptionInDestructor();
+        }
+        try {
+            cleanupBuild(false);
+        } catch (...) {
+            ignoreExceptionInDestructor();
+        }
+    }
+
 protected:
 
     /**
@@ -184,7 +218,7 @@ public:
 
     void startBuilder() override;
 
-    std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() override;
+    SingleDrvOutputs unprepareBuild() override;
 
 protected:
 
@@ -278,9 +312,11 @@ private:
      */
     void startDaemon();
 
-public:
-
-    void stopDaemon() override;
+    /**
+     * Stop the in-process nix daemon thread.
+     * @see startDaemon
+     */
+    void stopDaemon();
 
 protected:
 
@@ -335,22 +371,25 @@ private:
      */
     SingleDrvOutputs registerOutputs();
 
+protected:
+
     /**
-     * Check that an output meets the requirements specified by the
-     * 'outputChecks' attribute (or the legacy
-     * '{allowed,disallowed}{References,Requisites}' attributes).
+     * Delete the temporary directory, if we have one.
+     *
+     * @param force We know the build suceeded, so don't attempt to
+     * preseve anything for debugging.
      */
-    void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs);
+    virtual void cleanupBuild(bool force);
+
+    /**
+     * Kill any processes running under the build user UID or in the
+     * cgroup of the build.
+     */
+    virtual void killSandbox(bool getStats);
 
 public:
 
-    void deleteTmpDir(bool force) override;
-
-    void killSandbox(bool getStats) override;
-
-protected:
-
-    virtual void cleanupBuild();
+    bool killChild() override;
 
 private:
 
@@ -413,6 +452,24 @@ void DerivationBuilderImpl::killSandbox(bool getStats)
     }
 }
 
+bool DerivationBuilderImpl::killChild()
+{
+    bool ret = pid != -1;
+    if (ret) {
+        /* If we're using a build user, then there is a tricky race
+           condition: if we kill the build user before the child has
+           done its setuid() to the build user uid, then it won't be
+           killed, and we'll potentially lock up in pid.wait().  So
+           also send a conventional kill to the child. */
+        ::kill(-pid, SIGKILL); /* ignore the result */
+
+        killSandbox(true);
+
+        pid.wait();
+    }
+    return ret;
+}
+
 bool DerivationBuilderImpl::prepareBuild()
 {
     if (useBuildUsers()) {
@@ -426,16 +483,8 @@ bool DerivationBuilderImpl::prepareBuild()
     return true;
 }
 
-std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> DerivationBuilderImpl::unprepareBuild()
+SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
 {
-    // FIXME: get rid of this, rely on RAII.
-    Finally releaseBuildUser([&]() {
-        /* Release the build user at the end of this function. We don't do
-           it right away because we don't want another build grabbing this
-           uid and then messing around with our output. */
-        buildUser.reset();
-    });
-
     /* Since we got an EOF on the logger pipe, the builder is presumed
        to have terminated.  In fact, the builder could also have
        simply have closed its end of the pipe, so just to be sure,
@@ -475,63 +524,28 @@ std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> Derivation
             ((double) buildResult.cpuSystem->count()) / 1000000);
     }
 
-    bool diskFull = false;
+    /* Check the exit status. */
+    if (!statusOk(status)) {
 
-    try {
+        /* Check *before* cleaning up. */
+        bool diskFull = decideWhetherDiskFull();
 
-        /* Check the exit status. */
-        if (!statusOk(status)) {
+        cleanupBuild(false);
 
-            diskFull |= decideWhetherDiskFull();
-
-            cleanupBuild();
-
-            auto msg =
-                fmt("Cannot build '%s'.\n"
-                    "Reason: " ANSI_RED "builder %s" ANSI_NORMAL ".",
-                    Magenta(store.printStorePath(drvPath)),
-                    statusToString(status));
-
-            msg += showKnownOutputs(store, drv);
-
-            miscMethods->appendLogTailErrorMsg(msg);
-
-            if (diskFull)
-                msg += "\nnote: build failure may have been caused by lack of free disk space";
-
-            throw BuildError(msg);
-        }
-
-        /* Compute the FS closure of the outputs and register them as
-           being valid. */
-        auto builtOutputs = registerOutputs();
-
-        StorePathSet outputPaths;
-        for (auto & [_, output] : builtOutputs)
-            outputPaths.insert(output.outPath);
-        runPostBuildHook(store, *logger, drvPath, outputPaths);
-
-        /* Delete unused redirected outputs (when doing hash rewriting). */
-        for (auto & i : redirectedOutputs)
-            deletePath(store.Store::toRealPath(i.second));
-
-        deleteTmpDir(true);
-
-        return std::move(builtOutputs);
-
-    } catch (BuildError & e) {
-        BuildResult::Status st = dynamic_cast<NotDeterministic *>(&e)        ? BuildResult::NotDeterministic
-                                 : statusOk(status)                          ? BuildResult::OutputRejected
-                                 : !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure
-                                                                             : BuildResult::PermanentFailure;
-
-        return std::pair{std::move(st), std::move(e)};
+        throw BuilderFailureError{
+            !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure,
+            status,
+            diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "",
+        };
     }
-}
 
-void DerivationBuilderImpl::cleanupBuild()
-{
-    deleteTmpDir(false);
+    /* Compute the FS closure of the outputs and register them as
+       being valid. */
+    auto builtOutputs = registerOutputs();
+
+    cleanupBuild(true);
+
+    return builtOutputs;
 }
 
 static void chmod_(const Path & path, mode_t mode)
@@ -693,7 +707,7 @@ void DerivationBuilderImpl::startBuilder()
                 fmt("\nNote: run `%s` to run programs for x86_64-darwin",
                     Magenta("/usr/sbin/softwareupdate --install-rosetta && launchctl stop org.nixos.nix-daemon"));
 
-        throw BuildError(msg);
+        throw BuildError(BuildResult::InputRejected, msg);
     }
 
     auto buildDir = store.config->getBuildDir();
@@ -1003,19 +1017,13 @@ void DerivationBuilderImpl::initEnv()
     /* Write the final environment. Note that this is intentionally
        *not* `drv.env`, because we've desugared things like like
        "passAFile", "expandReferencesGraph", structured attrs, etc. */
-    for (const auto & [name, info] : finalEnv) {
-        if (info.nameOfPassAsFile) {
-            auto & fileName = *info.nameOfPassAsFile;
-            writeBuilderFile(fileName, rewriteStrings(info.value, inputRewrites));
-            env[name] = tmpDirInSandbox() + "/" + fileName;
-        } else {
-            env[name] = info.value;
-        }
+    for (const auto & [name, info] : desugaredEnv.variables) {
+        env[name] = info.prependBuildDirectory ? tmpDirInSandbox() + "/" + info.value : info.value;
     }
 
     /* Add extra files, similar to `finalEnv` */
-    for (const auto & [fileName, value] : extraFiles) {
-        writeBuilderFile(fileName, value);
+    for (const auto & [fileName, value] : desugaredEnv.extraFiles) {
+        writeBuilderFile(fileName, rewriteStrings(value, inputRewrites));
     }
 
     /* For convenience, set an environment pointing to the top build
@@ -1334,8 +1342,6 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
        outputs to allow hard links between outputs. */
     InodesSeen inodesSeen;
 
-    std::exception_ptr delayedException;
-
     /* The paths that can be referenced are the input closures, the
        output paths, and any paths that have been built via recursive
        Nix calls. */
@@ -1389,6 +1395,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         auto optSt = maybeLstat(actualPath.c_str());
         if (!optSt)
             throw BuildError(
+                BuildResult::OutputRejected,
                 "builder for '%s' failed to produce output path for output '%s' at '%s'",
                 store.printStorePath(drvPath),
                 outputName,
@@ -1403,6 +1410,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         if ((!S_ISLNK(st.st_mode) && (st.st_mode & (S_IWGRP | S_IWOTH)))
             || (buildUser && st.st_uid != buildUser->getUID()))
             throw BuildError(
+                BuildResult::OutputRejected,
                 "suspicious ownership or permission on '%s' for output '%s'; rejecting this build output",
                 actualPath,
                 outputName);
@@ -1439,7 +1447,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         {[&](const std::string & name) {
             auto orifu = get(outputReferencesIfUnregistered, name);
             if (!orifu)
-                throw BuildError("no output reference for '%s' in build of '%s'", name, store.printStorePath(drvPath));
+                throw BuildError(
+                    BuildResult::OutputRejected,
+                    "no output reference for '%s' in build of '%s'",
+                    name,
+                    store.printStorePath(drvPath));
             return std::visit(
                 overloaded{
                     /* Since we'll use the already installed versions of these, we
@@ -1461,6 +1473,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         {[&](const std::string & path, const std::string & parent) {
             // TODO with more -vvvv also show the temporary paths for manual inspection.
             return BuildError(
+                BuildResult::OutputRejected,
                 "cycle detected in build of '%s' in the references of output '%s' from output '%s'",
                 store.printStorePath(drvPath),
                 path,
@@ -1554,11 +1567,12 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo {
             auto st = get(outputStats, outputName);
             if (!st)
-                throw BuildError("output path %1% without valid stats info", actualPath);
+                throw BuildError(BuildResult::OutputRejected, "output path %1% without valid stats info", actualPath);
             if (outputHash.method.getFileIngestionMethod() == FileIngestionMethod::Flat) {
                 /* The output path should be a regular file without execute permission. */
                 if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0)
                     throw BuildError(
+                        BuildResult::OutputRejected,
                         "output path '%1%' should be a non-executable regular file "
                         "since recursive hashing is not enabled (one of outputHashMode={flat,text} is true)",
                         actualPath);
@@ -1646,35 +1660,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
 
                     std::filesystem::rename(tmpOutput, actualPath);
 
-                    auto newInfo0 = newInfoFromCA(
+                    return newInfoFromCA(
                         DerivationOutput::CAFloating{
                             .method = dof.ca.method,
                             .hashAlgo = wanted.algo,
                         });
-
-                    /* Check wanted hash */
-                    assert(newInfo0.ca);
-                    auto & got = newInfo0.ca->hash;
-                    if (wanted != got) {
-                        /* Throw an error after registering the path as
-                           valid. */
-                        miscMethods->noteHashMismatch();
-                        delayedException = std::make_exception_ptr(BuildError(
-                            "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 (!newInfo0.references.empty()) {
-                        auto numViolations = newInfo.references.size();
-                        delayedException = std::make_exception_ptr(BuildError(
-                            "fixed-output derivations must not reference store paths: '%s' references %d distinct paths, e.g. '%s'",
-                            store.printStorePath(drvPath),
-                            numViolations,
-                            store.printStorePath(*newInfo.references.begin())));
-                    }
-
-                    return newInfo0;
                 },
 
                 [&](const DerivationOutput::CAFloating & dof) { return newInfoFromCA(dof); },
@@ -1738,85 +1728,90 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         }
 
         if (buildMode == bmCheck) {
+            /* Check against already registered outputs */
 
-            if (!store.isValidPath(newInfo.path))
-                continue;
-            ValidPathInfo oldInfo(*store.queryPathInfo(newInfo.path));
-            if (newInfo.narHash != oldInfo.narHash) {
-                miscMethods->noteCheckMismatch();
-                if (settings.runDiffHook || settings.keepFailed) {
-                    auto dst = store.toRealPath(finalDestPath + ".check");
-                    deletePath(dst);
-                    movePath(actualPath, dst);
+            if (store.isValidPath(newInfo.path)) {
+                ValidPathInfo oldInfo(*store.queryPathInfo(newInfo.path));
+                if (newInfo.narHash != oldInfo.narHash) {
+                    if (settings.runDiffHook || settings.keepFailed) {
+                        auto dst = store.toRealPath(finalDestPath + ".check");
+                        deletePath(dst);
+                        movePath(actualPath, dst);
 
-                    handleDiffHook(
-                        buildUser ? buildUser->getUID() : getuid(),
-                        buildUser ? buildUser->getGID() : getgid(),
-                        finalDestPath,
-                        dst,
-                        store.printStorePath(drvPath),
-                        tmpDir);
+                        handleDiffHook(
+                            buildUser ? buildUser->getUID() : getuid(),
+                            buildUser ? buildUser->getGID() : getgid(),
+                            finalDestPath,
+                            dst,
+                            store.printStorePath(drvPath),
+                            tmpDir);
 
-                    throw NotDeterministic(
-                        "derivation '%s' may not be deterministic: output '%s' differs from '%s'",
-                        store.printStorePath(drvPath),
-                        store.toRealPath(finalDestPath),
-                        dst);
-                } else
-                    throw NotDeterministic(
-                        "derivation '%s' may not be deterministic: output '%s' differs",
-                        store.printStorePath(drvPath),
-                        store.toRealPath(finalDestPath));
+                        throw NotDeterministic(
+                            "derivation '%s' may not be deterministic: output '%s' differs from '%s'",
+                            store.printStorePath(drvPath),
+                            store.toRealPath(finalDestPath),
+                            dst);
+                    } else
+                        throw NotDeterministic(
+                            "derivation '%s' may not be deterministic: output '%s' differs",
+                            store.printStorePath(drvPath),
+                            store.toRealPath(finalDestPath));
+                }
+
+                /* Since we verified the build, it's now ultimately trusted. */
+                if (!oldInfo.ultimate) {
+                    oldInfo.ultimate = true;
+                    store.signPathInfo(oldInfo);
+                    store.registerValidPaths({{oldInfo.path, oldInfo}});
+                }
+            }
+        } else {
+            /* do tasks relating to registering these outputs */
+
+            /* For debugging, print out the referenced and unreferenced paths. */
+            for (auto & i : inputPaths) {
+                if (references.count(i))
+                    debug("referenced input: '%1%'", store.printStorePath(i));
+                else
+                    debug("unreferenced input: '%1%'", store.printStorePath(i));
             }
 
-            /* Since we verified the build, it's now ultimately trusted. */
-            if (!oldInfo.ultimate) {
-                oldInfo.ultimate = true;
-                store.signPathInfo(oldInfo);
-                store.registerValidPaths({{oldInfo.path, oldInfo}});
-            }
+            store.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences()
 
-            continue;
+            newInfo.deriver = drvPath;
+            newInfo.ultimate = true;
+            store.signPathInfo(newInfo);
+
+            finish(newInfo.path);
+
+            /* If it's a CA path, register it right away. This is necessary if it
+               isn't statically known so that we can safely unlock the path before
+               the next iteration
+
+               This is also good so that if a fixed-output produces the
+               wrong path, we still store the result (just don't consider
+               the derivation sucessful, so if someone fixes the problem by
+               just changing the wanted hash, the redownload (or whateer
+               possibly quite slow thing it was) doesn't have to be done
+               again. */
+            if (newInfo.ca)
+                store.registerValidPaths({{newInfo.path, newInfo}});
         }
 
-        /* For debugging, print out the referenced and unreferenced paths. */
-        for (auto & i : inputPaths) {
-            if (references.count(i))
-                debug("referenced input: '%1%'", store.printStorePath(i));
-            else
-                debug("unreferenced input: '%1%'", store.printStorePath(i));
-        }
-
-        store.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences()
-        miscMethods->markContentsGood(newInfo.path);
-
-        newInfo.deriver = drvPath;
-        newInfo.ultimate = true;
-        store.signPathInfo(newInfo);
-
-        finish(newInfo.path);
-
-        /* If it's a CA path, register it right away. This is necessary if it
-           isn't statically known so that we can safely unlock the path before
-           the next iteration */
-        if (newInfo.ca)
-            store.registerValidPaths({{newInfo.path, newInfo}});
-
+        /* Do this in both the check and non-check cases, because we
+           want `checkOutputs` below to work, which needs these path
+           infos. */
         infos.emplace(outputName, std::move(newInfo));
     }
 
+    /* Apply output checks. This includes checking of the wanted vs got
+       hash of fixed-outputs. */
+    checkOutputs(store, drvPath, drv.outputs, drvOptions.outputChecks, infos);
+
     if (buildMode == bmCheck) {
-        /* In case of fixed-output derivations, if there are
-           mismatches on `--check` an error must be thrown as this is
-           also a source for non-determinism. */
-        if (delayedException)
-            std::rethrow_exception(delayedException);
         return {};
     }
 
-    /* Apply output checks. */
-    checkOutputs(infos);
-
     /* Register each output path as valid, and register the sets of
        paths referenced by each of them.  If there are cycles in the
        outputs, this will fail. */
@@ -1828,16 +1823,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         store.registerValidPaths(infos2);
     }
 
-    /* In case of a fixed-output derivation hash mismatch, throw an
-       exception now that we have registered the output as valid. */
-    if (delayedException)
-        std::rethrow_exception(delayedException);
-
-    /* If we made it this far, we are sure the output matches the derivation
-       (since the delayedException would be a fixed output CA mismatch). That
-       means it's safe to link the derivation to the output hash. We must do
-       that for floating CA derivations, which otherwise couldn't be cached,
-       but it's fine to do in all cases. */
+    /* If we made it this far, we are sure the output matches the
+       derivation That means it's safe to link the derivation to the
+       output hash. We must do that for floating CA derivations, which
+       otherwise couldn't be cached, but it's fine to do in all cases.
+       */
     SingleDrvOutputs builtOutputs;
 
     for (auto & [outputName, newInfo] : infos) {
@@ -1854,151 +1844,14 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
     return builtOutputs;
 }
 
-void DerivationBuilderImpl::checkOutputs(const std::map<std::string, ValidPathInfo> & outputs)
+void DerivationBuilderImpl::cleanupBuild(bool force)
 {
-    std::map<Path, const ValidPathInfo &> outputsByPath;
-    for (auto & output : outputs)
-        outputsByPath.emplace(store.printStorePath(output.second.path), output.second);
-
-    for (auto & output : outputs) {
-        auto & outputName = output.first;
-        auto & info = output.second;
-
-        /* 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(
-                    "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(
-                        "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(
-                            "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(
-                        "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);
-                },
-            },
-            drvOptions.outputChecks);
+    if (force) {
+        /* Delete unused redirected outputs (when doing hash rewriting). */
+        for (auto & i : redirectedOutputs)
+            deletePath(store.Store::toRealPath(i.second));
     }
-}
 
-void DerivationBuilderImpl::deleteTmpDir(bool force)
-{
     if (topTmpDir != "") {
         /* As an extra precaution, even in the event of `deletePath` failing to
          * clean up, the `tmpDir` will be chowned as if we were to move

src/libstore/unix/build/linux-derivation-builder.cc

@@ -492,7 +492,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
             createDirs(chrootRootDir + "/dev/shm");
             createDirs(chrootRootDir + "/dev/pts");
             ss.push_back("/dev/full");
-            if (store.Store::config.systemFeatures.get().count("kvm") && pathExists("/dev/kvm"))
+            if (systemFeatures.count("kvm") && pathExists("/dev/kvm"))
                 ss.push_back("/dev/kvm");
             ss.push_back("/dev/null");
             ss.push_back("/dev/random");
@@ -659,7 +659,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
             throw SysError("setuid failed");
     }
 
-    std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() override
+    SingleDrvOutputs unprepareBuild() override
     {
         sandboxMountNamespace = -1;
         sandboxUserNamespace = -1;

src/libutil-tests/strings.cc

@@ -2,6 +2,7 @@
 #include <rapidcheck/gtest.h>
 
 #include "nix/util/strings.hh"
+#include "nix/util/strings-inline.hh"
 #include "nix/util/error.hh"
 
 namespace nix {
@@ -271,113 +272,122 @@ TEST(tokenizeString, tokenizeSepEmpty)
  * splitString
  * --------------------------------------------------------------------------*/
 
-TEST(splitString, empty)
-{
-    Strings expected = {""};
+using SplitStringTestContainerTypes = ::testing::
+    Types<std::vector<std::string>, std::vector<std::string_view>, std::list<std::string>, std::list<std::string_view>>;
 
-    ASSERT_EQ(splitString<Strings>("", " \t\n\r"), expected);
+template<typename T>
+class splitStringTest : public ::testing::Test
+{};
+
+TYPED_TEST_SUITE(splitStringTest, SplitStringTestContainerTypes);
+
+TYPED_TEST(splitStringTest, empty)
+{
+    TypeParam expected = {""};
+
+    EXPECT_EQ(splitString<TypeParam>("", " \t\n\r"), expected);
 }
 
-TEST(splitString, oneSep)
+TYPED_TEST(splitStringTest, oneSep)
 {
-    Strings expected = {"", ""};
+    TypeParam expected = {"", ""};
 
-    ASSERT_EQ(splitString<Strings>(" ", " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(" ", " \t\n\r"), expected);
 }
 
-TEST(splitString, twoSep)
+TYPED_TEST(splitStringTest, twoSep)
 {
-    Strings expected = {"", "", ""};
+    TypeParam expected = {"", "", ""};
 
-    ASSERT_EQ(splitString<Strings>(" \n", " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(" \n", " \t\n\r"), expected);
 }
 
-TEST(splitString, tokenizeSpacesWithSpaces)
+TYPED_TEST(splitStringTest, tokenizeSpacesWithSpaces)
 {
     auto s = "foo bar baz";
-    Strings expected = {"foo", "bar", "baz"};
+    TypeParam expected = {"foo", "bar", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, " \t\n\r"), expected);
 }
 
-TEST(splitString, tokenizeTabsWithDefaults)
+TYPED_TEST(splitStringTest, tokenizeTabsWithDefaults)
 {
     auto s = "foo\tbar\tbaz";
     // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
-    Strings expected = {"foo", "bar", "baz"};
+    TypeParam expected = {"foo", "bar", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, " \t\n\r"), expected);
 }
 
-TEST(splitString, tokenizeTabsSpacesWithDefaults)
+TYPED_TEST(splitStringTest, tokenizeTabsSpacesWithDefaults)
 {
     auto s = "foo\t bar\t baz";
     // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
-    Strings expected = {"foo", "", "bar", "", "baz"};
+    TypeParam expected = {"foo", "", "bar", "", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, " \t\n\r"), expected);
 }
 
-TEST(splitString, tokenizeTabsSpacesNewlineWithDefaults)
+TYPED_TEST(splitStringTest, tokenizeTabsSpacesNewlineWithDefaults)
 {
     auto s = "foo\t\n bar\t\n baz";
     // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
-    Strings expected = {"foo", "", "", "bar", "", "", "baz"};
+    TypeParam expected = {"foo", "", "", "bar", "", "", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, " \t\n\r"), expected);
 }
 
-TEST(splitString, tokenizeTabsSpacesNewlineRetWithDefaults)
+TYPED_TEST(splitStringTest, tokenizeTabsSpacesNewlineRetWithDefaults)
 {
     auto s = "foo\t\n\r bar\t\n\r baz";
     // Using it like this is weird, but shows the difference with tokenizeString, which also has this test
-    Strings expected = {"foo", "", "", "", "bar", "", "", "", "baz"};
+    TypeParam expected = {"foo", "", "", "", "bar", "", "", "", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, " \t\n\r"), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, " \t\n\r"), expected);
 
     auto s2 = "foo \t\n\r bar \t\n\r baz";
-    Strings expected2 = {"foo", "", "", "", "", "bar", "", "", "", "", "baz"};
+    TypeParam expected2 = {"foo", "", "", "", "", "bar", "", "", "", "", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s2, " \t\n\r"), expected2);
+    EXPECT_EQ(splitString<TypeParam>(s2, " \t\n\r"), expected2);
 }
 
-TEST(splitString, tokenizeWithCustomSep)
+TYPED_TEST(splitStringTest, tokenizeWithCustomSep)
 {
     auto s = "foo\n,bar\n,baz\n";
-    Strings expected = {"foo\n", "bar\n", "baz\n"};
+    TypeParam expected = {"foo\n", "bar\n", "baz\n"};
 
-    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, ","), expected);
 }
 
-TEST(splitString, tokenizeSepAtStart)
+TYPED_TEST(splitStringTest, tokenizeSepAtStart)
 {
     auto s = ",foo,bar,baz";
-    Strings expected = {"", "foo", "bar", "baz"};
+    TypeParam expected = {"", "foo", "bar", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, ","), expected);
 }
 
-TEST(splitString, tokenizeSepAtEnd)
+TYPED_TEST(splitStringTest, tokenizeSepAtEnd)
 {
     auto s = "foo,bar,baz,";
-    Strings expected = {"foo", "bar", "baz", ""};
+    TypeParam expected = {"foo", "bar", "baz", ""};
 
-    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, ","), expected);
 }
 
-TEST(splitString, tokenizeSepEmpty)
+TYPED_TEST(splitStringTest, tokenizeSepEmpty)
 {
     auto s = "foo,,baz";
-    Strings expected = {"foo", "", "baz"};
+    TypeParam expected = {"foo", "", "baz"};
 
-    ASSERT_EQ(splitString<Strings>(s, ","), expected);
+    EXPECT_EQ(splitString<TypeParam>(s, ","), expected);
 }
 
 // concatStringsSep sep . splitString sep = id   if sep is 1 char
-RC_GTEST_PROP(splitString, recoveredByConcatStringsSep, (const std::string & s))
+RC_GTEST_TYPED_FIXTURE_PROP(splitStringTest, recoveredByConcatStringsSep, (const std::string & s))
 {
-    RC_ASSERT(concatStringsSep("/", splitString<Strings>(s, "/")) == s);
-    RC_ASSERT(concatStringsSep("a", splitString<Strings>(s, "a")) == s);
+    RC_ASSERT(concatStringsSep("/", splitString<TypeParam>(s, "/")) == s);
+    RC_ASSERT(concatStringsSep("a", splitString<TypeParam>(s, "a")) == s);
 }
 
 /* ----------------------------------------------------------------------------

src/libutil-tests/url.cc

@@ -3,6 +3,8 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
+#include <ranges>
+
 namespace nix {
 
 /* ----------- tests for url.hh --------------------------------------------------*/
@@ -10,6 +12,155 @@ namespace nix {
 using Authority = ParsedURL::Authority;
 using HostType = Authority::HostType;
 
+struct FixGitURLParam
+{
+    std::string_view input;
+    std::string_view expected;
+    ParsedURL parsed;
+};
+
+std::ostream & operator<<(std::ostream & os, const FixGitURLParam & param)
+{
+    return os << "Input: \"" << param.input << "\", Expected: \"" << param.expected << "\"";
+}
+
+class FixGitURLTestSuite : public ::testing::TestWithParam<FixGitURLParam>
+{};
+
+INSTANTIATE_TEST_SUITE_P(
+    FixGitURLs,
+    FixGitURLTestSuite,
+    ::testing::Values(
+        // https://github.com/NixOS/nix/issues/5958
+        // Already proper URL with git+ssh
+        FixGitURLParam{
+            .input = "git+ssh://user@domain:1234/path",
+            .expected = "git+ssh://user@domain:1234/path",
+            .parsed =
+                ParsedURL{
+                    .scheme = "git+ssh",
+                    .authority =
+                        ParsedURL::Authority{
+                            .host = "domain",
+                            .user = "user",
+                            .port = 1234,
+                        },
+                    .path = {"", "path"},
+                },
+        },
+        // SCP-like URL (rewritten to ssh://)
+        FixGitURLParam{
+            .input = "git@github.com:owner/repo.git",
+            .expected = "ssh://git@github.com/owner/repo.git",
+            .parsed =
+                ParsedURL{
+                    .scheme = "ssh",
+                    .authority =
+                        ParsedURL::Authority{
+                            .host = "github.com",
+                            .user = "git",
+                        },
+                    .path = {"", "owner", "repo.git"},
+                },
+        },
+        // SCP-like URL (no user)
+        FixGitURLParam{
+            .input = "github.com:owner/repo.git",
+            .expected = "ssh://github.com/owner/repo.git",
+            .parsed =
+                ParsedURL{
+                    .scheme = "ssh",
+                    .authority =
+                        ParsedURL::Authority{
+                            .host = "github.com",
+                        },
+                    .path = {"", "owner", "repo.git"},
+                },
+        },
+        // SCP-like URL (leading slash)
+        FixGitURLParam{
+            .input = "github.com:/owner/repo.git",
+            .expected = "ssh://github.com/owner/repo.git",
+            .parsed =
+                ParsedURL{
+                    .scheme = "ssh",
+                    .authority =
+                        ParsedURL::Authority{
+                            .host = "github.com",
+                        },
+                    .path = {"", "owner", "repo.git"},
+                },
+        },
+        // Absolute path (becomes file:)
+        FixGitURLParam{
+            .input = "/home/me/repo",
+            .expected = "file:///home/me/repo",
+            .parsed =
+                ParsedURL{
+                    .scheme = "file",
+                    .authority = ParsedURL::Authority{},
+                    .path = {"", "home", "me", "repo"},
+                },
+        },
+        // IPV6 test case
+        FixGitURLParam{
+            .input = "user@[2001:db8:1::2]:/home/file",
+            .expected = "ssh://user@[2001:db8:1::2]/home/file",
+            .parsed =
+                ParsedURL{
+                    .scheme = "ssh",
+                    .authority =
+                        ParsedURL::Authority{
+                            .hostType = HostType::IPv6,
+                            .host = "2001:db8:1::2",
+                            .user = "user",
+                        },
+                    .path = {"", "home", "file"},
+                },
+        }));
+
+TEST_P(FixGitURLTestSuite, parsesVariedGitUrls)
+{
+    auto & p = GetParam();
+    const auto actual = fixGitURL(p.input);
+    EXPECT_EQ(actual, p.parsed);
+    EXPECT_EQ(actual.to_string(), p.expected);
+}
+
+TEST_P(FixGitURLTestSuite, fixGitIsIdempotent)
+{
+    auto & p = GetParam();
+    const auto actual = fixGitURL(p.expected).to_string();
+    EXPECT_EQ(actual, p.expected);
+}
+
+TEST_P(FixGitURLTestSuite, fixGitOutputParses)
+{
+    auto & p = GetParam();
+    const auto parsed = fixGitURL(p.expected);
+    EXPECT_EQ(parseURL(parsed.to_string()), parsed);
+}
+
+TEST(FixGitURLTestSuite, properlyRejectFileURLWithAuthority)
+{
+    /* From the underlying `parseURL` validations. */
+    EXPECT_THAT(
+        []() { fixGitURL("file://var/repos/x"); },
+        ::testing::ThrowsMessage<BadURL>(
+            testing::HasSubstrIgnoreANSIMatcher("file:// URL 'file://var/repos/x' has unexpected authority 'var'")));
+}
+
+TEST(FixGitURLTestSuite, ambiguousScpLikeOrFileURL)
+{
+    /* Git/SCP treat this as a `<hostname>:<path>`, but under IETF RFC
+       8089 it is a valid (if sloppy) file URL. Rather than decide who
+       is right, we just make it an error. */
+    EXPECT_THAT(
+        []() { fixGitURL("file:/var/repos/x"); },
+        ::testing::ThrowsMessage<BadURL>(testing::HasSubstrIgnoreANSIMatcher(
+            "URL 'file:/var/repos/x' would parse as SCP authority = 'file', path = '/var/repos/x' but this is also a valid `file:..` URL, and so we choose to disallow it")));
+}
+
 TEST(parseURL, parsesSimpleHttpUrl)
 {
     auto s = "http://www.example.org/file.tar.gz";
@@ -18,7 +169,7 @@ TEST(parseURL, parsesSimpleHttpUrl)
     ParsedURL expected{
         .scheme = "http",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -35,7 +186,7 @@ TEST(parseURL, parsesSimpleHttpsUrl)
     ParsedURL expected{
         .scheme = "https",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -52,7 +203,7 @@ TEST(parseURL, parsesSimpleHttpUrlWithQueryAndFragment)
     ParsedURL expected{
         .scheme = "https",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {{"download", "fast"}, {"when", "now"}},
         .fragment = "hello",
     };
@@ -69,7 +220,7 @@ TEST(parseURL, parsesSimpleHttpUrlWithComplexFragment)
     ParsedURL expected{
         .scheme = "http",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {{"field", "value"}},
         .fragment = "?foo=bar#",
     };
@@ -85,7 +236,7 @@ TEST(parseURL, parsesFilePlusHttpsUrl)
     ParsedURL expected{
         .scheme = "file+https",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = "/video.mp4",
+        .path = {"", "video.mp4"},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -96,8 +247,10 @@ TEST(parseURL, parsesFilePlusHttpsUrl)
 
 TEST(parseURL, rejectsAuthorityInUrlsWithFileTransportation)
 {
-    auto s = "file://www.example.org/video.mp4";
-    ASSERT_THROW(parseURL(s), Error);
+    EXPECT_THAT(
+        []() { parseURL("file://www.example.org/video.mp4"); },
+        ::testing::ThrowsMessage<BadURL>(
+            testing::HasSubstrIgnoreANSIMatcher("has unexpected authority 'www.example.org'")));
 }
 
 TEST(parseURL, parseIPv4Address)
@@ -108,7 +261,7 @@ TEST(parseURL, parseIPv4Address)
     ParsedURL expected{
         .scheme = "http",
         .authority = Authority{.hostType = HostType::IPv4, .host = "127.0.0.1", .port = 8080},
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {{"download", "fast"}, {"when", "now"}},
         .fragment = "hello",
     };
@@ -125,7 +278,7 @@ TEST(parseURL, parseScopedRFC6874IPv6Address)
     ParsedURL expected{
         .scheme = "http",
         .authority = Authority{.hostType = HostType::IPv6, .host = "fe80::818c:da4d:8975:415c\%enp0s25", .port = 8080},
-        .path = "",
+        .path = {""},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -147,7 +300,7 @@ TEST(parseURL, parseIPv6Address)
                 .host = "2a02:8071:8192:c100:311d:192d:81ac:11ea",
                 .port = 8080,
             },
-        .path = "",
+        .path = {""},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -178,7 +331,7 @@ TEST(parseURL, parseUserPassword)
                 .password = "pass",
                 .port = 8080,
             },
-        .path = "/file.tar.gz",
+        .path = {"", "file.tar.gz"},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -195,11 +348,77 @@ TEST(parseURL, parseFileURLWithQueryAndFragment)
     ParsedURL expected{
         .scheme = "file",
         .authority = Authority{},
-        .path = "/none/of//your/business",
+        .path = {"", "none", "of", "", "your", "business"},
         .query = (StringMap) {},
         .fragment = "",
     };
 
+    ASSERT_EQ(parsed.renderPath(), "/none/of//your/business");
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ(s, parsed.to_string());
+}
+
+TEST(parseURL, parseFileURL)
+{
+    auto s = "file:/none/of/your/business/";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "file",
+        .authority = std::nullopt,
+        .path = {"", "none", "of", "your", "business", ""},
+    };
+
+    ASSERT_EQ(parsed.renderPath(), "/none/of/your/business/");
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ(s, parsed.to_string());
+}
+
+TEST(parseURL, parseFileURLWithAuthority)
+{
+    auto s = "file://///of/your/business//";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "file",
+        .authority = Authority{.host = ""},
+        .path = {"", "", "", "of", "your", "business", "", ""},
+    };
+
+    ASSERT_EQ(parsed.path, expected.path);
+    ASSERT_EQ(parsed.renderPath(), "///of/your/business//");
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ(s, parsed.to_string());
+}
+
+TEST(parseURL, parseFileURLNoLeadingSlash)
+{
+    auto s = "file:none/of/your/business/";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "file",
+        .authority = std::nullopt,
+        .path = {"none", "of", "your", "business", ""},
+    };
+
+    ASSERT_EQ(parsed.renderPath(), "none/of/your/business/");
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ("file:none/of/your/business/", parsed.to_string());
+}
+
+TEST(parseURL, parseHttpTrailingSlash)
+{
+    auto s = "http://example.com/";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = Authority{.host = "example.com"},
+        .path = {"", ""},
+    };
+
+    ASSERT_EQ(parsed.renderPath(), "/");
     ASSERT_EQ(parsed, expected);
     ASSERT_EQ(s, parsed.to_string());
 }
@@ -221,15 +440,20 @@ TEST(parseURL, parsedUrlsWithUnescapedChars)
      * 2. Unescaped spaces and quotes in query.
      */
     auto s = "http://www.example.org/file.tar.gz?query \"= 123\"#shevron^quote\"space ";
-    auto url = parseURL(s);
 
-    ASSERT_EQ(url.fragment, "shevron^quote\"space ");
+    /* Without leniency for back compat, this should throw. */
+    EXPECT_THROW(parseURL(s), Error);
+
+    /* With leniency for back compat, this should parse. */
+    auto url = parseURL(s, /*lenient=*/true);
+
+    EXPECT_EQ(url.fragment, "shevron^quote\"space ");
 
     auto query = StringMap{
         {"query \"", " 123\""},
     };
 
-    ASSERT_EQ(url.query, query);
+    EXPECT_EQ(url.query, query);
 }
 
 TEST(parseURL, parseFTPUrl)
@@ -240,7 +464,7 @@ TEST(parseURL, parseFTPUrl)
     ParsedURL expected{
         .scheme = "ftp",
         .authority = Authority{.hostType = HostType::Name, .host = "ftp.nixos.org"},
-        .path = "/downloads/nixos.iso",
+        .path = {"", "downloads", "nixos.iso"},
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -268,6 +492,225 @@ TEST(parseURL, emptyStringIsInvalidURL)
     ASSERT_THROW(parseURL(""), Error);
 }
 
+TEST(parseURL, parsesHttpUrlWithEmptyPort)
+{
+    auto s = "http://www.example.org:/file.tar.gz?foo=bar";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "file.tar.gz"},
+        .query = (StringMap) {{"foo", "bar"}},
+        .fragment = "",
+    };
+
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ("http://www.example.org/file.tar.gz?foo=bar", parsed.to_string());
+}
+
+/* ----------------------------------------------------------------------------
+ * parseURLRelative
+ * --------------------------------------------------------------------------*/
+
+TEST(parseURLRelative, resolvesRelativePath)
+{
+    ParsedURL base = parseURL("http://example.org/dir/page.html");
+    auto parsed = parseURLRelative("subdir/file.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "example.org"},
+        .path = {"", "dir", "subdir", "file.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, baseUrlIpv6AddressWithoutZoneId)
+{
+    ParsedURL base = parseURL("http://[fe80::818c:da4d:8975:415c]/dir/page.html");
+    auto parsed = parseURLRelative("subdir/file.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::IPv6, .host = "fe80::818c:da4d:8975:415c"},
+        .path = {"", "dir", "subdir", "file.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, resolvesRelativePathIpv6AddressWithZoneId)
+{
+    ParsedURL base = parseURL("http://[fe80::818c:da4d:8975:415c\%25enp0s25]:8080/dir/page.html");
+    auto parsed = parseURLRelative("subdir/file2.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = Authority{.hostType = HostType::IPv6, .host = "fe80::818c:da4d:8975:415c\%enp0s25", .port = 8080},
+        .path = {"", "dir", "subdir", "file2.txt"},
+        .query = {},
+        .fragment = "",
+    };
+
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, resolvesRelativePathWithDot)
+{
+    ParsedURL base = parseURL("http://example.org/dir/page.html");
+    auto parsed = parseURLRelative("./subdir/file.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "example.org"},
+        .path = {"", "dir", "subdir", "file.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, resolvesParentDirectory)
+{
+    ParsedURL base = parseURL("http://example.org:234/dir/page.html");
+    auto parsed = parseURLRelative("../up.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "example.org", .port = 234},
+        .path = {"", "up.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, resolvesParentDirectoryNotTrickedByEscapedSlash)
+{
+    ParsedURL base = parseURL("http://example.org:234/dir\%2Ffirst-trick/another-dir\%2Fsecond-trick/page.html");
+    auto parsed = parseURLRelative("../up.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "example.org", .port = 234},
+        .path = {"", "dir/first-trick", "up.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, replacesPathWithAbsoluteRelative)
+{
+    ParsedURL base = parseURL("http://example.org/dir/page.html");
+    auto parsed = parseURLRelative("/rooted.txt", base);
+    ParsedURL expected{
+        .scheme = "http",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "example.org"},
+        .path = {"", "rooted.txt"},
+        .query = {},
+        .fragment = "",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, keepsQueryAndFragmentFromRelative)
+{
+    // But discard query params on base URL
+    ParsedURL base = parseURL("https://www.example.org/path/index.html?z=3");
+    auto parsed = parseURLRelative("other.html?x=1&y=2#frag", base);
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "path", "other.html"},
+        .query = {{"x", "1"}, {"y", "2"}},
+        .fragment = "frag",
+    };
+    ASSERT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, absOverride)
+{
+    ParsedURL base = parseURL("http://example.org/path/page.html");
+    std::string_view abs = "https://127.0.0.1.org/secure";
+    auto parsed = parseURLRelative(abs, base);
+    auto parsedAbs = parseURL(abs);
+    ASSERT_EQ(parsed, parsedAbs);
+}
+
+TEST(parseURLRelative, absOverrideWithZoneId)
+{
+    ParsedURL base = parseURL("http://example.org/path/page.html");
+    std::string_view abs = "https://[fe80::818c:da4d:8975:415c\%25enp0s25]/secure?foo=bar";
+    auto parsed = parseURLRelative(abs, base);
+    auto parsedAbs = parseURL(abs);
+    ASSERT_EQ(parsed, parsedAbs);
+}
+
+TEST(parseURLRelative, bothWithoutAuthority)
+{
+    ParsedURL base = parseURL("mailto:mail-base@bar.baz?bcc=alice@asdf.com");
+    std::string_view over = "mailto:mail-override@foo.bar?subject=url-testing";
+    auto parsed = parseURLRelative(over, base);
+    auto parsedOverride = parseURL(over);
+    ASSERT_EQ(parsed, parsedOverride);
+}
+
+TEST(parseURLRelative, emptyRelative)
+{
+    ParsedURL base = parseURL("https://www.example.org/path/index.html?a\%20b=5\%206&x\%20y=34#frag");
+    auto parsed = parseURLRelative("", base);
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "path", "index.html"},
+        .query = {{"a b", "5 6"}, {"x y", "34"}},
+        .fragment = "",
+    };
+    EXPECT_EQ(base.fragment, "frag");
+    EXPECT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, fragmentRelative)
+{
+    ParsedURL base = parseURL("https://www.example.org/path/index.html?a\%20b=5\%206&x\%20y=34#frag");
+    auto parsed = parseURLRelative("#frag2", base);
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "path", "index.html"},
+        .query = {{"a b", "5 6"}, {"x y", "34"}},
+        .fragment = "frag2",
+    };
+    EXPECT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, queryRelative)
+{
+    ParsedURL base = parseURL("https://www.example.org/path/index.html?a\%20b=5\%206&x\%20y=34#frag");
+    auto parsed = parseURLRelative("?asdf\%20qwer=1\%202\%203", base);
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "path", "index.html"},
+        .query = {{"asdf qwer", "1 2 3"}},
+        .fragment = "",
+    };
+    EXPECT_EQ(parsed, expected);
+}
+
+TEST(parseURLRelative, queryFragmentRelative)
+{
+    ParsedURL base = parseURL("https://www.example.org/path/index.html?a\%20b=5\%206&x\%20y=34#frag");
+    auto parsed = parseURLRelative("?asdf\%20qwer=1\%202\%203#frag2", base);
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = ParsedURL::Authority{.hostType = HostType::Name, .host = "www.example.org"},
+        .path = {"", "path", "index.html"},
+        .query = {{"asdf qwer", "1 2 3"}},
+        .fragment = "frag2",
+    };
+    EXPECT_EQ(parsed, expected);
+}
+
 /* ----------------------------------------------------------------------------
  * decodeQuery
  * --------------------------------------------------------------------------*/
@@ -377,7 +820,121 @@ TEST(percentEncode, yen)
     ASSERT_EQ(percentDecode(e), s);
 }
 
+TEST(parseURL, gitlabNamespacedProjectUrls)
+{
+    // Test GitLab URL patterns with namespaced projects
+    // These should preserve %2F encoding in the path
+    auto s = "https://gitlab.example.com/api/v4/projects/group%2Fsubgroup%2Fproject/repository/archive.tar.gz";
+    auto parsed = parseURL(s);
+
+    ParsedURL expected{
+        .scheme = "https",
+        .authority = Authority{.hostType = HostType::Name, .host = "gitlab.example.com"},
+        .path = {"", "api", "v4", "projects", "group/subgroup/project", "repository", "archive.tar.gz"},
+        .query = {},
+        .fragment = "",
+    };
+
+    ASSERT_EQ(parsed, expected);
+    ASSERT_EQ(s, parsed.to_string());
+}
+
+/* ----------------------------------------------------------------------------
+ * pathSegments
+ * --------------------------------------------------------------------------*/
+
+struct ParsedURLPathSegmentsTestCase
+{
+    std::string url;
+    std::vector<std::string> segments;
+    std::string path;
+    bool skipEmpty;
+    std::string description;
+};
+
+class ParsedURLPathSegmentsTest : public ::testing::TestWithParam<ParsedURLPathSegmentsTestCase>
+{};
+
+TEST_P(ParsedURLPathSegmentsTest, segmentsAreCorrect)
+{
+    const auto & testCase = GetParam();
+    auto segments = parseURL(testCase.url).pathSegments(/*skipEmpty=*/testCase.skipEmpty)
+                    | std::ranges::to<decltype(testCase.segments)>();
+    EXPECT_EQ(segments, testCase.segments);
+    EXPECT_EQ(encodeUrlPath(segments), testCase.path);
+}
+
+INSTANTIATE_TEST_SUITE_P(
+    ParsedURL,
+    ParsedURLPathSegmentsTest,
+    ::testing::Values(
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme:",
+            .segments = {""},
+            .path = "",
+            .skipEmpty = false,
+            .description = "no_authority_empty_path",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme://",
+            .segments = {""},
+            .path = "",
+            .skipEmpty = false,
+            .description = "empty_authority_empty_path",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme:///",
+            .segments = {"", ""},
+            .path = "/",
+            .skipEmpty = false,
+            .description = "empty_authority_empty_path_trailing",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme://example.com/",
+            .segments = {"", ""},
+            .path = "/",
+            .skipEmpty = false,
+            .description = "non_empty_authority_empty_path",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme://example.com//",
+            .segments = {"", "", ""},
+            .path = "//",
+            .skipEmpty = false,
+            .description = "non_empty_authority_non_empty_path",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme://example.com///path///with//strange/empty///segments////",
+            .segments = {"path", "with", "strange", "empty", "segments"},
+            .path = "path/with/strange/empty/segments",
+            .skipEmpty = true,
+            .description = "skip_all_empty_segments_with_authority",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme://example.com///lots///empty///",
+            .segments = {"", "", "", "lots", "", "", "empty", "", "", ""},
+            .path = "///lots///empty///",
+            .skipEmpty = false,
+            .description = "empty_segments_with_authority",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme:/path///with//strange/empty///segments////",
+            .segments = {"path", "with", "strange", "empty", "segments"},
+            .path = "path/with/strange/empty/segments",
+            .skipEmpty = true,
+            .description = "skip_all_empty_segments_no_authority_starts_with_slash",
+        },
+        ParsedURLPathSegmentsTestCase{
+            .url = "scheme:path///with//strange/empty///segments////",
+            .segments = {"path", "with", "strange", "empty", "segments"},
+            .path = "path/with/strange/empty/segments",
+            .skipEmpty = true,
+            .description = "skip_all_empty_segments_no_authority_doesnt_start_with_slash",
+        }),
+    [](const auto & info) { return info.param.description; });
+
 TEST(nix, isValidSchemeName)
+
 {
     ASSERT_TRUE(isValidSchemeName("http"));
     ASSERT_TRUE(isValidSchemeName("https"));

src/libutil/error.cc

@@ -6,6 +6,7 @@
 #include "nix/util/terminal.hh"
 #include "nix/util/position.hh"
 
+#include <cinttypes>
 #include <iostream>
 #include <optional>
 #include "nix/util/serialise.hh"
@@ -436,13 +437,19 @@ void panic(std::string_view msg)
     writeErr("\n\n" ANSI_RED "terminating due to unexpected unrecoverable internal error: " ANSI_NORMAL);
     writeErr(msg);
     writeErr("\n");
-    abort();
+    std::terminate();
 }
 
-void panic(const char * file, int line, const char * func)
+void unreachable(std::source_location loc)
 {
     char buf[512];
-    int n = snprintf(buf, sizeof(buf), "Unexpected condition in %s at %s:%d", func, file, line);
+    int n = snprintf(
+        buf,
+        sizeof(buf),
+        "Unexpected condition in %s at %s:%" PRIuLEAST32,
+        loc.function_name(),
+        loc.file_name(),
+        loc.line());
     if (n < 0)
         panic("Unexpected condition and could not format error message");
     panic(std::string_view(buf, std::min(static_cast<int>(sizeof(buf)), n)));

src/libutil/include/nix/util/error.hh

@@ -22,6 +22,7 @@
 #include <list>
 #include <memory>
 #include <optional>
+#include <utility>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -299,23 +300,16 @@ using NativeSysError =
 void throwExceptionSelfCheck();
 
 /**
- * Print a message and abort().
+ * Print a message and std::terminate().
  */
 [[noreturn]]
 void panic(std::string_view msg);
 
 /**
- * Print a basic error message with source position and abort().
- * Use the unreachable() macro to call this.
- */
-[[noreturn]]
-void panic(const char * file, int line, const char * func);
-
-/**
- * Print a basic error message with source position and abort().
+ * Print a basic error message with source position and std::terminate().
  *
  * @note: This assumes that the logger is operational
  */
-#define unreachable() (::nix::panic(__FILE__, __LINE__, __func__))
+[[gnu::noinline, gnu::cold, noreturn]] void unreachable(std::source_location loc = std::source_location::current());
 
 } // namespace nix

src/libutil/include/nix/util/strings-inline.hh

@@ -26,18 +26,29 @@ C tokenizeString(std::string_view s, std::string_view separators)
 }
 
 template<class C, class CharT>
-C basicSplitString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
+void basicSplitStringInto(C & accum, std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
 {
-    C result;
     size_t pos = 0;
     while (pos <= s.size()) {
         auto end = s.find_first_of(separators, pos);
         if (end == s.npos)
             end = s.size();
-        result.insert(result.end(), std::basic_string<CharT>(s, pos, end - pos));
+        accum.insert(accum.end(), typename C::value_type{s.substr(pos, end - pos)});
         pos = end + 1;
     }
+}
 
+template<typename C>
+void splitStringInto(C & accum, std::string_view s, std::string_view separators)
+{
+    basicSplitStringInto<C, char>(accum, s, separators);
+}
+
+template<class C, class CharT>
+C basicSplitString(std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
+{
+    C result;
+    basicSplitStringInto(result, s, separators);
     return result;
 }
 

src/libutil/include/nix/util/url.hh

@@ -1,7 +1,11 @@
 #pragma once
 ///@file
 
+#include <ranges>
+#include <span>
+
 #include "nix/util/error.hh"
+#include "nix/util/canon-path.hh"
 
 namespace nix {
 
@@ -65,6 +69,7 @@ struct ParsedURL
     };
 
     std::string scheme;
+
     /**
      * Optional parsed authority component of the URL.
      *
@@ -75,18 +80,171 @@ struct ParsedURL
      * part of the URL.
      */
     std::optional<Authority> authority;
-    std::string path;
+
+    /**
+     * @note Unlike Unix paths, URLs provide a way to escape path
+     * separators, in the form of the `%2F` encoding of `/`. That means
+     * that if one percent-decodes the path into a single string, that
+     * decoding will be *lossy*, because `/` and `%2F` both become `/`.
+     * The right thing to do is instead split up the path on `/`, and
+     * then percent decode each part.
+     *
+     * For an example, the path
+     * ```
+     * foo/bar%2Fbaz/quux
+     * ```
+     * is parsed as
+     * ```
+     * {"foo, "bar/baz", "quux"}
+     * ```
+     *
+     * We're doing splitting and joining that assumes the separator (`/` in this case) only goes *between* elements.
+     *
+     * That means the parsed representation will begin with an empty
+     * element to make an initial `/`, and will end with an ementy
+     * element to make a trailing `/`. That means that elements of this
+     * vector mostly, but *not always*, correspond to segments of the
+     * path.
+     *
+     * Examples:
+     *
+     * - ```
+     *   https://foo.com/bar
+     *   ```
+     *   has path
+     *   ```
+     *   {"", "bar"}
+     *   ```
+     *
+     * - ```
+     *   https://foo.com/bar/
+     *   ```
+     *   has path
+     *   ```
+     *   {"", "bar", ""}
+     *   ```
+     *
+     * - ```
+     *   https://foo.com//bar///
+     *   ```
+     *   has path
+     *   ```
+     *   {"", "", "bar", "", "", ""}
+     *   ```
+     *
+     * - ```
+     *   https://foo.com
+     *   ```
+     *   has path
+     *   ```
+     *   {""}
+     *   ```
+     *
+     * - ```
+     *   https://foo.com/
+     *   ```
+     *   has path
+     *   ```
+     *   {"", ""}
+     *   ```
+     *
+     * - ```
+     *   tel:01234
+     *   ```
+     *   has path `{"01234"}` (and no authority)
+     *
+     * - ```
+     *   foo:/01234
+     *   ```
+     *   has path `{"", "01234"}` (and no authority)
+     *
+     * Note that both trailing and leading slashes are, in general,
+     * semantically significant.
+     *
+     * For trailing slashes, the main example affecting many schemes is
+     * that `../baz` resolves against a base URL different depending on
+     * the presence/absence of a trailing slash:
+     *
+     * - `https://foo.com/bar` is `https://foo.com/baz`
+     *
+     * - `https://foo.com/bar/` is `https://foo.com/bar/baz`
+     *
+     * See `parseURLRelative` for more details.
+     *
+     * For leading slashes, there are some requirements to be aware of.
+     *
+     * - When there is an authority, the path *must* start with a leading
+     *   slash. Otherwise the path will not be separated from the
+     *   authority, and will not round trip though the parser:
+     *
+     *   ```
+     *   {.scheme="https", .authority.host = "foo", .path={"bad"}}
+     *   ```
+     *   will render to `https://foobar`. but that would parse back as as
+     *   ```
+     *   {.scheme="https", .authority.host = "foobar", .path={}}
+     *   ```
+     *
+     * - When there is no authority, the path must *not* begin with two
+     *   slashes. Otherwise, there will be another parser round trip
+     *   issue:
+     *
+     *   ```
+     *   {.scheme="https", .path={"", "", "bad"}}
+     *   ```
+     *   will render to `https://bad`. but that would parse back as as
+     *   ```
+     *   {.scheme="https", .authority.host = "bad", .path={}}
+     *   ```
+     *
+     * These invariants will be checked in `to_string` and
+     * `renderAuthorityAndPath`.
+     */
+    std::vector<std::string> path;
+
     StringMap query;
+
     std::string fragment;
 
+    /**
+     * Render just the middle part of a URL, without the `//` which
+     * indicates whether the authority is present.
+     *
+     * @note This is kind of an ad-hoc
+     * operation, but it ends up coming up with some frequency, probably
+     * due to the current design of `StoreReference` in `nix-store`.
+     */
+    std::string renderAuthorityAndPath() const;
+
     std::string to_string() const;
 
+    /**
+     * Render the path to a string.
+     *
+     * @param encode Whether to percent encode path segments.
+     */
+    std::string renderPath(bool encode = false) const;
+
     auto operator<=>(const ParsedURL & other) const noexcept = default;
 
     /**
-     * Remove `.` and `..` path elements.
+     * Remove `.` and `..` path segments.
      */
     ParsedURL canonicalise();
+
+    /**
+     * Get a range of path segments (the substrings separated by '/' characters).
+     *
+     * @param skipEmpty Skip all empty path segments
+     */
+    auto pathSegments(bool skipEmpty) const &
+    {
+        return std::views::filter(path, [skipEmpty](std::string_view segment) {
+            if (skipEmpty)
+                return !segment.empty();
+            return true;
+        });
+    }
 };
 
 std::ostream & operator<<(std::ostream & os, const ParsedURL & url);
@@ -96,22 +254,62 @@ MakeError(BadURL, Error);
 std::string percentDecode(std::string_view in);
 std::string percentEncode(std::string_view s, std::string_view keep = "");
 
-StringMap decodeQuery(const std::string & query);
+/**
+ * Get the path part of the URL as an absolute or relative Path.
+ *
+ * @throws if any path component contains an slash (which would have
+ * been escaped `%2F` in the rendered URL). This is because OS file
+ * paths have no escape sequences --- file names cannot contain a
+ * `/`.
+ */
+Path renderUrlPathEnsureLegal(const std::vector<std::string> & urlPath);
+
+/**
+ * Percent encode path. `%2F` for "interior slashes" is the most
+ * important.
+ */
+std::string encodeUrlPath(std::span<const std::string> urlPath);
+
+/**
+ * @param lenient @see parseURL
+ */
+StringMap decodeQuery(std::string_view query, bool lenient = false);
 
 std::string encodeQuery(const StringMap & query);
 
 /**
- * Parse a Nix URL into a ParsedURL.
+ * Parse a URL into a ParsedURL.
  *
- * Nix URI is mostly compliant with RFC3986, but with some deviations:
+ * @parm lenient Also allow some long-supported Nix URIs that are not quite compliant with RFC3986.
+ * Here are the deviations:
  * - Fragments can contain unescaped (not URL encoded) '^', '"' or space literals.
  * - Queries may contain unescaped '"' or spaces.
  *
  * @note IPv6 ZoneId literals (RFC4007) are represented in URIs according to RFC6874.
  *
  * @throws BadURL
+ *
+ * The WHATWG specification of the URL constructor in Java Script is
+ * also a useful reference:
+ * https://url.spec.whatwg.org/#concept-basic-url-parser. Note, however,
+ * that it includes various scheme-specific normalizations / extra steps
+ * that we do not implement.
  */
-ParsedURL parseURL(std::string_view url);
+ParsedURL parseURL(std::string_view url, bool lenient = false);
+
+/**
+ * Like `parseURL`, but also accepts relative URLs, which are resolved
+ * against the given base URL.
+ *
+ * This is specified in [IETF RFC 3986, section 5](https://datatracker.ietf.org/doc/html/rfc3986#section-5)
+ *
+ * @throws BadURL
+ *
+ * Behavior should also match the `new URL(url, base)` JavaScript
+ * constructor, except for extra steps specific to the HTTP scheme. See
+ * `parseURL` for link to the relevant WHATWG standard.
+ */
+ParsedURL parseURLRelative(std::string_view url, const ParsedURL & base);
 
 /**
  * Although that’s not really standardized anywhere, an number of tools
@@ -129,10 +327,23 @@ struct ParsedUrlScheme
 
 ParsedUrlScheme parseUrlScheme(std::string_view scheme);
 
-/* Detects scp-style uris (e.g. git@github.com:NixOS/nix) and fixes
-   them by removing the `:` and assuming a scheme of `ssh://`. Also
-   changes absolute paths into file:// URLs. */
-std::string fixGitURL(const std::string & url);
+/**
+ * Normalize a Git remote string from various styles into a URL-like form.
+ * Input forms handled:
+ *   1) SCP-style SSH syntax:    "[user@]host:path"      -> "ssh://user@host/path"
+ *   2) Already "file:" URLs:    "file:/abs/or/rel"      -> unchanged
+ *   3) Bare paths / filenames:  "src/repo" or "/abs"    -> "file:src/repo" or "file:/abs"
+ *   4) Anything with "://":     treated as a proper URL -> unchanged
+ *
+ *  Note: for the scp-style, as they are converted to ssh-form, all paths are assumed to
+ *  then be absolute whereas in programs like git, they retain the scp form which allows
+ *  relative paths.
+ *
+ *  Additionally, if no url can be determined, it is returned as a file:// URI.
+ *  If the url does not start with a leading slash, one will be added since there are no
+ *  relative path URIs.
+ */
+ParsedURL fixGitURL(std::string_view url);
 
 /**
  * Whether a string is valid as RFC 3986 scheme name.
@@ -143,4 +354,63 @@ std::string fixGitURL(const std::string & url);
  */
 bool isValidSchemeName(std::string_view scheme);
 
+/**
+ * Either a ParsedURL or a verbatim string, but the string must be a valid
+ * ParsedURL. This is necessary because in certain cases URI must be passed
+ * verbatim (e.g. in builtin fetchers), since those are specified by the user.
+ * In those cases normalizations performed by the ParsedURL might be surprising
+ * and undesirable, since Nix must be a universal client that has to work with
+ * various broken services that might interpret URLs in quirky and non-standard ways.
+ *
+ * One of those examples is space-as-plus encoding that is very widespread, but it's
+ * not strictly RFC3986 compliant. We must preserve that information verbatim.
+ *
+ * Though we perform parsing and validation for internal needs.
+ */
+struct ValidURL : private ParsedURL
+{
+    std::optional<std::string> encoded;
+
+    ValidURL(std::string str)
+        : ParsedURL(parseURL(str, /*lenient=*/false))
+        , encoded(std::move(str))
+    {
+    }
+
+    ValidURL(std::string_view str)
+        : ValidURL(std::string{str})
+    {
+    }
+
+    ValidURL(ParsedURL parsed)
+        : ParsedURL{std::move(parsed)}
+    {
+    }
+
+    /**
+     * Get the encoded URL (if specified) verbatim or encode the parsed URL.
+     */
+    std::string to_string() const
+    {
+        return encoded.or_else([&]() -> std::optional<std::string> { return ParsedURL::to_string(); }).value();
+    }
+
+    const ParsedURL & parsed() const &
+    {
+        return *this;
+    }
+
+    std::string_view scheme() const &
+    {
+        return ParsedURL::scheme;
+    }
+
+    const auto & path() const &
+    {
+        return ParsedURL::path;
+    }
+};
+
+std::ostream & operator<<(std::ostream & os, const ValidURL & url);
+
 } // namespace nix

src/libutil/url.cc

@@ -3,6 +3,7 @@
 #include "nix/util/util.hh"
 #include "nix/util/split.hh"
 #include "nix/util/canon-path.hh"
+#include "nix/util/strings-inline.hh"
 
 #include <boost/url.hpp>
 
@@ -33,7 +34,7 @@ ParsedURL::Authority ParsedURL::Authority::parse(std::string_view encodedAuthori
     }();
 
     auto port = [&]() -> std::optional<uint16_t> {
-        if (!parsed->has_port())
+        if (!parsed->has_port() || parsed->port() == "")
             return std::nullopt;
         /* If the port number is non-zero and representable. */
         if (auto portNumber = parsed->port_number())
@@ -108,44 +109,58 @@ static std::string percentEncodeCharSet(std::string_view s, auto charSet)
     return res;
 }
 
-ParsedURL parseURL(std::string_view url)
+static ParsedURL fromBoostUrlView(boost::urls::url_view url, bool lenient);
+
+ParsedURL parseURL(std::string_view url, bool lenient)
 try {
     /* Account for several non-standard properties of nix urls (for back-compat):
      *  - Allow unescaped spaces ' ' and '"' characters in queries.
      *  - Allow '"', ' ' and '^' characters in the fragment component.
      * We could write our own grammar for this, but fixing it up here seems
      * more concise, since the deviation is rather minor.
+     *
+     * If `!lenient` don't bother initializing, because we can just
+     * parse `url` directly`.
      */
-    std::string fixedEncodedUrl = [&]() {
-        std::string fixed;
-        std::string_view view = url;
+    std::string fixedEncodedUrl;
 
-        if (auto beforeQuery = splitPrefixTo(view, '?')) {
-            fixed += *beforeQuery;
-            fixed += '?';
-            auto fragmentStart = view.find('#');
-            auto queryView = view.substr(0, fragmentStart);
-            auto fixedQuery = percentEncodeCharSet(queryView, extraAllowedCharsInQuery);
-            fixed += fixedQuery;
-            view.remove_prefix(std::min(fragmentStart, view.size()));
-        }
+    if (lenient) {
+        fixedEncodedUrl = [&] {
+            std::string fixed;
+            std::string_view view = url;
 
-        if (auto beforeFragment = splitPrefixTo(view, '#')) {
-            fixed += *beforeFragment;
-            fixed += '#';
-            auto fixedFragment = percentEncodeCharSet(view, extraAllowedCharsInFragment);
-            fixed += fixedFragment;
+            if (auto beforeQuery = splitPrefixTo(view, '?')) {
+                fixed += *beforeQuery;
+                fixed += '?';
+                auto fragmentStart = view.find('#');
+                auto queryView = view.substr(0, fragmentStart);
+                auto fixedQuery = percentEncodeCharSet(queryView, extraAllowedCharsInQuery);
+                fixed += fixedQuery;
+                view.remove_prefix(std::min(fragmentStart, view.size()));
+            }
+
+            if (auto beforeFragment = splitPrefixTo(view, '#')) {
+                fixed += *beforeFragment;
+                fixed += '#';
+                auto fixedFragment = percentEncodeCharSet(view, extraAllowedCharsInFragment);
+                fixed += fixedFragment;
+                return fixed;
+            }
+
+            fixed += view;
             return fixed;
-        }
+        }();
+    }
 
-        fixed += view;
-        return fixed;
-    }();
-
-    auto urlView = boost::urls::url_view(fixedEncodedUrl);
+    return fromBoostUrlView(boost::urls::url_view(lenient ? fixedEncodedUrl : url), lenient);
+} catch (boost::system::system_error & e) {
+    throw BadURL("'%s' is not a valid URL: %s", url, e.code().message());
+}
 
+static ParsedURL fromBoostUrlView(boost::urls::url_view urlView, bool lenient)
+{
     if (!urlView.has_scheme())
-        throw BadURL("'%s' doesn't have a scheme", url);
+        throw BadURL("'%s' doesn't have a scheme", urlView.buffer());
 
     auto scheme = urlView.scheme();
     auto authority = [&]() -> std::optional<ParsedURL::Authority> {
@@ -163,13 +178,16 @@ try {
      * scheme considers a missing authority or empty host invalid. */
     auto transportIsFile = parseUrlScheme(scheme).transport == "file";
     if (authority && authority->host.size() && transportIsFile)
-        throw BadURL("file:// URL '%s' has unexpected authority '%s'", url, *authority);
+        throw BadURL("file:// URL '%s' has unexpected authority '%s'", urlView.buffer(), *authority);
 
-    auto path = urlView.path();         /* Does pct-decoding */
     auto fragment = urlView.fragment(); /* Does pct-decoding */
 
-    if (transportIsFile && path.empty())
-        path = "/";
+    boost::core::string_view encodedPath = urlView.encoded_path();
+    if (transportIsFile && encodedPath.empty())
+        encodedPath = "/";
+
+    auto path = std::views::transform(splitString<std::vector<std::string_view>>(encodedPath, "/"), percentDecode)
+                | std::ranges::to<std::vector<std::string>>();
 
     /* Get the raw query. Store URI supports smuggling doubly nested queries, where
        the inner &/? are pct-encoded. */
@@ -178,12 +196,62 @@ try {
     return ParsedURL{
         .scheme = scheme,
         .authority = authority,
-        .path = path,
-        .query = decodeQuery(std::string(query)),
+        .path = std::move(path),
+        .query = decodeQuery(query, lenient),
         .fragment = fragment,
     };
-} catch (boost::system::system_error & e) {
-    throw BadURL("'%s' is not a valid URL: %s", url, e.code().message());
+}
+
+ParsedURL parseURLRelative(std::string_view urlS, const ParsedURL & base)
+try {
+
+    boost::urls::url resolved;
+
+    try {
+        resolved.set_scheme(base.scheme);
+        if (base.authority) {
+            auto & authority = *base.authority;
+            resolved.set_host_address(authority.host);
+            if (authority.user)
+                resolved.set_user(*authority.user);
+            if (authority.password)
+                resolved.set_password(*authority.password);
+            if (authority.port)
+                resolved.set_port_number(*authority.port);
+        }
+        resolved.set_encoded_path(encodeUrlPath(base.path));
+        resolved.set_encoded_query(encodeQuery(base.query));
+        resolved.set_fragment(base.fragment);
+    } catch (boost::system::system_error & e) {
+        throw BadURL("'%s' is not a valid URL: %s", base.to_string(), e.code().message());
+    }
+
+    boost::urls::url_view url;
+    try {
+        url = urlS;
+        resolved.resolve(url).value();
+    } catch (boost::system::system_error & e) {
+        throw BadURL("'%s' is not a valid URL: %s", urlS, e.code().message());
+    }
+
+    auto ret = fromBoostUrlView(resolved, /*lenient=*/false);
+
+    /* Hack: Boost `url_view` supports Zone IDs, but `url` does not.
+       Just manually take the authority from the original URL to work
+       around it. See https://github.com/boostorg/url/issues/919 for
+       details. */
+    if (!url.has_authority()) {
+        ret.authority = base.authority;
+    }
+
+    /* Hack, work around fragment of base URL improperly being preserved
+       https://github.com/boostorg/url/issues/920 */
+    ret.fragment = url.has_fragment() ? std::string{url.fragment()} : "";
+
+    return ret;
+} catch (BadURL & e) {
+    e.addTrace({}, "while resolving possibly-relative url '%s' against base URL '%s'", urlS, base);
+    throw;
 }
 
 std::string percentDecode(std::string_view in)
@@ -201,14 +269,17 @@ std::string percentEncode(std::string_view s, std::string_view keep)
         s, [keep](char c) { return boost::urls::unreserved_chars(c) || keep.find(c) != keep.npos; });
 }
 
-StringMap decodeQuery(const std::string & query)
+StringMap decodeQuery(std::string_view query, bool lenient)
 try {
-    /* For back-compat unescaped characters are allowed. */
-    auto fixedEncodedQuery = percentEncodeCharSet(query, extraAllowedCharsInQuery);
+    /* When `lenient = true`, for back-compat unescaped characters are allowed. */
+    std::string fixedEncodedQuery;
+    if (lenient) {
+        fixedEncodedQuery = percentEncodeCharSet(query, extraAllowedCharsInQuery);
+    }
 
     StringMap result;
 
-    auto encodedQuery = boost::urls::params_encoded_view(fixedEncodedQuery);
+    auto encodedQuery = boost::urls::params_encoded_view(lenient ? fixedEncodedQuery : query);
     for (auto && [key, value, value_specified] : encodedQuery) {
         if (!value_specified) {
             warn("dubious URI query '%s' is missing equal sign '%s', ignoring", std::string_view(key), "=");
@@ -224,7 +295,15 @@ try {
 }
 
 const static std::string allowedInQuery = ":@/?";
-const static std::string allowedInPath = ":@/";
+const static std::string allowedInPath = ":@";
+
+std::string encodeUrlPath(std::span<const std::string> urlPath)
+{
+    std::vector<std::string> encodedPath;
+    for (auto & p : urlPath)
+        encodedPath.push_back(percentEncode(p, allowedInPath));
+    return concatStringsSep("/", encodedPath);
+}
 
 std::string encodeQuery(const StringMap & ss)
 {
@@ -241,10 +320,62 @@ std::string encodeQuery(const StringMap & ss)
     return res;
 }
 
+Path renderUrlPathEnsureLegal(const std::vector<std::string> & urlPath)
+{
+    for (const auto & comp : urlPath) {
+        /* This is only really valid for UNIX. Windows has more restrictions. */
+        if (comp.contains('/'))
+            throw BadURL("URL path component '%s' contains '/', which is not allowed in file names", comp);
+        if (comp.contains(char(0)))
+            throw BadURL("URL path component '%s' contains NUL byte which is not allowed", comp);
+    }
+
+    return concatStringsSep("/", urlPath);
+}
+
+std::string ParsedURL::renderPath(bool encode) const
+{
+    if (encode)
+        return encodeUrlPath(path);
+    return concatStringsSep("/", path);
+}
+
+std::string ParsedURL::renderAuthorityAndPath() const
+{
+    std::string res;
+    /* The following assertions correspond to 3.3. Path [rfc3986]. URL parser
+       will never violate these properties, but hand-constructed ParsedURLs might. */
+    if (authority.has_value()) {
+        /* If a URI contains an authority component, then the path component
+           must either be empty or begin with a slash ("/") character. */
+        assert(path.empty() || path.front().empty());
+        res += authority->to_string();
+    } else if (std::ranges::equal(std::views::take(path, 2), std::views::repeat("", 2))) {
+        /* If a URI does not contain an authority component, then the path cannot begin
+           with two slash characters ("//") */
+        unreachable();
+    }
+    res += encodeUrlPath(path);
+    return res;
+}
+
 std::string ParsedURL::to_string() const
 {
-    return scheme + ":" + (authority ? "//" + authority->to_string() : "") + percentEncode(path, allowedInPath)
-           + (query.empty() ? "" : "?" + encodeQuery(query)) + (fragment.empty() ? "" : "#" + percentEncode(fragment));
+    std::string res;
+    res += scheme;
+    res += ":";
+    if (authority.has_value())
+        res += "//";
+    res += renderAuthorityAndPath();
+    if (!query.empty()) {
+        res += "?";
+        res += encodeQuery(query);
+    }
+    if (!fragment.empty()) {
+        res += "#";
+        res += percentEncode(fragment);
+    }
+    return res;
 }
 
 std::ostream & operator<<(std::ostream & os, const ParsedURL & url)
@@ -256,7 +387,7 @@ std::ostream & operator<<(std::ostream & os, const ParsedURL & url)
 ParsedURL ParsedURL::canonicalise()
 {
     ParsedURL res(*this);
-    res.path = CanonPath(res.path).abs();
+    res.path = splitString<std::vector<std::string>>(CanonPath(renderPath()).abs(), "/");
     return res;
 }
 
@@ -277,17 +408,91 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme)
     };
 }
 
-std::string fixGitURL(const std::string & url)
+struct ScpLike
 {
-    std::regex scpRegex("([^/]*)@(.*):(.*)");
-    if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex))
-        return std::regex_replace(url, scpRegex, "ssh://$1@$2/$3");
-    if (hasPrefix(url, "file:"))
-        return url;
-    if (url.find("://") == std::string::npos) {
-        return (ParsedURL{.scheme = "file", .authority = ParsedURL::Authority{}, .path = url}).to_string();
+    ParsedURL::Authority authority;
+    std::string_view path;
+};
+
+/**
+ * Parse a scp url. This is a helper struct for fixGitURL.
+ * This is needed since we support scp-style urls for git urls.
+ * https://git-scm.com/book/ms/v2/Git-on-the-Server-The-Protocols
+ *
+ * A good reference is libgit2 also allows scp style
+ * https://github.com/libgit2/libgit2/blob/58d9363f02f1fa39e46d49b604f27008e75b72f2/src/util/net.c#L806
+ */
+static std::optional<ScpLike> parseScp(const std::string_view s) noexcept
+{
+    if (s.empty() || s.front() == '/')
+        return std::nullopt;
+
+    // Find the colon that separates host from path.
+    // Find the right-most since ipv6 has colons
+    const auto colon = s.rfind(':');
+    if (colon == std::string_view::npos)
+        return std::nullopt;
+
+    // Split head:[path]
+    const auto head = s.substr(0, colon);
+    const auto path = s.substr(colon + 1);
+
+    if (head.empty())
+        return std::nullopt;
+
+    return ScpLike{
+        .authority = ParsedURL::Authority::parse(head),
+        .path = path,
+    };
+}
+
+ParsedURL fixGitURL(const std::string_view url)
+{
+    {
+        std::optional<ParsedURL> parsedOpt;
+        try {
+            parsedOpt = parseURL(url);
+        } catch (BadURL &) {
+            if (hasPrefix(url, "file:"))
+                throw;
+        }
+        if (parsedOpt) {
+            auto & parsed = *parsedOpt;
+            if (parsed.authority)
+                return parsed;
+            if (parsed.scheme == "file")
+                throw BadURL(
+                    "URL '%s' would parse as SCP authority = 'file', path = '%s' but this is also a valid `file:..` URL, and so we choose to disallow it",
+                    url,
+                    parsed.renderPath(true));
+        }
     }
-    return url;
+
+    // if the url does not start with forward slash, add one
+    auto splitMakeAbs = [&](std::string_view pathS) {
+        std::vector<std::string> path;
+
+        if (!hasPrefix(pathS, "/")) {
+            path.emplace_back("");
+        }
+        splitStringInto(path, pathS, "/");
+
+        return path;
+    };
+
+    if (auto scp = parseScp(url)) {
+        return ParsedURL{
+            .scheme = "ssh",
+            .authority = std::move(scp->authority),
+            .path = splitMakeAbs(scp->path),
+        };
+    }
+
+    return ParsedURL{
+        .scheme = "file",
+        .authority = ParsedURL::Authority{},
+        .path = splitMakeAbs(url),
+    };
 }
 
 // https://www.rfc-editor.org/rfc/rfc3986#section-3.1
@@ -299,4 +504,10 @@ bool isValidSchemeName(std::string_view s)
     return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default);
 }
 
+std::ostream & operator<<(std::ostream & os, const ValidURL & url)
+{
+    os << url.to_string();
+    return os;
+}
+
 } // namespace nix

src/nix/app.cc

@@ -103,11 +103,11 @@ UnresolvedApp InstallableValue::toApp(EvalState & state)
 
     else if (type == "derivation") {
         auto drvPath = cursor->forceDerivation();
-        auto outPath = cursor->getAttr(state.sOutPath)->getString();
-        auto outputName = cursor->getAttr(state.sOutputName)->getString();
-        auto name = cursor->getAttr(state.sName)->getString();
+        auto outPath = cursor->getAttr(state.s.outPath)->getString();
+        auto outputName = cursor->getAttr(state.s.outputName)->getString();
+        auto name = cursor->getAttr(state.s.name)->getString();
         auto aPname = cursor->maybeGetAttr("pname");
-        auto aMeta = cursor->maybeGetAttr(state.sMeta);
+        auto aMeta = cursor->maybeGetAttr(state.s.meta);
         auto aMainProgram = aMeta ? aMeta->maybeGetAttr("mainProgram") : nullptr;
         auto mainProgram = aMainProgram ? aMainProgram->getString() : aPname ? aPname->getString() : DrvName(name).name;
         auto program = outPath + "/bin/" + mainProgram;

src/nix/bundle.cc

@@ -100,7 +100,7 @@ struct CmdBundle : InstallableValueCommand
         if (!evalState->isDerivation(*vRes))
             throw Error("the bundler '%s' does not produce a derivation", bundler.what());
 
-        auto attr1 = vRes->attrs()->get(evalState->sDrvPath);
+        auto attr1 = vRes->attrs()->get(evalState->s.drvPath);
         if (!attr1)
             throw Error("the bundler '%s' does not produce a derivation", bundler.what());
 
@@ -109,7 +109,7 @@ struct CmdBundle : InstallableValueCommand
 
         drvPath.requireDerivation();
 
-        auto attr2 = vRes->attrs()->get(evalState->sOutPath);
+        auto attr2 = vRes->attrs()->get(evalState->s.outPath);
         if (!attr2)
             throw Error("the bundler '%s' does not produce a derivation", bundler.what());
 
@@ -123,7 +123,7 @@ struct CmdBundle : InstallableValueCommand
         });
 
         if (!outLink) {
-            auto * attr = vRes->attrs()->get(evalState->sName);
+            auto * attr = vRes->attrs()->get(evalState->s.name);
             if (!attr)
                 throw Error("attribute 'name' missing");
             outLink = evalState->forceStringNoCtx(*attr->value, attr->pos, "");

src/nix/develop.cc

@@ -647,7 +647,7 @@ struct CmdDevelop : Common, MixEnvironment
                 nixpkgs = i->nixpkgsFlakeRef();
 
             auto bashInstallable = make_ref<InstallableFlake>(
-                this,
+                nullptr, //< Don't barf when the command is run with --arg/--argstr
                 state,
                 std::move(nixpkgs),
                 "bashInteractive",

src/nix/flake.cc

@@ -1232,12 +1232,12 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
                 };
 
                 auto showDerivation = [&]() {
-                    auto name = visitor.getAttr(state->sName)->getString();
+                    auto name = visitor.getAttr(state->s.name)->getString();
 
                     if (json) {
                         std::optional<std::string> description;
-                        if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) {
-                            if (auto aDescription = aMeta->maybeGetAttr(state->sDescription))
+                        if (auto aMeta = visitor.maybeGetAttr(state->s.meta)) {
+                            if (auto aDescription = aMeta->maybeGetAttr(state->s.description))
                                 description = aDescription->getString();
                         }
                         j.emplace("type", "derivation");
@@ -1365,8 +1365,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
                     || (attrPath.size() == 3 && attrPathS[0] == "apps")) {
                     auto aType = visitor.maybeGetAttr("type");
                     std::optional<std::string> description;
-                    if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) {
-                        if (auto aDescription = aMeta->maybeGetAttr(state->sDescription))
+                    if (auto aMeta = visitor.maybeGetAttr(state->s.meta)) {
+                        if (auto aDescription = aMeta->maybeGetAttr(state->s.description))
                             description = aDescription->getString();
                     }
                     if (!aType || aType->getString() != "app")

src/nix/nix-env/user-env.cc

@@ -56,21 +56,21 @@ bool createUserEnv(
 
         auto attrs = state.buildBindings(7 + outputs.size());
 
-        attrs.alloc(state.sType).mkString("derivation");
-        attrs.alloc(state.sName).mkString(i.queryName());
+        attrs.alloc(state.s.type).mkString("derivation");
+        attrs.alloc(state.s.name).mkString(i.queryName());
         auto system = i.querySystem();
         if (!system.empty())
-            attrs.alloc(state.sSystem).mkString(system);
-        attrs.alloc(state.sOutPath).mkString(state.store->printStorePath(i.queryOutPath()));
+            attrs.alloc(state.s.system).mkString(system);
+        attrs.alloc(state.s.outPath).mkString(state.store->printStorePath(i.queryOutPath()));
         if (drvPath)
-            attrs.alloc(state.sDrvPath).mkString(state.store->printStorePath(*drvPath));
+            attrs.alloc(state.s.drvPath).mkString(state.store->printStorePath(*drvPath));
 
         // Copy each output meant for installation.
         auto outputsList = state.buildList(outputs.size());
         for (const auto & [m, j] : enumerate(outputs)) {
             (outputsList[m] = state.allocValue())->mkString(j.first);
             auto outputAttrs = state.buildBindings(2);
-            outputAttrs.alloc(state.sOutPath).mkString(state.store->printStorePath(*j.second));
+            outputAttrs.alloc(state.s.outPath).mkString(state.store->printStorePath(*j.second));
             attrs.alloc(j.first).mkAttrs(outputAttrs);
 
             /* This is only necessary when installing store paths, e.g.,
@@ -80,7 +80,7 @@ bool createUserEnv(
 
             references.insert(*j.second);
         }
-        attrs.alloc(state.sOutputs).mkList(outputsList);
+        attrs.alloc(state.s.outputs).mkList(outputsList);
 
         // Copy the meta attributes.
         auto meta = state.buildBindings(metaNames.size());
@@ -91,7 +91,7 @@ bool createUserEnv(
             meta.insert(state.symbols.create(j), v);
         }
 
-        attrs.alloc(state.sMeta).mkAttrs(meta);
+        attrs.alloc(state.s.meta).mkAttrs(meta);
 
         (list[n] = state.allocValue())->mkAttrs(attrs);
 
@@ -141,10 +141,10 @@ bool createUserEnv(
     debug("evaluating user environment builder");
     state.forceValue(topLevel, topLevel.determinePos(noPos));
     NixStringContext context;
-    auto & aDrvPath(*topLevel.attrs()->find(state.sDrvPath));
+    auto & aDrvPath(*topLevel.attrs()->find(state.s.drvPath));
     auto topLevelDrv = state.coerceToStorePath(aDrvPath.pos, *aDrvPath.value, context, "");
     topLevelDrv.requireDerivation();
-    auto & aOutPath(*topLevel.attrs()->find(state.sOutPath));
+    auto & aOutPath(*topLevel.attrs()->find(state.s.outPath));
     auto topLevelOut = state.coerceToStorePath(aOutPath.pos, *aOutPath.value, context, "");
 
     /* Realise the resulting store expression. */

src/nix/prefetch.cc

@@ -105,7 +105,7 @@ std::tuple<StorePath, Hash> prefetchFile(
 
             FdSink sink(fd.get());
 
-            FileTransferRequest req(url);
+            FileTransferRequest req(ValidURL{url});
             req.decompress = false;
             getFileTransfer()->download(std::move(req), sink);
         }

src/nix/profile.cc

@@ -105,7 +105,8 @@ std::string getNameFromElement(const ProfileElement & element)
 {
     std::optional<std::string> result = std::nullopt;
     if (element.source) {
-        result = getNameFromURL(parseURL(element.source->to_string()));
+        // Seems to be for Flake URLs
+        result = getNameFromURL(parseURL(element.source->to_string(), /*lenient=*/true));
     }
     return result.value_or(element.identifier());
 }
@@ -160,11 +161,15 @@ struct ProfileManifest
                         e["outputs"].get<ExtendedOutputsSpec>()};
                 }
 
-                std::string name =
-                    elems.is_object() ? elem.key()
-                    : element.source
-                        ? getNameFromURL(parseURL(element.source->to_string())).value_or(element.identifier())
-                        : element.identifier();
+                std::string name = [&] {
+                    if (elems.is_object())
+                        return elem.key();
+                    if (element.source) {
+                        if (auto optName = getNameFromURL(parseURL(element.source->to_string(), /*lenient=*/true)))
+                            return *optName;
+                    }
+                    return element.identifier();
+                }();
 
                 addElement(name, std::move(element));
             }

src/nix/search.cc

@@ -108,10 +108,10 @@ struct CmdSearch : InstallableValueCommand, MixJSON
                 };
 
                 if (cursor.isDerivation()) {
-                    DrvName name(cursor.getAttr(state->sName)->getString());
+                    DrvName name(cursor.getAttr(state->s.name)->getString());
 
-                    auto aMeta = cursor.maybeGetAttr(state->sMeta);
-                    auto aDescription = aMeta ? aMeta->maybeGetAttr(state->sDescription) : nullptr;
+                    auto aMeta = cursor.maybeGetAttr(state->s.meta);
+                    auto aDescription = aMeta ? aMeta->maybeGetAttr(state->s.description) : nullptr;
                     auto description = aDescription ? aDescription->getString() : "";
                     std::replace(description.begin(), description.end(), '\n', ' ');
                     auto attrPath2 = concatStringsSep(".", attrPathS);
@@ -176,7 +176,7 @@ struct CmdSearch : InstallableValueCommand, MixJSON
                     recurse();
 
                 else if (attrPathS[0] == "legacyPackages" && attrPath.size() > 2) {
-                    auto attr = cursor.maybeGetAttr(state->sRecurseForDerivations);
+                    auto attr = cursor.maybeGetAttr(state->s.recurseForDerivations);
                     if (attr && attr->getBool())
                         recurse();
                 }

src/nix/upgrade-nix.cc

@@ -156,7 +156,7 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand
         Activity act(*logger, lvlInfo, actUnknown, "querying latest Nix version");
 
         // FIXME: use nixos.org?
-        auto req = FileTransferRequest((std::string &) settings.upgradeNixStorePathUrl);
+        auto req = FileTransferRequest(parseURL(settings.upgradeNixStorePathUrl.get()));
         auto res = getFileTransfer()->download(req);
 
         auto state = std::make_unique<EvalState>(LookupPath{}, store, fetchSettings, evalSettings);

tests/functional/dyn-drv/failing-outer.sh

@@ -9,4 +9,4 @@ expected=100
 if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly
 
 expectStderr "$expected" nix-build ./text-hashed-output.nix -A failingWrapper --no-out-link \
-    | grepQuiet "build of '.*use-dynamic-drv-in-non-dynamic-drv-wrong.drv' failed"
+    | grepQuiet "build of resolved derivation '.*use-dynamic-drv-in-non-dynamic-drv-wrong.drv' failed"

tests/functional/fetchurl.sh

@@ -88,3 +88,8 @@ requireDaemonNewerThan "2.20"
 expected=100
 if [[ -v NIX_DAEMON_PACKAGE ]]; then expected=1; fi # work around the daemon not returning a 100 status correctly
 expectStderr $expected nix-build --expr '{ url }: builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; inherit url; outputHashMode = "flat"; }' --argstr url "file://$narxz" 2>&1 | grep 'must be a fixed-output or impure derivation'
+
+requireDaemonNewerThan "2.32.0pre20250831"
+
+expect 1 nix-build --expr 'import <nix/fetchurl.nix>' --argstr name 'name' --argstr url "file://authority.not.allowed/fetchurl.sh?a=1&a=2" --no-out-link |&
+        grepQuiet "error: file:// URL 'file://authority.not.allowed/fetchurl.sh?a=1&a=2' has unexpected authority 'authority.not.allowed'"

tests/functional/lang/eval-fail-fromTOML-overflow.err.exp

@@ -0,0 +1,13 @@
+error:
+       … while calling the 'fromTOML' builtin
+         at /pwd/lang/eval-fail-fromTOML-overflow.nix:1:1:
+            1| builtins.fromTOML ''attr = 9223372036854775808''
+             | ^
+            2|
+
+       error: while parsing TOML: [error] toml::parse_dec_integer: too large integer: current max digits = 2^63
+        --> fromTOML
+          |
+        1 | attr = 9223372036854775808
+          |                           ^-- must be < 2^63
+       

tests/functional/lang/eval-fail-fromTOML-overflow.nix

@@ -0,0 +1 @@
+builtins.fromTOML ''attr = 9223372036854775808''

tests/functional/lang/eval-fail-fromTOML-underflow.err.exp

@@ -0,0 +1,13 @@
+error:
+       … while calling the 'fromTOML' builtin
+         at /pwd/lang/eval-fail-fromTOML-underflow.nix:1:1:
+            1| builtins.fromTOML ''attr = -9223372036854775809''
+             | ^
+            2|
+
+       error: while parsing TOML: [error] toml::parse_dec_integer: too large integer: current max digits = 2^63
+        --> fromTOML
+          |
+        1 | attr = -9223372036854775809
+          |                            ^-- must be < 2^63
+       

tests/functional/lang/eval-fail-fromTOML-underflow.nix

@@ -0,0 +1 @@
+builtins.fromTOML ''attr = -9223372036854775809''

tests/functional/lang/eval-okay-fromTOML-timestamps.exp

@@ -1 +1 @@
-{ "1234" = "value"; "127.0.0.1" = "value"; a = { b = { c = { }; }; }; arr1 = [ 1 2 3 ]; arr2 = [ "red" "yellow" "green" ]; arr3 = [ [ 1 2 ] [ 3 4 5 ] ]; arr4 = [ "all" "strings" "are the same" "type" ]; arr5 = [ [ 1 2 ] [ "a" "b" "c" ] ]; arr7 = [ 1 2 3 ]; arr8 = [ 1 2 ]; bare-key = "value"; bare_key = "value"; bin1 = 214; bool1 = true; bool2 = false; "character encoding" = "value"; d = { e = { f = { }; }; }; dog = { "tater.man" = { type = { name = "pug"; }; }; }; flt1 = 1; flt2 = 3.1415; flt3 = -0.01; flt4 = 5e+22; flt5 = 1e+06; flt6 = -0.02; flt7 = 6.626e-34; flt8 = 9.22462e+06; fruit = [ { name = "apple"; physical = { color = "red"; shape = "round"; }; variety = [ { name = "red delicious"; } { name = "granny smith"; } ]; } { name = "banana"; variety = [ { name = "plantain"; } ]; } ]; g = { h = { i = { }; }; }; hex1 = 3735928559; hex2 = 3735928559; hex3 = 3735928559; int1 = 99; int2 = 42; int3 = 0; int4 = -17; int5 = 1000; int6 = 5349221; int7 = 12345; j = { "ʞ" = { l = { }; }; }; key = "value"; key2 = "value"; ld1 = { _type = "timestamp"; value = "1979-05-27"; }; ldt1 = { _type = "timestamp"; value = "1979-05-27T07:32:00"; }; ldt2 = { _type = "timestamp"; value = "1979-05-27T00:32:00.999999"; }; lt1 = { _type = "timestamp"; value = "07:32:00"; }; lt2 = { _type = "timestamp"; value = "00:32:00.999999"; }; name = "Orange"; oct1 = 342391; oct2 = 493; odt1 = { _type = "timestamp"; value = "1979-05-27T07:32:00Z"; }; odt2 = { _type = "timestamp"; value = "1979-05-27T00:32:00-07:00"; }; odt3 = { _type = "timestamp"; value = "1979-05-27T00:32:00.999999-07:00"; }; odt4 = { _type = "timestamp"; value = "1979-05-27T07:32:00Z"; }; physical = { color = "orange"; shape = "round"; }; products = [ { name = "Hammer"; sku = 738594937; } { } { color = "gray"; name = "Nail"; sku = 284758393; } ]; "quoted \"value\"" = "value"; site = { "google.com" = true; }; str = "I'm a string. \"You can quote me\". Name\tJosé\nLocation\tSF."; table-1 = { key1 = "some string"; key2 = 123; }; table-2 = { key1 = "another string"; key2 = 456; }; x = { y = { z = { w = { animal = { type = { name = "pug"; }; }; name = { first = "Tom"; last = "Preston-Werner"; }; point = { x = 1; y = 2; }; }; }; }; }; "ʎǝʞ" = "value"; }
+{ "1234" = "value"; "127.0.0.1" = "value"; a = { b = { c = { }; }; }; arr1 = [ 1 2 3 ]; arr2 = [ "red" "yellow" "green" ]; arr3 = [ [ 1 2 ] [ 3 4 5 ] ]; arr4 = [ "all" "strings" "are the same" "type" ]; arr5 = [ [ 1 2 ] [ "a" "b" "c" ] ]; arr7 = [ 1 2 3 ]; arr8 = [ 1 2 ]; bare-key = "value"; bare_key = "value"; bin1 = 214; bool1 = true; bool2 = false; "character encoding" = "value"; d = { e = { f = { }; }; }; dog = { "tater.man" = { type = { name = "pug"; }; }; }; flt1 = 1; flt2 = 3.1415; flt3 = -0.01; flt4 = 5e+22; flt5 = 1e+06; flt6 = -0.02; flt7 = 6.626e-34; flt8 = 9.22462e+06; fruit = [ { name = "apple"; physical = { color = "red"; shape = "round"; }; variety = [ { name = "red delicious"; } { name = "granny smith"; } ]; } { name = "banana"; variety = [ { name = "plantain"; } ]; } ]; g = { h = { i = { }; }; }; hex1 = 3735928559; hex2 = 3735928559; hex3 = 3735928559; int1 = 99; int2 = 42; int3 = 0; int4 = -17; int5 = 1000; int6 = 5349221; int7 = 12345; j = { "ʞ" = { l = { }; }; }; key = "value"; key2 = "value"; ld1 = { _type = "timestamp"; value = "1979-05-27"; }; ldt1 = { _type = "timestamp"; value = "1979-05-27T07:32:00"; }; ldt10 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123456789"; }; ldt11 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123456789"; }; ldt2 = { _type = "timestamp"; value = "1979-05-27T07:32:00.100"; }; ldt3 = { _type = "timestamp"; value = "1979-05-27T07:32:00.120"; }; ldt4 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123"; }; ldt5 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123400"; }; ldt6 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123450"; }; ldt7 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123456"; }; ldt8 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123456700"; }; ldt9 = { _type = "timestamp"; value = "1979-05-27T00:32:00.123456780"; }; lt1 = { _type = "timestamp"; value = "07:32:00"; }; lt10 = { _type = "timestamp"; value = "00:32:00.123456789"; }; lt11 = { _type = "timestamp"; value = "00:32:00.123456789"; }; lt2 = { _type = "timestamp"; value = "00:32:00.100"; }; lt3 = { _type = "timestamp"; value = "00:32:00.120"; }; lt4 = { _type = "timestamp"; value = "00:32:00.123"; }; lt5 = { _type = "timestamp"; value = "00:32:00.123400"; }; lt6 = { _type = "timestamp"; value = "00:32:00.123450"; }; lt7 = { _type = "timestamp"; value = "00:32:00.123456"; }; lt8 = { _type = "timestamp"; value = "00:32:00.123456700"; }; lt9 = { _type = "timestamp"; value = "00:32:00.123456780"; }; name = "Orange"; oct1 = 342391; oct2 = 493; odt1 = { _type = "timestamp"; value = "1979-05-27T07:32:00Z"; }; odt10 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123456Z"; }; odt11 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123456700Z"; }; odt12 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123456780Z"; }; odt13 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123456789Z"; }; odt14 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123456789Z"; }; odt2 = { _type = "timestamp"; value = "1979-05-27T00:32:00-07:00"; }; odt3 = { _type = "timestamp"; value = "1979-05-27T00:32:00.999999-07:00"; }; odt4 = { _type = "timestamp"; value = "1979-05-27T07:32:00Z"; }; odt5 = { _type = "timestamp"; value = "1979-05-27T07:32:00.100Z"; }; odt6 = { _type = "timestamp"; value = "1979-05-27T07:32:00.120Z"; }; odt7 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123Z"; }; odt8 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123400Z"; }; odt9 = { _type = "timestamp"; value = "1979-05-27T07:32:00.123450Z"; }; physical = { color = "orange"; shape = "round"; }; products = [ { name = "Hammer"; sku = 738594937; } { } { color = "gray"; name = "Nail"; sku = 284758393; } ]; "quoted \"value\"" = "value"; site = { "google.com" = true; }; str = "I'm a string. \"You can quote me\". Name\tJosé\nLocation\tSF."; table-1 = { key1 = "some string"; key2 = 123; }; table-2 = { key1 = "another string"; key2 = 456; }; x = { y = { z = { w = { animal = { type = { name = "pug"; }; }; name = { first = "Tom"; last = "Preston-Werner"; }; point = { x = 1; y = 2; }; }; }; }; }; "ʎǝʞ" = "value"; }