Hello @primeos, thank you for your extensive review ! I've re-read what you said on #67704 and with the information you given I've changed the way I implemented it :

• We now have a wrapper separated from sway itself.
• I've moved the wrapper from the sway module (programs.sway) to the wrapper itself.
  • It is enabled by default, and it has a toggle named withBaseWrapper.
• I've added a toggle named withGAppsWrapper, disabled by default.

I did not understood what you meant with the swaybg part on your last review. Could you please clarify this point ? :)

Maybe rename the withGAppsWrapper to something like withGTKWrapper? GApps is already widely used for Google Apps on the Android platform, I don't think I've ever heard it used in reference to GTK.

@Synthetica9 You are right, It wasn't really explicit. Fixed it.

@primeos I have just found your PR about adding Sway to dislpay managers :

    # To make a Sway session available if a display manager like SDDM is enabled:
    services.xserver.displayManager.extraSessionFilePackages = [ swayJoined ];

I believe it will be impacted by this PR (if merged). Do you want to merge yours first then let me rebase mine or the other way around ?

cc @gnidorah @colemickens (FYI, forgot that @GrahamcOfBorg doesn't notify module maintainers, feel free to add feedback)

Thank you again @primeos for your time and review, I've made the fixes you asked, important changes made in this iteration :

• We now have a wrapperFeatures that can be used like you proposed (it's way cleaner).
• I've moved swaybg to the wrapper.

EDIT: I'm still having trouble to understand the exact behavior of wrapGAppsHook, it seems what in buildInputs can affect how it behaves, but I'm not sure why.

Rebased branch after merging of #75363

@elyhaka thanks for your awesome work, I finally got around to properly review this (sorry for the long delay :o) and added a few minor improvements. Are you ok with these changes? If so I would run a final test, squash my commit into yours and it should be ready to be merged.

This looks good to me ! Thank you for taking the time to do the final changes 😄

@primeos : I've made some tests, and it seems we have a regression on gsettings. I'm investigating the cause.

EDIT : I managed to make it works again locally (I haven't committed since I do not want to touch your modification without being sure it fits the requirements).

Here are my steps to restore the expected behavior:

1. Removed the dontWrapGApps = true;

2. Rewrite the postBuild hook with the following content:

   wrapProgram $out/bin/sway --prefix PATH : "${swaybg}/bin"
   ${optionalString withGtkWrapper "wrapGAppsHook"}
   

I am not sure at all why it behaves this way

@primeos : It works with the last commit 👍

@elyhaka thanks, I totally missed that regression :o I verified that the wrapper was adding arguments but didn't realise that some where missing. I've had a look at pkgs/build-support/setup-hooks/wrap-gapps-hook.sh and noticed that wrapGAppsHook must be executed to initialize the variables. It should work as expected now. I've also updated buildInputs as it turns out that only gdk-pixbuf should be required.

The reason for the dontWrapGApps=true hack is that I would like to avoid wrapping all binaries and wrapping sway twice:

[root@nixos-unstable:/var/nixpkgs]# ls -al /nix/store/k4cm6pvbc37iwz9ac6vb38kvzpwipczc-sway-1.2/bin/
total 44
dr-xr-xr-x 2 root root 4096 Jan  1  1970 .
dr-xr-xr-x 5 root root 4096 Jan  1  1970 ..
-r-xr-xr-x 1 root root 1047 Jan  1  1970 sway
-r-xr-xr-x 1 root root  959 Jan  1  1970 swaybar
lrwxrwxrwx 1 root root   74 Jan  1  1970 .swaybar-wrapped -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaybar
-r-xr-xr-x 1 root root  959 Jan  1  1970 swaymsg
lrwxrwxrwx 1 root root   74 Jan  1  1970 .swaymsg-wrapped -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaymsg
-r-xr-xr-x 1 root root  959 Jan  1  1970 swaynag
lrwxrwxrwx 1 root root   74 Jan  1  1970 .swaynag-wrapped -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaynag
lrwxrwxrwx 1 root root   71 Jan  1  1970 .sway-wrapped -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/sway
-r-xr-xr-x 1 root root  956 Jan  1  1970 .sway-wrapped_

With this hack only sway will be wrapped and also only once (if I think about it it should also be possible to get rid of .sway-wrapped by using something like makeWrapper $oldBinary $wrapper $args but that could be optimized later):

[root@nixos-unstable:/var/nixpkgs]# ls -al /nix/store/0g4gkad9ikim9hf94sgzn9g3dgnganbv-sway-1.2/bin/
total 28
dr-xr-xr-x 2 root root 4096 Jan  1  1970 .
dr-xr-xr-x 5 root root 4096 Jan  1  1970 ..
-r-xr-xr-x 1 root root 1046 Jan  1  1970 sway
lrwxrwxrwx 1 root root   74 Jan  1  1970 swaybar -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaybar
lrwxrwxrwx 1 root root   74 Jan  1  1970 swaymsg -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaymsg
lrwxrwxrwx 1 root root   74 Jan  1  1970 swaynag -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/swaynag
lrwxrwxrwx 1 root root   71 Jan  1  1970 .sway-wrapped -> /nix/store/86gzfk3pj8bg4clw2var203id2br8wr2-sway-unwrapped-1.2/bin/sway

Edit:

@primeos : It works with the last commit +1

Awesome, thanks :)

@primeos Wow, really good catch ! Thank you very much for the explanation. It is way cleaner with this approach. Everything looks good to me !

Thank you too @primeos ! 😃