Navigation Menu

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

Jump to bottom

nixos/config: add defaultPackages option #97171

Merged
merged 1 commit into from Sep 8, 2020
Merged

nixos/config: add defaultPackages option #97171

merged 1 commit into from Sep 8, 2020

Conversation

davidak
Copy link
Member

@davidak davidak commented Sep 5, 2020 •
edited

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

nixos/modules/config/system-path.nix Show resolved Hide resolved
@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
@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 <timokau@zoho.com>
Co-authored-by: 8573 <8573@users.noreply.github.com>
@samuelgrf
Copy link
Member

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.

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

@8573 8573 8573 left review comments

@Ma27 Ma27 Ma27 left review comments

@timokau timokau timokau approved these changes

@worldofpeace worldofpeace worldofpeace approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@davidak @timokau @samuelgrf @stigtsp @jtojnar @8573 @Ma27 @worldofpeace