Use BackedStringView

(cherry picked from commit 1fe8f54bd3)

.gitignore

@@ -41,14 +41,14 @@ perl/Makefile.config
 /src/libexpr/parser-tab.hh
 /src/libexpr/parser-tab.output
 /src/libexpr/nix.tbl
-/src/libexpr/tests/libnixexpr-tests
+/tests/unit/libexpr/libnixexpr-tests
 
 # /src/libstore/
 *.gen.*
-/src/libstore/tests/libnixstore-tests
+/tests/unit/libstore/libnixstore-tests
 
 # /src/libutil/
-/src/libutil/tests/libnixutil-tests
+/tests/unit/libutil/libnixutil-tests
 
 /src/nix/nix
 

.version

@@ -1 +1 @@
-2.19.0
+2.19.3

Makefile

@@ -25,11 +25,13 @@ makefiles = \
 endif
 
 ifeq ($(ENABLE_BUILD)_$(ENABLE_TESTS), yes_yes)
-UNIT_TEST_ENV = _NIX_TEST_UNIT_DATA=unit-test-data
 makefiles += \
-  src/libutil/tests/local.mk \
-  src/libstore/tests/local.mk \
-  src/libexpr/tests/local.mk
+  tests/unit/libutil/local.mk \
+  tests/unit/libutil-support/local.mk \
+  tests/unit/libstore/local.mk \
+  tests/unit/libstore-support/local.mk \
+  tests/unit/libexpr/local.mk \
+  tests/unit/libexpr-support/local.mk
 endif
 
 ifeq ($(ENABLE_TESTS), yes)

doc/internal-api/doxygen.cfg.in

@@ -39,17 +39,21 @@ INPUT                  = \
   src/libcmd \
   src/libexpr \
   src/libexpr/flake \
-  src/libexpr/tests \
-  src/libexpr/tests/value \
+  tests/unit/libexpr \
+  tests/unit/libexpr/value \
+  tests/unit/libexpr/test \
+  tests/unit/libexpr/test/value \
   src/libexpr/value \
   src/libfetchers \
   src/libmain \
   src/libstore \
   src/libstore/build \
   src/libstore/builtins \
-  src/libstore/tests \
+  tests/unit/libstore \
+  tests/unit/libstore/test \
   src/libutil \
-  src/libutil/tests \
+  tests/unit/libutil \
+  tests/unit/libutil/test \
   src/nix \
   src/nix-env \
   src/nix-store

doc/manual/_redirects

@@ -0,0 +1,30 @@
+# redirect rules for paths (server-side) to prevent link rot.
+# see ./redirects.js for redirects based on URL fragments (client-side)
+#
+# concrete user story this supports:
+# - user finds URL to the manual for Nix x.y
+# - Nix x.z (z > y) is the most recent release
+# - updating the version in the URL will show the right thing
+#
+# format documentation:
+# - https://docs.netlify.com/routing/redirects/#syntax-for-the-redirects-file
+# - https://docs.netlify.com/routing/redirects/redirect-options/
+#
+# conventions:
+# - always force (<CODE>!) since this allows re-using file names
+# - group related paths to ease readability
+# - always append new redirects to the end of the file
+# - redirects that should have been there but are missing can be inserted where they belong
+
+/expressions/expression-language /language/ 301!
+/expressions/language-values /language/values 301!
+/expressions/language-constructs /language/constructs 301!
+/expressions/language-operators /language/operators 301!
+/expressions/* /language/:splat 301!
+
+/package-management/basic-package-mgmt /command-ref/nix-env 301!
+
+/package-management/channels* /command-ref/nix-channel 301!
+
+/package-management/s3-substituter* /command-ref/new-cli/nix3-help-stores#s3-binary-cache-store 301!
+

doc/manual/redirects.js

@@ -1,7 +1,9 @@
-// redirect rules for anchors ensure backwards compatibility of URLs.
-// this must be done on the client side, as web servers do not see the anchor part of the URL.
+// redirect rules for URL fragments (client-side) to prevent link rot.
+// this must be done on the client side, as web servers do not see the fragment part of the URL.
+// it will only work with JavaScript enabled in the browser, but this is the best we can do here.
+// see ./_redirects for path redirects (client-side)
 
-// redirections are declared as follows:
+// redirects are declared as follows:
 // each entry has as its key a path matching the requested URL path, relative to the mdBook document root.
 //
 //     IMPORTANT: it must specify the full path with file name and suffix
@@ -19,6 +21,7 @@ const redirects = {
     "chap-distributed-builds": "advanced-topics/distributed-builds.html",
     "chap-post-build-hook": "advanced-topics/post-build-hook.html",
     "chap-post-build-hook-caveats": "advanced-topics/post-build-hook.html#implementation-caveats",
+    "chap-writing-nix-expressions": "language/index.html",
     "part-command-ref": "command-ref/command-ref.html",
     "conf-allow-import-from-derivation": "command-ref/conf-file.html#conf-allow-import-from-derivation",
     "conf-allow-new-privileges": "command-ref/conf-file.html#conf-allow-new-privileges",

doc/manual/rl-next/allowed-uris-can-now-match-whole-schemes.md

@@ -0,0 +1,7 @@
+---
+synopsis: Option `allowed-uris` can now match whole schemes in URIs without slashes
+prs: 9547
+---
+
+If a scheme, such as `github:` is specified in the `allowed-uris` option, all URIs starting with `github:` are allowed.
+Previously this only worked for schemes whose URIs used the `://` syntax.

doc/manual/src/SUMMARY.md.in

@@ -114,7 +114,6 @@
   - [CLI guideline](contributing/cli-guideline.md)
   - [C++ style guide](contributing/cxx.md)
 - [Release Notes](release-notes/release-notes.md)
-  - [Release X.Y (202?-??-??)](release-notes/rl-next.md)
   - [Release 2.19 (2023-11-17)](release-notes/rl-2.19.md)
   - [Release 2.18 (2023-09-20)](release-notes/rl-2.18.md)
   - [Release 2.17 (2023-07-24)](release-notes/rl-2.17.md)

doc/manual/src/contributing/testing.md

@@ -20,6 +20,7 @@ The unit tests are defined using the [googletest] and [rapidcheck] frameworks.
 
 [googletest]: https://google.github.io/googletest/
 [rapidcheck]: https://github.com/emil-e/rapidcheck
+[property testing]: https://en.wikipedia.org/wiki/Property_testing
 
 ### Source and header layout
 
@@ -28,34 +29,50 @@ The unit tests are defined using the [googletest] and [rapidcheck] frameworks.
 > ```
 > src
 > ├── libexpr
+> │   ├── local.mk
 > │   ├── value/context.hh
 > │   ├── value/context.cc
+> │   …
+> │
+> ├── tests
 > │   │
 > │   …
->     └── tests
-> │       ├── value/context.hh
-> │       ├── value/context.cc
+> │   └── unit
+> │       ├── libutil
+> │       │   ├── local.mk
+> │       │   …
+> │       │   └── data
+> │       │       ├── git/tree.txt
+> │       │       …
 > │       │
-> │       …
-> │
-> ├── unit-test-data
-> │   ├── libstore
-> │   │   ├── worker-protocol/content-address.bin
-> │   │   …
-> │   …
+> │       ├── libexpr-support
+> │       │   ├── local.mk
+> │       │   └── tests
+> │       │       ├── value/context.hh
+> │       │       ├── value/context.cc
+> │       │       …
+> │       │
+> │       ├── libexpr
+> │       …   ├── local.mk
+> │           ├── value/context.cc
+> │           …
 > …
 > ```
 
-The unit tests for each Nix library (`libnixexpr`, `libnixstore`, etc..) live inside a directory `src/${library_shortname}/tests` within the directory for the library (`src/${library_shortname}`).
+The tests for each Nix library (`libnixexpr`, `libnixstore`, etc..) live inside a directory `tests/unit/${library_name_without-nix}`.
+Given a interface (header) and implementation pair in the original library, say, `src/libexpr/value/context.{hh,cc}`, we write tests for it in `tests/unit/libexpr/tests/value/context.cc`, and (possibly) declare/define additional interfaces for testing purposes in `tests/unit/libexpr-support/tests/value/context.{hh,cc}`.
 
-The data is in `unit-test-data`, with one subdir per library, with the same name as where the code goes.
-For example, `libnixstore` code is in `src/libstore`, and its test data is in `unit-test-data/libstore`.
-The path to the `unit-test-data` directory is passed to the unit test executable with the environment variable `_NIX_TEST_UNIT_DATA`.
+Data for unit tests is stored in a `data` subdir of the directory for each unit test executable.
+For example, `libnixstore` code is in `src/libstore`, and its test data is in `tests/unit/libstore/data`.
+The path to the `tests/unit/data` directory is passed to the unit test executable with the environment variable `_NIX_TEST_UNIT_DATA`.
+Note that each executable only gets the data for its tests.
 
-> **Note**
-> Due to the way googletest works, downstream unit test executables will actually include and re-run upstream library tests.
-> Therefore it is important that the same value for `_NIX_TEST_UNIT_DATA` be used with the tests for each library.
-> That is why we have the test data nested within a single `unit-test-data` directory.
+The unit test libraries are in `tests/unit/${library_name_without-nix}-lib`.
+All headers are in a `tests` subdirectory so they are included with `#include "tests/"`.
+
+The use of all these separate directories for the unit tests might seem inconvenient, as for example the tests are not "right next to" the part of the code they are testing.
+But organizing the tests this way has one big benefit:
+there is no risk of any build-system wildcards for the library accidentally picking up test code that should not built and installed as part of the library.
 
 ### Running tests
 
@@ -69,7 +86,7 @@ See [functional characterisation testing](#characterisation-testing-functional)
 Like with the functional characterisation, `_NIX_TEST_ACCEPT=1` is also used.
 For example:
 ```shell-session
-$ _NIX_TEST_ACCEPT=1 make libstore-tests-exe_RUN
+$ _NIX_TEST_ACCEPT=1 make libstore-tests_RUN
 ...
 [  SKIPPED ] WorkerProtoTest.string_read
 [  SKIPPED ] WorkerProtoTest.string_write
@@ -80,6 +97,18 @@ $ _NIX_TEST_ACCEPT=1 make libstore-tests-exe_RUN
 will regenerate the "golden master" expected result for the `libnixstore` characterisation tests.
 The characterisation tests will mark themselves "skipped" since they regenerated the expected result instead of actually testing anything.
 
+### Unit test support libraries
+
+There are headers and code which are not just used to test the library in question, but also downstream libraries.
+For example, we do [property testing] with the [rapidcheck] library.
+This requires writing `Arbitrary` "instances", which are used to describe how to generate values of a given type for the sake of running property tests.
+Because types contain other types, `Arbitrary` "instances" for some type are not just useful for testing that type, but also any other type that contains it.
+Downstream types frequently contain upstream types, so it is very important that we share arbitrary instances so that downstream libraries' property tests can also use them.
+
+It is important that these testing libraries don't contain any actual tests themselves.
+On some platforms they would be run as part of every test executable that uses them, which is redundant.
+On other platforms they wouldn't be run at all.
+
 ## Functional tests
 
 The functional tests reside under the `tests/functional` directory and are listed in `tests/functional/local.mk`.

doc/manual/src/release-notes/rl-2.19.md

@@ -18,7 +18,7 @@
 - `nix-shell` shebang lines now support single-quoted arguments.
 
 - `builtins.fetchTree` is now its own experimental feature, [`fetch-tree`](@docroot@/contributing/experimental-features.md#xp-fetch-tree).
-  As described in the documentation for that feature, this is because we anticipate polishing it and then stabilizing it before the rest of flakes.
+  This allows stabilising it independently of the rest of what is encompassed by [`flakes`](@docroot@/contributing/experimental-features.md#xp-fetch-tree).
 
 - The interface for creating and updating lock files has been overhauled:
 

flake.lock

@@ -34,16 +34,16 @@
     },
     "nixpkgs": {
       "locked": {
-        "lastModified": 1698876495,
-        "narHash": "sha256-nsQo2/mkDUFeAjuu92p0dEqhRvHHiENhkKVIV1y0/Oo=",
+        "lastModified": 1700748986,
+        "narHash": "sha256-/nqLrNU297h3PCw4QyDpZKZEUHmialJdZW2ceYFobds=",
         "owner": "NixOS",
         "repo": "nixpkgs",
-        "rev": "9eb24edd6a0027fed010ccfe300a9734d029983c",
+        "rev": "9ba29e2346bc542e9909d1021e8fd7d4b3f64db0",
         "type": "github"
       },
       "original": {
         "owner": "NixOS",
-        "ref": "release-23.05",
+        "ref": "nixos-23.05-small",
         "repo": "nixpkgs",
         "type": "github"
       }

flake.nix

@@ -1,9 +1,7 @@
 {
   description = "The purely functional package manager";
 
-  # FIXME go back to nixos-23.05-small once
-  # https://github.com/NixOS/nixpkgs/pull/264875 is included.
-  inputs.nixpkgs.url = "github:NixOS/nixpkgs/release-23.05";
+  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-23.05-small";
   inputs.nixpkgs-regression.url = "github:NixOS/nixpkgs/215d4d0fd80ca5163643b03a33fde804a29cc1e2";
   inputs.lowdown-src = { url = "github:kristapsdz/lowdown"; flake = false; };
   inputs.flake-compat = { url = "github:edolstra/flake-compat"; flake = false; };
@@ -13,7 +11,7 @@
     let
       inherit (nixpkgs) lib;
 
-      officialRelease = false;
+      officialRelease = true;
 
       version = lib.fileContents ./.version + versionSuffix;
       versionSuffix =
@@ -91,7 +89,7 @@
           ./misc
           ./precompiled-headers.h
           ./src
-          ./unit-test-data
+          ./tests/unit
           ./COPYING
           ./scripts/local.mk
           functionalTestFiles

mk/common-test.sh

@@ -1,7 +1,7 @@
 # Remove overall test dir (at most one of the two should match) and
 # remove file extension.
 test_name=$(echo -n "$test" | sed \
-    -e "s|^unit-test-data/||" \
+    -e "s|^tests/unit/[^/]*/data/||" \
     -e "s|^tests/functional/||" \
     -e "s|\.sh$||" \
     )

