Skip to content

fix: module-level dedup for host-aspects overlap support#468

Merged
sini merged 2 commits intomainfrom
fix/host-aspects-dedup
Apr 18, 2026
Merged

fix: module-level dedup for host-aspects overlap support#468
sini merged 2 commits intomainfrom
fix/host-aspects-dedup

Conversation

@sini
Copy link
Copy Markdown
Collaborator

@sini sini commented Apr 17, 2026

Named aspects now get a key attribute (class@identity) on their collected modules. The NixOS module system deduplicates by key across independent resolve calls, so a user can include an aspect directly AND receive it via host-aspects without duplicate option declarations.

Anonymous/synthetic aspects are excluded from keying so multiple anonymous includes coexist correctly.

Also fixes the pre-existing duplication where aspects included by both host and user produced duplicate nixos modules via the default context transition.

@sini sini force-pushed the fix/host-aspects-dedup branch 3 times, most recently from 7c327c9 to 2671570 Compare April 17, 2026 23:30
Copy link
Copy Markdown
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fix the issue I described in https://not-a-number.io/2026/evaluating-den-a-dendritic-configuration-framework/ !!!

Looking forward for it...

GG !!!

@sini sini self-assigned this Apr 18, 2026
@sini sini added allow-ci allow all CI integration tests bench-skip skip benchmark assert faster than base merge-approved Approved, merge yourself when ready labels Apr 18, 2026
sini added 2 commits April 18, 2026 12:40
Named aspects get a key attribute (class@identity) on their collected
NixOS modules. The module system deduplicates by key across independent
resolve calls, so a user can include an aspect directly AND receive it
via host-aspects without duplicate option declarations.

Anonymous aspects get stable identities derived from parent chain +
positional index (e.g. igloo/<anon>:0) via emitIncludes, replacing the
generic <anon> name. This enables correct dedup and improves tracing.

Anonymous/synthetic identities are excluded from keying so multiple
anonymous includes coexist correctly.
Forward's module-wrapping pattern (_: { imports = [...] }) works for
submodule targets but not for leaf options like environment.sessionVariables
which expect plain attribute values.

Add evalConfig flag to forward: when true, evaluates the source module
in a freeform submodule and sets the resulting config values directly at
the target path instead of wrapping in a module function.

Usage:
  den._.forward {
    fromClass = _: "variables";
    intoPath = _: [ "environment" "sessionVariables" ];
    evalConfig = true;
    ...
  };
@sini sini force-pushed the fix/host-aspects-dedup branch from f3fa9b5 to 5c85b95 Compare April 18, 2026 19:41
Copy link
Copy Markdown
Collaborator

@theutz theutz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oooh I hope this works! 🤞

@sini sini merged commit 815076e into main Apr 18, 2026
31 of 43 checks passed
@sini sini deleted the fix/host-aspects-dedup branch April 18, 2026 21:02
@vic
Copy link
Copy Markdown
Owner

vic commented Apr 19, 2026

Awesome @sini! Thanks for this!

With this working now -and fixing @drupol post- about duplication I believe we can bring back the builtin bidirectionality (that we removed because our legacy core was not able to do this dedup enabled in this fix).

What I mean by bidirectionality:

That host-aspects correctly contribute user classes homeManager, hjem to all its users without people having to use any battery.

We used to do that with this line: https://github.com/vic/den/pull/272/changes#diff-292456ccf182ad0171aa662b32637c8c4591f0d7e471b8bdec8753ba6d09e4efL20

We don't have necessarily to revert that file, but the ideas is that aspects as those defined by drupol's post must work naturally without any extension.

@vic
Copy link
Copy Markdown
Owner

vic commented Apr 19, 2026

We can chat about this, I'd like to know what you think about this (because I know you have better plans -- capabilities)

@sini
Copy link
Copy Markdown
Collaborator Author

sini commented Apr 19, 2026

Yes.

Honestly, I think the pattern you settled on is actually better, we simply need document and educate users about the new host-aspects battery. I have many users I install on my machines who only need slim accounts.

@theutz
Copy link
Copy Markdown
Collaborator

theutz commented Apr 19, 2026

I wonder if having a "isNormalUser"/"isSystemUser"-like toggle might help everyone then when they create new users? So the intentions are explicit for each new user?

@sini
Copy link
Copy Markdown
Collaborator Author

sini commented Apr 19, 2026

I wonder if having a "isNormalUser"/"isSystemUser"-like toggle might help everyone then when they create new users? So the intentions are explicit for each new user?

Is that not what the aspect include provides? Perhaps it simply needs a better name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-ci allow all CI integration tests bench-skip skip benchmark assert faster than base merge-approved Approved, merge yourself when ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants