nixos/slskd: refactor and add config file options

[melvyn2]

Description of changes

This PR adds the nixos module options matching those of the slskd config file and changes/fixes some behaviors of the module, most importantly fixing the nginx virtual-host behavior. Some of these are breaking changes.
This PR is a draft while I add the remaining options and fix the rough edges, to get some feedback early.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• 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/)
• 24.05 Release Notes (or backporting 23.05 and 23.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
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[melvyn2]

Some of the options added depend on #281233

[ppom0]

Hi! I'm wondering, why completely removing the log rotation? As logs are already captured on stdout by systemd, having them accumulate in /var/lib/slskd/logs (by default) is useless and takes more and more place…

[ppom0]

This remark aside, I like the nginx cleaner configuration, thanks!

[ppom0]

Also, (sorry, I think I really don't know how to be polite in English) but I'm wondering if being extensive and putting ~50 settings parameters is a good choice for maintainers. I'm not fond of rewriting the wheel, especially because the settings will change and we'll have to update those +400 SLOC reserved for settings definition.

I did a quick check on the settings options in NixOS (I might be missing other freeform options not named settings), and it seems like slskd would become the top 4 module with most options:

$ man configuration.nix | grep -Eo 'services\..*\.settings' | uniq -c | sort -n | tail
     22 services.sftpgo.settings
     24 services.syncthing.settings
     28 services.parsedmarc.settings
     28 services.transmission.settings
     35 services.system76-scheduler.settings
     37 services.headscale.settings
     43 services.matrix-synapse.settings
     92 services.grafana.settings
    145 services.tor.settings
    149 services.sourcehut.settings

[melvyn2]

Hi! I'm wondering, why completely removing the log rotation? As logs are already captured on stdout by systemd, having them accumulate in /var/lib/slskd/logs (by default) is useless and takes more and more place…

Disabling disk logging entirely by default seems like the easier option for relying on journald, so that's what I do in https://github.com/NixOS/nixpkgs/pull/281075/files#diff-4f8169c68e06f35fdf6ca23d92d2b10f8f6d9697554e2cc6a2689019b1bac9deR473

Also, (sorry, I think I really don't know how to be polite in English) but I'm wondering if being extensive and putting ~50 settings parameters is a good choice for maintainers. I'm not fond of rewriting the wheel, especially because the settings will change and we'll have to update those +400 SLOC reserved for settings definition.

This is a reasonable worry, but I don't think it's much of a problem in reality. slskd doesn't seem to have changed the configuration very much so far, and future changes will probably be just occasionally adding new options, and maybe renaming (so only a few lines will have to be changed, rather than the whole set). I don't think the workload will be much greater than already checking the changelog to make sure the module works.
It is a lot of options, but I don't see why these shouldn't be documented to the users. The way I see it, it's better to move as much of the work users must do into nix rather than other formats or documentation.

[ppom0]

Oh cool, I didn't see how to disable on-disk logging.

And fine for the large settings, as long as you add yourself as a maintainer on the package. If you can add both of us as maintainers of the module as well (doc even if I suppose you already know that), that would be nice!

[ppom0]

Also, can you document the breaking changes in the relases notes?

[ppom0]

Seems very nice. I'll test the PR on my server when it'll leave its draft status.

[h7x4]

It is a lot of options, but I don't see why these shouldn't be documented to the users. The way I see it, it's better to move as much of the work users must do into nix rather than other formats or documentation.

I disagree with this. There has already been a big discussion around this topic in RFC 42, part 2, where it was generally agreed upon that we should be a bit limited with what options we declare. Maintaining everyones documentation and config schema is not a viable option. I don't think this mass influx of option declarations is a good idea, even if I might be inclined to agree that it looks better in the manual or on search.nixos.org.

That being said, I do think there are some other very welcome changes in this PR.

[melvyn2]

I wasn’t aware of this rfc, thanks for the explanation. I’ll pare this down when I can, then.

[melvyn2]

I've removed most of the freeform options except those that I expect the majority of users to change - is this in line with the intention of that RFC?
I'll squash the commit when I undraft the PR (once these options seem reasonable, probably).

[h7x4]

Yes, I think this seems a lot better! I'll have a closer look at it later

[melvyn2]

Forgot to push two fixes I had locally... This should be ready for merge, I think.

[nixos-discourse]

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

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