Skip to content

Commit aaf1d6b

Browse files
infinisilinfinisil
committed
Various improvements
1 parent 237cad3 commit aaf1d6b

File tree

6 files changed

+193
-111
lines changed

6 files changed

+193
-111
lines changed

‎.github/OWNERS‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,19 @@
1-
# Like CODEOWNERS, but manually implemented with .github/workflows/codeowners.yml
2-
# Allows requesting reviews from people without write access.
3-
# And prevents mass pings when the base branch is wrong.
4-
# CODEOWNERS is intended to be migrated to this file soon.
1+
#
2+
# Currently unused! Use CODEOWNERS for now, see workflows/codeowners.yml
3+
#
4+
####################
5+
#
6+
# This file is used to describe who owns what in this repository.
7+
# Users/teams will get review requests for PRs that change their files.
8+
#
9+
# This file does not replace `meta.maintainers`
10+
# but is instead used for other things than derivations and modules,
11+
# like documentation, package sets, and other assets.
12+
#
13+
# This file uses the same syntax as the natively supported CODEOWNERS file,
14+
# see https://help.github.com/articles/about-codeowners/ for documentation.
15+
# However it comes with some notable differences:
16+
# - There is no need for user/team listed here to have write access.
17+
# - No reviews will be requested for PRs that target the wrong base branch.
18+
#
19+
# Processing of this file is implemented in workflows/codeowners.yml

‎.github/workflows/codeowners.yml‎

Lines changed: 36 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,5 @@
11
name: Codeowners
22

3-
on:
4-
pull_request_target:
5-
types: [opened, ready_for_review, synchronize, reopened, edited]
6-
7-
env:
8-
OWNERS_FILE: .github/OWNERS
9-
103
# This workflow depends on a GitHub App with the following permissions:
114
# - Repository > Administration: read-only
125
# - Organization > Members: read-only
@@ -15,27 +8,42 @@ env:
158
# the OWNER_APP_ID repository variable needs to be set
169
# the OWNER_APP_PRIVATE_KEY repository secret needs to be set
1710

11+
on:
12+
pull_request_target:
13+
types: [opened, ready_for_review, synchronize, reopened, edited]
14+
15+
env:
16+
# TODO: Once confirmed that this works by seeing that the action would request
17+
# reviews from the same people (or refuse for wrong base branches),
18+
# move all entries from CODEOWNERS to OWNERS and change this value here
19+
# OWNERS_FILE: .github/OWNERS
20+
OWNERS_FILE: .github/CODEOWNERS
21+
1822
jobs:
19-
check-owners:
23+
# Check that code owners is valid
24+
check:
2025
name: Check
2126
runs-on: ubuntu-latest
2227
steps:
23-
- uses: cachix/install-nix-action@ba0dd844c9180cbf77aa72a116d6fbc515d0e87b # v27
28+
- uses: cachix/install-nix-action@9f70348d77d0422624097c4b7a75563948901306 # v29
2429

25-
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
30+
# Important: Because we use pull_request_target, this checks out the base branch of the PR, not the PR itself.
31+
# We later build and run code from the base branch with access to secrets,
32+
# so it's important this is not the PRs code.
33+
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
2634
with:
2735
path: base
2836

2937
- name: Build codeowners validator
3038
run: nix-build base/ci -A codeownersValidator
3139

32-
- uses: actions/create-github-app-token@v1
40+
- uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0
3341
id: app-token
3442
with:
3543
app-id: ${{ vars.OWNER_APP_ID }}
3644
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
3745

38-
- uses: actions/checkout@v4
46+
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
3947
with:
4048
ref: refs/pull/${{ github.event.number }}/merge
4149
path: pr
@@ -50,26 +58,29 @@ jobs:
5058
# Set this to "notowned,avoid-shadowing" to check that all files are owned by somebody
5159
EXPERIMENTAL_CHECKS: "avoid-shadowing"
5260

61+
# Request reviews from code owners
5362
request:
5463
name: Request
5564
runs-on: ubuntu-latest
5665
# Don't trigger on draft PRs
5766
if: ${{ ! github.event.pull_request.draft }}
5867
steps:
59-
- uses: cachix/install-nix-action@ba0dd844c9180cbf77aa72a116d6fbc515d0e87b # v27
68+
- uses: cachix/install-nix-action@9f70348d77d0422624097c4b7a75563948901306 # v29
6069

61-
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4.1.7
70+
# Important: Because we use pull_request_target, this checks out the base branch of the PR, not the PR head.
71+
# This is intentional, because we need to request the review of owners as declared in the base branch.
72+
- uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
6273

63-
- name: Build review request package
64-
run: nix-build ci -A requestReviews
74+
- uses: actions/create-github-app-token@5d869da34e18e7287c1daad50e0b8ea0f506ce69 # v1.11.0
75+
id: app-token
76+
with:
77+
app-id: ${{ vars.OWNER_APP_ID }}
78+
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
6579

66-
- uses: actions/create-github-app-token@v1
67-
id: app-token
68-
with:
69-
app-id: ${{ vars.OWNER_APP_ID }}
70-
private-key: ${{ secrets.OWNER_APP_PRIVATE_KEY }}
80+
- name: Build review request package
81+
run: nix-build ci -A requestReviews
7182

72-
- name: Request reviews
73-
run: ./result/bin/request-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE"
74-
env:
75-
GH_TOKEN: ${{ steps.app-token.outputs.token }}
83+
- name: Request reviews
84+
run: result/bin/request-reviews.sh ${{ github.repository }} ${{ github.event.number }} "$OWNERS_FILE"
85+
env:
86+
GH_TOKEN: ${{ steps.app-token.outputs.token }}

‎ci/codeowners-validator/default.nix‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,13 @@ buildGoModule {
1818
url = "https://github.com/mszostok/codeowners-validator/compare/f3651e3810802a37bd965e6a9a7210728179d076...840eeb88b4da92bda3e13c838f67f6540b9e8529.patch";
1919
hash = "sha256-t3Dtt8SP9nbO3gBrM0nRE7+G6N/ZIaczDyVHYAG/6mU=";
2020
})
21-
./owners-file-name.patch
21+
# Undoes part of the above PR: We don't want to require write access
22+
# to the repository, that's only needed for GitHub's native CODEOWNERS.
23+
# Furthermore, it removes an unneccessary check from the code
24+
# that breaks tokens generated for GitHub Apps.
2225
./permissions.patch
26+
# Allows setting a custom CODEOWNERS path using the OWNERS_FILE env var
27+
./owners-file-name.patch
2328
];
2429
postPatch = "rm -r docs/investigation";
2530
vendorHash = "sha256-R+pW3xcfpkTRqfS2ETVOwG8PZr0iH5ewroiF7u8hcYI=";

‎ci/request-reviews/get-reviewers.sh‎

Lines changed: 34 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,50 +1,50 @@
11
#!/usr/bin/env bash
22

3-
# This script gets the list of codeowning users and teams based on a codeowners file
4-
# from a base commit and all files that have been changed since then.
5-
# The result is suitable as input to the GitHub REST API call to request reviewers for a PR.
6-
# This can be used to simulate the automatic codeowner review requests
3+
# Get the code owners of the files changed by a PR,
4+
# suitable to be consumed by the API endpoint to request reviews:
5+
# https://docs.github.com/en/rest/pulls/review-requests?apiVersion=2022-11-28#request-reviewers-for-a-pull-request
76

87
set -euo pipefail
98

10-
tmp=$(mktemp -d)
11-
trap 'rm -rf "$tmp"' exit
9+
log() {
10+
echo "$@" >&2
11+
}
1212

13-
if (( "$#" < 3 )); then
14-
echo "Usage: $0 LOCAL_REPO BASE_REF HEAD_REF OWNERS_FILE" >&2
13+
if (( "$#" < 5 )); then
14+
log "Usage: $0 GIT_REPO BASE_REF HEAD_REF OWNERS_FILE PR_AUTHOR"
1515
exit 1
1616
fi
17-
localRepo=$1
17+
18+
gitRepo=$1
1819
baseRef=$2
1920
headRef=$3
2021
ownersFile=$4
2122
prAuthor=$5
2223

