Skip to content

Comments

nixos/systemd: conditionally leave out some upstream units#374214

Merged
ElvishJerricco merged 2 commits intoNixOS:masterfrom
jmbaur:systemd-upstream-units-fix
Mar 19, 2025
Merged

nixos/systemd: conditionally leave out some upstream units#374214
ElvishJerricco merged 2 commits intoNixOS:masterfrom
jmbaur:systemd-upstream-units-fix

Conversation

@jmbaur
Copy link
Contributor

@jmbaur jmbaur commented Jan 16, 2025

Some upstream systemd units are conditionally installed into the systemd output, so we must make sure the feature that enables their installation is enabled on our side prior to trying to use them.

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/)
  • 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/` 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. labels Jan 16, 2025
@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 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 16, 2025
@jmbaur
Copy link
Contributor Author

jmbaur commented Mar 19, 2025

gentle ping @NixOS/systemd

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.

Do the TPM2 features really require building systemd-boot? There's a lot of TPM2 features in systemd that have nothing to do with systemd-boot. Most of them, in fact.

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.

Huh, apparently it does. In fact, it seems to require openssl too, which I suppose isn't surprising.

Frankly, I think this means that we should actually have assertions in systemd/default.nix like assert withTpm2Tss -> withBootloader && withOpenSSL;, rather than adding conditional logic to these modules.

@ElvishJerricco
Copy link
Contributor

The other, non-tpm2 parts of this PR look good though.

@ElvishJerricco
Copy link
Contributor

Ah, no, that's wrong. There are parts of systemd that use the TPM2 without requiring boot loader stuff, like systemd-creds. So an assertion in the package expression is wrong.

Regardless, it still seems like we need to add withOpenSSL to these checks, right? Maybe we could add an extra passthru like withTpm2Units = withTpm2Tss && withBootloader && withOpenSSL or something.

jmbaur added 2 commits March 19, 2025 08:13
Exposes if a special trio of options are enabled in the systemd build,
for convenience when accessing in NixOS modules.
Some upstream systemd units are conditionally installed into the systemd
output, so we must make sure the feature that enables their installation
is enabled on our side prior to trying to use them.
@jmbaur jmbaur force-pushed the systemd-upstream-units-fix branch from f9c9ff2 to dad880d Compare March 19, 2025 15:15
@jmbaur
Copy link
Contributor Author

jmbaur commented Mar 19, 2025

@ElvishJerricco applied your suggestions! Added changes to the systemd packge, though they all exist in passthru, so we shouldn't incur the cost of a rebuild.

@jmbaur jmbaur requested a review from ElvishJerricco March 19, 2025 15:59
@ElvishJerricco ElvishJerricco merged commit ecbf53f into NixOS:master Mar 19, 2025
26 of 27 checks passed
@jmbaur jmbaur deleted the systemd-upstream-units-fix branch March 20, 2025 01:53
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 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: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants