treewide/nixos: remove with lib; part 12

[Stunkymonkey]

Description of changes

part of #208242

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.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
  • (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.

[philiptaron]

Everything looks good. I'm planning on merging after running a few nixosTests.

[vcunat]

Again, breaking things.

       error: undefined variable 'optional'
       at /nix/store/1jvzqyay4rb8bqyv0jbdwk9wb0frrhfs-source/nixos/modules/services/x11/desktop-managers/xfce.nix:130:10:
          129|     ] # TODO: NetworkManager doesn't belong here
          130|       ++ optional config.networking.networkmanager.enable networkmanagerapplet
             |          ^

[philiptaron]

Damn it! Ugh. I'm not merging any more of these because I can't verify correctness if I can't trust the tool to mutate the right areas. I checked on the commit that introduced the error (4b798ee). It's within a "with pkgs" context and I suspect that's what caused the issue, but I can't tell.

[K900]

I'm going to ask the same question I've asked before:

How many of these do we need to explode to realize that we need better testing?

[Ma27]

Now that I'm thinking again about this: can somebody explain to me why we're doing it like this in the first place?

Like, new usages of the same pattern will come up so I think the time people "invested" on that would be way better served if tooling was developed that does this reliably and can be used by contributors easily on their new changes (and by maintainers on their code).

Fixing behind everyone's back is something that we'll never be done with given that new usages will arise every now and then (and yes, I consider it a horrible contribution experience if we'd actually block PRs by asking people to... idk... remove withs or add spaces between {} or what not).

[vcunat]

There were more of them, e.g. certmgr. A suspect list:
https://hydra.nixos.org/eval/1810800#tabs-removed

[philiptaron]

I'm clicking the revert button. I regret merging.

[philiptaron]

Reverted in #369795

[vcunat]

Oh, that hydra diff contains multiple of these "remove with lib;" pull requests, e.g. nixosTests.tor was broken by another one.

[vcunat]

Actually, all of the stuff detected by trunk-combined job removals was from other PRs or fixed in the meantime 🤦🏽 as we can see from the eval after merging the revert.

[philiptaron]

Actually, all of the stuff detected by trunk-combined job removals was from other PRs or fixed in the meantime 🤦🏽 as we can see from the eval after merging the revert.

Sigh. I am trying to learn the "mass-evaluate-all-NixOS-tests" strategy that Mic92 alluded to in the previous PR. It took me a few tries to understand the output and how to discern something was already broken vs. newly broken, but I think I got it dialed in.

I've fixed everything that I found in this PR:

• nixos: fix various modules after with lib removal #370042

There may be more latent issues for modules without a NixOS test, or in the same modules without a NixOS test covering them.

[vcunat]

Laziness can bite us here?

[vcunat]

I suppose I'll make the nixos-unstable channel avoid these in-between commits where several services had evaluation issues.