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/unl0kr: fix tests, change default package to buffybox (bumps unl0kr to 3.2.0-unstable-2024-11-10) #362825

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

hustlerone
Copy link
Contributor

@hustlerone hustlerone commented Dec 7, 2024

Things done

  • Fixed the test so it correctly assesses unl0kr on boot
  • The NixOS module for unl0kr now uses buffybox as the package that provides unl0kr. This is essentially a version bump.
  • Loosened up the restriction on early video loading (amdgpu on initrd). I haven't tested as to whether or not it no longer has issues with early video loading.
  • TODO: deprecated the unl0kr package
  • 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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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: module (update) This PR changes an existing module in `nixos/` labels Dec 7, 2024
@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 7, 2024

It seems that changing from the unl0kr to package to buffybox is trivial. I'll test to see if it works on a real installation, then test how nicely it plays with early video loading then I'll mark the unl0kr package as deprecated

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 7, 2024
@Aleksanaa Aleksanaa marked this pull request as draft December 7, 2024 15:52
@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 7, 2024

I'm about to open an issue regarding the module needed in order to create tests. It's unacceptable that it takes an N amount of seconds to take two screenshots and type a string of characters

@hustlerone hustlerone marked this pull request as ready for review December 7, 2024 17:51
@hustlerone hustlerone changed the title [DRAFT] Replace standalone unl0kr package to buffybox Replace standalone unl0kr package to buffybox Dec 7, 2024
@hustlerone hustlerone force-pushed the master branch 2 times, most recently from e3979d5 to 5b02606 Compare December 7, 2024 17:57
@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 7, 2024

How do I set a package as deprecated? (not to prevent anyone from using it just to warn that it's going to be removed soon)

@hustlerone hustlerone force-pushed the master branch 2 times, most recently from d0725e4 to 085dfe9 Compare December 7, 2024 18:19
@hustlerone hustlerone changed the title Replace standalone unl0kr package to buffybox nixos/unl0kr: fix tests, change default package to buffybox Dec 7, 2024
@hustlerone
Copy link
Contributor Author

let me finish the docs on this rq

@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 7, 2024

The long awaited unl0kr update is real now

@hustlerone hustlerone changed the title nixos/unl0kr: fix tests, change default package to buffybox nixos/unl0kr: fix tests, change default package to buffybox (bumps unl0kr to 3.2.0-unstable-2024-11-10) Dec 7, 2024
@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@hustlerone
Copy link
Contributor Author

oh come on how could this happen

@hustlerone hustlerone marked this pull request as draft December 7, 2024 20:35
@hustlerone hustlerone marked this pull request as ready for review December 7, 2024 20:57
@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/4932

@hustlerone
Copy link
Contributor Author

STILL TESTING ON AARCH64???

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.

Can we please just remove the old package? Why are we bothering to deprecate it? We literally have a current version of the same package, it's just named differently.

@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 8, 2024

Can we please just remove the old package? Why are we bothering to deprecate it? We literally have a current version of the same package, it's just named differently.

I like your mindset

@hustlerone hustlerone force-pushed the master branch 2 times, most recently from ebc17c4 to 5885b26 Compare December 8, 2024 07:27
@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 8, 2024

@ElvishJerricco looks like unl0kr's password agent has been re-implemented in C. Should I look into it after this PR or within this PR? Both (what we have in the module and what they have on their repo) should effectively do the same thing but I'm not sure how much better is the one from upstream.

However, it's not present on any stable release but it is present on the ref specified by the buffybox package.

@hustlerone hustlerone marked this pull request as draft December 11, 2024 11:25
@hustlerone
Copy link
Contributor Author

hustlerone commented Dec 11, 2024

Converted to draft again, matching the systemd service with upstream It's more interesting than what it looks like. It might even fix compatibility issues with plymouth

@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

@hustlerone
Copy link
Contributor Author

@ofborg test systemd-initrd-luks-unl0kr

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.

Sorry it took me so long to get around to reviewing this. Looks like CI isn't happy with the formatting somewhere.

unl0kr-ask-password = {
description = "Forward Password Requests to unl0kr";
unl0kr-agent = {
description = "Dispatch Password Requests to unl0kr";
Copy link
Contributor

@ElvishJerricco ElvishJerricco Dec 18, 2024

Choose a reason for hiding this comment

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

Did you try just adding buffybox to boot.initrd.systemd.packages rather than duplicating the upstream systemd units? Would be a lot cleaner. Should just need to manually add the necessary wantedBy to the path unit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm attempting to use the password agent from upstream, rewritten in C.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I currently don't know how to debug the systemd service, is there a manual entry for debugging stage 1 services?

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be a way to get a shell in stage 1, then you can manually run commands to check whatever you want to do. Would that be helpful?

Copy link
Contributor

@ElvishJerricco ElvishJerricco Dec 19, 2024

Choose a reason for hiding this comment

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

I'm attempting to use the password agent from upstream, rewritten in C.

Yes. The buffybox package has upstream unit files that can be included in initrd automatically by setting boot.initrd.systemd.packages = [ buffybox ]; so you don't have to reimplement the units yourself here.

I currently don't know how to debug the systemd service, is there a manual entry for debugging stage 1 services?

You can get a debug shell in systemd initrd by adding the rd.systemd.debug_shell kernel param and switching to VT9

@hustlerone hustlerone force-pushed the master branch 2 times, most recently from 075a074 to 5ca3e76 Compare December 18, 2024 12:25
@@ -1325,6 +1325,7 @@ mapAliases {
unifi8 = unifi; # Added 2024-11-15
unifiLTS = throw "'unifiLTS' has been removed since UniFi no longer has LTS and stable releases. Use `pkgs.unifi` instead."; # Added 2024-04-11
unifiStable = throw "'unifiStable' has been removed since UniFi no longer has LTS and stable releases. Use `pkgs.unifi` instead."; # Converted to throw 2024-04-11
unl0kr = throw "'unl0kr' is now included with buffybox. Use `pkgs.buffybox` instead.";
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we add a comment with the date at the end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgout about it

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: clean-up 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.

None yet

5 participants