nixos/logrotate: enable multiple paths per entry

[ju1m]

Motivation for this change

Being able to use multiple paths per entry.

Things done

• Change path from str to either str (listOf str), preserve backward-compatibility.
• Use ExecStart= directly instead of script=.
• Add internal name option.

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.05 Release Notes (or backporting 21.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[aanderse]

Good idea 👍 Thanks!

[ju1m]

Because of /var/log/{wtmp,btmp} existing by default and growing indefinitely over time, there's a case to be made about enabling services.logrotate.enable by default.

[aanderse]

Because of /var/log/{wtmp,btmp} existing by default and growing indefinitely over time, there's a case to be made about enabling services.logrotate.enable by default.

It is a good point... but I think erasing your darlings is so trendy in our community you might get push back on that 😄

Might be worth pulling in some other opinions on that. I mentioned that our distro doesn't use logrotate enough a long time ago and did some initial work on the problem, but never took it very far.

[martinetd]

Even erasing everything on boot doesn't get you close enough when some systems have years of uptime... which is bad in itself because no kernel update yes I agree but they still happen quite a bit of everywhere; I think it's orthogonal really. And if you didn't care that it wasn't here, you likely won't care either that it is here.

(I'm the one who brought up wtmp/btmp growing to @ju1m in the first place, thanks for this PR!)

To add a second point in favor of on-by-default, if services.logrotate.enable gets enabled by default I'll also add the few other paths I've noticed to the appropriate services (e.g. in services.nginx I'd add /var/log/nginx/*.log), but if it doesn't make it default I don't really see the point as people would likely not benefit from it: either they're enabling it themselves in which case they'll probably have the paths they need added or they don't and nothing happens.

[aanderse]

I think this PR is good to go as it currently is without any further discussion. Once @ryantm approves we can click merge.

I like your idea to enable logrotate by default. This requires more discussion and we shouldn't hang this PR up on that discussion which might take a while...

Should we start a new discussion on discourse, etc... about enabling logrotate by default?

[ryantm]

looks good to me

[ryantm]

Here's some discussion about rotating these tmp files systemd/systemd#8295

[martinetd]

Yes, this is definitely good on its own without enabling by default, let's discuss that elsewhere :)

[aanderse]

Thanks everyone!