stylix: pass stylixLib to extraOptions in mkTarget

[0xda157]

not having stylix lib passed prevents removal of config from module args in some modules

Things done

• [ ] Tested locally
• [ ] Tested in testbed
• [x] Commit message follows commit convention
• [ ] Fits style guide
• [ ] Respects license of any existing code used

Notify maintainers

[trueNAHO]

Cc: @Flameopathic @MattSturgeon

[0xda157]

alternatively we could override lib in /stylix/autoload.nix to add lib.stylix, which I would prefer.

[Flameopathic]

alternatively we could override lib in /stylix/autoload.nix to add lib.stylix, which I would prefer.

I would prefer this over passing it here. This patch seems to only help with using mkEnableWallpaper which will likely be removed once we implement https://github.com/nix-community/stylix/issues/208#issuecomment-1875851105, whereas a full lib extension in target files could be used for more functions with applications outside of extraOptions.

[MattSturgeon]

My instinct is that the more you rely on autoload to implement custom features outside of the module system, the more awkward it'll become to work with the module system.

So I'd caution against rushing to do more things like the mkTarget arg injection...

But if you are going to, then perhaps it's better to stop treating the target modules as, well, modules. Instead do what @trueNAHO suggested and make those files a custom "arguments for mkTarget" type.

I.e. no hm.nix or nixos.nix files would be raw modules anymore.

I'd suggest waiting until you have migrated all modules to mkTarget before such a change, though. That way you can identify pain points and rough edges gradually over time.

[danth]

Yeah, eventually turning them into not modules would also make it impossible to work around the safeguards in mkTarget, which is potentially a good thing once we know it's easy to work with.

On the other hand, it would mean you couldn't, for example, mkMerge multiple targets together within the same module (e.g. the Firefox module which also supports forks of the browser).

[0xda157]

On the other hand, it would mean you couldn't, for example, mkMerge multiple targets together within the same module (e.g. the Firefox module which also supports forks of the browser).

in the case of multiple targets in one module we can just return a list of modules and map over them with mkTarget

[MattSturgeon]

in the case of multiple targets in one module we can just return a list of modules and map over them with mkTarget

Seems viable. But the autoload and/or mkTarget code would need updating to do that as the target file will not be calling mkTarget itself internally.

You'd also need to update the autoload and/or mkTarget impl to support calling the value, as I assume it may still be useful to pass some arguments at the top of the file in some cases.

Easily done using the fnOrAttrs pattern:

targetFile = /* a modules/<module>/<platform>.nix file */;
args = /* compute the args you want to pass to mkTarget values */;

value = import targetFile;

values = lib.toList (
  if isFunction value then
    value args
  else
    value
);

toMkTargetArgs =
  fnOrAttrs:
  if isFunction fnOrAttrs then
    fnOrAttrs args
  else
    fnOrAttrs;

callMkTarget = fnOrAttrs: mkTarget (toMkTargetArgs fnOrAttrs);

modules = map callMkTarget values;

[0xda157]

You'd also need to update the autoload and/or mkTarget impl to support calling the value, as I assume it may still be useful to pass some arguments at the top of the file in some cases.

We will want to provide lib and pkgs at the top, but definitely not config if we can avoid it. If we do pass config we should try to remove the config.stylix and config.stylix.lib options to prevent unsafe access outside of the configElemenets and generalConfig

[MattSturgeon]

You'd also need to update the autoload and/or mkTarget impl to support calling the value, as I assume it may still be useful to pass some arguments at the top of the file in some cases.

We will want to provide lib and pkgs at the top, but definitely not config if we can avoid it.

Yes, that's why I said "some args" rather than "all module args" :wink:

[trueNAHO]

But if you are going to, then perhaps it's better to stop treating the target modules as, well, modules. Instead do what @trueNAHO suggested and make those files a custom "arguments for mkTarget" type.

What I meant with

I did not consider how the /modules/firefox/hm.nix module would map mkTarget targets. In that case, distributing mkTarget as a module argument seems like the best solution. Nonetheless, restricting simple modules not requiring mkTarget with an attribute set is still worthwhile.

-- https://github.com/nix-community/stylix/pull/1130#pullrequestreview-2847125149

and

https://github.com/nix-community/stylix/blob/3e447578effb29574adf26f74d5a9466f977c5ca/stylix/mk-target.nix#L118-L151

is that simple cases can be very restrictive with an attribute set, like

{
  name = /* ... */;
  humanName = /* ... */;
}

while complex setups requiring arbitrary logic to setup their final map mkTarget loop can be handled more permissively with:

{ lib, mkTarget, pkgs, ... }:
map
(
  mkTarget {
    name = /* ... */;
    humanName = /* ... */;
  }
)
/* targets */

With this approach, the /stylix/autoload.nix logic could distinguish based on whether /module/<MODULE>/<PLATFORM>.nix is an attribute set with the required name and humanName attributes or a function requesting mkTarget. These checks provide good throw error messages in case unexpected /module/<MODULE>/<PLATFORM>.nix formats are encountered.

We will want to provide lib and pkgs at the top, but definitely not config if we can avoid it. If we do pass config we should try to remove the config.stylix and config.stylix.lib options to prevent unsafe access outside of the configElemenets and generalConfig

In the more permissive case, it could maybe throw an error when undesired arguments, like config, are accessed. Unsure whether this is too restrictive.