build03: add rfc39

[zowoq]

No description provided.

[zowoq]

Okay, this seems to work, I tried a dry run of the bot on the stylix repo and it attempted to add people to @nix-community/stylix-maintainers.

@danth @khaneliman The bot reports errors via prom metrics but the way it is implemented is rather awkward and frankly isn't worth the hassle (NixOS/rfc39#14). Even if I do wire it to our monitoring and route a notification to you all it will tell you is that it started failing, nothing more specific. The logs have more detail but I can't make them public as it can leak the bots secrets.

https://github.com/NixOS/nixpkgs/blob/master/lib/tests/maintainers.nix

I think you're going to need to run the nixpkgs maintainers validation checks as a PR check in your repos to catch errors.

[Mic92]

Can merge fixes upstream in rfc39 btw.

[zowoq]

Seems incorrect quoting will break it completely.

called `Result::unwrap()` on an `Err` value: Serde(Error("invalid type: string \"179992797\", expected u64", line: 1, column: 81))

diff --git a/modules/lib/maintainers.nix b/modules/lib/maintainers.nix
index 4684ad38..69e428cb 100644
--- a/modules/lib/maintainers.nix
+++ b/modules/lib/maintainers.nix
@@ -11,7 +11,7 @@
     name = "Eulalia del Sol";
     email = "3ulalia@proton.me";
     github = "3ulalia";
