Samuel Dionne-Riel
40fb9e7c5a
The problem ----------- When rebuilding a system, if `switch-to-configuration-ng` fails to install bootloader files, it will (most likely) `exit(0)`. ``` /etc/nixos $ sudo nixos-rebuild --fast boot && reboot [sudo] password for samuel: building the system configuration... updating GRUB 2 menu... cannot copy /nix/store/.../initrd to /boot/kernels/...-initrd.tmp: No space left on device Failed to install bootloader Broadcast message from samuel@... on pts/1 (Tue 2024-12-31 16:48:26 EST): The system will reboot now! ``` This is a quite awkward breaking change with the expected behaviour. * * * The investigation ----------------- Compare: - https://github.com/NixOS/nixpkgs/blob/85b5f3e959327a3fa46f843848ebb8799069bb95/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs#L171-L179 - https://github.com/NixOS/nixpkgs/blob/85b5f3e959327a3fa46f843848ebb8799069bb95/nixos/modules/system/activation/switch-to-configuration.pl#L115-L117 Let's see what `die()` is all about: - https://github.com/NixOS/nixpkgs/blob/85b5f3e959327a3fa46f843848ebb8799069bb95/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs#L121-L125 ***sus.*** There are multiple issues converging here. **Incorrect port** The original implementation did not use `die`, but `exit 1`. So porting from `perl` following the script's idiosyncrasies was not done appropriately. **Incorrect `die` fac-simile** The `die` method is incomplete with regard to the semantics of perl. - https://perldoc.perl.org/5.40.0/functions/die Of importance to us: > If [die is called], the exit code is determined from the values of > `$!` and `$?` with this pseudocode: > > ``` > exit $! if $!; # errno > exit $? >> 8 if $? >> 8; # child exit status > exit 255; # last resort > ``` The `die()` method in `switch-to-configuration-ng` *only* checks `errno`, using its value directly to `exit()`. It does not handle some form of implicit child process exit status. And, due to incorrect assumptions, it will not fall back to anything. **Incorrect implementation** (Note that from this point on, I'm not a Rust expert, so bear with me if some nuances are lost or incorrectly represented.) The `die()` function implementation, as a port, might not even work correctly. Already, the `spawn` method does not mention it would be setting `errno`, so any `die()` following a `status.success()` is *sus* and should be investigated. Since it's not attempting to do anything "smart" with child processes. - https://doc.rust-lang.org/1.83.0/std/process/struct.Command.html#method.spawn And I'd argue that using *errno* in this manner in Rust is probably a mistake, and should not be done. > This should be called immediately after a call to a platform function, > otherwise the state of the error value is indeterminate. - https://doc.rust-lang.org/1.83.0/std/io/struct.Error.html#method.last_os_error Considering *platform function* is largely left undefined, I would (probably wrongly) intuit that it should be considered undefined behaviour to rely on it. Note that `raw_os_error` might have a surprising interface. > If this `Error` was constructed via `last_os_error` [...], > then this function will return `Some`, otherwise it will return `None`. - https://doc.rust-lang.org/1.83.0/std/io/struct.Error.html#method.raw_os_error Since it's used as `std::io::Error::last_os_error().raw_os_error()`, AFAIUI it will always return `Some`. Since this is exposing `errno`, the libc concept, it will behave the same, and may be set to `0` by default, just like here: ``` $ printf '#include <stdlib.h>\n#include <errno.h>\nint main() { exit(errno); }' \ | cc -x c - && ./a.out; echo $? 0 ``` Which means that, since no *platform function*[sic] changed its value, it will be zero, the `die()` function will be equivalent to `exit(errno)`, and the program will have failed “successfully” wrongly. * * * The fix ------- I've fixed the `do_pre_switch_check` and `do_install_bootloader` methods, both of which share the same defects (the original script uses `exit 1` for both). They were the only `status.success()` checks using `die()`. * * * Reproducing the issue --------------------- Remember how I said: > Considering *platform function* is largely left undefined, I would > (probably wrongly) intuit that it should be considered undefined > behaviour to rely on it. Here's why it's not some vague FUD. First, make sure a `nixos-rebuild boot` would need to write new files to the boot partition. Removing an older (but still alive) generation's initrd can do that. Fill the `/boot` partition to force an error. ``` $ sudo dd if=/dev/zero of=/boot/BOGUS.FILLINGS ``` Then, and here's the fun part, observe: ``` ~ $ sudo rm -r /run/nixos ~ $ sudo nixos-rebuild --fast boot ; echo $? building the system configuration... updating GRUB 2 menu... cannot copy /nix/store/x91w4p91l7iclkdp38chvdxcw6nr5113-mobile-nixos-initrd-generic/initrd to /boot/kernels/x91w4p91l7iclkdp38chvdxcw6nr5113-mobile-nixos-initrd-generic-initrd.tmp: No space left on device Failed to install bootloader 0 ~ $ sudo nixos-rebuild --fast boot ; echo $? building the system configuration... updating GRUB 2 menu... cannot copy /nix/store/x91w4p91l7iclkdp38chvdxcw6nr5113-mobile-nixos-initrd-generic/initrd to /boot/kernels/x91w4p91l7iclkdp38chvdxcw6nr5113-mobile-nixos-initrd-generic-initrd.tmp: No space left on device Failed to install bootloader warning: error(s) occurred while switching to the new configuration 1 ``` So... What's the deal with /run/nixos? It's where the lock file will reside. (And other transient files.) - https://github.com/NixOS/nixpkgs/blob/85b5f3e959327a3fa46f843848ebb8799069bb95/pkgs/by-name/sw/switch-to-configuration-ng/src/src/main.rs#L1018-L1027 But why does that matter here? ``` ~ $ errno 17 EEXIST 17 File exists ``` This error is produced by some *platform functions*[sic] that create either the directory, or the lockfile. The file already exists. So the script would end-up failing this way *only for the first invocation*. Which is why it's possible any of you all reviewing this ~~novel~~ PR haven't faced that issue. * * * Future work ----------- I believe `die()` *probably* should be switched to check the value, and `exit 255` if it's 0. Though I also believe `die()` shouldn't try to port perl semantics into Rust. I don't think it's working out. Additionally, a NixOS test should be authored to ensure that errors in these phases actually are handled appropriately. Signed-off-by: Samuel Dionne-Riel <samuel@dionne-riel.com>
Nixpkgs is a collection of over 100,000 software packages that can be installed with the Nix package manager. It also implements NixOS, a purely-functional Linux distribution.
Manuals
- NixOS Manual - how to install, configure, and maintain a purely-functional Linux distribution
- Nixpkgs Manual - contributing to Nixpkgs and using programming-language-specific Nix expressions
- Nix Package Manager Manual - how to write Nix expressions (programs), and how to use Nix command line tools
Community
- Discourse Forum
- Matrix Chat
- NixOS Weekly
- Official wiki
- Community-maintained list of ways to get in touch (Discord, Telegram, IRC, etc.)
Other Project Repositories
The sources of all official Nix-related projects are in the NixOS organization on GitHub. Here are some of the main ones:
- Nix - the purely functional package manager
- NixOps - the tool to remotely deploy NixOS machines
- nixos-hardware - NixOS profiles to optimize settings for different hardware
- Nix RFCs - the formal process for making substantial changes to the community
- NixOS homepage - the NixOS.org website
- hydra - our continuous integration system
- NixOS Artwork - NixOS artwork
Continuous Integration and Distribution
Nixpkgs and NixOS are built and tested by our continuous integration system, Hydra.
- Continuous package builds for unstable/master
- Continuous package builds for the NixOS 24.11 release
- Tests for unstable/master
- Tests for the NixOS 24.11 release
Artifacts successfully built with Hydra are published to cache at https://cache.nixos.org/. When successful build and test criteria are met, the Nixpkgs expressions are distributed via Nix channels.
Contributing
Nixpkgs is among the most active projects on GitHub. While thousands of open issues and pull requests might seem a lot at first, it helps consider it in the context of the scope of the project. Nixpkgs describes how to build tens of thousands of pieces of software and implements a Linux distribution. The GitHub Insights page gives a sense of the project activity.
Community contributions are always welcome through GitHub Issues and Pull Requests.
For more information about contributing to the project, please visit the contributing page.
Donations
The infrastructure for NixOS and related projects is maintained by a nonprofit organization, the NixOS Foundation. To ensure the continuity and expansion of the NixOS infrastructure, we are looking for donations to our organization.
You can donate to the NixOS foundation through SEPA bank transfers or by using Open Collective:
License
Nixpkgs is licensed under the MIT License.
Note: MIT license does not apply to the packages built by Nixpkgs, merely to the files in this repository (the Nix expressions, build scripts, NixOS modules, etc.). It also might not apply to patches included in Nixpkgs, which may be derivative works of the packages to which they apply. The aforementioned artifacts are all covered by the licenses of the respective packages.