ci/request-reviews: untangle owner-related bash code

.github/workflows/reviewers.yml

@@ -63,28 +63,6 @@ jobs:
           permission-members: read
           permission-pull-requests: write
 
-      - name: Log current API rate limits (app-token)
-        if: ${{ steps.app-token.outputs.token }}
-        env:
-          GH_TOKEN: ${{ steps.app-token.outputs.token }}
-        run: gh api /rate_limit | jq
-
-      - name: Requesting code owner reviews
-        if: steps.app-token.outputs.token
-        env:
-          GH_TOKEN: ${{ steps.app-token.outputs.token }}
-          REPOSITORY: ${{ github.repository }}
-          NUMBER: ${{ github.event.number }}
-          # Don't do anything on draft PRs
-          DRY_MODE: ${{ github.event.pull_request.draft && '1' || '' }}
-        run: result/bin/request-code-owner-reviews.sh "$REPOSITORY" "$NUMBER" ci/OWNERS
-
-      - name: Log current API rate limits (app-token)
-        if: ${{ steps.app-token.outputs.token }}
-        env:
-          GH_TOKEN: ${{ steps.app-token.outputs.token }}
-        run: gh api /rate_limit | jq
-
       - name: Log current API rate limits (github.token)
         env:
           GH_TOKEN: ${{ github.token }}
@@ -149,7 +127,7 @@ jobs:
           GH_TOKEN: ${{ github.token }}
         run: gh api /rate_limit | jq
 
-      - name: Requesting maintainer reviews
+      - name: Requesting reviews
         if: ${{ steps.app-token.outputs.token }}
         env:
           GH_TOKEN: ${{ github.token }}
@@ -164,6 +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 comparison/owners.txt - \
             | GH_TOKEN="$APP_GH_TOKEN" result/bin/request-reviewers.sh "$REPOSITORY" "$NUMBER" "$AUTHOR"
 
       - name: Log current API rate limits (app-token)

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"
   ''

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

ci/request-reviews/request-reviewers.sh

@@ -33,9 +33,41 @@ prAuthor=$3
 tmp=$(mktemp -d)
 trap 'rm -rf "$tmp"' exit
 
+# Associative array with the user as the key for easy de-duplication
+# Make sure to always lowercase keys to avoid duplicates with different casings
 declare -A users=()
 while read -r handle && [[ -n "$handle" ]]; do
-    users[${handle,,}]=
+    if [[ "$handle" =~ (.*)/(.*) ]]; then
+        # Teams look like $org/$team
+        org=${BASH_REMATCH[1]}
+        team=${BASH_REMATCH[2]}
+
+        # Instead of requesting a review from the team itself,
+        # we request reviews from the individual users.
+        # This is because once somebody from a team reviewed the PR,
+        # the API doesn't expose that the team was already requested for a review,
+        # so we wouldn't be able to avoid rerequesting reviews
+        # without saving some some extra state somewhere
+
+        # We could also consider implementing a more advanced heuristic
+        # in the future that e.g. only pings one team member,
+        # but escalates to somebody else if that member doesn't respond in time.
+        gh api \
+            --cache=1h \
+            -H "Accept: application/vnd.github+json" \
+            -H "X-GitHub-Api-Version: 2022-11-28" \
+            "/orgs/$org/teams/$team/members" \
+            --jq '.[].login' > "$tmp/team-members"
+        readarray -t members < "$tmp/team-members"
+        log "Team $entry has these members: ${members[*]}"
+
+        for user in "${members[@]}"; do
+            users[${user,,}]=
+        done
+    else
+        # Everything else is a user
+        users[${handle,,}]=
+    fi
 done
 
 # Cannot request a review from the author
@@ -68,7 +100,7 @@ for user in "${!users[@]}"; do
     fi
 done
 
-if [[ "${#users[@]}" -gt 10 ]]; then
+if [[ "${#users[@]}" -gt 15 ]]; then
     log "Too many reviewers (${!users[*]}), skipping review requests"
     exit 0
 fi