bug: IFD is used to set Kitty themes

[hacker1024]

Are you following the right branch?

• [X] My Nixpkgs and Home Manager versions are in sync

Is there an existing issue for this?

• [X] I have searched the existing issues

Issue description

The Kitty theme option added in #2710 uses import-from-derivation to find the theme file. In later revisions, an error is also thrown if the theme does not exist.

IFD slows down evaluation and makes instantiation harder in many deployment scenarios, and should be avoided where possible.

Either the themes.json from kitty-themes should be vendored, or the file finding logic should be moved into the activation script instead.

Maintainer CC

@NelsonJeppesen

System information

- system: `"x86_64-linux"`
- host os: `Linux 6.7.5-zen1, NixOS, 24.05 (Uakari), 24.05pre585741.b98a4e1746ac`
- multi-user?: `yes`
- sandbox: `yes`
- version: `nix-env (Nix) 2.18.1`
- channels(root): `"home-manager, impermanence, nixos, sops-nix"`
- nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

[NelsonJeppesen]

Happy to work on this, but have a few questions

In later revisions, an error is also thrown if the theme does not exist.

I'd assumed that's a feature - does home-manager have precedent for the desired approach? I could see benefits throwing and error and also not throwing an error.

IFD slows down evaluation and makes instantiation harder in many deployment scenarios, and should be avoided where possible.

I was not aware of this. Are the docs or policies I can look at to understand this better?

Either the themes.json from kitty-themes should be vendored

Can you show me example of how this would work. I'm not familiar how to vendor themese.json in this context. My understanding was themes.json and the theme.conf files need to be packaged together, since themes.json points to the conf files. Or maybe I'm not understanding what you're asking for?

or the file finding logic should be moved into the activation script instead

I can poke around and see what this looks like

[hacker1024]

I'd assumed that's a feature - does home-manager have precedent for the desired approach? I could see benefits throwing and error and also not throwing an error.

There are no problems there, I was just mentioning it for more context about why IFD is used.

I was not aware of this. Are the docs or policies I can look at to understand this better?

Nixpkgs prohibits it, stating the following:

Import From Derivation (IFD) is disallowed in Nixpkgs for performance reasons: Hydra evaluates the entire package set, and sequential builds during evaluation would increase evaluation times to become impractical.

The Nix manual elaborates:

This has performance implications: Evaluation can only finish when all required store objects are realised. Since the Nix language evaluator is sequential, it only finds store paths to read from one at a time. While realisation is always parallel, in this case it cannot be done for all required store paths at once, and is therefore much slower than otherwise.

I don't think IFD is explicitly forbidden in Home Manager, but the same performance concerns apply.

Can you show me example of how this would work. I'm not familiar how to vendor themese.json in this context. My understanding was themes.json and the theme.conf files need to be packaged together, since themes.json points to the conf files. Or maybe I'm not understanding what you're asking for?

The themes.json is the only file required to be read during evaluation time, as the module uses it to find the path to theme files. The theme files themselves are not needed to be read by anything other than Kitty.

It can be included in the Home Manager repository and read using lib.importJSON, rather than being obtained through another derivation.

[NelsonJeppesen]

I'd assumed that's a feature - does home-manager have precedent for the desired approach? I could see benefits throwing and error and also not throwing an error.

There are no problems there, I was just mentioning it for more context about why IFD is used.

I was not aware of this. Are the docs or policies I can look at to understand this better?

Nixpkgs prohibits it, stating the following:

Import From Derivation (IFD) is disallowed in Nixpkgs for performance reasons: Hydra evaluates the entire package set, and sequential builds during evaluation would increase evaluation times to become impractical.

The Nix manual elaborates:

This has performance implications: Evaluation can only finish when all required store objects are realised. Since the Nix language evaluator is sequential, it only finds store paths to read from one at a time. While realisation is always parallel, in this case it cannot be done for all required store paths at once, and is therefore much slower than otherwise.

I don't think IFD is explicitly forbidden in Home Manager, but the same performance concerns apply.

Thank you, this helps me understand the issue here and I agree with you. It's a bug

Can you show me example of how this would work. I'm not familiar how to vendor themese.json in this context. My understanding was themes.json and the theme.conf files need to be packaged together, since themes.json points to the conf files. Or maybe I'm not understanding what you're asking for?

The themes.json is the only file required to be read during evaluation time, as the module uses it to find the path to theme files. The theme files themselves are not needed to be read by anything other than Kitty.

It can be included in the Home Manager repository and read using lib.importJSON, rather than being obtained through another derivation.

I see where you're going with this. I worry this might make things more brittle. If the themes are renamed/changed, you now have the issue of keeping nixpkgs and home-manager channels in sync with each other. Same when themes are added, but less of an issue IMHO. Or, if you override the kitty-themes derivation (e.g. testing an update locally) you might have to override home-manager as well

Think I'll try first with moving the finding to the activation stage

[stale[bot]]

Thank you for your contribution! I marked this issue as stale due to inactivity. Please be considerate of people watching this issue and receiving notifications before commenting 'I have this issue too'. We welcome additional information that will help resolve this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

* If this is resolved, please consider closing it so that the maintainers know not to focus on this. * If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

* If you are also experiencing this issue, please add details of your situation to help with the debugging process. * If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

Don't be afraid to manually close an issue, even if it holds valuable information. Closed issues stay in the system for people to search, read, cross-reference, or even reopen – nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

[hacker1024]

This issue is still relevant.

[alejandro-angulo]

This causes nix flake check to fail when I try using deploy-rs, I worked around it by using extraConfig instead of the theme option.

extraConfig = ''
  include ${pkgs.kitty-themes}/share/kitty-themes/themes/Catppuccin-Mocha.conf
''

[alejandro-angulo]

I took a stab at fixing this in #5750 .