lib/types-dag: implement v2 merge protocol for dagOf type

[khaneliman]

The dagOf type was using an ad-hoc type // { check = ...; } pattern via inherit (attrEquivalent) check, which is incompatible with throw introduced in NixOS/nixpkgs#454964.

This commit properly implements the v2 merge protocol for dagOf by:

• Making check a functor with isV2MergeCoherent = true that delegates to the underlying attrsOf type's check function
• Making merge a functor with a v2 attribute that properly delegates to the underlying type's v2 merge implementation

This ensures compatibility with the v2 merge mechanism while maintaining all existing dagOf functionality.

Fixes evaluation errors like:
error: The option '...' has a type DAG of X' that uses an ad-hoc type // { check = ...; }' override, which is
incompatible with the v2 merge mechanism.

Description

@MattSturgeon sanity check please

Checklist

• Change is backwards compatible.

• Code formatted with nix fmt or
  nix-shell -p treefmt nixfmt deadnix keep-sorted --run treefmt.

• Code tested through nix run .#tests -- test-all or
  nix-shell --pure tests -A run.all.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

• If this PR adds a new module

  • Added myself as module maintainer. See example.
  • Generate a news entry. See News
  • Basic tests added. See Tests

• If this PR adds an exciting new feature or contains a breaking change.

  • Generate a news entry. See News

[MattSturgeon]

IIRC, there's a lib function for calling either v1 or v2 merge (whichever is available on a given type), which provides some backwards compatibility.

IIUC, that's not strictly needed in this case, unless it is possible the attrsOf can come from a different revision of nixpkgs-lib. Otherwise it should be safe to assume there is a attrsEquivialent.merge.v2.

[MattSturgeon]

It smells a bit off that this doesn't handle the case where self.merge.v2 returns errors. Is it assumed that it is handled by attrEquivialent.check?

As is, this looks effectively equivalent to just doing inherit (attrEquivialent) check, and assuming that v2 is handled there.

If you're fine with that assumption, maybe it's worth noting in a comment? But it kinda feels like check.isV2MergeCoherent = true is a lie.

[khaneliman]

I honestly am not familiar with any of this logic. Was just trying to fix the eval error preventing me from building and using the PR code as an example to work with.

attrEquivalent.check is coming from attrEquivalent = types.attrsOf (dagEntryOf elemType); so i'm not familiar with how that needs to be handled. I see a merge function above in the dagEntryOf that I was wondering if it needed the same treatment for true compatibility

  dagEntryOf =
    elemType:
    let
      submoduleType = types.submodule (
        { name, ... }:
        {
          options = {
            data = mkOption { type = elemType; };
            after = mkOption { type = with types; listOf str; };
            before = mkOption { type = with types; listOf str; };
          };
          config = mkIf (elemType.name == "submodule") {
            data._module.args.dagName = name;
          };
        }
      );
      maybeConvert =
        def:
        if hm.dag.isEntry def.value then
          def.value
        else
          hm.dag.entryAnywhere (if def ? priority then mkOrder def.priority def.value else def.value);
    in
    mkOptionType {
      name = "dagEntryOf";
      description = "DAG entry of ${elemType.description}";
      # leave the checking to the submodule type
      merge =
        loc: defs:
        submoduleType.merge loc (
          map (def: {
            inherit (def) file;
            value = maybeConvert def;
          }) defs
        );
    };

[MattSturgeon]

The V2 check&merge exists so that a merge can return additional metadata and/or type-checking errors alongside the actual merge result.

Yes, the dagEntryOf type should use the submodule's v2 check&merge so that it can propagate anything it has returned.

IIRC, v2-compliant check functors should eval self.merge.v2 and check for errors in the result.

IIRC, complying with V2 isn't a strict requirement for all types, but the new assertion you've run into exists to catch cases where errors returned by merge.v2 would be silently ignored.

You probably want to take a look at how nixpkgs types handle v2 check&merge, specifically in cases where composite types are wrapping elem-types.

(Sorry I can't be more specific or link to definitive examples, I'm reviewing from my phone)