Skip to content

nixos/config: add defaultPackages option #97171

Merged
worldofpeace merged 1 commit intoNixOS:masterfrom
davidak:defaultPackages
Sep 8, 2020
Merged

nixos/config: add defaultPackages option #97171
worldofpeace merged 1 commit intoNixOS:masterfrom
davidak:defaultPackages

Conversation

@davidak
Copy link
Member

@davidak davidak commented Sep 5, 2020 •
edited
Loading

Motivation for this change

The option makes it possible to remove default packages to have a more minimal system, by setting it to an empty list.

I suggested that in #32405 (comment).

This PR also readds perl, rsync (needed for NixOps) and strace (common debugging tool) which where removed in #91213.

We can add more packages in next release.

Manual:

Screenshot from 2020-09-06 18-30-10

Things done

I built a VM and made sure perl, rsync and strace are present. Also tested to set it to empty list.

nixos-rebuild build-vm -I nixpkgs=~/code/nixpkgs/ -I nixos-config='/home/davidak/root'

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

@davidak davidak mentioned this pull request Sep 5, 2020
Merged
10 tasks
@ofborg ofborg bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 8.has: module (update) This PR changes an existing module in `nixos/` labels Sep 5, 2020
jtojnar
jtojnar reviewed Sep 5, 2020
View reviewed changes
timokau
timokau reviewed Sep 5, 2020
View reviewed changes
timokau
timokau reviewed Sep 5, 2020
View reviewed changes
timokau
timokau requested changes Sep 5, 2020
View reviewed changes
Copy link
Member

@timokau timokau left a comment

Choose a reason for hiding this comment

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

Is there any reason to ever set defaultPackages to anything but an empty list? If "the default value or an empty list" are the only two valid options, should it maybe be a boolean in the first place?

@timokau
Copy link
Member

timokau commented Sep 5, 2020

I see at least one advantage of a list: The default values will show up in the documentation without the need to list them twice. Still, intuitively it feels like it should be a boolean option. What do you think?

Ma27
Ma27 reviewed Sep 5, 2020
View reviewed changes
Copy link
Member

@Ma27 Ma27 left a comment

Choose a reason for hiding this comment

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

Please amend the commit-message to make sure it fits our contribtion guidelines.

@davidak
Copy link
Member Author

davidak commented Sep 5, 2020

Is there any reason to ever set defaultPackages to anything but an empty list? If "the default value or an empty list" are the only two valid options, should it maybe be a boolean in the first place?

@timokau good question. you would not set a different list. that's what systemPackages is for. but you might remove single packages from the list. @infinisil want's to create an RFC for a better mechanism for that #32405 (comment)

@davidak davidak changed the title Add defaultPackages set nixos/config: add defaultPackages option Sep 5, 2020
8573
8573 reviewed Sep 6, 2020
View reviewed changes
@davidak davidak force-pushed the defaultPackages branch 2 times, most recently from f5d9845 to faf5dbb Compare September 6, 2020 16:23
@davidak @timokau @8573
nixos/config: add defaultPackages option
74b3d66 
readd perl (used in shell scripts), rsync (needed for NixOps) and strace (common debugging tool)

they where previously removed in NixOS#91213

Co-authored-by: Timo Kaufmann <[email protected]>
Co-authored-by: 8573 <[email protected]>
@samuelgrf
Copy link
Member

samuelgrf commented Sep 6, 2020

Should we also add nano to the list? Some people already have another editor installed and may want to get rid of applications they don't use.

@davidak
Copy link
Member Author

davidak commented Sep 6, 2020

@samuelgrf not in this PR. we can discuss that later

@davidak
Copy link
Member Author

davidak commented Sep 6, 2020

Tested again. Everything works.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 7, 2020
worldofpeace
worldofpeace approved these changes Sep 8, 2020
View reviewed changes
Copy link
Contributor

@worldofpeace worldofpeace left a comment

Choose a reason for hiding this comment

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

LGTM, perfectly fine to backport to 20.09 as well

@worldofpeace worldofpeace merged commit 2ab42dc into NixOS:master Sep 8, 2020
@davidak davidak deleted the defaultPackages branch September 9, 2020 00:22
@davidak davidak mentioned this pull request Sep 9, 2020
Merged
10 tasks
@andir andir mentioned this pull request Sep 11, 2020
Open
@deviant deviant mentioned this pull request Oct 20, 2020
Draft
10 tasks
@stigtsp stigtsp mentioned this pull request Sep 13, 2023
Open
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stigtsp stigtsp stigtsp left review comments

@jtojnar jtojnar jtojnar left review comments

@Ma27 Ma27 Ma27 left review comments

@timokau timokau timokau approved these changes

+2 more reviewers

@8573 8573 8573 left review comments

@worldofpeace worldofpeace worldofpeace approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 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 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@davidak @timokau @samuelgrf @stigtsp @jtojnar @8573 @Ma27 @worldofpeace