dwm: restored config patch interface

[neonfuz]

Motivation for this change

Add back the config patch interface for dwm. This feature was needlessly removed in a cleanup, and is now a breaking change between 20.09 and 21.05. Adding this interface back minimizes the changes people need to make when upgrading to 21.05.

Please try to backport this to 21.05 before release if possible.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Added a release notes entry if the change is major or breaking
• Fits CONTRIBUTING.md.

[neonfuz]

@viric for approval, soon please before 21.05 release.

[06kellyjac]

If you read the thread on the PR you'll see that it's not actually an option declared and documented in the dwm module, which is the reason for the removal.

https://search.nixos.org/options?channel=20.09&from=0&size=50&sort=relevance&query=dwm

https://github.com/NixOS/nixpkgs/blob/nixos-20.09/nixos/modules/services/x11/window-managers/dwm.nix

You'll also see the mention that we can bring it back if it's an issue for users.
I was expecting at least one NixOS dwm user to raise an issue asking for it back long before the 21.05 releasw.

Most people, and the docs that mention patching dwm, seem to modify it using an overlay with override attributes.

https://nixos.wiki/wiki/St#Using_DWM
https://www.reddit.com/r/NixOS/comments/egs3k6/how_to_configure_dwm_in_nixos/

If you want this option back it'd be best if you add the patches option explicitly to the module but since the release is soon please at least add a comment to avoid it being removed again in the future.
Also it'd be great if you could add yourself as the maintainer of the package & module to assist viric

---

If you want it included in the 21.05 release you'll want to mention it in this issue: #121972

[neonfuz]

Its not defined in the dwm module because it alters the dwm package not the dwm module. Other packages use this pattern too, see retroarch.

[neonfuz]

You'll also see the mention that we can bring it back if it's an issue for users.
I was expecting at least one NixOS dwm user to raise an issue asking for it back long before the 21.05 release.

Well, I am a nixos dwm user, I just happen to use stable. I'm testing 21.05 a few days before release to see if things go smoothly, this is one extra change I had to make so I thought I'd try to PR and bring the feature back before others upgrading from stable run into the issue.

[jonringer]

I don't really see any harm in this.

Please just add a comment as to why the change should stick around, so people aren't likely to clean it up later.

[06kellyjac]

Its not defined in the dwm module because it alters the dwm package not the dwm module.

There's no reason the dwm module couldn't override it's dwm with the patches defined in an option in the module.
Similar to how home-manager wraps plugins into neovim/firefox for you once set using their plugins option

Or it could expose the package to be overriden like many home-manager modules such as dwm-status, rofi, and polybar etc

Other packages use this pattern too, see retroarch.

There's only around 33 packages using this pattern to configure themselves and only around 3 of those have been added within the last year.

The more standard nix method is to use overrides (and where necessary overlays)

---

Either way thanks for adding the comment and for adding yourself as a maintainer 🙂👍

[neonfuz]

OK I added myself as a maintainer, and added a comment to try and make clear what the line does so it doesn't get removed in the future. I think I'll update the wiki with usage too, the current code examples have typos anyways.

There's no reason the dwm module couldn't override it's dwm with the patches defined in an option in the module.

Yes but that's a new feature, here I'm just trying to remove an unnecessary feature regression. And that feature would not be useful to people wanting to use dwm on a non nixos system. I used to use debian with nix, with dwm from nixpkgs.

In general I feel like this happened because of confusion around the "nixpkgs config set" and how it should be used. Is there good documentation on this somewhere?

[06kellyjac]

And that feature would not be useful to people wanting to use dwm on a non nixos system. I used to use debian with nix, with dwm from nixpkgs.

This was just a potential nixos config option available in addition

Another reason I recommended the use of the ubiquitous overrides and overlays
It works with any nix setup and is a very useful thing to be aware of when dealing with any nix.
Getting users used to it as a pattern for enabling, disabling, and later even fixing things for themselves is important.

In general I feel like this happened because of confusion around the "nixpkgs config set" and how it should be used. Is there good documentation on this somewhere?

In this specific case there wasn't really any confusion, it was brought up as a breaking issue in that thread.

I had a really quick look for some docs but I couldn't find anything.
Using the nixpkgs config set is highly uncommon and the strongest use is for cross program shared settings e.g. pulseaudio support and I believe some cuda stuff with the tensorflow related programs.

The go-to configuration is with overrides and overlays because it's a common nix interface and doesn't require extra argument passthrough in all-packages.nix

polybar has its arguments to enable features just like dwm has an argument for patches.
By default polybar has i3 support disabled but many users will override the package to turn it on (or just use polybarFull 🤷), no nixpkgs config set passthrough required

If you'd like to add some docs I'm sure it'd be welcomed but IDK if it's something that should be recommended people utilize from here-on (again unless its some kind of shared config across programs)

[neonfuz]

This was just a potential nixos config option available in addition

I agree it can be a good addition, but now is not the time, maybe in another PR for unstable / the next release.

Getting users used to it as a pattern for enabling, disabling, and later even fixing things for themselves is important.

I agree, so maybe the documentation should still recommend overlays, but when the feature was removed it should have been stubbed out and throw a build time error (like you did for dwm-git). Instead the build succeeded, but upon boot I was in a desktop with default dwm configuration. If I didn't have dmenu or xterm installed I would be stuck. Maybe encouraging the other pattern is good but unless this is fixed, other nixos-stable dwm users will end up on a broken system upon upgrade.

[06kellyjac]

I agree, so maybe the documentation should still recommend overlays, but when the feature was removed it should have been stubbed out and throw a build time error (like you did for dwm-git). Instead the build succeeded, but upon boot I was in a desktop with default dwm configuration. If I didn't have dmenu or xterm installed I would be stuck.

Yeah, getting a successful build but failing once you boot really sucks. Although in theory it is easy to get back to the last working generation 😅

Next time we remove this kind of setup then yes a throw would be best. I haven't looked into if it's been done in cases like this yet.

[jonringer]

At the very least, there should be some deprecation story, but this will probably help with some dwm users coming from 20.09. So I think it should be added.

[jonringer]

do you mind squashing the first two commits, I think adding the config option back and the comment are "one logical unit"

Some info, if not familiar:

Please follow CONTRIBUTING.md and manual#submitting-changes-making-patches and squash the fix-up commits.

git rebase -i is a powerful command which achieves this, I created a small video demonstrating it's use here. A more indepth text tutorial can be found here

[neonfuz]

My bad, I just committed as I added stuff, squished.

[github-actions]

Successfully created backport PR #126139 for release-21.05.