Skip to content

make-initrd-ng: support wrapped executables#215381

Merged
ElvishJerricco merged 4 commits intoNixOS:masterfrom
lilyinstarlight:fix/make-initrd-ng-wrapped-executables
Feb 20, 2023
Merged

make-initrd-ng: support wrapped executables#215381
ElvishJerricco merged 4 commits intoNixOS:masterfrom
lilyinstarlight:fix/make-initrd-ng-wrapped-executables

Conversation

@lilyinstarlight
Copy link
Member

@lilyinstarlight lilyinstarlight commented Feb 8, 2023

Description of changes

This makes it so that make-initrd-ng automatically copies the unwrapped files when it encounters wrapped files, preventing the need to predict and add each wrapped file to the systemd-initrd store paths manually

The issue was noticed when gzip became wrapped and vconsole setup in systemd-initrd stopped working because only the wrapped was copied and not the unwrapped executable, so I added a test that vconsole setup works correctly in systemd-initrd (though technically gzip won't necessarily always be wrapped so this test only indirectly tests the change right now)

cc: @ElvishJerricco (since we talked about this in #stage1-systemd:nixos.org)

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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@lilyinstarlight lilyinstarlight requested review from a team and dasJ as code owners February 8, 2023 21:20
@github-actions github-actions bot added 6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 8.has: module (update) This PR changes an existing module in `nixos/` labels Feb 8, 2023
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Feb 8, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

It's definitely weird that we had the unwrapped one here and the wrapped one in luksroot.nix. But maybe we should leave the wrapped one in luksroot.nix and simply remove the line from this file? I'm pretty sure we don't want this in initrd unless LUKS is actually being used.

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like there is a reference to the exe that could get dynamically added from systemd-gpt-auto-generator, but that's because the discoverable partitions spec does allow discovery of LUKS partitions and it's probably not important to have work if LUKS devices aren't defined in the config (someone can just boot.initrd.luks.forceLuksSupportInInitrd anyway to get it even if the device itself isn't in the config)

I'll push an update in a second

Copy link
Member Author

Choose a reason for hiding this comment

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

I've no clue why it was added to begin with though so 🤷🏻‍♀️

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like it came from #189676. I assume because they ran into the wrapped issue but they added it to the systemd-initrd module instead of the LUKS one for some reason

@zhaofengli Was there a reason for adding the wrapped executable here or can it be removed now that make-initrd-ng supports wrapped files with this PR?

@lilyinstarlight lilyinstarlight force-pushed the fix/make-initrd-ng-wrapped-executables branch from dd6a1da to d6d9f43 Compare February 8, 2023 23:18
@lilyinstarlight
Copy link
Member Author

@ofborg eval
@ofborg build nixosTests.systemd-initrd-vconsole

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.

I think the last thing this PR needs is a change to the README in pkgs/build-support/kernel/make-initrd-ng/README.md. That file outlines what the program does in detail, so it needs to be updated with information about this new behavior.

@lilyinstarlight lilyinstarlight force-pushed the fix/make-initrd-ng-wrapped-executables branch from d6d9f43 to 1fa1b58 Compare February 20, 2023 12:03
@lilyinstarlight
Copy link
Member Author

@ofborg build nixosTests.systemd-initrd-vconsole

I added one line to the file to document this new behavior. It's pretty short but so is the change itself, so let me know if this is still insufficient

@ElvishJerricco ElvishJerricco merged commit bb7cd63 into NixOS:master Feb 20, 2023
@lilyinstarlight lilyinstarlight deleted the fix/make-initrd-ng-wrapped-executables branch February 20, 2023 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: kernel The Linux kernel 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

Development

Successfully merging this pull request may close these issues.

2 participants