ci/github-script/labels: set maintainer labels from latest maintainer map (#457243)

This commit is contained in:
Wolfgang Walther
2025-11-01 10:11:01 +00:00
committed by GitHub
6 changed files with 143 additions and 39 deletions

View File

@@ -202,7 +202,6 @@ jobs:
- name: Compare against the target branch - name: Compare against the target branch
env: env:
AUTHOR_ID: ${{ github.event.pull_request.user.id }}
TARGET_SHA: ${{ inputs.mergedSha }} TARGET_SHA: ${{ inputs.mergedSha }}
run: | run: |
git -C nixpkgs/trusted diff --name-only "$TARGET_SHA" \ git -C nixpkgs/trusted diff --name-only "$TARGET_SHA" \
@@ -212,7 +211,6 @@ jobs:
nix-build nixpkgs/trusted/ci --arg nixpkgs ./nixpkgs/trusted-pinned -A eval.compare \ nix-build nixpkgs/trusted/ci --arg nixpkgs ./nixpkgs/trusted-pinned -A eval.compare \
--arg combinedDir ./combined \ --arg combinedDir ./combined \
--arg touchedFilesJson ./touched-files.json \ --arg touchedFilesJson ./touched-files.json \
--argstr githubAuthorId "$AUTHOR_ID" \
--out-link comparison --out-link comparison
cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY" cat comparison/step-summary.md >> "$GITHUB_STEP_SUMMARY"

View File

@@ -39,10 +39,6 @@ jobs:
update: update:
runs-on: ubuntu-24.04-arm runs-on: ubuntu-24.04-arm
if: github.event_name != 'schedule' || github.repository_owner == 'NixOS' if: github.event_name != 'schedule' || github.repository_owner == 'NixOS'
env:
# TODO: Remove after 2026-03-04, when Node 24 becomes the default.
# https://github.blog/changelog/2025-09-19-deprecation-of-node-20-on-github-actions-runners/
FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: "true"
steps: steps:
- uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0 - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5.0.0
with: with:

View File

@@ -48,7 +48,6 @@ in
{ {
combinedDir, combinedDir,
touchedFilesJson, touchedFilesJson,
githubAuthorId,
}: }:
let let
# Usually we expect a derivation, but when evaluating in multiple separate steps, we pass # Usually we expect a derivation, but when evaluating in multiple separate steps, we pass
@@ -155,22 +154,19 @@ let
# Only set this label when no other label with indication for staging has been set. # Only set this label when no other label with indication for staging has been set.
# This avoids confusion whether to target staging or batch this with kernel updates. # This avoids confusion whether to target staging or batch this with kernel updates.
lib.last (lib.sort lib.lessThan (lib.attrValues rebuildCountByKernel)) <= 500; lib.last (lib.sort lib.lessThan (lib.attrValues rebuildCountByKernel)) <= 500;
# Set the "11.by: package-maintainer" label to whether all packages directly
# changed are maintained by the PR's author.
"11.by: package-maintainer" =
maintainers ? ${githubAuthorId}
&& lib.all (lib.flip lib.elem maintainers.${githubAuthorId}) (
lib.flatten (lib.attrValues maintainers)
);
}; };
} }
); );
maintainers = callPackage ./maintainers.nix { } { inherit
changedattrs = lib.attrNames (lib.groupBy (a: a.name) changedPackagePlatformAttrs); (callPackage ./maintainers.nix { } {
changedpathsjson = touchedFilesJson; changedattrs = lib.attrNames (lib.groupBy (a: a.name) changedPackagePlatformAttrs);
removedattrs = lib.attrNames (lib.groupBy (a: a.name) removedPackagePlatformAttrs); changedpathsjson = touchedFilesJson;
}; removedattrs = lib.attrNames (lib.groupBy (a: a.name) removedPackagePlatformAttrs);
})
maintainers
packages
;
in in
runCommand "compare" runCommand "compare"
{ {
@@ -180,7 +176,11 @@ runCommand "compare"
cmp-stats cmp-stats
]; ];
maintainers = builtins.toJSON maintainers; maintainers = builtins.toJSON maintainers;
passAsFile = [ "maintainers" ]; packages = builtins.toJSON packages;
passAsFile = [
"maintainers"
"packages"
];
} }
'' ''
mkdir $out mkdir $out
@@ -223,4 +223,5 @@ runCommand "compare"
fi fi
cp "$maintainersPath" "$out/maintainers.json" cp "$maintainersPath" "$out/maintainers.json"
cp "$packagesPath" "$out/packages.json"
'' ''

View File

@@ -105,9 +105,9 @@ let
) attrsWithModifiedFiles; ) attrsWithModifiedFiles;
byMaintainer = lib.groupBy (ping: toString ping.id) listToPing; byMaintainer = lib.groupBy (ping: toString ping.id) listToPing;
packagesPerMaintainer = lib.mapAttrs (
maintainer: packages: map (pkg: pkg.packageName) packages
) byMaintainer;
in in
packagesPerMaintainer {
maintainers = lib.mapAttrs (_: lib.catAttrs "packageName") byMaintainer;
packages = lib.catAttrs "packageName" listToPing;
}

View File

@@ -281,9 +281,6 @@ let
# Whether to evaluate on a specific set of systems, by default all are evaluated # Whether to evaluate on a specific set of systems, by default all are evaluated
evalSystems ? if quickTest then [ "x86_64-linux" ] else supportedSystems, evalSystems ? if quickTest then [ "x86_64-linux" ] else supportedSystems,
baseline, baseline,
# Which maintainer should be considered the author?
# Defaults to nixpkgs-ci which is not a maintainer and skips the check.
githubAuthorId ? "nixpkgs-ci",
# What files have been touched? Defaults to none; use the expression below to calculate it. # What files have been touched? Defaults to none; use the expression below to calculate it.
# ``` # ```
# git diff --name-only --merge-base master HEAD \ # git diff --name-only --merge-base master HEAD \
@@ -307,7 +304,7 @@ let
}; };
comparisonReport = compare { comparisonReport = compare {
combinedDir = combine { diffDir = diffs; }; combinedDir = combine { diffDir = diffs; };
inherit touchedFilesJson githubAuthorId; inherit touchedFilesJson;
}; };
in in
comparisonReport; comparisonReport;

View File

@@ -3,9 +3,98 @@ module.exports = async ({ github, context, core, dry }) => {
const { DefaultArtifactClient } = require('@actions/artifact') const { DefaultArtifactClient } = require('@actions/artifact')
const { readFile, writeFile } = require('node:fs/promises') const { readFile, writeFile } = require('node:fs/promises')
const withRateLimit = require('./withRateLimit.js') const withRateLimit = require('./withRateLimit.js')
const { classify } = require('../supportedBranches.js')
const artifactClient = new DefaultArtifactClient() const artifactClient = new DefaultArtifactClient()
async function downloadMaintainerMap(branch) {
let run
const commits = (
await github.rest.repos.listCommits({
...context.repo,
sha: branch,
// We look at 10 commits to find a maintainer map, but this is an arbitrary number. The
// head commit might not have a map, if the queue was bypassed to merge it. This happens
// frequently on staging-esque branches. The branch with the highest chance of getting
// 10 consecutive bypassing commits is the stable staging-next branch. Luckily, this
// also means that the number of PRs open towards that branch is very low, so falling
// back to slightly imprecise maintainer data from master only has a marginal effect.
per_page: 10,
})
).data
for (const commit of commits) {
const run = (
await github.rest.actions.listWorkflowRuns({
...context.repo,
workflow_id: 'merge-group.yml',
status: 'success',
exclude_pull_requests: true,
per_page: 1,
head_sha: commit.sha,
})
).data.workflow_runs[0]
if (!run) continue
const artifact = (
await github.rest.actions.listWorkflowRunArtifacts({
...context.repo,
run_id: run.id,
name: 'maintainers',
})
).data.artifacts[0]
if (!artifact) continue
await artifactClient.downloadArtifact(artifact.id, {
findBy: {
repositoryName: context.repo.repo,
repositoryOwner: context.repo.owner,
token: core.getInput('github-token'),
},
path: path.resolve(path.join('branches', branch)),
expectedHash: artifact.digest,
})
return JSON.parse(
await readFile(
path.resolve(path.join('branches', branch, 'maintainers.json')),
'utf-8',
),
)
}
// We get here when none of the 10 commits we looked at contained a maintainer map.
// For the master branch, we don't have any fallback options, so we error out.
// For other branches, we select a suitable fallback below.
if (branch === 'master') throw new Error('No maintainer map found.')
const { stable, version } = classify(branch)
const release = `release-${version}`
if (stable && branch !== release) {
// Only fallback to the release branch from *other* stable branches.
// Explicitly avoids infinite recursion.
return await getMaintainerMap(release)
} else {
// Falling back to master as last resort.
// This can either be the case for unstable staging-esque or wip branches,
// or for the primary stable branch (release-XX.YY).
return await getMaintainerMap('master')
}
}
// Simple cache for maintainer maps to avoid downloading the same artifacts
// over and over again. Ultimately returns a promise, so the result must be
// awaited for.
const maintainerMaps = {}
function getMaintainerMap(branch) {
if (!maintainerMaps[branch]) {
maintainerMaps[branch] = downloadMaintainerMap(branch)
}
return maintainerMaps[branch]
}
async function handlePullRequest({ item, stats }) { async function handlePullRequest({ item, stats }) {
const log = (k, v) => core.info(`PR #${item.number} - ${k}: ${v}`) const log = (k, v) => core.info(`PR #${item.number} - ${k}: ${v}`)
@@ -177,21 +266,44 @@ module.exports = async ({ github, context, core, dry }) => {
expectedHash: artifact.digest, expectedHash: artifact.digest,
}) })
const maintainers = new Set(
Object.keys(
JSON.parse(
await readFile(`${pull_number}/maintainers.json`, 'utf-8'),
),
).map((m) => Number.parseInt(m, 10)),
)
const evalLabels = JSON.parse( const evalLabels = JSON.parse(
await readFile(`${pull_number}/changed-paths.json`, 'utf-8'), await readFile(`${pull_number}/changed-paths.json`, 'utf-8'),
).labels ).labels
// TODO: Get "changed packages" information from list of changed by-name files
// in addition to just the Eval results, to make this work for these packages
// when Eval results have expired as well.
let packages
try {
packages = JSON.parse(
await readFile(`${pull_number}/packages.json`, 'utf-8'),
)
} catch (e) {
if (e.code !== 'ENOENT') throw e
// TODO: Remove this fallback code once all old artifacts without packages.json
// have expired. This should be the case in ~ February 2026.
packages = Array.from(
new Set(
Object.values(
JSON.parse(
await readFile(`${pull_number}/maintainers.json`, 'utf-8'),
),
).flat(1),
),
)
}
const maintainers = await getMaintainerMap(pull_request.base.ref)
Object.assign(prLabels, evalLabels, { Object.assign(prLabels, evalLabels, {
'12.approved-by: package-maintainer': '11.by: package-maintainer':
maintainers.intersection(approvals).size > 0, packages.length &&
packages.every((pkg) =>
maintainers[pkg].includes(pull_request.user.id),
),
'12.approved-by: package-maintainer': packages.some((pkg) =>
maintainers[pkg].some((m) => approvals.has(m)),
),
}) })
} }