From 3bef0dcf6588c23c95ce882e17a592d6ec66aa21 Mon Sep 17 00:00:00 2001 From: Wolfgang Walther Date: Sat, 1 Nov 2025 14:27:15 +0100 Subject: [PATCH] ci/request-reviews: move get-code-owners to eval/compare This moves the parsing of ci/OWNERS into the Nix sandbox. We also get rid of checking out the nixpkgs repo another time in the reviewers workflow - we already have everything we need in the eval/compare job. The creation of owners.txt in this way is only temporary, it should eventually be moved further, similar to how maintainers.json is currently migrating to a maintainer map for the whole repo stored on the target branch as artifact. --- .github/workflows/reviewers.yml | 9 +-- ci/eval/compare/default.nix | 39 ++++++++++++ ci/request-reviews/default.nix | 8 --- ci/request-reviews/get-code-owners.sh | 51 --------------- .../request-code-owner-reviews.sh | 63 ------------------- 5 files changed, 40 insertions(+), 130 deletions(-) delete mode 100755 ci/request-reviews/get-code-owners.sh delete mode 100755 ci/request-reviews/request-code-owner-reviews.sh diff --git a/.github/workflows/reviewers.yml b/.github/workflows/reviewers.yml index a6822aa9af0e..03d8caa2661b 100644 --- a/.github/workflows/reviewers.yml +++ b/.github/workflows/reviewers.yml @@ -63,13 +63,6 @@ jobs: permission-members: read permission-pull-requests: write - - name: Determining code owner reviews - env: - REPOSITORY: ${{ github.repository }} - NUMBER: ${{ github.event.number }} - run: | - result/bin/request-code-owner-reviews.sh "$REPOSITORY" "$NUMBER" ci/OWNERS > owners.txt - - name: Log current API rate limits (github.token) env: GH_TOKEN: ${{ github.token }} @@ -149,7 +142,7 @@ jobs: # There appears to be no API to request reviews based on GitHub IDs jq -r 'keys[]' comparison/maintainers.json \ | while read -r id; do gh api /user/"$id" --jq .login; done \ - | cat owners.txt - \ + | cat comparison/owners.txt - \ | GH_TOKEN="$APP_GH_TOKEN" result/bin/request-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR" - name: Log current API rate limits (app-token) diff --git a/ci/eval/compare/default.nix b/ci/eval/compare/default.nix index eabc28af4d36..571e6ba41068 100644 --- a/ci/eval/compare/default.nix +++ b/ci/eval/compare/default.nix @@ -7,6 +7,7 @@ python3, stdenvNoCC, makeWrapper, + codeowners, }: let python = python3.withPackages (ps: [ @@ -48,6 +49,7 @@ in { combinedDir, touchedFilesJson, + ownersFile ? ../../OWNERS, }: let # Usually we expect a derivation, but when evaluating in multiple separate steps, we pass @@ -174,6 +176,7 @@ runCommand "compare" nativeBuildInputs = map lib.getBin [ jq cmp-stats + codeowners ]; maintainers = builtins.toJSON maintainers; packages = builtins.toJSON packages; @@ -222,6 +225,42 @@ runCommand "compare" } >> $out/step-summary.md fi + jq -r '.[]' "${touchedFilesJson}" > ./touched-files + readarray -t touchedFiles < ./touched-files + echo "This PR touches ''${#touchedFiles[@]} files" + + # TODO: Move ci/OWNERS to Nix and produce owners.json instead of owners.txt. + for file in "''${touchedFiles[@]}"; do + result=$(codeowners --file "${ownersFile}" "$file") + + # Remove the file prefix and trim the surrounding spaces + read -r owners <<< "''${result#"$file"}" + if [[ "$owners" == "(unowned)" ]]; then + echo "File $file is unowned" + continue + fi + echo "File $file is owned by $owners" + + # Split up multiple owners, separated by arbitrary amounts of spaces + IFS=" " read -r -a entries <<< "$owners" + + for entry in "''${entries[@]}"; do + # GitHub technically also supports Emails as code owners, + # but we can't easily support that, so let's not + if [[ ! "$entry" =~ @(.*) ]]; then + echo -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" + # Don't fail, because the PR for which this script runs can't fix it, + # it has to be fixed in the base branch + continue + fi + # The first regex match is everything after the @ + entry=''${BASH_REMATCH[1]} + + echo "$entry" >> "$out/owners.txt" + done + + done + cp "$maintainersPath" "$out/maintainers.json" cp "$packagesPath" "$out/packages.json" '' diff --git a/ci/request-reviews/default.nix b/ci/request-reviews/default.nix index 075ff52fd564..55a1889fe89f 100644 --- a/ci/request-reviews/default.nix +++ b/ci/request-reviews/default.nix @@ -3,20 +3,15 @@ stdenvNoCC, makeWrapper, coreutils, - codeowners, jq, - curl, github-cli, - gitMinimal, }: stdenvNoCC.mkDerivation { name = "request-reviews"; src = lib.fileset.toSource { root = ./.; fileset = lib.fileset.unions [ - ./get-code-owners.sh ./request-reviewers.sh - ./request-code-owner-reviews.sh ]; }; nativeBuildInputs = [ makeWrapper ]; @@ -29,11 +24,8 @@ stdenvNoCC.mkDerivation { --set PATH ${ lib.makeBinPath [ coreutils - codeowners jq - curl github-cli - gitMinimal ] } done diff --git a/ci/request-reviews/get-code-owners.sh b/ci/request-reviews/get-code-owners.sh deleted file mode 100755 index 33963bce7f30..000000000000 --- a/ci/request-reviews/get-code-owners.sh +++ /dev/null @@ -1,51 +0,0 @@ -#!/usr/bin/env bash - -# Get the code owners of the files changed by a PR, returning one username or team per line - -set -euo pipefail - -log() { - echo "$@" >&2 -} - -if (( "$#" < 4 )); then - log "Usage: $0 TOUCHED_FILES_FILE OWNERS_FILE" - exit 1 -fi - -touchedFilesFile=$1 -ownersFile=$2 - -readarray -t touchedFiles < "$touchedFilesFile" -log "This PR touches ${#touchedFiles[@]} files" - -for file in "${touchedFiles[@]}"; do - result=$(codeowners --file "$ownersFile" "$file") - - # Remove the file prefix and trim the surrounding spaces - read -r owners <<< "${result#"$file"}" - if [[ "$owners" == "(unowned)" ]]; then - log "File $file is unowned" - continue - fi - log "File $file is owned by $owners" - - # Split up multiple owners, separated by arbitrary amounts of spaces - IFS=" " read -r -a entries <<< "$owners" - - for entry in "${entries[@]}"; do - # GitHub technically also supports Emails as code owners, - # but we can't easily support that, so let's not - if [[ ! "$entry" =~ @(.*) ]]; then - warn -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" >&2 - # Don't fail, because the PR for which this script runs can't fix it, - # it has to be fixed in the base branch - continue - fi - # The first regex match is everything after the @ - entry=${BASH_REMATCH[1]} - - echo "$entry" - done - -done diff --git a/ci/request-reviews/request-code-owner-reviews.sh b/ci/request-reviews/request-code-owner-reviews.sh deleted file mode 100755 index 09df38304854..000000000000 --- a/ci/request-reviews/request-code-owner-reviews.sh +++ /dev/null @@ -1,63 +0,0 @@ -#!/usr/bin/env bash - -# Requests reviews for a PR - -set -euo pipefail -tmp=$(mktemp -d) -trap 'rm -rf "$tmp"' exit -SCRIPT_DIR=$(dirname "$0") - -log() { - echo "$@" >&2 -} - -if (( $# < 3 )); then - log "Usage: $0 GITHUB_REPO PR_NUMBER OWNERS_FILE" - exit 1 -fi -baseRepo=$1 -prNumber=$2 -ownersFile=$3 - -log "Fetching PR info" -prInfo=$(gh api \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "/repos/$baseRepo/pulls/$prNumber") - -baseBranch=$(jq -r .base.ref <<< "$prInfo") -log "Base branch: $baseBranch" -prRepo=$(jq -r .head.repo.full_name <<< "$prInfo") -log "PR repo: $prRepo" -prBranch=$(jq -r .head.ref <<< "$prInfo") -log "PR branch: $prBranch" - -extraArgs=() -if pwdRepo=$(git rev-parse --show-toplevel 2>/dev/null); then - # Speedup for local runs - extraArgs+=(--reference-if-able "$pwdRepo") -fi - -log "Fetching Nixpkgs commit history" -# We only need the commit history, not the contents, so we can do a tree-less clone using tree:0 -# https://github.blog/open-source/git/get-up-to-speed-with-partial-clone-and-shallow-clone/#user-content-quick-summary -git clone --bare --filter=tree:0 --no-tags --origin upstream "${extraArgs[@]}" https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git - -log "Fetching the PR commit history" -# Fetch the PR -git -C "$tmp/nixpkgs.git" remote add fork https://github.com/"$prRepo".git -# This remote config is the same as --filter=tree:0 when cloning -git -C "$tmp/nixpkgs.git" config remote.fork.partialclonefilter tree:0 -git -C "$tmp/nixpkgs.git" config remote.fork.promisor true - -git -C "$tmp/nixpkgs.git" fetch --no-tags fork "$prBranch" -headRef=$(git -C "$tmp/nixpkgs.git" rev-parse refs/remotes/fork/"$prBranch") - -git -C "$tmp/nixpkgs.git" diff --name-only --merge-base "$baseBranch" "$headRef" > "$tmp/touched-files" - -# Get the owners file from the base, because we don't want to allow PRs to -# remove code owners to avoid pinging them -git -C "$tmp/nixpkgs.git" show "$baseBranch":"$ownersFile" > "$tmp"/codeowners - -log "Requesting reviews from code owners" -"$SCRIPT_DIR"/get-code-owners.sh "$tmp/touched-files" "$tmp"/codeowners