diff --git a/.github/actions/install-nix-action/action.yaml b/.github/actions/install-nix-action/action.yaml new file mode 100644 index 000000000..28103f589 --- /dev/null +++ b/.github/actions/install-nix-action/action.yaml @@ -0,0 +1,50 @@ +name: "Install Nix" +description: "Helper action for installing Nix with support for dogfooding from master" +inputs: + dogfood: + description: "Whether to use Nix installed from the latest artifact from master branch" + required: true # Be explicit about the fact that we are using unreleased artifacts + extra_nix_config: + description: "Gets appended to `/etc/nix/nix.conf` if passed." + install_url: + description: "URL of the Nix installer" + required: false + default: "https://releases.nixos.org/nix/nix-2.30.1/install" + github_token: + description: "Github token" + required: true +runs: + using: "composite" + steps: + - name: "Download nix install artifact from master" + shell: bash + id: download-nix-installer + if: ${{ inputs.dogfood }} + run: | + RUN_ID=$(gh run list --repo "$DOGFOOD_REPO" --workflow ci.yml --branch master --status success --json databaseId --jq ".[0].databaseId") + + if [ "$RUNNER_OS" == "Linux" ]; then + INSTALLER_ARTIFACT="installer-linux" + elif [ "$RUNNER_OS" == "macOS" ]; then + INSTALLER_ARTIFACT="installer-darwin" + else + echo "::error ::Unsupported RUNNER_OS: $RUNNER_OS" + exit 1 + fi + + INSTALLER_DOWNLOAD_DIR="$GITHUB_WORKSPACE/$INSTALLER_ARTIFACT" + mkdir -p "$INSTALLER_DOWNLOAD_DIR" + + gh run download "$RUN_ID" --repo "$DOGFOOD_REPO" -n "$INSTALLER_ARTIFACT" -D "$INSTALLER_DOWNLOAD_DIR" + echo "installer-path=file://$INSTALLER_DOWNLOAD_DIR" >> "$GITHUB_OUTPUT" + + echo "::notice ::Dogfooding Nix installer from master (https://github.com/$DOGFOOD_REPO/actions/runs/$RUN_ID)" + env: + GH_TOKEN: ${{ inputs.github_token }} + DOGFOOD_REPO: "NixOS/nix" + - uses: cachix/install-nix-action@c134e4c9e34bac6cab09cf239815f9339aaaf84e # v31.5.1 + with: + # Ternary operator in GHA: https://www.github.com/actions/runner/issues/409#issuecomment-752775072 + install_url: ${{ inputs.dogfood && format('{0}/install', steps.download-nix-installer.outputs.installer-path) || inputs.install_url }} + install_options: ${{ inputs.dogfood && format('--tarball-url-prefix {0}', steps.download-nix-installer.outputs.installer-path) || '' }} + extra_nix_config: ${{ inputs.extra_nix_config }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29cb33f56..da6f35907 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,8 +13,13 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v31 - - run: nix --experimental-features 'nix-command flakes' flake show --all-systems --json + - uses: ./.github/actions/install-nix-action + with: + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} + - run: nix flake show --all-systems --json tests: strategy: @@ -34,8 +39,10 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v31 + - uses: ./.github/actions/install-nix-action with: + github_token: ${{ secrets.GITHUB_TOKEN }} + dogfood: true # The sandbox would otherwise be disabled by default on Darwin extra_nix_config: | sandbox = true @@ -175,7 +182,12 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - - uses: DeterminateSystems/nix-installer-action@main + - uses: ./.github/actions/install-nix-action + with: + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} - uses: DeterminateSystems/magic-nix-cache-action@main - run: | nix build -L \ @@ -201,6 +213,11 @@ jobs: with: repository: NixOS/flake-regressions-data path: flake-regressions/tests - - uses: DeterminateSystems/nix-installer-action@main + - uses: ./.github/actions/install-nix-action + with: + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} - uses: DeterminateSystems/magic-nix-cache-action@main - run: nix build -L --out-link ./new-nix && PATH=$(pwd)/new-nix/bin:$PATH MAX_FLAKES=25 flake-regressions/eval-all.sh diff --git a/.mergify.yml b/.mergify.yml index 8941711a9..f49144113 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -150,3 +150,14 @@ pull_request_rules: labels: - automatic backport - merge-queue + + - name: backport patches to 2.30 + conditions: + - label=backport 2.30-maintenance + actions: + backport: + branches: + - "2.30-maintenance" + labels: + - automatic backport + - merge-queue diff --git a/.version b/.version index 6a6900382..bafceb320 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.30.0 +2.31.0 diff --git a/doc/manual/meson.build b/doc/manual/meson.build index 6fe2374a7..0779cd267 100644 --- a/doc/manual/meson.build +++ b/doc/manual/meson.build @@ -8,6 +8,7 @@ nix = find_program('nix', native : true) mdbook = find_program('mdbook', native : true) bash = find_program('bash', native : true) +rsync = find_program('rsync', required: true, native: true) pymod = import('python') python = pymod.find_installation('python3') @@ -84,7 +85,7 @@ manual = custom_target( @0@ @INPUT0@ @CURRENT_SOURCE_DIR@ > @DEPFILE@ @0@ @INPUT1@ summary @2@ < @CURRENT_SOURCE_DIR@/source/SUMMARY.md.in > @2@/source/SUMMARY.md sed -e 's|@version@|@3@|g' < @INPUT2@ > @2@/book.toml - rsync -r --include='*.md' @CURRENT_SOURCE_DIR@/ @2@/ + @4@ -r --include='*.md' @CURRENT_SOURCE_DIR@/ @2@/ (cd @2@; RUST_LOG=warn @1@ build -d @2@ 3>&2 2>&1 1>&3) | { grep -Fv "because fragment resolution isn't implemented" || :; } 3>&2 2>&1 1>&3 rm -rf @2@/manual mv @2@/html @2@/manual @@ -94,6 +95,7 @@ manual = custom_target( mdbook.full_path(), meson.current_build_dir(), meson.project_version(), + rsync.full_path(), ), ], input : [ diff --git a/doc/manual/rl-next/build-cores-auto-detect.md b/doc/manual/rl-next/build-cores-auto-detect.md new file mode 100644 index 000000000..67ab6995b --- /dev/null +++ b/doc/manual/rl-next/build-cores-auto-detect.md @@ -0,0 +1,6 @@ +--- +synopsis: "`build-cores = 0` now auto-detects CPU cores" +prs: [13402] +--- + +When `build-cores` is set to `0`, nix now automatically detects the number of available CPU cores and passes this value via `NIX_BUILD_CORES`, instead of passing `0` directly. This matches the behavior when `build-cores` is unset. This prevents the builder from having to detect the number of cores. diff --git a/doc/manual/rl-next/build-dir-mandatory.md b/doc/manual/rl-next/build-dir-mandatory.md deleted file mode 100644 index cb45a4315..000000000 --- a/doc/manual/rl-next/build-dir-mandatory.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -synopsis: "`build-dir` no longer defaults to `$TMPDIR`" ---- - -The directory in which temporary build directories are created no longer defaults -to `TMPDIR` or `/tmp`, to avoid builders making their directories -world-accessible. This behavior allowed escaping the build sandbox and can -cause build impurities even when not used maliciously. We now default to `builds` -in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). diff --git a/doc/manual/rl-next/deprecate__json.md b/doc/manual/rl-next/deprecate__json.md deleted file mode 100644 index 7fc05832f..000000000 --- a/doc/manual/rl-next/deprecate__json.md +++ /dev/null @@ -1,11 +0,0 @@ ---- -synopsis: Deprecate manually making structured attrs with `__json = ...;` -prs: [13220] ---- - -The proper way to create a derivation using [structured attrs] in the Nix language is by using `__structuredAttrs = true` with [`builtins.derivation`]. -However, by exploiting how structured attrs are implementated, it has also been possible to create them by setting the `__json` environment variable to a serialized JSON string. -This sneaky alternative method is now deprecated, and may be disallowed in future versions of Nix. - -[structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs -[`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation diff --git a/doc/manual/rl-next/eval-profiler.md b/doc/manual/rl-next/eval-profiler.md deleted file mode 100644 index 6b2bf2ce7..000000000 --- a/doc/manual/rl-next/eval-profiler.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -synopsis: Add stack sampling evaluation profiler -prs: [13220] ---- - -Nix evaluator now supports stack sampling evaluation profiling via `--eval-profiler flamegraph` setting. -It collects collapsed call stack information to output file specified by -`--eval-profile-file` (`nix.profile` by default) in a format directly consumable -by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). -Sampling frequency can be configured via `--eval-profiler-frequency` (99 Hz by default). - -Unlike existing `--trace-function-calls` this profiler includes the name of the function -being called when it's available. diff --git a/doc/manual/rl-next/json-logger.md b/doc/manual/rl-next/json-logger.md deleted file mode 100644 index 867b5d8b3..000000000 --- a/doc/manual/rl-next/json-logger.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -synopsis: "`json-log-path` setting" -prs: [13003] ---- - -New setting `json-log-path` that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. diff --git a/doc/manual/rl-next/nix-profile-add.md b/doc/manual/rl-next/nix-profile-add.md deleted file mode 100644 index a2fbcaa8d..000000000 --- a/doc/manual/rl-next/nix-profile-add.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -synopsis: "Rename `nix profile install` to `nix profile add`" -prs: [13224] ---- - -The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". diff --git a/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md b/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md deleted file mode 100644 index 479a2f5cb..000000000 --- a/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -synopsis: Non-flake inputs now contain a `sourceInfo` attribute -issues: 13164 -prs: 13170 ---- - -Flakes have always a `sourceInfo` attribute which describes the source of the flake. -The `sourceInfo.outPath` is often identical to the flake's `outPath`, however it can differ when the flake is located in a subdirectory of its source. - -Non-flake inputs (i.e. inputs with `flake = false`) can also be located at some path _within_ a wider source. -This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. -Such relative inputs will now inherit their parent's `sourceInfo`. - -This also means it is now possible to use `?dir=subdir` on non-flake inputs. - -This iterates on the work done in 2.26 to improve relative path support ([#10089](https://github.com/NixOS/nix/pull/10089)), -and resolves a regression introduced in 2.28 relating to nested relative path inputs ([#13164](https://github.com/NixOS/nix/issues/13164)). diff --git a/doc/manual/rl-next/revert-77.md b/doc/manual/rl-next/revert-77.md deleted file mode 100644 index c2cef74d3..000000000 --- a/doc/manual/rl-next/revert-77.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -synopsis: Revert incomplete closure mixed download and build feature -issues: [77, 12628] -prs: [13176] ---- - -Since Nix 1.3 (299141ecbd08bae17013226dbeae71e842b4fdd7 in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). -This feature is now removed. - -Worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. -In practice, however, we doubt even the more building is very likely to happen. -Remote stores that are missing dependencies in arbitrary ways (e.g. corruption) don't seem to be very common. - -On the contrary, when remote stores fail to implement the [closure property](@docroot@/store/store-object.md#closure-property), it is usually an *intentional* choice on the part of the remote store, because it wishes to serve as an "overlay" store over another store, such as `https://cache.nixos.org`. -If an "incomplete closure" is encountered in that situation, the right fix is not to do some sort of "franken-building" as this feature implemented, but instead to make sure both substituters are enabled in the settings. - -(In the future, we should make it easier for remote stores to indicate this to clients, to catch settings that won't work in general before a missing dependency is actually encountered.) diff --git a/doc/manual/source/SUMMARY.md.in b/doc/manual/source/SUMMARY.md.in index 8326a96e3..bfb921567 100644 --- a/doc/manual/source/SUMMARY.md.in +++ b/doc/manual/source/SUMMARY.md.in @@ -137,6 +137,7 @@ - [Contributing](development/contributing.md) - [Releases](release-notes/index.md) {{#include ./SUMMARY-rl-next.md}} + - [Release 2.30 (2025-07-07)](release-notes/rl-2.30.md) - [Release 2.29 (2025-05-14)](release-notes/rl-2.29.md) - [Release 2.28 (2025-04-02)](release-notes/rl-2.28.md) - [Release 2.27 (2025-03-03)](release-notes/rl-2.27.md) diff --git a/doc/manual/source/package-management/garbage-collector-roots.md b/doc/manual/source/package-management/garbage-collector-roots.md index 30c5b7f8d..925a33162 100644 --- a/doc/manual/source/package-management/garbage-collector-roots.md +++ b/doc/manual/source/package-management/garbage-collector-roots.md @@ -12,7 +12,7 @@ $ ln -s /nix/store/d718ef...-foo /nix/var/nix/gcroots/bar That is, after this command, the garbage collector will not remove `/nix/store/d718ef...-foo` or any of its dependencies. -Subdirectories of `prefix/nix/var/nix/gcroots` are also searched for -symlinks. Symlinks to non-store paths are followed and searched for -roots, but symlinks to non-store paths *inside* the paths reached in -that way are not followed to prevent infinite recursion. +Subdirectories of `prefix/nix/var/nix/gcroots` are searched +recursively. Symlinks to store paths count as roots. Symlinks to +non-store paths are ignored, unless the non-store path is itself a +symlink to a store path. \ No newline at end of file diff --git a/doc/manual/source/release-notes/rl-2.24.md b/doc/manual/source/release-notes/rl-2.24.md index bac0a5998..d4af3cb51 100644 --- a/doc/manual/source/release-notes/rl-2.24.md +++ b/doc/manual/source/release-notes/rl-2.24.md @@ -269,7 +269,7 @@ e.g. `--warn-large-path-threshold 100M`. -# Contributors +## Contributors This release was made possible by the following 43 contributors: diff --git a/doc/manual/source/release-notes/rl-2.25.md b/doc/manual/source/release-notes/rl-2.25.md index 29e3e509c..cfde8b1ef 100644 --- a/doc/manual/source/release-notes/rl-2.25.md +++ b/doc/manual/source/release-notes/rl-2.25.md @@ -77,7 +77,7 @@ `` is also known as the builtin derivation builder `builtin:fetchurl`. It's not to be confused with the evaluation-time function `builtins.fetchurl`, which was not affected by this issue. -# Contributors +## Contributors This release was made possible by the following 58 contributors: diff --git a/doc/manual/source/release-notes/rl-2.26.md b/doc/manual/source/release-notes/rl-2.26.md index d2a890eb6..0c3df828f 100644 --- a/doc/manual/source/release-notes/rl-2.26.md +++ b/doc/manual/source/release-notes/rl-2.26.md @@ -76,7 +76,7 @@ - Evaluation caching now works for dirty Git workdirs [#11992](https://github.com/NixOS/nix/pull/11992) -# Contributors +## Contributors This release was made possible by the following 45 contributors: diff --git a/doc/manual/source/release-notes/rl-2.27.md b/doc/manual/source/release-notes/rl-2.27.md index 3643f7476..34da62525 100644 --- a/doc/manual/source/release-notes/rl-2.27.md +++ b/doc/manual/source/release-notes/rl-2.27.md @@ -47,7 +47,7 @@ blake3-34P4p+iZXcbbyB1i4uoF7eWCGcZHjmaRn6Y7QdynLwU= ``` -# Contributors +## Contributors This release was made possible by the following 21 contributors: diff --git a/doc/manual/source/release-notes/rl-2.28.md b/doc/manual/source/release-notes/rl-2.28.md index 6da09546e..93ea2cfde 100644 --- a/doc/manual/source/release-notes/rl-2.28.md +++ b/doc/manual/source/release-notes/rl-2.28.md @@ -82,7 +82,7 @@ This completes the infrastructure overhaul for the [RFC 132](https://github.com/ Although this change is not as critical, we figured it would be good to do this API change at the same time, also. Also note that we try to keep the C API compatible, but we decided to break this function because it was young and likely not in widespread use yet. This frees up time to make important progress on the rest of the C API. -# Contributors +## Contributors This earlier-than-usual release was made possible by the following 16 contributors: diff --git a/doc/manual/source/release-notes/rl-2.29.md b/doc/manual/source/release-notes/rl-2.29.md index ad63fff2f..b59d6d6f0 100644 --- a/doc/manual/source/release-notes/rl-2.29.md +++ b/doc/manual/source/release-notes/rl-2.29.md @@ -111,7 +111,7 @@ This fact is counterbalanced by the fact that most of those changes are bug fixe This in particular prevents parts of GCC 14's diagnostics from being improperly filtered away. -# Contributors +## Contributors This release was made possible by the following 40 contributors: diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md new file mode 100644 index 000000000..34d3e5bab --- /dev/null +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -0,0 +1,153 @@ +# Release 2.30.0 (2025-07-07) + +## Backward-incompatible changes and deprecations + +- [`build-dir`] no longer defaults to `$TMPDIR` + + The directory in which temporary build directories are created no longer defaults + to `TMPDIR` or `/tmp`, to avoid builders making their directories + world-accessible. This behavior allowed escaping the build sandbox and can + cause build impurities even when not used maliciously. We now default to `builds` + in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). + +- Deprecate manually making structured attrs using the `__json` attribute [#13220](https://github.com/NixOS/nix/pull/13220) + + The proper way to create a derivation using [structured attrs] in the Nix language is by using `__structuredAttrs = true` with [`builtins.derivation`]. + However, by exploiting how structured attrs are implementated, it has also been possible to create them by setting the `__json` environment variable to a serialized JSON string. + This sneaky alternative method is now deprecated, and may be disallowed in future versions of Nix. + + [structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs + [`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation + +- Rename `nix profile install` to [`nix profile add`] [#13224](https://github.com/NixOS/nix/pull/13224) + + The command `nix profile install` has been renamed to [`nix profile add`] (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". + +## Performance improvements + +This release has a number performance improvements, in particular: + +- Reduce the size of value from 24 to 16 bytes [#13407](https://github.com/NixOS/nix/pull/13407) + + This shaves off a very significant amount of memory used for evaluation (~20% percent reduction in maximum heap size and ~17% in total bytes). + +## Features + +- Add [stack sampling evaluation profiler] [#13220](https://github.com/NixOS/nix/pull/13220) + + The Nix evaluator now supports [stack sampling evaluation profiling](@docroot@/advanced-topics/eval-profiler.md) via the [`--eval-profiler flamegraph`] setting. + It outputs collapsed call stack information to the file specified by + [`--eval-profile-file`] (`nix.profile` by default) in a format directly consumable + by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). + Sampling frequency can be configured via [`--eval-profiler-frequency`] (99 Hz by default). + + Unlike the existing [`--trace-function-calls`], this profiler includes the name of the function + being called when it's available. + +- [`nix repl`] prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) + + Instead of `Added variables` it now prints the first 10 variables that were added to the global scope. + +- `nix flake archive`: Add [`--no-check-sigs`] option [#13277](https://github.com/NixOS/nix/pull/13277) + + This is useful when using [`nix flake archive`] with the destination set to a remote store. + +- Emit warnings for IFDs with [`trace-import-from-derivation`] option [#13279](https://github.com/NixOS/nix/pull/13279) + + While we have the setting [`allow-import-from-derivation`] to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. + +- `json-log-path` setting [#13003](https://github.com/NixOS/nix/pull/13003) + + New setting [`json-log-path`] that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. + +- Non-flake inputs now contain a `sourceInfo` attribute [#13164](https://github.com/NixOS/nix/issues/13164) [#13170](https://github.com/NixOS/nix/pull/13170) + + Flakes have always had a `sourceInfo` attribute which describes the source of the flake. + The `sourceInfo.outPath` is often identical to the flake's `outPath`. However, it can differ when the flake is located in a subdirectory of its source. + + Non-flake inputs (i.e. inputs with [`flake = false`]) can also be located at some path _within_ a wider source. + This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. + Such relative inputs will now inherit their parent's `sourceInfo`. + + This also means it is now possible to use `?dir=subdir` on non-flake inputs. + + This iterates on the work done in 2.26 to improve relative path support ([#10089](https://github.com/NixOS/nix/pull/10089)), + and resolves a regression introduced in 2.28 relating to nested relative path inputs ([#13164](https://github.com/NixOS/nix/issues/13164)). + +## Miscellaneous changes + +- [`builtins.sort`] uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) + + Previously it used libstdc++'s `std::stable_sort()`. However, that implementation is not reliable if the user-supplied comparison function is not a strict weak ordering. + +- Revert incomplete closure mixed download and build feature [#77](https://github.com/NixOS/nix/issues/77) [#12628](https://github.com/NixOS/nix/issues/12628) [#13176](https://github.com/NixOS/nix/pull/13176) + + Since Nix 1.3 ([commit `299141e`] in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). + This feature is now removed. + + In the worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. + In practice, however, we doubt even more building is very likely to happen. + Remote stores that are missing dependencies in arbitrary ways (e.g. corruption) don't seem to be very common. + + On the contrary, when remote stores fail to implement the [closure property](@docroot@/store/store-object.md#closure-property), it is usually an *intentional* choice on the part of the remote store, because it wishes to serve as an "overlay" store over another store, such as `https://cache.nixos.org`. + If an "incomplete closure" is encountered in that situation, the right fix is not to do some sort of "franken-building" as this feature implemented, but instead to make sure both substituters are enabled in the settings. + + (In the future, we should make it easier for remote stores to indicate this to clients, to catch settings that won't work in general before a missing dependency is actually encountered.) + +## Contributors + +This release was made possible by the following 32 contributors: + +- Cole Helbling [**(@cole-h)**](https://github.com/cole-h) +- Eelco Dolstra [**(@edolstra)**](https://github.com/edolstra) +- Egor Konovalov [**(@egorkonovalov)**](https://github.com/egorkonovalov) +- Farid Zakaria [**(@fzakaria)**](https://github.com/fzakaria) +- Graham Christensen [**(@grahamc)**](https://github.com/grahamc) +- gustavderdrache [**(@gustavderdrache)**](https://github.com/gustavderdrache) +- Gwenn Le Bihan [**(@gwennlbh)**](https://github.com/gwennlbh) +- h0nIg [**(@h0nIg)**](https://github.com/h0nIg) +- Jade Masker [**(@donottellmetonottellyou)**](https://github.com/donottellmetonottellyou) +- jayeshv [**(@jayeshv)**](https://github.com/jayeshv) +- Jeremy Fleischman [**(@jfly)**](https://github.com/jfly) +- John Ericson [**(@Ericson2314)**](https://github.com/Ericson2314) +- Jonas Chevalier [**(@zimbatm)**](https://github.com/zimbatm) +- Jörg Thalheim [**(@Mic92)**](https://github.com/Mic92) +- kstrafe [**(@kstrafe)**](https://github.com/kstrafe) +- Luc Perkins [**(@lucperkins)**](https://github.com/lucperkins) +- Matt Sturgeon [**(@MattSturgeon)**](https://github.com/MattSturgeon) +- Nikita Krasnov [**(@synalice)**](https://github.com/synalice) +- Peder Bergebakken Sundt [**(@pbsds)**](https://github.com/pbsds) +- pennae [**(@pennae)**](https://github.com/pennae) +- Philipp Otterbein +- Pol Dellaiera [**(@drupol)**](https://github.com/drupol) +- PopeRigby [**(@poperigby)**](https://github.com/poperigby) +- Raito Bezarius +- Robert Hensing [**(@roberth)**](https://github.com/roberth) +- Samuli Thomasson [**(@SimSaladin)**](https://github.com/SimSaladin) +- Sergei Zimmerman [**(@xokdvium)**](https://github.com/xokdvium) +- Seth Flynn [**(@getchoo)**](https://github.com/getchoo) +- Stefan Boca [**(@stefanboca)**](https://github.com/stefanboca) +- tomberek [**(@tomberek)**](https://github.com/tomberek) +- Tristan Ross [**(@RossComputerGuy)**](https://github.com/RossComputerGuy) +- Valentin Gagarin [**(@fricklerhandwerk)**](https://github.com/fricklerhandwerk) +- Vladimír Čunát [**(@vcunat)**](https://github.com/vcunat) +- Wolfgang Walther [**(@wolfgangwalther)**](https://github.com/wolfgangwalther) + + +[stack sampling evaluation profiler]: @docroot@/advanced-topics/eval-profiler.md +[`--eval-profiler`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler +[`--eval-profiler flamegraph`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler +[`--trace-function-calls`]: @docroot@/command-ref/conf-file.md#conf-trace-function-calls +[`--eval-profile-file`]: @docroot@/command-ref/conf-file.md#conf-eval-profile-file +[`--eval-profiler-frequency`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler-frequency +[`build-dir`]: @docroot@/command-ref/conf-file.md#conf-build-dir +[`nix profile add`]: @docroot@/command-ref/new-cli/nix3-profile-add.md +[`nix repl`]: @docroot@/command-ref/new-cli/nix3-repl.md +[`nix flake archive`]: @docroot@/command-ref/new-cli/nix3-flake-archive.md +[`json-log-path`]: @docroot@/command-ref/conf-file.md#conf-json-log-path +[`trace-import-from-derivation`]: @docroot@/command-ref/conf-file.md#conf-trace-import-from-derivation +[`allow-import-from-derivation`]: @docroot@/command-ref/conf-file.md#conf-allow-import-from-derivation +[`builtins.sort`]: @docroot@/language/builtins.md#builtins-sort +[`flake = false`]: @docroot@/command-ref/new-cli/nix3-flake.md?highlight=false#flake-inputs +[`--no-check-sigs`]: @docroot@/command-ref/new-cli/nix3-flake-archive.md#opt-no-check-sigs +[commit `299141e`]: https://github.com/NixOS/nix/commit/299141ecbd08bae17013226dbeae71e842b4fdd7 diff --git a/docker.nix b/docker.nix index c6e8e478e..410e4a178 100644 --- a/docker.nix +++ b/docker.nix @@ -1,10 +1,10 @@ { # Core dependencies - pkgs, - lib, - dockerTools, - runCommand, - buildPackages, + pkgs ? import { }, + lib ? pkgs.lib, + dockerTools ? pkgs.dockerTools, + runCommand ? pkgs.runCommand, + buildPackages ? pkgs.buildPackages, # Image configuration name ? "nix", tag ? "latest", @@ -28,24 +28,24 @@ }, Cmd ? [ (lib.getExe bashInteractive) ], # Default Packages - nix, - bashInteractive, - coreutils-full, - gnutar, - gzip, - gnugrep, - which, - curl, - less, - wget, - man, - cacert, - findutils, - iana-etc, - gitMinimal, - openssh, + nix ? pkgs.nix, + bashInteractive ? pkgs.bashInteractive, + coreutils-full ? pkgs.coreutils-full, + gnutar ? pkgs.gnutar, + gzip ? pkgs.gzip, + gnugrep ? pkgs.gnugrep, + which ? pkgs.which, + curl ? pkgs.curl, + less ? pkgs.less, + wget ? pkgs.wget, + man ? pkgs.man, + cacert ? pkgs.cacert, + findutils ? pkgs.findutils, + iana-etc ? pkgs.iana-etc, + gitMinimal ? pkgs.gitMinimal, + openssh ? pkgs.openssh, # Other dependencies - shadow, + shadow ? pkgs.shadow, }: let defaultPkgs = [ @@ -184,11 +184,14 @@ let } " = "; }; - nixConfContents = toConf { - sandbox = false; - build-users-group = "nixbld"; - trusted-public-keys = [ "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=" ]; - }; + nixConfContents = toConf ( + { + sandbox = false; + build-users-group = "nixbld"; + trusted-public-keys = [ "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=" ]; + } + // nixConf + ); userHome = if uid == 0 then "/root" else "/home/${uname}"; diff --git a/maintainers/data/release-credits-email-to-handle.json b/maintainers/data/release-credits-email-to-handle.json index c77fe4684..48e8685e6 100644 --- a/maintainers/data/release-credits-email-to-handle.json +++ b/maintainers/data/release-credits-email-to-handle.json @@ -166,5 +166,24 @@ "the-tumultuous-unicorn-of-darkness@gmx.com": "TheTumultuousUnicornOfDarkness", "dev@rodney.id.au": "rvl", "pe@pijul.org": "P-E-Meunier", - "yannik@floxdev.com": "ysndr" + "yannik@floxdev.com": "ysndr", + "73017521+egorkonovalov@users.noreply.github.com": "egorkonovalov", + "raito@lix.systems": null, + "nikita.nikita.krasnov@gmail.com": "synalice", + "lucperkins@gmail.com": "lucperkins", + "vladimir.cunat@nic.cz": "vcunat", + "walther@technowledgy.de": "wolfgangwalther", + "jayesh.mail@gmail.com": "jayeshv", + "samuli.thomasson@pm.me": "SimSaladin", + "kevin@stravers.net": "kstrafe", + "poperigby@mailbox.org": "poperigby", + "cole.helbling@determinate.systems": "cole-h", + "donottellmetonottellyou@gmail.com": "donottellmetonottellyou", + "getchoo@tuta.io": "getchoo", + "alex.ford@determinate.systems": "gustavderdrache", + "stefan.r.boca@gmail.com": "stefanboca", + "gwenn.lebihan7@gmail.com": "gwennlbh", + "hey@ewen.works": "gwennlbh", + "matt@sturgeon.me.uk": "MattSturgeon", + "pbsds@hotmail.com": "pbsds" } \ No newline at end of file diff --git a/maintainers/data/release-credits-handle-to-name.json b/maintainers/data/release-credits-handle-to-name.json index 734476078..a6352c44b 100644 --- a/maintainers/data/release-credits-handle-to-name.json +++ b/maintainers/data/release-credits-handle-to-name.json @@ -146,5 +146,21 @@ "ajlekcahdp4": "Alexander Romanov", "Valodim": "Vincent Breitmoser", "rvl": "Rodney Lorrimar", - "whatsthecraic": "Dean De Leo" + "whatsthecraic": "Dean De Leo", + "gwennlbh": "Gwenn Le Bihan", + "donottellmetonottellyou": "Jade Masker", + "kstrafe": null, + "synalice": "Nikita Krasnov", + "poperigby": "PopeRigby", + "MattSturgeon": "Matt Sturgeon", + "lucperkins": "Luc Perkins", + "gustavderdrache": null, + "SimSaladin": "Samuli Thomasson", + "getchoo": "Seth Flynn", + "stefanboca": "Stefan Boca", + "wolfgangwalther": "Wolfgang Walther", + "pbsds": "Peder Bergebakken Sundt", + "egorkonovalov": "Egor Konovalov", + "jayeshv": "jayeshv", + "vcunat": "Vladim\u00edr \u010cun\u00e1t" } \ No newline at end of file diff --git a/maintainers/release-notes b/maintainers/release-notes index 6586b22dc..5bb492227 100755 --- a/maintainers/release-notes +++ b/maintainers/release-notes @@ -157,7 +157,7 @@ section_title="Release $version_full ($DATE)" if ! $IS_PATCH; then echo - echo "# Contributors" + echo "## Contributors" echo VERSION=$version_full ./maintainers/release-credits fi diff --git a/maintainers/release-process.md b/maintainers/release-process.md index f2c61302b..37b38fb9f 100644 --- a/maintainers/release-process.md +++ b/maintainers/release-process.md @@ -39,10 +39,6 @@ release: * Proof-read / edit / rearrange the release notes if needed. Breaking changes and highlights should go to the top. -* Run `maintainers/release-credits` to make sure the credits script works - and produces a sensible output. Some emails might not automatically map to - a GitHub handle. - * Push. ```console diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index f051ccc46..e9ddfc014 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -834,8 +834,13 @@ install_from_extracted_nix() { ( cd "$EXTRACTED_NIX_PATH" - _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ - cp -RPp ./store/* "$NIX_ROOT/store/" + if is_os_darwin; then + _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ + cp -RPp ./store/* "$NIX_ROOT/store/" + else + _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ + cp -RP --preserve=ownership,timestamps ./store/* "$NIX_ROOT/store/" + fi _sudo "to make the new store non-writable at $NIX_ROOT/store" \ chmod -R ugo-w "$NIX_ROOT/store/" diff --git a/scripts/install-nix-from-tarball.sh b/scripts/install-nix-from-tarball.sh index 8d127a9c5..ec3264793 100644 --- a/scripts/install-nix-from-tarball.sh +++ b/scripts/install-nix-from-tarball.sh @@ -167,7 +167,11 @@ for i in $(cd "$self/store" >/dev/null && echo ./*); do rm -rf "$i_tmp" fi if ! [ -e "$dest/store/$i" ]; then - cp -RPp "$self/store/$i" "$i_tmp" + if [ "$(uname -s)" = "Darwin" ]; then + cp -RPp "$self/store/$i" "$i_tmp" + else + cp -RP --preserve=ownership,timestamps "$self/store/$i" "$i_tmp" + fi chmod -R a-w "$i_tmp" chmod +w "$i_tmp" mv "$i_tmp" "$dest/store/$i" diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1321e00a5..47cc35daa 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1602,7 +1602,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") - .withFrame(*fun.lambda().env, lambda) + .withFrame(*vCur.lambda().env, lambda) .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); @@ -1629,7 +1629,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, .atPos(lambda.pos) .withTrace(pos, "from call site") .withSuggestions(suggestions) - .withFrame(*fun.lambda().env, lambda) + .withFrame(*vCur.lambda().env, lambda) .debugThrow(); } unreachable(); diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index f88f13874..eee3b0f0e 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -211,6 +211,8 @@ struct EvalSettings : Config * `flamegraph` stack sampling profiler. Outputs folded format, one line per stack (suitable for `flamegraph.pl` and compatible tools). Use [`eval-profile-file`](#conf-eval-profile-file) to specify where the profile is saved. + + See [Using the `eval-profiler`](@docroot@/advanced-topics/eval-profiler.md). )"}; Setting evalProfileFile{this, "nix.profile", "eval-profile-file", diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 20a05a09d..4dedf3d91 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -7,12 +7,7 @@ #include "nix/util/error.hh" #include -#define USE_FLAT_SYMBOL_SET (BOOST_VERSION >= 108100) -#if USE_FLAT_SYMBOL_SET -# include -#else -# include -#endif +#include namespace nix { @@ -214,12 +209,7 @@ private: * Transparent lookup of string view for a pointer to a ChunkedVector entry -> return offset into the store. * ChunkedVector references are never invalidated. */ -#if USE_FLAT_SYMBOL_SET boost::unordered_flat_set symbols{SymbolStr::chunkSize}; -#else - using SymbolValueAlloc = std::pmr::polymorphic_allocator; - boost::unordered_set symbols{SymbolStr::chunkSize, {&buffer}}; -#endif public: @@ -230,19 +220,7 @@ public: // Most symbols are looked up more than once, so we trade off insertion performance // for lookup performance. // FIXME: make this thread-safe. - return [&](T && key) -> Symbol { - if constexpr (requires { symbols.insert(key); }) { - auto [it, _] = symbols.insert(key); - return Symbol(*it); - } else { - auto it = symbols.find(key); - if (it != symbols.end()) - return Symbol(*it); - - it = symbols.emplace(key).first; - return Symbol(*it); - } - }(SymbolStr::Key{store, s, stringAlloc}); + return Symbol(*symbols.insert(SymbolStr::Key{store, s, stringAlloc}).first); } std::vector resolve(const std::vector & symbols) const @@ -287,5 +265,3 @@ struct std::hash return std::hash{}(s.id); } }; - -#undef USE_FLAT_SYMBOL_SET diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f9f834a62..99ca19d7e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -4122,9 +4122,9 @@ static RegisterPrimOp primop_lessThan({ .name = "__lessThan", .args = {"e1", "e2"}, .doc = R"( - Return `true` if the number *e1* is less than the number *e2*, and - `false` otherwise. Evaluation aborts if either *e1* or *e2* does not - evaluate to a number. + Return `true` if the value *e1* is less than the value *e2*, and `false` otherwise. + Evaluation aborts if either *e1* or *e2* does not evaluate to a number, string or path. + Furthermore, it aborts if *e2* does not match *e1*'s type according to the aforementioned classification of number, string or path. )", .fun = prim_lessThan, }); diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index ea6145f6f..4be4dac8f 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -124,7 +124,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg for (auto & attr : *args[0]->attrs()) { const auto & attrName = state.symbols[attr.name]; auto attrHint = [&]() -> std::string { - return "while evaluating the '" + attrName + "' attribute passed to builtins.fetchClosure"; + return fmt("while evaluating the attribute '%s' passed to builtins.fetchClosure", attrName); }; if (attrName == "fromPath") { diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 9fe271fe8..d76f6879d 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -545,15 +545,20 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this append(gitArgs, {"--depth", "1"}); append(gitArgs, {std::string("--"), url, refspec}); - runProgram(RunOptions { + auto [status, output] = runProgram(RunOptions { .program = "git", .lookupPath = true, // FIXME: git stderr messes up our progress indicator, so // we're using --quiet for now. Should process its stderr. .args = gitArgs, .input = {}, + .mergeStderrToStdout = true, .isInteractive = true }); + + if (status > 0) { + throw Error("Failed to fetch git repository %s : %s", url, output); + } } void verifyCommit( diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index cf255c001..88fe2e83d 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -444,7 +444,11 @@ struct GitInputScheme : InputScheme // repo, treat as a remote URI to force a clone. static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; // for testing auto url = parseURL(getStrAttr(input.attrs, "url")); - bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git"); + + // Why are we checking for bare repository? + // well if it's a bare repository we want to force a git fetch rather than copying the folder + bool isBareRepository = url.scheme == "file" && pathExists(url.path) && + !pathExists(url.path + "/.git"); // // FIXME: here we turn a possibly relative path into an absolute path. // This allows relative git flake inputs to be resolved against the @@ -462,6 +466,12 @@ struct GitInputScheme : InputScheme "See https://github.com/NixOS/nix/issues/12281 for details.", url); } + + // If we don't check here for the path existence, then we can give libgit2 any directory + // and it will initialize them as git directories. + if (!pathExists(url.path)) { + throw Error("The path '%s' does not exist.", url.path); + } repoInfo.location = std::filesystem::absolute(url.path); } else { if (url.scheme == "file") @@ -599,7 +609,7 @@ struct GitInputScheme : InputScheme ? cacheDir / ref : cacheDir / "refs/heads" / ref; - bool doFetch; + bool doFetch = false; time_t now = time(0); /* If a rev was specified, we need to fetch if it's not in the diff --git a/src/libfetchers/include/nix/fetchers/input-cache.hh b/src/libfetchers/include/nix/fetchers/input-cache.hh index 9b1c5a310..f9278053a 100644 --- a/src/libfetchers/include/nix/fetchers/input-cache.hh +++ b/src/libfetchers/include/nix/fetchers/input-cache.hh @@ -1,4 +1,4 @@ -#include "fetchers.hh" +#include "nix/fetchers/fetchers.hh" namespace nix::fetchers { diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index b0822cc33..59316eabd 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -111,6 +111,25 @@ static DownloadTarballResult downloadTarball_( const Headers & headers, const std::string & displayPrefix) { + + // Some friendly error messages for common mistakes. + // Namely lets catch when the url is a local file path, but + // it is not in fact a tarball. + if (url.rfind("file://", 0) == 0) { + // Remove "file://" prefix to get the local file path + std::string localPath = url.substr(7); + if (!std::filesystem::exists(localPath)) { + throw Error("tarball '%s' does not exist.", localPath); + } + if (std::filesystem::is_directory(localPath)) { + if (std::filesystem::exists(localPath + "/.git")) { + throw Error( + "tarball '%s' is a git repository, not a tarball. Please use `git+file` as the scheme.", localPath); + } + throw Error("tarball '%s' is a directory, not a file.", localPath); + } + } + Cache::Key cacheKey{"tarball", {{"url", url}}}; auto cached = settings.getCache()->lookupExpired(cacheKey); diff --git a/src/libflake-tests/meson.build b/src/libflake-tests/meson.build index 593b0e18d..8c082c7e0 100644 --- a/src/libflake-tests/meson.build +++ b/src/libflake-tests/meson.build @@ -60,6 +60,7 @@ test( env : { '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data', 'NIX_CONFIG': 'extra-experimental-features = flakes', + 'HOME': meson.current_build_dir() / 'test-home', }, protocol : 'gtest', ) diff --git a/src/libflake-tests/package.nix b/src/libflake-tests/package.nix index 714f3791a..397ef4192 100644 --- a/src/libflake-tests/package.nix +++ b/src/libflake-tests/package.nix @@ -3,6 +3,7 @@ buildPackages, stdenv, mkMesonExecutable, + writableTmpDirAsHomeHook, nix-flake, nix-flake-c, @@ -55,19 +56,14 @@ mkMesonExecutable (finalAttrs: { runCommand "${finalAttrs.pname}-run" { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; + buildInputs = [ writableTmpDirAsHomeHook ]; } - ( - lib.optionalString stdenv.hostPlatform.isWindows '' - export HOME="$PWD/home-dir" - mkdir -p "$HOME" - '' - + '' - export _NIX_TEST_UNIT_DATA=${resolvePath ./data} - export NIX_CONFIG="extra-experimental-features = flakes" - ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} - touch $out - '' - ); + ('' + export _NIX_TEST_UNIT_DATA=${resolvePath ./data} + export NIX_CONFIG="extra-experimental-features = flakes" + ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} + touch $out + ''); }; }; diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 322abaa4a..7a11e6047 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -715,16 +715,12 @@ LockedFlake lockFlake( Finally cleanup([&]() { parents.pop_back(); }); /* Recursively process the inputs of this - flake. Also, unless we already have this flake - in the top-level lock file, use this flake's - own lock file. */ + flake, using its own lock file. */ nodePaths.emplace(childNode, inputFlake.path.parent()); computeLocks( inputFlake.inputs, childNode, inputAttrPath, - oldLock - ? std::dynamic_pointer_cast(oldLock) - : readLockFile(state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), - oldLock ? followsPrefix : inputAttrPath, + readLockFile(state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), + inputAttrPath, inputFlake.path, false); } diff --git a/src/libmain/include/nix/main/shared.hh b/src/libmain/include/nix/main/shared.hh index 2ff57135b..4d4b816e7 100644 --- a/src/libmain/include/nix/main/shared.hh +++ b/src/libmain/include/nix/main/shared.hh @@ -35,15 +35,17 @@ void printVersion(const std::string & programName); void printGCWarning(); class Store; +struct MissingPaths; void printMissing( ref store, const std::vector & paths, Verbosity lvl = lvlInfo); -void printMissing(ref store, const StorePathSet & willBuild, - const StorePathSet & willSubstitute, const StorePathSet & unknown, - uint64_t downloadSize, uint64_t narSize, Verbosity lvl = lvlInfo); +void printMissing( + ref store, + const MissingPaths & missing, + Verbosity lvl = lvlInfo); std::string getArg(const std::string & opt, Strings::iterator & i, const Strings::iterator & end); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 3b431f021..0982810d1 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -46,43 +46,41 @@ void printGCWarning() void printMissing(ref store, const std::vector & paths, Verbosity lvl) { - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); - printMissing(store, willBuild, willSubstitute, unknown, downloadSize, narSize, lvl); + printMissing(store, store->queryMissing(paths), lvl); } -void printMissing(ref store, const StorePathSet & willBuild, - const StorePathSet & willSubstitute, const StorePathSet & unknown, - uint64_t downloadSize, uint64_t narSize, Verbosity lvl) +void printMissing( + ref store, + const MissingPaths & missing, + Verbosity lvl) { - if (!willBuild.empty()) { - if (willBuild.size() == 1) + if (!missing.willBuild.empty()) { + if (missing.willBuild.size() == 1) printMsg(lvl, "this derivation will be built:"); else - printMsg(lvl, "these %d derivations will be built:", willBuild.size()); - auto sorted = store->topoSortPaths(willBuild); + printMsg(lvl, "these %d derivations will be built:", missing.willBuild.size()); + auto sorted = store->topoSortPaths(missing.willBuild); reverse(sorted.begin(), sorted.end()); for (auto & i : sorted) printMsg(lvl, " %s", store->printStorePath(i)); } - if (!willSubstitute.empty()) { - const float downloadSizeMiB = downloadSize / (1024.f * 1024.f); - const float narSizeMiB = narSize / (1024.f * 1024.f); - if (willSubstitute.size() == 1) { + if (!missing.willSubstitute.empty()) { + const float downloadSizeMiB = missing.downloadSize / (1024.f * 1024.f); + const float narSizeMiB = missing.narSize / (1024.f * 1024.f); + if (missing.willSubstitute.size() == 1) { printMsg(lvl, "this path will be fetched (%.2f MiB download, %.2f MiB unpacked):", downloadSizeMiB, narSizeMiB); } else { printMsg(lvl, "these %d paths will be fetched (%.2f MiB download, %.2f MiB unpacked):", - willSubstitute.size(), + missing.willSubstitute.size(), downloadSizeMiB, narSizeMiB); } std::vector willSubstituteSorted = {}; - std::for_each(willSubstitute.begin(), willSubstitute.end(), + std::for_each(missing.willSubstitute.begin(), missing.willSubstitute.end(), [&](const StorePath &p) { willSubstituteSorted.push_back(&p); }); std::sort(willSubstituteSorted.begin(), willSubstituteSorted.end(), [](const StorePath *lhs, const StorePath *rhs) { @@ -95,10 +93,10 @@ void printMissing(ref store, const StorePathSet & willBuild, printMsg(lvl, " %s", store->printStorePath(*p)); } - if (!unknown.empty()) { + if (!missing.unknown.empty()) { printMsg(lvl, "don't know how to build these paths%s:", (settings.readOnlyMode ? " (may be caused by read-only store access)" : "")); - for (auto & i : unknown) + for (auto & i : missing.unknown) printMsg(lvl, " %s", store->printStorePath(i)); } } diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index 8a1ff40f0..79f21620e 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -100,6 +100,8 @@ test( this_exe, env : { '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data', + 'HOME': meson.current_build_dir() / 'test-home', + 'NIX_REMOTE': meson.current_build_dir() / 'test-home' / 'store', }, protocol : 'gtest', ) diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index 3d9f7908b..05373cb88 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -28,10 +28,6 @@ TEST_F(nix_api_store_test, nix_store_get_uri) TEST_F(nix_api_util_context, nix_store_get_storedir_default) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // skipping test in sandbox because nix_store_open tries to create /nix/var/nix/profiles - GTEST_SKIP(); - } nix_libstore_init(ctx); Store * store = nix_store_open(ctx, nullptr, nullptr); assert_ctx_ok(); @@ -141,10 +137,6 @@ TEST_F(nix_api_store_test, nix_store_real_path) TEST_F(nix_api_util_context, nix_store_real_path_relocated) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // Can't open default store from within sandbox - GTEST_SKIP(); - } auto tmp = nix::createTempDir(); std::string storeRoot = tmp + "/store"; std::string stateDir = tmp + "/state"; @@ -184,13 +176,7 @@ TEST_F(nix_api_util_context, nix_store_real_path_relocated) TEST_F(nix_api_util_context, nix_store_real_path_binary_cache) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // TODO: override NIX_CACHE_HOME? - // skipping test in sandbox because narinfo cache can't be written - GTEST_SKIP(); - } - - Store * store = nix_store_open(ctx, "https://cache.nixos.org", nullptr); + Store * store = nix_store_open(ctx, nix::fmt("file://%s/binary-cache", nix::createTempDir()).c_str(), nullptr); assert_ctx_ok(); ASSERT_NE(store, nullptr); diff --git a/src/libstore-tests/package.nix b/src/libstore-tests/package.nix index b39ee7fa7..f606604ba 100644 --- a/src/libstore-tests/package.nix +++ b/src/libstore-tests/package.nix @@ -3,6 +3,7 @@ buildPackages, stdenv, mkMesonExecutable, + writableTmpDirAsHomeHook, nix-store, nix-store-c, @@ -72,18 +73,14 @@ mkMesonExecutable (finalAttrs: { runCommand "${finalAttrs.pname}-run" { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; + buildInputs = [ writableTmpDirAsHomeHook ]; } - ( - lib.optionalString stdenv.hostPlatform.isWindows '' - export HOME="$PWD/home-dir" - mkdir -p "$HOME" - '' - + '' - export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} - ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} - touch $out - '' - ); + ('' + export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} + export NIX_REMOTE=$HOME/store + ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} + touch $out + ''); }; }; diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 9a91b3592..3019d9d72 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1,5 +1,5 @@ #include "nix/store/build/derivation-building-goal.hh" -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -72,7 +72,7 @@ std::string DerivationBuildingGoal::key() /* Ensure that derivations get built in order of their name, i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals always happen before - derivation goals (due to "b$"). */ + derivation goals (due to "bd$"). */ return "bd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); } @@ -266,7 +266,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() auto mEntry = get(inputGoals, drvPath); if (!mEntry) return std::nullopt; - auto buildResult = (*mEntry)->getBuildResult(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}}); + auto & buildResult = (*mEntry)->buildResult; if (!buildResult.success()) return std::nullopt; auto i = get(buildResult.builtOutputs, outputName); @@ -296,9 +296,11 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() worker.store.printStorePath(pathResolved), }); - // FIXME wanted outputs - auto resolvedDrvGoal = worker.makeDerivationGoal( - makeConstantStorePathRef(pathResolved), OutputsSpec::All{}, buildMode); + /* TODO https://github.com/NixOS/nix/issues/13247 we should + let the calling goal do this, so it has a change to pass + just the output(s) it cares about. */ + auto resolvedDrvGoal = worker.makeDerivationTrampolineGoal( + pathResolved, OutputsSpec::All{}, drvResolved, buildMode); { Goals waitees{resolvedDrvGoal}; co_await await(std::move(waitees)); @@ -306,20 +308,16 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() trace("resolved derivation finished"); - auto resolvedDrv = *resolvedDrvGoal->drv; - auto resolvedResult = resolvedDrvGoal->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(pathResolved), - .outputs = OutputsSpec::All{}, - }); + auto resolvedResult = resolvedDrvGoal->buildResult; SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); + auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); StorePathSet outputPaths; - for (auto & outputName : resolvedDrv.outputNames()) { + for (auto & outputName : drvResolved.outputNames()) { auto initialOutput = get(initialOutputs, outputName); auto resolvedHash = get(resolvedHashes, outputName); if ((!initialOutput) || (!resolvedHash)) @@ -340,7 +338,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", - resolvedDrvGoal->drvReq->to_string(worker.store), outputName); + worker.store.printStorePath(pathResolved), outputName); }(); if (!drv->type().isImpure()) { diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fcc376ed..128d360c9 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -24,35 +24,18 @@ namespace nix { -DerivationGoal::DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, loadDerivation()) - , drvReq(drvReq) - , wantedOutputs(wantedOutputs) - , buildMode(buildMode) -{ - name = fmt( - "building of '%s' from .drv file", - DerivedPath::Built { drvReq, wantedOutputs }.to_string(worker.store)); - trace("created"); - - mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); - worker.updateProgress(); -} - - -DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, haveDerivation(drvPath)) - , drvReq(makeConstantStorePathRef(drvPath)) - , wantedOutputs(wantedOutputs) +DerivationGoal::DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode) + : Goal(worker, haveDerivation()) + , drvPath(drvPath) + , wantedOutput(wantedOutput) , buildMode(buildMode) { this->drv = std::make_unique(drv); name = fmt( "building of '%s' from in-memory derivation", - DerivedPath::Built { drvReq, drv.outputNames() }.to_string(worker.store)); + DerivedPath::Built { makeConstantStorePathRef(drvPath), drv.outputNames() }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -61,114 +44,20 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation } -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 DerivationGoal::key() { /* Ensure that derivations get built in order of their name, i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals always happen before derivation goals (due to "b$"). */ - return "b$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); + return "b$" + std::string(drvPath.name()) + "$" + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(drvPath), + .output = wantedOutput, + }.to_string(worker.store); } -void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) -{ - auto newWanted = wantedOutputs.union_(outputs); - switch (needRestart) { - case NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed: - if (!newWanted.isSubsetOf(wantedOutputs)) - needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed; - break; - case NeedRestartForMoreOutputs::OutputsAddedDoNeed: - /* No need to check whether we added more outputs, because a - restart is already queued up. */ - break; - case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed: - /* We are already building all outputs, so it doesn't matter if - we now want more. */ - break; - }; - wantedOutputs = newWanted; -} - - -Goal::Co DerivationGoal::loadDerivation() { - trace("need to load derivation from file"); - - { - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be built from another - derivation, or merely substituted. We can make goal to get it - and not worry about which method it takes to get the - derivation. */ - - 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; - } - auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath); - return cond ? 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))); - } else { - trace("need to obtain drv we want to build"); - Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; - co_await await(std::move(waitees)); - } - - trace("loading derivation"); - - if (nrFailed != 0) { - co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); - } - - StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); - - /* `drvPath' should already be a root, but let's be on the safe - side: if the user forgot to make it a root, we wouldn't want - things being garbage collected while we're busy. */ - worker.evalStore.addTempRoot(drvPath); - - /* Get the derivation. It is probably in the eval store, but it might be inthe main store: - - - Resolved derivation are resolved against main store realisations, and so must be stored there. - - - Dynamic derivations are built, and so are found in the main store. - */ - for (auto * drvStore : { &worker.evalStore, &worker.store }) { - if (drvStore->isValidPath(drvPath)) { - drv = std::make_unique(drvStore->readDerivation(drvPath)); - break; - } - } - assert(drv); - - co_return haveDerivation(drvPath); - } -} - - -Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) +Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); @@ -205,18 +94,26 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) trace("outer build done"); - buildResult = g->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = wantedOutputs, - }); + buildResult = g->buildResult; if (buildMode == bmCheck) { /* In checking mode, the builder will not register any outputs. So we want to make sure the ones that we wanted to check are properly there. */ - buildResult.builtOutputs = assertPathValidity(drvPath); + buildResult.builtOutputs = assertPathValidity(); } + for (auto it = buildResult.builtOutputs.begin(); it != buildResult.builtOutputs.end(); ) { + if (it->first != wantedOutput) { + it = buildResult.builtOutputs.erase(it); + } else { + ++it; + } + } + + if (buildResult.success()) + assert(buildResult.builtOutputs.count(wantedOutput) > 0); + co_return amDone(g->exitCode, g->ex); }; @@ -261,11 +158,11 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) { /* Check what outputs paths are not already valid. */ - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { - co_return done(drvPath, BuildResult::AlreadyValid, std::move(validOutputs)); + co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); } } @@ -302,25 +199,20 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) assert(!drv->type().isImpure()); if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) { - co_return done(drvPath, BuildResult::TransientFailure, {}, + co_return done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); } nrFailed = nrNoSubstituters = 0; - if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { - needRestart = NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed; - co_return haveDerivation(std::move(drvPath)); - } - - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (buildMode == bmNormal && allValid) { - co_return done(drvPath, BuildResult::Substituted, std::move(validOutputs)); + co_return done(BuildResult::Substituted, std::move(validOutputs)); } if (buildMode == bmRepair && allValid) { - co_return repairClosure(std::move(drvPath)); + co_return repairClosure(); } if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", @@ -343,7 +235,7 @@ struct value_comparison }; -Goal::Co DerivationGoal::repairClosure(StorePath drvPath) +Goal::Co DerivationGoal::repairClosure() { assert(!drv->type().isImpure()); @@ -353,11 +245,10 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) that produced those outputs. */ /* Get the output closure. */ - auto outputs = queryDerivationOutputMap(drvPath); + auto outputs = queryDerivationOutputMap(); StorePathSet outputClosure; - for (auto & i : outputs) { - if (!wantedOutputs.contains(i.first)) continue; - worker.store.computeFSClosure(i.second, outputClosure); + if (auto mPath = get(outputs, wantedOutput)) { + worker.store.computeFSClosure(*mPath, outputClosure); } /* Filter out our own outputs (which we have already checked). */ @@ -411,11 +302,11 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) throw Error("some paths in the output closure of derivation '%s' could not be repaired", worker.store.printStorePath(drvPath)); } - co_return done(drvPath, BuildResult::AlreadyValid, assertPathValidity(drvPath)); + co_return done(BuildResult::AlreadyValid, assertPathValidity()); } -std::map> DerivationGoal::queryPartialDerivationOutputMap(const StorePath & drvPath) +std::map> DerivationGoal::queryPartialDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -431,7 +322,7 @@ std::map> DerivationGoal::queryPartialDeri return res; } -OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath) +OutputPathMap DerivationGoal::queryDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -447,28 +338,21 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath } -std::pair DerivationGoal::checkPathValidity(const StorePath & drvPath) +std::pair DerivationGoal::checkPathValidity() { if (drv->type().isImpure()) return { false, {} }; bool checkHash = buildMode == bmRepair; - auto wantedOutputsLeft = std::visit(overloaded { - [&](const OutputsSpec::All &) { - return StringSet {}; - }, - [&](const OutputsSpec::Names & names) { - return static_cast(names); - }, - }, wantedOutputs.raw); + StringSet wantedOutputsLeft{wantedOutput}; SingleDrvOutputs validOutputs; - for (auto & i : queryPartialDerivationOutputMap(drvPath)) { + for (auto & i : queryPartialDerivationOutputMap()) { auto initialOutput = get(initialOutputs, i.first); if (!initialOutput) // this is an invalid output, gets caught with (!wantedOutputsLeft.empty()) continue; auto & info = *initialOutput; - info.wanted = wantedOutputs.contains(i.first); + info.wanted = wantedOutput == i.first; if (info.wanted) wantedOutputsLeft.erase(i.first); if (i.second) { @@ -527,9 +411,9 @@ std::pair DerivationGoal::checkPathValidity(const StoreP } -SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) +SingleDrvOutputs DerivationGoal::assertPathValidity() { - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (!allValid) throw Error("some outputs are unexpectedly invalid"); return validOutputs; @@ -537,7 +421,6 @@ SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) Goal::Done DerivationGoal::done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional ex) @@ -553,7 +436,7 @@ Goal::Done DerivationGoal::done( mcExpectedBuilds.reset(); if (buildResult.success()) { - auto wantedBuiltOutputs = filterDrvOutputs(wantedOutputs, std::move(builtOutputs)); + auto wantedBuiltOutputs = filterDrvOutputs(OutputsSpec::Names{wantedOutput}, std::move(builtOutputs)); assert(!wantedBuiltOutputs.empty()); buildResult.builtOutputs = std::move(wantedBuiltOutputs); if (status == BuildResult::Built) diff --git a/src/libstore/build/derivation-trampoline-goal.cc b/src/libstore/build/derivation-trampoline-goal.cc new file mode 100644 index 000000000..e8ca47dfe --- /dev/null +++ b/src/libstore/build/derivation-trampoline-goal.cc @@ -0,0 +1,175 @@ +#include "nix/store/build/derivation-trampoline-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" + +namespace nix { + +DerivationTrampolineGoal::DerivationTrampolineGoal( + ref drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, init()) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +DerivationTrampolineGoal::DerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode) + : Goal(worker, haveDerivation(drvPath, drv)) + , drvReq(makeConstantStorePathRef(drvPath)) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +void DerivationTrampolineGoal::commonInit() +{ + 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(); +} + +DerivationTrampolineGoal::~DerivationTrampolineGoal() {} + +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 DerivationTrampolineGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals, derivation goals, and derivation building goals always happen before + derivation goals (due to "bt$"). */ + return "bt$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + DerivedPath::Built{ + .drvPath = drvReq, + .outputs = wantedOutputs, + }.to_string(worker.store); +} + +void DerivationTrampolineGoal::timedOut(Error && ex) {} + +Goal::Co DerivationTrampolineGoal::init() +{ + trace("need to load derivation from file"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be built from another derivation, + or merely substituted. We can make goal to get it and not worry + about which method it takes to get the derivation. */ + 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; + } + auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath); + return cond ? 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))); + } else { + trace("need to obtain drv we want to build"); + Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; + co_await await(std::move(waitees)); + } + + trace("outer load and build derivation"); + + if (nrFailed != 0) { + co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + } + + StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); + + /* `drvPath' should already be a root, but let's be on the safe + side: if the user forgot to make it a root, we wouldn't want + things being garbage collected while we're busy. */ + worker.evalStore.addTempRoot(drvPath); + + /* Get the derivation. It is probably in the eval store, but it might be in the main store: + + - Resolved derivation are resolved against main store realisations, and so must be stored there. + + - Dynamic derivations are built, and so are found in the main store. + */ + auto drv = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(drvPath)) + return drvStore->readDerivation(drvPath); + assert(false); + }(); + + co_return haveDerivation(std::move(drvPath), std::move(drv)); +} + +Goal::Co DerivationTrampolineGoal::haveDerivation(StorePath drvPath, Derivation drv) +{ + trace("have derivation, will kick off derivations goals per wanted output"); + + auto resolvedWantedOutputs = std::visit( + overloaded{ + [&](const OutputsSpec::Names & names) -> OutputsSpec::Names { return names; }, + [&](const OutputsSpec::All &) -> OutputsSpec::Names { + StringSet outputs; + for (auto & [outputName, _] : drv.outputs) + outputs.insert(outputName); + return outputs; + }, + }, + wantedOutputs.raw); + + Goals concreteDrvGoals; + + /* Build this step! */ + + for (auto & output : resolvedWantedOutputs) { + auto g = upcast_goal(worker.makeDerivationGoal(drvPath, drv, output, buildMode)); + g->preserveException = true; + /* We will finish with it ourselves, as if we were the derivational goal. */ + concreteDrvGoals.insert(std::move(g)); + } + + // Copy on purpose + co_await await(Goals(concreteDrvGoals)); + + trace("outer build done"); + + auto & g = *concreteDrvGoals.begin(); + buildResult = g->buildResult; + for (auto & g2 : concreteDrvGoals) { + for (auto && [x, y] : g2->buildResult.builtOutputs) + buildResult.builtOutputs.insert_or_assign(x, y); + } + + co_return amDone(g->exitCode, g->ex); +} + +} diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 39fd471c4..6c842554c 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,8 +1,7 @@ +#include "nix/store/derivations.hh" #include "nix/store/build/worker.hh" #include "nix/store/build/substitution-goal.hh" -#ifndef _WIN32 // TODO Enable building on Windows -# include "nix/store/build/derivation-goal.hh" -#endif +#include "nix/store/build/derivation-trampoline-goal.hh" #include "nix/store/local-store.hh" #include "nix/util/strings.hh" @@ -28,12 +27,9 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { -#ifndef _WIN32 // TODO Enable building on Windows - if (auto i2 = dynamic_cast(i.get())) + if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvReq->to_string(*this)); - else -#endif - if (auto i2 = dynamic_cast(i.get())) + else if (auto i2 = dynamic_cast(i.get())) failed.insert(printStorePath(i2->storePath)); } } @@ -70,7 +66,7 @@ std::vector Store::buildPathsWithResults( for (auto & [req, goalPtr] : state) results.emplace_back(KeyedBuildResult { - goalPtr->getBuildResult(req), + goalPtr->buildResult, /* .path = */ req, }); @@ -81,19 +77,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { Worker worker(*this, *this); -#ifndef _WIN32 // TODO Enable building on Windows - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode); -#else - std::shared_ptr goal; - throw UnimplementedError("Building derivations not yet implemented on windows."); -#endif + auto goal = worker.makeDerivationTrampolineGoal(drvPath, OutputsSpec::All {}, drv, buildMode); try { worker.run(Goals{goal}); - return goal->getBuildResult(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = OutputsSpec::All {}, - }); + return goal->buildResult; } catch (Error & e) { return BuildResult { .status = BuildResult::MiscFailure, diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 8a8d79283..88b0c28c0 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -101,30 +101,6 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { return s1 < s2; } - -BuildResult Goal::getBuildResult(const DerivedPath & req) const { - BuildResult res { buildResult }; - - if (auto pbp = std::get_if(&req)) { - auto & bp = *pbp; - - /* Because goals are in general shared between derived paths - that share the same derivation, we need to filter their - results to get back just the results we care about. - */ - - for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { - if (bp.outputs.contains(it->first)) - ++it; - else - it = res.builtOutputs.erase(it); - } - } - - return res; -} - - void addToWeakGoals(WeakGoals & goals, GoalPtr p) { if (goals.find(p) != goals.end()) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index a3419d9da..2b901f818 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -5,6 +5,7 @@ #include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-building-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows # include "nix/store/build/hook-instance.hh" #endif @@ -53,52 +54,40 @@ std::shared_ptr Worker::initGoalIfNeeded(std::weak_ptr & goal_weak, Args & return goal; } -std::shared_ptr Worker::makeDerivationGoalCommon( +std::shared_ptr Worker::makeDerivationTrampolineGoal( ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal) + BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationGoals.ensureSlot(*drvReq).value; - std::shared_ptr goal = goal_weak.lock(); - if (!goal) { - goal = mkDrvGoal(); - goal_weak = goal; - wakeUp(goal); - } else { - goal->addWantedOutputs(wantedOutputs); - } - return goal; + return initGoalIfNeeded( + derivationTrampolineGoals.ensureSlot(*drvReq).value[wantedOutputs], + drvReq, wantedOutputs, *this, buildMode); } -std::shared_ptr Worker::makeDerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode) { - return makeDerivationGoalCommon(drvReq, wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvReq, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded( + derivationTrampolineGoals.ensureSlot(DerivedPath::Opaque{drvPath}).value[wantedOutputs], + drvPath, wantedOutputs, drv, *this, buildMode); } -std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode) + +std::shared_ptr Worker::makeDerivationGoal(const StorePath & drvPath, + const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode) { - return makeDerivationGoalCommon(makeConstantStorePathRef(drvPath), wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode); } std::shared_ptr Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationBuildingGoals[drvPath]; - auto goal = goal_weak.lock(); // FIXME - if (!goal) { - goal = std::make_shared(drvPath, drv, *this, buildMode); - goal_weak = goal; - wakeUp(goal); - } - return goal; + return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode); } @@ -118,7 +107,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode); + return makeDerivationTrampolineGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -126,46 +115,52 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) }, req.raw()); } - -template -static void cullMap(std::map & goalMap, F f) +/** + * This function is polymorphic (both via type parameters and + * overloading) and recursive in order to work on a various types of + * trees + * + * @return Whether the tree node we are processing is not empty / should + * be kept alive. In the case of this overloading the node in question + * is the leaf, the weak reference itself. If the weak reference points + * to the goal we are looking for, our caller can delete it. In the + * inductive case where the node is an interior node, we'll likewise + * return whether the interior node is non-empty. If it is empty + * (because we just deleted its last child), then our caller can + * likewise delete it. + */ +template +static bool removeGoal(std::shared_ptr goal, std::weak_ptr & gp) { - for (auto i = goalMap.begin(); i != goalMap.end();) - if (!f(i->second)) + return gp.lock() != goal; +} + +template +static bool removeGoal(std::shared_ptr goal, std::map & goalMap) +{ + /* !!! inefficient */ + for (auto i = goalMap.begin(); i != goalMap.end();) { + if (!removeGoal(goal, i->second)) i = goalMap.erase(i); - else ++i; + else + ++i; + } + return !goalMap.empty(); } - -template -static void removeGoal(std::shared_ptr goal, std::map> & goalMap) +template +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>>::ChildNode & node) { - /* !!! inefficient */ - 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(); - }); + return removeGoal(goal, node.value) || removeGoal(goal, node.childMap); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) - nix::removeGoal(drvGoal, derivationGoals.map); + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationTrampolineGoals.map); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationGoals); else if (auto drvBuildingGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvBuildingGoal, derivationBuildingGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) @@ -312,21 +307,18 @@ 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 = goal->drvReq, .outputs = goal->wantedOutputs, }); - } else - if (auto goal = dynamic_cast(i.get())) { + } else if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Opaque{goal->storePath}); } } /* Call queryMissing() to efficiently query substitutes. */ - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize); + store.queryMissing(topPaths); debug("entered goal loop"); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 4bca75228..b946ccbb5 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -949,14 +949,12 @@ static void performOp(TunnelLogger * logger, ref store, case WorkerProto::Op::QueryMissing: { auto targets = WorkerProto::Serialise::read(*store, rconn); logger->startWork(); - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store->queryMissing(targets, willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = store->queryMissing(targets); logger->stopWork(); - WorkerProto::write(*store, wconn, willBuild); - WorkerProto::write(*store, wconn, willSubstitute); - WorkerProto::write(*store, wconn, unknown); - conn.to << downloadSize << narSize; + WorkerProto::write(*store, wconn, missing.willBuild); + WorkerProto::write(*store, wconn, missing.willSubstitute); + WorkerProto::write(*store, wconn, missing.unknown); + conn.to << missing.downloadSize << missing.narSize; break; } diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index 408d1a6b9..e34deb744 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -52,7 +52,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" namespace nix { template<> @@ -69,7 +69,7 @@ std::strong_ordering DerivedPathMap::ChildNode::operator <=> ( template struct DerivedPathMap::ChildNode; template struct DerivedPathMap; -template struct DerivedPathMap>; +template struct DerivedPathMap>>; }; diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index de5128347..1f80cb379 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -140,7 +140,7 @@ std::vector getUserConfigFiles() return files; } -unsigned int Settings::getDefaultCores() +unsigned int Settings::getDefaultCores() const { const unsigned int concurrency = std::max(1U, std::thread::hardware_concurrency()); const unsigned int maxCPU = getMaxCPU(); diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index bff2e7a89..569d1ddbb 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -29,7 +29,9 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for building a derivation. Substitution, (or any other method of + * obtaining the outputs) will not be attempted, so it is the calling goal's + * responsibility to try to substitute first. */ struct DerivationBuildingGoal : public Goal { diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 9d4257cb3..ac9ec5346 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -22,47 +22,33 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for realising a single output of a derivation. Various sorts of + * fetching (which will be done by other goal types) is tried, and if none of + * those succeed, the derivation is attempted to be built. + * + * This is a purely "administrative" goal type, which doesn't do any + * "real work" of substituting (that would be `PathSubstitutionGoal` or + * `DrvOutputSubstitutionGoal`) or building (that would be a + * `DerivationBuildingGoal`). This goal type creates those types of + * goals to attempt each way of realisation a derivation; they are tried + * sequentially in order of preference. + * + * The derivation must already be gotten (in memory, in C++, parsed) and passed + * to the caller. If the derivation itself needs to be gotten first, a + * `DerivationTrampolineGoal` goal must be used instead. */ struct DerivationGoal : public Goal { /** The path of the derivation. */ - ref drvReq; + StorePath drvPath; /** * The specific outputs that we need to build. */ - OutputsSpec wantedOutputs; + OutputName wantedOutput; /** - * See `needRestart`; just for that field. - */ - enum struct NeedRestartForMoreOutputs { - /** - * The goal state machine is progressing based on the current value of - * `wantedOutputs. No actions are needed. - */ - OutputsUnmodifiedDontNeed, - /** - * `wantedOutputs` has been extended, but the state machine is - * proceeding according to its old value, so we need to restart. - */ - OutputsAddedDoNeed, - /** - * The goal state machine has progressed to the point of doing a build, - * in which case all outputs will be produced, so extensions to - * `wantedOutputs` no longer require a restart. - */ - BuildInProgressWillNotNeed, - }; - - /** - * Whether additional wanted outputs have been added. - */ - NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed; - - /** - * The derivation stored at `drvReq`. + * The derivation stored at drvPath. */ std::unique_ptr drv; @@ -76,11 +62,8 @@ struct DerivationGoal : public Goal std::unique_ptr> mcExpectedBuilds; - DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, - BuildMode buildMode = bmNormal); - DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, + DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal() = default; @@ -88,24 +71,18 @@ struct DerivationGoal : public Goal std::string key() override; - /** - * Add wanted outputs to an already existing derivation goal. - */ - void addWantedOutputs(const OutputsSpec & outputs); - /** * The states. */ - Co loadDerivation(); - Co haveDerivation(StorePath drvPath); + Co haveDerivation(); /** * Wrappers around the corresponding Store methods that first consult the * derivation. This is currently needed because when there is no drv file * there also is no DB entry. */ - std::map> queryPartialDerivationOutputMap(const StorePath & drvPath); - OutputPathMap queryDerivationOutputMap(const StorePath & drvPath); + std::map> queryPartialDerivationOutputMap(); + OutputPathMap queryDerivationOutputMap(); /** * Update 'initialOutputs' to determine the current status of the @@ -113,18 +90,17 @@ struct DerivationGoal : public Goal * whether all outputs are valid and non-corrupt, and a * 'SingleDrvOutputs' structure containing the valid outputs. */ - std::pair checkPathValidity(const StorePath & drvPath); + std::pair checkPathValidity(); /** * Aborts if any output is not valid or corrupt, and otherwise * returns a 'SingleDrvOutputs' structure containing all outputs. */ - SingleDrvOutputs assertPathValidity(const StorePath & drvPath); + SingleDrvOutputs assertPathValidity(); - Co repairClosure(StorePath drvPath); + Co repairClosure(); Done done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional ex = {}); diff --git a/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh new file mode 100644 index 000000000..648337695 --- /dev/null +++ b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh @@ -0,0 +1,134 @@ +#pragma once +///@file + +#include "nix/store/parsed-derivations.hh" +#include "nix/store/store-api.hh" +#include "nix/store/pathlocks.hh" +#include "nix/store/build/goal.hh" + +namespace nix { + +/** + * This is the "outermost" goal type relating to derivations --- by that + * we mean that this one calls all the others for a given derivation. + * + * This is a purely "administrative" goal type, which doesn't do any "real + * work". See `DerivationGoal` for what we mean by such an administrative goal. + * + * # Rationale + * + * It exists to solve two problems: + * + * 1. We want to build a derivation we don't yet have. + * + * Traditionally, that simply means we try to substitute the missing + * derivation; simple enough. However, with (currently experimental) + * dynamic derivations, derivations themselves can be the outputs of + * other derivations. That means the general case is that a + * `DerivationTrampolineGoal` needs to create *another* + * `DerivationTrampolineGoal` goal to realize the derivation it needs. + * That goal in turn might need to create a third + * `DerivationTrampolineGoal`, the induction down to a statically known + * derivation as the base case is arbitrary deep. + * + * 2. Only a subset of outputs is needed, but such subsets are discovered + * dynamically. + * + * Consider derivations: + * + * - A has outputs x, y, and z + * + * - B needs A^x,y + * + * - C needs A^y,z and B's single output + * + * With the current `Worker` architecture, we're first discover + * needing `A^y,z` and then discover needing `A^x,y`. Of course, we + * don't want to download `A^y` twice, either. + * + * The way we handle sharing work for `A^y` is to have + * `DerivationGoal` just handle a single output, and do slightly more + * work (though it is just an "administrative" goal too), and + * `DerivationTrampolineGoal` handle sets of goals, but have it (once the + * derivation itself has been gotten) *just* create + * `DerivationGoal`s. + * + * That means it is fine to create man `DerivationTrampolineGoal` with + * overlapping sets of outputs, because all the "real work" will be + * coordinated via `DerivationGoal`s, and sharing will be discovered. + * + * Both these problems *can* be solved by having just a more powerful + * `DerivationGoal`, but that makes `DerivationGoal` more complex. + * However the more complex `DerivationGoal` has these downsides: + * + * 1. It needs to cope with only sometimes knowing a `StorePath drvPath` + * (as opposed to a more general `SingleDerivedPath drvPath` with will + * be only resolved to a `StorePath` part way through the control flow). + * + * 2. It needs complicated "restarting logic" to cope with the set of + * "wanted outputs" growing over time. + * + * (1) is not so bad, but (2) is quite scary, and has been a source of + * bugs in the past. By splitting out `DerivationTrampolineGoal`, we + * crucially avoid a need for (2), letting goal sharing rather than + * ad-hoc retry mechanisms accomplish the deduplication we need. Solving + * (1) is just a by-product and extra bonus of creating + * `DerivationTrampolineGoal`. + * + * # Misc Notes + * + * If we already have the derivation (e.g. if the evaluator has created + * the derivation locally and then instructed the store to build it), we + * can skip the derivation-getting goal entirely as a small + * optimization. + */ +struct DerivationTrampolineGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + DerivationTrampolineGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode = bmNormal); + + DerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode = bmNormal); + + virtual ~DerivationTrampolineGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + + BuildMode buildMode; + + Co init(); + Co haveDerivation(StorePath drvPath, Derivation drv); + + /** + * Shared between both constructors + */ + void commonInit(); +}; + +} diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index 577ce1e84..ee69c9cc7 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -105,13 +105,11 @@ public: */ ExitCode exitCode = ecBusy; -protected: /** * Build result. */ BuildResult buildResult; -public: /** * Suspend our goal and wait until we get `work`-ed again. * `co_await`-able by @ref Co. @@ -358,18 +356,6 @@ protected: public: virtual void cleanup() { } - /** - * Project a `BuildResult` with just the information that pertains - * to the given request. - * - * In general, goals may be aliased between multiple requests, and - * the stored `BuildResult` has information for the union of all - * requests. We don't want to leak what the other request are for - * sake of both privacy and determinism, and this "safe accessor" - * ensures we don't. - */ - BuildResult getBuildResult(const DerivedPath &) const; - /** * Hack to say that this goal should not log `ex`, but instead keep * it around. Set by a waitee which sees itself as the designated diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index c70c72377..491b8f494 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -14,6 +14,7 @@ namespace nix { /* Forward definition. */ +struct DerivationTrampolineGoal; struct DerivationGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; @@ -33,6 +34,7 @@ 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; @@ -106,8 +108,9 @@ private: * same derivation / path. */ - DerivedPathMap> derivationGoals; + DerivedPathMap>> derivationTrampolineGoals; + std::map>> derivationGoals; std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -204,16 +207,20 @@ private: template std::shared_ptr initGoalIfNeeded(std::weak_ptr & goal_weak, Args && ...args); - std::shared_ptr makeDerivationGoalCommon( - ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal); -public: - std::shared_ptr makeDerivationGoal( + std::shared_ptr makeDerivationTrampolineGoal( ref drvReq, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); - std::shared_ptr makeBasicDerivationGoal( - const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); + +public: + std::shared_ptr makeDerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode = bmNormal); + + std::shared_ptr makeDerivationGoal( + const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, BuildMode buildMode = bmNormal); /** * @ref DerivationBuildingGoal "derivation goal" @@ -232,7 +239,7 @@ public: * Make a goal corresponding to the `DerivedPath`. * * It will be a `DerivationGoal` for a `DerivedPath::Built` or - * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + * a `PathSubstitutionGoal` for a `DerivedPath::Opaque`. */ GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); diff --git a/src/libstore/include/nix/store/derived-path-map.hh b/src/libstore/include/nix/store/derived-path-map.hh index 16ffeb05e..6dae73fab 100644 --- a/src/libstore/include/nix/store/derived-path-map.hh +++ b/src/libstore/include/nix/store/derived-path-map.hh @@ -22,7 +22,7 @@ namespace nix { * @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 + * `DerivedPathMap>` to * remember which goals correspond to which outputs. `* const Something` * or `std::optional` would also be good choices for * "optional" types. diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 0ac689b55..3f9d8697e 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -43,8 +43,6 @@ const uint32_t maxIdsPerBuild = class Settings : public Config { - unsigned int getDefaultCores(); - StringSet getDefaultSystemFeatures(); StringSet getDefaultExtraPlatforms(); @@ -57,6 +55,8 @@ public: Settings(); + unsigned int getDefaultCores() const; + Path nixPrefix; /** @@ -153,7 +153,7 @@ public: Setting buildCores{ this, - getDefaultCores(), + 0, "cores", R"( Sets the value of the `NIX_BUILD_CORES` environment variable in the [invocation of the `builder` executable](@docroot@/language/derivations.md#builder-execution) of a derivation. @@ -166,15 +166,13 @@ public: --> For instance, in Nixpkgs, if the attribute `enableParallelBuilding` for the `mkDerivation` build helper is set to `true`, it passes the `-j${NIX_BUILD_CORES}` flag to GNU Make. - The value `0` means that the `builder` should use all available CPU cores in the system. + If set to `0`, nix will detect the number of CPU cores and pass this number via NIX_BUILD_CORES. > **Note** > > The number of parallel local Nix build jobs is independently controlled with the [`max-jobs`](#conf-max-jobs) setting. )", - {"build-cores"}, - // Don't document the machine-specific default value - false}; + {"build-cores"}}; /** * Read-only mode. Don't copy stuff to the store, don't change @@ -873,7 +871,7 @@ public: On Linux, Nix can run builds in a user namespace where they run as root (UID 0) and have 65,536 UIDs available. This is primarily useful for running containers such as `systemd-nspawn` inside a Nix build. For an example, see [`tests/systemd-nspawn/nix`][nspawn]. - [nspawn]: https://github.com/NixOS/nix/blob/67bcb99700a0da1395fa063d7c6586740b304598/tests/systemd-nspawn.nix. + [nspawn]: https://github.com/NixOS/nix/blob/67bcb99700a0da1395fa063d7c6586740b304598/tests/systemd-nspawn.nix Included by default on Linux if the [`auto-allocate-uids`](#conf-auto-allocate-uids) setting is enabled. )", diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index fd7e6fc36..abaf1b981 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -435,7 +435,6 @@ private: void addBuildLog(const StorePath & drvPath, std::string_view log) override; friend struct PathSubstitutionGoal; - friend struct SubstitutionGoal; friend struct DerivationGoal; }; diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index a18430417..44d9815de 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -15,6 +15,7 @@ headers = [config_pub_h] + files( 'build/derivation-goal.hh', 'build/derivation-building-goal.hh', 'build/derivation-building-misc.hh', + 'build/derivation-trampoline-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh', diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index dd2396fe3..18c02456f 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -149,9 +149,7 @@ struct RemoteStore : void addSignatures(const StorePath & storePath, const StringSet & sigs) override; - void queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize) override; + MissingPaths queryMissing(const std::vector & targets) override; void addBuildLog(const StorePath & drvPath, std::string_view log) override; diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 72ca1f437..e0a3e67d1 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -71,6 +71,18 @@ struct KeyedBuildResult; typedef std::map> StorePathCAMap; +/** + * Information about what paths will be built or substituted, returned + * by Store::queryMissing(). + */ +struct MissingPaths +{ + StorePathSet willBuild; + StorePathSet willSubstitute; + StorePathSet unknown; + uint64_t downloadSize{0}; + uint64_t narSize{0}; +}; /** * About the class hierarchy of the store types: @@ -694,9 +706,7 @@ public: * derivations that will be built, and the set of output paths that * will be substituted. */ - virtual void queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize); + virtual MissingPaths queryMissing(const std::vector & targets); /** * Sort a set of paths topologically under the references diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 2f23135fa..03a9bd055 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -39,7 +39,6 @@ struct LocalBinaryCacheStore : , BinaryCacheStore{*config} , config{config} { - init(); } void init() override; @@ -126,10 +125,12 @@ StringSet LocalBinaryCacheStoreConfig::uriSchemes() } ref LocalBinaryCacheStoreConfig::openStore() const { - return make_ref(ref{ + auto store = make_ref(ref{ // FIXME we shouldn't actually need a mutable config std::const_pointer_cast(shared_from_this()) }); + store->init(); + return store; } static RegisterStoreImplementation regLocalBinaryCacheStore; diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 94b8951fd..d82bcddc1 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -255,6 +255,7 @@ sources = files( 'build-result.cc', 'build/derivation-goal.cc', 'build/derivation-building-goal.cc', + 'build/derivation-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc', diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index dabae647f..7c97dbc57 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -98,23 +98,17 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv) return nullptr; } -void Store::queryMissing(const std::vector & targets, - StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_, - uint64_t & downloadSize_, uint64_t & narSize_) +MissingPaths Store::queryMissing(const std::vector & targets) { Activity act(*logger, lvlDebug, actUnknown, "querying info about missing paths"); - downloadSize_ = narSize_ = 0; - // FIXME: make async. ThreadPool pool(fileTransferSettings.httpConnections); struct State { std::unordered_set done; - StorePathSet & unknown, & willSubstitute, & willBuild; - uint64_t & downloadSize; - uint64_t & narSize; + MissingPaths res; }; struct DrvState @@ -125,7 +119,7 @@ void Store::queryMissing(const std::vector & targets, DrvState(size_t left) : left(left) { } }; - Sync state_(State{{}, unknown_, willSubstitute_, willBuild_, downloadSize_, narSize_}); + Sync state_; std::function doPath; @@ -143,7 +137,7 @@ void Store::queryMissing(const std::vector & targets, auto mustBuildDrv = [&](const StorePath & drvPath, const Derivation & drv) { { auto state(state_.lock()); - state->willBuild.insert(drvPath); + state->res.willBuild.insert(drvPath); } for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) { @@ -203,7 +197,7 @@ void Store::queryMissing(const std::vector & targets, if (!isValidPath(drvPath)) { // FIXME: we could try to substitute the derivation. auto state(state_.lock()); - state->unknown.insert(drvPath); + state->res.unknown.insert(drvPath); return; } @@ -282,7 +276,7 @@ void Store::queryMissing(const std::vector & targets, if (infos.empty()) { auto state(state_.lock()); - state->unknown.insert(bo.path); + state->res.unknown.insert(bo.path); return; } @@ -291,9 +285,9 @@ void Store::queryMissing(const std::vector & targets, { auto state(state_.lock()); - state->willSubstitute.insert(bo.path); - state->downloadSize += info->second.downloadSize; - state->narSize += info->second.narSize; + state->res.willSubstitute.insert(bo.path); + state->res.downloadSize += info->second.downloadSize; + state->res.narSize += info->second.narSize; } for (auto & ref : info->second.references) @@ -306,6 +300,8 @@ void Store::queryMissing(const std::vector & targets, pool.enqueue(std::bind(doPath, path)); pool.process(); + + return std::move(state_.lock()->res); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 3151f319c..1b8bad048 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -855,9 +855,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s } -void RemoteStore::queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize) +MissingPaths RemoteStore::queryMissing(const std::vector & targets) { { auto conn(getConnection()); @@ -868,16 +866,16 @@ void RemoteStore::queryMissing(const std::vector & targets, conn->to << WorkerProto::Op::QueryMissing; WorkerProto::write(*this, *conn, targets); conn.processStderr(); - willBuild = WorkerProto::Serialise::read(*this, *conn); - willSubstitute = WorkerProto::Serialise::read(*this, *conn); - unknown = WorkerProto::Serialise::read(*this, *conn); - conn->from >> downloadSize >> narSize; - return; + MissingPaths res; + res.willBuild = WorkerProto::Serialise::read(*this, *conn); + res.willSubstitute = WorkerProto::Serialise::read(*this, *conn); + res.unknown = WorkerProto::Serialise::read(*this, *conn); + conn->from >> res.downloadSize >> res.narSize; + return res; } fallback: - return Store::queryMissing(targets, willBuild, willSubstitute, - unknown, downloadSize, narSize); + return Store::queryMissing(targets); } diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index 0485f5584..69435122a 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -143,13 +143,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor unsupported("addSignatures"); } - void queryMissing( - const std::vector & targets, - StorePathSet & willBuild, - StorePathSet & willSubstitute, - StorePathSet & unknown, - uint64_t & downloadSize, - uint64_t & narSize) override; + MissingPaths queryMissing(const std::vector & targets) override; virtual std::optional getBuildLogExact(const StorePath & path) override { @@ -306,19 +300,14 @@ std::vector RestrictedStore::buildPathsWithResults( return results; } -void RestrictedStore::queryMissing( - const std::vector & targets, - StorePathSet & willBuild, - StorePathSet & willSubstitute, - StorePathSet & unknown, - uint64_t & downloadSize, - uint64_t & narSize) +MissingPaths RestrictedStore::queryMissing(const std::vector & targets) { /* This is slightly impure since it leaks information to the client about what paths will be built/substituted or are already present. Probably not a big deal. */ std::vector allowed; + StorePathSet unknown; for (auto & req : targets) { if (goal.isAllowed(req)) allowed.emplace_back(req); @@ -326,7 +315,12 @@ void RestrictedStore::queryMissing( unknown.insert(pathPartOfReq(req)); } - next->queryMissing(allowed, willBuild, willSubstitute, unknown, downloadSize, narSize); + auto res = next->queryMissing(allowed); + + for (auto & p : unknown) + res.unknown.insert(p); + + return res; } } diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 618112d1c..9bb47a010 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -37,10 +37,11 @@ namespace nix { struct S3Error : public Error { Aws::S3::S3Errors err; + Aws::String exceptionName; template - S3Error(Aws::S3::S3Errors err, const Args & ... args) - : Error(args...), err(err) { }; + S3Error(Aws::S3::S3Errors err, Aws::String exceptionName, const Args & ... args) + : Error(args...), err(err), exceptionName(exceptionName) { }; }; /* Helper: given an Outcome, return R in case of success, or @@ -51,6 +52,7 @@ R && checkAws(std::string_view s, Aws::Utils::Outcome && outcome) if (!outcome.IsSuccess()) throw S3Error( outcome.GetError().GetErrorType(), + outcome.GetError().GetExceptionName(), fmt( "%s: %s (request id: %s)", s, @@ -226,7 +228,13 @@ S3Helper::FileTransferResult S3Helper::getObject( } catch (S3Error & e) { if ((e.err != Aws::S3::S3Errors::NO_SUCH_KEY) && - (e.err != Aws::S3::S3Errors::ACCESS_DENIED)) throw; + (e.err != Aws::S3::S3Errors::ACCESS_DENIED) && + // Expired tokens are not really an error, more of a caching problem. Should be treated same as 403. + // + // AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843) + // so use this hack + (e.exceptionName != "ExpiredToken") + ) throw; } auto now2 = std::chrono::steady_clock::now(); @@ -281,8 +289,6 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore , s3Helper(config->profile, config->region, config->scheme, config->endpoint) { diskCache = getNarInfoDiskCache(); - - init(); } std::string getUri() override @@ -334,6 +340,10 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore auto & error = res.GetError(); if (error.GetErrorType() == Aws::S3::S3Errors::RESOURCE_NOT_FOUND || error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_KEY + // Expired tokens are not really an error, more of a caching problem. Should be treated same as 403. + // AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843) + // so use this hack + || (error.GetErrorType() == Aws::S3::S3Errors::UNKNOWN && error.GetExceptionName() == "ExpiredToken") // If bucket listing is disabled, 404s turn into 403s || error.GetErrorType() == Aws::S3::S3Errors::ACCESS_DENIED) return false; @@ -585,10 +595,12 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore ref S3BinaryCacheStoreImpl::Config::openStore() const { - return make_ref(ref{ + auto store = make_ref(ref{ // FIXME we shouldn't actually need a mutable config std::const_pointer_cast(shared_from_this()) }); + store->init(); + return store; } static RegisterStoreImplementation regS3BinaryCacheStore; diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 55b967ed6..c3fb1f413 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -250,7 +250,7 @@ void handleSQLiteBusy(const SQLiteBusy & e, time_t & nextWarning) if (now > nextWarning) { nextWarning = now + 10; logWarning({ - .msg = HintFmt(e.what()) + .msg = e.info().msg }); } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 296f2251a..9aeab1d1f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -790,15 +790,12 @@ void Store::substitutePaths(const StorePathSet & paths) for (auto & path : paths) if (!path.isDerivation()) paths2.emplace_back(DerivedPath::Opaque{path}); - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - queryMissing(paths2, - willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = queryMissing(paths2); - if (!willSubstitute.empty()) + if (!missing.willSubstitute.empty()) try { std::vector subs; - for (auto & p : willSubstitute) subs.emplace_back(DerivedPath::Opaque{p}); + for (auto & p : missing.willSubstitute) subs.emplace_back(DerivedPath::Opaque{p}); buildPaths(subs); } catch (Error & e) { logWarning(e.info()); diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 5e06dbe55..3985498c1 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -160,6 +160,8 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") { Strings sandboxArgs; + sandboxArgs.push_back("_NIX_BUILD_TOP"); + sandboxArgs.push_back(tmpDir); sandboxArgs.push_back("_GLOBAL_TMP_DIR"); sandboxArgs.push_back(globalTmpDir); if (drvOptions.allowLocalNetworking) { diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index fd62aa664..cf6c0a5b1 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1083,7 +1083,7 @@ void DerivationBuilderImpl::initEnv() env["NIX_STORE"] = store.storeDir; /* The maximum number of cores to utilize for parallel building. */ - env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores); + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); /* In non-structured mode, set all bindings either directory in the environment or via a file, as specified by diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index c27b87163..b23c8003f 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -368,6 +368,13 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) throw SysError("cannot change ownership of '%1%'", chrootStoreDir); + pathsInChroot = getPathsInSandbox(); + + for (auto & i : inputPaths) { + auto p = store.printStorePath(i); + pathsInChroot.insert_or_assign(p, store.toRealPath(p)); + } + /* If we're repairing, checking or rebuilding part of a multiple-outputs derivation, it's possible that we're rebuilding a path that is in settings.sandbox-paths @@ -391,13 +398,6 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder chownToBuilder(*cgroup + "/cgroup.threads"); // chownToBuilder(*cgroup + "/cgroup.subtree_control"); } - - pathsInChroot = getPathsInSandbox(); - - for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - pathsInChroot.insert_or_assign(p, store.toRealPath(p)); - } } Strings getPreBuildHookArgs() override diff --git a/src/libstore/unix/build/sandbox-defaults.sb b/src/libstore/unix/build/sandbox-defaults.sb index 15cd6daf5..dd6a064c1 100644 --- a/src/libstore/unix/build/sandbox-defaults.sb +++ b/src/libstore/unix/build/sandbox-defaults.sb @@ -29,12 +29,14 @@ R""( ; Allow getpwuid. (allow mach-lookup (global-name "com.apple.system.opendirectoryd.libinfo")) -; Access to /tmp. +; Access to /tmp and the build directory. ; The network-outbound/network-inbound ones are for unix domain sockets, which ; we allow access to in TMPDIR (but if we allow them more broadly, you could in ; theory escape the sandbox) (allow file* process-exec network-outbound network-inbound - (literal "/tmp") (subpath TMPDIR)) + (literal "/tmp") + (subpath TMPDIR) + (subpath (param "_NIX_BUILD_TOP"))) ; Some packages like to read the system version. (allow file-read* diff --git a/src/libstore/unix/user-lock.cc b/src/libstore/unix/user-lock.cc index 6a07cb7cc..f5d164e5b 100644 --- a/src/libstore/unix/user-lock.cc +++ b/src/libstore/unix/user-lock.cc @@ -197,7 +197,7 @@ bool useBuildUsers() #ifdef __linux__ static bool b = (settings.buildUsersGroup != "" || settings.autoAllocateUids) && isRootUser(); return b; - #elif defined(__APPLE__) && defined(__FreeBSD__) + #elif defined(__APPLE__) || defined(__FreeBSD__) static bool b = settings.buildUsersGroup != "" && isRootUser(); return b; #else diff --git a/src/libutil/meson.build b/src/libutil/meson.build index f5ad2b1f6..f48c8f3d7 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -59,6 +59,7 @@ boost = dependency( 'boost', modules : ['context', 'coroutine', 'iostreams'], include_type: 'system', + version: '>=1.82.0' ) # boost is a public dependency, but not a pkg-config dependency unfortunately, so we # put in `deps_other`. diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 55397c6d4..a74531582 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -194,10 +194,6 @@ size_t StringSource::read(char * data, size_t len) } -#if BOOST_VERSION >= 106300 && BOOST_VERSION < 106600 -#error Coroutines are broken in this version of Boost! -#endif - std::unique_ptr sourceToSink(std::function fun) { struct SourceToSink : FinishSink diff --git a/src/libutil/util.cc b/src/libutil/util.cc index c9cc80fef..23dafe8c9 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -190,8 +190,10 @@ void ignoreExceptionInDestructor(Verbosity lvl) try { try { throw; + } catch (Error & e) { + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.info().msg); } catch (std::exception & e) { - printMsg(lvl, "error (ignored): %1%", e.what()); + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.what()); } } catch (...) { } } @@ -202,8 +204,10 @@ void ignoreExceptionExceptInterrupt(Verbosity lvl) throw; } catch (const Interrupted & e) { throw; + } catch (Error & e) { + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.info().msg); } catch (std::exception & e) { - printMsg(lvl, "error (ignored): %1%", e.what()); + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.what()); } } diff --git a/src/libutil/windows/windows-async-pipe.cc b/src/libutil/windows/windows-async-pipe.cc index d47930a1b..da47c37a8 100644 --- a/src/libutil/windows/windows-async-pipe.cc +++ b/src/libutil/windows/windows-async-pipe.cc @@ -1,7 +1,8 @@ -#include "nix/util/windows-async-pipe.hh" -#include "nix/util/windows-error.hh" + #ifdef _WIN32 +# include "nix/util/windows-async-pipe.hh" +# include "nix/util/windows-error.hh" namespace nix::windows { diff --git a/src/libutil/windows/windows-error.cc b/src/libutil/windows/windows-error.cc index 1e7aff830..0761bdfd5 100644 --- a/src/libutil/windows/windows-error.cc +++ b/src/libutil/windows/windows-error.cc @@ -1,6 +1,5 @@ -#include "nix/util/windows-error.hh" - #ifdef _WIN32 +#include "nix/util/windows-error.hh" #include #define WIN32_LEAN_AND_MEAN #include diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index cde9d6742..3977aaffe 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -420,15 +420,8 @@ static void main_nix_build(int argc, char * * argv) state->maybePrintStats(); auto buildPaths = [&](const std::vector & paths) { - /* Note: we do this even when !printMissing to efficiently - fetch binary cache data. */ - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing(paths, - willBuild, willSubstitute, unknown, downloadSize, narSize); - if (settings.printMissing) - printMissing(ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize); + printMissing(ref(store), paths); if (!dryRun) store->buildPaths(paths, buildMode, evalStore); @@ -542,7 +535,7 @@ static void main_nix_build(int argc, char * * argv) env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmpDir.path().string(); env["NIX_STORE"] = store->storeDir; - env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); auto parsedDrv = StructuredAttrs::tryParse(drv.env); DerivationOptions drvOptions; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 0d758f44b..3da7a8ac1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -146,23 +146,19 @@ static void opRealise(Strings opFlags, Strings opArgs) for (auto & i : opArgs) paths.push_back(followLinksToStorePathWithOutputs(*store, i)); - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing( - toDerivedPaths(paths), - willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = store->queryMissing(toDerivedPaths(paths)); /* Filter out unknown paths from `paths`. */ if (ignoreUnknown) { std::vector paths2; for (auto & i : paths) - if (!unknown.count(i.path)) paths2.push_back(i); + if (!missing.unknown.count(i.path)) paths2.push_back(i); paths = std::move(paths2); - unknown = StorePathSet(); + missing.unknown = StorePathSet(); } if (settings.printMissing) - printMissing(ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize); + printMissing(ref(store), missing); if (dryRun) return; diff --git a/src/nix/main.cc b/src/nix/main.cc index 6144f746f..502e04e60 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -212,6 +212,14 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs, virtual RootArgs lowdown. */ static void showHelp(std::vector subcommand, NixArgs & toplevel) { + // Check for aliases if subcommand has exactly one element + if (subcommand.size() == 1) { + auto alias = toplevel.aliases.find(subcommand[0]); + if (alias != toplevel.aliases.end()) { + subcommand = alias->second.replacement; + } + } + auto mdName = subcommand.empty() ? "nix" : fmt("nix3-%s", concatStringsSep("-", subcommand)); evalSettings.restrictEval = false; diff --git a/tests/functional/build-cores.nix b/tests/functional/build-cores.nix new file mode 100644 index 000000000..9b763f7d8 --- /dev/null +++ b/tests/functional/build-cores.nix @@ -0,0 +1,11 @@ +with import ./config.nix; + +{ + # Test derivation that checks the NIX_BUILD_CORES environment variable + testCores = mkDerivation { + name = "test-build-cores"; + buildCommand = '' + echo "$NIX_BUILD_CORES" > $out + ''; + }; +} diff --git a/tests/functional/build-cores.sh b/tests/functional/build-cores.sh new file mode 100755 index 000000000..a226774c6 --- /dev/null +++ b/tests/functional/build-cores.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash + +source common.sh + +clearStoreIfPossible + +echo "Testing build-cores configuration behavior..." + +# Test 1: When build-cores is set to a non-zero value, NIX_BUILD_CORES should have that value +echo "Testing build-cores=4..." +rm -f "$TEST_ROOT"/build-cores-output +nix-build --cores 4 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output +result=$(cat "$(readlink "$TEST_ROOT"/build-cores-output)") +if [[ "$result" != "4" ]]; then + echo "FAIL: Expected NIX_BUILD_CORES=4, got $result" + exit 1 +fi +echo "PASS: build-cores=4 correctly sets NIX_BUILD_CORES=4" +rm -f "$TEST_ROOT"/build-cores-output + +# Test 2: When build-cores is set to 0, NIX_BUILD_CORES should be resolved to getDefaultCores() +echo "Testing build-cores=0..." +nix-build --cores 0 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output +result=$(cat "$(readlink "$TEST_ROOT"/build-cores-output)") +if [[ "$result" == "0" ]]; then + echo "FAIL: NIX_BUILD_CORES should not be 0 when build-cores=0" + exit 1 +fi +echo "PASS: build-cores=0 resolves to NIX_BUILD_CORES=$result (should be > 0)" +rm -f "$TEST_ROOT"/build-cores-output + +echo "All build-cores tests passed!" diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index a41aa35c0..dc5d8f818 100755 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -53,6 +53,27 @@ rm -rf $TEST_HOME/.cache/nix path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") [[ $(cat $path/hello) = world ]] +# Fetch again. This should be cached. +# NOTE: This has to be done before the test case below which tries to pack-refs +# the reason being that the lookup on the cache uses the ref-file `/refs/heads/master` +# which does not exist after packing. +mv $repo ${repo}-tmp +path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") +[[ $path = $path2 ]] + +[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]] + +# Fetching with a explicit hash should succeed. +path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") +[[ $path = $path2 ]] + +path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath") +[[ $(cat $path2/hello) = utrecht ]] + +mv ${repo}-tmp $repo + # Fetch when the cache has packed-refs # Regression test of #8822 git -C $TEST_HOME/.cache/nix/gitv3/*/ pack-refs --all @@ -83,24 +104,6 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \" # But without a hash, it fails. expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' doesn't fetch unlocked input" -# Fetch again. This should be cached. -mv $repo ${repo}-tmp -path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") -[[ $path = $path2 ]] - -[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]] - -# Fetching with a explicit hash should succeed. -path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] - -path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath") -[[ $(cat $path2/hello) = utrecht ]] - -mv ${repo}-tmp $repo - # Using a clean working tree should produce the same result. path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [[ $path = $path2 ]] diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index ce695a6cb..7fd9dc9b5 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -432,3 +432,41 @@ nix flake metadata "$flake2Dir" --reference-lock-file $TEST_ROOT/flake2-overridd # reference-lock-file can only be used if allow-dirty is set. expectStderr 1 nix flake metadata "$flake2Dir" --no-allow-dirty --reference-lock-file $TEST_ROOT/flake2-overridden.lock + +# After changing an input (flake2 from newFlake2Rev to prevFlake2Rev), we should have the transitive inputs locked by revision $prevFlake2Rev of flake2. +prevFlake1Rev=$(nix flake metadata --json "$flake1Dir" | jq -r .revision) +prevFlake2Rev=$(nix flake metadata --json "$flake2Dir" | jq -r .revision) + +echo "# bla" >> "$flake1Dir/flake.nix" +git -C "$flake1Dir" commit flake.nix -m 'bla' + +nix flake update --flake "$flake2Dir" +git -C "$flake2Dir" commit flake.lock -m 'bla' + +newFlake1Rev=$(nix flake metadata --json "$flake1Dir" | jq -r .revision) +newFlake2Rev=$(nix flake metadata --json "$flake2Dir" | jq -r .revision) + +cat > "$flake3Dir/flake.nix" < "$flake3Dir/flake.nix" < /tmp/nix-build-open-build-dir.drv-0/build/syncPoint") + machine.succeed(f"echo foo > {dir}/build/syncPoint") with subtest("Being able to execute stuff as the build user doesn't give access to the build dir"): machine.succeed(r""" @@ -124,16 +125,17 @@ in args = [ (builtins.storePath "${create-hello-world}") ]; }' >&2 & """.strip()) - machine.wait_for_file("/tmp/nix-build-innocent.drv-0/build/syncPoint") + machine.wait_until_succeeds("stat /nix/var/nix/builds/nix-build-innocent.drv-*/build/syncPoint") + dir = machine.succeed("ls -d /nix/var/nix/builds/nix-build-innocent.drv-*").strip() # The build ran as `nixbld1` (which is the only build user on the # machine), but a process running as `nixbld1` outside the sandbox # shouldn't be able to touch the build directory regardless - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls /tmp/nix-build-innocent.drv-0/build'") - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > /tmp/nix-build-innocent.drv-0/build/result'") + machine.fail(f"su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls {dir}/build'") + machine.fail(f"su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > {dir}/build/result'") # Finish the build - machine.succeed("echo foo > /tmp/nix-build-innocent.drv-0/build/syncPoint") + machine.succeed(f"echo foo > {dir}/build/syncPoint") # Check that the build was not affected machine.succeed(r"""