feat: entity.hasAspect <ref> query method on context entities#439
feat: entity.hasAspect <ref> query method on context entities#439
entity.hasAspect <ref> query method on context entities#439Conversation
|
Found two real bugs in the new regression test suite to cover this feature. Pushing fix now. :) |
|
This is functionally complete, but I'm now going through the deeper human code review/refactor process. Not ready for external reviewers yet. |
entity.hasAspect <ref> query method on context entities
|
The next feature PR on top of this will be guarded include wrappers to work around the cycle dependency -- we can create an intermediate wrapper aspect that can evaluate the constraint during resolve time. |
drupol
left a comment
There was a problem hiding this comment.
Just made very minor nit comments.
| if builtins.isAttrs result && fn ? meta && !(result ? meta) then | ||
| result // { inherit (fn) meta; } | ||
| else | ||
| result; |
There was a problem hiding this comment.
Nit:
| if builtins.isAttrs result && fn ? meta && !(result ? meta) then | |
| result // { inherit (fn) meta; } | |
| else | |
| result; | |
| result // lib.optionalAttrs (builtins.isAttrs result && fn ? meta && !(result ? meta)) { inherit (fn) meta; } |
There was a problem hiding this comment.
This isn't a bad nit. I would take it if vic hadn't already merged this commit. :)
| if config ? class then | ||
| [ config.class ] | ||
| else | ||
| throw "den.schema.conf.hasAspect: entity has no `class` or `classes`" |
There was a problem hiding this comment.
Nit: I think we could use lib.throwIf/lib.throwIfNot ?
| if classes == [ ] then | ||
| throw "den.schema.conf.hasAspect: entity has empty `classes` list" | ||
| else | ||
| lib.head classes; |
vic
left a comment
There was a problem hiding this comment.
You were right @sini, reviewing commit per commit was way easier. Maybe at the end of this can we add documentation for all meta.* and adapters.* (maybe a dedicated page on how resolve + adapters work). And also entity.hasAspect and warning-asides for its cyclic-usage.
|
Feel free to merge when conflicts resolved. |
|
just wanted to say how cool this functionality is yall, great job! |
New terminal adapter for walking a resolved aspect tree and
collecting aspectPaths of every non-tombstone aspect visited.
Result shape:
{ paths = [ [providerSeg..., name], ... ]; }
Depth-first, not deduplicated at this layer. Used by the has-aspect
lib (landing next) and available for tests, introspection tooling,
and future diag features that need structural-tree visibility.
Adds three small helpers, all exported:
- pathKey : slash-joined key for an aspectPath, matching the
convention used by adapters.structuredTrace.
- toPathSet : list-of-paths → attrset-as-set keyed by pathKey,
for O(1) membership checks.
- collectPathsInner : the raw walker shared with oneOfAspects
(landing next) so the two consumers don't duplicate the logic.
collectPaths itself is just filterIncludes wrapping the inner.
meta.adapter that keeps the first candidate structurally present in
a parent's subtree and tombstones the rest via excludeAspect:
meta.adapter = oneOfAspects [ <agenix-rekey> <sops-nix> ];
When both candidates are present the first wins; when only one is
present it survives unchanged; when neither is present the adapter
is a no-op. Complements excludeAspect / substituteAspect — the three
together cover the include-this / exclude-this / swap-this-for-that
space of structural decisions.
Shares the collectPathsInner walker with collectPaths (via the
bypassing form, to avoid re-entering our own meta.adapter and
infinitely recursing) and the pathKey / toPathSet helpers for the
subtree presence lookup. No code duplication with the surrounding
adapters.
Requires adapters.nix to accept \`den\` in its argument set so it
can reference den.lib.aspects.resolve.withAdapter from inside the
body. That reference is a fixpoint closure that's safe via Nix
laziness — the adapter body isn't forced until resolve walks a
tree, by which point den.lib.aspects is fully constructed.
nix/lib/aspects/has-aspect.nix exports:
- hasAspectIn { tree; class; ref } — low-level query. Walks tree
under class via collectPaths, returns whether ref is present.
- collectPathSet { tree; class } — set-as-attrset of slash-joined
paths for O(1) membership lookups. Thin wrapper over the
adapters.toPathSet / collectPaths combo.
- mkEntityHasAspect { tree; primaryClass; classes } — entity-facing
functor+attrs constructor: the bare form runs under primaryClass,
.forClass picks a specific class, .forAnyClass unions across all
classes the entity participates in. Per-class path sets are
thunk-cached so repeated calls share one traversal per class.
Identity is aspectPath (= meta.provider ++ [name]). refKey validates
that its argument has both `name` and `meta`, rejecting malformed
inputs loudly rather than silently producing an <anon> key. Uses
the pathKey / toPathSet helpers exported from adapters.nix — no
duplication of slash-joining or set construction.
Re-exported from nix/lib/aspects/default.nix at the canonical
den.lib.aspects.* paths. Ships with lib-level tests that exercise
the primitives in isolation against handcrafted trees plus a
factory-function identity lock-in assertion.
Adds the hasAspect query method to every entity that imports den.schema.conf — host, user, home out of the box, and any user- defined kind that imports conf inherits it for free. Zero changes to nix/lib/types.nix; the per-entity submodule definitions stay untouched. The option lives in modules/context/has-aspect.nix, auto-imported as a flake-level module by nix/flakeModule.nix. The outer module captures config.den so the inner entity submodule can reach den.lib.aspects.mkEntityHasAspect at entity-eval time. Class-protocol: prefer `classes` (list), fall back to wrapping `class` (string), else throw. Entities without a matching den.ctx.<kind> produce a call-time throw — the fallback shape preserves the functor+attrs shape so forClass / forAnyClass attribute access doesn't leak a cryptic error; all three paths throw the same error on invocation. Note: the test access path is den.hosts.<system>.<name>.hasAspect (and den.hosts.<system>.<name>.users.<user>.hasAspect for users), not the denTest `igloo` specialArg — that's the resolved NixOS config, not the den host entity where the option lives. Includes a smoke test suite covering host/user bare usage, forClass, forAnyClass, tombstone respect, absent aspect, and angle-bracket sugar equivalence. The full regression-class test matrix lands in a follow-up commit.
Extends the smoke suite with the complete regression-class matrix, organized by aspect shape: - Group A: sanity / transitive chains - Group B: parametric contexts, incl. vic#423 static-sub-in-parametric- parent and vic#413 bare-function-sub-aspect shapes as explicit regression lock-ins - Group C: factory-function aspects — direct factory reference in includes (factory-path identity is stable; inline invocation produces a differently-named sibling, tested here as a caveat) - Group D: provider sub-aspects + identity disambiguation - Group E: mutual-provider / provides chains (to-users, per-user, to-hosts) - Group F: substituteAspect, composition at different levels (NOT nested-reaching per filterIncludes.tag semantics), oneOfAspects integration - Group G: multi-class users (primary, forClass, forAnyClass, unknown class) - Group H: extensibility contract — den.schema.conf owns the option - Group I: error cases — bad ref throws - Group J: real-world call from inside a deferred nixos module body, validating the cycle-safety guarantee for the primary intended use case hasAspect reads config.resolved — the output of the ctxApply + parametric resolution pipeline. Every aspect-shape that recently produced a regression (vic#408, vic#413, vic#423, vic#429) has a lock-in test here, so a future regression in parametric.nix or aspects/types.nix that affects any of those shapes fails a hasAspect test before it reaches user code.
Three-section example file demonstrating the intended usage patterns: 1. Reading structure via host.hasAspect from inside a class-config body — the common "branch on companion aspect" case. 2. Deciding structure via oneOfAspects as a meta.adapter — the "prefer A over B" structural decision, with the adapter doing the tree walk instead of hasAspect trying to read its own output. 3. Anti-pattern section explaining why hasAspect can't be used inside an aspect's `includes =` list, with a pointer to the adapter library (oneOfAspects, excludeAspect, substituteAspect) as the correct alternatives. Ties the two features (hasAspect for reading structure, adapters for writing structure) into a unified user-facing story. Uses `example-` prefixed aspect names to avoid colliding with anything in the existing template.
feat:
entity.hasAspect <ref>query methodWhat this does
Adds a
.hasAspectmethod on context entities (host,user,home, and any custom entity kind that importsden.schema.conf). It answers "is this aspect structurally present in my resolved tree?" from inside class-config module bodies:There are two variants for when the bare form isn't enough:
Identity is compared by
aspectPath(meta.provider ++ [name]), so provider sub-aspects likefoo._.subkeep their full path. Refs can be plain aspect values (den.aspects.facter) or<angle-bracket>sugar, they're the same thing after__findFileresolves.Alongside
hasAspect, this ships a companiononeOfAspectsadapter. That's the structural-decision primitive for "prefer A over B when both are present", which is the thing you actually want when you're tempted to usehasAspectto decide includes (see the guardrails section below).Why
Real patterns that users hit:
<impermanence>config depends on whether<zfs-root>or<btrfs-root>is also configured on the host<agenix-rekey>when present andfall back to
<sops-nix>otherwiseToday these get worked around with
config.*lookups, hand-maintainedlib.elemchecks, or structural hacks.hasAspectis the first-class primitive for the read side.oneOfAspectsis the first-class primitive for the write side.Commits
Each commit is independently reviewable and builds green on its own.
fix(parametric): preserve meta on materialized parametric resultsOpened as it's own PR #440
feat(adapters): add collectPaths terminal adapterNew public terminal adapter. Walks a resolved tree via
filterIncludesand returns{ paths = [ [providerSeg..., name], ... ]; }, depth-first, not deduplicated. Tombstones are skipped via themeta.excludedcheck.Ships with two small helpers exported from the same file:
pathKey(slash-joined path key) andtoPathSet(list of paths to attrset-as-set for O(1) lookups). Both are used byhasAspectandoneOfAspects, so exporting them keeps the two consumers from duplicating the same one-liners.feat(adapters): add oneOfAspects structural-decision adaptermeta.adapterthat keeps the first structurally-present candidate and tombstones the rest viaexcludeAspect:Complements
excludeAspectandsubstituteAspect. The three together cover include-this / exclude-this / swap-this-for-that. Internally it walks the parent subtree with the raw collector (bypassingfilterIncludesso it doesn't re-enter itself), finds which candidates are present, and foldsexcludeAspectover the losers. No code duplication withcollectPathsthanks to the shared helpers from the previous commit.feat(aspects): add has-aspect library primitivesNew file
nix/lib/aspects/has-aspect.nixexporting:hasAspectIn { tree; class; ref }for any resolved tree, not justentity contexts
collectPathSet { tree; class }for an attrset-as-set of visiblepaths
mkEntityHasAspect { tree; primaryClass; classes }which builds thefunctor-plus-attrs value attached to entities. Per-class path sets
are thunk-cached, so repeated calls share one traversal per class
refKeyvalidates that its input has bothnameandmetabefore reaching foraspectPath, throwing loudly rather than silently producing an<anon>path key.feat(context): add entity.hasAspect method via den.schema.confThe wiring.
modules/context/has-aspect.nixis a flake-level module that self-wires intoden.schema.confviaconfig.den.schema.conf.imports. Every entity type that importsconf(host, user, home, and any user-defined kind) inherits.hasAspectautomatically. Zero changes tonix/lib/types.nix.Class-protocol: prefers
classes(list), falls back to[ class ], throws otherwise. Entities without a matchingden.ctx.<kind>(so noconfig.resolved) produce a call-time throw. The fallback value preserves the functor-plus-attrs shape soforClass/forAnyClassattribute access doesn't leak a cryptic error before reaching the real one.test(has-aspect): full regression-class coverage for entity method31 tests organized by aspect-construction shape. Every shape that's produced a recent regression (
#408,#413,#423,#429) has a lock-in test. A future regression inparametric.nixoraspects/types.nixthat breaks any of these shapes trips ahas-aspecttest before it reaches user code.Groups cover basic and transitive chains, parametric contexts including the static and bare-function sub-aspect shapes, factory functions, provider sub-aspects and identity disambiguation, mutual-provider and provides chains,
meta.adaptercomposition, multi-class users, the extensibility contract, error cases, and the primary intended use case (callinghasAspectfrom inside a deferrednixosmodule body).docs(example): add hasAspect + oneOfAspects worked examplesUser-facing pedagogical file in
templates/example/. Three sections: reading structure viahost.hasAspectfrom a class-config body, writing structure viaoneOfAspectsas ameta.adapter, and an anti-pattern section explaining whyhasAspectcan't decide an aspect'sincludeslist with a pointer at the adapter library.Design guardrails
hasAspectis a read-only query on frozen structure. You call it from inside class-config module bodies (nixos = ...,homeManager = ...) or from lazy positions in aspect functor bodies. It's cycle-safe by construction because by the time deferred class modules evaluate, the aspect tree has already been resolved and frozen.What it is not for: deciding an aspect's
includeslist. That's cyclic. The tree you want to query depends on the decision you wanthasAspectto inform. Users who need that reach formeta.adaptercomposed viaoneOfAspects,excludeAspect,substituteAspect, orfilter/filterIncludes. Those run during the tree walk with full structural visibility, so they can't cycle. The template example file has an explicit anti-pattern section with the failing shape and the correct rewrite.Two tools, two jobs:
hasAspectmeta.adapter+oneOfAspects/ friendsTest plan
just cipasses 331/331 at branch tipjust fmtis idempotent across the treedeadbugs/issue-413-*,deadbugs/issue-423-*, orissue-408testsproduced a recent bug
Migration
None. Purely additive.
hasAspectis a new option name, no conflict.parametric.nix,resolve.nix,ctx-apply.nix,types.nix, or the other core library files.adapters.nixgains new exports (collectPaths,oneOfAspects,pathKey,toPathSet). Nothing is renamed or removed.nix/lib/aspects/default.nixre-exports the new lib functions underden.lib.aspects.*at their canonical paths.modules/context/has-aspect.nixis a new file, picked up by the existingimport-treeflake setup.nix/lib/types.nix.Follow-ups
perHost/perUserwrappers when the aspect has no actual ctx dependence, which makes structural tooling less reliable than it could be. Docs-only, separate PR.onlyIf guard target. A small adapter sitting next tooneOfAspectsthat lets users writeincludes = [ (onlyIf <zfs-root> <zfs-impermanence>) ]for "include target iff guard is structurally present." Same cycle-avoidance trick (decision runs in the wrapper's own meta.adapter, walks the subtree with the rawcollectPathsInnercollector so it doesn't re-enter itself). Reuses every helper this PR exports. Rough spec is written up, scoped as its own follow-up PR.