Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/systemd-boot: update bootloader if needed #90386

Merged
merged 3 commits into from Jun 16, 2020

Conversation

danielfullmer
Copy link
Contributor

Motivation for this change

Fixes #86422

Messing with people's existing bootloaders is always a little scary, so I'd appreciate a careful review.

Tested with

$ nix-build ./nixos/tests/systemd-boot.nix -A update
$ nix-build ./nixos/tests/installer.nix -A simpleUefiSystemdBoot

Also tested on my own physical machine.

CC @flokli @Mic92

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@danielfullmer
Copy link
Contributor Author

Rebased and pushed. I initially forgot to actually include the default boot entry selection fix from @flokli in #86422

Copy link
Contributor

@flokli flokli left a comment

Choose a reason for hiding this comment

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

Some small nits, but apart from that, tested and LGTM.

danielfullmer and others added 3 commits June 15, 2020 20:22
systemd/systemd@6cd12eb
changed behaviour - now the "default" entry needs to identity an entry
with its full name, including the ".conf".

Reported-In: NixOS#86422
@flokli
Copy link
Contributor

flokli commented Jun 16, 2020

Thanks!

else:
# Update bootloader to latest if needed
systemd_version = subprocess.check_output(["@systemd@/bin/bootctl", "--version"], universal_newlines=True).split()[1]
sdboot_status = subprocess.check_output(["@systemd@/bin/bootctl", "--path=@efiSysMountPoint@", "status"], universal_newlines=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This line fails on my system, because of Boot into FW: not supported, which causes the entire command to fail, when then causes check_output to throw an exception. Is that the expected behavior in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ignore this comment, I submitted an issue about this instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

systemd-boot: default boot entry ignored with systemd v245
3 participants