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/{hyprland, wayland-common}: disable wlr portal for hyprland, enable xdg autostart for all wayland compositors #315827

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

JohnRTitor
Copy link
Contributor

@JohnRTitor JohnRTitor commented May 30, 2024

Description of changes

Hyprland has its own portal xdg-desktop-portal-hyprland and does not need the wlr portal.

As a follow up to #240989,

  • Adds wlr-portal override of wayland-session module (enabled by default)
  • Disable it for hyprland module

Additionally, enable xdg autostart service for Wayland Window compositors as unlike DEs, they do not handle it themselves.

Things done

  • Tested, as applicable:
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

CC: @fufexan @SuperSandro2000


Add a 👍 reaction to pull requests you find important.

This commit:
- Adds wlr-portal override of wayland-session module (enabled by default)
- Disable it for hyprland module
Copy link
Contributor

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

LGTM.

@JohnRTitor
Copy link
Contributor Author

@fufexan is there a reason you are not in meta.maintainers of nixos/hyprland module? :)

@fufexan
Copy link
Contributor

fufexan commented May 30, 2024

AFAIK it's not required to have a maintainer for a module, so I didn't add myself when I created it.
Feel free to add me though, it may allow for greater visibility in issues.

@JohnRTitor
Copy link
Contributor Author

Feel free to add me though, it may allow for greater visibility in issues.

Exactly my point. Done

@JohnRTitor
Copy link
Contributor Author

I plan to backport this to stable as well, but #314978 should be merged for backport to work.

@JohnRTitor JohnRTitor added the backport release-24.05 Backport PR automatically label May 31, 2024
@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/4014

@JohnRTitor JohnRTitor requested a review from wineee June 1, 2024 04:59
@JohnRTitor
Copy link
Contributor Author

JohnRTitor commented Jun 1, 2024

@fufexan perhaps also add services.xserver.desktopManager.runXdgAutostartIfNone = true to this?

From the description:

Whether to run XDG autostart files for sessions without a desktop manager (with only a window manager), these sessions usually don’t handle XDG autostart files by default.

@fufexan
Copy link
Contributor

fufexan commented Jun 1, 2024

@fufexan perhaps also add services.xserver.desktopManager.runXdgAutostartIfNone = true to this?

From the description:

Whether to run XDG autostart files for sessions without a desktop manager (with only a window manager), these sessions usually don’t handle XDG autostart files by default.

Might make sense. Would it conflict with the HM option? It is being set to true by default in the HM module for Hyprland.

@JohnRTitor
Copy link
Contributor Author

The mechanism behind the option on Nixpkgs is just a systemd service.

I feel like Home Manager, by itself, has a nice mechanism for avoiding conflicts.
For example, some bash and zsh options are enabled by default in nixos configuration, but if their counterpart options are disabled in home manager module, NixOS respects and prioritises Home manager modules.

I believe if any actions are to be needed, they need to be performed on the Home Manager side. Home manager bases its development off Nixpkgs master, not the other way around.

@JohnRTitor JohnRTitor changed the title nixos/hyprland: don't enable wlr portal for hyprland module nixos/{hyprland, wayland-common}: disable wlr portal for hyprland, enable xdg autostart for all wayland compositors Jun 1, 2024
@JohnRTitor
Copy link
Contributor Author

I added it to the common wayland-session module, so that other WMs can utilise this as well.

@JohnRTitor JohnRTitor requested a review from fufexan June 1, 2024 17:04
 xwayland, wlr-portal ->
 enable-xwayland, enable-wlr-portal
@eclairevoyant eclairevoyant removed the backport release-24.05 Backport PR automatically label Jun 3, 2024
@Mic92
Copy link
Member

Mic92 commented Jun 4, 2024

@fufexan is this pull request good to go?

Copy link
Contributor

@fufexan fufexan left a comment

Choose a reason for hiding this comment

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

@Mic92 yes, LGTM.

@JohnRTitor
Copy link
Contributor Author

Thanks! @Mic92 would love to see this merged.

@Mic92 Mic92 merged commit 7d98bbf into NixOS:master Jun 5, 2024
22 checks passed
@JohnRTitor JohnRTitor deleted the hyprland-module branch June 5, 2024 08:06
@BhasherBEL
Copy link
Contributor

BhasherBEL commented Jun 6, 2024

Is there a particular reason for which this change isn't flagged with a lib.mkDefault? In my configuration, I set it to true (without mkForce or mkDefault) but it now has conflicting definitions since the last update.

error: The option `xdg.portal.wlr.enable' has conflicting definition values:
- In `/nix/store/aacfsg6r005hi7gna3s1vlpzrg5v5264-source/hosts/shared/pc/hyprland.nix': true
- In `/nix/store/8s55w0927lh3mdbkxf434zb0c5hqsz8z-source/nixos/modules/programs/wayland/hyprland.nix': false
Use `lib.mkForce value` or `lib.mkDefault value` to change the priority on any of these definitions.

@eclairevoyant
Copy link
Member

eclairevoyant commented Jun 6, 2024

FYI you don't need the wlr portal if you use hyprland. The hyprland portal was forked from the wlroots one.

@SuperSandro2000
Copy link
Member

Is there a particular reason for which this change isn't flagged with a lib.mkDefault?

That people will notice it and hopefully remove it.

@wxlyyy
Copy link

wxlyyy commented Jun 8, 2024

services.flatpak.enable = true;

       error:
       Failed assertions:
       - To use Flatpak you must enable XDG Desktop Portals with xdg.portal.enable.

@eclairevoyant
Copy link
Member

Please create a new issue with config details @wxlyyy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet