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.

Sign up for GitHub

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

Jump to bottom

add boot.initrd.unl0kr #218441

Merged
merged 3 commits into from Nov 28, 2023
Merged

add boot.initrd.unl0kr #218441

merged 3 commits into from Nov 28, 2023

Conversation

tomfitzhenry
Copy link
Contributor

@tomfitzhenry tomfitzhenry commented Feb 26, 2023 •
edited

Description of changes

unl0kr is a "Framebuffer-based disk unlocker for the initramfs based on LVGL".

Test via nix-build -A nixosTests.systemd-initrd-luks-unl0kr.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@tomfitzhenry tomfitzhenry mentioned this pull request Feb 26, 2023
Closed
13 tasks
@tomfitzhenry tomfitzhenry marked this pull request as draft February 26, 2023 13:39
@tomfitzhenry tomfitzhenry removed the request for review from dasJ February 26, 2023 13:55
@ElvishJerricco ElvishJerricco added this to To Do in systemd in Stage 1 via automation Feb 27, 2023
@tomfitzhenry
Copy link
Contributor Author

The test currently non-deterministically fails, I think due to https://gitlab.com/cherrypicker/unl0kr/-/issues/36 .

https://gitlab.com/cherrypicker/unl0kr/-/issues/36 is now fixed, and after updating this test to use unl0kr 2.0.0, this test now passes. I'll tidy up this PR.

@tomfitzhenry tomfitzhenry force-pushed the unl0kr branch 2 times, most recently from 3fcb448 to a9bc245 Compare July 30, 2023 06:05
@tomfitzhenry tomfitzhenry changed the title WIP: add boot.initrd.unl0kr add boot.initrd.unl0kr Jul 30, 2023
@tomfitzhenry tomfitzhenry marked this pull request as ready for review July 30, 2023 06:08
@tomfitzhenry
Copy link
Contributor Author

@ElvishJerricco @samueldr, thoughts?

ElvishJerricco
ElvishJerricco reviewed Sep 19, 2023
View reviewed changes
nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
nixos/modules/system/boot/unl0kr.nix Outdated
storePaths = with pkgs; [
gnugrep
libinput
xkeyboard_config
systemd
unl0kr
];
Copy link
Contributor

Choose a reason for hiding this comment

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

These should probably be minimized, if possible. Like ideally we would be overriding unl0kr or whatever needs systemd to use config.boot.initrd.systemd.package instead. And only the files specifically needed from these other packages would be listed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SGTM. I've reduced systemd to just systemd's systemd-reply-password binary, and pulled that from config.boot.initrd.systemd.package.

I will leave this comment thread open for me to look into reducing the size of other dependencies if possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

@tomfitzhenry I still think these need to be reduced further. As is, you're including things like manpages. Like maybe just the /bin subdirectories or something like that.

nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
AndersonTorres
AndersonTorres previously requested changes Sep 22, 2023
View reviewed changes
pkgs/tools/misc/unl0kr/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/unl0kr/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/unl0kr/default.nix Outdated Show resolved Hide resolved
pkgs/tools/misc/unl0kr/default.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
systemd in Stage 1 automation moved this from To Do to Ready for review Sep 22, 2023
@tomfitzhenry tomfitzhenry mentioned this pull request Sep 29, 2023
Open
samueldr
samueldr reviewed Oct 1, 2023
View reviewed changes
Copy link
Member

@samueldr samueldr left a comment

Choose a reason for hiding this comment

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

Two nits, mainly, though I haven't tested, only read the changes to see if it looked fine.

nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
nixos/modules/system/boot/unl0kr.nix Outdated Show resolved Hide resolved
@tomfitzhenry @AndersonTorres
unl0kr: init
46b1742 
Co-authored-by: Anderson Torres <torres.anderson.85@protonmail.com>
@ElvishJerricco
Copy link
Contributor

@ofborg test systemd-initrd-luks-unl0kr

ElvishJerricco
ElvishJerricco approved these changes Nov 19, 2023
View reviewed changes
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.

Assuming ofborg passes the test, LGTM

@ElvishJerricco ElvishJerricco merged commit 7f21311 into NixOS:master Nov 28, 2023
29 checks passed
systemd in Stage 1 automation moved this from Ready for review to Done Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ElvishJerricco ElvishJerricco ElvishJerricco approved these changes

@AndersonTorres AndersonTorres AndersonTorres left review comments

@999eagle 999eagle Awaiting requested review from 999eagle

@samueldr samueldr Awaiting requested review from samueldr

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (new) 8.has: module (update) 8.has: package (new) 8.has: tests 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 11.by: package-maintainer
Projects
systemd in Stage 1
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@tomfitzhenry @ElvishJerricco @samueldr @AndersonTorres @h7x4