-    githubId = "179992797";
+    githubId = 179992797;
   };
   "9p4" = {
     name = "9p4";
@@ -268,7 +268,7 @@
   henrisota = {
     email = "henrisota@users.noreply.github.com";
     github = "henrisota";
-    githubId = "56848082";
+    githubId = 56848082;
     name = "Henri Sota";
   };
   hey2022 = {
@@ -558,7 +558,7 @@
   msfjarvis = {
     email = "me@msfjarvis.dev";
     github = "msfjarvis";
-    githubId = "13348378";
+    githubId = 13348378;
     name = "Harsh Shandilya";
     keys = [
       {

Maintainers missing information who are skipped.

WARN Missing GitHub ID, github_account: hey2022, exec-mode: CheckHandles, module: rfc39::op_check_handles:17
ERRO Missing GitHub Account, but ID present, github_id: 43591752, who: jess, exec-mode: CheckHandles, module: rfc39::op_check_handles:21
WARN Missing GitHub ID, github_account: silmarp, exec-mode: CheckHandles, module: rfc39::op_check_handles:17

[zowoq]

I didn't notice that the maintainer lists are deduplicated with nixpkgs maintainers, rfc39 only works for maintainers that are listed in the file. Would need to add an entry in the maintainer file, e.g. inherit (pkgs.lib.maintainers) zowoq; for nixpkgs maintainers. @danth @khaneliman Is that feasible or do we need to look for a different method of doing the invites?

[MattSturgeon]

Would need to add an entry in the maintainer file, e.g. inherit (pkgs.lib.maintainers) zowoq; for nixpkgs maintainers.

I don't think that'll scale very well... and currently the maintainers list in most projects is a plain attrset, with no access to lib.

On the one hand, maybe it would be nice to explicitly list all project maintainers; either literally or inherited from nixpkgs.

On the other hand, that sounds like a lot of unnecessary boilerplate.

Is there any way we can extend the rfc39 impl to accept multiple maintainer lists? E.g. something like:

rfc39 \
    --maintainers ${./maintainers.nix} \
    --maintainers ${nixpkgs}/maintainers/maintainer-list.nix

?

[zowoq]

rfc39 \
    --maintainers ${./maintainers.nix} \
    --maintainers ${nixpkgs}/maintainers/maintainer-list.nix

Using the nixpkgs maintainer list will invite everyone from nixpkgs.

rfc39 \
    --maintainers ${./maintainers.nix} \
    --maintainers ${./maintainers-nixpkgs.nix}

If you meant something like this, I don't see how that is any different from a single file? There will still be the boilerplate and both files will need a validation check, if a someone is removed from the upstream maintainer list in nixpkgs it'll break the downstream list and the rfc39 invites.

[MattSturgeon]

Ah I see. I was thinking of it kinda backwards.

In that case I think what we want is for projects that don't want to explicitly list all their maintainers to generate a list of their maintainers.

E.g.

let
  # Assume this configuration has a `meta.maintainer` option that maps filename→maintainer
  configuration = /* some module eval */;

  # Collect all the module maintainers
  maintainers = lib.pipe configuration.config.meta.maintainer [
    builtins.attrValues
    lib.unique
    (builtins.filter (m: m ? github))
    (map (m: lib.nameValuePair m.github m))
    builtins.listToAttrs
  ];
in

# return a JSON file
writers.writeJSON "maintainers.json" maintainers

I.e. it is up to the downstream project to provide a list of maintainers to invite, either using an in-repo file (as-per nixpkgs) or a generated file (as above)

[zowoq]

either using an in-repo file (as-per nixpkgs) or a generated file (as above)

I think generated file would need to be nix but yes, either would work. The downstream project would still need to ensure that the generated file passes the validation check.

[khaneliman]

Basically, we just need an exported file that contains all the maintainers that would possibly be pinged so that they can be invited to the nix-community team for unprivileged maintainers? That should be doable.

[MattSturgeon]

Could we make it so that a repo entry can some arbitrary command to run? Or some file to nix-instantiate --eval? Or some flakeref to nix eval?

Assuming you want an eval that spits out a nix value, rather than a build that builds a file.

E.g. nixvim might want something like nix-instantiate --eval ./.github/list-maimtainers.nix which would internally use flake-compat to load nixvim and would then scan an empty nixvimConfiguration for plugin maintainers.

I assume stylix would do something similar, scanning their modules/«module»/meta.nix files.

IIRC home-manager uses a similar meta.maintainers module to NixOS, so they would probably get their maintainers from an empty homeConfiguration.

[zowoq]

Downstream projects will already need to eval and validate the file to ensure that it isn't going to break, just check the file into the repo, don't make it more complicated then it needs to be by doing evals here as well. If multiple projects are going to be using this bot I want this side to be kept as simple as possible.

[MattSturgeon]

If the file has to be checked into the repo anyway, then I don't see any value in generating it dynamically.

---

However, does rfc39 even support evaluating a list that is actually a function?

I.e. this is different from what it is used to consuming from nixpkgs:

{ lib }:
{
  # ...
}

---

It would also be important to ensure the correct nixpkgs revision is used, in case a maintainer was recently added or removed.

In that case should downstream maintainer lists be reading their lockfile to download the correct nixpkgs?

Does rfc39 support a "real" nix eval of the maintainers list file? Would rfc39 support evaluating a "compatibility wrapper" that imports the real list internally?

# maintainers-list-compat.nix
let
  lockFile = builtins.fromJSON (builtins.readFile ../flake.lock);
  nixpkgs = builtins.fetchTarball {
    # ...
  };
  lib = import (nixpkgs + "/lib");
in
import ./maintainer-list.nix { inherit lib; }

[zowoq]

Again, this is making it more complicated then it needs to be and pushing potential problems upstream, I want the infra side to be as simple as possible.

[MattSturgeon]

Again, this is making it more complicated then it needs to be and pushing potential problems upstream, I want the infra side to be as simple as possible.

Maybe I wasn't clear - this would be downstream code.

I'm only asking if the rfc39 tool evaluates the maintainers-list file using nix (or lix, etc) or if it uses some other static analysis tool like rnix. I.e. whether it'd support evaluating a downstream wrapper expression that fetches the right nixpkgs and passes it to a maintainers list file internally.

If the file can't be evaluated "properly", then downstream projects can't inherit maintainers from nixpkgs lib.maintainers as suggested earlier.

[zowoq]

Sorry, seems I wasn't clear.

No wrappers, generate the file downstream and check it in to the repo.

If the file can't be evaluated "properly", then downstream projects can't inherit maintainers from nixpkgs lib.maintainers as suggested earlier.

Disregard this suggestion.

[zowoq]

This PR is waiting on a downstream repo having a generated and checked in maintainer list. Probably want the validation checks to ensure that the maintainer list is correct as well but I won't block on that.

[06kellyjac]

I can report it worked as I was invited to @nix-community/home-manager-maintainers this morning to my surprise 🙂