Skip to content

lib: add lib.attrsets.collect'#342330

Open
h7x4 wants to merge 2 commits intoNixOS:masterfrom
h7x4:lib-add-collect-alt
Open

lib: add lib.attrsets.collect'#342330
h7x4 wants to merge 2 commits intoNixOS:masterfrom
h7x4:lib-add-collect-alt

Conversation

@h7x4
Copy link
Member

@h7x4 h7x4 commented Sep 16, 2024

Description of changes

This PR adds a new function lib.attrsets.collect'. Similarly to collect, it will recursively collect items that satisfy a predicate from an attrset tree, but this version will also track the attribute path to the items it collects.

This can be useful for collecting leaves in a tree, and either reporting detailed errors to the user, or for use in configuration file generation.

I've also added tests for the normal collect implementation, since it seemed to be missing.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@h7x4 h7x4 requested a review from roberth September 16, 2024 15:21
@h7x4 h7x4 requested a review from infinisil as a code owner September 16, 2024 15:21
@github-actions github-actions bot added the 6.topic: lib The Nixpkgs function library label Sep 16, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 16, 2024
@infinisil
Copy link
Member

Can you tell me a bit more about the use case of this? Would be great to have some links to code where this could be used :)

@h7x4
Copy link
Member Author

h7x4 commented Oct 26, 2024

Can you tell me a bit more about the use case of this? Would be great to have some links to code where this could be used :)

I imagine it would be most useful for collecting leaf nodes for freeform trees. The current usecase I have for this is #342372, where I want to collect warnings and assertions from a freeform tree and include the names of the warnings / assertions that trigger in the error message. It could be used for similar purposes in places where we already use lib.collect, for providing better error messages or just transforming a settings tree into a list of paths. I unfortunately don't have any other concrete examples.

@h7x4 h7x4 requested a review from hsjobeki March 31, 2025 10:31
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
@h7x4 h7x4 force-pushed the lib-add-collect-alt branch from 6773c45 to 4d27868 Compare April 4, 2025 10:45
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 4, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Apr 4, 2025
@h7x4 h7x4 force-pushed the lib-add-collect-alt branch from 4d27868 to bbf2f01 Compare April 4, 2025 10:56
Copy link
Contributor

@hsjobeki hsjobeki left a comment

Choose a reason for hiding this comment

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

I think #237776 shouldn't block this. Since it only adds path as argument to the collect function.
collect' isn't a great name. Maybe we should name it collectAttrs or collectPath similar to mapAttrs and filterAttrs.

I like moving forward with this unless we have a clear collision with #237776 .

Copy link
Contributor

@hsjobeki hsjobeki left a comment

Choose a reason for hiding this comment

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

Hm after thinking about #395160
These are kind of duplicates of each other.
The only major difference is that the other PR has a value function f to produce the result.
Also it has a slight difference in the recursion flow.

@hsjobeki hsjobeki self-requested a review April 4, 2025 13:49
hsjobeki

This comment was marked as duplicate.

@hsjobeki hsjobeki self-requested a review April 4, 2025 13:51
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 25, 2025
@nixpkgs-ci nixpkgs-ci bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: lib The Nixpkgs function library 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants