Last active
May 17, 2023 09:23
-
-
Save Shastick/dfdc42b49a4191e82ce1ed2853b561a4 to your computer and use it in GitHub Desktop.
Contains a (draft) pre-receive commit hook to be used on a phabricator instance in order to verify that inbound commits correspond to the content of their Diff.
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
#!/bin/bash | |
set -eo pipefail | |
# Note to readers: this hook lives within a phabricator instance that runs from a xiimoon/phabricator docker image. | |
# Additionally, you'll need: | |
# - jq | |
# - cmp | |
# - mounting a valid conduit api token to /env_dir/PHABRICATOR_CONDUIT_TOKEN | |
# | |
# For cases where this hook yields false positives, it will also check if the diff corresponding to the commit being pushed | |
# has been accepted by a special 'bypass' group, in which case the commit will not be compared to the diff | |
# | |
# Note that this is a WIP: reading env vars from a hook (at least in phabricator) does not work, | |
# hence some improvements are required | |
if [ -t 0 ]; then | |
# Useful for testing this script | |
echo "Running in interactive mode..." | |
if [ $# -ne 2 ]; then | |
echo "Missing parameter(s)!" | |
echo "usage: $0 <from_revision> <to_revision>" | |
exit 1 | |
fi | |
oldrev="$1" | |
newrev="$2" | |
else | |
# When run as a commit hook via phabricator, arguments are passed via Stdin | |
read -rs oldrev newrev _ | |
fi | |
# Group that bypasses the checks if it accepted the pushed commits Diff. | |
bypass_group=${LAND_CHECK_BYPASS_GROUP:-"#flight_control"} | |
# If set to anything else than 'true', the checks will run, | |
# Set this ENV VAR to 'true' to disable the hook entirely. | |
# Meant to be used if we have a bad bug and need to disable checks temporarily | |
disabled=${LAND_CHECK_DISABLED:-"false"} | |
main_branch="master" | |
working_dir=$(pwd) | |
if [[ $disabled == "true" ]]; then | |
printf "\n\tWARNING:\tLANDING CHECKS ARE DISABLED.\n" | |
printf "\t\t\tIF THIS IS NOT EXPECTED CONTACT THE BUILD TEAM.\n\n" | |
exit 0; | |
fi | |
revlist=$(git rev-list "$oldrev".."$newrev") | |
# Env vars aren't exposed to hooks. Using the filesystem instead... | |
conduit_token=${PHABRICATOR_CONDUIT_TOKEN:-$(cat /env_hack/PHABRICATOR_CONDUIT_TOKEN)} | |
if [[ -z "${conduit_token}" ]]; then | |
echo "Conduit token not set for land-check. Please fix." | |
exit 1 | |
fi | |
# Set LAND_CHECK_DEV to anything so that the script may be run | |
# locally on a dev's workstation | |
if [[ -z "${LAND_CHECK_DEV}" ]]; then | |
conduit="/srv/phabricator/phabricator/bin/conduit call --local --input - --method" | |
res_field="result" | |
arc="/srv/phabricator/arcanist/bin/arc --conduit-uri http://<your_instance>/api/ --conduit-token ${conduit_token}" | |
working_copy="/repos/working-copies/23" | |
else | |
conduit="arc call-conduit --conduit-uri https://<your_instance>/api -- " | |
res_field="response" | |
arc="arc" | |
working_copy="/tmp/test_checkout" | |
fi | |
# Iterate over all the commits | |
for commit in $revlist; do | |
# Cat the commit message's content, only retain the last line | |
# (Which is where we read the revision identifier from) | |
# If you modify this, be aware that we would not want user-generated content to be mistaken | |
# for the differential revision | |
diff_id=$(git cat-file commit "${commit}" | tail -n1 | grep "Differential Revision" | cut -d "/" -f 4) | |
phids_lookup=$(echo '{"names": ["'"${diff_id}"'","'"$bypass_group"'"]}' | $conduit phid.lookup) | |
diff_phid=$(echo "${phids_lookup}" | jq ".${res_field}.${diff_id}.phid") | |
# Some quoting fun, because the group may contain a '#', jq needs it to be double quoted. | |
bypass_group_phid=$(echo "${phids_lookup}" | jq ".${res_field}."'"'"$bypass_group"'"'".phid") | |
# echo "Revision ${diff_id}. Diff PHID: ${diff_phid}, Bypass group phid: ${bypass_group_phid}" | |
diff_details_json=$(echo '{"constraints": {"phids": ['"${diff_phid}"']}, "attachments": {"reviewers": true}}' \ | |
| $conduit differential.revision.search) | |
# Check if the 'bypass' group ID is a reviewer and has accepted the diff | |
bypass_reviewer=$(echo "${diff_details_json}" \ | |
| jq ".${res_field}.data[0].attachments.reviewers.reviewers" \ | |
| jq "map(select( any(.; .reviewerPHID == ${bypass_group_phid} )))") | |
# Note: if the bypass group has not reviewed, the returned value will be "null" | |
bypass_status=$(echo "${bypass_reviewer}" | jq '.[0].status') | |
if [[ $bypass_status == '"accepted"' ]]; then | |
echo "NOTE: This commit will bypass the additional landing checks, as its revision was accepted by ${bypass_group}" | |
exit 0 | |
else | |
printf "\nThis commit is subject to a comparison with the reviewed code\n" | |
fi | |
# Need to compare patches | |
reviewed_patch="/tmp/patch_check_reviewed${newrev}.patch" | |
inbound_patch="/tmp/patch_check_inbound${newrev}.patch" | |
# Build the patch for the inbound commit: easy | |
git diff "$oldrev".."$newrev" > "${inbound_patch}" | |
# Build the patch for the reviewed code: more involved | |
# Move to the working copy, make sure it's up to date and master is checked out | |
cd "${working_copy}" | |
# Must be unset otherwise this takes precedence | |
unset GIT_DIR | |
# Must be unset otherwise we're not allowed to update any ref | |
unset GIT_QUARANTINE_PATH | |
# Make sure the working copy is up to date | |
git fetch -v --quiet | |
# Set it to the revision the inbound commit is applied to | |
git reset --hard "${oldrev}" --quiet | |
$arc patch --revision "${diff_id}" --nobranch --nocommit > /dev/null | |
git diff $main_branch > "${reviewed_patch}" | |
# Move back to the working directory (only required when run on a dev's machine) | |
cd "$working_dir" | |
# Note: cmp gives absolutely no details | |
if cmp "${inbound_patch}" "${reviewed_patch}" | |
then | |
printf "\nAll Good: commit land may proceed.\n" | |
else | |
echo | |
echo "Mismatch between this commit and the corresponding reviewed diff (${diff_id})" | |
echo "please update your diff prior to landing. If this persists, check with the build crew or members of ${bypass_group}" | |
exit 1 | |
fi | |
rm "${reviewed_patch}" "${inbound_patch}" | |
done |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Note to readers: the above (3rd revision) has issues in scenarios where a file is both copied and moved. Arc and git may generate two patch files that are functionally equivalent but not comparable in a naïve fashion.
This might get fixed at some point.
edit: the hook now updates a separate working copy and compares effective
git diff
output. Note that currently working copies must be created manually beforehand.