I know this is not directly related to this PR but it also highlights the issue: Hard-coding localhost is not a good idea, because sometimes the server does not listen on localhost at all but only on the primary interface (when using -rxbind server options). This case is currently not handled by the systemd scripts. In this PR the problem gets a new quality, because it is not so easy anymore to fix by mkForce'ing the correct ExecStart and ExecStop command lines.

The logic should be as follows:

• if advertisedAddresses != [] use the first advertised address
• otherwise use localhost

This makes it an error to use -rxbind without also specifying advertisedAddresses, which I find okay. This also makes it an error to use a whole subnet for the first advertised address (which is permitted by the netInfo specification).

The user interface could be made safer by abstracting -rxbind into its own option, which can then enforce the necessary behaviour. On the other hand, a piece of documentation may suffice, because -rxbind needs a lot of administrator knowledge already to be useful. So, they are likely going to figure out the mechanics anyway by looking at the NixOS module (and the documentation).

Otherwise the PR looks fine. Thank you.

@spacefrogg sorry for taking so long to respond, I have been on a hiatus from NixOS contributions, but now I'm back.

I am sorry I cannot quite understand your comment, or get any suggestion on how to improve my PR from it. Did I do something wrong?
You talk about overriding ExecStart and ExecStop via mkForce, but this PR does not alter any of those fields, instead relying on preStart and ExecPostStart to carry out its job. Am I missing some source of incompatibility in so doing?

Could you link any relevant issues that could be related to your concern? I might find some time to spare and work on it.

If you do not find any reason to keep this PR from being merged, could you also approve it?

I would ask you to update your PR in the following way:

• Instead of hardcoding localhost, build the following logic:
  • if advertisedAddresses != [] use the first advertised address
  • otherwise use localhost

While I was at it, I implemented the same logic for ExecStop as well, so that we can save on another PR. Is it ok?

Since you removed the backport tag (understandable, although not having this functionality in release 24.11 for a 2-days deadline overrun is quite sad) should I further alter the release notes to target 25.05?

Moved the release notes to the 25.05 file, as it became highly unlikely that this change could land in 24.11 (and maybe that's for the better?).
Moving the release notes implied a rebase onto master, as the target release log file did not exist at the time this PR was created.

Yes, you did right. Although, I removed the backport label, because these kinds of improvements are not to be backported.

Sure, no offense taken, I understand my mistake, but please, if you modify something I did, let me at least know why you did it, however stupid my misstep might have been.

Anyway, thanks for sticking with me until this point! It is difficult to find people willing to review these niche systems and actually know what they're doing.

Resolved merge conflict on release notes, all other modifications left unchanged.

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/5030

Resolved merge conflict on release notes, all other modifications left unchanged.
Hopefully there will be no more conflicts on the release notes after this...