23-
readarray -d '' -t touchedFiles < \
24-
<(
25-
# The names of all files, null-delimited, starting from HEAD, stopping before the base
26-
git -C "$localRepo" diff --name-only -z --merge-base "$baseRef" "$headRef" |
27-
# Remove duplicates
28-
sort -z --unique
29-
)
24+
tmp=$(mktemp -d)
25+
trap 'rm -rf "$tmp"' exit
3026

31-
#echo "These files were touched: ${touchedFiles[*]}" >&2
27+
readarray -t touchedFiles < \
28+
<(git -C "$gitRepo" diff --name-only --merge-base "$baseRef" "$headRef")
29+
30+
log "This PR touches ${#touchedFiles[@]} files"
3231

3332
# Get the owners file from the base, because we don't want to allow PRs to
3433
# remove code owners to avoid pinging them
35-
git -C "$localRepo" show "$baseRef":"$ownersFile" > "$tmp"/codeowners
34+
git -C "$gitRepo" show "$baseRef":"$ownersFile" > "$tmp"/codeowners
3635

37-
# Associative array, where the key is the team/user, while the value is "1"
38-
# This makes it very easy to get deduplication
36+
# Associative arrays with the team/user as the key for easy deduplication
3937
declare -A teams users
4038

4139
for file in "${touchedFiles[@]}"; do
42-
read -r file owners <<< "$(codeowners --file "$tmp"/codeowners "$file")"
40+
result=$(codeowners --file "$tmp"/codeowners "$file")
41+
42+
read -r file owners <<< "$result"
4343
if [[ "$owners" == "(unowned)" ]]; then
44-
#echo "File $file doesn't have an owner" >&2
44+
log "File $file is unowned"
4545
continue
4646
fi
47-
#echo "Owner of $file is $owners" >&2
47+
log "File $file is owned by $owners"
4848

4949
# Split up multiple owners, separated by arbitrary amounts of spaces
5050
IFS=" " read -r -a entries <<< "$owners"
@@ -53,27 +53,31 @@ for file in "${touchedFiles[@]}"; do
5353
# GitHub technically also supports Emails as code owners,
5454
# but we can't easily support that, so let's not
5555
if [[ ! "$entry" =~ @(.*) ]]; then
56-
echo -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" >&2
56+
warn -e "\e[33mCodeowner \"$entry\" for file $file is not valid: Must start with \"@\"\e[0m" >&2
5757
# Don't fail, because the PR for which this script runs can't fix it,
5858
# it has to be fixed in the base branch
5959
continue
6060
fi
6161
# The first regex match is everything after the @
6262
entry=${BASH_REMATCH[1]}
6363
if [[ "$entry" =~ .*/(.*) ]]; then
64-
# Only teams have a /
65-
teams[${BASH_REMATCH[1]}]=1
64+
# Teams look like $org/$team, where we only need $team for the API
65+
# call to request reviews from teams
66+
teams[${BASH_REMATCH[1]}]=
6667
else
6768
# Everything else is a user
68-
# But cannot request a review from the author
69-
if [[ "$entry" != "$prAuthor" ]]; then
70-
users[$entry]=1
71-
fi
69+
users[$entry]=
7270
fi
7371
done
7472

7573
done
7674

75+
# Cannot request a review from the author
76+
if [[ -v users[$prAuthor] ]]; then
77+
log "One or more files are owned by the PR author, ignoring"
78+
unset 'users[$prAuthor]'
79+
fi
80+
7781
# Turn it into a JSON for the GitHub API call to request PR reviewers
7882
jq -n \
7983
--arg users "${!users[*]}" \

‎ci/request-reviews/request-reviews.sh‎

Lines changed: 41 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,76 @@
11
#!/usr/bin/env bash
22

3+
# Requests reviews for a PR after verifying that the base branch is correct
4+
35
set -euo pipefail
46
tmp=$(mktemp -d)
57
trap 'rm -rf "$tmp"' exit
68
SCRIPT_DIR=$(dirname "$0")
79

