From e53ca90334ecbd84752f38fd328f077cbe7e6c6a Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Mon, 15 Dec 2025 23:27:12 +0100 Subject: [PATCH] lib.teams: Collect all errors for missing lib.maintainers entries In https://github.com/NixOS/nixpkgs/pull/471116#pullrequestreview-3580366406 we had two users without maintainer entries, while CI only showed one --- maintainers/computed-team-list.nix | 103 +++++++++++++++++------------ 1 file changed, 60 insertions(+), 43 deletions(-) diff --git a/maintainers/computed-team-list.nix b/maintainers/computed-team-list.nix index 7ee37f4283e3..543a6d216d37 100644 --- a/maintainers/computed-team-list.nix +++ b/maintainers/computed-team-list.nix @@ -12,48 +12,65 @@ let ]; maintainerSetToList = - githubTeam: - lib.mapAttrsToList ( - login: id: - maintainersById.${toString id} - or (throw "lib.teams: No maintainer entry for GitHub user @${login} who's part of the @NixOS/${githubTeam} team") - ); - -in -# TODO: Consider automatically exposing all GitHub teams under `lib.teams`, -# so that no manual PRs are necessary to add such teams anymore -lib.mapAttrs ( - name: attrs: - if attrs ? github then + githubTeam: users: let - githubTeam = - githubTeams.${attrs.github} - or (throw "lib.teams.${name}: Corresponding GitHub team ${attrs.github} not known yet, make sure to create it and wait for the regular team sync"); + missingUsers = lib.filter (login: !maintainersById ? ${toString users.${login}}) ( + lib.attrNames users + ); in - # TODO: Consider specifying `githubId` in team-list.nix and inferring `github` from it (or even dropping it) - # This would make renames easier because no additional place needs to keep track of them. - # Though in a future where all Nixpkgs GitHub teams are automatically exposed under `lib.teams`, - # this would also not be an issue anymore. - assert lib.assertMsg (!attrs ? githubId) - "lib.teams.${name}: Both `githubId` and `github` is set, but the former is synced from the latter"; - assert lib.assertMsg (!attrs ? shortName) - "lib.teams.${name}: Both `shortName` and `github` is set, but the former is synced from the latter"; - assert lib.assertMsg ( - !attrs ? scope - ) "lib.teams.${name}: Both `scope` and `github` is set, but the former is synced from the latter"; - assert lib.assertMsg ( - !attrs ? members - ) "lib.teams.${name}: Both `members` and `github` is set, but the former is synced from the latter"; - attrs - // { - githubId = githubTeam.id; - shortName = githubTeam.name; - scope = githubTeam.description; - members = - maintainerSetToList attrs.github githubTeam.maintainers - ++ maintainerSetToList attrs.github githubTeam.members; - githubMaintainers = maintainerSetToList attrs.github githubTeam.maintainers; - } - else - attrs -) teams + if missingUsers == [ ] then + lib.mapAttrsToList (login: id: maintainersById.${toString id}) users + else + { + errors = map ( + login: + "\n- No maintainer entry for GitHub user @${login} who's part of the @NixOS/${githubTeam} team" + ) missingUsers; + }; + + # TODO: Consider automatically exposing all GitHub teams under `lib.teams`, + # so that no manual PRs are necessary to add such teams anymore + result = lib.mapAttrs ( + name: attrs: + if attrs ? github then + let + githubTeam = + githubTeams.${attrs.github} + or (throw "lib.teams.${name}: Corresponding GitHub team ${attrs.github} not known yet, make sure to create it and wait for the regular team sync"); + maintainers = maintainerSetToList attrs.github githubTeam.maintainers; + nonMaintainers = maintainerSetToList attrs.github githubTeam.members; + in + # TODO: Consider specifying `githubId` in team-list.nix and inferring `github` from it (or even dropping it) + # This would make renames easier because no additional place needs to keep track of them. + # Though in a future where all Nixpkgs GitHub teams are automatically exposed under `lib.teams`, + # this would also not be an issue anymore. + assert lib.assertMsg (!attrs ? githubId) + "lib.teams.${name}: Both `githubId` and `github` is set, but the former is synced from the latter"; + assert lib.assertMsg (!attrs ? shortName) + "lib.teams.${name}: Both `shortName` and `github` is set, but the former is synced from the latter"; + assert lib.assertMsg ( + !attrs ? scope + ) "lib.teams.${name}: Both `scope` and `github` is set, but the former is synced from the latter"; + assert lib.assertMsg ( + !attrs ? members + ) "lib.teams.${name}: Both `members` and `github` is set, but the former is synced from the latter"; + if !maintainers ? errors && !nonMaintainers ? errors then + attrs + // { + githubId = githubTeam.id; + shortName = githubTeam.name; + scope = githubTeam.description; + members = maintainers ++ nonMaintainers; + githubMaintainers = maintainers; + } + else + maintainers.errors or [ ] ++ nonMaintainers.errors or [ ] + else + attrs + ) teams; + + errors = lib.concatLists ( + lib.mapAttrsToList (name: value: value) (lib.filterAttrs (name: lib.isList) result) + ); +in +if errors == [ ] then result else throw ("lib.teams:" + lib.concatStrings errors)