Skip to content

Commit

Permalink
Maintainer review requests
Browse files Browse the repository at this point in the history
  • Loading branch information
infinisil committed Nov 28, 2024
1 parent af1aa40 commit be4026a
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 2 deletions.
15 changes: 14 additions & 1 deletion .github/workflows/eval.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 }}
20 changes: 19 additions & 1 deletion ci/eval/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ let
"pkgs"
".version"
"ci/supportedSystems.nix"
"ci/eval/maintainers.nix"
]
);
};
Expand Down Expand Up @@ -239,11 +240,17 @@ let
'';

compare =
{ beforeResultDir, afterResultDir }:
{
beforeResultDir,
afterResultDir,
changedPathsJson,
baseNixpkgs,
}:
runCommand "compare"
{
nativeBuildInputs = [
jq
nix
];
}
''
Expand All @@ -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
'';

Expand Down
125 changes: 125 additions & 0 deletions ci/eval/maintainers.nix
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit be4026a

Please sign in to comment.