Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow imperative usage with home-manager module #68

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

DaniD3v
Copy link
Collaborator

@DaniD3v DaniD3v commented May 3, 2024

This PR addresses my issue #60. It is built to allow both a declarative and imperative nix config.

Draft Status

Note that the module currently errors when using the wallpaper option

Currently the imperative part is completely implemented. To imitate functionality of the old home-manager module I'd like to request a feature on the main matugen binary. The option matugen --prefix <path>, which should manipulate the out_paths for all targets would make this module significantly more simple and maintainable. I also think the prefix option would be useful in general.

I plan on adding a wrapper script that automatically removes the symlinks made by home-manager once someone runs matugen. That way users can have, as mentioned before, both a declerative matugen setup, and still use the command normally. (Because otherwise you'd get a IO error: File system is immutable when trying to write in the nix store). For convenience, and because there's barely a difference in implementing that I'll also allow users to hook into that script.

Duplicate

#65 is a PR solving the same issue. I didn't realize there was already someone else trying to fix this and by the time I realized I was already 90% done. Sry abt that.

RFC

One thing I wanted to mention is that we should probably remove the osCfg. There's no point in that and it adds unnecessary complexity. No one configures themes system-wide.

Example Usage

Configuring the settings would look something like this.

programs.matugen = {
    enable = true;

    settings = {
      config = {
        reload_apps = true;
        reload_apps_list = {
          gtk_theme = true;
          kitty = true;
          dunst = true;
        };
      };
    };
  };

You can add templates like this

programs.matugen.templates.firefox = {
    input_path = ../../external/templates/colors.json;
    output_path = "${config.xdg.cacheHome}/wal/colors.json";
  };

@DaniD3v DaniD3v mentioned this pull request May 3, 2024
@DaniD3v DaniD3v marked this pull request as draft May 3, 2024 15:57
@InioX
Copy link
Owner

InioX commented May 5, 2024

I'm not sure i understand the --prefix option correctly, could you explain it a bit further?

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented May 5, 2024

I'm not sure i understand the --prefix option correctly, could you explain it a bit further?

A path that gets inserted before every template output_path
so a template with output_path = ~/.cache/wal/colors.json with matugen image ... --prefix $out/ gets saved to $out/$HOME/.cache/wal/colors.json

@InioX
Copy link
Owner

InioX commented May 19, 2024

I'll start working on it.

@InioX
Copy link
Owner

InioX commented May 29, 2024

I've been trying but the paths on windows are kinda weird, I gotta install nixos back first.

@InioX
Copy link
Owner

InioX commented Jul 1, 2024

I forgot to mention this PR when i added the --prefix flag, my bad.

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 21, 2024

I tried picking this up again but it seems that there's something wrong with the --prefix option.

  1. the documentation is wrong: matugen --help
  -c, --config <FILE>        Sets a custom config file
  -p, --prefix <PATH>        Sets a custom config file
  1. when I run this command
matugen image /nix/store/h9wzjbpllh2s7lww0adpqfs9szvyqp8r-simple-blue-2016-02-19/share/backgrounds/nixos/nix-wallpaper-simple-blue.png --config /nix/store/kkr8q14s48qhx6xh1j1s3yjrymnajgmh-config.toml --prefix /nix/store/2y0mshl6qs2ppikqfwm0qbf032vz7fsw-results

with this config

[config]
reload_apps = true

[config.reload_apps_list]
dunst = true
gtk_theme = true
kitty = true

[templates]
[templates.firefox]
input_path = "/nix/store/2mbczqpdq473xw2wrqbkzx48cq147dkf-colors.json"
output_path = "/home/notyou/.cache/wal/colors.json"

[templates.gtk]
input_path = "/nix/store/kn5iczmp5iy7h1rzjqd06bhm8wgxrj4p-gtk.css"
output_path = "~/.config/gtk-4.0/gtk.css"

I get

       > ✖ The /home/notyou/.cache/wal folder doesnt exist, trying to create...
       > Error:
       >    0: No such file or directory (os error 2)
       >
       > Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
       > Run with RUST_BACKTRACE=full to include source snippets.

which is really weird because the script should never access that path. Note that this was ran in the nix environment so matugen probably has some issues accessing $HOME

now when I manually execute the command I get

Error:
   0: No such file or directory (os error 2)

Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Run with RUST_BACKTRACE=full to include source snippets.

when I remove the prefix it works.

here's the backtrace:

Error:
   0: No such file or directory (os error 2)

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 3 frames hidden ⋮
   4: matugen::util::template::Template::generate::hedb60ba49c98cafe
      at <unknown source file>:<unknown line>
   5: matugen::main::h6d65d1a0d0b00a01
      at <unknown source file>:<unknown line>
   6: std::sys_common::backtrace::__rust_begin_short_backtrace::h647e91c7638224fa
      at <unknown source file>:<unknown line>
   7: std::rt::lang_start::{{closure}}::h2140cd84acf47713
      at <unknown source file>:<unknown line>
   8: std::panicking::try::ha86251cf5daa9bea
      at <unknown source file>:<unknown line>
   9: std::rt::lang_start_internal::ha6a51778162f8d22
      at <unknown source file>:<unknown line>
  10: main<unknown>
      at <unknown source file>:<unknown line>
  11: __libc_start_call_main<unknown>
      at <unknown source file>:<unknown line>
  12: __libc_start_main@@GLIBC_2.34<unknown>
      at <unknown source file>:<unknown line>
  13: _start<unknown>
      at <unknown source file>:<unknown line>

Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering.
Run with RUST_BACKTRACE=full to include source snippets.

@InioX
Copy link
Owner

InioX commented Jul 23, 2024

I think I fixed it. I'll also add some warnings when you try to write to root and matugen isn't running with elevated privileges.

@DaniD3v DaniD3v force-pushed the main branch 3 times, most recently from dc840f3 to e8a5324 Compare July 31, 2024 15:38
@DaniD3v DaniD3v marked this pull request as ready for review July 31, 2024 15:38
@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 31, 2024

I deviated a little bit from my original plan. Now we're using an activation script to basically just run matugen in the home-manager activation script.

This is kind of ugly because it makes the system have state. The flake can still be pure because only the activation script has to be reproducable tho.

Why I went with the activation script

  • It's less work
  • When using a wrapper script for matugen image and removing the symlinks beforehand running home-manager activate will throw an error because of the existing files, which is not ergonomic to use at all
  • When copying the outputs manually (only run a cp in the activation script which is a bit cleaner) I can't automatically delete old removed templates.

Alternatives

I was thinking about using home-manager specializations. They are probably the best way to do this, however they're not yet stable
I will probably open a new PR once specializations are stabilized

Reasons for Specializations:

  • matugen doesn't even need to be installed to allow switching between themes and wallpapers (lower system footprint)
  • the activation is always instant and atomic (because of home-manager)
  • the configuration is more pure because there's 0 state

Downsides:

  • way more code for the module
  • We would also need to have a directory of wallpapers somewhere so that the user can use rofi/ other scripts to select a wallpaper. Linking this wallpaper to the correct specialization is kinda difficult.
  • We pollute the specializations config, which the user might expect to be clean (shouldn't really matter tho)

Sorry abt not using the --prefix option in the end. I only came up with the method of putting matugen in the activation script after re-designing the whole PR. I still left a commend in there fixing the option (again). I you're worried about minimal PRs I can remove it.

module.nix Outdated Show resolved Hide resolved
@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 31, 2024

If it's fine with the author of #65 we can close that PR (it seems to be inactive anyways.). If he needs certain changes I can also implement them in this PR.

also fixes #60.

@Charging1948
Copy link

Looking very much forward to this getting merged, thanks for implementing!

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 31, 2024

I forgot to update the README. I'll also make a PR to the Wiki once I'm ready.

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 31, 2024

If anyone wants to try this (for reviewing) here's some snippets from my current configuration.

programs.matugen = {
  enable = true;

  settings = {
    config = {
      reload_apps = true;
      reload_apps_list = {
        gtk_theme = true;
        kitty = true;
        dunst = true;
      };
    };
  };

  wallpaper = "${pkgs.nixos-artwork.wallpapers.simple-blue}/share/backgrounds/nixos/nix-wallpaper-simple-blue.png";
};

programs.matugen.templates.firefox = {
  input_path = ../../external/templates/colors.json;
  output_path = "${config.xdg.cacheHome}/wal/colors.json";
};

programs.matugen.templates.gtk = {
  input_path = ../../external/templates/gtk.css;
  output_path = "${config.xdg.configHome}/gtk-4.0/gtk.css";
};

Don't forget to change your flake input to

# replace once `https://github.com/InioX/matugen/pull/68` gets merged
matugen.url = "github:DaniD3v/Matugen/"

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Jul 31, 2024

Hi, considering this PR hasn't moved in a few months but seems to be in a pretty good shape, is there a chance to get this finalized for the 2.4.0 release?

I could use the feature myself so I can pitch in some effort if there's anything I could help with.

Originally posted by @AtomicTroop in #65 (comment)

you're probably interested in this as well

@Charging1948
Copy link

I tried using the module, but when i enable the option reload_apps, i always get following error when activating home-manager module:

warning: the following units failed: home-manager-jk.service

× home-manager-jk.service - Home Manager environment for jk
     Loaded: loaded (/etc/systemd/system/home-manager-jk.service; enabled; preset: enabled)
     Active: failed (Result: exit-code) since Thu 2024-08-01 10:42:14 CEST; 84ms ago
   Duration: 1min 11.484s
    Process: 112970 ExecStart=/nix/store/qznyj3wda5rqh2j67g6xhgnf9b1wgijw-hm-setup-env /nix/store/y4fc1hbi73m92r0s9l3fq3xnbxxgm73w-home-manager-generation (code=exited, status=1/FAILURE)
   Main PID: 112970 (code=exited, status=1/FAILURE)
         IP: 0B in, 0B out
        CPU: 2.539s

Aug 01 10:37:31 protean hm-activate-jk[96874]: Creating home file links in /home/jk
Aug 01 10:37:33 protean hm-activate-jk[96874]: Activating matugenCopyWallpapers
Aug 01 10:37:33 protean hm-activate-jk[98266]: Error:
Aug 01 10:37:33 protean hm-activate-jk[98266]:    0: No such file or directory (os error 2)
Aug 01 10:37:33 protean hm-activate-jk[98266]: Backtrace omitted. Run with RUST_BACKTRACE=1 environment variable to display it.
Aug 01 10:37:33 protean hm-activate-jk[98266]: Run with RUST_BACKTRACE=full to include source snippets.

If i disable the option, the activation is working as expected.

@AtomicTroop
Copy link

Gave this a try too. Basic configuration worked as expected and initial rebuild updated my colorschemes correctly.
After updating the settings.config attributes again and rebuilding, the activation script doesn't seem to run anymore. Both config.toml and activation-script were built though.

Not sure what's going wrong, will have to dig into this later tonight.

@AtomicTroop
Copy link

AtomicTroop commented Aug 10, 2024

Turns out the issue was in fact not activation related, but a combination of two wallpapers producing the same (default?) colorscheme and swww failing to run during activation:
Failed to set wallpaper, the program swww was not found in PATH
Of course I also looked at outdated logs initially, this gave me the impression that nothing was being run at all.

I guess during activation I would need to point to swww's executable directly but matugen doesn't support this I think?

On a related note, I'm also getting the 0: No such file or directory (os error 2) when the reload_apps option is enabled. Manually running the tool in verbose mode points to something interesting. All supported apps seem to attempt a reload despite my configuration only having kitty reloads enabled:

ℹ Restarting waybar
ℹ Restarting kitty
ℹ Restarting dunst
ℹ Setting gtk theme to adw-gtk3-dark
Error:
   0: No such file or directory (os error 2)

  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ BACKTRACE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
                                ⋮ 3 frames hidden ⋮
   4: matugen::reload::unix::set_theme::h82ffbca410a0011b
      at <unknown source file>:<unknown line>
   5: matugen::reload::unix::reload::h60ef175f10ff03b3
      at <unknown source file>:<unknown line>
   6: matugen::main::hcd52ee859bda1041
      at <unknown source file>:<unknown line>
   7: std::sys_common::backtrace::__rust_begin_short_backtrace::h7b1097ab81e8c437
      at <unknown source file>:<unknown line>
   8: std::rt::lang_start::{{closure}}::haf3c04536d67cb07
      at <unknown source file>:<unknown line>
   9: std::panicking::try::h8ebc32e168e5ed61
      at <unknown source file>:<unknown line>
  10: std::rt::lang_start_internal::h5dd16626185ad212
      at <unknown source file>:<unknown line>
  11: main<unknown>
      at <unknown source file>:<unknown line>
  12: __libc_start_call_main<unknown>
      at <unknown source file>:<unknown line>
  13: __libc_start_main@@GLIBC_2.34<unknown>
      at <unknown source file>:<unknown line>
  14: _start<unknown>
      at <unknown source file>:<unknown line>

I tried explicitly disabling the gtk reload functionality, but it seems to have no effect here. The relevant lines from my generated config:

[config]
reload_apps = true

[config.reload_apps_list]
gtk_theme = false
kitty = true

Edit:
So turns out that explicitly disabling waybar and dunst reloading too fixed that particular issue during manual runs:

[config.reload_apps_list]
gtk_theme = false
kitty = true
waybar = false
dunst = false

I don't think the default for these values should be true if the reload fails when the tools in question are missing.
Regardless, it looks like the issue still appears when running the activation script so I'm guessing it's not finding the tools it's trying to reload just like it can't find swww for the wallpaper refresh.
I can work around this by setting home.emptyActivationPath = false; but this is not recommended, so it's a hacky solution at best.

@InioX
Copy link
Owner

InioX commented Aug 16, 2024

You could try pointing to the executable directly inside a hook as a quick test.

[templates.foo]
# ...
post_hook = "<sww command> {{ image }}"

I could also add config options to set program paths. As I was refactoring the code, I started thinking about rewriting the whole reload section anyways, pretty sure it was written in a rush.

@DaniD3v
Copy link
Collaborator Author

DaniD3v commented Aug 17, 2024

I just realized that the reload-apps feature probably doesn't work anyways.

For example, you could have swww "installed" but not in your path because you
only access it directly via ${pkgs.swww}/bin/swww in your rofi command.

I think I'll move these options out of matugen.settings.reload_apps_list and into a custom
nix option that automatically adds the binaries to the path when the options are enabled.

The issue is that the user could have some version conflicts by, for example,
using pkgs.unstable.swww (or any other application that has a reload option) manually in a systemd service.
That's more of a general Nix issue, though.


I'll override the default for the reload_apps options to false because automatically pulling in swww as a dependency just because matugen is installed is a bad idea. Having different defaults is not a good idea either, so I'll suggest we change this in the main config as well.

This would also make the code a bit cleaner because we can use serde's #[default] and avoid wrapping everything in Option.

(This is the struct I'm talking about)

#[derive(Deserialize, Serialize, Debug)]
pub struct Apps {
    pub kitty: Option<bool>,
    pub waybar: Option<bool>,
    pub gtk_theme: Option<bool>,
    pub dunst: Option<bool>,
}

@AtomicTroop
Copy link

I think leaning into the pre/post hooks and removing the whole reload_apps concept is a better way to go.
The four supported tools right now could be replaced with example hooks instead, and this would limit matugen's focus to what it does best rather than taking on an endless maintenance effort by supporting an ever-growing list of officially supported apps.

By taking this approach, you'd also work around this whole activation script path issue as the responsibility for providing an absolute path to the binaries would be on the user writing the hooks in their nix configuration.

module.nix Show resolved Hide resolved
module.nix Outdated Show resolved Hide resolved
@ys5g
Copy link
Contributor

ys5g commented Oct 27, 2024

Also, it's just a matter of preference but you might want to remove templates from programs.matugen and add it to settings like this:

settings = with lib; mkOption {
  type = types.submodule {
    freeformType = tomlFormat.type;
    
    options = {
      templates = lib.mkOption {
        type = with lib; types.attrsOf (types.submodule {
          options = {
            input_path = mkOption {
              type = types.path;
              example = "./style.css";
              description = "Path to the template";
            };
            output_path = mkOption {
              type = types.str;
              example = ".config/style.css";
              description = "Absolute path where the generated file will be written to";
            };
            post_hook = mkOption {
              type = types.str;
              example = "";
              description = "Shell command to run after generating a template. {{placeholders}} are allowed";
            };
          };
        });
        default = osCfg.templates or {};
        
        description = ''
          Templates that have `{{placeholders}}` which will be replaced by the respective colors.
          See <https://github.com/InioX/matugen/wiki/Configuration#example-of-all-the-color-keywords> for a list of colors.
        '';
      };
    };
  };
  example = lib.literalExpression ''
    settings = {
      config.wallpaper = {
        set = true;
        command = "\${lib.getExe pkgs.swww}";
        arguments = [
          "img"
          "--transition-type"
          "center"
        ];
      };
      templates.gtk3 = {
        input_path = ./gtk.css;
        output_path = "\${config.xdg.configHome}/gtk-3.0/gtk.css";
      };
    };
  '';
  description = ''
    Settings to write to the matugen configuration file
    A manual for configuring matugen can be found at <https://github.com/InioX/matugen/wiki/Configuration>
  '';
}

And for the record, you could just make the type of both settings.templates and settings.config types.attrs. If you're wondering why you shouldn't just leave it empty, last time I checked, the templates and config sections of matugen's config need to be present, even if empty.
Edit: while you're at, I think it would be better to just use inherit (tomlFormat) type for the entirety of programs.matugen.settings and make the default { config = {}; templates = {}; }

InioX and others added 2 commits November 6, 2024 17:17
Co-authored-by: ys5g <136090707+ys5g@users.noreply.github.com>
Co-authored-by: ys5g <136090707+ys5g@users.noreply.github.com>
@InioX InioX force-pushed the main branch 2 times, most recently from 09682fb to 4dbb91c Compare November 13, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants