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/gnome-remote-desktop: add package to environment.systemPackages… #301246

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

Nanotwerp
Copy link
Contributor

…, services.dbus.packages, create gnome-remote-desktop user and group (fixes for GNOME 46)

This adds the g-r-d package to environment.systemPackages (allowing the usage of the grdctl command along with enabling g-r-d's polkit rule), makes its dbus-related files recognizable to dbus, and creates the gnome-remote-desktop user and group necessary for systemd's running of the gnome-remote-desktop-daemon with the --system subcommand and enabling Remote Login.

Description of changes

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 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: module (update) This PR changes an existing module in `nixos/` labels Apr 3, 2024
@Nanotwerp Nanotwerp mentioned this pull request Apr 3, 2024
27 tasks

systemd.packages = [ pkgs.gnome.gnome-remote-desktop ];
systemd.tmpfiles.packages = [ pkgs.gnome.gnome-remote-desktop ];

users = {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I thought it would be automatically set up but looks like it is still experimental in NixOS and disabled by default:

Note: This is experimental.

Copy link
Contributor Author

@Nanotwerp Nanotwerp Apr 3, 2024

Choose a reason for hiding this comment

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

Ooh, I didn't even know that NixOS even had that feature! Would you recommend that I swap out the manual user and group creation for systemd.sysusers.enable = true?

EDIT: Absolutely nevermind. I just experienced the scariest thing known to man after enabling systemd.sysusers while having mutableUsers disabled. All of my users were rendered useless and I couldn't login to any of them. Thank every god known to mankind for Nix generations. I should've listened when I heard it was experimental.

Copy link
Contributor

@amaxine amaxine Apr 3, 2024

Choose a reason for hiding this comment

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

Should user creation be conditional based on sysusers being enabled? Do we do that anywhere right now - and should we? Afaict we only have a comment next to flatpak to switch to sysusers at some point when possible.

Copy link
Member

Choose a reason for hiding this comment

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

Would you recommend that I swap out the manual user and group creation for systemd.sysusers.enable = true?

Yeah, that could have unintended side-effects, as you found out. Stuff like this is usually better enabled top-down, rather than bottom up.

Should user creation be conditional based on sysusers being enabled? Do we do that anywhere right now - and should we?

Not sure how it behaves when there are conflicts between sysusers and users.users. It might be a good idea to test it for future-proofing once the sysusers.nix functionality is enabled but not critical at the moment.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, looking at the sysuser.nix that just makes it so users.users users are created using sysusers.d – the systemd functionality should already be enabled.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that has any effect beyond being built with necessary support, same as homed support always being compiled in but not on by default - but I'd prefer if maybe someone who maintains systemd in nix could weigh in.

Maybe for now it's enough to just add a comment in the module saying that the package provides a sysusers.d file we should switch to one day.

Copy link
Contributor

@amaxine amaxine left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for taking a look at this and getting things working.

@amaxine amaxine force-pushed the gnome-46-grd-fix branch from 74938c3 to 42c13b9 Compare April 3, 2024 17:08
Add package to environment.systemPackages, services.dbus.packages, create gnome-remote-desktop user and group (fixes for GNOME 46)

This adds the `g-r-d` package to environment.systemPackages (allowing the usage of the `grdctl` command along with enabling `g-r-d`'s polkit rule), makes its dbus-related files recognizable to dbus, and creates the `gnome-remote-desktop` user and group necessary for systemd's running of the `gnome-remote-desktop-daemon` with the `--system` subcommand and enabling Remote Login.
@amaxine amaxine force-pushed the gnome-46-grd-fix branch from 42c13b9 to be22f7c Compare April 3, 2024 17:09
@amaxine amaxine merged commit 3923e7d into NixOS:gnome-46 Apr 3, 2024
4 of 6 checks passed
@Nanotwerp Nanotwerp deleted the gnome-46-grd-fix branch April 3, 2024 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 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/`
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants