Skip to content

nixos/systemd-boot: refactor json.load() logic for better error message#421916

Merged
ElvishJerricco merged 1 commit intoNixOS:masterfrom
jackrosenberg:fix/boot
Jul 28, 2025
Merged

nixos/systemd-boot: refactor json.load() logic for better error message#421916
ElvishJerricco merged 1 commit intoNixOS:masterfrom
jackrosenberg:fix/boot

Conversation

@jackrosenberg
Copy link
Member

@jackrosenberg jackrosenberg commented Jul 2, 2025

Last weekend I ran into a few bugs while trying to rebuild in emergency mode after managing to corrupt my filesystem. The problem was a corrupted boot.json in one of the system profiles stored in /nix/var/nix/profiles/system-x-link/. This caused nixos-rebuild bootto fail.
image

The solution was to remove this corrupt boot.json and rebuild boot from there. In a system with many generations it can be challenging to find the corrupted file and remove it. To fix this I've added a try except to throw an error if parsing the json fails. This allows the user to find the guilty file easily. Below is the new error that is thrown. (outdated)

20250702_23h30m24s_grim

@ElvishJerricco @emilazy @JulienMalka

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@jackrosenberg jackrosenberg requested a review from JulienMalka July 2, 2025 21:33
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Jul 2, 2025
Copy link
Contributor

@ElvishJerricco ElvishJerricco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably still need to throw the exception, not just print it. I don't know python well enough; what's the appropriate way to add context to an exception but still throw it? @JulienMalka?

@emilazy
Copy link
Member

emilazy commented Jul 2, 2025

I think for a user‐facing error a print is more appropriate. Tools shouldn’t throw exceptions to the user unless something went wrong internally. But it should sys.exit(1) after printing the error (to sys.stderr).

@JulienMalka
Copy link
Member

I agree with @emilazy

@jackrosenberg
Copy link
Member Author

jackrosenberg commented Jul 3, 2025

20250703_09h34m27s_grim
The error message now looks like this. Since the remaining logic is never reached, the stack trace is much shorter and clearer.

@jackrosenberg jackrosenberg force-pushed the fix/boot branch 3 times, most recently from c852bd4 to 75f09d7 Compare July 3, 2025 08:18
@jackrosenberg
Copy link
Member Author

@JulienMalka @emilazy @ElvishJerricco , anything more that needs to happen?

@ElvishJerricco ElvishJerricco merged commit 4b3b18a into NixOS:master Jul 28, 2025
25 of 27 checks passed
@jackrosenberg jackrosenberg deleted the fix/boot branch July 28, 2025 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants