Skip to content

[release-25.11] stylix/palette: allow impure absolute stylix.image path#2150

Merged
trueNAHO merged 1 commit intonix-community:release-25.11from
trueNAHO:stylix-palette-allow-impure-absolute-stylix-image-paths
Jan 26, 2026
Merged

[release-25.11] stylix/palette: allow impure absolute stylix.image path#2150
trueNAHO merged 1 commit intonix-community:release-25.11from
trueNAHO:stylix-palette-allow-impure-absolute-stylix-image-paths

Conversation

@trueNAHO
Copy link
Copy Markdown
Member

@trueNAHO trueNAHO commented Jan 19, 2026

Allow an impure absolute stylix.image path for end-user convenience by
relaxing the guarantees established in commits ca1bc329e910
("stylix/palette: coerce derivations to store paths") and 61c9f4dd1435
("treewide: remove redundant stylix.image escaping and string
coercion").

When stylix.image cannot be copied to the store, it is not guaranteed to
be a stringified store path with a valid string context and no special
characters, resulting in UB.

This PR makes nix eval .#testbed:alacritty:dark pass with the following test:

diff --git a/stylix/testbed/images.nix b/stylix/testbed/images.nix
index f03b0be2..f100bfa4 100644
--- a/stylix/testbed/images.nix
+++ b/stylix/testbed/images.nix
@@ -1,10 +1,6 @@
 { fetchurl, runCommandLocal }:
 {
-  dark = fetchurl {
-    name = "mountains.jpg";
-    url = "https://unsplash.com/photos/ZqLeQDjY6fY/download?ixid=M3wxMjA3fDB8MXxhbGx8fHx8fHx8fHwxNzE2MzY1NDY4fA&force=true";
-    hash = "sha256-Dm/0nKiTFOzNtSiARnVg7zM0J1o+EuIdUQ3OAuasM58=";
-  };
+  dark = "/";

   light =
     let

This undefined feature has been requested in #2134 (comment).

I am fine with adding this because the following task would eventually also remove the store guarantees:

  • (6) Implement slideshow with post-processing capabilities

-- #534

Until then, it would be good to revert the hidden breaking change from commit ca1bc32 ("stylix/palette: coerce derivations to store paths").

CC: @MattSturgeon


@trueNAHO trueNAHO added the backport: release-25.11 To be backported to the release-25.11 stable branch label Jan 19, 2026
@stylix-automation stylix-automation bot added the topic: stylix /stylix/ subsystem label Jan 19, 2026
@trueNAHO trueNAHO changed the title stylix/palette: allow impure absolute stylix.image paths stylix/palette: allow impure absolute stylix.image path Jan 19, 2026
@MattSturgeon
Copy link
Copy Markdown
Member

MattSturgeon commented Jan 20, 2026

With #2150, it would be better to consider the store copying aspect as an implementation detail that is irrelvant to end-users, especially because the store copy cannot be guaranteed.

Unfortunately it's not quite that simple.

  1. coercedTo already documents both the coerced-to and coerced-from types:

    path in the Nix store or absolute path convertible to it

    absolute path or absolute path convertible to it

  2. An option's public API is not just what users define, it's also the option's final value.

If you really want the coercion to be undocumented, then you need types.coercedTo ... // { inherit (types.path) description descriptionClass; }:

type = with lib.types; nullOr (
  coercedTo path (src: "${src}") path
  // { inherit (path) description descriptionClass; }
);

However, undocumented coercion can confuse end-users. Imagine we have an option foo that coerces numbers to strings: if you define foo = 1, you may be confused that config.foo is "1".

In this case, path-like strings are not copied to store, but path-values are. If a user defines stylix.image = ./foo, then config.stylix.image is not ./foo - it is an in-store copy. That's fine if the user knows to expect it, but if it's undocumented it is confusing.

For example, this can break assumptions about relative paths. I might assume I can do config.stylix.image + "/../bar", however .. here is the nix store.

Until then, it would be good to revert the hidden breaking change from commit ca1bc32 ("stylix/palette: coerce derivations to store paths").

That's up to you. It's certainly valid to resolve the breaking change. It's also valid to consider that breaking change as intentional; it ensures the option is used correctly.

@trueNAHO
Copy link
Copy Markdown
Member Author

trueNAHO commented Jan 20, 2026

With #2150, it would be better to consider the store copying aspect as an implementation detail that is irrelvant to end-users, especially because the store copy cannot be guaranteed.

Unfortunately it's not quite that simple.

  1. coercedTo already documents both the coerced-to and coerced-from types:

    path in the Nix store or absolute path convertible to it

    absolute path or absolute path convertible to it

  2. An option's public API is not just what users define, it's also the option's final value.

If you really want the coercion to be undocumented, then you need types.coercedTo ... // { inherit (types.path) description descriptionClass; }:

type = with lib.types; nullOr (
  coercedTo path (src: "${src}") path
  // { inherit (path) description descriptionClass; }
);

However, undocumented coercion can confuse end-users. Imagine we have an option foo that coerces numbers to strings: if you define foo = 1, you may be confused that config.foo is "1".

In this case, path-like strings are not copied to store, but path-values are. If a user defines stylix.image = ./foo, then config.stylix.image is not ./foo - it is an in-store copy. That's fine if the user knows to expect it, but if it's undocumented it is confusing.

For example, this can break assumptions about relative paths. I might assume I can do config.stylix.image + "/../bar", however .. here is the nix store.

Agreed. I crucially forgot to mention in my previous message that extending the option description to something as follows is arguably overkill and confusing, meaning it should be fine without extending the description:

(
  coercedTo path (p: "${p}") path
  // {
    description = path.description + ", copied to store if possible";
  }
)

-- #2134 (comment)

Until then, it would be good to revert the hidden breaking change from commit ca1bc32 ("stylix/palette: coerce derivations to store paths").

That's up to you. It's certainly valid to resolve the breaking change. It's also valid to consider that breaking change as intentional; it ensures the option is used correctly.

Considering the pathInStore guarantee will eventually be removed either way, I have no strong opinion on preemptively relaxing the store guarantees. From a design perspective, introducing pathInStore in the meantime is better than sticking with the incorrect path type.

Despite Stylix having no documented backport policy, what about keeping the pathInStore change on the master branch and reverting to the path type on the stable release-25.11 branch? This would revert the breaking change on the stable branch, which should probably not receive unnecessary breaking changes. In that case, the target branch of this PR should be changed. I am in favor of this approach. @0xda157 and @danth, what do you think?

For reference to end-users, migrating from impure absolute stylix.image paths to pathInStore can be achieved by copying the file into the configuration repository and accessing it with something like ./wallpaper.png, or uploading the file out-of-tree and downloading it with fetchurl. Either way, stylix.image becomes a static reference coupled to the configuration evaluation.

@trueNAHO trueNAHO changed the base branch from master to release-25.11 January 22, 2026 17:07
@trueNAHO trueNAHO changed the title stylix/palette: allow impure absolute stylix.image path [release-25.11] stylix/palette: allow impure absolute stylix.image path Jan 22, 2026
@stylix-automation stylix-automation bot added topic: documentation Documentation additions or improvements topic: dependencies Dependency updates topic: nixos NixOS target topic: home-manager Home Manager target topic: darwin nix-darwin target topic: droid Nix-on-Droid target topic: overlay Overlay changes topic: testbed Testbed changes topic: ci /.github/ subsystem topic: flake /flake.nix, /flake.lock, and /flake/ subsystems topic: modules /modules/ subsystem labels Jan 22, 2026
@trueNAHO trueNAHO force-pushed the stylix-palette-allow-impure-absolute-stylix-image-paths branch from 84f008d to ebac86c Compare January 22, 2026 17:10
Comment thread stylix/palette.nix Outdated
Allow an impure absolute stylix.image path for end-user convenience by
relaxing the guarantees established in commits ca1bc32
("stylix/palette: coerce derivations to store paths") and 61c9f4d
("treewide: remove redundant stylix.image escaping and string
coercion").

When stylix.image cannot be copied to the store, it is not guaranteed to
be a stringified store path with a valid string context and no special
characters, resulting in UB.
@trueNAHO trueNAHO force-pushed the stylix-palette-allow-impure-absolute-stylix-image-paths branch from ebac86c to aaa4f24 Compare January 23, 2026 20:33
@trueNAHO trueNAHO removed backport: release-25.11 To be backported to the release-25.11 stable branch topic: documentation Documentation additions or improvements topic: dependencies Dependency updates topic: nixos NixOS target topic: home-manager Home Manager target topic: darwin nix-darwin target topic: droid Nix-on-Droid target labels Jan 23, 2026
@trueNAHO trueNAHO removed topic: overlay Overlay changes topic: testbed Testbed changes topic: ci /.github/ subsystem topic: flake /flake.nix, /flake.lock, and /flake/ subsystems topic: modules /modules/ subsystem labels Jan 23, 2026
@trueNAHO trueNAHO merged commit 05e53b5 into nix-community:release-25.11 Jan 26, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic: stylix /stylix/ subsystem

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants