Skip to content

maintainers: require GitHub handle (enforce via CI)#437085

Merged
wolfgangwalther merged 1 commit into
NixOS:masterfrom
wolfgangwalther:ci-maintainers
Sep 2, 2025
Merged

maintainers: require GitHub handle (enforce via CI)#437085
wolfgangwalther merged 1 commit into
NixOS:masterfrom
wolfgangwalther:ci-maintainers

Conversation

@wolfgangwalther

@wolfgangwalther wolfgangwalther commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

At the scale of Nixpkgs, actively maintaining a package is only possible with integration into CI. To be able to be pinged for review requests, the maintainer must have a GitHub handle, which:

  • leads to an invitation to the NixOS org, which comes with additional privileges,
  • allows to request the maintainer for review as a member of this org, and
  • automatically requests the maintainer for review in CI.

Currently, the GitHub handle is not strictly enforced. This leads to some new maintainers accidentally forgetting to set these. We can avoid these mistakes and enforce them via CI.

We currently have 3 maintainers without GitHub handle, according to my last run of maintainers/scripts/fix-maintainers.pl:

These will have to be resolved first, before the tests will start passing.

Note: I am aware of the closed RFC 167. I don't think we need an RFC for this nowadays, if somebody insists they could raise the issue to the SC.

Closes #437407

Things done


Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 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 This PR causes 1 package to rebuild on Linux. 6.topic: lib The Nixpkgs function library labels Aug 26, 2025
@nix-owners nix-owners Bot requested review from hsjobeki and infinisil August 26, 2025 11:28
@SigmaSquadron

SigmaSquadron commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

sigh

This is a step in the wrong direction, but given we'll likely not migrate off GitHub anytime soon, I guess we might as well increase our reliance in Microsoft's platform.

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

This is a step in the wrong direction, but given we'll likely not migrate off GitHub anytime soon, I guess we might as well increase our reliance in Microsoft's platform.

I feel exactly the same way. Also, making this a requirement or not doesn't change the practical relevance for a theoretical migration off of GitHub - if we were to do that, we'd need to migrate all these GitHub handles to something else anyway. This PR is not about "using more GitHub", but about making whatever the current primary (only!) channel for notifications is, a required thing. If we migrate, we'd make whatever the replacement for the github handle is, required.

@acid-bong

This comment was marked as resolved.

@wolfgangwalther

This comment was marked as resolved.

@iedame

iedame commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

Sounds fair. Given our reliance on GitHub CI, this is the logical path forward.

@philiptaron

This comment was marked as off-topic.

@nixpkgs-ci nixpkgs-ci Bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 26, 2025

@pbsds pbsds left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 from me

Although i don't like how this is essentially vendor lock-in, we can always change the requirements in the future.

Prior art:

@pbsds

pbsds commented Aug 26, 2025

Copy link
Copy Markdown
Member

There's another question I've been mulling: how do we ensure coherence between the GitHub ID and the name? I've been doing spot checks using gh api /user/<id> and seeing that at the time of commit, the name matches what's in the maintainer list and is the ID on the PR, but because this is under the user's control asynchronously with everything, it feels quite fragile to me.

Do you have any thoughts on how to bulk up the testing here?

This is a case of the githubId having a typo: #264035

I think the best approach is to consider the githubId the source of truth, and to verify in CI that the name and id match when a new maintainer entry is added.

@philiptaron

This comment was marked as resolved.

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

CI currently uses the GitHub IDs, then fetches names for these, then pings these names.

I think it's good to keep the ID, because it's an impossible-to-change source of truth. For example I removed an old maintainer in #437078, where the same GitHub name had meanwhile been taken by a different account. We should certainly keep the ID for these cases.

I would like to find a way to keep the names mostly up2date, though, so that we don't need to make 1 API request per maintainer to fetch the right name on every pull request. My first try was #437078 in running the script that is already there, but we probably need to do more than that. With so many maintainers, these will be outdated quickly. Maybe we can fully automate these kinds of updates.

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

Prior art:

It should be noted that this previous proposal of the same thing had 9 committer approvals already. I don't want to open a huge discussion around this again, but we should take that into consideration should we be able to get to a position where we can technically make the change without breaking things.

@zeuner

zeuner commented Aug 26, 2025

Copy link
Copy Markdown
Contributor

This is a step in the wrong direction, but given we'll likely not migrate off GitHub anytime soon, I guess we might as well increase our reliance in Microsoft's platform.

I agree that it's a step in the wrong direction, but not with the implication of doing it anyway. As of 2025, reducing sovereignty seems rather anachronistic, and will in fact make it harder to migrate off.

In fact, the PR seems to be based on a couple of misconceptions:

A GitHub handle is not required to ping maintainers. On various issues, I was able to ping maintainers easily by e-mail using a simple script (https://codeberg.org/gm6k/nix-maintainer-tooling/src/branch/main/mail-maintainers.nix) and a pipeline-capable MTA. This might be hacky, but I'm also available to contribute a more robust solution if there is a lack of developers who are familiar with non-proprietary communication protocols. In fact, I started on this some while ago (NixOS/ofborg#665), but the reviewing and merging was slower that the underlying infrastructure being changed.

Also, it seems inappropriate to presume carelessness ("maintainers accidently forgetting to set these") in maintainers who decide that a communication channel offering more control (e.g. mail or matrix) works better for them. Providing a GitHub handle was purposefully made optional. With respect to the documented maintainer file format, strictly enforcing a GitHub handle should be considered a bug.

The current policy seems to be that people are more important than code (#356607 (comment)), so we cannot consider maintainers (= human beings) to be issues "to be resolved first".

If the different channels for notifications that are implied by the documented maintainer file format have become so invisible that people start thinking only GitHub exists, let's streamline them and make them more visible, rather than hunting down those who use them. We'll also get a more resilient ecosystem by doing so.

@zeuner zeuner mentioned this pull request Aug 26, 2025
1 task
@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

A GitHub handle is not required to ping maintainers.

In theory it is not. In practice it is, because at the scale we're operating we can't ping maintainers manually and we don't have automation for anything else.

I was able to ping maintainers easily by e-mail

To implement maintainer pings by email, we need to:

  • not send these from within GitHub Actions, I'm sure neither GHA likes that, nor the receiving side does. I sure don't want to have the NixOS org accidentally be classified as sending spam.
  • implement a mechanism for email verification first. When an email address is added to a PR, it needs to be verified to belong to the user submitting it first. Otherwise we risk sending spam to users who didn't actually sign up for that. Or we risk sending to wrong addresses and the email never making it to the user.

This is actually quite a non-trivial system to implement, especially considering how the email address verification state should be stored. I can imagine some kinds of solutions that would require some kind of cryptographic signature, only sent via email, which can then be stored in git. Or.. we'd need to store that verification state in a separate database. Oh, wait, now we're starting to roll our own identity provider.

We already have one, though: It's GitHub. GitHub does exactly that for us. It verifies email addresses and allows us to securely send to them.

It's entirely unreasonable to request building such a system to be able to ping 3 out of 4583 maintainers. It is much more reasonable to require that everyone supports the primary notification channel.

As of 2025, reducing sovereignty seems rather anachronistic, and will in fact make it harder to migrate off.

As I said above, this will not make it any harder to migrate off in practice.

Also note: I do share you concern about sovereignty and would love to see us move off of GitHub in general. I have actually worked towards better support for that in the last couple of months as well: You can run nix-build ci in your local nixpkgs checkout and get almost all of the tests that CI runs locally now as well. I intend to do more, too - my vision is to have CI split into two separate parts, where one is forge independent and the other is GitHub specific. This would allow us to "just" replace the specific part on a move, but keep all the forge independent stuff.

But there will always be forge specific code for CI, no matter which platform we are using. And I'm not shy to use that either to give us the best integration with the platform we are currently using that we can have. This platform is currently GitHub, that's a fact. And notifications are a core feature of whatever platform we use.

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

Also let's not forget one other important fact: Having a GitHub handle in the maintainer entry is not just about notifications. It's also about joining the NixOS org and having privileges. We expect maintainers to be able to deal with issues about their packages. We expect them to be able to close issues that are about their packages, but not relevant or already solved etc. To a certain degree we also hope to have maintainers help with other issues, too, helping to manage this huge backlog of stuff.

Once a GitHub handle is added to a maintainer entry, they will be automatically invited to the NixOS org and will automatically receive these permissions when accepting the invitation.

This is important and can only be done with an associated GitHub handle.

@SigmaSquadron

SigmaSquadron commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

It's entirely unreasonable to request building such a system to be able to ping 3 out of 4583 maintainers. It is much more reasonable to require that everyone supports the primary notification channel.

Precisely. We shouldn't make our lives more difficult for a tiny minority that is still using GitHub anyway. I'm not sure why this was even a discussion in the first place. The options here seem pretty clear, and this should not be blocked further.

If we ever move off GitHub, NixOS' own identity provider would render all of this obsolete. Unfortunately, we're not there yet, so it's a bit of a moot point.

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Aug 27, 2025
@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

I would like to separate the decision about whether we are going to do this from enforcing this via CI. This PR enforces it via CI. We essentially have already agreed on the fact that we want to do this, by big majority, taking the previous discussions and this PR together. Thus, I opened #437469 to put this decision down into our guidelines and documentation.

As mentioned over there, to not split discussion among different places, please keep any discussion about "do we want to do this at all" in here. Before posting any new arguments, please make sure to go through the arguments already presented here and in #273220, especially the nice summary in #273220 (comment). It only makes sense to continue this discussion with new input, not bringing up everything that has been discussed already.

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

There are now PRs for all 3 existing maintainer handles without github information:

MattSturgeon

This comment was marked as resolved.

@wolfgangwalther

This comment was marked as resolved.

@zeuner

zeuner commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

A GitHub handle is not required to ping maintainers.

In theory it is not. In practice it is, because at the scale we're operating we can't ping maintainers manually and we don't have automation for anything else.

This is hardly a surprise as long as every PR which implements GitHub-related logic gets merged and every PR which would head for more forge-independent solutions gets closed. We can change this situation every day.

I was able to ping maintainers easily by e-mail

To implement maintainer pings by email, we need to:

* not send these from within GitHub Actions, I'm sure neither GHA likes that, nor the receiving side does. I sure don't want to have the NixOS org accidentally be classified as sending spam.

* implement a mechanism for email verification first. When an email address is added to a PR, it needs to be _verified to belong to the user submitting it_ first. Otherwise we risk sending spam to users who didn't actually sign up for that. Or we risk sending to wrong addresses and the email never making it to the user.

I don't think FUD will do us any good here. It's the responsibility of the maintainers to keep their maintainer entry up to date, and to ensure that the contact methods given there are reachable. When sending mail to entries in maintainer-list.nix in good faith, the risk should be negligible. Should there be persistent bounces or even complaints, this would obviously be a reason to flag or delete the corresponding e-mail entry. AFAIK, something similar was done when contacting SC voters.

Personally, I use e-mail as the primary pinging methods when posting issues. I never received a complaint, nor has my hoster contacted me about any.

This is actually quite a non-trivial system to implement, especially considering how the email address verification state should be stored. I can imagine some kinds of solutions that would require some kind of cryptographic signature, only sent via email, which can then be stored in git. Or.. we'd need to store that verification state in a separate database. Oh, wait, now we're starting to roll our own identity provider.

Of course we could also build more sophisticated verification. I think this should be opt-in, considering that it's the responsibility of the maintainers if they want to be contacted by e-mail.

Before implementing anything new, maybe the NixOS discourse can help. With consent given there, it should be no problem to use it for verification of e-mail addresses before sending. Or we finally add discourse handles to maintainer-list.nix...

We already have one, though: It's GitHub. GitHub does exactly that for us. It verifies email addresses and allows us to securely send to them.

It's entirely unreasonable to request building such a system to be able to ping 3 out of 4583 maintainers. It is much more reasonable to require that everyone supports the primary notification channel.

Monoculture also means having a single point of failure. And, speaking for me, GitHub has repeatedly proven unreliable for me, so I want to give a notification channel over which I have enough control to make sure it will work properly.

I'm not sure who "requested building such a system". For me, it would fully suffice if PRs heading to build a more sovereign system wouldn't be closed so often.

As of 2025, reducing sovereignty seems rather anachronistic, and will in fact make it harder to migrate off.

As I said above, this will not make it any harder to migrate off in practice.

I disagree. The assumption is based on one specific migration scenario, which might not even be realistic.

Also note: I do share you concern about sovereignty and would love to see us move off of GitHub in general. I have actually worked towards better support for that in the last couple of months as well: You can run nix-build ci in your local nixpkgs checkout and get almost all of the tests that CI runs locally now as well. I intend to do more, too - my vision is to have CI split into two separate parts, where one is forge independent and the other is GitHub specific. This would allow us to "just" replace the specific part on a move, but keep all the forge independent stuff.

That's great to hear.

But there will always be forge specific code for CI, no matter which platform we are using. And I'm not shy to use that either to give us the best integration with the platform we are currently using that we can have. This platform is currently GitHub, that's a fact. And notifications are a core feature of whatever platform we use.

I can't fully agree here, either. IMHO, notifications are even more central to a resilient system than any forge it may use. In the (hopefully unlikely) case that the forge fails for a longer time, I think any users should, for each component, still be able to contact a person that's familiar with it (the maintainer). It's fine if there is also GitHub, but this doesn't make the rest obsolete, and it doesn't justify to restrict other communication methods more and more.

@infinisil

infinisil commented Sep 2, 2025

Copy link
Copy Markdown
Member

@zeuner Among the people with commit access, everybody who was asked has agreed that we should do this. This is so uncontroversial among committers that there isn't even any justification for involving the Steering Committee. Unless a number of committers come out against this, the decision is settled. It's time to stop arguing now (not just here, but also in #439407, #437469 (comment) and in your revert PR: #439389) and move on to focus on other things.

@SigmaSquadron SigmaSquadron mentioned this pull request Sep 2, 2025
13 tasks
SigmaSquadron added a commit to SigmaSquadron/nixpkgs that referenced this pull request Sep 2, 2025
Although this maintainer has responded to requests for maintainance and
is active in Nixpkgs, the new data collection requirements introduced in
NixOS#437085 have been a privacy
concern for this maintainer, who has stated (NixOS#437082 (comment))
that they do not wish for their data to be added to the maintainer list.

For this reason, there is no other recourse than to remove this
maintainer's information from newer revisions of Nixpkgs, as their
maintainer entry is now non-compliant with Nixpkgs' latest policies.
Once more, this removal is NOT due to the usual inactivity reasons, but
for specific privacy concerns expressed by the maintainer being removed.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
At the scale of Nixpkgs, actively maintaining a package is only possible
with integration into CI. To be able to be pinged for review requests,
the maintainer must have a GitHub handle, which:
- Leads to an invitation to the NixOS org, which comes with additional
privileges.
- Allows to request the maintainer for review as a member of this org.
- Automatically requests the maintainer for review in CI.

Currently, the GitHub handle is not strictly enforced. This leads to
some new maintainers accidentally forgetting to set these. We can avoid
these mistakes and enforce them via CI.
@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

Just rebased after all 3 PRs / maintainer entries have been handled - CI should now pass. Will now backport the maintainer changes and then merge & backport this one.

SigmaSquadron added a commit to SigmaSquadron/nixpkgs that referenced this pull request Sep 2, 2025
Although this maintainer has responded to requests for maintainance and
is active in Nixpkgs, the new data collection requirements introduced in
NixOS#437085 have been a privacy
concern for this maintainer, who has stated (NixOS#437082 (comment))
that they do not wish for their data to be added to the maintainer list.

For this reason, there is no other recourse than to remove this
maintainer's information from newer revisions of Nixpkgs, as their
maintainer entry is now non-compliant with Nixpkgs' latest policies.
Once more, this removal is NOT due to the usual inactivity reasons, but
for specific privacy concerns expressed by the maintainer being removed.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>

(cherry picked from commit 81d1a3a)
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
@SigmaSquadron

SigmaSquadron commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Oh, I was backporting gm6k already. (#439452)

@wolfgangwalther

Copy link
Copy Markdown
Contributor Author

Thanks for the overwhelming support everyone. It will surely take a while until I get to merge a PR with 8 approvals again...

@wolfgangwalther wolfgangwalther merged commit 9dffc88 into NixOS:master Sep 2, 2025
28 of 31 checks passed
@wolfgangwalther wolfgangwalther deleted the ci-maintainers branch September 2, 2025 08:09
@nixpkgs-ci

nixpkgs-ci Bot commented Sep 2, 2025

Copy link
Copy Markdown
Contributor

Successfully created backport PR for release-25.05:

@github-actions github-actions Bot added the 8.has: port to stable This PR already has a backport to the stable release. label Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: lib The Nixpkgs function library 8.has: port to stable This PR already has a backport to the stable release. 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. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.