Skip to content

Instantly share code, notes, and snippets.

@Shastick

Shastick/land-check.sh

Last active Jan 18, 2021
Embed
What would you like to do?
#!/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://phab.open.ch/api/ --conduit-token ${conduit_token}"
working_copy="/repos/working-copies/23"
else
conduit="arc call-conduit --conduit-uri https://phab.open.ch/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
@Shastick

This comment has been minimized.

Copy link
Owner Author

@Shastick Shastick commented Jan 15, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment