Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib/attrsets: merge concatMapAttrs recursively #206965

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SuperSandro2000
Copy link
Member

This makes it possible to drop name and just inherit values.

concatMapAttrsRecursive (name: value: { inherit (value) a b; }) { foo = { a = { q = null; }; b = { q = null;  }; bar = { a = { p = null;  }; b = { p= null; };};};}

Before the above only returned { a = { q = null; }; b = { q = null; }; } and with that change p is also included.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@SuperSandro2000 SuperSandro2000 requested review from roberth, figsoda and infinisil and removed request for edolstra, nbp and infinisil December 20, 2022 02:44
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Dec 20, 2022
@figsoda
Copy link
Member

figsoda commented Dec 20, 2022

I would suggest having an additional concatMapAttrsRecursive function instead

@roberth
Copy link
Member

roberth commented Dec 20, 2022

Also skip ahead to the concatMapAttrsRecursiveUntil variant, because recursive merging/updating without knowing the meaning of the attrsets is bad.
For example, you might accidentally merge packages, where an attribute that's unique to the lower package ends up in the end result that's mostly the upper package. Good luck figuring out that bug.

This makes it possible to drop name and just inherit values.

concatMapAttrsRecursive (name: value: { inherit (value) a b; }) { foo = { a = { q = null; }; b = { q = null; }; }; bar = { a = { p = null;  }; b = { p= null; };};}
@SuperSandro2000 SuperSandro2000 force-pushed the concatMapAttrs-merge-recursive branch from f8a1b85 to a5d9627 Compare December 22, 2022 02:27
@SuperSandro2000
Copy link
Member Author

Added the recursive variant, don't really have the time to do a until variant right now.

/* Same as concatMapAttrs but merges the attrsets recurisvely.

Type:
concatMapAttrs ::
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
concatMapAttrs ::
concatMapAttrsRecursive ::

@infinisil
Copy link
Member

What are the use cases for such a function?

@roberth
Copy link
Member

roberth commented Dec 22, 2022

I take back my previous comment. A truly recursive concatMapAttrs would not just merge one layer of values recursively, but merge each layer of values recursively. This is how I misunderstood this PR.

Instead of recursion, the new function seems to be about combining attribute values instead of picking one, so I think zipWithAttrs is the appropriate alternative to a "recursive" concatMapAttrs.
Perhaps this alternative could be documented in concatMapAttrs.

@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict This PR has merge conflicts with the target branch labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 4, 2024
@drupol drupol changed the title lib/attrsets: merge concatMapAttrs recurisvely lib/attrsets: merge concatMapAttrs recursively Sep 27, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 27, 2024
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Sep 27, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants