From 3b3822ea1ddf34f8fcb44c67cda2b778b5b41a34 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 7 Jul 2023 15:08:25 +0200 Subject: [PATCH 01/20] tests: Reformat exit code error message Now looks like: Expected exit code '123' but got '0' from command 'echo' 'hi' --- tests/common/vars-and-functions.sh.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/common/vars-and-functions.sh.in b/tests/common/vars-and-functions.sh.in index a9e6c802f..33a764c3f 100644 --- a/tests/common/vars-and-functions.sh.in +++ b/tests/common/vars-and-functions.sh.in @@ -195,7 +195,7 @@ expect() { shift "$@" && res=0 || res="$?" if [[ $res -ne $expected ]]; then - echo "Expected '$expected' but got '$res' while running '${*@Q}'" >&2 + echo "Expected exit code '$expected' but got '$res' from command ${*@Q}" >&2 return 1 fi return 0 @@ -209,7 +209,7 @@ expectStderr() { shift "$@" 2>&1 && res=0 || res="$?" if [[ $res -ne $expected ]]; then - echo "Expected '$expected' but got '$res' while running '${*@Q}'" >&2 + echo "Expected exit code '$expected' but got '$res' from command ${*@Q}" >&2 return 1 fi return 0 From c4dbb55ba93c9c81d798faa6a19285ebe2717a25 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 11 Aug 2023 17:22:55 +0200 Subject: [PATCH 02/20] initLibUtil: Add exception handling self-check --- src/libutil/error.cc | 5 +++++ src/libutil/error.hh | 4 ++++ src/libutil/util.cc | 17 +++++++++++++++++ 3 files changed, 26 insertions(+) diff --git a/src/libutil/error.cc b/src/libutil/error.cc index c9d61942a..4a1b346ef 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -14,6 +14,11 @@ void BaseError::addTrace(std::shared_ptr && e, hintformat hint, boo err.traces.push_front(Trace { .pos = std::move(e), .hint = hint, .frame = frame }); } +void throwExceptionSelfCheck(){ + // This is meant to be caught in initLibUtil() + throw SysError("C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded."); +} + // c++ std::exception descendants must have a 'const char* what()' function. // This stringifies the error and caches it for use by what(), or similarly by msg(). const std::string & BaseError::calcWhat() const diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 6a0923081..be5c5e252 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -214,4 +214,8 @@ public: } }; +/** Throw an exception for the purpose of checking that exception handling works; see 'initLibUtil()'. + */ +void throwExceptionSelfCheck(); + } diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 26f9dc8a8..f24a6e165 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -48,6 +48,23 @@ extern char * * environ __attribute__((weak)); namespace nix { void initLibUtil() { + // Check that exception handling works. Exception handling has been observed + // not to work on darwin when the linker flags aren't quite right. + // In this case we don't want to expose the user to some unrelated uncaught + // exception, but rather tell them exactly that exception handling is + // broken. + // When exception handling fails, the message tends to be printed by the + // C++ runtime, followed by an abort. + // For example on macOS we might see an error such as + // libc++abi: terminating with uncaught exception of type nix::SysError: error: C++ exception handling is broken. This would appear to be a problem with the way Nix was compiled and/or linked and/or loaded. + bool caught = false; + try { + throwExceptionSelfCheck(); + } catch (nix::Error _e) { + caught = true; + } + // This is not actually the main point of this check, but let's make sure anyway: + assert(caught); } std::optional getEnv(const std::string & key) From 1ef8008ca7a7b38570f06abdbb7960e93ac044ef Mon Sep 17 00:00:00 2001 From: Alex Zero Date: Sun, 13 Aug 2023 01:31:32 +0100 Subject: [PATCH 03/20] Fix follow path checking at depths greater than 2 We need to recurse into the input tree to handle follows paths that trarverse multiple inputs that may or may not be follow paths themselves. --- src/libexpr/flake/lockfile.cc | 2 +- tests/flakes/follow-paths.sh | 63 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/libexpr/flake/lockfile.cc b/src/libexpr/flake/lockfile.cc index ba2fd46f0..3c202967a 100644 --- a/src/libexpr/flake/lockfile.cc +++ b/src/libexpr/flake/lockfile.cc @@ -345,7 +345,7 @@ void LockFile::check() for (auto & [inputPath, input] : inputs) { if (auto follows = std::get_if<1>(&input)) { - if (!follows->empty() && !get(inputs, *follows)) + if (!follows->empty() && !findInput(*follows)) throw Error("input '%s' follows a non-existent input '%s'", printInputPath(inputPath), printInputPath(*follows)); diff --git a/tests/flakes/follow-paths.sh b/tests/flakes/follow-paths.sh index fe9b51c65..a70d9acb2 100644 --- a/tests/flakes/follow-paths.sh +++ b/tests/flakes/follow-paths.sh @@ -148,3 +148,66 @@ git -C $flakeFollowsA add flake.nix nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid'" nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid2'" + +# Now test follow path overloading +flakeFollowsOverloadA=$TEST_ROOT/follows/overload/flakeA +flakeFollowsOverloadB=$TEST_ROOT/follows/overload/flakeA/flakeB +flakeFollowsOverloadC=$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC +flakeFollowsOverloadD=$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC/flakeD + +# Test following path flakerefs. +createGitRepo $flakeFollowsOverloadA +mkdir -p $flakeFollowsOverloadB +mkdir -p $flakeFollowsOverloadC +mkdir -p $flakeFollowsOverloadD + +cat > $flakeFollowsOverloadD/flake.nix < $flakeFollowsOverloadC/flake.nix < $flakeFollowsOverloadB/flake.nix < $flakeFollowsOverloadA/flake.nix < Date: Mon, 14 Aug 2023 18:52:21 +0100 Subject: [PATCH 04/20] Add release notes for the previous commit --- doc/manual/src/release-notes/rl-next.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 7ddb5ca00..5a870f1ab 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -22,3 +22,5 @@ - Introduce a new [`outputOf`](@docroot@/language/builtins.md#builtins-outputOf) builtin. It is part of the [`dynamic-derivations`](@docroot@/contributing/experimental-features.md#xp-feature-dynamic-derivations) experimental feature. + +- Flake follow paths at depths greater than 2 are now handled correctly, preventing "follows a non-existent input" errors. From 20d9c672d14de3f05a791251a67618d679cf4173 Mon Sep 17 00:00:00 2001 From: Vertex <5567402+VertexA115@users.noreply.github.com> Date: Tue, 15 Aug 2023 10:10:27 +0100 Subject: [PATCH 05/20] Update tests/flakes/follow-paths.sh Co-authored-by: Robert Hensing --- tests/flakes/follow-paths.sh | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/flakes/follow-paths.sh b/tests/flakes/follow-paths.sh index a70d9acb2..f9d5c8c80 100644 --- a/tests/flakes/follow-paths.sh +++ b/tests/flakes/follow-paths.sh @@ -150,6 +150,25 @@ nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override fo nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid2'" # Now test follow path overloading +# This tests a lockfile checking regression https://github.com/NixOS/nix/pull/8819 +# +# We construct the following graph, where p->q means p has input q. +# A double edge means that the edge gets overridden using `follows`. +# +# A +# / \ +# / \ +# v v +# B ==> C --- follows declared in A +# \\ / +# \\/ --- follows declared in B +# v +# D +# +# The message was +# error: input 'B/D' follows a non-existent input 'B/C/D' +# +# Note that for `B` to resolve its follow for `D`, it needs `C/D`, for which it needs to resolve the follow on `C` first. flakeFollowsOverloadA=$TEST_ROOT/follows/overload/flakeA flakeFollowsOverloadB=$TEST_ROOT/follows/overload/flakeA/flakeB flakeFollowsOverloadC=$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC From 75243c96937b472665b94729df81f8296b262085 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 18 Aug 2023 14:46:13 +0200 Subject: [PATCH 06/20] test/flakes/follow-paths.sh: Quote Co-authored-by: Alex Ameen --- tests/flakes/follow-paths.sh | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/tests/flakes/follow-paths.sh b/tests/flakes/follow-paths.sh index f9d5c8c80..dc97027ac 100644 --- a/tests/flakes/follow-paths.sh +++ b/tests/flakes/follow-paths.sh @@ -146,8 +146,8 @@ EOF git -C $flakeFollowsA add flake.nix -nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid'" -nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid2'" +nix flake lock "$flakeFollowsA" 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid'" +nix flake lock "$flakeFollowsA" 2>&1 | grep "warning: input 'B' has an override for a non-existent input 'invalid2'" # Now test follow path overloading # This tests a lockfile checking regression https://github.com/NixOS/nix/pull/8819 @@ -169,18 +169,18 @@ nix flake lock $flakeFollowsA 2>&1 | grep "warning: input 'B' has an override fo # error: input 'B/D' follows a non-existent input 'B/C/D' # # Note that for `B` to resolve its follow for `D`, it needs `C/D`, for which it needs to resolve the follow on `C` first. -flakeFollowsOverloadA=$TEST_ROOT/follows/overload/flakeA -flakeFollowsOverloadB=$TEST_ROOT/follows/overload/flakeA/flakeB -flakeFollowsOverloadC=$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC -flakeFollowsOverloadD=$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC/flakeD +flakeFollowsOverloadA="$TEST_ROOT/follows/overload/flakeA" +flakeFollowsOverloadB="$TEST_ROOT/follows/overload/flakeA/flakeB" +flakeFollowsOverloadC="$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC" +flakeFollowsOverloadD="$TEST_ROOT/follows/overload/flakeA/flakeB/flakeC/flakeD" # Test following path flakerefs. -createGitRepo $flakeFollowsOverloadA -mkdir -p $flakeFollowsOverloadB -mkdir -p $flakeFollowsOverloadC -mkdir -p $flakeFollowsOverloadD +createGitRepo "$flakeFollowsOverloadA" +mkdir -p "$flakeFollowsOverloadB" +mkdir -p "$flakeFollowsOverloadC" +mkdir -p "$flakeFollowsOverloadD" -cat > $flakeFollowsOverloadD/flake.nix < "$flakeFollowsOverloadD/flake.nix" < $flakeFollowsOverloadD/flake.nix < $flakeFollowsOverloadC/flake.nix < "$flakeFollowsOverloadC/flake.nix" < $flakeFollowsOverloadC/flake.nix < $flakeFollowsOverloadB/flake.nix < "$flakeFollowsOverloadB/flake.nix" < $flakeFollowsOverloadB/flake.nix < $flakeFollowsOverloadA/flake.nix < "$flakeFollowsOverloadA/flake.nix" < $flakeFollowsOverloadA/flake.nix < Date: Tue, 22 Aug 2023 19:29:25 +0200 Subject: [PATCH 07/20] Tarball trees: Propagate lastModified This makes them behave consistently with GitHub/GitLab flakes. --- src/libfetchers/tarball.cc | 5 ++++- tests/tarball.sh | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 107d38e92..e3a41e75e 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -232,7 +232,7 @@ struct CurlInputScheme : InputScheme if (type != inputType()) return {}; // FIXME: some of these only apply to TarballInputScheme. - std::set allowedNames = {"type", "url", "narHash", "name", "unpack", "rev", "revCount"}; + std::set allowedNames = {"type", "url", "narHash", "name", "unpack", "rev", "revCount", "lastModified"}; for (auto & [name, value] : attrs) if (!allowedNames.count(name)) throw Error("unsupported %s input attribute '%s'", *type, name); @@ -310,6 +310,9 @@ struct TarballInputScheme : CurlInputScheme input = immutableInput; } + if (result.lastModified && !input.attrs.contains("lastModified")) + input.attrs.insert_or_assign("lastModified", uint64_t(result.lastModified)); + return {result.tree.storePath, std::move(input)}; } }; diff --git a/tests/tarball.sh b/tests/tarball.sh index 5f39658c9..6e621a28c 100644 --- a/tests/tarball.sh +++ b/tests/tarball.sh @@ -9,6 +9,7 @@ rm -rf $tarroot mkdir -p $tarroot cp dependencies.nix $tarroot/default.nix cp config.nix dependencies.builder*.sh $tarroot/ +touch -d '@1000000000' $tarroot $tarroot/* hash=$(nix hash path $tarroot) @@ -36,6 +37,8 @@ test_tarball() { nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file:///does-not-exist/must-remain-unused/$tarball; narHash = \"$hash\"; })" expectStderr 102 nix-build -o $TEST_ROOT/result -E "import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"sha256-xdKv2pq/IiwLSnBBJXW8hNowI4MrdZfW+SYqDQs7Tzc=\"; })" | grep 'NAR hash mismatch in input' + [[ $(nix eval --impure --expr "(fetchTree file://$tarball).lastModified") = 1000000000 ]] + nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" >&2 nix-instantiate --strict --eval -E "!((import (fetchTree { type = \"tarball\"; url = file://$tarball; narHash = \"$hash\"; })) ? submodules)" 2>&1 | grep 'true' From 4a435ad2288bffd5c28d90758e4e26160b53fb8c Mon Sep 17 00:00:00 2001 From: Uri Zafrir Date: Wed, 23 Aug 2023 18:18:25 +0300 Subject: [PATCH 08/20] Add introductory sentence to advanced topics (#8861) --- doc/manual/src/advanced-topics/advanced-topics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/src/advanced-topics/advanced-topics.md b/doc/manual/src/advanced-topics/advanced-topics.md index 8b1378917..9a4d12a33 100644 --- a/doc/manual/src/advanced-topics/advanced-topics.md +++ b/doc/manual/src/advanced-topics/advanced-topics.md @@ -1 +1 @@ - +This section lists advanced topics related to builds and builds performance From 7d82341633d81f50f3e5e0b0896cfac5ea805d57 Mon Sep 17 00:00:00 2001 From: p01arst0rm Date: Wed, 23 Aug 2023 19:28:24 +0100 Subject: [PATCH 09/20] update system definitions --- flake.nix | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/flake.nix b/flake.nix index 6e1f48cd7..cf7c1fa12 100644 --- a/flake.nix +++ b/flake.nix @@ -19,10 +19,12 @@ then "" else "pre${builtins.substring 0 8 (self.lastModifiedDate or self.lastModified or "19700101")}_${self.shortRev or "dirty"}"; + linux32BitSystems = [ "i686-linux" ]; linux64BitSystems = [ "x86_64-linux" "aarch64-linux" ]; - linuxSystems = linux64BitSystems ++ [ "i686-linux" ]; - systems = linuxSystems ++ [ "x86_64-darwin" "aarch64-darwin" ]; - + linuxSystems = linux32BitSystems ++ linux64BitSystems; + darwinSystems = [ "x86_64-darwin" "aarch64-darwin" ]; + systems = linuxSystems ++ darwinSystems; + crossSystems = [ "armv6l-linux" "armv7l-linux" ]; stdenvs = [ "gccStdenv" "clangStdenv" "clang11Stdenv" "stdenv" "libcxxStdenv" "ccacheStdenv" ]; From d5b130ef13bd9a77d803950ca814ac9a612f77b8 Mon Sep 17 00:00:00 2001 From: Valentin Gagarin Date: Thu, 24 Aug 2023 10:00:17 +0200 Subject: [PATCH 10/20] glossary: dedent list and do not use forced line breaks this makes it slightly easier to work with and consistent with all the other markdown lists in use --- doc/manual/src/glossary.md | 367 ++++++++++++++++++++----------------- 1 file changed, 200 insertions(+), 167 deletions(-) diff --git a/doc/manual/src/glossary.md b/doc/manual/src/glossary.md index ac0bb3c2f..d950a4ca8 100644 --- a/doc/manual/src/glossary.md +++ b/doc/manual/src/glossary.md @@ -1,236 +1,269 @@ # Glossary - - [derivation]{#gloss-derivation}\ - A description of a build task. The result of a derivation is a - store object. Derivations are typically specified in Nix expressions - using the [`derivation` primitive](./language/derivations.md). These are - translated into low-level *store derivations* (implicitly by - `nix-env` and `nix-build`, or explicitly by `nix-instantiate`). +- [derivation]{#gloss-derivation} - [derivation]: #gloss-derivation + A description of a build task. The result of a derivation is a + store object. Derivations are typically specified in Nix expressions + using the [`derivation` primitive](./language/derivations.md). These are + translated into low-level *store derivations* (implicitly by + `nix-env` and `nix-build`, or explicitly by `nix-instantiate`). - - [store derivation]{#gloss-store-derivation}\ - A [derivation] represented as a `.drv` file in the [store]. - It has a [store path], like any [store object]. + [derivation]: #gloss-derivation - Example: `/nix/store/g946hcz4c8mdvq2g8vxx42z51qb71rvp-git-2.38.1.drv` +- [store derivation]{#gloss-store-derivation} - See [`nix derivation show`](./command-ref/new-cli/nix3-derivation-show.md) (experimental) for displaying the contents of store derivations. + A [derivation] represented as a `.drv` file in the [store]. + It has a [store path], like any [store object]. - [store derivation]: #gloss-store-derivation + Example: `/nix/store/g946hcz4c8mdvq2g8vxx42z51qb71rvp-git-2.38.1.drv` - - [instantiate]{#gloss-instantiate}, instantiation\ - Translate a [derivation] into a [store derivation]. + See [`nix derivation show`](./command-ref/new-cli/nix3-derivation-show.md) (experimental) for displaying the contents of store derivations. - See [`nix-instantiate`](./command-ref/nix-instantiate.md). + [store derivation]: #gloss-store-derivation - [instantiate]: #gloss-instantiate +- [instantiate]{#gloss-instantiate}, instantiation - - [realise]{#gloss-realise}, realisation\ - Ensure a [store path] is [valid][validity]. + Translate a [derivation] into a [store derivation]. - This means either running the `builder` executable as specified in the corresponding [derivation] or fetching a pre-built [store object] from a [substituter]. + See [`nix-instantiate`](./command-ref/nix-instantiate.md). - See [`nix-build`](./command-ref/nix-build.md) and [`nix-store --realise`](@docroot@/command-ref/nix-store/realise.md). + [instantiate]: #gloss-instantiate - See [`nix build`](./command-ref/new-cli/nix3-build.md) (experimental). +- [realise]{#gloss-realise}, realisation - [realise]: #gloss-realise + Ensure a [store path] is [valid][validity]. - - [content-addressed derivation]{#gloss-content-addressed-derivation}\ - A derivation which has the - [`__contentAddressed`](./language/advanced-attributes.md#adv-attr-__contentAddressed) - attribute set to `true`. + This means either running the `builder` executable as specified in the corresponding [derivation] or fetching a pre-built [store object] from a [substituter]. - - [fixed-output derivation]{#gloss-fixed-output-derivation}\ - A derivation which includes the - [`outputHash`](./language/advanced-attributes.md#adv-attr-outputHash) attribute. + See [`nix-build`](./command-ref/nix-build.md) and [`nix-store --realise`](@docroot@/command-ref/nix-store/realise.md). - - [store]{#gloss-store}\ - The location in the file system where store objects live. Typically - `/nix/store`. + See [`nix build`](./command-ref/new-cli/nix3-build.md) (experimental). - From the perspective of the location where Nix is - invoked, the Nix store can be referred to - as a "_local_" or a "_remote_" one: + [realise]: #gloss-realise - + A [local store]{#gloss-local-store} exists on the filesystem of - the machine where Nix is invoked. You can use other - local stores by passing the `--store` flag to the - `nix` command. Local stores can be used for building derivations. +- [content-addressed derivation]{#gloss-content-addressed-derivation} - + A *remote store* exists anywhere other than the - local filesystem. One example is the `/nix/store` - directory on another machine, accessed via `ssh` or - served by the `nix-serve` Perl script. + A derivation which has the + [`__contentAddressed`](./language/advanced-attributes.md#adv-attr-__contentAddressed) + attribute set to `true`. - [store]: #gloss-store - [local store]: #gloss-local-store +- [fixed-output derivation]{#gloss-fixed-output-derivation} - - [chroot store]{#gloss-chroot-store}\ - A [local store] whose canonical path is anything other than `/nix/store`. + A derivation which includes the + [`outputHash`](./language/advanced-attributes.md#adv-attr-outputHash) attribute. - - [binary cache]{#gloss-binary-cache}\ - A *binary cache* is a Nix store which uses a different format: its - metadata and signatures are kept in `.narinfo` files rather than in a - [Nix database]. This different format simplifies serving store objects - over the network, but cannot host builds. Examples of binary caches - include S3 buckets and the [NixOS binary cache](https://cache.nixos.org). +- [store]{#gloss-store} - - [store path]{#gloss-store-path}\ - The location of a [store object] in the file system, i.e., an - immediate child of the Nix store directory. + The location in the file system where store objects live. Typically + `/nix/store`. - Example: `/nix/store/a040m110amc4h71lds2jmr8qrkj2jhxd-git-2.38.1` + From the perspective of the location where Nix is + invoked, the Nix store can be referred to + as a "_local_" or a "_remote_" one: - [store path]: #gloss-store-path + + A [local store]{#gloss-local-store} exists on the filesystem of + the machine where Nix is invoked. You can use other + local stores by passing the `--store` flag to the + `nix` command. Local stores can be used for building derivations. - - [file system object]{#gloss-store-object}\ - The Nix data model for representing simplified file system data. + + A *remote store* exists anywhere other than the + local filesystem. One example is the `/nix/store` + directory on another machine, accessed via `ssh` or + served by the `nix-serve` Perl script. - See [File System Object](@docroot@/architecture/file-system-object.md) for details. + [store]: #gloss-store + [local store]: #gloss-local-store - [file system object]: #gloss-file-system-object +- [chroot store]{#gloss-chroot-store} - - [store object]{#gloss-store-object}\ + A [local store] whose canonical path is anything other than `/nix/store`. - A store object consists of a [file system object], [reference]s to other store objects, and other metadata. - It can be referred to by a [store path]. +- [binary cache]{#gloss-binary-cache} - [store object]: #gloss-store-object + A *binary cache* is a Nix store which uses a different format: its + metadata and signatures are kept in `.narinfo` files rather than in a + [Nix database]. This different format simplifies serving store objects + over the network, but cannot host builds. Examples of binary caches + include S3 buckets and the [NixOS binary cache](https://cache.nixos.org). - - [input-addressed store object]{#gloss-input-addressed-store-object}\ - A store object produced by building a - non-[content-addressed](#gloss-content-addressed-derivation), - non-[fixed-output](#gloss-fixed-output-derivation) - derivation. +- [store path]{#gloss-store-path} - - [output-addressed store object]{#gloss-output-addressed-store-object}\ - A [store object] whose [store path] is determined by its contents. - This includes derivations, the outputs of [content-addressed derivations](#gloss-content-addressed-derivation), and the outputs of [fixed-output derivations](#gloss-fixed-output-derivation). + The location of a [store object] in the file system, i.e., an + immediate child of the Nix store directory. - - [substitute]{#gloss-substitute}\ - A substitute is a command invocation stored in the [Nix database] that - describes how to build a store object, bypassing the normal build - mechanism (i.e., derivations). Typically, the substitute builds the - store object by downloading a pre-built version of the store object - from some server. + Example: `/nix/store/a040m110amc4h71lds2jmr8qrkj2jhxd-git-2.38.1` - - [substituter]{#gloss-substituter}\ - An additional [store]{#gloss-store} from which Nix can obtain store objects instead of building them. - Often the substituter is a [binary cache](#gloss-binary-cache), but any store can serve as substituter. + [store path]: #gloss-store-path - See the [`substituters` configuration option](./command-ref/conf-file.md#conf-substituters) for details. +- [file system object]{#gloss-store-object} - [substituter]: #gloss-substituter + The Nix data model for representing simplified file system data. - - [purity]{#gloss-purity}\ - The assumption that equal Nix derivations when run always produce - the same output. This cannot be guaranteed in general (e.g., a - builder can rely on external inputs such as the network or the - system time) but the Nix model assumes it. + See [File System Object](@docroot@/architecture/file-system-object.md) for details. - - [Nix database]{#gloss-nix-database}\ - An SQlite database to track [reference]s between [store object]s. - This is an implementation detail of the [local store]. + [file system object]: #gloss-file-system-object - Default location: `/nix/var/nix/db`. +- [store object]{#gloss-store-object} - [Nix database]: #gloss-nix-database - - [Nix expression]{#gloss-nix-expression}\ - A high-level description of software packages and compositions - thereof. Deploying software using Nix entails writing Nix - expressions for your packages. Nix expressions are translated to - derivations that are stored in the Nix store. These derivations can - then be built. + A store object consists of a [file system object], [reference]s to other store objects, and other metadata. + It can be referred to by a [store path]. - - [reference]{#gloss-reference}\ - A [store object] `O` is said to have a *reference* to a store object `P` if a [store path] to `P` appears in the contents of `O`. + [store object]: #gloss-store-object - Store objects can refer to both other store objects and themselves. - References from a store object to itself are called *self-references*. - References other than a self-reference must not form a cycle. +- [input-addressed store object]{#gloss-input-addressed-store-object} - [reference]: #gloss-reference + A store object produced by building a + non-[content-addressed](#gloss-content-addressed-derivation), + non-[fixed-output](#gloss-fixed-output-derivation) + derivation. - - [reachable]{#gloss-reachable}\ - A store path `Q` is reachable from another store path `P` if `Q` - is in the *closure* of the *references* relation. +- [output-addressed store object]{#gloss-output-addressed-store-object} - - [closure]{#gloss-closure}\ - The closure of a store path is the set of store paths that are - directly or indirectly “reachable” from that store path; that is, - it’s the closure of the path under the *references* relation. For - a package, the closure of its derivation is equivalent to the - build-time dependencies, while the closure of its output path is - equivalent to its runtime dependencies. For correct deployment it - is necessary to deploy whole closures, since otherwise at runtime - files could be missing. The command `nix-store --query --requisites ` prints out - closures of store paths. + A [store object] whose [store path] is determined by its contents. + This includes derivations, the outputs of [content-addressed derivations](#gloss-content-addressed-derivation), and the outputs of [fixed-output derivations](#gloss-fixed-output-derivation). - As an example, if the [store object] at path `P` contains a [reference] - to a store object at path `Q`, then `Q` is in the closure of `P`. Further, if `Q` - references `R` then `R` is also in the closure of `P`. +- [substitute]{#gloss-substitute} - [closure]: #gloss-closure + A substitute is a command invocation stored in the [Nix database] that + describes how to build a store object, bypassing the normal build + mechanism (i.e., derivations). Typically, the substitute builds the + store object by downloading a pre-built version of the store object + from some server. - - [output path]{#gloss-output-path}\ - A [store path] produced by a [derivation]. +- [substituter]{#gloss-substituter} - [output path]: #gloss-output-path + An additional [store]{#gloss-store} from which Nix can obtain store objects instead of building them. + Often the substituter is a [binary cache](#gloss-binary-cache), but any store can serve as substituter. - - [deriver]{#gloss-deriver}\ - The [store derivation] that produced an [output path]. + See the [`substituters` configuration option](./command-ref/conf-file.md#conf-substituters) for details. - - [validity]{#gloss-validity}\ - A store path is valid if all [store object]s in its [closure] can be read from the [store]. + [substituter]: #gloss-substituter - For a [local store], this means: - - The store path leads to an existing [store object] in that [store]. - - The store path is listed in the [Nix database] as being valid. - - All paths in the store path's [closure] are valid. +- [purity]{#gloss-purity} - [validity]: #gloss-validity + The assumption that equal Nix derivations when run always produce + the same output. This cannot be guaranteed in general (e.g., a + builder can rely on external inputs such as the network or the + system time) but the Nix model assumes it. - - [user environment]{#gloss-user-env}\ - An automatically generated store object that consists of a set of - symlinks to “active” applications, i.e., other store paths. These - are generated automatically by - [`nix-env`](./command-ref/nix-env.md). See *profiles*. +- [Nix database]{#gloss-nix-database} - - [profile]{#gloss-profile}\ - A symlink to the current *user environment* of a user, e.g., - `/nix/var/nix/profiles/default`. + An SQlite database to track [reference]s between [store object]s. + This is an implementation detail of the [local store]. - - [installable]{#gloss-installable}\ - Something that can be realised in the Nix store. + Default location: `/nix/var/nix/db`. - See [installables](./command-ref/new-cli/nix.md#installables) for [`nix` commands](./command-ref/new-cli/nix.md) (experimental) for details. + [Nix database]: #gloss-nix-database - - [NAR]{#gloss-nar}\ - A *N*ix *AR*chive. This is a serialisation of a path in the Nix - store. It can contain regular files, directories and symbolic - links. NARs are generated and unpacked using `nix-store --dump` - and `nix-store --restore`. +- [Nix expression]{#gloss-nix-expression} - - [`∅`]{#gloss-emtpy-set}\ - The empty set symbol. In the context of profile history, this denotes a package is not present in a particular version of the profile. + A high-level description of software packages and compositions + thereof. Deploying software using Nix entails writing Nix + expressions for your packages. Nix expressions are translated to + derivations that are stored in the Nix store. These derivations can + then be built. - - [`ε`]{#gloss-epsilon}\ - The epsilon symbol. In the context of a package, this means the version is empty. More precisely, the derivation does not have a version attribute. +- [reference]{#gloss-reference} - - [string interpolation]{#gloss-string-interpolation}\ - Expanding expressions enclosed in `${ }` within a [string], [path], or [attribute name]. + A [store object] `O` is said to have a *reference* to a store object `P` if a [store path] to `P` appears in the contents of `O`. - See [String interpolation](./language/string-interpolation.md) for details. + Store objects can refer to both other store objects and themselves. + References from a store object to itself are called *self-references*. + References other than a self-reference must not form a cycle. - [string]: ./language/values.md#type-string - [path]: ./language/values.md#type-path - [attribute name]: ./language/values.md#attribute-set + [reference]: #gloss-reference - - [experimental feature]{#gloss-experimental-feature}\ - Not yet stabilized functionality guarded by named experimental feature flags. - These flags are enabled or disabled with the [`experimental-features`](./command-ref/conf-file.html#conf-experimental-features) setting. +- [reachable]{#gloss-reachable} - See the contribution guide on the [purpose and lifecycle of experimental feaures](@docroot@/contributing/experimental-features.md). + A store path `Q` is reachable from another store path `P` if `Q` + is in the *closure* of the *references* relation. + +- [closure]{#gloss-closure} + + The closure of a store path is the set of store paths that are + directly or indirectly “reachable” from that store path; that is, + it’s the closure of the path under the *references* relation. For + a package, the closure of its derivation is equivalent to the + build-time dependencies, while the closure of its output path is + equivalent to its runtime dependencies. For correct deployment it + is necessary to deploy whole closures, since otherwise at runtime + files could be missing. The command `nix-store --query --requisites ` prints out + closures of store paths. + + As an example, if the [store object] at path `P` contains a [reference] + to a store object at path `Q`, then `Q` is in the closure of `P`. Further, if `Q` + references `R` then `R` is also in the closure of `P`. + + [closure]: #gloss-closure + +- [output path]{#gloss-output-path} + + A [store path] produced by a [derivation]. + + [output path]: #gloss-output-path + +- [deriver]{#gloss-deriver} + + The [store derivation] that produced an [output path]. + +- [validity]{#gloss-validity} + + A store path is valid if all [store object]s in its [closure] can be read from the [store]. + + For a [local store], this means: + - The store path leads to an existing [store object] in that [store]. + - The store path is listed in the [Nix database] as being valid. + - All paths in the store path's [closure] are valid. + + [validity]: #gloss-validity + +- [user environment]{#gloss-user-env} + + An automatically generated store object that consists of a set of + symlinks to “active” applications, i.e., other store paths. These + are generated automatically by + [`nix-env`](./command-ref/nix-env.md). See *profiles*. + +- [profile]{#gloss-profile} + + A symlink to the current *user environment* of a user, e.g., + `/nix/var/nix/profiles/default`. + +- [installable]{#gloss-installable} + + Something that can be realised in the Nix store. + + See [installables](./command-ref/new-cli/nix.md#installables) for [`nix` commands](./command-ref/new-cli/nix.md) (experimental) for details. + +- [NAR]{#gloss-nar} + + A *N*ix *AR*chive. This is a serialisation of a path in the Nix + store. It can contain regular files, directories and symbolic + links. NARs are generated and unpacked using `nix-store --dump` + and `nix-store --restore`. + +- [`∅`]{#gloss-emtpy-set} + + The empty set symbol. In the context of profile history, this denotes a package is not present in a particular version of the profile. + +- [`ε`]{#gloss-epsilon} + + The epsilon symbol. In the context of a package, this means the version is empty. More precisely, the derivation does not have a version attribute. + +- [string interpolation]{#gloss-string-interpolation} + + Expanding expressions enclosed in `${ }` within a [string], [path], or [attribute name]. + + See [String interpolation](./language/string-interpolation.md) for details. + + [string]: ./language/values.md#type-string + [path]: ./language/values.md#type-path + [attribute name]: ./language/values.md#attribute-set + +- [experimental feature]{#gloss-experimental-feature} + + Not yet stabilized functionality guarded by named experimental feature flags. + These flags are enabled or disabled with the [`experimental-features`](./command-ref/conf-file.html#conf-experimental-features) setting. + + See the contribution guide on the [purpose and lifecycle of experimental feaures](@docroot@/contributing/experimental-features.md). From 2f5d3da8062ae58242a8de2bad470a66478edea4 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 25 Aug 2023 09:53:12 -0400 Subject: [PATCH 11/20] Introduce `OutputName` and `OutputNameView` type aliases Hopefully they make the code easier to understand! --- src/libexpr/primops.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/build/local-derivation-goal.hh | 2 +- src/libstore/derivations.cc | 12 +++++----- src/libstore/derivations.hh | 12 +++++----- src/libstore/derived-path.cc | 4 ++-- src/libstore/derived-path.hh | 4 ++-- src/libstore/downstream-placeholder.cc | 4 ++-- src/libstore/downstream-placeholder.hh | 4 ++-- src/libstore/outputs-spec.hh | 26 +++++++++++++++------ src/libstore/realisation.hh | 6 ++--- 11 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 5067da449..6b99b91e4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1843,7 +1843,7 @@ static void prim_outputOf(EvalState & state, const PosIdx pos, Value * * args, V { SingleDerivedPath drvPath = state.coerceToSingleDerivedPath(pos, *args[0], "while evaluating the first argument to builtins.outputOf"); - std::string_view outputName = state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument to builtins.outputOf"); + OutputNameView outputName = state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument to builtins.outputOf"); state.mkSingleDerivedPathString( SingleDerivedPath::Built { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 78f943d1f..64b55ca6a 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2955,7 +2955,7 @@ bool LocalDerivationGoal::isReadDesc(int fd) } -StorePath LocalDerivationGoal::makeFallbackPath(std::string_view outputName) +StorePath LocalDerivationGoal::makeFallbackPath(OutputNameView outputName) { return worker.store.makeStorePath( "rewrite:" + std::string(drvPath.to_string()) + ":name:" + std::string(outputName), diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 8827bfca3..0a05081c7 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -297,7 +297,7 @@ struct LocalDerivationGoal : public DerivationGoal * @todo Add option to randomize, so we can audit whether our * rewrites caught everything */ - StorePath makeFallbackPath(std::string_view outputName); + StorePath makeFallbackPath(OutputNameView outputName); }; } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 0b8bdaf1c..dc32c3847 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -12,7 +12,7 @@ namespace nix { -std::optional DerivationOutput::path(const Store & store, std::string_view drvName, std::string_view outputName) const +std::optional DerivationOutput::path(const Store & store, std::string_view drvName, OutputNameView outputName) const { return std::visit(overloaded { [](const DerivationOutput::InputAddressed & doi) -> std::optional { @@ -36,7 +36,7 @@ std::optional DerivationOutput::path(const Store & store, std::string } -StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const +StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, OutputNameView outputName) const { return store.makeFixedOutputPathFromCA( outputPathName(drvName, outputName), @@ -466,7 +466,7 @@ bool isDerivation(std::string_view fileName) } -std::string outputPathName(std::string_view drvName, std::string_view outputName) { +std::string outputPathName(std::string_view drvName, OutputNameView outputName) { std::string res { drvName }; if (outputName != "out") { res += "-"; @@ -810,7 +810,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr } -std::string hashPlaceholder(const std::string_view outputName) +std::string hashPlaceholder(const OutputNameView outputName) { // FIXME: memoize? return "/" + hashString(htSHA256, concatStrings("nix-output:", outputName)).to_string(Base32, false); @@ -963,7 +963,7 @@ void Derivation::checkInvariants(Store & store, const StorePath & drvPath) const const Hash impureOutputHash = hashString(htSHA256, "impure"); nlohmann::json DerivationOutput::toJSON( - const Store & store, std::string_view drvName, std::string_view outputName) const + const Store & store, std::string_view drvName, OutputNameView outputName) const { nlohmann::json res = nlohmann::json::object(); std::visit(overloaded { @@ -990,7 +990,7 @@ nlohmann::json DerivationOutput::toJSON( DerivationOutput DerivationOutput::fromJSON( - const Store & store, std::string_view drvName, std::string_view outputName, + const Store & store, std::string_view drvName, OutputNameView outputName, const nlohmann::json & _json, const ExperimentalFeatureSettings & xpSettings) { diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index a92082089..106056f2d 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -55,7 +55,7 @@ struct DerivationOutput * @param drvName The name of the derivation this is an output of, without the `.drv`. * @param outputName The name of this output. */ - StorePath path(const Store & store, std::string_view drvName, std::string_view outputName) const; + StorePath path(const Store & store, std::string_view drvName, OutputNameView outputName) const; GENERATE_CMP(CAFixed, me->ca); }; @@ -132,19 +132,19 @@ struct DerivationOutput * the safer interface provided by * BasicDerivation::outputsAndOptPaths */ - std::optional path(const Store & store, std::string_view drvName, std::string_view outputName) const; + std::optional path(const Store & store, std::string_view drvName, OutputNameView outputName) const; nlohmann::json toJSON( const Store & store, std::string_view drvName, - std::string_view outputName) const; + OutputNameView outputName) const; /** * @param xpSettings Stop-gap to avoid globals during unit tests. */ static DerivationOutput fromJSON( const Store & store, std::string_view drvName, - std::string_view outputName, + OutputNameView outputName, const nlohmann::json & json, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); }; @@ -405,7 +405,7 @@ bool isDerivation(std::string_view fileName); * This is usually -, but is just when * the output name is "out". */ -std::string outputPathName(std::string_view drvName, std::string_view outputName); +std::string outputPathName(std::string_view drvName, OutputNameView outputName); /** @@ -499,7 +499,7 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr * own outputs without needing to use the hash of a derivation in * itself, making the hash near-impossible to calculate. */ -std::string hashPlaceholder(const std::string_view outputName); +std::string hashPlaceholder(const OutputNameView outputName); extern const Hash impureOutputHash; diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 3594b7570..47d784deb 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -167,7 +167,7 @@ void drvRequireExperiment( SingleDerivedPath::Built SingleDerivedPath::Built::parse( const Store & store, ref drv, - std::string_view output, + OutputNameView output, const ExperimentalFeatureSettings & xpSettings) { drvRequireExperiment(*drv, xpSettings); @@ -179,7 +179,7 @@ SingleDerivedPath::Built SingleDerivedPath::Built::parse( DerivedPath::Built DerivedPath::Built::parse( const Store & store, ref drv, - std::string_view outputsS, + OutputNameView outputsS, const ExperimentalFeatureSettings & xpSettings) { drvRequireExperiment(*drv, xpSettings); diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index ec30dac61..4d7033df2 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -42,7 +42,7 @@ struct SingleDerivedPath; */ struct SingleDerivedPathBuilt { ref drvPath; - std::string output; + OutputName output; /** * Get the store path this is ultimately derived from (by realising @@ -71,7 +71,7 @@ struct SingleDerivedPathBuilt { */ static SingleDerivedPathBuilt parse( const Store & store, ref drvPath, - std::string_view outputs, + OutputNameView outputs, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); nlohmann::json toJSON(Store & store) const; diff --git a/src/libstore/downstream-placeholder.cc b/src/libstore/downstream-placeholder.cc index d951b7b7d..7e3f7548d 100644 --- a/src/libstore/downstream-placeholder.cc +++ b/src/libstore/downstream-placeholder.cc @@ -11,7 +11,7 @@ std::string DownstreamPlaceholder::render() const DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput( const StorePath & drvPath, - std::string_view outputName, + OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings) { xpSettings.require(Xp::CaDerivations); @@ -25,7 +25,7 @@ DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput( DownstreamPlaceholder DownstreamPlaceholder::unknownDerivation( const DownstreamPlaceholder & placeholder, - std::string_view outputName, + OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings) { xpSettings.require(Xp::DynamicDerivations); diff --git a/src/libstore/downstream-placeholder.hh b/src/libstore/downstream-placeholder.hh index d58a2ac14..c911ecea2 100644 --- a/src/libstore/downstream-placeholder.hh +++ b/src/libstore/downstream-placeholder.hh @@ -58,7 +58,7 @@ public: */ static DownstreamPlaceholder unknownCaOutput( const StorePath & drvPath, - std::string_view outputName, + OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); /** @@ -72,7 +72,7 @@ public: */ static DownstreamPlaceholder unknownDerivation( const DownstreamPlaceholder & drvPlaceholder, - std::string_view outputName, + OutputNameView outputName, const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings); /** diff --git a/src/libstore/outputs-spec.hh b/src/libstore/outputs-spec.hh index ae19f1040..1ef99a5fc 100644 --- a/src/libstore/outputs-spec.hh +++ b/src/libstore/outputs-spec.hh @@ -13,24 +13,36 @@ namespace nix { +/** + * An (owned) output name. Just a type alias used to make code more + * readible. + */ +typedef std::string OutputName; + +/** + * A borrowed output name. Just a type alias used to make code more + * readible. + */ +typedef std::string_view OutputNameView; + struct OutputsSpec { /** * A non-empty set of outputs, specified by name */ - struct Names : std::set { - using std::set::set; + struct Names : std::set { + using std::set::set; /* These need to be "inherited manually" */ - Names(const std::set & s) - : std::set(s) + Names(const std::set & s) + : std::set(s) { assert(!empty()); } /** * Needs to be "inherited manually" */ - Names(std::set && s) - : std::set(s) + Names(std::set && s) + : std::set(s) { assert(!empty()); } /* This set should always be non-empty, so we delete this @@ -57,7 +69,7 @@ struct OutputsSpec { */ OutputsSpec() = delete; - bool contains(const std::string & output) const; + bool contains(const OutputName & output) const; /** * Create a new OutputsSpec which is the union of this and that. diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 0548b30c1..559483ce3 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -34,7 +34,7 @@ struct DrvOutput { /** * The name of the output. */ - std::string outputName; + OutputName outputName; std::string to_string() const; @@ -84,7 +84,7 @@ struct Realisation { * Since these are the outputs of a single derivation, we know the * output names are unique so we can use them as the map key. */ -typedef std::map SingleDrvOutputs; +typedef std::map SingleDrvOutputs; /** * Collection type for multiple derivations' outputs' `Realisation`s. @@ -146,7 +146,7 @@ public: MissingRealisation(DrvOutput & outputId) : MissingRealisation(outputId.outputName, outputId.strHash()) {} - MissingRealisation(std::string_view drv, std::string outputName) + MissingRealisation(std::string_view drv, OutputName outputName) : Error( "cannot operate on output '%s' of the " "unbuilt derivation '%s'", outputName, From 1c4caef14b51dbb4f749c45311c8e2c9acb75a60 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 16 Aug 2023 00:05:35 -0400 Subject: [PATCH 12/20] Throw `MissingRealisation` not plain `Error` in both `resolveDerivedPath` Now we are consistent with the other `resolveDerivedPath`, and other such functions. --- src/libstore/misc.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 631213306..c043b9b93 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -399,8 +399,7 @@ StorePath resolveDerivedPath(Store & store, const SingleDerivedPath & req, Store store.printStorePath(drvPath), bfd.output); auto & optPath = outputPaths.at(bfd.output); if (!optPath) - throw Error("'%s' does not yet map to a known concrete store path", - bfd.to_string(store)); + throw MissingRealisation(bfd.drvPath->to_string(store), bfd.output); return *optPath; }, }, req.raw()); From 692074f7142fcf8ede1266b6d8cbbd5feaf3221f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 15 Jan 2023 17:47:24 -0500 Subject: [PATCH 13/20] Use `Worker::makeDerivationGoal` less We're about to split up `DerivationGoal` a bit. At that point `makeDerivationGoal` will mean something more specific than it does today. (Perhaps a future rename will make this clearer.) On the other hand, the more public `Worker::makeGoal` function will continue to work exactly as before. So by moving some call sites to use that instead, we preemptively avoid issues in the next step. --- src/libstore/build/derivation-goal.cc | 14 ++++++++++++-- src/libstore/build/entry-points.cc | 7 +++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 84da7f2e1..ea56c0288 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -380,7 +380,12 @@ void DerivationGoal::gaveUpOnSubstitution() worker.store.printStorePath(i.first)); } - addWaitee(worker.makeDerivationGoal(i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal)); + addWaitee(worker.makeGoal( + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(i.first), + .outputs = i.second, + }, + buildMode == bmRepair ? bmRepair : bmNormal)); } /* Copy the input sources from the eval store to the build @@ -452,7 +457,12 @@ void DerivationGoal::repairClosure() if (drvPath2 == outputsToDrv.end()) addWaitee(upcast_goal(worker.makePathSubstitutionGoal(i, Repair))); else - addWaitee(worker.makeDerivationGoal(drvPath2->second, OutputsSpec::All(), bmRepair)); + addWaitee(worker.makeGoal( + DerivedPath::Built { + .drvPath = makeConstantStorePathRef(drvPath2->second), + .outputs = OutputsSpec::All { }, + }, + bmRepair)); } if (waitees.empty()) { diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index e941b4e65..f71fb35a6 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -124,8 +124,11 @@ void Store::repairPath(const StorePath & path) auto info = queryPathInfo(path); if (info->deriver && isValidPath(*info->deriver)) { goals.clear(); - // FIXME: Should just build the specific output we need. - goals.insert(worker.makeDerivationGoal(*info->deriver, OutputsSpec::All { }, bmRepair)); + goals.insert(worker.makeGoal(DerivedPath::Built { + .drvPath = makeConstantStorePathRef(*info->deriver), + // FIXME: Should just build the specific output we need. + .outputs = OutputsSpec::All { }, + }, bmRepair)); worker.run(goals); } else throw Error(worker.failingExitStatus(), "cannot repair path '%s'", printStorePath(path)); From 5e3986f59cb58f48186a49dcec7aa317b4787522 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 8 Mar 2021 16:24:49 -0500 Subject: [PATCH 14/20] Adapt scheduler to work with dynamic derivations To avoid dealing with an optional `drvPath` (because we might not know it yet) everywhere, make an `CreateDerivationAndRealiseGoal`. This goal just builds/substitutes the derivation file, and then kicks of a build for that obtained derivation; in other words it does the chaining of goals when the drv file is missing (as can already be the case) or computed (new case). This also means the `getDerivation` state can be removed from `DerivationGoal`, which makes the `BasicDerivation` / in memory case and `Derivation` / drv file file case closer together. The map type is factored out for clarity, and because we will soon hvae a second use for it (`Derivation` itself). Co-authored-by: Robert Hensing --- .../create-derivation-and-realise-goal.cc | 157 ++++++++++++++++++ .../create-derivation-and-realise-goal.hh | 96 +++++++++++ src/libstore/build/derivation-goal.cc | 22 +-- src/libstore/build/derivation-goal.hh | 15 +- .../build/drv-output-substitution-goal.hh | 4 +- src/libstore/build/entry-points.cc | 11 +- src/libstore/build/goal.cc | 2 +- src/libstore/build/goal.hh | 22 ++- src/libstore/build/substitution-goal.hh | 4 +- src/libstore/build/worker.cc | 118 ++++++++++--- src/libstore/build/worker.hh | 22 +++ src/libstore/derived-path-map.cc | 33 ++++ src/libstore/derived-path-map.hh | 73 ++++++++ tests/dyn-drv/build-built-drv.sh | 4 +- 14 files changed, 525 insertions(+), 58 deletions(-) create mode 100644 src/libstore/build/create-derivation-and-realise-goal.cc create mode 100644 src/libstore/build/create-derivation-and-realise-goal.hh create mode 100644 src/libstore/derived-path-map.cc create mode 100644 src/libstore/derived-path-map.hh diff --git a/src/libstore/build/create-derivation-and-realise-goal.cc b/src/libstore/build/create-derivation-and-realise-goal.cc new file mode 100644 index 000000000..b01042f00 --- /dev/null +++ b/src/libstore/build/create-derivation-and-realise-goal.cc @@ -0,0 +1,157 @@ +#include "create-derivation-and-realise-goal.hh" +#include "worker.hh" + +namespace nix { + +CreateDerivationAndRealiseGoal::CreateDerivationAndRealiseGoal(ref drvReq, + const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, DerivedPath::Built { .drvPath = drvReq, .outputs = wantedOutputs }) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + state = &CreateDerivationAndRealiseGoal::getDerivation; + name = fmt( + "outer obtaining drv from '%s' and then building outputs %s", + drvReq->to_string(worker.store), + std::visit(overloaded { + [&](const OutputsSpec::All) -> std::string { + return "* (all of them)"; + }, + [&](const OutputsSpec::Names os) { + return concatStringsSep(", ", quoteStrings(os)); + }, + }, wantedOutputs.raw)); + trace("created outer"); + + worker.updateProgress(); +} + + +CreateDerivationAndRealiseGoal::~CreateDerivationAndRealiseGoal() +{ +} + + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit(overloaded { + [&](const SingleDerivedPath::Opaque & bo) { + return bo.path; + }, + [&](const SingleDerivedPath::Built & bfd) { + return pathPartOfReq(*bfd.drvPath); + }, + }, req.raw()); +} + + +std::string CreateDerivationAndRealiseGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals and inner derivation goals always happen before + derivation goals (due to "b$"). */ + return "c$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); +} + + +void CreateDerivationAndRealiseGoal::timedOut(Error && ex) +{ +} + + +void CreateDerivationAndRealiseGoal::work() +{ + (this->*state)(); +} + + +void CreateDerivationAndRealiseGoal::addWantedOutputs(const OutputsSpec & outputs) +{ + /* If we already want all outputs, there is nothing to do. */ + auto newWanted = wantedOutputs.union_(outputs); + bool needRestart = !newWanted.isSubsetOf(wantedOutputs); + wantedOutputs = newWanted; + + if (!needRestart) return; + + if (!optDrvPath) + // haven't started steps where the outputs matter yet + return; + worker.makeDerivationGoal(*optDrvPath, outputs, buildMode); +} + + +void CreateDerivationAndRealiseGoal::getDerivation() +{ + trace("outer init"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be created through a + substitute. */ + if (auto optDrvPath = [this]() -> std::optional { + if (buildMode != bmNormal) return std::nullopt; + + auto drvPath = StorePath::dummy; + try { + drvPath = resolveDerivedPath(worker.store, *drvReq); + } catch (MissingRealisation) { + return std::nullopt; + } + return worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath) + ? std::optional { drvPath } + : std::nullopt; + }()) { + trace(fmt("already have drv '%s' for '%s', can go straight to building", + worker.store.printStorePath(*optDrvPath), + drvReq->to_string(worker.store))); + + loadAndBuildDerivation(); + } else { + trace("need to obtain drv we want to build"); + + addWaitee(worker.makeGoal(DerivedPath::fromSingle(*drvReq))); + + state = &CreateDerivationAndRealiseGoal::loadAndBuildDerivation; + if (waitees.empty()) work(); + } +} + + +void CreateDerivationAndRealiseGoal::loadAndBuildDerivation() +{ + trace("outer load and build derivation"); + + if (nrFailed != 0) { + amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + return; + } + + StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); + /* Build this step! */ + concreteDrvGoal = worker.makeDerivationGoal(drvPath, wantedOutputs, buildMode); + addWaitee(upcast_goal(concreteDrvGoal)); + state = &CreateDerivationAndRealiseGoal::buildDone; + optDrvPath = std::move(drvPath); + if (waitees.empty()) work(); +} + + +void CreateDerivationAndRealiseGoal::buildDone() +{ + trace("outer build done"); + + buildResult = upcast_goal(concreteDrvGoal)->getBuildResult(DerivedPath::Built { + .drvPath = drvReq, + .outputs = wantedOutputs, + }); + + if (buildResult.success()) + amDone(ecSuccess); + else + amDone(ecFailed, Error("building '%s' failed", drvReq->to_string(worker.store))); +} + + +} diff --git a/src/libstore/build/create-derivation-and-realise-goal.hh b/src/libstore/build/create-derivation-and-realise-goal.hh new file mode 100644 index 000000000..ca936fc95 --- /dev/null +++ b/src/libstore/build/create-derivation-and-realise-goal.hh @@ -0,0 +1,96 @@ +#pragma once + +#include "parsed-derivations.hh" +#include "lock.hh" +#include "store-api.hh" +#include "pathlocks.hh" +#include "goal.hh" + +namespace nix { + +struct DerivationGoal; + +/** + * This goal type is essentially the serial composition (like function + * composition) of a goal for getting a derivation, and then a + * `DerivationGoal` using the newly-obtained derivation. + * + * In the (currently experimental) general inductive case of derivations + * that are themselves build outputs, that first goal will be *another* + * `CreateDerivationAndRealiseGoal`. In the (much more common) base-case + * where the derivation has no provence and is just referred to by + * (content-addressed) store path, that first goal is a + * `SubstitutionGoal`. + * + * If we already have the derivation (e.g. if the evalutator has created + * the derivation locally and then instructured the store to build it), + * we can skip the first goal entirely as a small optimization. + */ +struct CreateDerivationAndRealiseGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The path of the derivation, once obtained. + **/ + std::optional optDrvPath; + + /** + * The goal for the corresponding concrete derivation. + **/ + std::shared_ptr concreteDrvGoal; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + typedef void (CreateDerivationAndRealiseGoal::*GoalState)(); + GoalState state; + + /** + * The final output paths of the build. + * + * - For input-addressed derivations, always the precomputed paths + * + * - For content-addressed derivations, calcuated from whatever the + * hash ends up being. (Note that fixed outputs derivations that + * produce the "wrong" output still install that data under its + * true content-address.) + */ + OutputPathMap finalOutputs; + + BuildMode buildMode; + + CreateDerivationAndRealiseGoal(ref drvReq, + const OutputsSpec & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); + virtual ~CreateDerivationAndRealiseGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + void work() override; + + /** + * Add wanted outputs to an already existing derivation goal. + */ + void addWantedOutputs(const OutputsSpec & outputs); + + /** + * The states. + */ + void getDerivation(); + void loadAndBuildDerivation(); + void buildDone(); + + JobCategory jobCategory() const override { + return JobCategory::Administration; + }; +}; + +} diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index ea56c0288..bec0bc538 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -71,7 +71,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, , wantedOutputs(wantedOutputs) , buildMode(buildMode) { - state = &DerivationGoal::getDerivation; + state = &DerivationGoal::loadDerivation; name = fmt( "building of '%s' from .drv file", DerivedPath::Built { makeConstantStorePathRef(drvPath), wantedOutputs }.to_string(worker.store)); @@ -164,24 +164,6 @@ void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) } -void DerivationGoal::getDerivation() -{ - trace("init"); - - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be created through a - substitute. */ - if (buildMode == bmNormal && worker.evalStore.isValidPath(drvPath)) { - loadDerivation(); - return; - } - - addWaitee(upcast_goal(worker.makePathSubstitutionGoal(drvPath))); - - state = &DerivationGoal::loadDerivation; -} - - void DerivationGoal::loadDerivation() { trace("loading derivation"); @@ -1493,7 +1475,7 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result) if (!useDerivation) return; auto & fullDrv = *dynamic_cast(drv.get()); - auto * dg = dynamic_cast(&*waitee); + auto * dg = tryGetConcreteDrvGoal(waitee); if (!dg) return; auto outputs = fullDrv.inputDrvs.find(dg->drvPath); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 9d6fe1c0f..62b122c27 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -50,6 +50,13 @@ struct InitialOutput { std::optional known; }; +/** + * A goal for building some or all of the outputs of a derivation. + * + * The derivation must already be present, either in the store in a drv + * or in memory. If the derivation itself needs to be gotten first, a + * `CreateDerivationAndRealiseGoal` goal must be used instead. + */ struct DerivationGoal : public Goal { /** @@ -66,8 +73,7 @@ struct DerivationGoal : public Goal std::shared_ptr resolvedDrvGoal; /** - * The specific outputs that we need to build. Empty means all of - * them. + * The specific outputs that we need to build. */ OutputsSpec wantedOutputs; @@ -229,7 +235,6 @@ struct DerivationGoal : public Goal /** * The states. */ - void getDerivation(); void loadDerivation(); void haveDerivation(); void outputsSubstitutionTried(); @@ -334,7 +339,9 @@ struct DerivationGoal : public Goal StorePathSet exportReferences(const StorePathSet & storePaths); - JobCategory jobCategory() override { return JobCategory::Build; }; + JobCategory jobCategory() const override { + return JobCategory::Build; + }; }; MakeError(NotDeterministic, BuildError); diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 5d1253a71..da2426e5e 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -73,7 +73,9 @@ public: void work() override; void handleEOF(int fd) override; - JobCategory jobCategory() override { return JobCategory::Substitution; }; + JobCategory jobCategory() const override { + return JobCategory::Substitution; + }; }; } diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index f71fb35a6..f0f0e5519 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,5 +1,6 @@ #include "worker.hh" #include "substitution-goal.hh" +#include "create-derivation-and-realise-goal.hh" #include "derivation-goal.hh" #include "local-store.hh" @@ -15,7 +16,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod worker.run(goals); - StorePathSet failed; + StringSet failed; std::optional ex; for (auto & i : goals) { if (i->ex) { @@ -25,8 +26,10 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { - if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvPath); - else if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->storePath); + if (auto i2 = dynamic_cast(i.get())) + failed.insert(i2->drvReq->to_string(*this)); + else if (auto i2 = dynamic_cast(i.get())) + failed.insert(printStorePath(i2->storePath)); } } @@ -35,7 +38,7 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod throw std::move(*ex); } else if (!failed.empty()) { if (ex) logError(ex->info()); - throw Error(worker.failingExitStatus(), "build of %s failed", showPaths(failed)); + throw Error(worker.failingExitStatus(), "build of %s failed", concatStringsSep(", ", quoteStrings(failed))); } } diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index ca7097a68..f8db98280 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -11,7 +11,7 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { } -BuildResult Goal::getBuildResult(const DerivedPath & req) { +BuildResult Goal::getBuildResult(const DerivedPath & req) const { BuildResult res { buildResult }; if (auto pbp = std::get_if(&req)) { diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index d3127caea..01d3c3491 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -41,8 +41,24 @@ typedef std::map WeakGoalMap; * of each category in parallel. */ enum struct JobCategory { + /** + * A build of a derivation; it will use CPU and disk resources. + */ Build, + /** + * A substitution an arbitrary store object; it will use network resources. + */ Substitution, + /** + * A goal that does no "real" work by itself, and just exists to depend on + * other goals which *do* do real work. These goals therefore are not + * limited. + * + * These goals cannot infinitely create themselves, so there is no risk of + * a "fork bomb" type situation (which would be a problem even though the + * goal do no real work) either. + */ + Administration, }; struct Goal : public std::enable_shared_from_this @@ -110,7 +126,7 @@ public: * sake of both privacy and determinism, and this "safe accessor" * ensures we don't. */ - BuildResult getBuildResult(const DerivedPath &); + BuildResult getBuildResult(const DerivedPath &) const; /** * Exception containing an error message, if any. @@ -144,7 +160,7 @@ public: void trace(std::string_view s); - std::string getName() + std::string getName() const { return name; } @@ -166,7 +182,7 @@ public: * @brief Hint for the scheduler, which concurrency limit applies. * @see JobCategory */ - virtual JobCategory jobCategory() = 0; + virtual JobCategory jobCategory() const = 0; }; void addToWeakGoals(WeakGoals & goals, GoalPtr p); diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 1b693baa1..1d389d328 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -117,7 +117,9 @@ public: /* Called by destructor, can't be overridden */ void cleanup() override final; - JobCategory jobCategory() override { return JobCategory::Substitution; }; + JobCategory jobCategory() const override { + return JobCategory::Substitution; + }; }; } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index b58fc5c1c..f65f63b99 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -2,6 +2,7 @@ #include "worker.hh" #include "substitution-goal.hh" #include "drv-output-substitution-goal.hh" +#include "create-derivation-and-realise-goal.hh" #include "local-derivation-goal.hh" #include "hook-instance.hh" @@ -41,6 +42,24 @@ Worker::~Worker() } +std::shared_ptr Worker::makeCreateDerivationAndRealiseGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + BuildMode buildMode) +{ + std::weak_ptr & goal_weak = outerDerivationGoals.ensureSlot(*drvReq).value; + std::shared_ptr goal = goal_weak.lock(); + if (!goal) { + goal = std::make_shared(drvReq, wantedOutputs, *this, buildMode); + goal_weak = goal; + wakeUp(goal); + } else { + goal->addWantedOutputs(wantedOutputs); + } + return goal; +} + + std::shared_ptr Worker::makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, @@ -111,10 +130,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - if (auto bop = std::get_if(&*bfd.drvPath)) - return makeDerivationGoal(bop->path, bfd.outputs, buildMode); - else - throw UnimplementedError("Building dynamic derivations in one shot is not yet implemented."); + return makeCreateDerivationAndRealiseGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -123,24 +139,46 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) } +template +static void cullMap(std::map & goalMap, F f) +{ + for (auto i = goalMap.begin(); i != goalMap.end();) + if (!f(i->second)) + i = goalMap.erase(i); + else ++i; +} + + template static void removeGoal(std::shared_ptr goal, std::map> & goalMap) { /* !!! inefficient */ - for (auto i = goalMap.begin(); - i != goalMap.end(); ) - if (i->second.lock() == goal) { - auto j = i; ++j; - goalMap.erase(i); - i = j; - } - else ++i; + cullMap(goalMap, [&](const std::weak_ptr & gp) -> bool { + return gp.lock() != goal; + }); +} + +template +static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap); + +template +static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap) +{ + /* !!! inefficient */ + cullMap(goalMap, [&](DerivedPathMap>::ChildNode & node) -> bool { + if (node.value.lock() == goal) + node.value.reset(); + removeGoal(goal, node.childMap); + return !node.value.expired() || !node.childMap.empty(); + }); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, outerDerivationGoals.map); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvGoal, derivationGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(subGoal, substitutionGoals); @@ -198,8 +236,19 @@ void Worker::childStarted(GoalPtr goal, const std::set & fds, child.respectTimeouts = respectTimeouts; children.emplace_back(child); if (inBuildSlot) { - if (goal->jobCategory() == JobCategory::Substitution) nrSubstitutions++; - else nrLocalBuilds++; + switch (goal->jobCategory()) { + case JobCategory::Substitution: + nrSubstitutions++; + break; + case JobCategory::Build: + nrLocalBuilds++; + break; + case JobCategory::Administration: + /* Intentionally not limited, see docs */ + break; + default: + abort(); + } } } @@ -211,12 +260,20 @@ void Worker::childTerminated(Goal * goal, bool wakeSleepers) if (i == children.end()) return; if (i->inBuildSlot) { - if (goal->jobCategory() == JobCategory::Substitution) { + switch (goal->jobCategory()) { + case JobCategory::Substitution: assert(nrSubstitutions > 0); nrSubstitutions--; - } else { + break; + case JobCategory::Build: assert(nrLocalBuilds > 0); nrLocalBuilds--; + break; + case JobCategory::Administration: + /* Intentionally not limited, see docs */ + break; + default: + abort(); } } @@ -267,9 +324,9 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); - if (auto goal = dynamic_cast(i.get())) { + if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(goal->drvPath), + .drvPath = goal->drvReq, .outputs = goal->wantedOutputs, }); } else if (auto goal = dynamic_cast(i.get())) { @@ -522,11 +579,26 @@ void Worker::markContentsGood(const StorePath & path) } -GoalPtr upcast_goal(std::shared_ptr subGoal) { - return subGoal; -} -GoalPtr upcast_goal(std::shared_ptr subGoal) { +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ return subGoal; } +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + +GoalPtr upcast_goal(std::shared_ptr subGoal) +{ + return subGoal; +} + +const DerivationGoal * tryGetConcreteDrvGoal(GoalPtr waitee) +{ + auto * odg = dynamic_cast(&*waitee); + if (!odg) return nullptr; + return &*odg->concreteDrvGoal; +} + } diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 5abceca0d..a778e311c 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -4,6 +4,7 @@ #include "types.hh" #include "lock.hh" #include "store-api.hh" +#include "derived-path-map.hh" #include "goal.hh" #include "realisation.hh" @@ -13,6 +14,7 @@ namespace nix { /* Forward definition. */ +struct CreateDerivationAndRealiseGoal; struct DerivationGoal; struct PathSubstitutionGoal; class DrvOutputSubstitutionGoal; @@ -31,9 +33,23 @@ class DrvOutputSubstitutionGoal; */ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; +/** + * The current implementation of impure derivations has + * `DerivationGoal`s accumulate realisations from their waitees. + * Unfortunately, `DerivationGoal`s don't directly depend on other + * goals, but instead depend on `CreateDerivationAndRealiseGoal`s. + * + * We try not to share any of the details of any goal type with any + * other, for sake of modularity and quicker rebuilds. This means we + * cannot "just" downcast and fish out the field. So as an escape hatch, + * we have made the function, written in `worker.cc` where all the goal + * types are visible, and use it instead. + */ +const DerivationGoal * tryGetConcreteDrvGoal(GoalPtr waitee); /** * A mapping used to remember for each child process to what goal it @@ -102,6 +118,9 @@ private: * Maps used to prevent multiple instantiations of a goal for the * same derivation / path. */ + + DerivedPathMap> outerDerivationGoals; + std::map> derivationGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -189,6 +208,9 @@ public: * @ref DerivationGoal "derivation goal" */ private: + std::shared_ptr makeCreateDerivationAndRealiseGoal( + ref drvPath, + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); std::shared_ptr makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, std::function()> mkDrvGoal); diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc new file mode 100644 index 000000000..5c8c7a4f2 --- /dev/null +++ b/src/libstore/derived-path-map.cc @@ -0,0 +1,33 @@ +#include "derived-path-map.hh" + +namespace nix { + +template +typename DerivedPathMap::ChildNode & DerivedPathMap::ensureSlot(const SingleDerivedPath & k) +{ + std::function initIter; + initIter = [&](const auto & k) -> auto & { + return std::visit(overloaded { + [&](const SingleDerivedPath::Opaque & bo) -> auto & { + // will not overwrite if already there + return map[bo.path]; + }, + [&](const SingleDerivedPath::Built & bfd) -> auto & { + auto & n = initIter(*bfd.drvPath); + return n.childMap[bfd.output]; + }, + }, k.raw()); + }; + return initIter(k); +} + +} + +// instantiations + +#include "create-derivation-and-realise-goal.hh" +namespace nix { + +template struct DerivedPathMap>; + +} diff --git a/src/libstore/derived-path-map.hh b/src/libstore/derived-path-map.hh new file mode 100644 index 000000000..9ce58206e --- /dev/null +++ b/src/libstore/derived-path-map.hh @@ -0,0 +1,73 @@ +#pragma once + +#include "types.hh" +#include "derived-path.hh" + +namespace nix { + +/** + * A simple Trie, of sorts. Conceptually a map of `SingleDerivedPath` to + * values. + * + * Concretely, an n-ary tree, as described below. A + * `SingleDerivedPath::Opaque` maps to the value of an immediate child + * of the root node. A `SingleDerivedPath::Built` maps to a deeper child + * node: the `SingleDerivedPath::Built::drvPath` is first mapped to a a + * child node (inductively), and then the + * `SingleDerivedPath::Built::output` is used to look up that child's + * child via its map. In this manner, every `SingleDerivedPath` is + * mapped to a child node. + * + * @param V A type to instantiate for each output. It should probably + * should be an "optional" type so not every interior node has to have a + * value. For example, the scheduler uses + * `DerivedPathMap>` to + * remember which goals correspond to which outputs. `* const Something` + * or `std::optional` would also be good choices for + * "optional" types. + */ +template +struct DerivedPathMap { + /** + * A child node (non-root node). + */ + struct ChildNode { + /** + * Value of this child node. + * + * @see DerivedPathMap for what `V` should be. + */ + V value; + + /** + * The map type for the root node. + */ + using Map = std::map; + + /** + * The map of the root node. + */ + Map childMap; + }; + + /** + * The map type for the root node. + */ + using Map = std::map; + + /** + * The map of root node. + */ + Map map; + + /** + * Find the node for `k`, creating it if needed. + * + * The node is referred to as a "slot" on the assumption that `V` is + * some sort of optional type, so the given key can be set or unset + * by changing this node. + */ + ChildNode & ensureSlot(const SingleDerivedPath & k); +}; + +} diff --git a/tests/dyn-drv/build-built-drv.sh b/tests/dyn-drv/build-built-drv.sh index 647be9457..94f3550bd 100644 --- a/tests/dyn-drv/build-built-drv.sh +++ b/tests/dyn-drv/build-built-drv.sh @@ -18,4 +18,6 @@ clearStore drvDep=$(nix-instantiate ./text-hashed-output.nix -A producingDrv) -expectStderr 1 nix build "${drvDep}^out^out" --no-link | grepQuiet "Building dynamic derivations in one shot is not yet implemented" +out2=$(nix build "${drvDep}^out^out" --no-link) + +test $out1 == $out2 From d2e6cfa0750cb38ceef7dc0b8bb31cf3b0387e9c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 25 Aug 2023 17:17:33 +0200 Subject: [PATCH 15/20] tests/lang/eval-okay-pathexists: Add cases --- tests/lang/eval-okay-pathexists.nix | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/lang/eval-okay-pathexists.nix b/tests/lang/eval-okay-pathexists.nix index 50c28ee0c..8eae37e70 100644 --- a/tests/lang/eval-okay-pathexists.nix +++ b/tests/lang/eval-okay-pathexists.nix @@ -1,4 +1,6 @@ -builtins.pathExists (builtins.toPath ./lib.nix) +builtins.pathExists (./lib.nix) +&& builtins.pathExists (builtins.toPath ./lib.nix) +&& builtins.pathExists (builtins.toString ./lib.nix) && builtins.pathExists (builtins.toPath (builtins.toString ./lib.nix)) && !builtins.pathExists (builtins.toPath (builtins.toString ./bla.nix)) && builtins.pathExists ./lib.nix From 1e08e12d8138b09e6872cb498b723ade9ad71d68 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 25 Aug 2023 17:18:37 +0200 Subject: [PATCH 16/20] pathExists: isDir when endswith / Fixes https://github.com/NixOS/nix/issues/8838 --- src/libexpr/primops.cc | 16 +++++++++++++--- tests/lang/eval-okay-pathexists.nix | 1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 283f99a48..915b872c8 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1520,15 +1520,25 @@ static RegisterPrimOp primop_storePath({ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, Value & v) { + auto & arg = *args[0]; + /* We don’t check the path right now, because we don’t want to throw if the path isn’t allowed, but just return false (and we can’t just catch the exception here because we still want to - throw if something in the evaluation of `*args[0]` tries to + throw if something in the evaluation of `arg` tries to access an unauthorized path). */ - auto path = realisePath(state, pos, *args[0], { .checkForPureEval = false }); + auto path = realisePath(state, pos, arg, { .checkForPureEval = false }); + + /* SourcePath doesn't know about trailing slash. */ + auto mustBeDir = arg.type() == nString && arg.str().ends_with("/"); try { - v.mkBool(state.checkSourcePath(path).pathExists()); + auto checked = state.checkSourcePath(path); + auto exists = checked.pathExists(); + if (exists && mustBeDir) { + exists = checked.lstat().type == InputAccessor::tDirectory; + } + v.mkBool(exists); } catch (SysError & e) { /* Don't give away info from errors while canonicalising ‘path’ in restricted mode. */ diff --git a/tests/lang/eval-okay-pathexists.nix b/tests/lang/eval-okay-pathexists.nix index 8eae37e70..e1246e370 100644 --- a/tests/lang/eval-okay-pathexists.nix +++ b/tests/lang/eval-okay-pathexists.nix @@ -1,6 +1,7 @@ builtins.pathExists (./lib.nix) && builtins.pathExists (builtins.toPath ./lib.nix) && builtins.pathExists (builtins.toString ./lib.nix) +&& !builtins.pathExists (builtins.toString ./lib.nix + "/") && builtins.pathExists (builtins.toPath (builtins.toString ./lib.nix)) && !builtins.pathExists (builtins.toPath (builtins.toString ./bla.nix)) && builtins.pathExists ./lib.nix From 736b9cede73692a1cf92a6c21c5259498a04c961 Mon Sep 17 00:00:00 2001 From: Bryan Honof Date: Mon, 28 Aug 2023 15:43:34 +0200 Subject: [PATCH 17/20] Port the flags of nix-daemon to nix daemon (#8788) The new `nix daemon` command didn't accept the same flags that `nix-daemon` did. * docs(daemon): clarify the daemon trust override flags * fix: change declaration order * docs: add examples of nix daemon usage * Apply suggestions from code review --------- Co-authored-by: Eelco Dolstra Co-authored-by: John Ericson Co-authored-by: tomberek --- src/nix/daemon.cc | 41 ++++++++++++++++++++++++++++++++++++++++- src/nix/daemon.md | 30 +++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 1511f9e6e..af428018a 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -500,6 +500,45 @@ static RegisterLegacyCommand r_nix_daemon("nix-daemon", main_nix_daemon); struct CmdDaemon : StoreCommand { + bool stdio = false; + std::optional isTrustedOpt = std::nullopt; + + CmdDaemon() + { + addFlag({ + .longName = "stdio", + .description = "Attach to standard I/O, instead of trying to bind to a UNIX socket.", + .handler = {&stdio, true}, + }); + + addFlag({ + .longName = "force-trusted", + .description = "Force the daemon to trust connecting clients.", + .handler = {[&]() { + isTrustedOpt = Trusted; + }}, + .experimentalFeature = Xp::DaemonTrustOverride, + }); + + addFlag({ + .longName = "force-untrusted", + .description = "Force the daemon to not trust connecting clients. The connection will be processed by the receiving daemon before forwarding commands.", + .handler = {[&]() { + isTrustedOpt = NotTrusted; + }}, + .experimentalFeature = Xp::DaemonTrustOverride, + }); + + addFlag({ + .longName = "default-trust", + .description = "Use Nix's default trust.", + .handler = {[&]() { + isTrustedOpt = std::nullopt; + }}, + .experimentalFeature = Xp::DaemonTrustOverride, + }); + } + std::string description() override { return "daemon to perform store operations on behalf of non-root clients"; @@ -516,7 +555,7 @@ struct CmdDaemon : StoreCommand void run(ref store) override { - runDaemon(false, std::nullopt); + runDaemon(stdio, isTrustedOpt); } }; diff --git a/src/nix/daemon.md b/src/nix/daemon.md index d5cdadf08..b1ea850ed 100644 --- a/src/nix/daemon.md +++ b/src/nix/daemon.md @@ -1,20 +1,44 @@ R""( -# Example +# Examples -* Run the daemon in the foreground: +* Run the daemon: ```console # nix daemon ``` +* Run the daemon and listen on standard I/O instead of binding to a UNIX socket: + + ```console + # nix daemon --stdio + ``` + +* Run the daemon and force all connections to be trusted: + + ```console + # nix daemon --force-trusted + ``` + +* Run the daemon and force all connections to be untrusted: + + ```console + # nix daemon --force-untrusted + ``` + +* Run the daemon, listen on standard I/O, and force all connections to use Nix's default trust: + + ```console + # nix daemon --stdio --default-trust + ``` + # Description This command runs the Nix daemon, which is a required component in multi-user Nix installations. It runs build tasks and other operations on the Nix store on behalf of non-root users. Usually you don't run the daemon directly; instead it's managed by a service -management framework such as `systemd`. +management framework such as `systemd` on Linux, or `launchctl` on Darwin. Note that this daemon does not fork into the background. From 151120a1ae082c6460f9a54cf795c57d154f6c27 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 28 Aug 2023 22:14:01 +0200 Subject: [PATCH 18/20] Document nix-prefetch-url defaults (#8878) --- doc/manual/src/command-ref/nix-prefetch-url.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/manual/src/command-ref/nix-prefetch-url.md b/doc/manual/src/command-ref/nix-prefetch-url.md index 3bcd209e2..45ef01e02 100644 --- a/doc/manual/src/command-ref/nix-prefetch-url.md +++ b/doc/manual/src/command-ref/nix-prefetch-url.md @@ -31,15 +31,18 @@ store already contains a file with the same hash and base name. Otherwise, the file is downloaded, and an error is signaled if the actual hash of the file does not match the specified hash. -This command prints the hash on standard output. Additionally, if the -option `--print-path` is used, the path of the downloaded file in the -Nix store is also printed. +This command prints the hash on standard output. +The hash is printed using base-32 unless `--type md5` is specified, +in which case it's printed using base-16. +Additionally, if the option `--print-path` is used, +the path of the downloaded file in the Nix store is also printed. # Options - `--type` *hashAlgo*\ - Use the specified cryptographic hash algorithm, which can be one of - `md5`, `sha1`, `sha256`, and `sha512`. + Use the specified cryptographic hash algorithm, + which can be one of `md5`, `sha1`, `sha256`, and `sha512`. + The default is `sha256`. - `--print-path`\ Print the store path of the downloaded file on standard output. From 56763ff918eb308db23080e560ed2ea3e00c80a7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 29 Aug 2023 15:04:06 +0200 Subject: [PATCH 19/20] Document that redirected tarball flakerefs can specify lastModified --- doc/manual/src/protocols/tarball-fetcher.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/protocols/tarball-fetcher.md b/doc/manual/src/protocols/tarball-fetcher.md index 0d3212303..274fa6d63 100644 --- a/doc/manual/src/protocols/tarball-fetcher.md +++ b/doc/manual/src/protocols/tarball-fetcher.md @@ -20,8 +20,8 @@ Link: ; rel="immutable" (Note the required `<` and `>` characters around *flakeref*.) -*flakeref* must be a tarball flakeref. It can contain flake attributes -such as `narHash`, `rev` and `revCount`. If `narHash` is included, its +*flakeref* must be a tarball flakeref. It can contain the tarball flake attributes +`narHash`, `rev`, `revCount` and `lastModified`. If `narHash` is included, its value must be the NAR hash of the unpacked tarball (as computed via `nix hash path`). Nix checks the contents of the returned tarball against the `narHash` attribute. The `rev` and `revCount` attributes From 46478b44ffb477387235b3c597c24178845fb2a3 Mon Sep 17 00:00:00 2001 From: Cole Helbling Date: Tue, 29 Aug 2023 15:13:35 -0700 Subject: [PATCH 20/20] docs/testing: point out the existence of `GTEST_FILTER` (#8883) --- doc/manual/src/contributing/testing.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/manual/src/contributing/testing.md b/doc/manual/src/contributing/testing.md index c3c82e3c0..cd94d5cfb 100644 --- a/doc/manual/src/contributing/testing.md +++ b/doc/manual/src/contributing/testing.md @@ -7,7 +7,8 @@ under `src/{library_name}/tests` using the [googletest](https://google.github.io/googletest/) and [rapidcheck](https://github.com/emil-e/rapidcheck) frameworks. -You can run the whole testsuite with `make check`, or the tests for a specific component with `make libfoo-tests_RUN`. Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option. +You can run the whole testsuite with `make check`, or the tests for a specific component with `make libfoo-tests_RUN`. +Finer-grained filtering is also possible using the [--gtest_filter](https://google.github.io/googletest/advanced.html#running-a-subset-of-the-tests) command-line option, or the `GTEST_FILTER` environment variable. ## Functional tests