mk/programs.mk

@@ -87,6 +87,6 @@ define build-program
   # Phony target to run this program (typically as a dependency of 'check').
   .PHONY: $(1)_RUN
   $(1)_RUN: $$($(1)_PATH)
-	$(trace-test) $$(UNIT_TEST_ENV) $$($(1)_PATH)
+	$(trace-test) $$($(1)_ENV) $$($(1)_PATH)
 
 endef

scripts/nix-profile-daemon.sh.in

@@ -31,7 +31,7 @@ fi
 export NIX_PROFILES="@localstatedir@/nix/profiles/default $NIX_LINK"
 
 # Populate bash completions, .desktop files, etc
-if [ -z "$XDG_DATA_DIRS" ]; then
+if [ -z "${XDG_DATA_DIRS-}" ]; then
     # According to XDG spec the default is /usr/local/share:/usr/share, don't set something that prevents that default
     export XDG_DATA_DIRS="/usr/local/share:/usr/share:$NIX_LINK/share:/nix/var/nix/profiles/default/share"
 else

scripts/nix-profile.sh.in

@@ -33,7 +33,7 @@ if [ -n "$HOME" ] && [ -n "$USER" ]; then
     export NIX_PROFILES="@localstatedir@/nix/profiles/default $NIX_LINK"
 
     # Populate bash completions, .desktop files, etc
-    if [ -z "$XDG_DATA_DIRS" ]; then
+    if [ -z "${XDG_DATA_DIRS-}" ]; then
         # According to XDG spec the default is /usr/local/share:/usr/share, don't set something that prevents that default
         export XDG_DATA_DIRS="/usr/local/share:/usr/share:$NIX_LINK/share:/nix/var/nix/profiles/default/share"
     else

src/libcmd/installables.cc

@@ -47,6 +47,22 @@ MixFlakeOptions::MixFlakeOptions()
 {
     auto category = "Common flake-related options";
 
+    addFlag({
+        .longName = "recreate-lock-file",
+        .description = R"(
+    Recreate the flake's lock file from scratch.
+
+    > **DEPRECATED**
+    >
+    > Use [`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) instead.
+        )",
+        .category = category,
+        .handler = {[&]() {
+            lockFlags.recreateLockFile = true;
+            warn("'--recreate-lock-file' is deprecated and will be removed in a future version; use 'nix flake update' instead.");
+        }}
+    });
+
     addFlag({
         .longName = "no-update-lock-file",
         .description = "Do not allow any updates to the flake's lock file.",
@@ -63,8 +79,13 @@ MixFlakeOptions::MixFlakeOptions()
 
     addFlag({
         .longName = "no-registries",
-        .description =
-          "Don't allow lookups in the flake registries. This option is deprecated; use `--no-use-registries`.",
+        .description = R"(
+    Don't allow lookups in the flake registries.
+
+    > **DEPRECATED**
+    >
+    > Use [`--no-use-registries`](#opt-no-use-registries) instead.
+        )",
         .category = category,
         .handler = {[&]() {
             lockFlags.useRegistries = false;
@@ -79,6 +100,26 @@ MixFlakeOptions::MixFlakeOptions()
         .handler = {&lockFlags.commitLockFile, true}
     });
 
+    addFlag({
+        .longName = "update-input",
+        .description = R"(
+    Update a specific flake input (ignoring its previous entry in the lock file).
+
+    > **DEPRECATED**
+    >
+    > Use [`nix flake update`](@docroot@/command-ref/new-cli/nix3-flake-update.md) instead.
+        )",
+        .category = category,
+        .labels = {"input-path"},
+        .handler = {[&](std::string s) {
+            warn("'--update-input' is a deprecated alias for 'flake update' and will be removed in a future version.");
+            lockFlags.inputUpdates.insert(flake::parseInputPath(s));
+        }},
+        .completer = {[&](AddCompletions & completions, size_t, std::string_view prefix) {
+            completeFlakeInputPath(completions, getEvalState(), getFlakeRefsForCompletion(), prefix);
+        }}
+    });
+
     addFlag({
         .longName = "override-input",
         .description = "Override a specific flake input (e.g. `dwarffs/nixpkgs`). This implies `--no-write-lock-file`.",

src/libexpr/eval-settings.hh

@@ -68,6 +68,11 @@ struct EvalSettings : Config
           evaluation mode. For example, when set to
           `https://github.com/NixOS`, builtin functions such as `fetchGit` are
           allowed to access `https://github.com/NixOS/patchelf.git`.
+
+          Access is granted when
+          - the URI is equal to the prefix,
+          - or the URI is a subpath of the prefix,
+          - or the prefix is a URI scheme ended by a colon `:` and the URI has the same scheme.
         )"};
 
     Setting<bool> traceFunctionCalls{this, false, "trace-function-calls",

src/libexpr/eval.cc

@@ -16,6 +16,7 @@
 #include "fs-input-accessor.hh"
 #include "memory-input-accessor.hh"
 #include "signals.hh"
+#include "url.hh"
 
 #include <algorithm>
 #include <chrono>
@@ -602,6 +603,15 @@ void EvalState::allowAndSetStorePathString(const StorePath & storePath, Value &
     mkStorePathString(storePath, v);
 }
 
+inline static bool isJustSchemePrefix(std::string_view prefix)
+{
+    return
+        !prefix.empty()
+        && prefix[prefix.size() - 1] == ':'
+        && isValidSchemeName(prefix.substr(0, prefix.size() - 1));
+}
+
+
 SourcePath EvalState::checkSourcePath(const SourcePath & path_)
 {
     // Don't check non-rootFS accessors, they're in a different namespace.
@@ -650,21 +660,37 @@ SourcePath EvalState::checkSourcePath(const SourcePath & path_)
 }
 
 
+bool isAllowedURI(std::string_view uri, const Strings & allowedUris)
+{
+    /* 'uri' should be equal to a prefix, or in a subdirectory of a
+       prefix. Thus, the prefix https://github.co does not permit
+       access to https://github.com. */
+    for (auto & prefix : allowedUris) {
+        if (uri == prefix
+            // Allow access to subdirectories of the prefix.
+            || (uri.size() > prefix.size()
+                && prefix.size() > 0
+                && hasPrefix(uri, prefix)
+                && (
+                    // Allow access to subdirectories of the prefix.
+                    prefix[prefix.size() - 1] == '/'
+                    || uri[prefix.size()] == '/'
+
+                    // Allow access to whole schemes
+                    || isJustSchemePrefix(prefix)
+                    )
+                ))
+            return true;
+    }
+
+    return false;
+}
+
 void EvalState::checkURI(const std::string & uri)
 {
     if (!evalSettings.restrictEval) return;
 
-    /* 'uri' should be equal to a prefix, or in a subdirectory of a
-       prefix. Thus, the prefix https://github.co does not permit
-       access to https://github.com. Note: this allows 'http://' and
-       'https://' as prefixes for any http/https URI. */
-    for (auto & prefix : evalSettings.allowedUris.get())
-        if (uri == prefix ||
-            (uri.size() > prefix.size()
-            && prefix.size() > 0
-            && hasPrefix(uri, prefix)
-            && (prefix[prefix.size() - 1] == '/' || uri[prefix.size()] == '/')))
-            return;
+    if (isAllowedURI(uri, evalSettings.allowedUris.get())) return;
 
     /* If the URI is a path, then check it against allowedPaths as
        well. */

src/libexpr/eval.hh

@@ -841,6 +841,11 @@ std::string showType(const Value & v);
  */
 SourcePath resolveExprPath(SourcePath path);
 
+/**
+ * Whether a URI is allowed, assuming restrictEval is enabled
+ */
+bool isAllowedURI(std::string_view uri, const Strings & allowedPaths);
+
 struct InvalidPathError : EvalError
 {
     Path path;

src/libexpr/flake/flakeref.cc

@@ -90,7 +90,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
         fragment = percentDecode(url.substr(fragmentStart+1));
     }
     if (pathEnd != std::string::npos && fragmentStart != std::string::npos) {
-        query = decodeQuery(url.substr(pathEnd+1, fragmentStart));
+        query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1));
     }
 
     if (baseDir) {

src/libexpr/local.mk

@@ -45,8 +45,6 @@ $(foreach i, $(wildcard src/libexpr/flake/*.hh), \
 
 $(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh
 
-$(d)/eval.cc: $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh
-
-$(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh
+$(d)/eval.cc: $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh $(d)/flake/call-flake.nix.gen.hh
 
 src/libexpr/primops/fromTOML.o:	ERROR_SWITCH_ENUM =

src/libexpr/primops.cc

@@ -82,16 +82,15 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
     /* Build/substitute the context. */
     std::vector<DerivedPath> buildReqs;
     for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d });
-    store->buildPaths(buildReqs);
+    buildStore->buildPaths(buildReqs, bmNormal, store);
+
+    StorePathSet outputsToCopyAndAllow;
 
     for (auto & drv : drvs) {
-        auto outputs = resolveDerivedPath(*store, drv);
+        auto outputs = resolveDerivedPath(*buildStore, drv, &*store);
         for (auto & [outputName, outputPath] : outputs) {
-            /* Add the output of this derivations to the allowed
-               paths. */
-            if (allowedPaths) {
-                allowPath(outputPath);
-            }
+            outputsToCopyAndAllow.insert(outputPath);
+
             /* Get all the output paths corresponding to the placeholders we had */
             if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
                 res.insert_or_assign(
@@ -100,12 +99,21 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
                             .drvPath = drv.drvPath,
                             .output = outputName,
                         }).render(),
-                    store->printStorePath(outputPath)
+                    buildStore->printStorePath(outputPath)
                 );
             }
         }
     }
 
+    if (store != buildStore) copyClosure(*buildStore, *store, outputsToCopyAndAllow);
+    if (allowedPaths) {
+        for (auto & outputPath : outputsToCopyAndAllow) {
+            /* Add the output of this derivations to the allowed
+               paths. */
+            allowPath(store->toRealPath(outputPath));
+        }
+    }
+
     return res;
 }
 

