Also good point, would this be able to make specifying teams on the website better at https://nixos.org/community/#governance-teams?

After some minor changes, I consider this ready!

Further follow-ups:

• [Reverted] First-class GitHub team reviews #456341
• lib.teams: Sync matching teams from GitHub #456345
• [Tracking] Nixpkgs GitHub teams should match lib.teams #456365

Backport not needed because the workflow runs on a schedule, which only trigger on the default branch.

No, that's the wrong conclusion. We backport every change to CI, otherwise we will hit merge conflicts later, for example for automated dependabot PRs etc.

This should be backported as well.

We could consider whether the team sync and the weekly PRs should all get backported though.

And yes, I think we should do that, too. That's because requesting reviewers always works with the target branch's data, so we want up2date data on the stable branches, too. That means we need to backport the sync, which in turn means we need to backport everything leading up to it.

@infinisil are you working on the backport or should i?

@wolfgangwalther Not working on the backport right now, but I could later today

I'm working on the backport right now. Need to backport a few maintainer additions, too...

Backport in #456422.

After fixing #450864 (comment) and manually triggering it, we've got the first automated sync PR: #456477 :D

This makes CI fail for me in #455942, because the maintainer I am proposing to drop is a member of a team, and I remove their maintainers entry.

This makes sense, and the error message was even descriptive of the actual problem! But I'm not really sure what the solution is. Should I ping an org owner, or someone listed in the "maintainers" field of maintainers/github-teams.json? When should I do so: now (during the one-week waiting period), or later? Would they be allowed to remove membership before the waiting period is up? And will we need to wait (in the worst case) a second week for the teams to sync, or is manually running the sync workflow considered to be a reasonable option?

Ideally, I think the answers to these would be addressed in the error messages? I only knew to "blame" this because I had happened to see this PR (although I'm sure grepping for the error message would have gotten me here too), and I only noticed the significance of the "maintainers" field on the second read of this PR.

Should I ping an org owner, or someone listed in the "maintainers" field of maintainers/github-teams.json?

Pinging the most locally relevant person is always best (in line with our distributed decision-making value!), so yeah team maintainer first, and then to the Nixpkgs core team, and then to org owners.

When should I do so: now (during the one-week waiting period), or later? Would they be allowed to remove membership before the waiting period is up?

I'd only do it after the one-week waiting period, it shouldn't be removed before that.

And will we need to wait (in the worst case) a second week for the teams to sync, or is manually running the sync workflow considered to be a reasonable option?

Yeah manually running sounds totally fine, all committers can do that and it won't break anything

Ideally, I think the answers to these would be addressed in the error messages? I only knew to "blame" this because I had happened to see this PR (although I'm sure grepping for the error message would have gotten me here too), and I only noticed the significance of the "maintainers" field on the second read of this PR.

Sounds like a good idea, PR appreciated! Note that the error can occur both when removing a maintainer or when a GitHub user is added to a team without having a maintainer entry, so the error message would need to present both options.

Another option is also to remove the maintainer by hand from the synced file, so that CI in the "drop the maintainer" PR is happy. Once this is merged, the weekly sync will try to bring back the maintainer, at which point the org owner can clean this up.

(this is a bit annoying, because effectively only org owners can merge the weekly sync that way...)

We might have just hit a serious limitation of treating GitHub as the source of truth...

I didn't really see this before, but I do now: With this sync from GitHub to Nixpkgs, we are changing the way how maintainers and teams are managed massively: Where ~200 committers were able to make a call on an addition to a team or the removal of a maintainers before, this is (in some cases) now reduced to a few people only: Those marked as maintainers of the GitHub team, or org owners as fallback. A lot of teams don't even have these maintainers.

This creates a practical bottleneck on one side - and on the other suddenly makes every team invite-only (aka requiring approval of an existing team member), where before we only had this for a handful of selected teams (mostly company teams), mentioned explicitly.

I don't have a solution at hand immediately, but I do see this as a significant problem.

Another issue that @emilazy pointed out: There is no clear process for addition of new members to teams anymore. Before, this required a pull request to the team-list, which allowed for discussion and a paper trail. Now? I'm not even sure. Do I just need to reach out to an existing maintainer of a team in some way, and they will just make a decision / add me ad-hoc on the GitHub side?

This seems wrong.

We will need to think about our workflows / processes for these cases - and then see whether we can still stick with the idea of syncing GitHub->Nixpkgs, I think.

I wonder about flipping the logic to be more in line with the rfc39 approach? GitHub becomes the source-of-truth, and periodic team syncs (invites/removals) are done by the rfc39 bot.

I'm sure we discussed this before, but OTTOMH I can't remember why we decided against this approach.

I wonder about flipping the logic to be more in line with the rfc39 approach? GitHub becomes the source-of-truth, and periodic team syncs (invites/removals) are done by the rfc39 bot.

I'm sure we discussed this before, but OTTOMH I can't remember why we decided against this approach.

This was one of the options considered in #447514 (comment) (and following), but I argued against it in #447514 (comment). My main arguments were security and consistency with a potential move away from rfc39 for different reasons.

Now, I guess we can address the problems mentioned in that comment that currently exist with the rfc39 implementation in a different way, too. The security aspect still makes me very uneasy about a nixpkgs -> github migration, though. It's rather simple to add members to precisely one team (nixpkgs-maintainers) as rfc39 currently does. But to migrate team membership, we need to be very careful on which teams we can migrate memberships to, so that we don't accidentally allow privilege escalating, for example by merging members into the nix team or security team or core team (especially the latter has a lot of privileges!), ...

AFAIK we can't provide "scoped" privileges for the org-wide team maintenance token, that would be used for that. Although I'm not sure whether GitHub Enterprise could solve that?

Where ~200 committers were able to make a call on an addition to a team or the removal of a maintainers before, this is (in some cases) now reduced to a few people only: Those marked as maintainers of the GitHub team, or org owners as fallback.

This is only a problem if we ignore how lib.teams should stay in sync with the linked GitHub team. Because before this change, you'd need both a Nixpkgs committer and a GitHub team maintainer to make a synchronised team change, whereas now you only need a GitHub team maintainer.

A lot of teams don't even have these maintainers.

As listed in #456365, there's just 5 real teams that don't have a maintainer. But this was also a problem before if we care about teams being in sync. Once this is addressed for those teams, we can introduce an assert that ensures all teams have at least one maintainer, so this would be detected in the weekly sync going forward.

A related problem (though also not introduced by this PR) is that just having a GitHub team maintainer doesn't mean they're actually maintaining the team in practice. For that I suggest creating an issue that explains how to reach out to team maintainers and to encourage commenting there if they're unresponsive, in which case a Nixpkgs core member / org owner can assign a new maintainer.

The only issue I see this PR affecting is the desire for currently lib.teams-only teams to have a synced GitHub team, which would change them from Nixpkgs committer-maintained to self-maintained. While you could see it as a bottleneck, I see it as a way of empowering non-committers to take ownership of and maintain teams independently, putting less unnecessary¹ burden on Nixpkgs committers.