nixos/systemPackages: clean up #91213

davidak · 2020-06-21T07:07:46Z

Motivation for this change

clean up unneeded packages, reduce NixOS closure size

in detail:

perl (there shouldn't be a script that depend on it implicitly. nixos-generate-config.pl does not)
rsync (was needed for nixos-install, but not anymore 0aa7520)
strace (not really an enduser tool)

Things done

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)

ran some random tests

tests.login.x86_64-linux
tests.env.x86_64-linux
nix-build nixos/tests/installer.nix Killed because it needs more than 40 GB RAM
nix-build nixos/tests/misc.nix
nix-build nixos/tests/boot.nix

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)

the closure size is not different:

[davidak@gaming:~/code/nixpkgs]$ nix-build -I nixpkgs=/home/davidak/code/nixpkgs nixos/release.nix -A containerTarball.x86_64-linux

[davidak@gaming:~/code/nixpkgs]$ du -sclh $(nix-store -qR ./result)
141M	/nix/store/74ja4msci7dgc67b1x5nijwk0qqdqlbb-tarball
141M	total

[davidak@gaming:~/code/nixpkgs]$ du -sclh $(nix-store -qR ./result)
141M	/nix/store/zz2575f226dbkiydqhvghh1wacwwz5fs-tarball
141M	total

the difference:

these derivations will be built:
  /nix/store/1cbghy1ahiacglpahix2i2xp4mkcba38-system-path.drv
  /nix/store/nh4k6q73ld8b5jdr8gs5lqwl348gj0q1-dbus-1.drv
  /nix/store/c365dnl3ahqvz0ysa6fqgaa5l2g4mgl0-unit-dbus.service.drv
  /nix/store/r0psm5vmx2vdxza0n7ld5picxrz17p5g-unit-polkit.service.drv
  /nix/store/zlvqbp5ddbc6fb76f2nh51iwfas2nrmr-unit-systemd-fsck-.service.drv
  /nix/store/f0ywnddw6qmqzk0ypmvlywknzwskrprq-system-units.drv
  /nix/store/5gvkvjqs0gr9iisksf3yk2f76avb98f5-unit-dbus.service.drv
  /nix/store/p43i555b9nlymcf7fhsqs00kql6xppzw-user-units.drv
  /nix/store/1cm5vvcnlyahp9vqfjmp8z8brlpsfh65-etc.drv
  /nix/store/dqwcjx8762f7b1nz6kvg0nhhnwklvhg9-nixos-20.09pre130979.gfedcba.drv
  /nix/store/l1nh855k4gdkzm0y99z2xvv6pfgx4l6w-local-cmds.drv
  /nix/store/krrn5h1910man8qk6x604x3spbfmlms5-stage-2-init.sh.drv
  /nix/store/0lwd9iy6zzmcgsj97b7h12qcis5qmghr-nixos-system-nixos-20.09pre130979.gfedcba.drv
  /nix/store/wdp3705pa6hy0m81w48jkkq26qh773fw-closure-info.drv
  /nix/store/8nnnjg0lszvip6bhwlghr0vvdx1dld7b-tarball.drv
these paths will be fetched (4.03 MiB download, 5.53 MiB unpacked):
  /nix/store/gzp6iwfqai2695745ll1japqqv00kh5h-perl-5.30.3-man
  /nix/store/jvvx1r502zp5sqnknd7r986zvjv0xhgm-strace-5.7
  /nix/store/ps9d4h1brmbwv67z0kkk2lhxmn6dgg7l-rsync-3.1.3
  /nix/store/r84yia1yf5w0rf6vxmk1x026kjr8q62g-libunwind-1.4.0
  /nix/store/z72xf9frcp3vk2pjx8xq0c3km8lyammp-popt-1.16

Ensured that relevant documentation is up to date
Fits CONTRIBUTING.md.

Ma27 · 2020-06-21T22:04:30Z

Well, why is this an improvement if the closure size isn't decreased? I assume that at least a perl is still in the closure for e.g. nixos-generate-config.

davidak · 2020-06-22T11:01:56Z

I assume that at least a perl is still in the closure for e.g. nixos-generate-config.

correct, but that changes hopefully in the future

Well, why is this an improvement if the closure size isn't decreased?

it's cleaner when you have less. it don't really make sense to install these tools on any NixOS system

it might decrease in other situations. like the build output shows 5.53 MiB

might also save hydra builds and cache traffic

davidak · 2020-06-23T19:59:34Z

correct, but that changes hopefully in the future