src/libexpr/primops/fetchTree.cc

@@ -425,7 +425,8 @@ static RegisterPrimOp primop_fetchGit({
 
       - `shallow` (default: `false`)
 
-        A Boolean parameter that specifies whether fetching a shallow clone is allowed.
+        A Boolean parameter that specifies whether fetching from a shallow remote repository is allowed.
+        This still performs a full clone of what is available on the remote.
 
       - `allRefs`
 

src/libexpr/tests/local.mk

@@ -1,23 +0,0 @@
-check: libexpr-tests_RUN
-
-programs += libexpr-tests
-
-libexpr-tests_NAME := libnixexpr-tests
-
-libexpr-tests_DIR := $(d)
-
-ifeq ($(INSTALL_UNIT_TESTS), yes)
-  libexpr-tests_INSTALL_DIR := $(checkbindir)
-else
-  libexpr-tests_INSTALL_DIR :=
-endif
-
-libexpr-tests_SOURCES := \
-    $(wildcard $(d)/*.cc) \
-    $(wildcard $(d)/value/*.cc)
-
-libexpr-tests_CXXFLAGS += -I src/libexpr -I src/libutil -I src/libstore -I src/libexpr/tests -I src/libfetchers
-
-libexpr-tests_LIBS = libstore-tests libutils-tests libexpr libutil libstore libfetchers
-
-libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock

src/libstore/build/substitution-goal.cc

@@ -2,6 +2,7 @@
 #include "substitution-goal.hh"
 #include "nar-info.hh"
 #include "finally.hh"
+#include "signals.hh"
 
 namespace nix {
 
@@ -217,6 +218,8 @@ void PathSubstitutionGoal::tryToRun()
 
     thr = std::thread([this]() {
         try {
+            ReceiveInterrupts receiveInterrupts;
+
             /* Wake up the worker loop when we're done. */
             Finally updateStats([this]() { outPipe.writeSide.close(); });
 

src/libstore/build/worker.cc

@@ -238,7 +238,7 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers)
 
 void Worker::waitForBuildSlot(GoalPtr goal)
 {
-    debug("wait for build slot");
+    goal->trace("wait for build slot");
     bool isSubstitutionGoal = goal->jobCategory() == JobCategory::Substitution;
     if ((!isSubstitutionGoal && getNrLocalBuilds() < settings.maxBuildJobs) ||
         (isSubstitutionGoal && getNrSubstitutions() < settings.maxSubstitutionJobs))

src/libstore/derivations.cc

@@ -1002,13 +1002,13 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String
 
 }
 
-std::optional<BasicDerivation> Derivation::tryResolve(Store & store) const
+std::optional<BasicDerivation> Derivation::tryResolve(Store & store, Store * evalStore) const
 {
     std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
 
     std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accum;
     accum = [&](auto & inputDrv, auto & node) {
-        for (auto & [outputName, outputPath] : store.queryPartialDerivationOutputMap(inputDrv)) {
+        for (auto & [outputName, outputPath] : store.queryPartialDerivationOutputMap(inputDrv, evalStore)) {
             if (outputPath) {
                 inputDrvOutputs.insert_or_assign({inputDrv, outputName}, *outputPath);
                 if (auto p = get(node.childMap, outputName))

src/libstore/derivations.hh

@@ -340,7 +340,7 @@ struct Derivation : BasicDerivation
      * 2. Input placeholders are replaced with realized input store
      *    paths.
      */
-    std::optional<BasicDerivation> tryResolve(Store & store) const;
+    std::optional<BasicDerivation> tryResolve(Store & store, Store * evalStore = nullptr) const;
 
     /**
      * Like the above, but instead of querying the Nix database for

src/libstore/local-store.cc

@@ -1202,7 +1202,11 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
     Finally cleanup = [&]() {
         if (!narRead) {
             NullParseSink sink;
-            parseDump(sink, source);
+            try {
+                parseDump(sink, source);
+            } catch (...) {
+                ignoreException();
+            }
         }
     };
 

src/libstore/remote-store.cc

@@ -16,6 +16,8 @@
 #include "logging.hh"
 #include "callback.hh"
 #include "filetransfer.hh"
+#include "signals.hh"
+
 #include <nlohmann/json.hpp>
 
 namespace nix {
@@ -65,6 +67,7 @@ void RemoteStore::initConnection(Connection & conn)
 {
     /* Send the magic greeting, check for the reply. */
     try {
+        conn.from.endOfFileError = "Nix daemon disconnected unexpectedly (maybe it crashed?)";
         conn.to << WORKER_MAGIC_1;
         conn.to.flush();
         StringSink saved;
@@ -1071,6 +1074,7 @@ void RemoteStore::ConnectionHandle::withFramedSink(std::function<void(Sink & sin
     std::thread stderrThread([&]()
     {
         try {
+            ReceiveInterrupts receiveInterrupts;
             processStderr(nullptr, nullptr, false);
         } catch (...) {
             ex = std::current_exception();

src/libstore/store-api.cc

@@ -545,8 +545,8 @@ std::map<std::string, std::optional<StorePath>> Store::queryPartialDerivationOut
     return outputs;
 }
 
-OutputPathMap Store::queryDerivationOutputMap(const StorePath & path) {
-    auto resp = queryPartialDerivationOutputMap(path);
+OutputPathMap Store::queryDerivationOutputMap(const StorePath & path, Store * evalStore) {
+    auto resp = queryPartialDerivationOutputMap(path, evalStore);
     OutputPathMap result;
     for (auto & [outName, optOutPath] : resp) {
         if (!optOutPath)
@@ -982,6 +982,11 @@ void copyStorePath(
     RepairFlag repair,
     CheckSigsFlag checkSigs)
 {
+    /* Bail out early (before starting a download from srcStore) if
+       dstStore already has this path. */
+    if (!repair && dstStore.isValidPath(storePath))
+        return;
+
     auto srcUri = srcStore.getUri();
     auto dstUri = dstStore.getUri();
     auto storePathS = srcStore.printStorePath(storePath);

src/libstore/store-api.hh

@@ -460,7 +460,7 @@ public:
      * Query the mapping outputName=>outputPath for the given derivation.
      * Assume every output has a mapping and throw an exception otherwise.
      */
-    OutputPathMap queryDerivationOutputMap(const StorePath & path);
+    OutputPathMap queryDerivationOutputMap(const StorePath & path, Store * evalStore = nullptr);
 
     /**
      * Query the full store path given the hash part of a valid store

src/libstore/tests/local.mk

@@ -1,37 +0,0 @@
-check: libstore-tests-exe_RUN
-
-programs += libstore-tests-exe
-
-libstore-tests-exe_NAME = libnixstore-tests
-
-libstore-tests-exe_DIR := $(d)
-
-ifeq ($(INSTALL_UNIT_TESTS), yes)
-  libstore-tests-exe_INSTALL_DIR := $(checkbindir)
-else
-  libstore-tests-exe_INSTALL_DIR :=
-endif
-
-libstore-tests-exe_LIBS = libstore-tests
-
-libstore-tests-exe_LDFLAGS := $(GTEST_LIBS)
-
-libraries += libstore-tests
-
-libstore-tests_NAME = libnixstore-tests
-
-libstore-tests_DIR := $(d)
-
-ifeq ($(INSTALL_UNIT_TESTS), yes)
-  libstore-tests_INSTALL_DIR := $(checklibdir)
-else
-  libstore-tests_INSTALL_DIR :=
-endif
-
-libstore-tests_SOURCES := $(wildcard $(d)/*.cc)
-
-libstore-tests_CXXFLAGS += -I src/libstore -I src/libutil
-
-libstore-tests_LIBS = libutil-tests libstore libutil
-
-libstore-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)

src/libutil/experimental-features.cc

@@ -80,12 +80,11 @@ constexpr std::array<ExperimentalFeatureDetails, numXpFeatures> xpFeatureDetails
         .description = R"(
             Enable the use of the [`fetchTree`](@docroot@/language/builtins.md#builtins-fetchTree) built-in function in the Nix language.
 
-            `fetchTree` exposes a large suite of fetching functionality in a more systematic way.
+            `fetchTree` exposes a generic interface for fetching remote file system trees from different types of remote sources.
             The [`flakes`](#xp-feature-flakes) feature flag always enables `fetch-tree`.
+            This built-in was previously guarded by the `flakes` experimental feature because of that overlap.
 
-            This built-in was previously guarded by the `flakes` experimental feature because of that overlap,
-            but since the plan is to work on stabilizing this first (due 2024 Q1), we are putting it underneath a separate feature.
-            Once we've made the changes we want to make, enabling just this feature will serve as a "release candidate" --- allowing users to try out the functionality we want to stabilize and not any other functionality we don't yet want to, in isolation.
+            Enabling just this feature serves as a "release candidate", allowing users to try it out in isolation.
         )",
     },
     {

src/libutil/serialise.cc

@@ -132,7 +132,7 @@ size_t FdSource::readUnbuffered(char * data, size_t len)
         n = ::read(fd, data, len);
     } while (n == -1 && errno == EINTR);
     if (n == -1) { _good = false; throw SysError("reading from file"); }
-    if (n == 0) { _good = false; throw EndOfFile("unexpected end-of-file"); }
+    if (n == 0) { _good = false; throw EndOfFile(std::string(*endOfFileError)); }
     read += n;
     return n;
 }
@@ -448,7 +448,7 @@ Error readError(Source & source)
     auto msg = readString(source);
     ErrorInfo info {
         .level = level,
-        .msg = hintformat(fmt("%s", msg)),
+        .msg = hintfmt(msg),
     };
     auto havePos = readNum<size_t>(source);
     assert(havePos == 0);
@@ -457,7 +457,7 @@ Error readError(Source & source)
         havePos = readNum<size_t>(source);
         assert(havePos == 0);
         info.traces.push_back(Trace {
-            .hint = hintformat(fmt("%s", readString(source)))
+            .hint = hintfmt(readString(source))
         });
     }
     return Error(std::move(info));

src/libutil/serialise.hh

@@ -153,12 +153,13 @@ struct FdSource : BufferedSource
 {
     int fd;
     size_t read = 0;
+    BackedStringView endOfFileError{"unexpected end-of-file"};
 
     FdSource() : fd(-1) { }
     FdSource(int fd) : fd(fd) { }
-    FdSource(FdSource&&) = default;
+    FdSource(FdSource &&) = default;
 
-    FdSource& operator=(FdSource && s)
+    FdSource & operator=(FdSource && s)
     {
         fd = s.fd;
         s.fd = -1;

src/libutil/tests/local.mk

@@ -1,41 +0,0 @@
-check: libutil-tests-exe_RUN
-
-programs += libutil-tests-exe
-
-libutil-tests-exe_NAME = libnixutil-tests
-
-libutil-tests-exe_DIR := $(d)
-
-ifeq ($(INSTALL_UNIT_TESTS), yes)
-  libutil-tests-exe_INSTALL_DIR := $(checkbindir)
-else
-  libutil-tests-exe_INSTALL_DIR :=
-endif
-
-libutil-tests-exe_LIBS = libutil-tests
-
-libutil-tests-exe_LDFLAGS := $(GTEST_LIBS)
-
-libraries += libutil-tests
-
-libutil-tests_NAME = libnixutil-tests
-
-libutil-tests_DIR := $(d)
-
-ifeq ($(INSTALL_UNIT_TESTS), yes)
-  libutil-tests_INSTALL_DIR := $(checklibdir)
-else
-  libutil-tests_INSTALL_DIR :=
-endif
-
-libutil-tests_SOURCES := $(wildcard $(d)/*.cc)
-
-libutil-tests_CXXFLAGS += -I src/libutil
-
-libutil-tests_LIBS = libutil
-
-libutil-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS)
-
-check: unit-test-data/libutil/git/check-data.sh.test
-
-$(eval $(call run-test,unit-test-data/libutil/git/check-data.sh))

src/libutil/thread-pool.cc

@@ -79,6 +79,8 @@ void ThreadPool::process()
 
 void ThreadPool::doWork(bool mainThread)
 {
+    ReceiveInterrupts receiveInterrupts;
+
     if (!mainThread)
         interruptCheck = [&]() { return (bool) quit; };
 

src/libutil/url-parts.hh

@@ -8,7 +8,7 @@ namespace nix {
 
 // URI stuff.
 const static std::string pctEncoded = "(?:%[0-9a-fA-F][0-9a-fA-F])";
-const static std::string schemeRegex = "(?:[a-z][a-z0-9+.-]*)";
+const static std::string schemeNameRegex = "(?:[a-z][a-z0-9+.-]*)";
 const static std::string ipv6AddressSegmentRegex = "[0-9a-fA-F:]+(?:%\\w+)?";
 const static std::string ipv6AddressRegex = "(?:\\[" + ipv6AddressSegmentRegex + "\\]|" + ipv6AddressSegmentRegex + ")";
 const static std::string unreservedRegex = "(?:[a-zA-Z0-9-._~])";

src/libutil/url.cc

@@ -12,7 +12,7 @@ std::regex revRegex(revRegexS, std::regex::ECMAScript);
 ParsedURL parseURL(const std::string & url)
 {
     static std::regex uriRegex(
-        "((" + schemeRegex + "):"
+        "((" + schemeNameRegex + "):"
         + "(?:(?://(" + authorityRegex + ")(" + absPathRegex + "))|(/?" + pathRegex + ")))"
         + "(?:\\?(" + queryRegex + "))?"
         + "(?:#(" + queryRegex + "))?",
@@ -175,4 +175,12 @@ std::string fixGitURL(const std::string & url)
     }
 }
 
+// https://www.rfc-editor.org/rfc/rfc3986#section-3.1
+bool isValidSchemeName(std::string_view s)
+{
+    static std::regex regex(schemeNameRegex, std::regex::ECMAScript);
+
+    return std::regex_match(s.begin(), s.end(), regex, std::regex_constants::match_default);
+}
+
 }

src/libutil/url.hh

@@ -50,4 +50,13 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme);
    changes absolute paths into file:// URLs. */
 std::string fixGitURL(const std::string & url);
 
+/**
+ * Whether a string is valid as RFC 3986 scheme name.
+ * Colon `:` is part of the URI; not the scheme name, and therefore rejected.
+ * See https://www.rfc-editor.org/rfc/rfc3986#section-3.1
+ *
+ * Does not check whether the scheme is understood, as that's context-dependent.
+ */
+bool isValidSchemeName(std::string_view scheme);
+
 }

src/nix/flake.cc

@@ -89,7 +89,13 @@ public:
             .label="inputs",
             .optional=true,
             .handler={[&](std::string inputToUpdate){
-                auto inputPath = flake::parseInputPath(inputToUpdate);
+                InputPath inputPath;
+                try {
+                    inputPath = flake::parseInputPath(inputToUpdate);
+                } catch (Error & e) {
+                    warn("Invalid flake input '%s'. To update a specific flake, use 'nix flake update --flake %s' instead.", inputToUpdate, inputToUpdate);
+                    throw e;
+                }
                 if (lockFlags.inputUpdates.contains(inputPath))
                     warn("Input '%s' was specified multiple times. You may have done this by accident.");
                 lockFlags.inputUpdates.insert(inputPath);

tests/functional/eval-store.sh

@@ -28,3 +28,11 @@ nix-build dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result"
 [[ -e $TEST_ROOT/result/foobar ]]
 (! ls $NIX_STORE_DIR/*.drv)
 ls $eval_store/nix/store/*.drv
+
+clearStore
+rm -rf "$eval_store"
+
+# Confirm that import-from-derivation builds on the build store
+[[ $(nix eval --eval-store "$eval_store?require-sigs=false" --impure --raw --file ./ifd.nix) = hi ]]
+ls $NIX_STORE_DIR/*dependencies-top/foobar
+(! ls $eval_store/nix/store/*dependencies-top/foobar)

tests/functional/flakes/flakes.sh

@@ -193,6 +193,14 @@ nix build -o "$TEST_ROOT/result" flake1
 nix build -o "$TEST_ROOT/result" "$flake1Dir"
 nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir"
 
+# Test explicit packages.default.
+nix build -o "$TEST_ROOT/result" "$flake1Dir#default"
+nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir#default"
+
+# Test explicit packages.default with query.
+nix build -o "$TEST_ROOT/result" "$flake1Dir?ref=HEAD#default"
+nix build -o "$TEST_ROOT/result" "git+file://$flake1Dir?ref=HEAD#default"
+
 # Check that store symlinks inside a flake are not interpreted as flakes.
 nix build -o "$flake1Dir/result" "git+file://$flake1Dir"
 nix path-info "$flake1Dir/result"

tests/functional/ifd.nix

@@ -0,0 +1,10 @@
+with import ./config.nix;
+import (
+  mkDerivation {
+    name = "foo";
+    bla = import ./dependencies.nix {};
+    buildCommand = "
+      echo \\\"hi\\\" > $out
+    ";
+  }
+)

tests/functional/remote-store.sh

@@ -19,18 +19,7 @@ else
 fi
 
 # Test import-from-derivation through the daemon.
-[[ $(nix eval --impure --raw --expr '
-  with import ./config.nix;
-  import (
-    mkDerivation {
-      name = "foo";
-      bla = import ./dependencies.nix {};
-      buildCommand = "
-        echo \\\"hi\\\" > $out
-      ";
-    }
-  )
-') = hi ]]
+[[ $(nix eval --impure --raw --file ./ifd.nix) = hi ]]
 
 storeCleared=1 NIX_REMOTE_=$NIX_REMOTE $SHELL ./user-envs.sh
 

tests/unit/libexpr-support/local.mk

@@ -0,0 +1,23 @@
+libraries += libexpr-test-support
+
+libexpr-test-support_NAME = libnixexpr-test-support
+
+libexpr-test-support_DIR := $(d)
+
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libexpr-test-support_INSTALL_DIR := $(checklibdir)
+else
+  libexpr-test-support_INSTALL_DIR :=
+endif
+
+libexpr-test-support_SOURCES := \
+    $(wildcard $(d)/tests/*.cc) \
+    $(wildcard $(d)/tests/value/*.cc)
+
+libexpr-test-support_CXXFLAGS += $(libexpr-tests_EXTRA_INCLUDES)
+
+libexpr-test-support_LIBS = \
+    libstore-test-support libutil-test-support \
+    libexpr libstore libutil
+
+libexpr-test-support_LDFLAGS := -pthread -lrapidcheck

tests/unit/libexpr-support/tests/value/context.cc

@@ -0,0 +1,30 @@
+#include <rapidcheck.h>
+
+#include "tests/path.hh"
+#include "tests/value/context.hh"
+
+namespace rc {
+using namespace nix;
+
+Gen<NixStringContextElem::DrvDeep> Arbitrary<NixStringContextElem::DrvDeep>::arbitrary()
+{
+    return gen::just(NixStringContextElem::DrvDeep {
+        .drvPath = *gen::arbitrary<StorePath>(),
+    });
+}
+
+Gen<NixStringContextElem> Arbitrary<NixStringContextElem>::arbitrary()
+{
+    switch (*gen::inRange<uint8_t>(0, std::variant_size_v<NixStringContextElem::Raw>)) {
+    case 0:
+        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Opaque>());
+    case 1:
+        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::DrvDeep>());
+    case 2:
+        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Built>());
+    default:
+        assert(false);
+    }
+}
+
+}

tests/unit/libexpr-support/tests/value/context.hh

@@ -3,7 +3,7 @@
 
 #include <rapidcheck/gen/Arbitrary.h>
 
-#include <value/context.hh>
+#include "value/context.hh"
 
 namespace rc {
 using namespace nix;

tests/unit/libexpr/eval.cc

@@ -0,0 +1,141 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "eval.hh"
+#include "tests/libexpr.hh"
+
+namespace nix {
+
+TEST(nix_isAllowedURI, http_example_com) {
+    Strings allowed;
+    allowed.push_back("http://example.com");
+
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.co", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.como", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed));
+}
+
+TEST(nix_isAllowedURI, http_example_com_foo) {
+    Strings allowed;
+    allowed.push_back("http://example.com/foo");
+
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.como", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.org/foo", allowed));
+    // Broken?
+    // ASSERT_TRUE(isAllowedURI("http://example.com/foo?ok=1", allowed));
+}
+
+TEST(nix_isAllowedURI, http) {
+    Strings allowed;
+    allowed.push_back("http://");
+
+    ASSERT_TRUE(isAllowedURI("http://", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com/foo/", allowed));
+    ASSERT_TRUE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("https://", allowed));
+    ASSERT_FALSE(isAllowedURI("http:foo", allowed));
+}
+
+TEST(nix_isAllowedURI, https) {
+    Strings allowed;
+    allowed.push_back("https://");
+
+    ASSERT_TRUE(isAllowedURI("https://example.com", allowed));
+    ASSERT_TRUE(isAllowedURI("https://example.com/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/https:", allowed));
+}
+
+TEST(nix_isAllowedURI, absolute_path) {
+    Strings allowed;
+    allowed.push_back("/var/evil"); // bad idea
+
+    ASSERT_TRUE(isAllowedURI("/var/evil", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("/var/evil/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed));
+}
+
+TEST(nix_isAllowedURI, file_url) {
+    Strings allowed;
+    allowed.push_back("file:///var/evil"); // bad idea
+
+    ASSERT_TRUE(isAllowedURI("file:///var/evil", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/foo", allowed));
+    ASSERT_TRUE(isAllowedURI("file:///var/evil/foo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("/var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com/var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://example.com//var/evil/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("http://var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http:///var/evil", allowed));
+    ASSERT_FALSE(isAllowedURI("http://var/evil/", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evi", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo/", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///var/evilo/foo", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///", allowed));
+    ASSERT_FALSE(isAllowedURI("file://", allowed));
+}
+
+TEST(nix_isAllowedURI, github_all) {
+    Strings allowed;
+    allowed.push_back("github:");
+    ASSERT_TRUE(isAllowedURI("github:", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github://foo/bar", allowed));
+    ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("github", allowed));
+}
+
+TEST(nix_isAllowedURI, github_org) {
+    Strings allowed;
+    allowed.push_back("github:foo");
+    ASSERT_FALSE(isAllowedURI("github:", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar/feat-multi-bar", allowed));
+    ASSERT_TRUE(isAllowedURI("github:foo/bar?ref=refs/heads/feat-multi-bar", allowed));
+    ASSERT_FALSE(isAllowedURI("github://foo/bar", allowed));
+    ASSERT_FALSE(isAllowedURI("https://github:443/foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file://github:foo/bar/archive/master.tar.gz", allowed));
+    ASSERT_FALSE(isAllowedURI("file:///github:foo/bar/archive/master.tar.gz", allowed));
+}
+
+TEST(nix_isAllowedURI, non_scheme_colon) {
+    Strings allowed;
+    allowed.push_back("https://foo/bar:");
+    ASSERT_TRUE(isAllowedURI("https://foo/bar:", allowed));
+    ASSERT_TRUE(isAllowedURI("https://foo/bar:/baz", allowed));
+    ASSERT_FALSE(isAllowedURI("https://foo/bar:baz", allowed));
+}
+
+} // namespace nix

tests/unit/libexpr/local.mk

@@ -0,0 +1,36 @@
+check: libexpr-tests_RUN
+
+programs += libexpr-tests
+
+libexpr-tests_NAME := libnixexpr-tests
+
+libexpr-tests_ENV := _NIX_TEST_UNIT_DATA=$(d)/data
+
+libexpr-tests_DIR := $(d)
+
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libexpr-tests_INSTALL_DIR := $(checkbindir)
+else
+  libexpr-tests_INSTALL_DIR :=
+endif
+
+libexpr-tests_SOURCES := \
+    $(wildcard $(d)/*.cc) \
+    $(wildcard $(d)/value/*.cc)
+
+libexpr-tests_EXTRA_INCLUDES = \
+    -I tests/unit/libexpr-support \
+    -I tests/unit/libstore-support \
+    -I tests/unit/libutil-support \
+    -I src/libexpr \
+    -I src/libfetchers \
+    -I src/libstore \
+    -I src/libutil
+
+libexpr-tests_CXXFLAGS += $(libexpr-tests_EXTRA_INCLUDES)
+
+libexpr-tests_LIBS = \
+    libexpr-test-support libstore-test-support libutils-test-support \
+    libexpr libfetchers libstore libutil
+
+libexpr-tests_LDFLAGS := -lrapidcheck $(GTEST_LIBS) -lgmock

tests/unit/libexpr/value/context.cc

@@ -117,36 +117,6 @@ TEST(NixStringContextElemTest, built_built_xp) {
         NixStringContextElem::parse("!foo!bar!g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x.drv"),        MissingExperimentalFeature);
 }
 
-}
-
-namespace rc {
-using namespace nix;
-
-Gen<NixStringContextElem::DrvDeep> Arbitrary<NixStringContextElem::DrvDeep>::arbitrary()
-{
-    return gen::just(NixStringContextElem::DrvDeep {
-        .drvPath = *gen::arbitrary<StorePath>(),
-    });
-}
-
-Gen<NixStringContextElem> Arbitrary<NixStringContextElem>::arbitrary()
-{
-    switch (*gen::inRange<uint8_t>(0, std::variant_size_v<NixStringContextElem::Raw>)) {
-    case 0:
-        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Opaque>());
-    case 1:
-        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::DrvDeep>());
-    case 2:
-        return gen::just<NixStringContextElem>(*gen::arbitrary<NixStringContextElem::Built>());
-    default:
-        assert(false);
-    }
-}
-
-}
-
-namespace nix {
-
 #ifndef COVERAGE
 
 RC_GTEST_PROP(

tests/unit/libstore-support/local.mk

@@ -0,0 +1,21 @@
+libraries += libstore-test-support
+
+libstore-test-support_NAME = libnixstore-test-support
+
+libstore-test-support_DIR := $(d)
+
+ifeq ($(INSTALL_UNIT_TESTS), yes)
+  libstore-test-support_INSTALL_DIR := $(checklibdir)
+else
+  libstore-test-support_INSTALL_DIR :=
+endif
+
+libstore-test-support_SOURCES := $(wildcard $(d)/tests/*.cc)
+
+libstore-test-support_CXXFLAGS += $(libstore-tests_EXTRA_INCLUDES)
+
+libstore-test-support_LIBS = \
+    libutil-test-support \
+    libstore libutil
+
+libstore-test-support_LDFLAGS := -pthread -lrapidcheck

tests/unit/libstore-support/tests/derived-path.cc

@@ -0,0 +1,57 @@
+#include <regex>
+
+#include <rapidcheck.h>
+
+#include "tests/derived-path.hh"
+
+namespace rc {
+using namespace nix;
+
+Gen<DerivedPath::Opaque> Arbitrary<DerivedPath::Opaque>::arbitrary()
+{
+    return gen::just(DerivedPath::Opaque {
+        .path = *gen::arbitrary<StorePath>(),
+    });
+}
+
+Gen<SingleDerivedPath::Built> Arbitrary<SingleDerivedPath::Built>::arbitrary()
+{
+    return gen::just(SingleDerivedPath::Built {
+        .drvPath = make_ref<SingleDerivedPath>(*gen::arbitrary<SingleDerivedPath>()),
+        .output = (*gen::arbitrary<StorePathName>()).name,
+    });
+}
+
+Gen<DerivedPath::Built> Arbitrary<DerivedPath::Built>::arbitrary()
+{
+    return gen::just(DerivedPath::Built {
+        .drvPath = make_ref<SingleDerivedPath>(*gen::arbitrary<SingleDerivedPath>()),
+        .outputs = *gen::arbitrary<OutputsSpec>(),
+    });
+}
+
+Gen<SingleDerivedPath> Arbitrary<SingleDerivedPath>::arbitrary()
+{
+    switch (*gen::inRange<uint8_t>(0, std::variant_size_v<SingleDerivedPath::Raw>)) {
+    case 0:
+        return gen::just<SingleDerivedPath>(*gen::arbitrary<SingleDerivedPath::Opaque>());
+    case 1:
+        return gen::just<SingleDerivedPath>(*gen::arbitrary<SingleDerivedPath::Built>());
+    default:
+        assert(false);
+    }
+}
+
+Gen<DerivedPath> Arbitrary<DerivedPath>::arbitrary()
+{
+    switch (*gen::inRange<uint8_t>(0, std::variant_size_v<DerivedPath::Raw>)) {
+    case 0:
+        return gen::just<DerivedPath>(*gen::arbitrary<DerivedPath::Opaque>());
+    case 1:
+        return gen::just<DerivedPath>(*gen::arbitrary<DerivedPath::Built>());
+    default:
+        assert(false);
+    }
+}
+
+}

tests/unit/libstore-support/tests/outputs-spec.cc

@@ -0,0 +1,24 @@
+#include "tests/outputs-spec.hh"
+
+#include <rapidcheck.h>
+
+namespace rc {
+using namespace nix;
+
+Gen<OutputsSpec> Arbitrary<OutputsSpec>::arbitrary()
+{
+    switch (*gen::inRange<uint8_t>(0, std::variant_size_v<OutputsSpec::Raw>)) {
+    case 0:
+        return gen::just((OutputsSpec) OutputsSpec::All { });
+    case 1:
+        return gen::just((OutputsSpec) OutputsSpec::Names {
+            *gen::nonEmpty(gen::container<StringSet>(gen::map(
+                gen::arbitrary<StorePathName>(),
+                [](StorePathName n) { return n.name; }))),
+        });
+    default:
+        assert(false);
+    }
+}
+
+}

tests/unit/libstore-support/tests/outputs-spec.hh

@@ -5,7 +5,7 @@
 
 #include <outputs-spec.hh>
 
-#include <tests/path.hh>
+#include "tests/path.hh"
 
 namespace rc {
 using namespace nix;

tests/unit/libstore-support/tests/path.cc

@@ -0,0 +1,82 @@
+#include <regex>
+
+#include <rapidcheck.h>
+
+#include "path-regex.hh"
+#include "store-api.hh"
+
+#include "tests/hash.hh"
+#include "tests/path.hh"
+
+namespace nix {
+
+void showValue(const StorePath & p, std::ostream & os)
+{
+    os << p.to_string();
+}
+
+}
+
+namespace rc {
+using namespace nix;
+
+Gen<StorePathName> Arbitrary<StorePathName>::arbitrary()
+{
+    auto len = *gen::inRange<size_t>(
+        1,
+        StorePath::MaxPathLen - StorePath::HashLen);
+
+    std::string pre;
+    pre.reserve(len);
+
+    for (size_t c = 0; c < len; ++c) {
+        switch (auto i = *gen::inRange<uint8_t>(0, 10 + 2 * 26 + 6)) {
+            case 0 ... 9:
+                pre += '0' + i;
+            case 10 ... 35:
+                pre += 'A' + (i - 10);
+                break;
+            case 36 ... 61:
+                pre += 'a' + (i - 36);
+                break;
+            case 62:
+                pre += '+';
+                break;
+            case 63:
+                pre += '-';
+                break;
+            case 64:
+                // names aren't permitted to start with a period,
+                // so just fall through to the next case here
+                if (c != 0) {
+                    pre += '.';
+                    break;
+                }
+            case 65:
+                pre += '_';
+                break;
+            case 66:
+                pre += '?';
+                break;
+            case 67:
+                pre += '=';
+                break;
+            default:
+                assert(false);
+        }
+    }
+
+    return gen::just(StorePathName {
+        .name = std::move(pre),
+    });
+}
+
+Gen<StorePath> Arbitrary<StorePath>::arbitrary()
+{
+    return gen::just(StorePath {
+        *gen::arbitrary<Hash>(),
+        (*gen::arbitrary<StorePathName>()).name,
+    });
+}
+
+} // namespace rc

tests/unit/libstore-support/tests/path.hh

@@ -11,6 +11,9 @@ struct StorePathName {
     std::string name;
 };
 
+// For rapidcheck
+void showValue(const StorePath & p, std::ostream & os);
+
 }
 
 namespace rc {

tests/unit/libstore-support/tests/protocol.hh

@@ -12,7 +12,7 @@ namespace nix {
 template<class Proto, const char * protocolDir>
 class ProtoTest : public CharacterizationTest, public LibStoreTest
 {
-    Path unitTestData = getUnitTestData() + "/libstore/" + protocolDir;
+    Path unitTestData = getUnitTestData() + "/" + protocolDir;
 
     Path goldenMaster(std::string_view testStem) const override {
         return unitTestData + "/" + testStem + ".bin";