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

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

.mergify.yml

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

.version

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

doc/manual/rl-next/shorter-build-dir-names.md

@@ -0,0 +1,6 @@
+---
+synopsis: "Temporary build directories no longer include derivation names"
+prs: [13839]
+---
+
+Temporary build directories created during derivation builds no longer include the derivation name in their path to avoid build failures when the derivation name is too long. This change ensures predictable prefix lengths for build directories under `/nix/var/nix/builds`.

doc/manual/source/development/building.md

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

docker.nix

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

flake.nix

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

maintainers/README.md

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

maintainers/release-notes-todo

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

maintainers/release-process.md

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

packaging/dependencies.nix

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

src/libcmd/installable-flake.cc

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

src/libcmd/installables.cc

@@ -178,10 +178,16 @@ MixFlakeOptions::MixFlakeOptions()
             for (auto & [inputName, input] : flake.lockFile.root->inputs) {
                 auto input2 = flake.lockFile.findInput({inputName}); // resolve 'follows' nodes
                 if (auto input3 = std::dynamic_pointer_cast<const flake::LockedNode>(input2)) {
+                    fetchers::Attrs extraAttrs;
+
+                    if (!input3->lockedRef.subdir.empty()) {
+                        extraAttrs["dir"] = input3->lockedRef.subdir;
+                    }
+
                     overrideRegistry(
                         fetchers::Input::fromAttrs(fetchSettings, {{"type", "indirect"}, {"id", inputName}}),
                         input3->lockedRef.input,
-                        {});
+                        extraAttrs);
                 }
             }
         }},

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.s.type, &vDerivation);
+    builder.insert(state.sType, &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.s.type, &vDerivation);
-    builder.insert(state.s.drvPath, &vError);
+    builder.insert(state.sType, &vDerivation);
+    builder.insert(state.sDrvPath, &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.s.epsilon};
+        return {0, root->state.sEpsilon};
     if (!parent->first->cachedValue) {
         parent->first->cachedValue = root->db->getAttr(parent->first->getKey());
         assert(parent->first->cachedValue);
@@ -702,7 +702,7 @@ bool AttrCursor::isDerivation()
 
 StorePath AttrCursor::forceDerivation()
 {
-    auto aDrvPath = getAttr(root->state.s.drvPath);
+    auto aDrvPath = getAttr(root->state.sDrvPath);
     auto drvPath = root->state.store->parseStorePath(aDrvPath->getString());
     drvPath.requireDerivation();
     if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) {

src/libexpr/eval-profiler.cc

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

src/libexpr/eval.cc

@@ -203,65 +203,124 @@ EvalState::EvalState(
     std::shared_ptr<Store> buildStore)
     : fetchSettings{fetchSettings}
     , settings{settings}
-    , symbols(StaticEvalSymbols::staticSymbolTable())
+    , sWith(symbols.create("<with>"))
+    , sOutPath(symbols.create("outPath"))
+    , sDrvPath(symbols.create("drvPath"))
+    , sType(symbols.create("type"))
+    , sMeta(symbols.create("meta"))
+    , sName(symbols.create("name"))
+    , sValue(symbols.create("value"))
+    , sSystem(symbols.create("system"))
+    , sOverrides(symbols.create("__overrides"))
+    , sOutputs(symbols.create("outputs"))
+    , sOutputName(symbols.create("outputName"))
+    , sIgnoreNulls(symbols.create("__ignoreNulls"))
+    , sFile(symbols.create("file"))
+    , sLine(symbols.create("line"))
+    , sColumn(symbols.create("column"))
+    , sFunctor(symbols.create("__functor"))
+    , sToString(symbols.create("__toString"))
+    , sRight(symbols.create("right"))
+    , sWrong(symbols.create("wrong"))
+    , sStructuredAttrs(symbols.create("__structuredAttrs"))
+    , sJson(symbols.create("__json"))
+    , sAllowedReferences(symbols.create("allowedReferences"))
+    , sAllowedRequisites(symbols.create("allowedRequisites"))
+    , sDisallowedReferences(symbols.create("disallowedReferences"))
+    , sDisallowedRequisites(symbols.create("disallowedRequisites"))
+    , sMaxSize(symbols.create("maxSize"))
+    , sMaxClosureSize(symbols.create("maxClosureSize"))
+    , sBuilder(symbols.create("builder"))
+    , sArgs(symbols.create("args"))
+    , sContentAddressed(symbols.create("__contentAddressed"))
+    , sImpure(symbols.create("__impure"))
+    , sOutputHash(symbols.create("outputHash"))
+    , sOutputHashAlgo(symbols.create("outputHashAlgo"))
+    , sOutputHashMode(symbols.create("outputHashMode"))
+    , sRecurseForDerivations(symbols.create("recurseForDerivations"))
+    , sDescription(symbols.create("description"))
+    , sSelf(symbols.create("self"))
+    , sEpsilon(symbols.create(""))
+    , sStartSet(symbols.create("startSet"))
+    , sOperator(symbols.create("operator"))
+    , sKey(symbols.create("key"))
+    , sPath(symbols.create("path"))
+    , sPrefix(symbols.create("prefix"))
+    , sOutputSpecified(symbols.create("outputSpecified"))
+    , exprSymbols{
+        .sub = symbols.create("__sub"),
+        .lessThan = symbols.create("__lessThan"),
+        .mul = symbols.create("__mul"),
+        .div = symbols.create("__div"),
+        .or_ = symbols.create("or"),
+        .findFile = symbols.create("__findFile"),
+        .nixPath = symbols.create("__nixPath"),
+        .body = symbols.create("body"),
+    }
     , repair(NoRepair)
     , emptyBindings(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())
@@ -595,7 +654,7 @@ std::optional<EvalState::Doc> EvalState::getDoc(Value & v)
     }
     if (isFunctor(v)) {
         try {
-            Value & functor = *v.attrs()->find(s.functor)->value;
+            Value & functor = *v.attrs()->find(sFunctor)->value;
             Value * vp[] = {&v};
             Value partiallyApplied;
             // The first parameter is not user-provided, and may be
@@ -919,8 +978,8 @@ void EvalState::mkPos(Value & v, PosIdx p)
     auto origin = positions.originOf(p);
     if (auto path = std::get_if<SourcePath>(&origin)) {
         auto attrs = buildBindings(3);
-        attrs.alloc(s.file).mkString(path->path.abs());
-        makePositionThunks(*this, p, attrs.alloc(s.line), attrs.alloc(s.column));
+        attrs.alloc(sFile).mkString(path->path.abs());
+        makePositionThunks(*this, p, attrs.alloc(sLine), attrs.alloc(sColumn));
         v.mkAttrs(attrs);
     } else
         v.mkNull();
@@ -1186,7 +1245,7 @@ void ExprAttrs::eval(EvalState & state, Env & env, Value & v)
         dynamicEnv = &env2;
         Env * inheritEnv = inheritFromExprs ? buildInheritFromEnv(state, env2) : nullptr;
 
-        AttrDefs::iterator overrides = attrs.find(state.s.overrides);
+        AttrDefs::iterator overrides = attrs.find(state.sOverrides);
         bool hasOverrides = overrides != attrs.end();
 
         /* The recursive attributes are evaluated in the new
@@ -1658,7 +1717,7 @@ void EvalState::callFunction(Value & fun, std::span<Value *> args, Value & vRes,
             }
         }
 
-        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(s.functor))) {
+        else if (vCur.type() == nAttrs && (functor = vCur.attrs()->get(sFunctor))) {
             /* 'vCur' may be allocated on the stack of the calling
                function, but for functors we may keep a reference, so
                heap-allocate a copy and use that instead. */
@@ -1720,7 +1779,7 @@ void EvalState::autoCallFunction(const Bindings & args, Value & fun, Value & res
     forceValue(fun, pos);
 
     if (fun.type() == nAttrs) {
-        auto found = fun.attrs()->find(s.functor);
+        auto found = fun.attrs()->find(sFunctor);
         if (found != fun.attrs()->end()) {
             Value * v = allocValue();
             callFunction(*found->value, fun, *v, pos);
@@ -2182,7 +2241,7 @@ Bindings::const_iterator EvalState::getAttr(Symbol attrSym, const Bindings * att
 
 bool EvalState::isFunctor(const Value & fun) const
 {
-    return fun.type() == nAttrs && fun.attrs()->find(s.functor) != fun.attrs()->end();
+    return fun.type() == nAttrs && fun.attrs()->find(sFunctor) != fun.attrs()->end();
 }
 
 void EvalState::forceFunction(Value & v, const PosIdx pos, std::string_view errorCtx)
@@ -2251,7 +2310,7 @@ bool EvalState::isDerivation(Value & v)
 {
     if (v.type() != nAttrs)
         return false;
-    auto i = v.attrs()->get(s.type);
+    auto i = v.attrs()->get(sType);
     if (!i)
         return false;
     forceValue(*i->value, i->pos);
@@ -2263,7 +2322,7 @@ bool EvalState::isDerivation(Value & v)
 std::optional<std::string>
 EvalState::tryAttrsToString(const PosIdx pos, Value & v, NixStringContext & context, bool coerceMore, bool copyToStore)
 {
-    auto i = v.attrs()->find(s.toString);
+    auto i = v.attrs()->find(sToString);
     if (i != v.attrs()->end()) {
         Value v1;
         callFunction(*i->value, v, v1, pos);
@@ -2309,7 +2368,7 @@ BackedStringView EvalState::coerceToString(
         auto maybeString = tryAttrsToString(pos, v, context, coerceMore, copyToStore);
         if (maybeString)
             return std::move(*maybeString);
-        auto i = v.attrs()->find(s.outPath);
+        auto i = v.attrs()->find(sOutPath);
         if (i == v.attrs()->end()) {
             error<TypeError>(
                 "cannot coerce %1% to a string: %2%", showType(v), ValuePrinter(*this, v, errorPrintOptions))
@@ -2416,7 +2475,7 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, NixStringContext
     /* Similarly, handle __toString where the result may be a path
        value. */
     if (v.type() == nAttrs) {
-        auto i = v.attrs()->find(s.toString);
+        auto i = v.attrs()->find(sToString);
         if (i != v.attrs()->end()) {
             Value v1;
             callFunction(*i->value, v, v1, pos);
@@ -2606,8 +2665,8 @@ void EvalState::assertEqValues(Value & v1, Value & v2, const PosIdx pos, std::st
 
     case nAttrs: {
         if (isDerivation(v1) && isDerivation(v2)) {
-            auto i = v1.attrs()->get(s.outPath);
-            auto j = v2.attrs()->get(s.outPath);
+            auto i = v1.attrs()->get(sOutPath);
+            auto j = v2.attrs()->get(sOutPath);
             if (i && j) {
                 try {
                     assertEqValues(*i->value, *j->value, pos, errorCtx);
@@ -2760,8 +2819,8 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         /* If both sets denote a derivation (type = "derivation"),
            then compare their outPaths. */
         if (isDerivation(v1) && isDerivation(v2)) {
-            auto i = v1.attrs()->get(s.outPath);
-            auto j = v2.attrs()->get(s.outPath);
+            auto i = v1.attrs()->get(sOutPath);
+            auto j = v2.attrs()->get(sOutPath);
             if (i && j)
                 return eqValues(*i->value, *j->value, pos, errorCtx);
         }
@@ -3137,7 +3196,8 @@ Expr * EvalState::parse(
         docComments = &it->second;
     }
 
-    auto result = parseExprFromBuf(text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS);
+    auto result = parseExprFromBuf(
+        text, length, origin, basePath, symbols, settings, positions, *docComments, rootFS, exprSymbols);
 
     result->bindVars(*this, staticEnv);
 

src/libexpr/get-drvs.cc

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

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

@@ -213,100 +213,23 @@ struct DebugTrace
     }
 };
 
-struct StaticEvalSymbols
-{
-    Symbol with, outPath, drvPath, type, meta, name, value, system, overrides, outputs, outputName, ignoreNulls, file,
-        line, column, functor, toString, right, wrong, structuredAttrs, json, allowedReferences, allowedRequisites,
-        disallowedReferences, disallowedRequisites, maxSize, maxClosureSize, builder, args, contentAddressed, impure,
-        outputHash, outputHashAlgo, outputHashMode, recurseForDerivations, description, self, epsilon, startSet,
-        operator_, key, path, prefix, outputSpecified;
-
-    Expr::AstSymbols exprSymbols;
-
-    static constexpr auto preallocate()
-    {
-        StaticSymbolTable alloc;
-
-        StaticEvalSymbols staticSymbols = {
-            .with = alloc.create("<with>"),
-            .outPath = alloc.create("outPath"),
-            .drvPath = alloc.create("drvPath"),
-            .type = alloc.create("type"),
-            .meta = alloc.create("meta"),
-            .name = alloc.create("name"),
-            .value = alloc.create("value"),
-            .system = alloc.create("system"),
-            .overrides = alloc.create("__overrides"),
-            .outputs = alloc.create("outputs"),
-            .outputName = alloc.create("outputName"),
-            .ignoreNulls = alloc.create("__ignoreNulls"),
-            .file = alloc.create("file"),
-            .line = alloc.create("line"),
-            .column = alloc.create("column"),
-            .functor = alloc.create("__functor"),
-            .toString = alloc.create("__toString"),
-            .right = alloc.create("right"),
-            .wrong = alloc.create("wrong"),
-            .structuredAttrs = alloc.create("__structuredAttrs"),
-            .json = alloc.create("__json"),
-            .allowedReferences = alloc.create("allowedReferences"),
-            .allowedRequisites = alloc.create("allowedRequisites"),
-            .disallowedReferences = alloc.create("disallowedReferences"),
-            .disallowedRequisites = alloc.create("disallowedRequisites"),
-            .maxSize = alloc.create("maxSize"),
-            .maxClosureSize = alloc.create("maxClosureSize"),
-            .builder = alloc.create("builder"),
-            .args = alloc.create("args"),
-            .contentAddressed = alloc.create("__contentAddressed"),
-            .impure = alloc.create("__impure"),
-            .outputHash = alloc.create("outputHash"),
-            .outputHashAlgo = alloc.create("outputHashAlgo"),
-            .outputHashMode = alloc.create("outputHashMode"),
-            .recurseForDerivations = alloc.create("recurseForDerivations"),
-            .description = alloc.create("description"),
-            .self = alloc.create("self"),
-            .epsilon = alloc.create(""),
-            .startSet = alloc.create("startSet"),
-            .operator_ = alloc.create("operator"),
-            .key = alloc.create("key"),
-            .path = alloc.create("path"),
-            .prefix = alloc.create("prefix"),
-            .outputSpecified = alloc.create("outputSpecified"),
-            .exprSymbols = {
-                .sub = alloc.create("__sub"),
-                .lessThan = alloc.create("__lessThan"),
-                .mul = alloc.create("__mul"),
-                .div = alloc.create("__div"),
-                .or_ = alloc.create("or"),
-                .findFile = alloc.create("__findFile"),
-                .nixPath = alloc.create("__nixPath"),
-                .body = alloc.create("body"),
-            }};
-
-        return std::pair{staticSymbols, alloc};
-    }
-
-    static consteval StaticEvalSymbols create()
-    {
-        return preallocate().first;
-    }
-
-    static constexpr StaticSymbolTable staticSymbolTable()
-    {
-        return preallocate().second;
-    }
-};
-
 class EvalState : public std::enable_shared_from_this<EvalState>
 {
 public:
-    static constexpr StaticEvalSymbols s = StaticEvalSymbols::create();
-
     const fetchers::Settings & fetchSettings;
     const EvalSettings & settings;
     SymbolTable symbols;
     PosTable positions;
 
+    const Symbol sWith, sOutPath, sDrvPath, sType, sMeta, sName, sValue, sSystem, sOverrides, sOutputs, sOutputName,
+        sIgnoreNulls, sFile, sLine, sColumn, sFunctor, sToString, sRight, sWrong, sStructuredAttrs, sJson,
+        sAllowedReferences, sAllowedRequisites, sDisallowedReferences, sDisallowedRequisites, sMaxSize, sMaxClosureSize,
+        sBuilder, sArgs, sContentAddressed, sImpure, sOutputHash, sOutputHashAlgo, sOutputHashMode,
+        sRecurseForDerivations, sDescription, sSelf, sEpsilon, sStartSet, sOperator, sKey, sPath, sPrefix,
+        sOutputSpecified;
+
+    const Expr::AstSymbols exprSymbols;
+
     /**
      * If set, force copying files to the Nix store even if they
      * already exist there.

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

@@ -595,17 +595,12 @@ struct ExprOpNot : Expr
         {                                                                                    \
             return pos;                                                                      \
         }                                                                                    \
-    }
+    };
 
-MakeBinOp(ExprOpEq, "==");
-MakeBinOp(ExprOpNEq, "!=");
-MakeBinOp(ExprOpAnd, "&&");
-MakeBinOp(ExprOpOr, "||");
-MakeBinOp(ExprOpImpl, "->");
-MakeBinOp(ExprOpUpdate, "//");
-MakeBinOp(ExprOpConcatLists, "++");
+MakeBinOp(ExprOpEq, "==") MakeBinOp(ExprOpNEq, "!=") MakeBinOp(ExprOpAnd, "&&") MakeBinOp(ExprOpOr, "||")
+    MakeBinOp(ExprOpImpl, "->") MakeBinOp(ExprOpUpdate, "//") MakeBinOp(ExprOpConcatLists, "++")
 
-struct ExprConcatStrings : Expr
+        struct ExprConcatStrings : Expr
 {
     PosIdx pos;
     bool forceString;

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

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

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

@@ -28,8 +28,6 @@ public:
     }
 };
 
-class StaticSymbolTable;
-
 /**
  * Symbols have the property that they can be compared efficiently
  * (using an equality test), because the symbol table stores only one
@@ -39,29 +37,36 @@ class Symbol
 {
     friend class SymbolStr;
     friend class SymbolTable;
-    friend class StaticSymbolTable;
 
 private:
     uint32_t id;
 
-    explicit constexpr Symbol(uint32_t id) noexcept
+    explicit Symbol(uint32_t id) noexcept
         : id(id)
     {
     }
 
 public:
-    constexpr Symbol() noexcept
+    Symbol() noexcept
         : id(0)
     {
     }
 
     [[gnu::always_inline]]
-    constexpr explicit operator bool() const noexcept
+    explicit operator bool() const noexcept
     {
         return id > 0;
     }
 
-    constexpr auto operator<=>(const Symbol & other) const noexcept = default;
+    auto operator<=>(const Symbol other) const noexcept
+    {
+        return id <=> other.id;
+    }
+
+    bool operator==(const Symbol other) const noexcept
+    {
+        return id == other.id;
+    }
 
     friend class std::hash<Symbol>;
 };
@@ -205,39 +210,6 @@ public:
     };
 };
 
-class SymbolTable;
-
-/**
- * Convenience class to statically assign symbol identifiers at compile-time.
- */
-class StaticSymbolTable
-{
-    static constexpr std::size_t maxSize = 1024;
-
-    struct StaticSymbolInfo
-    {
-        std::string_view str;
-        Symbol sym;
-    };
-
-    std::array<StaticSymbolInfo, maxSize> symbols;
-    std::size_t size = 0;
-
-public:
-    constexpr StaticSymbolTable() = default;
-
-    constexpr Symbol create(std::string_view str)
-    {
-        /* No need to check bounds because out of bounds access is
-           a compilation error. */
-        auto sym = Symbol(size + 1); //< +1 because Symbol with id = 0 is reserved
-        symbols[size++] = {str, sym};
-        return sym;
-    }
-
-    void copyIntoSymbolTable(SymbolTable & symtab) const;
-};
-
 /**
  * Symbol table used by the parser and evaluator to represent and look
  * up identifiers and attributes efficiently.
@@ -260,10 +232,6 @@ private:
     boost::unordered_flat_set<SymbolStr, SymbolStr::Hash, SymbolStr::Equal> symbols{SymbolStr::chunkSize};
 
 public:
-    SymbolTable(const StaticSymbolTable & staticSymtab)
-    {
-        staticSymtab.copyIntoSymbolTable(*this);
-    }
 
     /**
      * Converts a string into a symbol.
@@ -308,16 +276,6 @@ public:
     }
 };
 
-inline void StaticSymbolTable::copyIntoSymbolTable(SymbolTable & symtab) const
-{
-    for (std::size_t i = 0; i < size; ++i) {
-        auto [str, staticSym] = symbols[i];
-        auto sym = symtab.create(str);
-        if (sym != staticSym) [[unlikely]]
-            unreachable();
-    }
-}
-
 } // namespace nix
 
 template<>

src/libexpr/meson.build

@@ -40,7 +40,10 @@ endforeach
 
 boost = dependency(
   'boost',
-  modules : [ 'container', 'context' ],
+  modules : [
+    'container',
+    'context',
+  ],
   include_type : 'system',
 )
 # boost is a public dependency, but not a pkg-config dependency unfortunately, so we

src/libexpr/parser.y

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

src/libexpr/primops.cc

@@ -214,20 +214,20 @@ void derivationToValue(
     auto path2 = path.path.abs();
     Derivation drv = state.store->readDerivation(storePath);
     auto attrs = state.buildBindings(3 + drv.outputs.size());
-    attrs.alloc(state.s.drvPath)
+    attrs.alloc(state.sDrvPath)
         .mkString(
             path2,
             {
                 NixStringContextElem::DrvDeep{.drvPath = storePath},
             });
-    attrs.alloc(state.s.name).mkString(drv.env["name"]);
+    attrs.alloc(state.sName).mkString(drv.env["name"]);
 
     auto list = state.buildList(drv.outputs.size());
     for (const auto & [i, o] : enumerate(drv.outputs)) {
         mkOutputString(state, attrs, storePath, o);
         (list[i] = state.allocValue())->mkString(o.first);
     }
-    attrs.alloc(state.s.outputs).mkList(list);
+    attrs.alloc(state.sOutputs).mkList(list);
 
     auto w = state.allocValue();
     w->mkAttrs(attrs);
@@ -731,7 +731,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the start set. */
     auto startSet = state.getAttr(
-        state.s.startSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.sStartSet, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
 
     state.forceList(
         *startSet->value,
@@ -749,7 +749,7 @@ static void prim_genericClosure(EvalState & state, const PosIdx pos, Value ** ar
 
     /* Get the operator. */
     auto op = state.getAttr(
-        state.s.operator_, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
+        state.sOperator, args[0]->attrs(), "in the attrset passed as argument to builtins.genericClosure");
     state.forceFunction(
         *op->value, noPos, "while evaluating the 'operator' attribute passed as argument to builtins.genericClosure");
 
@@ -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.s.key,
+            state.sKey,
             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.s.value, args[0]);
+        attrs.insert(state.sValue, args[0]);
         attrs.insert(state.symbols.create("success"), &state.vTrue);
     } catch (AssertionError & e) {
         // `value = false;` is unfortunate but removing it is a breaking change.
-        attrs.insert(state.s.value, &state.vFalse);
+        attrs.insert(state.sValue, &state.vFalse);
         attrs.insert(state.symbols.create("success"), &state.vFalse);
     }
 
@@ -1292,8 +1292,7 @@ static void prim_derivationStrict(EvalState & state, const PosIdx pos, Value **
     auto attrs = args[0]->attrs();
 
     /* Figure out the name first (for stack backtraces). */
-    auto nameAttr =
-        state.getAttr(state.s.name, attrs, "in the attrset passed as argument to builtins.derivationStrict");
+    auto nameAttr = state.getAttr(state.sName, attrs, "in the attrset passed as argument to builtins.derivationStrict");
 
     std::string_view drvName;
     try {
@@ -1367,7 +1366,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     using nlohmann::json;
     std::optional<StructuredAttrs> jsonObject;
     auto pos = v.determinePos(noPos);
-    auto attr = attrs->find(state.s.structuredAttrs);
+    auto attr = attrs->find(state.sStructuredAttrs);
     if (attr != attrs->end()
         && state.forceBool(
             *attr->value,
@@ -1378,7 +1377,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
 
     /* Check whether null attributes should be ignored. */
     bool ignoreNulls = false;
-    attr = attrs->find(state.s.ignoreNulls);
+    attr = attrs->find(state.sIgnoreNulls);
     if (attr != attrs->end())
         ignoreNulls = state.forceBool(
             *attr->value,
@@ -1402,7 +1401,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     outputs.insert("out");
 
     for (auto & i : attrs->lexicographicOrder(state.symbols)) {
-        if (i->name == state.s.ignoreNulls)
+        if (i->name == state.sIgnoreNulls)
             continue;
         auto key = state.symbols[i->name];
         vomit("processing attribute '%1%'", key);
@@ -1454,19 +1453,19 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                     continue;
             }
 
-            if (i->name == state.s.contentAddressed && state.forceBool(*i->value, pos, context_below)) {
+            if (i->name == state.sContentAddressed && state.forceBool(*i->value, pos, context_below)) {
                 contentAddressed = true;
                 experimentalFeatureSettings.require(Xp::CaDerivations);
             }
 
-            else if (i->name == state.s.impure && state.forceBool(*i->value, pos, context_below)) {
+            else if (i->name == state.sImpure && state.forceBool(*i->value, pos, context_below)) {
                 isImpure = true;
                 experimentalFeatureSettings.require(Xp::ImpureDerivations);
             }
 
             /* The `args' attribute is special: it supplies the
                command-line arguments to the builder. */
-            else if (i->name == state.s.args) {
+            else if (i->name == state.sArgs) {
                 state.forceList(*i->value, pos, context_below);
                 for (auto elem : i->value->listView()) {
                     auto s = state
@@ -1483,22 +1482,22 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
 
                 if (jsonObject) {
 
-                    if (i->name == state.s.structuredAttrs)
+                    if (i->name == state.sStructuredAttrs)
                         continue;
 
                     jsonObject->structuredAttrs.emplace(key, printValueAsJSON(state, true, *i->value, pos, context));
 
-                    if (i->name == state.s.builder)
+                    if (i->name == state.sBuilder)
                         drv.builder = state.forceString(*i->value, context, pos, context_below);
-                    else if (i->name == state.s.system)
+                    else if (i->name == state.sSystem)
                         drv.platform = state.forceStringNoCtx(*i->value, pos, context_below);
-                    else if (i->name == state.s.outputHash)
+                    else if (i->name == state.sOutputHash)
                         outputHash = state.forceStringNoCtx(*i->value, pos, context_below);
-                    else if (i->name == state.s.outputHashAlgo)
+                    else if (i->name == state.sOutputHashAlgo)
                         outputHashAlgo = parseHashAlgoOpt(state.forceStringNoCtx(*i->value, pos, context_below));
-                    else if (i->name == state.s.outputHashMode)
+                    else if (i->name == state.sOutputHashMode)
                         handleHashMode(state.forceStringNoCtx(*i->value, pos, context_below));
-                    else if (i->name == state.s.outputs) {
+                    else if (i->name == state.sOutputs) {
                         /* Require ‘outputs’ to be a list of strings. */
                         state.forceList(*i->value, pos, context_below);
                         Strings ss;
@@ -1507,51 +1506,51 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
                         handleOutputs(ss);
                     }
 
-                    if (i->name == state.s.allowedReferences)
+                    if (i->name == state.sAllowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedReferences'; use 'outputChecks.<output>.allowedReferences' instead",
                             drvName);
-                    if (i->name == state.s.allowedRequisites)
+                    if (i->name == state.sAllowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'allowedRequisites'; use 'outputChecks.<output>.allowedRequisites' instead",
                             drvName);
-                    if (i->name == state.s.disallowedReferences)
+                    if (i->name == state.sDisallowedReferences)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedReferences'; use 'outputChecks.<output>.disallowedReferences' instead",
                             drvName);
-                    if (i->name == state.s.disallowedRequisites)
+                    if (i->name == state.sDisallowedRequisites)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'disallowedRequisites'; use 'outputChecks.<output>.disallowedRequisites' instead",
                             drvName);
-                    if (i->name == state.s.maxSize)
+                    if (i->name == state.sMaxSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxSize'; use 'outputChecks.<output>.maxSize' instead",
                             drvName);
-                    if (i->name == state.s.maxClosureSize)
+                    if (i->name == state.sMaxClosureSize)
                         warn(
                             "In a derivation named '%s', 'structuredAttrs' disables the effect of the derivation attribute 'maxClosureSize'; use 'outputChecks.<output>.maxClosureSize' instead",
                             drvName);
 
                 } else {
                     auto s = state.coerceToString(pos, *i->value, context, context_below, true).toOwned();
-                    if (i->name == state.s.json) {
+                    if (i->name == state.sJson) {
                         warn(
                             "In derivation '%s': setting structured attributes via '__json' is deprecated, and may be disallowed in future versions of Nix. Set '__structuredAttrs = true' instead.",
                             drvName);
                         drv.structuredAttrs = StructuredAttrs::parse(s);
                     } else {
                         drv.env.emplace(key, s);
-                        if (i->name == state.s.builder)
+                        if (i->name == state.sBuilder)
                             drv.builder = std::move(s);
-                        else if (i->name == state.s.system)
+                        else if (i->name == state.sSystem)
                             drv.platform = std::move(s);
-                        else if (i->name == state.s.outputHash)
+                        else if (i->name == state.sOutputHash)
                             outputHash = std::move(s);
-                        else if (i->name == state.s.outputHashAlgo)
+                        else if (i->name == state.sOutputHashAlgo)
                             outputHashAlgo = parseHashAlgoOpt(s);
-                        else if (i->name == state.s.outputHashMode)
+                        else if (i->name == state.sOutputHashMode)
                             handleHashMode(s);
-                        else if (i->name == state.s.outputs)
+                        else if (i->name == state.sOutputs)
                             handleOutputs(tokenizeString<Strings>(s));
                     }
                 }
@@ -1723,7 +1722,7 @@ static void derivationStrictInternal(EvalState & state, std::string_view drvName
     }
 
     auto result = state.buildBindings(1 + drv.outputs.size());
-    result.alloc(state.s.drvPath)
+    result.alloc(state.sDrvPath)
         .mkString(
             drvPathS,
             {
@@ -2007,14 +2006,14 @@ static void prim_findFile(EvalState & state, const PosIdx pos, Value ** args, Va
         state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.findFile");
 
         std::string prefix;
-        auto i = v2->attrs()->find(state.s.prefix);
+        auto i = v2->attrs()->find(state.sPrefix);
         if (i != v2->attrs()->end())
             prefix = state.forceStringNoCtx(
                 *i->value,
                 pos,
                 "while evaluating the `prefix` attribute of an element of the list passed to builtins.findFile");
 
-        i = state.getAttr(state.s.path, v2->attrs(), "in an element of the __nixPath");
+        i = state.getAttr(state.sPath, v2->attrs(), "in an element of the __nixPath");
 
         NixStringContext context;
         auto path =
@@ -2787,7 +2786,7 @@ static void prim_path(EvalState & state, const PosIdx pos, Value ** args, Value
         if (n == "path")
             path.emplace(state.coerceToPath(
                 attr.pos, *attr.value, context, "while evaluating the 'path' attribute passed to 'builtins.path'"));
-        else if (attr.name == state.s.name)
+        else if (attr.name == state.sName)
             name = state.forceStringNoCtx(
                 *attr.value, attr.pos, "while evaluating the `name` attribute passed to builtins.path");
         else if (n == "filter")
@@ -3106,7 +3105,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args,
     for (const auto & [n, v2] : enumerate(listView)) {
         state.forceAttrs(*v2, pos, "while evaluating an element of the list passed to builtins.listToAttrs");
 
-        auto j = state.getAttr(state.s.name, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.sName, v2->attrs(), "in a {name=...; value=...;} pair");
 
         auto name = state.forceStringNoCtx(
             *j->value,
@@ -3133,7 +3132,7 @@ static void prim_listToAttrs(EvalState & state, const PosIdx pos, Value ** args,
         // Note that .value is actually a Value * *; see earlier comments
         Value * v2 = *std::bit_cast<ElemPtr>(attr.value);
 
-        auto j = state.getAttr(state.s.value, v2->attrs(), "in a {name=...; value=...;} pair");
+        auto j = state.getAttr(state.sValue, v2->attrs(), "in a {name=...; value=...;} pair");
         prev = attr.name;
         bindings.push_back({prev, j->value, j->pos});
     }
@@ -3949,13 +3948,13 @@ static void prim_partition(EvalState & state, const PosIdx pos, Value ** args, V
     auto rlist = state.buildList(rsize);
     if (rsize)
         memcpy(rlist.elems, right.data(), sizeof(Value *) * rsize);
-    attrs.alloc(state.s.right).mkList(rlist);
+    attrs.alloc(state.sRight).mkList(rlist);
 
     auto wsize = wrong.size();
     auto wlist = state.buildList(wsize);
     if (wsize)
         memcpy(wlist.elems, wrong.data(), sizeof(Value *) * wsize);
-    attrs.alloc(state.s.wrong).mkList(wlist);
+    attrs.alloc(state.sWrong).mkList(wlist);
 
     v.mkAttrs(attrs);
 }
@@ -4874,7 +4873,7 @@ static void prim_parseDrvName(EvalState & state, const PosIdx pos, Value ** args
         state.forceStringNoCtx(*args[0], pos, "while evaluating the first argument passed to builtins.parseDrvName");
     DrvName parsed(name);
     auto attrs = state.buildBindings(2);
-    attrs.alloc(state.s.name).mkString(parsed.name);
+    attrs.alloc(state.sName).mkString(parsed.name);
     attrs.alloc("version").mkString(parsed.version);
     v.mkAttrs(attrs);
 }

src/libexpr/primops/context.cc

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

src/libexpr/primops/fetchMercurial.cc

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

src/libexpr/primops/fetchTree.cc

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

src/libexpr/print.cc

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

src/libexpr/value-to-json.cc

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

src/libexpr/value-to-xml.cc

@@ -98,14 +98,14 @@ static void printValueAsXML(
             XMLAttrs xmlAttrs;
 
             Path drvPath;
-            if (auto a = v.attrs()->get(state.s.drvPath)) {
+            if (auto a = v.attrs()->get(state.sDrvPath)) {
                 if (strict)
                     state.forceValue(*a->value, a->pos);
                 if (a->value->type() == nString)
                     xmlAttrs["drvPath"] = drvPath = a->value->c_str();
             }
 
-            if (auto a = v.attrs()->get(state.s.outPath)) {
+            if (auto a = v.attrs()->get(state.sOutPath)) {
                 if (strict)
                     state.forceValue(*a->value, a->pos);
                 if (a->value->type() == nString)

src/libfetchers/git-lfs-fetch.cc

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

src/libfetchers/git.cc

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

src/libfetchers/github.cc

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

src/libfetchers/include/nix/fetchers/input-cache.hh

@@ -11,6 +11,7 @@ struct InputCache
         ref<SourceAccessor> accessor;
         Input resolvedInput;
         Input lockedInput;
+        Attrs extraAttrs;
     };
 
     CachedResult getAccessor(ref<Store> store, const Input & originalInput, UseRegistries useRegistries);
@@ -19,6 +20,7 @@ struct InputCache
     {
         Input lockedInput;
         ref<SourceAccessor> accessor;
+        Attrs extraAttrs;
     };
 
     virtual std::optional<CachedInput> lookup(const Input & originalInput) const = 0;

src/libfetchers/indirect.cc

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

src/libfetchers/input-cache.cc

@@ -22,7 +22,8 @@ InputCache::getAccessor(ref<Store> store, const Input & originalInput, UseRegist
                 fetched = lookup(resolvedInput);
                 if (!fetched) {
                     auto [accessor, lockedInput] = resolvedInput.getAccessor(store);
-                    fetched.emplace(CachedInput{.lockedInput = lockedInput, .accessor = accessor});
+                    fetched.emplace(
+                        CachedInput{.lockedInput = lockedInput, .accessor = accessor, .extraAttrs = extraAttrs});
                 }
                 upsert(resolvedInput, *fetched);
             } else {
@@ -36,7 +37,7 @@ InputCache::getAccessor(ref<Store> store, const Input & originalInput, UseRegist
 
     debug("got tree '%s' from '%s'", fetched->accessor, fetched->lockedInput.to_string());
 
-    return {fetched->accessor, resolvedInput, fetched->lockedInput};
+    return {fetched->accessor, resolvedInput, fetched->lockedInput, fetched->extraAttrs};
 }
 
 struct InputCacheImpl : InputCache

src/libfetchers/mercurial.cc

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

src/libfetchers/path.cc

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

src/libfetchers/tarball.cc

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

src/libflake-tests/flakeref.cc

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

src/libflake/flake.cc

@@ -232,7 +232,7 @@ static Flake readFlake(
         .path = flakePath,
     };
 
-    if (auto description = vInfo.attrs()->get(state.s.description)) {
+    if (auto description = vInfo.attrs()->get(state.sDescription)) {
         expectType(state, nString, *description->value, description->pos);
         flake.description = description->value->c_str();
     }
@@ -253,7 +253,7 @@ static Flake readFlake(
 
         if (outputs->value->isLambda() && outputs->value->lambda().fun->hasFormals()) {
             for (auto & formal : outputs->value->lambda().fun->formals->formals) {
-                if (formal.name != state.s.self)
+                if (formal.name != state.sSelf)
                     flake.inputs.emplace(
                         state.symbols[formal.name],
                         FlakeInput{.ref = parseFlakeRef(state.fetchSettings, std::string(state.symbols[formal.name]))});
@@ -305,8 +305,7 @@ static Flake readFlake(
     }
 
     for (auto & attr : *vInfo.attrs()) {
-        if (attr.name != state.s.description && attr.name != sInputs && attr.name != sOutputs
-            && attr.name != sNixConfig)
+        if (attr.name != state.sDescription && attr.name != sInputs && attr.name != sOutputs && attr.name != sNixConfig)
             throw Error(
                 "flake '%s' has an unsupported attribute '%s', at %s",
                 resolvedRef,
@@ -341,8 +340,9 @@ static Flake getFlake(
     // Fetch a lazy tree first.
     auto cachedInput = state.inputCache->getAccessor(state.store, originalRef.input, useRegistries);
 
-    auto resolvedRef = FlakeRef(std::move(cachedInput.resolvedInput), originalRef.subdir);
-    auto lockedRef = FlakeRef(std::move(cachedInput.lockedInput), originalRef.subdir);
+    auto subdir = fetchers::maybeGetStrAttr(cachedInput.extraAttrs, "dir").value_or(originalRef.subdir);
+    auto resolvedRef = FlakeRef(std::move(cachedInput.resolvedInput), subdir);
+    auto lockedRef = FlakeRef(std::move(cachedInput.lockedInput), subdir);
 
     // Parse/eval flake.nix to get at the input.self attributes.
     auto flake = readFlake(state, originalRef, resolvedRef, lockedRef, {cachedInput.accessor}, lockRootAttrPath);

src/libflake/flakeref.cc

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

src/libflake/url-name.cc

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

src/libstore-tests/data/store-reference/daemon_shorthand.txt

@@ -0,0 +1 @@
+daemon

src/libstore-tests/data/store-reference/local_shorthand_3.txt

@@ -0,0 +1 @@
+local

src/libstore-tests/data/store-reference/ssh_unbracketed_ipv6_1.txt

@@ -0,0 +1 @@
+ssh://::1

src/libstore-tests/data/store-reference/ssh_unbracketed_ipv6_2.txt

@@ -0,0 +1 @@
+ssh://userinfo@fea5:23e1:3916:fc24:cb52:2837:2ecb:ea8e

src/libstore-tests/data/store-reference/ssh_unbracketed_ipv6_3.txt

@@ -0,0 +1 @@
+ssh://userinfo@fea5:23e1:3916:fc24:cb52:2837:2ecb:ea8e?a=b&c=d

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

@@ -28,27 +28,6 @@ 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,
@@ -58,11 +37,3 @@ 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/local-store.cc

@@ -33,4 +33,10 @@ TEST(LocalStore, constructConfig_rootPath)
     EXPECT_EQ(config.rootDir.get(), std::optional{"/foo/bar"});
 }
 
+TEST(LocalStore, constructConfig_to_string)
+{
+    LocalStoreConfig config{"local", "", {}};
+    EXPECT_EQ(config.getReference().render(), "local");
+}
+
 } // namespace nix

src/libstore-tests/s3.cc

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

src/libstore-tests/store-reference.cc

@@ -107,6 +107,13 @@ URI_TEST_READ(local_shorthand_1, localExample_1)
 
 URI_TEST_READ(local_shorthand_2, localExample_2)
 
+URI_TEST(
+    local_shorthand_3,
+    (StoreReference{
+        .variant = StoreReference::Local{},
+        .params = {},
+    }))
+
 static StoreReference unixExample{
     .variant =
         StoreReference::Specified{
@@ -134,4 +141,46 @@ URI_TEST(
         .params = {},
     }))
 
+URI_TEST(
+    daemon_shorthand,
+    (StoreReference{
+        .variant = StoreReference::Daemon{},
+        .params = {},
+    }))
+
+static StoreReference sshLoopbackIPv6{
+    .variant =
+        StoreReference::Specified{
+            .scheme = "ssh",
+            .authority = "[::1]",
+        },
+};
+
+URI_TEST_READ(ssh_unbracketed_ipv6_1, sshLoopbackIPv6)
+
+static StoreReference sshIPv6AuthorityWithUserinfo{
+    .variant =
+        StoreReference::Specified{
+            .scheme = "ssh",
+            .authority = "userinfo@[fea5:23e1:3916:fc24:cb52:2837:2ecb:ea8e]",
+        },
+};
+
+URI_TEST_READ(ssh_unbracketed_ipv6_2, sshIPv6AuthorityWithUserinfo)
+
+static StoreReference sshIPv6AuthorityWithUserinfoAndParams{
+    .variant =
+        StoreReference::Specified{
+            .scheme = "ssh",
+            .authority = "userinfo@[fea5:23e1:3916:fc24:cb52:2837:2ecb:ea8e]",
+        },
+    .params =
+        {
+            {"a", "b"},
+            {"c", "d"},
+        },
+};
+
+URI_TEST_READ(ssh_unbracketed_ipv6_3, sshIPv6AuthorityWithUserinfoAndParams)
+
 } // namespace nix

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

@@ -16,4 +16,10 @@ TEST(UDSRemoteStore, constructConfigWrongScheme)
     EXPECT_THROW(UDSRemoteStoreConfig("http", "/tmp/socket", {}), UsageError);
 }
 
+TEST(UDSRemoteStore, constructConfig_to_string)
+{
+    UDSRemoteStoreConfig config{"unix", "", {}};
+    EXPECT_EQ(config.getReference().render(), "daemon");
+}
+
 } // namespace nix

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

@@ -1,5 +1,4 @@
 #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"
@@ -54,9 +53,24 @@ 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)
-        builder.reset();
+    if (builder) {
+        try {
+            builder->stopDaemon();
+        } catch (...) {
+            ignoreExceptionInDestructor();
+        }
+        try {
+            builder->deleteTmpDir(false);
+        } catch (...) {
+            ignoreExceptionInDestructor();
+        }
+    }
 #endif
     try {
         closeLogFile();
@@ -80,8 +94,22 @@ void DerivationBuildingGoal::killChild()
     hook.reset();
 #endif
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows
-    if (builder && builder->killChild())
+    if (builder && builder->pid != -1) {
         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
 }
 
@@ -90,7 +118,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 _ = doneFailure({BuildResult::TimedOut, std::move(ex)});
+    [[maybe_unused]] Done _ = done(BuildResult::TimedOut, {}, std::move(ex));
 }
 
 /**
@@ -120,9 +148,6 @@ 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()
@@ -230,7 +255,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
                 nrFailed,
                 nrFailed == 1 ? "dependency" : "dependencies");
         msg += showKnownOutputs(worker.store, *drv);
-        co_return doneFailure(BuildError(BuildResult::DependencyFailed, msg));
+        co_return done(BuildResult::DependencyFailed, {}, Error(msg));
     }
 
     /* Gather information necessary for computing the closure and/or
@@ -331,9 +356,9 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
 
             auto resolvedResult = resolvedDrvGoal->buildResult;
 
-            if (resolvedResult.success()) {
-                SingleDrvOutputs builtOutputs;
+            SingleDrvOutputs builtOutputs;
 
+            if (resolvedResult.success()) {
                 auto resolvedHashes = staticOutputHashes(worker.store, drvResolved);
 
                 StorePathSet outputPaths;
@@ -383,19 +408,13 @@ 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 */
@@ -520,7 +539,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 doneSuccess(BuildResult::AlreadyValid, std::move(validOutputs));
+        co_return done(BuildResult::AlreadyValid, std::move(validOutputs));
     }
 
     /* If any of the outputs already exist but are not valid, delete
@@ -624,6 +643,21 @@ 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();
@@ -633,13 +667,19 @@ 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();
-            DesugaredEnv desugaredEnv;
+            decltype(DerivationBuilderParams::finalEnv) finalEnv;
+            decltype(DerivationBuilderParams::extraFiles) extraFiles;
 
             /* Add the closure of store paths to the chroot. */
             StorePathSet closure;
@@ -658,11 +698,58 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             }
 
             try {
-                desugaredEnv = DesugaredEnv::create(worker.store, *drv, *drvOptions, inputPaths);
+                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));
+                    }
+                }
             } catch (BuildError & e) {
                 outputLocks.unlock();
                 worker.permanentFailure = true;
-                co_return doneFailure(std::move(e));
+                co_return done(BuildResult::InputRejected, {}, std::move(e));
             }
 
             /* If we have to wait and retry (see below), then `builder` will
@@ -671,16 +758,16 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                 *localStoreP,
                 std::make_unique<DerivationBuildingGoalCallbacks>(*this, builder),
                 DerivationBuilderParams{
-                    .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),
+                    drvPath,
+                    buildMode,
+                    buildResult,
+                    *drv,
+                    *drvOptions,
+                    inputPaths,
+                    initialOutputs,
+                    std::move(defaultPathsInChroot),
+                    std::move(finalEnv),
+                    std::move(extraFiles),
                 });
         }
 
@@ -709,7 +796,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
         builder.reset();
         outputLocks.unlock();
         worker.permanentFailure = true;
-        co_return doneFailure(std::move(e)); // InputRejected
+        co_return done(BuildResult::InputRejected, {}, std::move(e));
     }
 
     started();
@@ -717,60 +804,26 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
 
     trace("build done");
 
-    SingleDrvOutputs builtOutputs;
-    try {
-        builtOutputs = builder->unprepareBuild();
-    } catch (BuilderFailureError & e) {
-        builder.reset();
+    auto res = builder->unprepareBuild();
+    // N.B. cannot use `std::visit` with co-routine return
+    if (auto * ste = std::get_if<0>(&res)) {
         outputLocks.unlock();
-        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);
-
+        co_return done(std::move(ste->first), {}, std::move(ste->second));
+    } else if (auto * builtOutputs = std::get_if<1>(&res)) {
         /* 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 doneSuccess(BuildResult::Built, std::move(builtOutputs));
+        co_return done(BuildResult::Built, std::move(*builtOutputs));
+    } else {
+        unreachable();
     }
 #endif
 }
 
-static void runPostBuildHook(
+void runPostBuildHook(
     const StoreDirConfig & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths)
 {
     auto hook = settings.postBuildHook;
@@ -836,16 +889,8 @@ static void runPostBuildHook(
     });
 }
 
-BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailureError e)
+void DerivationBuildingGoal::appendLogTailErrorMsg(std::string & msg)
 {
-    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) {
@@ -862,10 +907,6 @@ BuildError DerivationBuildingGoal::fixupBuilderFailureErrorMessage(BuilderFailur
                 nixLogCommand,
                 worker.store.printStorePath(drvPath));
     }
-
-    msg += e.extraMsgAfter;
-
-    return BuildError{e.status, msg};
 }
 
 Goal::Co DerivationBuildingGoal::hookDone()
@@ -906,13 +947,21 @@ Goal::Co DerivationBuildingGoal::hookDone()
 
     /* Check the exit status. */
     if (!statusOk(status)) {
-        auto e = fixupBuilderFailureErrorMessage({BuildResult::MiscFailure, 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);
 
         outputLocks.unlock();
 
         /* TODO (once again) support fine-grained error codes, see issue #12641. */
 
-        co_return doneFailure(std::move(e));
+        co_return done(BuildResult::MiscFailure, {}, BuildError(msg));
     }
 
     /* Compute the FS closure of the outputs and register them as
@@ -939,7 +988,7 @@ Goal::Co DerivationBuildingGoal::hookDone()
     outputLocks.setDeletion(true);
     outputLocks.unlock();
 
-    co_return doneSuccess(BuildResult::Built, std::move(builtOutputs));
+    co_return done(BuildResult::Built, std::move(builtOutputs));
 }
 
 HookReply DerivationBuildingGoal::tryBuildHook()
@@ -1121,11 +1170,10 @@ 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 _ = doneFailure(BuildError(
+            [[maybe_unused]] Done _ = done(
                 BuildResult::LogLimitExceeded,
-                "%s killed after writing more than %d bytes of log output",
-                getName(),
-                settings.maxLogSize));
+                {},
+                Error("%s killed after writing more than %d bytes of log output", getName(), settings.maxLogSize));
             return;
         }
 
@@ -1286,29 +1334,13 @@ SingleDrvOutputs DerivationBuildingGoal::assertPathValidity()
     return validOutputs;
 }
 
-Goal::Done DerivationBuildingGoal::doneSuccess(BuildResult::Status status, SingleDrvOutputs builtOutputs)
+Goal::Done
+DerivationBuildingGoal::done(BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional<Error> ex)
 {
     outputLocks.unlock();
     buildResult.status = status;
-
-    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 (ex)
+        buildResult.errorMsg = fmt("%s", Uncolored(ex->info().msg));
     if (buildResult.status == BuildResult::TimedOut)
         worker.timedOut = true;
     if (buildResult.status == BuildResult::PermanentFailure)
@@ -1316,12 +1348,25 @@ Goal::Done DerivationBuildingGoal::doneFailure(BuildError ex)
 
     mcRunningBuilds.reset();
 
-    if (ex.status != BuildResult::DependencyFailed)
-        worker.failedBuilds++;
+    if (buildResult.success()) {
+        buildResult.builtOutputs = std::move(builtOutputs);
+        if (status == BuildResult::Built)
+            worker.doneBuilds++;
+    } else {
+        if (status != BuildResult::DependencyFailed)
+            worker.failedBuilds++;
+    }
 
     worker.updateProgress();
 
-    return amDone(ecFailed, {std::move(ex)});
+    auto traceBuiltOutputsFile = getEnv("_NIX_TRACE_BUILT_OUTPUTS").value_or("");
+    if (traceBuiltOutputsFile != "") {
+        std::fstream fs;
+        fs.open(traceBuiltOutputsFile, std::fstream::out);
+        fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl;
+    }
+
+    return amDone(buildResult.success() ? ecSuccess : ecFailed, std::move(ex));
 }
 
 } // namespace nix

src/libstore/build/derivation-check.cc

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

src/libstore/build/derivation-check.hh

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

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

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

src/libstore/build/derivation-goal.cc

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

src/libstore/builtins/fetchurl.cc

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

src/libstore/derivation-options.cc

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

src/libstore/derivations.cc

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

src/libstore/filetransfer.cc

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

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

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

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

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

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

@@ -8,33 +8,9 @@
 #include "nix/store/parsed-derivations.hh"
 #include "nix/util/processes.hh"
 #include "nix/store/restricted-store.hh"
-#include "nix/store/build/derivation-env-desugar.hh"
 
 namespace nix {
 
-/**
- * Denotes a build failure that stemmed from the builder exiting with a
- * failing exist status.
- */
-struct BuilderFailureError : BuildError
-{
-    int builderStatus;
-
-    std::string extraMsgAfter;
-
-    BuilderFailureError(BuildResult::Status status, int builderStatus, std::string extraMsgAfter)
-        : BuildError{
-            status,
-              /* No message for now, because the caller will make for
-                 us, with extra context */
-              "",
-          }
-        , builderStatus{std::move(builderStatus)}
-        , extraMsgAfter{std::move(extraMsgAfter)}
-    {
-    }
-};
-
 /**
  * Stuff we need to pass to initChild().
  */
@@ -89,15 +65,60 @@ struct DerivationBuilderParams
      */
     PathsInChroot defaultPathsInChroot;
 
-    /**
-     * May be used to control various platform-specific functionality.
-     *
-     * For example, on Linux, the `kvm` system feature controls whether
-     * `/dev/kvm` should be exposed to the builder within the sandbox.
-     */
-    StringSet systemFeatures;
+    struct EnvEntry
+    {
+        /**
+         * Actually, this should be passed as a file, but with a custom
+         * name (rather than hash-derived name for usual "pass as file").
+         */
+        std::optional<std::string> nameOfPassAsFile;
 
-    DesugaredEnv desugaredEnv;
+        /**
+         * String value of env var, or contents of the file
+         */
+        std::string value;
+    };
+
+    /**
+     * The final environment variables to additionally set, possibly
+     * indirectly via a file.
+     *
+     * This is used by the caller to desugar the "structured attrs"
+     * mechanism, so `DerivationBuilder` doesn't need to know about it.
+     */
+    std::map<std::string, EnvEntry, std::less<>> finalEnv;
+
+    /**
+     * Inserted in the temp dir, but no file names placed in env, unlike
+     * `EnvEntry::nameOfPassAsFile` above.
+     */
+    StringMap extraFiles;
+
+    DerivationBuilderParams(
+        const StorePath & drvPath,
+        const BuildMode & buildMode,
+        BuildResult & buildResult,
+        const Derivation & drv,
+        const DerivationOptions & drvOptions,
+        const StorePathSet & inputPaths,
+        std::map<std::string, InitialOutput> & initialOutputs,
+        PathsInChroot defaultPathsInChroot,
+        std::map<std::string, EnvEntry, std::less<>> finalEnv,
+        StringMap extraFiles)
+        : drvPath{drvPath}
+        , buildResult{buildResult}
+        , drv{drv}
+        , drvOptions{drvOptions}
+        , inputPaths{inputPaths}
+        , initialOutputs{initialOutputs}
+        , buildMode{buildMode}
+        , defaultPathsInChroot{std::move(defaultPathsInChroot)}
+        , finalEnv{std::move(finalEnv)}
+        , extraFiles{std::move(extraFiles)}
+    {
+    }
+
+    DerivationBuilderParams(DerivationBuilderParams &&) = default;
 };
 
 /**
@@ -117,6 +138,8 @@ struct DerivationBuilderCallbacks
      */
     virtual void closeLogFile() = 0;
 
+    virtual void appendLogTailErrorMsg(std::string & msg) = 0;
+
     /**
      * Hook up `builderOut` to some mechanism to ingest the log
      *
@@ -128,6 +151,11 @@ 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;
 };
 
 /**
@@ -143,6 +171,11 @@ struct DerivationBuilderCallbacks
  */
 struct DerivationBuilder : RestrictionContext
 {
+    /**
+     * The process ID of the builder.
+     */
+    Pid pid;
+
     DerivationBuilder() = default;
     virtual ~DerivationBuilder() = default;
 
@@ -175,18 +208,25 @@ struct DerivationBuilder : RestrictionContext
      * processing. A status code and exception are returned, providing
      * more information. The second case indicates success, and
      * realisations for each output of the derivation are returned.
-     *
-     * @throws BuildError
      */
-    virtual SingleDrvOutputs unprepareBuild() = 0;
+    virtual std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() = 0;
 
     /**
-     * Forcibly kill the child process, if any.
-     *
-     * @returns whether the child was still alive and needed to be
-     * killed.
+     * Stop the in-process nix daemon thread.
+     * @see startDaemon
      */
-    virtual bool killChild() = 0;
+    virtual void stopDaemon() = 0;
+
+    /**
+     * Delete the temporary directory, if we have one.
+     */
+    virtual void deleteTmpDir(bool force) = 0;
+
+    /**
+     * Kill any processes running under the build user UID or in the
+     * cgroup of the build.
+     */
+    virtual void killSandbox(bool getStats) = 0;
 };
 
 #ifndef _WIN32 // TODO enable `DerivationBuilder` on Windows

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

@@ -24,6 +24,11 @@
 
 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

@@ -64,7 +64,29 @@ struct StoreReference
         auto operator<=>(const Specified & rhs) const = default;
     };
 
-    typedef std::variant<Auto, Specified> Variant;
+    /**
+     * Special case for `daemon` to avoid normalization.
+     */
+    struct Daemon : Specified
+    {
+        Daemon()
+            : Specified({.scheme = "unix"})
+        {
+        }
+    };
+
+    /**
+     * Special case for `local` to avoid normalization.
+     */
+    struct Local : Specified
+    {
+        Local()
+            : Specified({.scheme = "local"})
+        {
+        }
+    };
+
+    typedef std::variant<Auto, Specified, Daemon, Local> Variant;
 
     Variant variant;
 
@@ -77,22 +99,12 @@ 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

@@ -456,12 +456,17 @@ LocalStore::~LocalStore()
 
 StoreReference LocalStoreConfig::getReference() const
 {
+    auto params = getQueryParams();
+    /* Back-compatibility kludge. Tools like nix-output-monitor expect 'local'
+       and can't parse 'local://'. */
+    if (params.empty())
+        return {.variant = StoreReference::Local{}};
     return {
         .variant =
             StoreReference::Specified{
                 .scheme = *uriSchemes().begin(),
             },
-        .params = getQueryParams(),
+        .params = std::move(params),
     };
 }
 
@@ -1002,10 +1007,7 @@ void LocalStore::registerValidPaths(const ValidPathInfos & infos)
             }},
             {[&](const StorePath & path, const StorePath & parent) {
                 return BuildError(
-                    BuildResult::OutputRejected,
-                    "cycle detected in the references of '%s' from '%s'",
-                    printStorePath(path),
-                    printStorePath(parent));
+                    "cycle detected in the references of '%s' from '%s'", printStorePath(path), printStorePath(parent));
             }});
 
         txn.commit();

src/libstore/meson.build

@@ -101,7 +101,12 @@ subdir('nix-meson-build-support/libatomic')
 
 boost = dependency(
   'boost',
-  modules : [ 'container', 'regex' ],
+  modules : [
+    'container',
+    # Shouldn't list, because can header-only, and Meson currently looks for libs
+    #'regex',
+    'url',
+  ],
   include_type : 'system',
 )
 # boost is a public dependency, but not a pkg-config dependency unfortunately, so we
@@ -265,8 +270,6 @@ 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,10 +322,7 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths)
         }},
         {[&](const StorePath & path, const StorePath & parent) {
             return BuildError(
-                BuildResult::OutputRejected,
-                "cycle detected in the references of '%s' from '%s'",
-                printStorePath(path),
-                printStorePath(parent));
+                "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(BuildResult::OutputRejected, "invalid ownership on file '%1%'", path);
+            throw BuildError("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,11 +1,6 @@
 #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 {
 
@@ -13,8 +8,10 @@ using namespace std::string_view_literals;
 
 #if NIX_WITH_S3_SUPPORT
 
-ParsedS3URL ParsedS3URL::parse(const ParsedURL & parsed)
+ParsedS3URL ParsedS3URL::parse(std::string_view uri)
 try {
+    auto parsed = parseURL(uri);
+
     if (parsed.scheme != "s3"sv)
         throw BadURL("URI scheme '%s' is not 's3'", parsed.scheme);
 
@@ -27,6 +24,10 @@ 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
      */
@@ -40,14 +41,10 @@ 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 = parsed.authority->host,
-        .key = std::move(path),
+        .bucket = std::move(parsed.authority->host),
+        .key = std::string{key},
         .profile = getOptionalParam("profile"),
         .region = getOptionalParam("region"),
         .scheme = getOptionalParam("scheme"),
@@ -65,57 +62,10 @@ try {
         }(),
     };
 } catch (BadURL & e) {
-    e.addTrace({}, "while parsing S3 URI: '%s'", parsed.to_string());
+    e.addTrace({}, "while parsing S3 URI: '%s'", uri);
     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/store-api.cc

@@ -770,7 +770,6 @@ 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));
 
@@ -818,7 +817,13 @@ makeCopyPathMessage(const StoreConfig & srcCfg, const StoreConfig & dstCfg, std:
 
     auto isShorthand = [](const StoreReference & ref) {
         /* At this point StoreReference **must** be resolved. */
-        const auto & specified = std::get<StoreReference::Specified>(ref.variant);
+        const auto & specified = std::visit(
+            overloaded{
+                [](const StoreReference::Auto &) -> const StoreReference::Specified & { unreachable(); },
+                [](const StoreReference::Specified & specified) -> const StoreReference::Specified & {
+                    return specified;
+                }},
+            ref.variant);
         const auto & scheme = specified.scheme;
         return (scheme == "local" || scheme == "unix") && specified.authority.empty();
     };

src/libstore/store-reference.cc

@@ -1,11 +1,12 @@
-#include <regex>
-
 #include "nix/util/error.hh"
+#include "nix/util/split.hh"
 #include "nix/util/url.hh"
 #include "nix/store/store-reference.hh"
 #include "nix/util/file-system.hh"
 #include "nix/util/util.hh"
 
+#include <boost/url/ipv6_address.hpp>
+
 namespace nix {
 
 static bool isNonUriPath(const std::string & spec)
@@ -25,6 +26,8 @@ std::string StoreReference::render(bool withParams) const
     std::visit(
         overloaded{
             [&](const StoreReference::Auto &) { res = "auto"; },
+            [&](const StoreReference::Daemon &) { res = "daemon"; },
+            [&](const StoreReference::Local &) { res = "local"; },
             [&](const StoreReference::Specified & g) {
                 res = g.scheme;
                 res += "://";
@@ -41,6 +44,29 @@ std::string StoreReference::render(bool withParams) const
     return res;
 }
 
+namespace {
+
+struct SchemeAndAuthorityWithPath
+{
+    std::string_view scheme;
+    std::string_view authority;
+};
+
+} // namespace
+
+/**
+ * Return the 'scheme' and remove the '://' or ':' separator.
+ */
+static std::optional<SchemeAndAuthorityWithPath> splitSchemePrefixTo(std::string_view string)
+{
+    auto scheme = splitPrefixTo(string, ':');
+    if (!scheme)
+        return std::nullopt;
+
+    splitPrefix(string, "//");
+    return SchemeAndAuthorityWithPath{.scheme = *scheme, .authority = string};
+}
+
 StoreReference StoreReference::parse(const std::string & uri, const StoreReference::Params & extraParams)
 {
     auto params = extraParams;
@@ -48,11 +74,13 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen
         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 = parsedUri.renderAuthorityAndPath(),
+                    .authority = std::move(baseURI),
                 },
             .params = std::move(params),
         };
@@ -66,21 +94,17 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen
                 .params = std::move(params),
             };
         } else if (baseURI == "daemon") {
+            if (params.empty())
+                return {.variant = Daemon{}};
             return {
-                .variant =
-                    Specified{
-                        .scheme = "unix",
-                        .authority = "",
-                    },
+                .variant = Specified{.scheme = "unix", .authority = ""},
                 .params = std::move(params),
             };
         } else if (baseURI == "local") {
+            if (params.empty())
+                return {.variant = Local{}};
             return {
-                .variant =
-                    Specified{
-                        .scheme = "local",
-                        .authority = "",
-                    },
+                .variant = Specified{.scheme = "local", .authority = ""},
                 .params = std::move(params),
             };
         } else if (isNonUriPath(baseURI)) {
@@ -92,6 +116,32 @@ StoreReference StoreReference::parse(const std::string & uri, const StoreReferen
                     },
                 .params = std::move(params),
             };
+        } else if (auto schemeAndAuthority = splitSchemePrefixTo(baseURI)) {
+            /* Back-compatibility shim to accept unbracketed IPv6 addresses after the scheme.
+             * Old versions of nix allowed that. Note that this is ambiguous and does not allow
+             * specifying the port number. For that the address must be bracketed, otherwise it's
+             * greedily assumed to be the part of the host address. */
+            auto authorityString = schemeAndAuthority->authority;
+            auto userinfo = splitPrefixTo(authorityString, '@');
+            auto maybeIpv6 = boost::urls::parse_ipv6_address(authorityString);
+            if (maybeIpv6) {
+                std::string fixedAuthority;
+                if (userinfo) {
+                    fixedAuthority += *userinfo;
+                    fixedAuthority += '@';
+                }
+                fixedAuthority += '[';
+                fixedAuthority += authorityString;
+                fixedAuthority += ']';
+                return {
+                    .variant =
+                        Specified{
+                            .scheme = std::string(schemeAndAuthority->scheme),
+                            .authority = fixedAuthority,
+                        },
+                    .params = std::move(params),
+                };
+            }
         }
     }
 

src/libstore/uds-remote-store.cc

@@ -57,15 +57,16 @@ UDSRemoteStore::UDSRemoteStore(ref<const Config> config)
 
 StoreReference UDSRemoteStoreConfig::getReference() const
 {
+    /* We specifically return "daemon" here instead of "unix://" or "unix://${path}"
+     * to be more compatible with older versions of nix. Some tooling out there
+     * tries hard to parse store references and it might not be able to handle "unix://". */
+    if (path == settings.nixDaemonSocketFile)
+        return {.variant = StoreReference::Daemon{}};
     return {
         .variant =
             StoreReference::Specified{
                 .scheme = *uriSchemes().begin(),
-                // We return the empty string when the path looks like the
-                // default path, but we could also just return the path
-                // verbatim always, to be robust to overall config changes
-                // at the cost of some verbosity.
-                .authority = path == settings.nixDaemonSocketFile ? "" : path,
+                .authority = path,
             },
     };
 }

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

@@ -22,6 +22,13 @@ 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;
@@ -159,13 +166,13 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
         return !needsHashRewrite() ? chrootRootDir + p : store.toRealPath(p);
     }
 
-    void cleanupBuild(bool force) override
+    void cleanupBuild() override
     {
-        DerivationBuilderImpl::cleanupBuild(force);
+        DerivationBuilderImpl::cleanupBuild();
 
         /* Move paths out of the chroot for easier debugging of
            build failures. */
-        if (!force && buildMode == bmNormal)
+        if (buildMode == bmNormal)
             for (auto & [_, status] : initialOutputs) {
                 if (!status.known)
                     continue;
@@ -175,8 +182,6 @@ 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,7 +17,6 @@
 #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>
 
@@ -43,17 +42,10 @@
 #include "nix/util/signals.hh"
 
 #include "store-config-private.hh"
-#include "build/derivation-check.hh"
 
 namespace nix {
 
-struct NotDeterministic : BuildError
-{
-    NotDeterministic(auto &&... args)
-        : BuildError(BuildResult::NotDeterministic, args...)
-    {
-    }
-};
+MakeError(NotDeterministic, BuildError);
 
 /**
  * This class represents the state for building locally.
@@ -71,11 +63,6 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder
 {
 protected:
 
-    /**
-     * The process ID of the builder.
-     */
-    Pid pid;
-
     LocalStore & store;
 
     std::unique_ptr<DerivationBuilderCallbacks> miscMethods;
@@ -91,27 +78,6 @@ 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:
 
     /**
@@ -218,7 +184,7 @@ public:
 
     void startBuilder() override;
 
-    SingleDrvOutputs unprepareBuild() override;
+    std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() override;
 
 protected:
 
@@ -312,11 +278,9 @@ private:
      */
     void startDaemon();
 
-    /**
-     * Stop the in-process nix daemon thread.
-     * @see startDaemon
-     */
-    void stopDaemon();
+public:
+
+    void stopDaemon() override;
 
 protected:
 
@@ -371,25 +335,22 @@ private:
      */
     SingleDrvOutputs registerOutputs();
 
-protected:
-
     /**
-     * Delete the temporary directory, if we have one.
-     *
-     * @param force We know the build suceeded, so don't attempt to
-     * preseve anything for debugging.
+     * Check that an output meets the requirements specified by the
+     * 'outputChecks' attribute (or the legacy
+     * '{allowed,disallowed}{References,Requisites}' attributes).
      */
-    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);
+    void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs);
 
 public:
 
-    bool killChild() override;
+    void deleteTmpDir(bool force) override;
+
+    void killSandbox(bool getStats) override;
+
+protected:
+
+    virtual void cleanupBuild();
 
 private:
 
@@ -452,24 +413,6 @@ 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()) {
@@ -483,8 +426,16 @@ bool DerivationBuilderImpl::prepareBuild()
     return true;
 }
 
-SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
+std::variant<std::pair<BuildResult::Status, Error>, 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,
@@ -524,28 +475,63 @@ SingleDrvOutputs DerivationBuilderImpl::unprepareBuild()
             ((double) buildResult.cpuSystem->count()) / 1000000);
     }
 
-    /* Check the exit status. */
-    if (!statusOk(status)) {
+    bool diskFull = false;
 
-        /* Check *before* cleaning up. */
-        bool diskFull = decideWhetherDiskFull();
+    try {
 
-        cleanupBuild(false);
+        /* Check the exit status. */
+        if (!statusOk(status)) {
 
-        throw BuilderFailureError{
-            !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure,
-            status,
-            diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "",
-        };
+            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)};
     }
+}
 
-    /* Compute the FS closure of the outputs and register them as
-       being valid. */
-    auto builtOutputs = registerOutputs();
-
-    cleanupBuild(true);
-
-    return builtOutputs;
+void DerivationBuilderImpl::cleanupBuild()
+{
+    deleteTmpDir(false);
 }
 
 static void chmod_(const Path & path, mode_t mode)
@@ -707,7 +693,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(BuildResult::InputRejected, msg);
+        throw BuildError(msg);
     }
 
     auto buildDir = store.config->getBuildDir();
@@ -720,7 +706,7 @@ void DerivationBuilderImpl::startBuilder()
 
     /* Create a temporary directory where the build will take
        place. */
-    topTmpDir = createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700);
+    topTmpDir = createTempDir(buildDir, "nix", 0700);
     setBuildTmpDir();
     assert(!tmpDir.empty());
 
@@ -1017,13 +1003,19 @@ 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] : desugaredEnv.variables) {
-        env[name] = info.prependBuildDirectory ? tmpDirInSandbox() + "/" + info.value : info.value;
+    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;
+        }
     }
 
     /* Add extra files, similar to `finalEnv` */
-    for (const auto & [fileName, value] : desugaredEnv.extraFiles) {
-        writeBuilderFile(fileName, rewriteStrings(value, inputRewrites));
+    for (const auto & [fileName, value] : extraFiles) {
+        writeBuilderFile(fileName, value);
     }
 
     /* For convenience, set an environment pointing to the top build
@@ -1342,6 +1334,8 @@ 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. */
@@ -1395,7 +1389,6 @@ 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,
@@ -1410,7 +1403,6 @@ 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);
@@ -1447,11 +1439,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         {[&](const std::string & name) {
             auto orifu = get(outputReferencesIfUnregistered, name);
             if (!orifu)
-                throw BuildError(
-                    BuildResult::OutputRejected,
-                    "no output reference for '%s' in build of '%s'",
-                    name,
-                    store.printStorePath(drvPath));
+                throw BuildError("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
@@ -1473,7 +1461,6 @@ 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,
@@ -1567,12 +1554,11 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo {
             auto st = get(outputStats, outputName);
             if (!st)
-                throw BuildError(BuildResult::OutputRejected, "output path %1% without valid stats info", actualPath);
+                throw BuildError("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);
@@ -1660,11 +1646,35 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
 
                     std::filesystem::rename(tmpOutput, actualPath);
 
-                    return newInfoFromCA(
+                    auto newInfo0 = 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); },
@@ -1728,90 +1738,85 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         }
 
         if (buildMode == bmCheck) {
-            /* Check against already registered outputs */
 
-            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);
+            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);
 
-                        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));
-                }
-
-                /* 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));
+                    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));
             }
 
-            store.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences()
+            /* Since we verified the build, it's now ultimately trusted. */
+            if (!oldInfo.ultimate) {
+                oldInfo.ultimate = true;
+                store.signPathInfo(oldInfo);
+                store.registerValidPaths({{oldInfo.path, oldInfo}});
+            }
 
-            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}});
+            continue;
         }
 
-        /* Do this in both the check and non-check cases, because we
-           want `checkOutputs` below to work, which needs these path
-           infos. */
+        /* 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}});
+
         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. */
@@ -1823,11 +1828,16 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
         store.registerValidPaths(infos2);
     }
 
-    /* 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.
-       */
+    /* 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. */
     SingleDrvOutputs builtOutputs;
 
     for (auto & [outputName, newInfo] : infos) {
@@ -1844,14 +1854,151 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs()
     return builtOutputs;
 }
 
-void DerivationBuilderImpl::cleanupBuild(bool force)
+void DerivationBuilderImpl::checkOutputs(const std::map<std::string, ValidPathInfo> & outputs)
 {
-    if (force) {
-        /* Delete unused redirected outputs (when doing hash rewriting). */
-        for (auto & i : redirectedOutputs)
-            deletePath(store.Store::toRealPath(i.second));
-    }
+    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);
+    }
+}
+
+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 (systemFeatures.count("kvm") && pathExists("/dev/kvm"))
+            if (store.Store::config.systemFeatures.get().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");
     }
 
-    SingleDrvOutputs unprepareBuild() override
+    std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() override
     {
         sandboxMountNamespace = -1;
         sandboxUserNamespace = -1;

src/libutil-tests/strings.cc

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

src/libutil-tests/url.cc

@@ -3,8 +3,6 @@
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
 
-#include <ranges>
-
 namespace nix {
 
 /* ----------- tests for url.hh --------------------------------------------------*/
@@ -12,155 +10,6 @@ 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";
@@ -169,7 +18,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 = "",
     };
@@ -186,7 +35,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 = "",
     };
@@ -203,7 +52,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",
     };
@@ -220,7 +69,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#",
     };
@@ -236,7 +85,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 = "",
     };
@@ -247,10 +96,8 @@ TEST(parseURL, parsesFilePlusHttpsUrl)
 
 TEST(parseURL, rejectsAuthorityInUrlsWithFileTransportation)
 {
-    EXPECT_THAT(
-        []() { parseURL("file://www.example.org/video.mp4"); },
-        ::testing::ThrowsMessage<BadURL>(
-            testing::HasSubstrIgnoreANSIMatcher("has unexpected authority 'www.example.org'")));
+    auto s = "file://www.example.org/video.mp4";
+    ASSERT_THROW(parseURL(s), Error);
 }
 
 TEST(parseURL, parseIPv4Address)
@@ -261,7 +108,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",
     };
@@ -278,7 +125,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 = "",
     };
@@ -300,7 +147,7 @@ TEST(parseURL, parseIPv6Address)
                 .host = "2a02:8071:8192:c100:311d:192d:81ac:11ea",
                 .port = 8080,
             },
-        .path = {""},
+        .path = "",
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -331,7 +178,7 @@ TEST(parseURL, parseUserPassword)
                 .password = "pass",
                 .port = 8080,
             },
-        .path = {"", "file.tar.gz"},
+        .path = "/file.tar.gz",
         .query = (StringMap) {},
         .fragment = "",
     };
@@ -348,77 +195,11 @@ 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());
 }