Maybe even faster than expected #91353

xaverdh · 2020-06-24T09:29:22Z

Actually I am a bit surprised the requiredPackages list is that large. And what about config.programs.ssh.package, why doesn't the ssh module put that into systemPackages ?

vcunat

Sounds good. (but I haven't done any testing myself)

Ma27 · 2020-06-24T10:51:52Z

it's cleaner when you have less. it don't really make sense to install these tools on any NixOS system

Fair enough. 👍 from me then :)

nixos/doc/manual/release-notes/rl-2009.xml

Co-authored-by: Jan Tojnar <jtojnar@gmail.com>

davidak · 2020-06-24T18:33:54Z

Actually I am a bit surprised the requiredPackages list is that large.

yeah, me too. the worst part is that most of it is actually needed by nixos core tools like nixos-rebuild... implicitly

nixos/doc/manual/release-notes/rl-2009.xml

Co-authored-by: 8573 <8573@users.noreply.github.com>

xaverdh · 2020-07-05T11:10:16Z

Maybe its not wise to blow up the scope of this pr, but two other packages which can probably be removed without trouble are awk and the time command. awk will probably still be on the system for stdenv though, and time is a rather small binary (sans glibc).

nixos-discourse · 2020-08-07T21:06:12Z

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/199

davidak · 2020-08-09T06:06:15Z

/marvin opt-in
/status needs_merger

more changes go in another pr

marvin-mk2 · 2020-08-09T06:06:17Z

Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage here.

marvin-mk2 · 2020-08-09T06:06:18Z

The PR author cannot set the status to needs_merger. Please wait for an external review.

If you are not the PR author and you are reading this, please review the usage of this bot. You may be able to help. Please make an honest attempt to resolve all outstanding issues before setting to needs_merger.

davidak · 2020-08-09T06:09:12Z

there are already 4 reviews, but ok

/status needs_reviewer

stigtsp · 2020-08-25T08:20:22Z

I don't think removing perl from systemPackages is a good idea. In addition to being a general purpose language, it is frequently used for oneliners, as a awk/grep/sed/etc replacement in shell scripts, etc.

davidak · 2020-08-25T10:18:39Z

Removing perl is pointless, since we still depend on perl in the base system.

as said above, that changes hopefully in the future

strace is IMHO an indispensable debugging tool.

it is not needed for the system to run, so requiredPackages is at least the wrong place. i suggested to add a defaultPackages with packages a user would expect from a minimal linux. we can discuss if strace and perl belong in this categorie

#32405 (comment)

NixOS#91213 removed `perl` from $PATH. This adds a patch to oh-my-zsh, using `sed` instead of `perl` to do the regexp substitution.

They are no longer installed as part of the standard required packages since NixOS/nixpkgs#91213 I haven't been explicitly using perl for a while, so let's see how long I can go before I miss it.

dhess · 2020-09-02T16:14:56Z

This breaks NixOps, which assumes that rsync is available on the target host. Worse yet, it deploys one last time just fine and everything looks peachy keen, but then the next time you deploy, it breaks when it attempts to upload keys.

Ma27 · 2020-09-02T16:47:45Z

So due to the big variety of broken things (the nixops issue @dhess described is especially bad IMHO) and the fact that people "hope" that some things will change in the future I'm actually reconsidering my 👍 on this.

To avoid even more issues in the upcoming weeks I'd suggest to revert this patch, at least for 20.09.

aanderse · 2020-09-02T17:11:00Z

Gosh... it is impurities like these that convince me this change is a good idea. But I agree this should be reverted for 20.09 and discussed early in the 21.03 release cycle.

davidak · 2020-09-02T21:06:51Z

We can introduce defaultPackages now and add these two tools back. Then nothing changes in the next release.

davidak · 2020-09-05T01:00:02Z

Check out my new PR: #97171

I hope that this resolves all concerns.

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>

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> (cherry picked from commit 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>

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> Co-authored-by: davidak <git@davidak.de> Co-authored-by: Timo Kaufmann <timokau@zoho.com> Co-authored-by: 8573 <8573@users.noreply.github.com>

Previously this worked because rsync was included in NixOS by default, not anymore the case with NixOS/nixpkgs#91213

nixos/systemPackages: clean up

623aaa2

ofborg bot added 6.topic: nixos 8.has: changelog 8.has: documentation 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 labels Jun 21, 2020

vcunat approved these changes Jun 24, 2020

View reviewed changes

jtojnar reviewed Jun 24, 2020

View reviewed changes

nixos/doc/manual/release-notes/rl-2009.xml Outdated Show resolved Hide resolved

Update nixos/doc/manual/release-notes/rl-2009.xml

ec5005e

Co-authored-by: Jan Tojnar <jtojnar@gmail.com>

8573 suggested changes Jun 24, 2020

View reviewed changes

nixos/doc/manual/release-notes/rl-2009.xml Outdated Show resolved Hide resolved

Update nixos/doc/manual/release-notes/rl-2009.xml

75b01e7

Co-authored-by: 8573 <8573@users.noreply.github.com>

jtojnar requested a review from edolstra June 24, 2020 22:37

Ma27 approved these changes Jun 29, 2020

View reviewed changes

marvin-mk2 bot added the marvin label Aug 9, 2020

marvin-mk2 bot added the needs_reviewer label Aug 9, 2020

marvin-mk2 bot requested a review from timokau August 11, 2020 10:23

marvin-mk2 bot removed the needs_reviewer label Aug 11, 2020

flokli mentioned this pull request Aug 28, 2020

oh-my-zsh: cleanups, don't require perl in pygmalion theme anymore #96532

Merged

10 tasks

flokli added a commit to flokli/nixpkgs that referenced this pull request Aug 28, 2020

oh-my-zsh: apply patch to remove usages of perl

b07a1dc

NixOS#91213 removed `perl` from $PATH. This adds a patch to oh-my-zsh, using `sed` instead of `perl` to do the regexp substitution.

davidak mentioned this pull request Sep 5, 2020

Add NixOps test #97166

Closed

davidak mentioned this pull request Sep 5, 2020

nixos/config: add defaultPackages option #97171

Merged

10 tasks

cole-h mentioned this pull request Sep 7, 2020

cantata: dynamic playlists broken due to removal of perl from default system packages #97402

Closed

andir mentioned this pull request Sep 11, 2020

nixops send-keys depends on rsync being on PATH NixOS/nixops#1399

Open

Gabriella439 mentioned this pull request Sep 13, 2021

nixos/config: add defaultPackages option awakesecurity/nixpkgs#16

Merged

J4NV5 pushed a commit to J4NV5/DarkOps that referenced this pull request Dec 2, 2021

Make sure to use an existing rsync on the target host

4040cf8

Previously this worked because rsync was included in NixOS by default, not anymore the case with NixOS/nixpkgs#91213

stigtsp mentioned this pull request Sep 13, 2023

nixos/system-path: Drop default Perl package #254942

Open

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/systemPackages: clean up #91213

nixos/systemPackages: clean up #91213

davidak commented Jun 21, 2020

Ma27 commented Jun 21, 2020

davidak commented Jun 22, 2020

davidak commented Jun 23, 2020

xaverdh commented Jun 24, 2020

vcunat left a comment

Ma27 commented Jun 24, 2020

davidak commented Jun 24, 2020

xaverdh commented Jul 5, 2020

nixos-discourse commented Aug 7, 2020

davidak commented Aug 9, 2020

marvin-mk2 bot commented Aug 9, 2020

marvin-mk2 bot commented Aug 9, 2020

davidak commented Aug 9, 2020

stigtsp commented Aug 25, 2020

davidak commented Aug 25, 2020

dhess commented Sep 2, 2020 •

edited

Ma27 commented Sep 2, 2020

aanderse commented Sep 2, 2020

davidak commented Sep 2, 2020

davidak commented Sep 5, 2020

nixos/systemPackages: clean up #91213

nixos/systemPackages: clean up #91213

Conversation

davidak commented Jun 21, 2020

Motivation for this change

Things done

Ma27 commented Jun 21, 2020

davidak commented Jun 22, 2020

davidak commented Jun 23, 2020

xaverdh commented Jun 24, 2020

vcunat left a comment

Choose a reason for hiding this comment

Ma27 commented Jun 24, 2020

davidak commented Jun 24, 2020

xaverdh commented Jul 5, 2020

nixos-discourse commented Aug 7, 2020

davidak commented Aug 9, 2020

marvin-mk2 bot commented Aug 9, 2020

marvin-mk2 bot commented Aug 9, 2020

davidak commented Aug 9, 2020

stigtsp commented Aug 25, 2020

davidak commented Aug 25, 2020

dhess commented Sep 2, 2020 • edited

Ma27 commented Sep 2, 2020

aanderse commented Sep 2, 2020

davidak commented Sep 2, 2020

davidak commented Sep 5, 2020

dhess commented Sep 2, 2020 •

edited