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: don't enable modules for excludePackages #98510

Merged
merged 1 commit into from Oct 6, 2020

Conversation

@mvnetbiz
Copy link
Contributor

@mvnetbiz mvnetbiz commented Sep 23, 2020

Motivation for this change

#24667
some packages disabled through environment.gnome3.excludePackages end up enabled anyways by their program module.
example:

services.xserver.desktopManager.gnome3.enable = true;
environment.gnome3.excludePackages = [ pkgs.gnome3.sushi ];
$ which sushi
/run/current-system/sw/bin/sushi
Things done

Enable program module by default only if it's corresponding program is not listed in "excludedPackages"

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Sep 23, 2020

These are confusing options since for these specific programs using environment.gnome3.excludePackages doesn't do anything, and if this solution is accepted, then there are two separate ways to configure the same thing. Perhaps a better way would be in each relevant program module (ex. nixos/modules/services/desktops/gnome3/sushi.nix) to have an assertion that the package is not in excludePackages?

@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Sep 23, 2020

I think it actually belongs in gnome3 module since that is what enables it, and the program modules should probably be independent of gnome3, and some people might use them separately. I don't think anyone is relying on the silent ignoring of excludePackages that will get left without a program they had been using, so I think it belongs here.

@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Oct 1, 2020

@mvnetbiz This is nice!!! I was wondering of a way to do this, because it would make everything a lot nicer.
I did document the old behavior in #86288, and I wasn't happy that excludePackages wouldn't work for modules.

@worldofpeace worldofpeace requested a review from Oct 1, 2020
@worldofpeace worldofpeace added this to the 20.09 milestone Oct 1, 2020
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Can you make your PR title the same as the commit message?

@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Oct 1, 2020

Maybe we should have integration testing for excludePackages. I wouldn't require it in this PR because our tests for gnome aren't that great.

@mvnetbiz mvnetbiz changed the title nixos/gnome3: don't enable modules for excludePackages gnome3: don't enable modules for excludePackages Oct 1, 2020
@worldofpeace worldofpeace force-pushed the gnome3-excludepackages branch from 6a7b076 to bc970e9 Oct 2, 2020
@worldofpeace worldofpeace changed the title gnome3: don't enable modules for excludePackages nixos/gnome3: don't enable modules for excludePackages Oct 2, 2020
@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Oct 2, 2020

Sorry about that, it seems what I said could easily been interpreted the other way around. Module changes should have nixos/.

Copy link
Contributor

@worldofpeace worldofpeace left a comment

I tested in a gnome vm with

environment.gnome3.excludePackages = [
  pkgs.gnome3.geary
];

and verified that geary wasn't included in the installed system.

@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Oct 2, 2020

Oh that makes sense. I was confused by GitHub UI I couldn't tell what the suggested review actually was. It doesn't explicitly show in the files/review tab when there is commit message amendment I think.

@worldofpeace worldofpeace merged commit 89281dd into NixOS:master Oct 6, 2020
17 checks passed
@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Oct 6, 2020

backported in 3c4e4be
Thanks a lot for this, it feels elegant in that the user doesn't have to care about the silly fact that not everything can be installed with environment.systemPackages

@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Oct 6, 2020

Thanks! I didn't want to overcomplicate it, maybe there will be a more general approach to this kind of situation at some point.

@mvnetbiz mvnetbiz deleted the gnome3-excludepackages branch Oct 6, 2020
@jtojnar jtojnar mentioned this pull request Dec 1, 2020
10 tasks
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Mar 15, 2021

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/debloating-gnome/11975/2

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

Successfully merging this pull request may close these issues.

None yet

3 participants