nixos/qbittorrent-nox: init

[Redhawk18]

use tmpfiles instead of prescript

Description of changes

This adds a service for the qbittorrent nox web ui with some basic options for the systemd job.

Things done

• Built on platform(s)
  • [x] 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.11 Release Notes (or backporting 23.11 and 24.05 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
  • [x] (Module addition) Added a release notes entry if adding a new NixOS module
• [x] Fits CONTRIBUTING.md.

---

Add a :+1: reaction to pull requests you find important.

[Redhawk18]

Is the uid and gid okay? I wasn't sure if there's a way to generate it dynamically.

[Redhawk18]

If I remember correctly, qbittorrent-nox will want you to accept some kind of legal agreement or it won't start. How is this supposed to be handled?

In my module I can just set services.qbittorrent.settings.LegalNotice.Accepted = true;, something like this would be great. There could also be an assert like this.

I believe the desktop app does, But I installed this to test it on my desktop and I was never prompted once to agree to it. It's likely a bug on their end, and even if we added an assert the eula would still show up. It's not like with minecraft where you could write true to a file and have it be accepted I believe.

[Redhawk18]

@NixOS/nix-formatting The error log told me to ping you with errors.

[misuzu]

Is the uid and gid okay? I wasn't sure if there's a way to generate it dynamically.

See https://github.com/NixOS/rfcs/blob/62d1245ec1eae275edb996229585637035c02538/rfcs/0052-dynamic-ids.md#using-ids-dynamically-allocated-by-nixos

But I installed this to test it on my desktop and I was never prompted once to agree to it.

You need to run your module on a fresh machine. A NixOS test would actually be great way to make sure it works.

even if we added an assert the eula would still show up. It's not like with minecraft where you could write true to a file and have it be accepted I believe.

Why not? Adding this to the qBittorrent.conf works just fine:

[LegalNotice]
Accepted=true

It's exactly what the module I linked does.

@NixOS/nix-formatting The error log told me to ping you with errors.

The error log says this:

Please go to the Nixpkgs root directory, run nix-shell, then: nixfmt 'nixos/modules/services/torrent/qbittorrent-nox.nix'

[Redhawk18]

I read the error message lol, I asked because I did what it suggested and it still showed an error, but it looks like it can't find the file to begin with.

[Redhawk18]

Is the uid and gid okay? I wasn't sure if there's a way to generate it dynamically.

See https://github.com/NixOS/rfcs/blob/62d1245ec1eae275edb996229585637035c02538/rfcs/0052-dynamic-ids.md#using-ids-dynamically-allocated-by-nixos

But I installed this to test it on my desktop and I was never prompted once to agree to it.

You need to run your module on a fresh machine. A NixOS test would actually be great way to make sure it works.

even if we added an assert the eula would still show up. It's not like with minecraft where you could write true to a file and have it be accepted I believe.

Why not? Adding this to the qBittorrent.conf works just fine:

[LegalNotice]
Accepted=true

It's exactly what the module I linked does.

@NixOS/nix-formatting The error log told me to ping you with errors.

The error log says this:

Please go to the Nixpkgs root directory, run nix-shell, then: nixfmt 'nixos/modules/services/torrent/qbittorrent-nox.nix'

Will this override the config even if the settings option is not set?

[misuzu]

I read the error message lol, I asked because I did what it suggested and it still showed an error, but it looks like it can't find the file to begin with.

Not sure what you mean, it works fine:

% nix run nixpkgs#gh -- pr checkout -f 337109
% nix-shell
$ nixfmt 'nixos/modules/services/torrent/qbittorrent-nox.nix'
$ git diff
diff --git a/nixos/modules/services/torrent/qbittorrent-nox.nix b/nixos/modules/services/torrent/qbittorrent-nox.nix
index 55bd6805d4b7..cf2cfe5bdbbd 100644
--- a/nixos/modules/services/torrent/qbittorrent-nox.nix
+++ b/nixos/modules/services/torrent/qbittorrent-nox.nix
@@ -1,6 +1,13 @@
-{ config, lib, pkgs, ... }:
-let cfg = config.services.qbittorrent-nox;
-in {
+{
+  config,
+  lib,
+  pkgs,
+  ...
+}:
+let
+  cfg = config.services.qbittorrent-nox;
+in
+{
   # meta.doc = "./qbittorrent-nox.md"; # TODO remind me, since I rather write it later, than write now and rewrite later.

   options.services.qbittorrent-nox = {
@@ -9,15 +16,13 @@ in {
     dataDir = lib.mkOption {
       type = lib.types.path;
:...skipping...

[Redhawk18]

nixfmt 'nixos/modules/services/torrent/qbittorrent-nox.nix'

Ah I ran nix-shell -p nixfmt I didn't know there was a specific version of nixfmt just for nixpkgs.

[Redhawk18]

Are the meta.docs what are shown on the nixos wiki?

[infinisil]

Improving the message to hopefully prevent such confusion with https://github.com/NixOS/nixpkgs/pull/337577 :smile:

[Redhawk18]

Is the uid and gid okay? I wasn't sure if there's a way to generate it dynamically.

See https://github.com/NixOS/rfcs/blob/62d1245ec1eae275edb996229585637035c02538/rfcs/0052-dynamic-ids.md#using-ids-dynamically-allocated-by-nixos

But I installed this to test it on my desktop and I was never prompted once to agree to it.

You need to run your module on a fresh machine. A NixOS test would actually be great way to make sure it works.

even if we added an assert the eula would still show up. It's not like with minecraft where you could write true to a file and have it be accepted I believe.

Why not? Adding this to the qBittorrent.conf works just fine:

[LegalNotice]
Accepted=true

It's exactly what the module I linked does.

@NixOS/nix-formatting The error log told me to ping you with errors.

The error log says this:

Please go to the Nixpkgs root directory, run nix-shell, then: nixfmt 'nixos/modules/services/torrent/qbittorrent-nox.nix'

Hey I did some testing on this idea and it's insanely buggy and hard to get working correctly. I'm likely going to remove the ability to add settings. There's also no docs explaining the options, so it leads me to believe that the qbittorrent team views this as an internal config file. Which means they will change how data is structured without warning which would mean all settings set by us would no longer work and there's no declarative way to really stop this.

[misuzu]

Are the meta.docs what are shown on the nixos wiki?

They are shown here https://nixos.org/manual/nixos/unstable/

[Redhawk18]

I think this pr is different enough, that other pr seems very focused on all the details of a declarative system and is inactive, this is just the ground work for this service. We can always submit a new pr with those options later.

[misuzu]

Right, you should also add a release notes entry for your module.

[Redhawk18]

I hope thats it!

[Redhawk18]

Reviewed points

* [x]  module path fits the guidelines

* [ ]  module tests succeed on ARCHITECTURE (no NixOS test)

* [x]  options have appropriate types

* [x]  options have default

* [ ]  options have example (no need here imo)

* [x]  options have descriptions

* [x]  no unneeded package is added to `environment.systemPackages`

* [x]  `meta.maintainers` is set

* [x]  module documentation is declared in `meta.doc`

* [x]  a release notes entry for a module is present

Possible improvements

I'm not sure the dataDir option is actually useful. On my qBittorrent instance, which has around 500 active torrents, the profile directory takes only 42M of disk space. What actually would be useful is an option to change the default downloads directory.

Also a NixOS test would be great, but it's optional.

Comments

Looks good enough to me!

What kind of test would you want sense this a a ui? Also I agree that datadir is a bad option, but currently I'm not a nix god with merging options into config files and theres little to no docs for those options and how qbittorrent internally organizes/names them. So I wanted to skip that rabbit hole for this pr at least. It seems #287923 is working on that so maybe they can rebase ontop of this pr.

[misuzu]

What kind of test would you want sense this a a ui?

Something like this: https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/bittorrent.nix

[Redhawk18]

What kind of test would you want sense this a a ui?

Something like this: https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/bittorrent.nix

I don't think I'm advanced enough to write a test like that yet.

[misuzu]

I don't think I'm advanced enough to write a test like that yet.

Like I said, the test is optional 😁

[Redhawk18]

I don't think I'm advanced enough to write a test like that yet.

Like I said, the test is optional 😁

Thanks, I believe this is ready for review now.

[bbigras]

I got this error:

       error: The option `systemd.services.qbittorrent-nox.Wants' does not exist. Definition values:
       - In `/nix/store/k1z1bn2qrkdj7yvd1s5b7r93h1ygnbxc-source/nixos/modules/services/torrent/qbittorrent-nox.nix':
           [
             "network-online.target"
           ]

is Wants supposed to be lowercase?

[Redhawk18]

I got this error:

       error: The option `systemd.services.qbittorrent-nox.Wants' does not exist. Definition values:
       - In `/nix/store/k1z1bn2qrkdj7yvd1s5b7r93h1ygnbxc-source/nixos/modules/services/torrent/qbittorrent-nox.nix':
           [
             "network-online.target"
           ]

is Wants supposed to be lowercase?

All systemd serviceconfigs should be uppercase, but all others are lowercase

I got this error:

       error: The option `systemd.services.qbittorrent-nox.Wants' does not exist. Definition values:
       - In `/nix/store/k1z1bn2qrkdj7yvd1s5b7r93h1ygnbxc-source/nixos/modules/services/torrent/qbittorrent-nox.nix':
           [
             "network-online.target"
           ]

is Wants supposed to be lowercase?

Yeah systemd and nix mix upper and lower cases for extra fun

[Redhawk18]

Please rereview @bbigras

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/4514

[Redhawk18]

@SuperSandro2000

[Redhawk18]

The port option is going to be ambiguous when someone adds an option for --torrenting-port.

In general, I think #287923 is in a slightly better shape (with declarative config mostly done, and hardening options on the service). I would rather we merge the parts that are already working from that PR rather than deviate more.

I highly disagree, and blocking my pr seems incredibility hostile to me. Like I already said in the other pr @fsnkty admitted they are unable to work on this and the pr is incomplete. I rather have a working service that doesn't have all the options it possibility could than a pr that has been open for years that is almost perfect. My pr is finished and already has 4 reviews, blocking it at this point seems like whoever was first gets to merge their pr. Not whoever has the better and more active pr with more approval.

[ambroisie]

@Redhawk18 I'm not blocking your PR, I'm pointing out that both PRs are to add essentially the same module. I did see that @fsnkty is not currently able to work on their PR, and I do appreciate you wanting to get something merged in.

But from comparing both PRs, I think yours is diverging from the first one for no reason that I can see, and I would rather we just have the same basis as I think the first PR had more reviews done to it and resolved some of those same pain points.

I said:

I would rather we merge the parts that are already working from that PR

This might have been phrased badly, but I hope my answer cleared up than I'm not blocking this PR. I do want to see it be merged, but I voiced what I think is wrong in my review (namely, the naming of the module, the port option, and the hardening).

[Redhawk18]

This might have been phrased badly, but I hope my answer cleared up than I'm not blocking this PR. I do want to see it be merged, but I voiced what I think is wrong in my review (namely, the naming of the module, the port option, and the hardening).

Fine, I renamed the port and added the hardening, but I'm not renaming the module. If you can prove qbittorrent can run over xorg which is on life support first off and work on wayland I will rename the module. The name is correct how it is now because it only uses nox, so it should be named after it.

@ambroisie

[fsnkty]

blocking my pr seems incredibility hostile to me.

nothings personal or being blocked.

Like I already said in the other pr @fsnkty admitted they are unable to work on this and the pr is incomplete.

it needs testing and some minor fixup, it's been in use by multiple people for months now including myself, qbit is a bizarre program and I simply didn't want to have the module included just for it to break in bizarre ways due to some qbit feature I was unaware of. it's simply nicer to get stuff right in the first PR instead of messing about with stages of them and messing about with the config/settings of what people already have done. I mean if you take a look at the Todo list it's testing, making tests and an option type left. I hope you understand it well, though if you do id really appreciate your insight into how qbit consumes new config ^^`

I rather have a working service that doesn't have all the options it possibility could than a pr that has been open for years that is almost perfect.

take what works 100% then, no sense in nearly duplicating work and rerequesting reviews, it also creates more work for anyone who now has to work around and refine the module you've made, or argue for a change in it's config

My pr is finished and already has 4 reviews, blocking it at this point seems like whoever was first gets to merge their pr. Not whoever has the better and more active pr with more approval.

not being blocked, just please be cautious of the extra work this causes for yourself and for anyone else who wants to work on the module as it'd now make extra changes. that PR of mine has some discussion on how to handle things like port config/setting and has afaict decent structure and good practices on stuff like service hardening, please take that into consideration at the very least, let's avoid creating extra work 😞

[Redhawk18]

blocking my pr seems incredibility hostile to me.

nothings personal or being blocked.

I still believe blocking my pr over a minor name change that implies this service works with both the ui and headless version of qbittorrent is a bad idea.

take what works 100% then, no sense in nearly duplicating work and rerequesting reviews, it also creates more work for anyone who now has to work around and refine the module you've made, or argue for a change in it's config

I had no idea until the pr had progressed far enough to be reviewed. It seems my pr has made your pr more active as well by the threat of being closer to merging. And I understand you don't want to edit my code, and rebase your pr which I understand. I'd also rather write everything myself aswell. I made this pr because I wanted this service and you were inactive, I understand you can't choose when you have time in your life for free unpaid volunteer work that gets no credit anywhere. But I believe I have been incredibility reasonable with all suggestions, and everything was progressing smoothly until now.

The reason a block offenses me so much is the amount of work I've put into this for someone on the internet to just click a reject button with a bad suggestion, and prevent this from being merged is insane to me.