nixos-24.11 no ordering with samba module settings

kkt4 commented 4 days ago

Describe the bug

It seems https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/network-filesystems/samba.nix was changed after 24.05 to respect https://github.com/NixOS/rfcs/blob/master/rfcs/0042-config-option.md. But some smb.conf options have a dependency of order (include=...). This option (as per doc) "[...] allows you to include one config file inside another. The file is included literally, as though typed in place." There doesn't seem to be a way to order parameters anymore. Looks like parameters are just ordered lexically now.

Steps To Reproduce

services.samba = {
  enable = true;
  settings = {
    global = {
      security = "user";
      include = "/etc/smb2.conf";
    };
  };
};

gives

[global]
include = /etc/smb2.conf
security = "user";

Expected behavior

[global]
security = "user";
include=/etc/smb2.conf

Notify maintainers

@anthonyroussel @bachp

Note for maintainers: Please tag this issue in your PR.

Add a :+1: reaction to issues you find important.

bachp commented 2 days ago

We already had an issue with the global section needing to be first. It was solved with differen config snippets.

It seems there are other issues in the community around the same problem that attrsets are not ordered:

https://github.com/nix-community/home-manager/issues/2519

Do you have a proposal how to fix this?

kkt4 commented 1 day ago

I'm not familiar enough with Nix to tell. The DAG-based solution from that thread looks like a good idea, but then I don't think you would get merge rules if you do go that way (e.g. services.samba.settings.global."invalid users" merging from multiple modules). I think for now a good solution would be to just add an option to allow smb.conf override for more complicated use cases. Or maybe rfc-042 shouldn't be used for packages that care about ordering like samba.

The only real change I would want to see is to use environment.etc."samba/smb.conf".source instead of configFile so it's overrideable. Right now if you override smb.conf the restartTriggers don't get updated.

EDIT: I see it like this, the issue currently is the mapping function settings -> text, the settings idea itself is good. Since we need a order-preserving mapping function for more complex setups (which is not possible without changing the settings format or adding out-of-band information), letting the end-user do the mapping himself is the easiest solution until we revisit that RFC or drop it.

NixOS / nixpkgs