Unverified Commit 0a667643 authored by nixpkgs-ci[bot]'s avatar nixpkgs-ci[bot] Committed by GitHub
Browse files

Merge master into staging-nixos

parents 460a5098 1771826f
Loading
Loading
Loading
Loading
+8 −7
Original line number Diff line number Diff line
@@ -123,9 +123,7 @@ let
  # - values: lists of `packagePlatformPath`s
  diffAttrs = builtins.fromJSON (builtins.readFile "${combined}/combined-diff.json");

  changedPackagePlatformAttrs = convertToPackagePlatformAttrs diffAttrs.changed;
  rebuildsPackagePlatformAttrs = convertToPackagePlatformAttrs diffAttrs.rebuilds;
  removedPackagePlatformAttrs = convertToPackagePlatformAttrs diffAttrs.removed;

  changed-paths =
    let
@@ -160,11 +158,14 @@ let
      }
    );

  getMaintainers = callPackage ./maintainers.nix { };

  inherit
    (callPackage ./maintainers.nix {
      changedattrs = lib.attrNames (lib.groupBy (a: a.name) changedPackagePlatformAttrs);
      changedpathsjson = touchedFilesJson;
      removedattrs = lib.attrNames (lib.groupBy (a: a.name) removedPackagePlatformAttrs);
    (getMaintainers {
      affectedAttrPaths = map (a: a.packagePath) (
        convertToPackagePlatformAttrs (diffAttrs.changed ++ diffAttrs.removed)
      );
      changedFiles = lib.importJSON touchedFilesJson;
    })
    users
    teams
@@ -181,7 +182,7 @@ runCommand "compare"
    ];
    users = builtins.toJSON users;
    teams = builtins.toJSON teams;
    packages = builtins.toJSON packages;
    packages = builtins.toJSON (lib.map (lib.concatStringsSep ".") packages);
    passAsFile = [
      "users"
      "teams"
+89 −68
Original line number Diff line number Diff line
# Figure out which maintainers (users/teams) are relevant for a PR:
# - All maintainers that can be linked directly to changedFiles
# - Maintainers of affectedAttrPaths if a file directly related to the attribute is in changedFiles
#
# Files and attributes are linked in various ways:
# - pkgs/by-name/<attr>/* is linked to pkgs.<attr>
# - The file position of various attributes of pkgs.<attr>
# - Explicitly specified file positions in derivations
#
# Test with
#   nix-instantiate --eval --strict --json test.nix -A result | jq
#
# Empty list as an output means success

# Dependencies coming from the CI-pinned Nixpkgs
{
  lib,
  changedattrs,
  changedpathsjson,
  removedattrs,
}:
let
  pkgs = import ../../.. {
# Function arguments
{
  # Files that were changed
  # Type: ListOf (Nixpkgs-root-relative path)
  changedFiles,
  # Attributes whose value was affected by the change
  # Type: ListOf (ListOf String)
  affectedAttrPaths,
  # Nixpkgs used to check maintainers. Customisable for testing
  pkgs ? import ../../.. {
    system = "x86_64-linux";
    # We should never try to ping maintainers through package aliases, this can only lead to errors.
    # One example case is, where an attribute is a throw alias, but then re-introduced in a PR.
    # This would trigger the throw. By disabling aliases, we can fallback gracefully below.
    config.allowAliases = false;
  };
    overlays = [ ];
  },
}:
let
  nixpkgsRoot = toString ../../.. + "/";
  stripNixpkgsRootFromKeys = lib.mapAttrs' (
    file: value: lib.nameValuePair (lib.removePrefix nixpkgsRoot file) value
  );

  changedpaths = lib.importJSON changedpathsjson;
  moduleMeta = (pkgs.nixos { }).config.meta;

  # Extract attributes that changed from by-name paths.
  # This allows pinging reviewers for pure refactors.
  touchedattrs = lib.pipe changedpaths [
    (lib.filter (changed: lib.hasPrefix "pkgs/by-name/" changed && changed != "pkgs/by-name/README.md"))
    (map (lib.splitString "/"))
    (map (path: lib.elemAt path 3))
    lib.unique
  ];
  # Currently just nixos module maintainers, but in the future we can use this for code owners too
  fileUsers = stripNixpkgsRootFromKeys moduleMeta.maintainers;
  fileTeams = stripNixpkgsRootFromKeys moduleMeta.teams;

  anyMatchingFile = filename: lib.any (lib.hasPrefix filename) changedpaths;
  anyMatchingFile = filename: lib.any (lib.hasPrefix filename) changedFiles;

  anyMatchingFiles = files: lib.any anyMatchingFile files;

  sharded = name: "${lib.substring 0 2 name}/${name}";

  attrsWithMaintainers = lib.pipe (changedattrs ++ removedattrs ++ touchedattrs) [
    # An attribute can appear in changed/removed *and* touched
    lib.unique
    (map (
      name:
      let
        path = lib.splitString "." name;
        # Some packages might be reported as changed on a different platform, but
        # not even have an attribute on the platform the maintainers are requested on.
        # Fallback to `null` for these to filter them out below.
        package = lib.attrByPath path null pkgs;
      in
      {
        inherit name package;
        # Adds all files in by-name to each package, no matter whether they are discoverable
        # via meta attributes below. For example, this allows pinging maintainers for
        # updates to .json files.
        # TODO: Support by-name package sets.
        filenames = lib.optional (lib.length path == 1) "pkgs/by-name/${sharded (lib.head path)}/";
        # meta.maintainers also contains all individual team members.
        # We only want to ping individuals if they're added individually as maintainers, not via teams.
        users = package.meta.nonTeamMaintainers or [ ];
        teams = package.meta.teams or [ ];
      }
    ))
    # No need to match up packages without maintainers with their files.
    # This also filters out attributes where `package = null`, which is the
    # case for libintl, for example.
    (lib.filter (pkg: pkg.users != [ ] || pkg.teams != [ ]))
  ];

  relevantFilenames =
    drv:
    (lib.unique (
      map (pos: lib.removePrefix "${toString ../../..}/" pos.file) (
      map (pos: lib.removePrefix nixpkgsRoot pos.file) (
        lib.filter (x: x != null) [
          (drv.meta.maintainersPosition or null)
          (drv.meta.teamsPosition or null)
@@ -87,50 +76,82 @@ let
      )
    ));

  attrsWithFilenames = map (
    pkg: pkg // { filenames = pkg.filenames ++ relevantFilenames pkg.package; }
  ) attrsWithMaintainers;
  relevantAffectedAttrPaths = lib.filter (
    attrPath:
    # Some packages might be reported as changed on a different platform, but
    # not even have an attribute on the platform the maintainers are requested on.
    # Fallback to `null` for these to filter them out
    let
      package = lib.attrByPath attrPath null pkgs;
    in
    package != null && anyMatchingFiles (relevantFilenames package)
  ) affectedAttrPaths;

  # Extract attributes that changed from by-name paths.
  # This allows pinging reviewers for pure refactors.
  changedByNameAttrPaths = lib.pipe changedFiles [
    (lib.filter (changed: lib.hasPrefix "pkgs/by-name/" changed))
    (map (lib.splitString "/"))
    # Filters out e.g. pkgs/by-name/README.md
    (lib.filter (path: lib.length path > 3))
    (map (path: lib.elemAt path 3))
    (map lib.singleton)
  ];

  # An attribute can appear in affected *and* touched
  attrPathsToGetMaintainersFor = lib.unique (relevantAffectedAttrPaths ++ changedByNameAttrPaths);

  attrPathEntities = lib.concatMap (
    attrPath:
    let
      package = lib.getAttrFromPath attrPath pkgs;
    in
    # meta.maintainers also contains all individual team members.
    # We only want to ping individuals if they're added individually as maintainers, not via teams.
    userPings { inherit attrPath; } (package.meta.nonTeamMaintainers or [ ])
    ++ lib.concatMap (teamPings { inherit attrPath; }) (package.meta.teams or [ ])
  ) attrPathsToGetMaintainersFor;

  attrsWithModifiedFiles = lib.filter (pkg: anyMatchingFiles pkg.filenames) attrsWithFilenames;
  changedFileEntities = lib.concatMap (
    file:
    userPings { inherit file; } (fileUsers.${file} or [ ])
    ++ lib.concatMap (teamPings { inherit file; }) (fileTeams.${file} or [ ])
  ) changedFiles;

  userPings =
    pkg:
    context:
    map (maintainer: {
      type = "user";
      userId = maintainer.githubId;
      packageName = pkg.name;
      inherit context;
    });

  teamPings =
    pkg: team:
    if team ? github then
    context: team:
    if team ? githubId then
      [
        {
          type = "team";
          teamId = team.githubId;
          packageName = pkg.name;
          inherit context;
        }
      ]
    else
      userPings pkg team.members;

  maintainersToPing = lib.concatMap (
    pkg: userPings pkg pkg.users ++ lib.concatMap (teamPings pkg) pkg.teams
  ) attrsWithModifiedFiles;
      userPings context team.members;

  byType = lib.groupBy (ping: ping.type) maintainersToPing;
  byType = lib.groupBy (ping: ping.type) (attrPathEntities ++ changedFileEntities);

  byUser = lib.pipe (byType.user or [ ]) [
    (lib.groupBy (ping: toString ping.userId))
    (lib.mapAttrs (_user: lib.map (pkg: pkg.packageName)))
    (lib.mapAttrs (_user: lib.map (pkg: pkg.context)))
  ];
  byTeam = lib.pipe (byType.team or [ ]) [
    (lib.groupBy (ping: toString ping.teamId))
    (lib.mapAttrs (_team: lib.map (pkg: pkg.packageName)))
    (lib.mapAttrs (_team: lib.map (pkg: pkg.context)))
  ];
in
{
  users = byUser;
  teams = byTeam;
  packages = lib.catAttrs "name" attrsWithModifiedFiles;
  packages = attrPathsToGetMaintainersFor;
}
+228 −0
Original line number Diff line number Diff line
{
  pkgs ? import ../../.. {
    config = { };
    overlays = [ ];
  },
  lib ? pkgs.lib,
}:
let
  fun = import ./maintainers.nix;

  mockPkgs =
    {
      packages ? [ ],
      modules ? [ ],
      githubTeams ? true,
    }:
    lib.updateManyAttrsByPath
      (lib.imap0 (i: p: {
        path = p;
        update = _: {
          meta.maintainersPosition.file = lib.concatStringsSep "/" p;
          meta.nonTeamMaintainers = [ { githubId = i; } ];
          meta.teams =
            if githubTeams then [ { githubId = i + 100; } ] else [ { members = [ { githubId = i + 100; } ]; } ];
        };
      }) packages)
      {
        nixos =
          { }:
          {
            config.meta.maintainers = lib.listToAttrs (
              lib.imap0 (i: m: lib.nameValuePair m [ { githubId = i; } ]) modules
            );
            config.meta.teams = lib.listToAttrs (
              lib.imap0 (
                i: m:
                lib.nameValuePair m (
                  if githubTeams then [ { githubId = i + 100; } ] else [ { members = [ { githubId = i + 100; } ]; } ]
                )
              ) modules
            );
          };
      };

  tests = {
    testEmpty = {
      expr = fun {
        pkgs = mockPkgs { };
        inherit lib;
        changedFiles = [ ];
        affectedAttrPaths = [ ];
      };
      expected = {
        packages = [ ];
        teams = { };
        users = { };
      };
    };
    testNonExistentAffected = {
      expr = fun {
        pkgs = mockPkgs { };
        inherit lib;
        changedFiles = [ "a" ];
        affectedAttrPaths = [ [ "b" ] ];
      };
      expected = {
        packages = [ ];
        teams = { };
        users = { };
      };
    };
    testIrrelevantAffected = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "b" ] ];
        };
        inherit lib;
        changedFiles = [ "a" ];
        affectedAttrPaths = [ [ "b" ] ];
      };
      expected = {
        packages = [ ];
        teams = { };
        users = { };
      };
    };
    testRelevantAffected = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "b" ] ];
        };
        inherit lib;
        # Also tests that subpaths work
        changedFiles = [ "b/c" ];
        affectedAttrPaths = [ [ "b" ] ];
      };
      expected = {
        packages = [ [ "b" ] ];
        teams."100" = [
          { attrPath = [ "b" ]; }
        ];
        users."0" = [
          { attrPath = [ "b" ]; }
        ];
      };
    };
    testRelevantAffectedNonGitHub = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "b" ] ];
          githubTeams = false;
        };
        inherit lib;
        changedFiles = [ "b/c" ];
        affectedAttrPaths = [ [ "b" ] ];
      };
      expected = {
        packages = [ [ "b" ] ];
        teams = { };
        users."0" = [
          { attrPath = [ "b" ]; }
        ];
        users."100" = [
          { attrPath = [ "b" ]; }
        ];
      };
    };
    testByNameChanged = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "hello" ] ];
        };
        inherit lib;
        changedFiles = [ "pkgs/by-name/he/hello/sources.json" ];
        affectedAttrPaths = [ ];
      };
      expected = {
        packages = [ [ "hello" ] ];
        teams."100" = [
          { attrPath = [ "hello" ]; }
        ];
        users."0" = [
          { attrPath = [ "hello" ]; }
        ];
      };
    };
    testByNameReadmeChanged = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "hello" ] ];
        };
        inherit lib;
        changedFiles = [ "pkgs/by-name/README.md" ];
        affectedAttrPaths = [ ];
      };
      expected = {
        packages = [ ];
        teams = { };
        users = { };
      };
    };
    testNoDuplicates = {
      expr = fun {
        pkgs = mockPkgs {
          packages = [ [ "hello" ] ];
        };
        inherit lib;
        changedFiles = [
          "hello"
          "pkgs/by-name/he/hello/sources.json"
        ];
        affectedAttrPaths = [ [ "hello" ] ];
      };
      expected = {
        packages = [ [ "hello" ] ];
        teams."100" = [
          { attrPath = [ "hello" ]; }
        ];
        users."0" = [
          { attrPath = [ "hello" ]; }
        ];
      };
    };
    testModuleMaintainers = {
      expr = fun {
        pkgs = mockPkgs {
          modules = [ "a" ];
        };
        inherit lib;
        changedFiles = [ "a" ];
        affectedAttrPaths = [ ];
      };
      expected = {
        packages = [ ];
        teams."100" = [
          { file = "a"; }
        ];
        users."0" = [
          { file = "a"; }
        ];
      };
    };
    testModuleMaintainersNonGithub = {
      expr = fun {
        pkgs = mockPkgs {
          modules = [ "a" ];
          githubTeams = false;
        };
        inherit lib;
        changedFiles = [ "a" ];
        affectedAttrPaths = [ ];
      };
      expected = {
        packages = [ ];
        teams = { };
        users."100" = [
          { file = "a"; }
        ];
        users."0" = [
          { file = "a"; }
        ];
      };
    };
  };
in
{
  result = lib.runTests tests;
}
+18 −0
Original line number Diff line number Diff line
@@ -5709,6 +5709,12 @@
    githubId = 217899;
    name = "Cyryl Płotnicki";
  };
  cyrusknopf = {
    email = "cyrus.knopf@gmail.com";
    github = "cyrusknopf";
    githubId = 26168196;
    name = "Cyrus Knopf";
  };
  cything = {
    name = "cy";
    email = "nix@cything.io";
@@ -22230,6 +22236,12 @@
    github = "rafameou";
    githubId = 26395874;
  };
  rafanochi = {
    email = "zawkindev@gmail.com";
    name = "Rafa";
    github = "rafanochi";
    githubId = 139969429;
  };
  ragge = {
    email = "r.dahlen@gmail.com";
    github = "ragnard";
@@ -23232,6 +23244,12 @@
    githubId = 55679162;
    keys = [ { fingerprint = "1401 1B63 393D 16C1 AA9C  C521 8526 B757 4A53 6236"; } ];
  };
  roquess = {
    name = "Steve Roques";
    email = "steve.roques@gmail.com";
    github = "roquess";
    githubId = 8398054;
  };
  rorosen = {
    email = "robert.rose@mailbox.org";
    github = "rorosen";
+27 −31
Original line number Diff line number Diff line
@@ -4,39 +4,12 @@
let
  inherit (lib)
    mkOption
    mkOptionType
    types
    ;

  maintainer = mkOptionType {
    name = "maintainer";
    check = email: lib.elem email (lib.attrValues lib.maintainers);
    merge = loc: defs: {
      # lib.last: Perhaps this could be merged instead, if "at most once per module"
      # is a problem (see option description).
      ${(lib.last defs).file} = (lib.last defs).value;
    };
  };

  listOfMaintainers = types.listOf maintainer // {
    merge =
      loc: defs:
      lib.zipAttrs (
        lib.flatten (
          lib.imap1 (
            n: def:
            lib.imap1 (
              m: def':
              maintainer.merge (loc ++ [ "[${toString n}-${toString m}]" ]) [
                {
                  inherit (def) file;
                  value = def';
                }
              ]
            ) def.value
          ) defs
        )
      );
  # The resulting value of this type shows where all values were defined
  sourceList = types.listOf types.raw // {
    merge = loc: defs: lib.listToAttrs (lib.map ({ file, value }: lib.nameValuePair file value) defs);
  };
in
{
@@ -44,7 +17,14 @@ in
  options = {
    meta = {
      maintainers = mkOption {
        type = listOfMaintainers;
        type =
          let
            allMaintainers = lib.attrValues lib.maintainers;
          in
          lib.types.addCheck sourceList (lib.all (v: lib.elem v allMaintainers))
          // {
            description = "list of lib.maintainers";
          };
        default = [ ];
        example = lib.literalExpression "[ lib.maintainers.alice lib.maintainers.bob ]";
        description = ''
@@ -54,6 +34,22 @@ in
          The option value is not a list of maintainers, but an attribute set that maps module file names to lists of maintainers.
        '';
      };
      teams = mkOption {
        type =
          let
            allTeams = lib.attrValues lib.teams;
          in
          lib.types.addCheck sourceList (lib.all (v: lib.elem v allTeams))
          // {
            description = "list of lib.teams";
          };
        default = [ ];
        example = lib.literalExpression "[ lib.teams.acme lib.teams.haskell ]";
        description = ''
          List of team maintainers of each module.
          This option should be defined at most once per module.
        '';
      };
    };
  };
  meta.maintainers = with lib.maintainers; [
Loading