@@ -464,7 +245,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 = "",
     };
@@ -500,7 +281,7 @@ TEST(parseURL, parsesHttpUrlWithEmptyPort)
     ParsedURL expected{
         .scheme = "http",
         .authority = Authority{.hostType = HostType::Name, .host = "www.example.org"},
-        .path = {"", "file.tar.gz"},
+        .path = "/file.tar.gz",
         .query = (StringMap) {{"foo", "bar"}},
         .fragment = "",
     };
@@ -509,208 +290,6 @@ TEST(parseURL, parsesHttpUrlWithEmptyPort)
     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
  * --------------------------------------------------------------------------*/
@@ -820,121 +399,7 @@ 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,7 +6,6 @@
 #include "nix/util/terminal.hh"
 #include "nix/util/position.hh"
 
-#include <cinttypes>
 #include <iostream>
 #include <optional>
 #include "nix/util/serialise.hh"
@@ -437,19 +436,13 @@ void panic(std::string_view msg)
     writeErr("\n\n" ANSI_RED "terminating due to unexpected unrecoverable internal error: " ANSI_NORMAL);
     writeErr(msg);
     writeErr("\n");
-    std::terminate();
+    abort();
 }
 
-void unreachable(std::source_location loc)
+void panic(const char * file, int line, const char * func)
 {
     char buf[512];
-    int n = snprintf(
-        buf,
-        sizeof(buf),
-        "Unexpected condition in %s at %s:%" PRIuLEAST32,
-        loc.function_name(),
-        loc.file_name(),
-        loc.line());
+    int n = snprintf(buf, sizeof(buf), "Unexpected condition in %s at %s:%d", func, file, 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,7 +22,6 @@
 #include <list>
 #include <memory>
 #include <optional>
-#include <utility>
 
 #include <sys/types.h>
 #include <sys/stat.h>
@@ -300,16 +299,23 @@ using NativeSysError =
 void throwExceptionSelfCheck();
 
 /**
- * Print a message and std::terminate().
+ * Print a message and abort().
  */
 [[noreturn]]
 void panic(std::string_view msg);
 
 /**
- * Print a basic error message with source position and std::terminate().
+ * 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().
  *
  * @note: This assumes that the logger is operational
  */
-[[gnu::noinline, gnu::cold, noreturn]] void unreachable(std::source_location loc = std::source_location::current());
+#define unreachable() (::nix::panic(__FILE__, __LINE__, __func__))
 
 } // namespace nix

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

@@ -26,29 +26,18 @@ C tokenizeString(std::string_view s, std::string_view separators)
 }
 
 template<class C, class CharT>
-void basicSplitStringInto(C & accum, std::basic_string_view<CharT> s, std::basic_string_view<CharT> separators)
+C basicSplitString(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();
-        accum.insert(accum.end(), typename C::value_type{s.substr(pos, end - pos)});
+        result.insert(result.end(), std::basic_string<CharT>(s, 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,11 +1,7 @@
 #pragma once
 ///@file
 
-#include <ranges>
-#include <span>
-
 #include "nix/util/error.hh"
-#include "nix/util/canon-path.hh"
 
 namespace nix {
 
@@ -69,7 +65,6 @@ struct ParsedURL
     };
 
     std::string scheme;
-
     /**
      * Optional parsed authority component of the URL.
      *
@@ -80,171 +75,18 @@ struct ParsedURL
      * part of the URL.
      */
     std::optional<Authority> authority;
-
-    /**
-     * @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;
-
+    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 segments.
+     * Remove `.` and `..` path elements.
      */
     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);
@@ -254,22 +96,6 @@ MakeError(BadURL, Error);
 std::string percentDecode(std::string_view in);
 std::string percentEncode(std::string_view s, std::string_view keep = "");
 
-/**
- * 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
  */
@@ -288,29 +114,9 @@ std::string encodeQuery(const StringMap & query);
  * @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, 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
  * use a scheme of the form 'x+y' in urls, where y is the “transport layer”
@@ -327,23 +133,10 @@ struct ParsedUrlScheme
 
 ParsedUrlScheme parseUrlScheme(std::string_view scheme);
 
-/**
- * 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);
+/* 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);
 
 /**
  * Whether a string is valid as RFC 3986 scheme name.
@@ -354,63 +147,4 @@ ParsedURL fixGitURL(std::string_view 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/meson.build

@@ -57,7 +57,12 @@ deps_private += blake3
 
 boost = dependency(
   'boost',
-  modules : [ 'context', 'coroutine', 'iostreams', 'url' ],
+  modules : [
+    'context',
+    'coroutine',
+    'iostreams',
+    'url',
+  ],
   include_type : 'system',
   version : '>=1.82.0',
 )

src/libutil/unix/include/nix/util/monitor-fd.hh

@@ -2,15 +2,18 @@
 ///@file
 
 #include <thread>
-#include <atomic>
+#include <cassert>
 
-#include <cstdlib>
 #include <poll.h>
-#include <sys/types.h>
-#include <unistd.h>
-#include <signal.h>
+#include <errno.h>
+
+#ifdef __APPLE__
+#  include <sys/types.h>
+#  include <sys/event.h>
+#endif
 
 #include "nix/util/signals.hh"
+#include "nix/util/file-descriptor.hh"
 
 namespace nix {
 
@@ -20,111 +23,113 @@ private:
     std::thread thread;
     Pipe notifyPipe;
 
+    void runThread(int watchFd, int notifyFd);
+
 public:
-    MonitorFdHup(int fd)
-    {
-        notifyPipe.create();
-        thread = std::thread([this, fd]() {
-            while (true) {
-                // There is a POSIX violation on macOS: you have to listen for
-                // at least POLLHUP to receive HUP events for a FD. POSIX says
-                // this is not so, and you should just receive them regardless.
-                // However, as of our testing on macOS 14.5, the events do not
-                // get delivered if in the all-bits-unset case, but do get
-                // delivered if `POLLHUP` is set.
-                //
-                // This bug filed as rdar://37537852
-                // (https://openradar.appspot.com/37537852).
-                //
-                // macOS's own man page
-                // (https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/poll.2.html)
-                // additionally says that `POLLHUP` is ignored as an input. It
-                // seems the likely order of events here was
-                //
-                // 1. macOS did not follow the POSIX spec
-                //
-                // 2. Somebody ninja-fixed this other spec violation to make
-                // sure `POLLHUP` was not forgotten about, even though they
-                // "fixed" this issue in a spec-non-compliant way. Whatever,
-                // we'll use the fix.
-                //
-                // Relevant code, current version, which shows the :
-                // https://github.com/apple-oss-distributions/xnu/blob/94d3b452840153a99b38a3a9659680b2a006908e/bsd/kern/sys_generic.c#L1751-L1758
-                //
-                // The `POLLHUP` detection was added in
-                // https://github.com/apple-oss-distributions/xnu/commit/e13b1fa57645afc8a7b2e7d868fe9845c6b08c40#diff-a5aa0b0e7f4d866ca417f60702689fc797e9cdfe33b601b05ccf43086c35d395R1468
-                // That means added in 2007 or earlier. Should be good enough
-                // for us.
-                short hangup_events =
-#ifdef __APPLE__
-                    POLLHUP
-#else
-                    0
-#endif
-                    ;
-
-                /* Wait indefinitely until a POLLHUP occurs. */
-                constexpr size_t num_fds = 2;
-                struct pollfd fds[num_fds] = {
-                    {
-                        .fd = fd,
-                        .events = hangup_events,
-                    },
-                    {
-                        .fd = notifyPipe.readSide.get(),
-                        .events = hangup_events,
-                    },
-                };
-
-                auto count = poll(fds, num_fds, -1);
-                if (count == -1) {
-                    if (errno == EINTR || errno == EAGAIN)
-                        continue;
-                    throw SysError("failed to poll() in MonitorFdHup");
-                }
-                /* This shouldn't happen, but can on macOS due to a bug.
-                   See rdar://37550628.
-
-                   This may eventually need a delay or further
-                   coordination with the main thread if spinning proves
-                   too harmful.
-                */
-                if (count == 0)
-                    continue;
-                if (fds[0].revents & POLLHUP) {
-                    unix::triggerInterrupt();
-                    break;
-                }
-                if (fds[1].revents & POLLHUP) {
-                    break;
-                }
-                // On macOS, (jade thinks that) it is possible (although not
-                // observed on macOS 14.5) that in some limited cases on buggy
-                // kernel versions, all the non-POLLHUP events for the socket
-                // get delivered.
-                //
-                // We could sleep to avoid pointlessly spinning a thread on
-                // those, but this opens up a different problem, which is that
-                // if do sleep, it will be longer before the daemon fork for a
-                // client exits. Imagine a sequential shell script, running Nix
-                // commands, each of which talk to the daemon. If the previous
-                // command registered a temp root, exits, and then the next
-                // command issues a delete request before the temp root is
-                // cleaned up, that delete request might fail.
-                //
-                // Not sleeping doesn't actually fix the race condition --- we
-                // would need to block on the old connections' tempt roots being
-                // cleaned up in in the new connection --- but it does make it
-                // much less likely.
-            }
-        });
-    };
+    MonitorFdHup(int fd);
 
     ~MonitorFdHup()
     {
+        // Close the write side to signal termination via POLLHUP
         notifyPipe.writeSide.close();
         thread.join();
     }
 };
 
