danth / stylix

System-wide colorscheming and typography for NixOS
https://stylix.danth.me/
MIT License
1.22k stars 159 forks source link

kde: reduce package scope #315

Open trueNAHO opened 7 months ago

trueNAHO commented 7 months ago

IIRC, all Stylix targets are enabled by default, resulting in themePackage being installed:

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L218-L219

However, themePackage is only required at Nix build time in the home.activation.stylixLookAndFeel expression at:

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L247

I assume replacing pkgs.runCommandLocal with pkgs.writeShellApplication, and removing themePackage from home.packages would allow themePackage to be garbage collected as the /nix/store path to themePackage is interpolated with "${}". The current implementation would prevent Nix from garbage collecting themePackage as it is installed with home.packages:

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L141-L169

Additionally pkgs.writeShellApplication { nativeBuildInputs = [pkgs.imagemagick]; ... } has the advantage of removing the explicit PATH statement:

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L150

trueNAHO commented 7 months ago

I assume replacing pkgs.runCommandLocal with pkgs.writeShellApplication, and removing themePackage from home.packages would allow themePackage to be garbage collected as the /nix/store path to themePackage is interpolated with "${}". The current implementation would prevent Nix from garbage collecting themePackage as it is installed with home.packages:

Actually, unlike ${toString ...}, "${...}" links the /nix/store path to the generated string, preventing Nix from garbage collecting it. So scratch that point.

Additionally pkgs.writeShellApplication { nativeBuildInputs = [pkgs.imagemagick]; ... } has the advantage of removing the explicit PATH statement:

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L150

Replacing

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L235

with

home.activation.stylixLookAndFeel = let
  themePackage = pkgs.writeShellApplication {
    nativeBuildInputs = [pkgs.imagemagick];
    /* ... */
  };
in lib.hm.dag.entryAfter [ "writeBoundary" ] ''

and removing

https://github.com/danth/stylix/blob/fdf8fd261eba972e23e8926caeb3aa41c5e3ac68/modules/kde/hm.nix#L219

would improve maintainability.

danth commented 7 months ago

If I remember correctly, KDE expects the look-and-feel package to exist in a standard location, which is why it's installed to ~/.nix-profile/share/plasma/look-and-feel.

Also consider that the code in home.activation.stylixLookAndFeel runs at activation time, rather than build time, so therefore anything it refers to will always be part of the system closure.