10+
log() {
11+
echo "$@" >&2
12+
}
13+
14+
if (( $# < 3 )); then
15+
log "Usage: $0 GITHUB_REPO PR_NUMBER OWNERS_FILE"
16+
exit 1
17+
fi
818
baseRepo=$1
919
prNumber=$2
1020
ownersFile=$3
1121

22+
log "Fetching PR info"
1223
prInfo=$(gh api \
1324
-H "Accept: application/vnd.github+json" \
1425
-H "X-GitHub-Api-Version: 2022-11-28" \
1526
"/repos/$baseRepo/pulls/$prNumber")
1627

1728
baseBranch=$(jq -r .base.ref <<< "$prInfo")
29+
log "Base branch: $baseBranch"
1830
prRepo=$(jq -r .head.repo.full_name <<< "$prInfo")
31+
log "PR repo: $prRepo"
1932
prBranch=$(jq -r .head.ref <<< "$prInfo")
33+
log "PR branch: $prBranch"
2034
prAuthor=$(jq -r .user.login <<< "$prInfo")
35+
log "PR author: $prAuthor"
2136

22-
headRef=refs/remotes/fork/pr
37+
extraArgs=()
38+
if pwdRepo=$(git rev-parse --show-toplevel 2>/dev/null); then
39+
# Speedup for local runs
40+
extraArgs+=(--reference-if-able "$pwdRepo")
41+
fi
42+
43+
log "Fetching Nixpkgs commit history"
44+
git clone --bare --filter=tree:0 --no-tags --origin upstream "${extraArgs[@]}" https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git
2345

24-
git clone --bare --filter=tree:0 --no-tags --origin upstream https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git
46+
log "Fetching the PR commit history"
2547
# Fetch the PR
2648
git -C "$tmp/nixpkgs.git" remote add fork https://github.com/"$prRepo".git
2749
# Make sure we only fetch the commit history, nothing else
2850
git -C "$tmp/nixpkgs.git" config remote.fork.promisor true
2951
git -C "$tmp/nixpkgs.git" config remote.fork.partialclonefilter tree:0
52+
53+
# This should not conflict with any refs in Nixpkgs
54+
headRef=refs/remotes/fork/pr
3055
# Only fetch into a remote ref, because the local ref namespace is used by Nixpkgs, don't want any conflicts
3156
git -C "$tmp/nixpkgs.git" fetch --no-tags fork "$prBranch":"$headRef"
3257

33-
58+
log "Checking correctness of the base branch"
3459
"$SCRIPT_DIR"/verify-base-branch.sh "$tmp/nixpkgs.git" "$headRef" "$baseRepo" "$baseBranch" "$prRepo" "$prBranch"
3560

36-
reviewersJSON=$("$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile" "$prAuthor")
61+
log "Getting code owners to request reviews from"
62+
"$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile" "$prAuthor" > "$tmp/reviewers.json"
3763

38-
echo "$reviewersJSON"
64+
log "Requesting reviews from: $(<"$tmp/reviewers.json")"
3965

40-
if ! response=$(curl -LsS --fail-with-body \
41-
-X POST \
42-
-H "Accept: application/vnd.github+json" \
43-
-H "Authorization: Bearer $(gh auth token)" \
44-
-H "X-GitHub-Api-Version: 2022-11-28" \
45-
https://api.github.com/repos/"$baseRepo"/pulls/"$prNumber"/requested_reviewers \
46-
-d "$reviewersJSON"); then
47-
echo "Failed to request reviews: $response"
48-
exit 1
66+
if ! response=$(gh api \
67+
--method POST \
68+
-H "Accept: application/vnd.github+json" \
69+
-H "X-GitHub-Api-Version: 2022-11-28" \
70+
"/repos/$baseRepo/pulls/$prNumber/requested_reviewers" \
71+
--input "$tmp/reviewers.json"); then
72+
log "Failed to request reviews: $response"
73+
exit 1
4974
fi
75+
76+
log "Successfully requested reviews"

0 commit comments

Comments
 (0)