+#ifdef __APPLE__
+/* This custom kqueue usage exists because Apple's poll implementation is
+ * broken and loses event subscriptions if EVFILT_READ fires without matching
+ * the requested `events` in the pollfd.
+ *
+ * We use EVFILT_READ, which causes some spurious wakeups (at most one per write
+ * from the client, in addition to the socket lifecycle events), because the
+ * alternate API, EVFILT_SOCK, doesn't work on pipes, which this is also used
+ * to monitor in certain situations.
+ *
+ * See (EVFILT_SOCK):
+ * https://github.com/netty/netty/blob/64bd2f4eb62c2fb906bc443a2aabf894c8b7dce9/transport-classes-kqueue/src/main/java/io/netty/channel/kqueue/AbstractKQueueChannel.java#L434
+ *
+ * See: https://git.lix.systems/lix-project/lix/issues/729
+ * Apple bug in poll(2): FB17447257, available at https://openradar.appspot.com/FB17447257
+ */
+inline void MonitorFdHup::runThread(int watchFd, int notifyFd)
+{
+    int kqResult = kqueue();
+    if (kqResult < 0) {
+        throw SysError("MonitorFdHup kqueue");
+    }
+    AutoCloseFD kq{kqResult};
+
+    std::array<struct kevent, 2> kevs;
+
+    // kj uses EVFILT_WRITE for this, but it seems that it causes more spurious
+    // wakeups in our case of doing blocking IO from another thread compared to
+    // EVFILT_READ.
+    //
+    // EVFILT_WRITE and EVFILT_READ (for sockets at least, where I am familiar
+    // with the internals) both go through a common filter which catches EOFs
+    // and generates spurious wakeups for either readable/writable events.
+    EV_SET(&kevs[0], watchFd, EVFILT_READ, EV_ADD | EV_ENABLE | EV_CLEAR, 0, 0, nullptr);
+    EV_SET(&kevs[1], notifyFd, EVFILT_READ, EV_ADD | EV_ENABLE | EV_CLEAR, 0, 0, nullptr);
+
+    int result = kevent(kq.get(), kevs.data(), kevs.size(), nullptr, 0, nullptr);
+    if (result < 0) {
+        throw SysError("MonitorFdHup kevent add");
+    }
+
+    while (true) {
+        struct kevent event;
+        int numEvents = kevent(kq.get(), nullptr, 0, &event, 1, nullptr);
+        if (numEvents < 0) {
+            throw SysError("MonitorFdHup kevent watch");
+        }
+
+        if (numEvents > 0 && (event.flags & EV_EOF)) {
+            if (event.ident == uintptr_t(watchFd)) {
+                unix::triggerInterrupt();
+            }
+            // Either watched fd or notify fd closed, exit
+            return;
+        }
+    }
+}
+#else
+inline void MonitorFdHup::runThread(int watchFd, int notifyFd)
+{
+    while (true) {
+        struct pollfd fds[2];
+        fds[0].fd = watchFd;
+        fds[0].events = 0; // POSIX: POLLHUP is always reported
+        fds[1].fd = notifyFd;
+        fds[1].events = 0;
+
+        auto count = poll(fds, 2, -1);
+        if (count == -1) {
+            if (errno == EINTR || errno == EAGAIN) {
+                continue;
+            } else {
+                throw SysError("in MonitorFdHup poll()");
+            }
+        }
+
+        if (fds[0].revents & POLLHUP) {
+            unix::triggerInterrupt();
+            break;
+        }
+
+        if (fds[1].revents & POLLHUP) {
+            // Notify pipe closed, exit thread
+            break;
+        }
+    }
+}
+#endif
+
+inline MonitorFdHup::MonitorFdHup(int fd)
+{
+    notifyPipe.create();
+    int notifyFd = notifyPipe.readSide.get();
+    thread = std::thread([this, fd, notifyFd]() { this->runThread(fd, notifyFd); });
+};
+
 } // namespace nix

