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/pipewire: add assertions for migration to extraConfig/configPackages #291946

Merged

Conversation

hcsch
Copy link
Contributor

@hcsch hcsch commented Feb 27, 2024

Description of changes

The PR #282377 made files/directories specified in environment.etc."pipewire<...>" and environment.etc."wireplumber<...>" conflict with existing configuration of the PipeWire NixOS module due to how the configPackages options were implemented. This sadly wasn't easily avoidable. As this can cause breakage for users moving from 23.11 to 24.05 though, assertions can help guide them to use services.pipewire.extraConfig or services.pipewire.configPackages / services.wireplumber.configPackages instead, fixing the breakage.

Wireplumber probably still needs an additional extraConfig and extraScrips (or extraPolicies, extraScripts; open for bikeshedding) options.

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: module (update) This PR changes an existing module in `nixos/` labels Feb 27, 2024
@tpwrules
Copy link
Contributor

Need to look more carefully, but "deprecated" should be replaced by "removed". If the old ones don't work and it's an assert failure to use them it's not a deprecation.

@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 28, 2024
…Packages`

The PR NixOS#282377 made files/directories specified in
`environment.etc."pipewire<...>"` and `environment.etc."wireplumber<...>"`
conflict with existing configuration of the PipeWire NixOS module due to how
the `configPackages` options were implemented. This sadly wasn't easily
avoidable. As this can cause breakage for users moving from 23.11 to 24.05
though, assertions can help guide them to use `services.pipewire.extraConfig`
or `services.pipewire.configPackages` / `services.wireplumber.configPackages`
instead, fixing the breakage.
@hcsch hcsch force-pushed the pipewire-wireplumber-config-packages-migration branch from 2077e78 to 5f6dca8 Compare February 28, 2024 00:22
@K900 K900 merged commit a64a75a into NixOS:master Feb 28, 2024
20 checks passed
@hcsch hcsch deleted the pipewire-wireplumber-config-packages-migration branch February 28, 2024 08:35
@NotAShelf
Copy link
Member

I don't understand the decision to force an user to package WirePlumber rules to be able to write a few rules for WP. Is there a reason why this assertion was thrown in, but not an extraConfig option?

@hcsch
Copy link
Contributor Author

hcsch commented Feb 28, 2024

This assertion PR was just a quick fix. I'm afraid the main PR was just not ready yet in hindsight. I'll work on a follow-up for an extraConfig option for wireplumber today to get that fixed

@leonm1
Copy link
Contributor

leonm1 commented Mar 2, 2024

Is it possible to add this to the backwards incompatible changes section in the release notes?

I ran into this issue and checked the release notes, but I was unable to figure out how to use the new option, since man configuration.nix did not contain the referenced option (yet), since I wasn't able to build a nixos configuration containing that option 😅

I did find this commit, which would help others experiencing the same.

hcsch added a commit to hcsch/nixpkgs that referenced this pull request Mar 4, 2024
Document the breaking change caused by NixOS#282377. Assertions for better
error messages for this breaking change were added in NixOS#291946. NixOS#292115
will add a better migration path for WirePlumber by adding `extraConfig`
style options that previously did not exist for WirePlumber.
@hcsch
Copy link
Contributor Author

hcsch commented Mar 4, 2024

@leonm1 created #293248 to document this breaking change in the release notes of 24.05. Thanks for bringing the release notes to my attention

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

6 participants