-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
WIP: Scripts to handle codeowner pings, preventing mass pings
- Loading branch information
Showing
3 changed files
with
202 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
#!/usr/bin/env nix-shell | ||
#!nix-shell --pure -i bash -p codeowners jq gitMinimal cacert | ||
|
||
# This script gets the list of codeowning users and teams based on a codeowners file | ||
# from a base commit and all files that have been changed since then. | ||
# The result is suitable as input to the GitHub REST API call to request reviewers for a PR. | ||
# This can be used to simulate the automatic codeowner review requests | ||
|
||
set -euo pipefail | ||
|
||
tmp=$(mktemp -d) | ||
trap 'rm -rf "$tmp"' exit | ||
|
||
if (( "$#" < 3 )); then | ||
echo "Usage: $0 LOCAL_REPO BASE_REF HEAD_REF OWNERS_FILE" >&2 | ||
exit 1 | ||
fi | ||
localRepo=$1 | ||
baseRef=$2 | ||
headRef=$3 | ||
ownersFile=$4 | ||
|
||
readarray -d '' -t touchedFiles < \ | ||
<( | ||
# The names of all files, null-delimited, starting from HEAD, stopping before the base | ||
git -C "$localRepo" diff --name-only -z --merge-base "$baseRef" "$headRef" | | ||
# Remove duplicates | ||
sort -z --unique | ||
) | ||
|
||
#echo "These files were touched: ${touchedFiles[*]}" >&2 | ||
|
||
# Get the owners file from the base, because we don't want to allow PRs to | ||
# remove code owners to avoid pinging them | ||
git show "$baseRef":"$ownersFile" > "$tmp"/codeowners | ||
|
||
# Associative array, where the key is the team/user, while the value is "1" | ||
# This makes it very easy to get deduplication | ||
declare -A teams users | ||
|
||
for file in "${touchedFiles[@]}"; do | ||
read -r file owners <<< "$(codeowners --file "$tmp"/codeowners "$file")" | ||
if [[ "$owners" == "(unowned)" ]]; then | ||
#echo "File $file doesn't have an owner" >&2 | ||
continue | ||
fi | ||
#echo "Owner of $file is $owners" >&2 | ||
|
||
# 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" >&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]} | ||
if [[ "$entry" == */* ]]; then | ||
# Only teams have a / | ||
teams[$entry]=1 | ||
else | ||
# Everything else is a user | ||
users[$entry]=1 | ||
fi | ||
done | ||
|
||
done | ||
|
||
# Turn it into a JSON for the GitHub API call to request PR reviewers | ||
jq -n \ | ||
--arg users "${!users[*]}" \ | ||
--arg teams "${!teams[*]}" \ | ||
'{ | ||
reviewers: $users | split(" "), | ||
team_reviewers: $teams | split(" ") | ||
}' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
#!/usr/bin/env bash | ||
|
||
set -euo pipefail | ||
tmp=$(mktemp -d) | ||
trap 'rm -rf "$tmp"' exit | ||
SCRIPT_DIR=$(dirname "$0") | ||
|
||
baseRepo=$1 | ||
prNumber=$2 | ||
ownersFile=$3 | ||
|
||
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") | ||
prRepo=$(jq -r .head.repo.full_name <<< "$prInfo") | ||
prBranch=$(jq -r .head.ref <<< "$prInfo") | ||
|
||
headRef=refs/remotes/fork/pr | ||
|
||
git clone --bare --filter=tree:0 --no-tags --origin upstream https://github.com/"$baseRepo".git "$tmp"/nixpkgs.git | ||
# Fetch the PR | ||
git -C "$tmp/nixpkgs.git" remote add fork https://github.com/"$prRepo".git | ||
# Make sure we only fetch the commit history, nothing else | ||
git -C "$tmp/nixpkgs.git" config remote.fork.promisor true | ||
git -C "$tmp/nixpkgs.git" config remote.fork.partialclonefilter tree:0 | ||
# Only fetch into a remote ref, because the local ref namespace is used by Nixpkgs, don't want any conflicts | ||
git -C "$tmp/nixpkgs.git" fetch --no-tags fork "$prBranch":"$headRef" | ||
|
||
|
||
"$SCRIPT_DIR"/verify-base-branch.sh "$tmp/nixpkgs.git" "$headRef" "$baseRepo" "$baseBranch" "$prRepo" "$prBranch" | ||
|
||
reviewersJSON=$("$SCRIPT_DIR"/get-reviewers.sh "$tmp/nixpkgs.git" "$baseBranch" "$headRef" "$ownersFile") | ||
|
||
echo "$reviewersJSON" | ||
exit 0 | ||
|
||
curl -L \ | ||
-X POST \ | ||
-H "Accept: application/vnd.github+json" \ | ||
-H "Authorization: Bearer $GITHUB_TOKEN" \ | ||
-H "X-GitHub-Api-Version: 2022-11-28" \ | ||
https://api.github.com/repos/"$baseRepo"/pulls/"$prNumber"/requested_reviewers \ | ||
-d "$reviewersJSON" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
#!/usr/bin/env nix-shell | ||
#!nix-shell -i bash --pure -p gitMinimal cacert | ||
|
||
# This script Checks that a PR doesn't include commits that are already in other development branches | ||
# This commonly happens when users pick the wrong base branch for a PR | ||
|
||
set -euo pipefail | ||
|
||
# Small helper to check whether an element is in a list | ||
# Usage: `elementIn foo "${list[@]}"` | ||
elementIn() { | ||
local e match=$1 | ||
shift | ||
for e; do | ||
if [[ "$e" == "$match" ]]; then | ||
return 0 | ||
fi | ||
done | ||
return 1 | ||
} | ||
|
||
if (( $# < 5 )); then | ||
echo "Usage: $0 LOCAL_REPO PR_HEAD_REF BASE_REPO BASE_BRANCH PR_REPO PR_BRANCH" | ||
exit 1 | ||
fi | ||
localRepo=$1 | ||
headRef=$2 | ||
baseRepo=$3 | ||
baseBranch=$4 | ||
prRepo=$5 | ||
prBranch=$6 | ||
|
||
readarray -t developmentBranches < <(git -C "$localRepo" branch --list --format "%(refname:short)" {master,staging{,-next}} 'release-*' 'staging-*' 'staging-next-*') | ||
|
||
if ! elementIn "$baseBranch" "${developmentBranches[@]}"; then | ||
echo "PR does not go to any base branch among (${developmentBranches[*]}), no commit check necessary" >&2 | ||
exit 0 | ||
fi | ||
|
||
if [[ "$baseRepo" == "$prRepo" ]] && elementIn "$prBranch" "${developmentBranches[@]}"; then | ||
echo "This is a merge of $prBranch into $baseBranch, no commit check necessary" >&2 | ||
exit 0 | ||
fi | ||
|
||
for branch in "${developmentBranches[@]}"; do | ||
|
||
if [[ -z "$(git -C "$localRepo" rev-list -1 --since="1 year ago" "$branch")" ]]; then | ||
# Skip branches that haven't been active for a year | ||
continue | ||
fi | ||
echo "Checking for extra commits from branch $branch" >&2 | ||
|
||
# The first ancestor of the PR head that already exists in the other branch | ||
mergeBase=$(git -C "$localRepo" merge-base "$headRef" "$branch") | ||
|
||
# The number of commits that are reachable from the PR head, not reachable from the PRs base branch | ||
# (up to here this would be the number of commits in the PR itself), | ||
# but that are _also_ in the development branch we're testing against. | ||
# So, in other words, the number of commits that the PR includes from other development branches | ||
count=$(git -C "$localRepo" rev-list --count "$mergeBase" ^"$baseBranch") | ||
|
||
if (( count != 0 )); then | ||
echo -en "\e[31m" | ||
echo "This PR's base branch is set to $baseBranch, but $count already-merged commits are included from the $branch branch." | ||
echo "To remedy this, first make sure you know the target branch for your changes: https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md#branch-conventions" | ||
echo "- If the changes should go to the $branch branch instead, change the base branch accordingly:" | ||
echo " https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-base-branch-of-a-pull-request" | ||
echo "- If the changes should really go to the $baseBranch branch, rebase your PR on top of the merge base with the $branch branch:" | ||
echo " git rebase --onto $mergeBase && git push --force-with-lease" | ||
echo -en "\e[0m" | ||
exit 1 | ||
fi | ||
done | ||
|
||
echo "All good, no extra commits from any development branch" |