src/libutil/url.cc

@@ -3,7 +3,6 @@
 #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>
 
@@ -109,8 +108,6 @@ static std::string percentEncodeCharSet(std::string_view s, auto charSet)
     return res;
 }
 
-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):
@@ -152,15 +149,10 @@ try {
         }();
     }
 
-    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());
-}
+    auto urlView = boost::urls::url_view(lenient ? fixedEncodedUrl : url);
 
-static ParsedURL fromBoostUrlView(boost::urls::url_view urlView, bool lenient)
-{
     if (!urlView.has_scheme())
-        throw BadURL("'%s' doesn't have a scheme", urlView.buffer());
+        throw BadURL("'%s' doesn't have a scheme", url);
 
     auto scheme = urlView.scheme();
     auto authority = [&]() -> std::optional<ParsedURL::Authority> {
@@ -178,16 +170,13 @@ static ParsedURL fromBoostUrlView(boost::urls::url_view urlView, bool lenient)
      * 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'", urlView.buffer(), *authority);
+        throw BadURL("file:// URL '%s' has unexpected authority '%s'", url, *authority);
 
+    auto path = urlView.path();         /* Does pct-decoding */
     auto fragment = urlView.fragment(); /* Does pct-decoding */
 
-    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>>();
+    if (transportIsFile && path.empty())
+        path = "/";
 
     /* Get the raw query. Store URI supports smuggling doubly nested queries, where
        the inner &/? are pct-encoded. */
@@ -196,62 +185,12 @@ static ParsedURL fromBoostUrlView(boost::urls::url_view urlView, bool lenient)
     return ParsedURL{
         .scheme = scheme,
         .authority = authority,
-        .path = std::move(path),
+        .path = path,
         .query = decodeQuery(query, lenient),
         .fragment = fragment,
     };
-}
-
-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;
+} catch (boost::system::system_error & e) {
+    throw BadURL("'%s' is not a valid URL: %s", url, e.code().message());
 }
 
 std::string percentDecode(std::string_view in)
