hyprwm / Hyprland

Hyprland is an independent, highly customizable, dynamic tiling Wayland compositor that doesn't sacrifice on its looks.
https://hyprland.org
BSD 3-Clause "New" or "Revised" License
21.63k stars 903 forks source link

remove home-manager module #4004

Closed andresilva closed 11 months ago

andresilva commented 11 months ago

Description

We should remove the home-manager module from this repo. The hyprland module that exists in home-manager is strictly better than the one here, and having two different modules leads to some confusion (see https://github.com/nix-community/home-manager/issues/4716 that wasn't the user's problem but I think the point still stands). The migration path is also easy since users of the module from this repo can just keep shoving everything into extraConfig.

cc @fufexan

fufexan commented 11 months ago

I agree. We can also probably remove the NixOS module as well, since the one in Nixpkgs already does the job. I'm thinking of removing the modules after NixOS 23.11 gets released.

Thoughts? @andresilva @NotAShelf @spikespaz @outfoxxed and other nixers.

jacekpoz commented 11 months ago

I agree, had some issues with the modules recently (mistook this hm one with the upstream one), having these removed would be great

NotAShelf commented 11 months ago

I'm in favor of keeping the upstream (here) modules. Both nixpkgs and home-manager move slow, and the upstream module serves (and should continue to serve) as a staging area before we push to downstream modules.

fufexan commented 11 months ago

Sure, that makes sense. But I think it would be better to only define new options in these modules, rather than have a copy of the upstream ones.

Jappie3 commented 11 months ago

The HM module is way nicer to use (in my opinion) & this one just causes confusion, but on the other hand I don't think the HM module can keep up with Hyprland's fast development... for example they still have the nvidiaPatches thing - that was removed several days ago :/

Vagahbond commented 11 months ago

I stand with Raf, Having those modules is great for using the latest version of Hyprland and testing specific commits when Vax asks for it.

Maybe make the separation more explicit between them, indicate the modules from the flake as staging for instance.

andresilva commented 11 months ago

I'm in favor of keeping the upstream (here) modules. Both nixpkgs and home-manager move slow, and the upstream module serves (and should continue to serve) as a staging area before we push to downstream modules.

This is true in theory, but at least for home-manager the opposite is true in practice. The upstream home-manager module now has more features than the one here, and I don't necessarily agree that it moves so slow, PRs get merged reasonably fast (usually in a day or two), and we get the added benefit of getting valuable input from rycee. I don't really have an opinion on the nixos module since I don't use it, and I use nixos-unstable so the story might be different for stable release users.

For backwards compatibility we could re-export the upstream home-manager module here with a deprecation warning (which should guarantee that no configs get broken).

andresilva commented 11 months ago

I stand with Raf, Having those modules is great for using the latest version of Hyprland and testing specific commits when Vax asks for it.

This can still be achieved by using the overlay from the flake we have here, this is what I'm doing. We just need to document this so that users know how to do it.

NotAShelf commented 11 months ago

This is true in theory, but at least for home-manager the opposite is true in practice. The upstream home-manager module now has more features than the one here ...

The presence of opposite practice sounds like something we should work on.

... PRs get merged reasonably fast (usually in a day or two), and we get the added benefit of getting valuable input from rycee.

You're right that it gets valuable input from rycee, but the pace at which PRs are merged is usually a mess. They get merged fast if and only if you are lucky.

I don't really have an opinion on the nixos module since I don't use it, and I use nixos-unstable so the story might be different for stable release users.

I explicitly disable the nixos module since my system flake already provides what it provides. I remain in favor of keeping both modules or/and striving for 100% feature parity between upstream and downstream.

This can still be achieved by using the overlay from the flake we have here, this is what I'm doing. We just need to document this so that users know how to do it.

Using the overlay is not a good example, explicitly because overlays suck.

andresilva commented 11 months ago

The presence of opposite practice sounds like something we should work on.

This is true, but I think it's still useful to look at the current state of things to inform our decisions. In this repo, there was a commit to the home-manger module 5 days ago (to remove the nvidia patches support), and before that the previous commit was in August (to remove the hidpi patches support). My point being that even though Hyprland is indeed a fast-moving project, the things we are discussing here aren't that fast-moving.

Using the overlay is not a good example, explicitly because overlays suck.

We can also instruct users to set the package directly in the home-manager module, the end-result would be the same as now.

Either way, I don't feel strongly enough about this to keep hammering on it, so if removing these modules is not consensual I am fine with keeping everything as-is (just making sure that the different modules are all in sync feature-wise).

ardishko commented 11 months ago

This is a pretty dumb issue, we have 2 choices and 2 of which are

OR

removing the module quite literally accomplishes nothing, deleting work for no reason other than the fact that it's already inside home manager which is fine but we need to keep in mind the home manager and nixos moves very slowly. There should be an option for those who want to use a more up to date version of the module and deleting it only deletes work.

Besides, It would be easier and quicker for the HM module inside this repo to be commited to home-manager's official modules if the hm module stayed here so other people can send a PR to home-manager to update it without relying on vaxry or anyone else with the source code.

fufexan commented 11 months ago

Besides, It would be easier and quicker for the HM module inside this repo to be commited to home-manager's official modules if the hm module stayed here so other people can send a PR to home-manager to update it without relying on vaxry or anyone else with the source code.

I'm not sure what you mean. I've read this twice and still don't get it.

fufexan commented 11 months ago

By the way, I made a PR which tries to address this issue, at #4012. Test it out and share your thoughts on it.

ardishko commented 11 months ago

I'm not sure what you mean. I've read this twice and still don't get it.

Since the module in this repo is more up to date, it would be better to keep the code here so if someone from the main hyprland team doesn't send a PR to home-manager in time when let's say an update happens or new features get added, someone else can send a PR since the code is already here in this repo. There's no harm in keeping it, regardless.

ardishko commented 11 months ago

I might be a little misinformed about the HM module in this repo being more up-to-date since apparently it doesn't have settings so correct me if I'm wrong.

fufexan commented 11 months ago

I might be a little misinformed about the HM module in this repo being more up-to-date since apparently it doesn't have settings so correct me if I'm wrong.

No, the module in this repo doesn't have the settings option, thus being out of date. The approach I took in the PR mentioned above allows us to keep the current modules minimal, only building on the upstream ones.

ardishko commented 11 months ago

No, the module in this repo doesn't have the settings option, thus being out of date. The approach I took in the PR mentioned above allows us to keep the current modules minimal, only building on the upstream ones.

Okay, my bad then. If this repo's hm module is outdated then yeah there's no reason to keep it since it could be confusing to newcomers.

Jappie3 commented 11 months ago

If this repo's hm module is outdated then yeah there's no reason to keep it since it could be confusing to newcomers.

But do we really want to wait for the HM repo to merge changes to the module (no offense)? Keeping it here will allow for quicker merging of MRs and more iterations... I think that keeping the module in the Hyprland repo is a good idea, but only if we downstream things like the settings option (I think everybody agrees that that's a nice thing to have).

fufexan commented 11 months ago

Please read the PR before adding more comments here.

fufexan commented 11 months ago

I've been talking with @spikespaz about the future state of the modules, and wanted to mention his own take on the modules, hyprland-flake.

I invite two volunteers to test it out. Let us know your opinions on it!

spikespaz commented 11 months ago

That flake I've been working on for a while now. It's big, but all of the modules are optional. It's doesn't have totally the same API as Home Manager's module, but I do plan on bringing my flake to parity.

It lets you specify multiple things in a more precise and piecewise manner, to integrate with the rest of your Nix configuration.

Check out some of the things I've done with it:

spikespaz commented 11 months ago

@Misterio77

Hey all! Came across this so thought I would drop my two cents:

I think the best approach would be to adopt @fufexan's upstreamed hm module.

  • Keep hm-module.nix exactly the same as HM's upstream module. Or remove it altogether.
    • If keeping it, only diverge if a change in hyprland absolutely needs it.
    • Fufexan's module uses RFC42 style settings, which means the module rarely (if ever) needs updating to match package changes.

@spikespaz's module is an awesome technical feat and should absolutely be an alternative for those who are interested in a more tightly integrated solution (we should link to it in the docs). I'll for sure considering trying it out in my own setup soon(tm).

In my opinion, however, it goes against the general (nixos and hm) trend of using RFC42 to make modules easier to maintain and closer to the actual config generated. RFC42 styled options are now commonplace and pretty much every single nixos/hm module uses it, including ones that manage software with configs MUCH more complex than hyprland's. In my experience, the single usecase currently awkward to handle with wayland.windowManager.hyprland.settings is submaps; but I'd argue this should be fixed within hyprland itself (e.g. make it a block like plugins have).

Hyprland's config format is weird and inconsistent, so I think fully modeling it (and keeping up with changes) is a super hard endeavour we can't assume will always be possible, so we should the simpler solution of using settings (which already play super nicely with module merging, and are super easy to learn) and extraConfig as a escape hatch (e.g. for submaps).

But I have some comments about your primary complaint: non-compliance.

There are two very good reasons for this:

First, every additional module is optional. The config option (should be renamed to settings) is compatible (in theory) with existing configurations using the freeform attributes. The config format (serializer) is also pretty solid and hasn't broken since I wrote it, which is part of the reason I never fulfilled that promise to re-write it. It has been stable for a while.

Second, generating the config with ad-hoc functions (like the HM module, which was derived from a code snippet from a user in another issue some time ago) breaks down quickly once you make your config slightly more advanced (you already mentioned submaps). And there are other issues that I can't remember, but the second one that I can remember is animations and bezier curves. The lines are sorted alphabetically due to Nix implementation details. A for Animation comes before B for Bezier, and Hyprland expects the Bezier curves to be defined before animations. Fufexan's module won't work for that, because the config lines need to be sorted manually before writing to a file.

But, for the maintainability part, you're totally right, though maybe not so resolutely as it would seem at first glance. I did mention that the config generator has been stable for almost a year, maybe even more. I haven't had to fix or update it a single time. The only changes I've made have been purely elective and academic. Noteworthy additions have been made, but there have been no fixes. I don't think the generator is something we have to worry about with regards to maintenance, as long as I'm alive. Now, the extra modules on the other hand...

They're still relatively straightforward? Except for... Alright maybe not. But they're provided on an "as-is basis" without warranty of any kind. And I don't expect them to break anyway. If they do, the config file is formatted pretty, so you can copy it, replace the link from the Nix store with a regular file, and carry on until a fix is implemented.

Unfortunately for me and my project, it is big and complicated, more than I would like, but it makes sense considering it's a collection of solutions to various personal problems.

So with that in mind, the nature of the beast, what would it take to encourage people and make them less afraid of the large flake? Can I make the codebase less unwieldy? I know we need documentation for the special modules, I'm looking into it.

As for differences between the option APIs, I intend to bring my API up to parity with the HM one.