From be4026af8d346c1137bc4f81827316fc9c917284 Mon Sep 17 00:00:00 2001 From: Silvan Mosberger Date: Thu, 28 Nov 2024 22:57:12 +0100 Subject: [PATCH] Maintainer review requests --- .github/workflows/eval.yml | 15 ++++- ci/eval/default.nix | 20 +++++- ci/eval/maintainers.nix | 125 +++++++++++++++++++++++++++++++++++++ 3 files changed, 158 insertions(+), 2 deletions(-) create mode 100644 ci/eval/maintainers.nix diff --git a/.github/workflows/eval.yml b/.github/workflows/eval.yml index 90be4670e414b..2d303a4ebd9e1 100644 --- a/.github/workflows/eval.yml +++ b/.github/workflows/eval.yml @@ -145,6 +145,7 @@ jobs: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 with: ref: ${{ needs.attrs.outputs.mergedSha }} + fetch-depth: 2 path: nixpkgs - name: Install Nix @@ -206,9 +207,15 @@ jobs: - name: Compare against the base branch if: steps.baseRunId.outputs.baseRunId run: | + git -C nixpkgs worktree add ../base ${{ needs.attrs.outputs.baseSha }} + git -C nixpkgs diff --name-only --merge-base ${{ needs.attrs.outputs.baseSha }} ${{ needs.attrs.outputs.mergedSha }} \ + | jq --raw-input . > touched-files.json + nix-build nixpkgs/ci -A eval.compare \ --arg beforeResultDir ./baseResult \ --arg afterResultDir ./prResult \ + --arg changedPathsJson ./touched-files.json \ + --arg baseNixpkgs ./base \ -o comparison # TODO: Request reviews from maintainers for packages whose files are modified in the PR @@ -240,6 +247,12 @@ jobs: gh api \ --method POST \ /repos/${{ github.repository }}/issues/${{ github.event.number }}/labels \ - --input <(jq -c '{ labels: .labels }' comparison/changed-paths.json) + --input <(jq '{ labels: .labels }' comparison/changed-paths.json) + + gh api \ + --method POST \ + /repos/${{ github.repository }}/pulls/${{ github.event.number }}/requested_reviewers \ + --input <(jq '{ reviewers: keys }' tmp/reviewers.json) + env: GH_TOKEN: ${{ github.token }} diff --git a/ci/eval/default.nix b/ci/eval/default.nix index 1855efc79f5e8..47e6acc373ffb 100644 --- a/ci/eval/default.nix +++ b/ci/eval/default.nix @@ -25,6 +25,7 @@ let "pkgs" ".version" "ci/supportedSystems.nix" + "ci/eval/maintainers.nix" ] ); }; @@ -239,11 +240,17 @@ let ''; compare = - { beforeResultDir, afterResultDir }: + { + beforeResultDir, + afterResultDir, + changedPathsJson, + baseNixpkgs, + }: runCommand "compare" { nativeBuildInputs = [ jq + nix ]; } '' @@ -253,6 +260,17 @@ let --slurpfile after ${afterResultDir}/outpaths.json \ > $out/changed-paths.json + jq '.attrdiff | [ .changed[] , .removed[] ] | map(split("."))' $out/changed-paths.json > changed-attrs.json + + export NIX_STATE_DIR=$(mktemp -d) + # Removed and changed + # Kind of important: Maintainers specified in the old branch should be pinged + nix-instantiate --eval --strict --json ${nixpkgs}/ci/eval/maintainers.nix \ + --arg changedattrsjson ./changed-attrs.json \ + --arg changedpathsjson ${changedPathsJson} \ + --arg baseNixpkgs ${baseNixpkgs} \ + > $out/maintainers.json \ + # TODO: Compare eval stats ''; diff --git a/ci/eval/maintainers.nix b/ci/eval/maintainers.nix new file mode 100644 index 0000000000000..3f512f7d6f31f --- /dev/null +++ b/ci/eval/maintainers.nix @@ -0,0 +1,125 @@ +# Almost directly vendored from https://github.com/NixOS/ofborg/blob/5a4e743f192fb151915fcbe8789922fa401ecf48/ofborg/src/maintainers.nix +{ baseNixpkgs, changedattrsjson, changedpathsjson }: +let + lib = import ../../lib; + + pkgs = import baseNixpkgs { + system = "x86_64-linux"; + config = {}; + overlays = []; + }; + + changedattrs = builtins.fromJSON (builtins.readFile changedattrsjson); + changedpaths = builtins.fromJSON (builtins.readFile changedpathsjson); + + anyMatchingFile = filename: + let + matching = builtins.filter + (changed: lib.strings.hasSuffix changed filename) + changedpaths; + in (builtins.length matching) > 0; + + anyMatchingFiles = files: + (builtins.length (builtins.filter anyMatchingFile files)) > 0; + + enrichedAttrs = builtins.map + (path: { + path = path; + name = builtins.concatStringsSep "." path; + }) + changedattrs; + + validPackageAttributes = builtins.filter + (pkg: + if (lib.attrsets.hasAttrByPath pkg.path pkgs) + then (if (builtins.tryEval (lib.attrsets.attrByPath pkg.path null pkgs)).success + then true + else builtins.trace "Failed to access ${pkg.name} even though it exists" false) + else builtins.trace "Failed to locate ${pkg.name}." false + ) + enrichedAttrs; + + attrsWithPackages = builtins.map + (pkg: pkg // { package = lib.attrsets.attrByPath pkg.path null pkgs; }) + validPackageAttributes; + + attrsWithMaintainers = builtins.map + (pkg: pkg // { maintainers = (pkg.package.meta or {}).maintainers or []; }) + attrsWithPackages; + + attrsWeCanPing = builtins.filter + (pkg: if (builtins.length pkg.maintainers) > 0 + then true + else builtins.trace "Package has no maintainers: ${pkg.name}" false + ) + attrsWithMaintainers; + + relevantFilenames = drv: + (lib.lists.unique + (builtins.map + (pos: lib.strings.removePrefix (toString baseNixpkgs) pos.file) + (builtins.filter (x: x != null) + [ + (builtins.unsafeGetAttrPos "maintainers" (drv.meta or {})) + (builtins.unsafeGetAttrPos "src" drv) + # broken because name is always set by stdenv: + # # A hack to make `nix-env -qa` and `nix search` ignore broken packages. + # # TODO(@oxij): remove this assert when something like NixOS/nix#1771 gets merged into nix. + # name = assert validity.handled; name + lib.optionalString + #(builtins.unsafeGetAttrPos "name" drv) + (builtins.unsafeGetAttrPos "pname" drv) + (builtins.unsafeGetAttrPos "version" drv) + + # Use ".meta.position" for cases when most of the package is + # defined in a "common" section and the only place where + # reference to the file with a derivation the "pos" + # attribute. + # + # ".meta.position" has the following form: + # "pkgs/tools/package-management/nix/default.nix:155" + # We transform it to the following: + # { file = "pkgs/tools/package-management/nix/default.nix"; } + { file = lib.head (lib.splitString ":" (drv.meta.position or "")); } + ] + ))); + + attrsWithFilenames = builtins.map + (pkg: pkg // { filenames = relevantFilenames pkg.package; }) + attrsWithMaintainers; + + attrsWithModifiedFiles = builtins.filter + (pkg: anyMatchingFiles pkg.filenames) + attrsWithFilenames; + + listToPing = lib.lists.flatten + (builtins.map + (pkg: + builtins.map (maintainer: { + handle = lib.toLower maintainer.github; + packageName = pkg.name; + dueToFiles = pkg.filenames; + }) + pkg.maintainers + ) + attrsWithModifiedFiles); + + byMaintainer = lib.lists.foldr + (ping: collector: collector // { "${ping.handle}" = [ { inherit (ping) packageName dueToFiles; } ] ++ (collector."${ping.handle}" or []); }) + {} + listToPing; + + textForPackages = packages: + lib.strings.concatStringsSep ", " ( + builtins.map (pkg: pkg.packageName) + packages); + + textPerMaintainer = lib.attrsets.mapAttrs + (maintainer: packages: "- @${maintainer} for ${textForPackages packages}") + byMaintainer; + + packagesPerMaintainer = lib.attrsets.mapAttrs + (maintainer: packages: + builtins.map (pkg: pkg.packageName) + packages) + byMaintainer; +in packagesPerMaintainer