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

Logrotate adds bloat to installation media closure #162001

Closed
ius opened this issue Feb 26, 2022 · 4 comments
Closed

Logrotate adds bloat to installation media closure #162001

ius opened this issue Feb 26, 2022 · 4 comments
Labels
0.kind: regression Something that worked before working no longer 6.topic: closure-size

Comments

@ius
Copy link
Contributor

ius commented Feb 26, 2022

A recently merged PR has made quite a negative impact on the size (and build time) of the sdImage and cd image closure by enabling logrotate by default.

Added size (last column) on my sdimage

unit-logrotate.service                                   531.04 MiB (60.84 MiB)
logrotate-3.19.0                                        225.29 MiB (60.83 MiB)
mailutils-3.14                                          224.68 MiB (60.72 MiB)
guile-2.2.7                                              89.45 MiB (43.27 MiB)

It adds 60 MB and pulls in Guile via indirect dependencies. 60 MB to send an email (I suppose) seems a bit much. mailutils has 11 MB NAR size and adds another 43 MB via Guile.

I noticed this as I caught my cross builder building Guile for over an hour.

Most problematic path:

# nix why-depends --derivation /nix/store/wan8hvakmsjfcmrgvnb70sffs7ybyjby-nixos-system-nixos-22.05pre-git.drv /nix/store/sa9rzkxqgxsf4yl5j9w8r4cb7d6cldql-guile-2.2.7.drv
/nix/store/wan8hvakmsjfcmrgvnb70sffs7ybyjby-nixos-system-nixos-22.05pre-git.drv
└───/: …-env.drv",["out"]),("/nix/store/ncyilwdnr3rg4xxklrihcl4wp50hy24a-etc.drv",["out"]),("/nix/store/…
    → /nix/store/ncyilwdnr3rg4xxklrihcl4wp50hy24a-etc.drv
    └───/: …ated.drv",["out"]),("/nix/store/6pf8r7n5v0jy91ya3r0p9kg116fkkljg-system-units.drv",["out"]),("/n…
        → /nix/store/6pf8r7n5v0jy91ya3r0p9kg116fkkljg-system-units.drv
        └───/: …vice.drv",["out"]),("/nix/store/in1aldq37mi7lx9r7fbilqlf9dmpnjqk-unit-logrotate.service.drv",["o…
            → /nix/store/in1aldq37mi7lx9r7fbilqlf9dmpnjqk-unit-logrotate.service.drv
            └───/: …-9.0.drv",["out"]),("/nix/store/jc26bi08y444zblqw5503n9ln0i5g49y-logrotate-3.19.0.drv",["out"]),…
                → /nix/store/jc26bi08y444zblqw5503n9ln0i5g49y-logrotate-3.19.0.drv
                └───/: …e-3.19.0","","")],[("/nix/store/6m5cyj32xkgd0rblxy5hwh7h4dv3q1dq-mailutils-3.14.drv",["out"]),("…
                    → /nix/store/6m5cyj32xkgd0rblxy5hwh7h4dv3q1dq-mailutils-3.14.drv
                    └───/: …10.0.drv",["out"]),("/nix/store/sa9rzkxqgxsf4yl5j9w8r4cb7d6cldql-guile-2.2.7.drv",["dev"]),("/ni…
                        → /nix/store/sa9rzkxqgxsf4yl5j9w8r4cb7d6cldql-guile-2.2.7.drv

Some ideas:

  • Disabling logrotate for installation media
  • Investigate whether there's an alternative to mailutils for logrotate?
  • Removing Guile support from mailutils (still would add ~17 MB)

Related:

@martinetd @orivej @vrthra @viric

@ius ius added 0.kind: bug 0.kind: regression Something that worked before working no longer and removed 0.kind: bug labels Feb 26, 2022
@martinetd
Copy link
Member

just, wow!
I'm sorry, I didn't expect logrotate to pull so much, I've also just enabled it on alpine for some tight system at work and it didn't pull in anything there so didn't even think of checking... It looks like something else pulls guile on my system anyway :| (pipewire -> libsndfile -> autogen -> guile... a runtime dependency on autogen really?! .. I'm getting sidetracked, back to logrotate)

A few points:

  • in my opinion logrotate doesn't make much sense for install media, logs will "rotate" anyway when it reboots, and few will be long lived enough to make a dent in wtmp/btmp while they're up.. So I'd probably go for that as a low hanging fruit regardless of the rest
  • even for real systems this is huge, I'm appalled at what I've done! ... although in my case I don't think a simple system mailer needs guile, nor do I think the default logrotate needs a mailer, so there are a few places we could strike. Focusing on logrotate, the package only pulls it in for the default mail command:
    "--with-default-mail-command=${mailutils}/bin/mail"
    and that could be done in the service configuration if required in my opinion (would require adding a new option to make it easy to use, but nothing difficult:
    • convert the module from extraConfig to "free form" from an attrset
    • check if the 'mail' attribute is set anywhere then pass --mail ${pkgs.mailutils}/bin/mail option to service start. Possibly add an extra toggle as well if autodetection fails or they use extraConfig.

That would remove the need for mailutils for most setups including default, and still work for people who need it.
How does that sound?

@Artturin
Copy link
Member

convert the module from extraConfig to "free form" from an attrset
check if the 'mail' attribute is set anywhere then pass --mail ${pkgs.mailutils}/bin/mail option to service start. Possibly add an extra toggle as well if autodetection fails or they use extraConfig.

i like this solution
when RFC42 options are added then the extraConfig option should be removed

@ius
Copy link
Contributor Author

ius commented Feb 27, 2022

That would remove the need for mailutils for most setups including default, and still work for people who need it.>
How does that sound?

Works for me! Should bring the size down to ~118 kB for logrotate sans mail.

@martinetd
Copy link
Member

martinetd commented Feb 27, 2022

quick follow up on this. First, thanks for the link to RFC42 -- I wasn't aware of the "freeformType" and co framework. Unfortunately logrotate config is a bit special and I wasn't able to warp my head around how to use it to generate a full config from this, so I tried a more direct approach -- I've posted what I have in a draft PR #162063 if you could take a look and advice.

It currently works in this state and does what we wanted, just a few rough edges but I won't have time to finish it for a few more days (probably) so depending on the degree of urgency you feel there is we might want to do the remaining points I've listed in the PR in two steps (e.g. finish fixing obvious problems quickly then continue with config validation steps and cleanup in a follow-up PR); I'd rather take the time to finish it properly but open to other opinions here.

thanks!

martinetd added a commit to martinetd/nixpkgs that referenced this issue Feb 28, 2022
having pkgs.logrotate depend on mailutils brings in quite a bit of dependencies
through mailutil itself and recursive dependency to guile when most people
do not need it.

Remove mailutils dependency from the package, and conditionally add it to the
service if the user specify the mail option either at top level or in a path

Fixes NixOS#162001
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: regression Something that worked before working no longer 6.topic: closure-size
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants