nixos: remove environment.noXlibs #341717
Conversation
mjm
force-pushed
the
push-tzktyytxolwl
branch
from
f916966
to
384cb17
Compare
|
Probably want to remove @SuperSandro2000 from Line 340 in 602d8ec And I guess we should get his approval since he went through the trouble to take ownership of it. :) |
This option is causing quite a bit of avoidable problems, and rarely what people actually want. @mjm provided solid reasoning as to why it would possibly be better to remove. I'm personally not sure it's a good idea to just not answer with solid reasoning on why it should stay. If that would become standard then within no time nixpkgs would become a big mess and minefield with personal preferences. |
This gave me a very good idea someone else should already had have: we are not tied to that name and we can just change it but instead of using mkRenamedOption we use mkRemovedOption and tell people about the implications and if they still want to use it and then they can make a decision for themselves. I am going to come to with a new name and purpose a PR with that idea.
The module code itself could very easily live out of tree but that's not even half of the story. It needs to match the pkgs being used and many PRs targeting noXlibs acompany a change in a package which must be merged in the same PR. Doing that out of tree will make this a big chore and probably so hard to maintain that it wouldn't exist for long. Also would that solve the problem? I personally think that foremost less people would use the module because it is more inconvenient to use and would probably be more often broken because of the harder maintenance.
That would just duplicate effort, time and not utilize the best part about NixOS: sharing configs
There are is another module in NixOS that following this would also need to be removed: the hardened profile Just enabling it has a good chance to just break the software you are using. Also the hardened kernel it uses is known to be incompatible with many kernel modules especially out of tree. Also we support many platforms where a wast majority of packages are broken like wasi or freebsd. I don't want to have precedence of removing things people want to keep and are actively maintaining.
That's not an argument. If we go by this we would need to remove features like immutable users, dbus-broker, the new /etc mount or long running experiments like systemd stage 1 initrd, too. Those modules expose options which can break your system and break things in subtle ways especially if people mindlessly enable them without beforehand reading even the options description. How should we help those users?
They can also just copy overlays which can cause the same and with your above suggestion to copy them into people personal configs and sharing them, I am afraid to tell you but I imagine this then happening more often then less.
Then we need to match up the documentation around it to tell people what it does and what doesn't. But this reminded me of two modules which should really be removed because they are broken and don't work as expected: I am going to create PRs for that if I don't forget. Also to name another example of an option that causes quite a lot of issues lately: autologin in SDDM and none working things like kwallet. |
This PR came after a very long discussion on Matrix involving Sandro where the problems of the module and the burden it causes on users, people giving support, and maintainers for the benefit of few were discussed, unfortunately to no productive end :(
A different name and lots of warnings in the description may help, but I don’t think they will get to the root of the problem.
Since those PRs all need review and package maintainers could have issues with or even block them, it seems like to achieve the desired end of the module it is already necessary for you to have local patches to the module and packages while waiting on a merge. (The alternative – that package maintainers are required to accept fixes and flags for Having to overlay, patch, or include things in your Nixpkgs fork locally while waiting on PR review is a normal part of advanced NixOS usage, especially if you don’t have commit access. This argument only really works if you are self‐merging these PRs without review. I don’t think being out‐of‐tree would have a significant effect on this; everyone’s configuration depends on Nixpkgs changes to some extent.
Fewer people using it is part of the goal here: most people who think they want
Not everyone’s modules and configurations are in NixOS. That’s unsustainable. Things have to pass a threshold of non‐shooting‐self‐in‐foot usefulness to be in the tree, and those have to be weighed against the burden they place on the rest of the project in terms of maintenance and support. I realize it’s convenient for you to have it in‐tree because you’re a committer, but you can surely understand that this isn’t an adequate rebuttal to arguments about how niche the thing it actually does in practice is, and the burdens that having it in the tree places on others.
As one of the listed maintainers of the hardened profile, I think it would be reasonable to remove it in its current state. I have long intended to work on improving it, but it’s currently in a bad state for both compatibility and truly enhancing security. If I was actively participating in providing support for its users that would be one thing, but if it’s causing burden on those providing support, or maintainers of packages and modules, then let’s drop it, at least until I can give it the attention it deserves and ensure that support queries are being triaged effectively. That said, I think that it’s much more obvious that hardening your system for security will break some things than the idea that disabling graphical support would cause a trillion rebuilds, some of which inevitably fail in confusing ways.
Maintainership is not ownership and having a single person taking care of a niche module be a veto over a long list of enumerated harms would be toxic to the project’s health. While you might be taking care of the code side of things, nobody is really taking adequate responsibility for the support burden. #319102 and #319122 remained open for months without resolution. Anyway, I don’t think the fact that people can break their systems with this option alone is reason to remove it. I think the fact that people can break their systems, that it expresses a set of preferences that most potential users find they don’t actually have when they run into what those preferences actually are, that the number of people who actually want what it expresses on reflection seems to be small (in the discussion we’ve so far been able to find maybe three enthusiastic users including you?), and that it imposes a burden on maintainers combined mean that it is causing more harm than good, as expressed at length by everyone who joined the discussion other than Sandro. Honestly, though, bigger than my concerns about this specific module is that we, as a project, are simply unable to deal with situations where one person who benefits can block addressing things that are a burden on the rest of the project, in general. I don’t think that’s healthy or will lead to sustainable maintenance for the project. Oh well… |
Which was started by a ping and afterwards immediately mentioning that a module I maintain and use should be completely removed and then downwards lecturing on me how bad, broken and terrible it is without leaving the idea open to letting me improve any aspect of it. If we want to have a productive discussion, there must be room to improve things. If the discussion is started with a made up mind and outcome, there won't be sparks of new ideas. I've talked to a handful of people since then and got several good ideas how to improve things and make the module fit better to what it actually does.
Which was in #260570 After taking a quick look at the module it is immediately clear that the module was in no good shape. There where lots of open TODOs, it didn't have a proper commit in almost 10 years and you cannot just replace service a (like openssh) with service b (lsh). I think it is unfair to compare noXlibs to gnu based on that.
So as a package maintainer I can also block required patches for musl or darwin, I am hearing about that the first time, or where is this going? Those options set by the noXlibs module are usually options which exist upstream and just get properly exposed in the nix package. No custom hacks. Or do you want to say musl and darwin are also burden for maintainers? I don't think that is what you are trying to say and I must probably misunderstand things.
This is just an assumption with no proof. I in fact have currently zero local overlays regarding noXlibs and I encourage everyone that still has some, to upstream them. Since I did the last change to noXlibs I started using a few new services without any problems. Also the other nixos vms I maintain and support didn't have a regression since the 23.11 upgrade and we added several more since then.
Same, I manage two https://github.com/superSandro2000/nixpkgs https://github.com/NuschtOS/nuschtpkgs/tree/backports-24.05
I don't think that's true. Just today I got to know that a friend was using the overlay on purpose and I haven't lobbied for it and always tell people about the drawbacks which soon is also properly reflected in the description. I admit that the usecase is a bit niche but I am confident that there are more users out there than you think there are but they are just silent.
Well, we can hack around it. We can change the option name and then make people update their configs by hand without using mkRenamedOptionModule. We also have assertions and warnings and I just got the idea in a productive discussion with a friend that we should add assertions to the module when eg. a graphical system is used. You'll find that in code in #341909
I know and things that don't fit that are also not upstream by me but usually land in https://github.com/NuschtOS/nixos-modules
I already wanted to remove noXlibs from the minimal profile in December 2022 #205318 but when I nudged people on matrix about it, they didn't support the idea and then I forgot about it. But we have now taken care of that and it is no longer set. Also to clarify some things: I didn't start the module. It was initiated by 7c6f434c 15 years ago and gained overlays in a3e6df6 to remove graphical dependencies when using NetworkManager on a server. Some 2 years ago I started to use it and contributed fixes for new services and changes in nixpkgs to it. I only added myself as codeowner in dd1e806 to get pinged about new PRs since ofborg doesn't work for modules. |
There was a problem hiding this comment.
I'd like to review this PR in its inverse. If this module didn't exist, and @SuperSandro2000 were to open a PR adding it, how would it be reviewed, and would it be accepted?
I think it would be rejected. It is clear that this is a personally curated set of changes that suit a particular set of needs, without being particularly comprehensive and without care for the effects it has on the rest of the package set. It would make no sense to add it as is.
Therefore it makes most sense to remove it.
|
I agree with @ElvishJerricco and @K900 here, but I want to respond to a couple specific points:
I agree that the
Of course Darwin and musl support are burdens on package maintainers; frequently quite significant ones. The need to balance the burdens of platform support with the benefits they bring is exactly why we had an RFC to formalize platform support in explicit tiers that delineate what is expected of maintainers for each platform. It explicitly lays out that for less‐supported platforms, fixes to packages for them can sometimes be validly rejected. So, yes, as a project we’ve decided that Darwin should block the channels. That’s been a very controversial decision in the past, for understandable reasons, and has caused calls from maintainers to reduce the support due to the burden it places on them. Thankfully I think the maintenance situation is improving there and Randy’s upcoming SDK rework will further reduce the burden Darwin places on maintainers, but it’s clearly part of the calculus. We make this trade‐off, not because Darwin doesn’t pose a maintenance and support burden to the project, but because we have a huge number of users on macOS who benefit from the support, and the project also benefits from that support, as it brings in contributors and acts as a new user funnel for NixOS. Similarly, musl support benefits users in large part because So, yeah, those things are burdens. But they’re burdens we take on as a project because they bring large benefits, and that we’ve achieved some level of consensus for. Using @ElvishJerricco’s rubric, I think that if we didn’t have Darwin or musl support and they were proposed for Nixpkgs, there would be strong interest in them and they would have a good shot of making it in. There would naturally be concerns about the burden on the project, and that might influence the expected level of support. But the response I expect I don’t think the fact that
|
mjm
force-pushed
the
push-tzktyytxolwl
branch
from
384cb17
to
768db40
Compare
|
I've updated to remove the CODEOWNERS, so I think this is in mergable state now. I appreciate everyone's input on this. It seems we've reached something of a consensus here, with several committers agreeing that this should be removed. I don't think it makes sense for anyone to have an absolute veto on a shared commons like Nixpkgs, so @SuperSandro2000 would you be willing to remove your blocking review here so this can be merged? |
|
I actively use this parameter in mine. Should I keep it or rename it? |
|
Once this is merged, you can copy the overlays that this option used to apply into your own config. Though I would expect this option to cause you more rebuilds than it would save, since each override is forcing a cache miss. |
|
On my system, the system is often rebuilding and doesn't always use the cache, so this setting doesn't affect that much. |
Description of changes
This removes the
environment.noXlibsoption, a common footgun for new/inexperienced users of NixOS who are tempted by the promise of a lighter, more minimal system. Instead, they end up with a system that can surprise them with unexpected rebuilds or even broken builds for reasons that aren't immediately obvious.This creates a frustrating experience for these users, but it also wastes the time of our volunteers and contributors as they try to determine why the user is experiencing a breakage that others aren't. It's always a frustrating outcome to eventually find it's because of the user enabling this option, either indirectly without knowing (such as a recent case of using an old version of microvm that enabled it) or directly, thinking it was well-supported and couldn't possibly be the cause of their issues.
The option name is a poor description of what it does: it can't completely remove X11 libraries from the system, since many packages include them without flags to disable them (and even if they did, it would be a game of whack-a-mole trying to ensure the overlay included all of them). And it goes beyond removing X11 in some cases, disabling other graphics-related functionality like Wayland, Vulkan, and CUPS. This is a nasty surprise for users who enable this thinking it will help them achieve a pure Wayland desktop. And users who really want to get rid of X11 end up frustrated that a package not included in this overlay still pulls it in. Both of these types of users would probably be better served by more precise overlays targeting the packages they care about.
The functionality this module provides could just as easily be provided out-of-tree, either in its own repo/flake or just copied into individual system configs. Having it in the tree makes users think it's more supported than it is and makes debugging its breakage become the responsibility of NixOS contributors. That's true of anything in nixpkgs, but because of that, we weigh the benefits of the options we provide against those costs. I firmly feel that
environment.noXlibsbeing in nixpkgs does not give benefits to justify its cost.Some prior issues where this has come up (though I've personally witnessed many more on Matrix that never made it to GitHub issues):
environment.noXlibscan't be built withnixos-rebuild build-vm#265675The one thing gained by having it in the tree is that someone looking to remove a package flag it overrides will likely do a search of the codebase and find that it is used by this. But that goal can just as easily be accomplished with a comment in the relevant packages, and doing so will avoid all the other problems this option introduces by its existence.
In terms of considering alternatives, I don't think this is a problem that can be solved by documentation. It's too easy to skip over and not read it. Users will still find the option in other existing configs and cargo-cult it without realising what it actually does. The most practical option to ease the burden imposed by it is to remove it.
cc @SuperSandro2000 @ElvishJerricco @emilazy @K900
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.