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: Add support for an XBOOTLDR partition #285401

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

sdht0
Copy link
Contributor

@sdht0 sdht0 commented Feb 1, 2024

Description of changes

Continuation of PR #260241 which I closed inadvertently.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: documentation This PR adds or changes documentation 8.has: changelog 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 1, 2024
@sdht0 sdht0 force-pushed the systemd-boot-xbootldr branch from 7afa4bc to a04c9ba Compare February 1, 2024 03:15
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Feb 1, 2024
@sdht0
Copy link
Contributor Author

sdht0 commented Feb 1, 2024

@nikstur @RaitoBezarius ideally qemu-vm should provide an option to override the systemImage. However, I avoided making any changes and indirectly used useBootLoader=false option as a workaround.

@ElvishJerricco ElvishJerricco requested a review from a team February 2, 2024 01:23
@sdht0 sdht0 force-pushed the systemd-boot-xbootldr branch from a04c9ba to 1bcaddb Compare February 4, 2024 22:40
@github-actions github-actions bot removed 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Feb 4, 2024
@sdht0 sdht0 force-pushed the systemd-boot-xbootldr branch from 1bcaddb to db53fad Compare February 4, 2024 22:43
@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 8.has: changelog labels Feb 4, 2024
@sdht0
Copy link
Contributor Author

sdht0 commented Feb 7, 2024

@nikstur @RaitoBezarius gentle ping

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-to-put-my-kernels-initrd-and-entries-in-the-boot-xbootldr-partition/19162/4

@sdht0
Copy link
Contributor Author

sdht0 commented Feb 23, 2024

@nikstur @RaitoBezarius can you please take a look at the last blocker for this PR? Specifically these lines.

@nikstur
Copy link
Contributor

nikstur commented Feb 24, 2024

Looks good from my side.

@sdht0
Copy link
Contributor Author

sdht0 commented Feb 24, 2024

Thanks.

@ElvishJerricco @JulienMalka can we merge this then?

cc @RaitoBezarius

@JulienMalka
Copy link
Member

@ofborg test systemd-boot

@sdht0 sdht0 force-pushed the systemd-boot-xbootldr branch from db53fad to b470b44 Compare February 27, 2024 03:00
@sdht0
Copy link
Contributor Author

sdht0 commented Feb 27, 2024

Fixed a merge conflict.

@sdht0
Copy link
Contributor Author

sdht0 commented Feb 27, 2024

@ofborg test systemd-boot

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.

LGTM. @RaitoBezarius shall we merge?

@RaitoBezarius
Copy link
Member

Let's go, I want to say that @sdht0 you have been incredibly patient with us, I would like to point out this is something I rarely see in nixpkgs contributions and a big big big thank you, this has been a very pleasant experience for me to review, even beyond the disagreements.

@ElvishJerricco ElvishJerricco merged commit a587a6a into NixOS:master Feb 28, 2024
25 checks passed
@sdht0 sdht0 deleted the systemd-boot-xbootldr branch February 28, 2024 02:32
@sdht0
Copy link
Contributor Author

sdht0 commented Feb 28, 2024

Thanks for the kind words @RaitoBezarius!

Nice to get this finally in. Thanks @JulienMalka @ElvishJerricco @nikstur for the reviews.

And thanks again @colemickens for handing over your PR. I got to have quite a bit of fun.

@colemickens
Copy link
Member

Very cool that (the original accidentally closed) PR was your first PR in nixpkgs, @sdht0! Quite a cool first contribution, and more already since then. 😎 🚀

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/add-windows-to-boot-menu/40675/5

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: changelog 8.has: documentation This PR adds or changes documentation 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants