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/gnome3: split up #67466

Merged
merged 3 commits into from Aug 26, 2019
Merged

Conversation

worldofpeace
Copy link
Contributor

This introduces the following options under the services.gnome3 namespace:

  • core-os-services.enable
  • core-shell.enable
  • core-utilities.enable
  • games.enable

The first three are all default enabled by gnome3.enable
and their purpose is to make gnome3 more flexable for users
usecases. In the case of core-utilities and games, it allows
users to easily switch on the default gnome3 applications
and games packages. Previously we had lists in gnome-3/default.nix
but they weren't visible to the user. By having options we have
generated documentation and an interface.

Motivation for this change

I "split it up" though I decided to intersect them.
If you dislike that approach feel free to suggest an alternate.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

core-os-services.enable = mkEnableOption "essential services for GNOME3";
core-shell.enable = mkEnableOption "GNOME Shell services";
core-utilities.enable = mkEnableOption "GNOME core utilities";
games.enable = mkEnableOption "GNOME games";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to document what packages get enabled with relatedPackages but I need some sort of hack like

It would also be nice in the case of core-utilities if there's a convenient way to show what programs options get enabled.

@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) labels Aug 25, 2019
@worldofpeace
Copy link
Contributor Author

This shouldn't bring any breaking changes yet like changing defaults.
I'll do that after this, since it'll look a lot clearer.

Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

Very nice work!

nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
environment.variables.XDG_DATA_DIRS = [ "${mimeAppsList}/share" ];

# Override GSettings schemas
environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = "${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas";
Copy link
Contributor

Choose a reason for hiding this comment

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

Just unrelated brain storm:

Suggested change
environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = "${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas";
environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = "${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/${nixos-gsettings-desktop-schemas.name}/glib-2.0/schemas";

or

Suggested change
environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = "${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas";
environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = glib.getSchemaPath nixos-gsettings-desktop-schemas;

and in glib

passthru.getSchemaPath = pkg: "${pkg}/share/gsettings-schemas/${pkg.name}/glib-2.0/schemas";

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, the function in glib is good. Will add that outside this pr.

environment.variables.NIX_GSETTINGS_OVERRIDES_DIR = "${nixos-gsettings-desktop-schemas}/share/gsettings-schemas/nixos-gsettings-overrides/glib-2.0/schemas";

# If gnome3 is installed, build vim for gtk3 too.
nixpkgs.config.vim.gui = "gtk3";
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could move this into a setting common for GTK desktop environments.

(mkIf serviceCfg.core-shell.enable {
services.colord.enable = mkDefault true;
services.gnome3.glib-networking.enable = true;
services.gnome3.gnome-keyring.enable = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Weird that keyring is here but goa or e-d-s, which rely on it, are in core-os.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Partial translation of https://gitlab.gnome.org/GNOME/gnome-build-meta/blob/master/elements/core/meta-gnome-core-shell.bst#L14

I think they only expressed the need for

services.gnome3.evolution-data-server.enable
services.gnome3.gnome-online-accounts.enable
services.gnome3.gnome-online-miners.enable
services.gnome3.tracker-miners.enable
services.gnome3.tracker.enable

as core-deps of their respective applications.

I'll promote it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment explaining the discrepancy.

services.upower.enable = config.powerManagement.enable;
services.xserver.libinput.enable = mkDefault true; # for controlling touchpad settings via gnome control center

xdg.portal.enable = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be mkDefault.

nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/gnome3.nix Outdated Show resolved Hide resolved
@jtojnar
Copy link
Contributor

jtojnar commented Aug 26, 2019

Otherwise, I would say add release notes and it will be ready for merge.

This introduces the following options under the services.gnome3 namespace:

* core-os-services.enable
* core-shell.enable
* core-utilities.enable
* games.enable

The first three are all default enabled by gnome3.enable
and their purpose is to make gnome3 more flexable for users
usecases. In the case of core-utilities and games, it allows
users to easily switch on the default gnome3 applications
and games packages. Previously we had lists in gnome-3/default.nix
but they weren't visible to the user. By having options we have
generated documentation and an interface.
@worldofpeace
Copy link
Contributor Author

Otherwise, I would say add release notes and it will be ready for merge.

I think it would be convenient for me to do this once I harmonize the defaults immediately following this.
Currently core-utilities is just the optional packages from before, and needs to split into the actual core and something like extra-packages.

@worldofpeace worldofpeace merged commit 450a180 into NixOS:master Aug 26, 2019
@worldofpeace worldofpeace deleted the gnome3-defaults-cleanup branch August 26, 2019 09:56
@worldofpeace
Copy link
Contributor Author

Thanks for a thorough review @jtojnar 🥇

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

2 participants