@@ -295,15 +234,7 @@ try {
 }
 
 const static std::string allowedInQuery = ":@/?";
-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);
-}
+const static std::string allowedInPath = ":@/";
 
 std::string encodeQuery(const StringMap & ss)
 {
@@ -320,62 +251,10 @@ 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
 {
-    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;
+    return scheme + ":" + (authority ? "//" + authority->to_string() : "") + percentEncode(path, allowedInPath)
+           + (query.empty() ? "" : "?" + encodeQuery(query)) + (fragment.empty() ? "" : "#" + percentEncode(fragment));
 }
 
 std::ostream & operator<<(std::ostream & os, const ParsedURL & url)
@@ -387,7 +266,7 @@ std::ostream & operator<<(std::ostream & os, const ParsedURL & url)
 ParsedURL ParsedURL::canonicalise()
 {
     ParsedURL res(*this);
-    res.path = splitString<std::vector<std::string>>(CanonPath(renderPath()).abs(), "/");
+    res.path = CanonPath(res.path).abs();
     return res;
 }
 
@@ -408,91 +287,17 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme)
     };
 }
 
-struct ScpLike
+std::string fixGitURL(const std::string & url)
 {
-    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));
-        }
+    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();
     }
-
-    // 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),
-    };
+    return url;
 }
 
 // https://www.rfc-editor.org/rfc/rfc3986#section-3.1
@@ -504,10 +309,4 @@ 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.s.outPath)->getString();
-        auto outputName = cursor->getAttr(state.s.outputName)->getString();
-        auto name = cursor->getAttr(state.s.name)->getString();
+        auto outPath = cursor->getAttr(state.sOutPath)->getString();
+        auto outputName = cursor->getAttr(state.sOutputName)->getString();
+        auto name = cursor->getAttr(state.sName)->getString();
         auto aPname = cursor->maybeGetAttr("pname");
-        auto aMeta = cursor->maybeGetAttr(state.s.meta);
+        auto aMeta = cursor->maybeGetAttr(state.sMeta);
         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->s.drvPath);
+        auto attr1 = vRes->attrs()->get(evalState->sDrvPath);
         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->s.outPath);
+        auto attr2 = vRes->attrs()->get(evalState->sOutPath);
         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->s.name);
+            auto * attr = vRes->attrs()->get(evalState->sName);
             if (!attr)
                 throw Error("attribute 'name' missing");
             outLink = evalState->forceStringNoCtx(*attr->value, attr->pos, "");

src/nix/flake.cc

@@ -1232,12 +1232,12 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
                 };
 
                 auto showDerivation = [&]() {
-                    auto name = visitor.getAttr(state->s.name)->getString();
+                    auto name = visitor.getAttr(state->sName)->getString();
 
                     if (json) {
                         std::optional<std::string> description;
-                        if (auto aMeta = visitor.maybeGetAttr(state->s.meta)) {
-                            if (auto aDescription = aMeta->maybeGetAttr(state->s.description))
+                        if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) {
+                            if (auto aDescription = aMeta->maybeGetAttr(state->sDescription))
                                 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->s.meta)) {
-                        if (auto aDescription = aMeta->maybeGetAttr(state->s.description))
+                    if (auto aMeta = visitor.maybeGetAttr(state->sMeta)) {
+                        if (auto aDescription = aMeta->maybeGetAttr(state->sDescription))
                             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.s.type).mkString("derivation");
-        attrs.alloc(state.s.name).mkString(i.queryName());
+        attrs.alloc(state.sType).mkString("derivation");
+        attrs.alloc(state.sName).mkString(i.queryName());
         auto system = i.querySystem();
         if (!system.empty())
-            attrs.alloc(state.s.system).mkString(system);
-        attrs.alloc(state.s.outPath).mkString(state.store->printStorePath(i.queryOutPath()));
+            attrs.alloc(state.sSystem).mkString(system);
+        attrs.alloc(state.sOutPath).mkString(state.store->printStorePath(i.queryOutPath()));
         if (drvPath)
-            attrs.alloc(state.s.drvPath).mkString(state.store->printStorePath(*drvPath));
+            attrs.alloc(state.sDrvPath).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.s.outPath).mkString(state.store->printStorePath(*j.second));
+            outputAttrs.alloc(state.sOutPath).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.s.outputs).mkList(outputsList);
+        attrs.alloc(state.sOutputs).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.s.meta).mkAttrs(meta);
+        attrs.alloc(state.sMeta).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.s.drvPath));
+    auto & aDrvPath(*topLevel.attrs()->find(state.sDrvPath));
     auto topLevelDrv = state.coerceToStorePath(aDrvPath.pos, *aDrvPath.value, context, "");
     topLevelDrv.requireDerivation();
-    auto & aOutPath(*topLevel.attrs()->find(state.s.outPath));
+    auto & aOutPath(*topLevel.attrs()->find(state.sOutPath));
     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(ValidURL{url});
+            FileTransferRequest req(url);
             req.decompress = false;
             getFileTransfer()->download(std::move(req), sink);
         }