Skip to content

Consider all companions from all PRs descriptions #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Dec 17, 2021
Merged

Consider all companions from all PRs descriptions #19

Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d4b1643
consider all companions from all PRs descriptions
joao-paulo-parity Dec 14, 2021
3b2ef59
avoid merging master before the check for the sake of more determinis…
joao-paulo-parity Dec 15, 2021
343a601
enhance docs
joao-paulo-parity Dec 15, 2021
a40d43f
remove escape hatch as it should no longer be needed
joao-paulo-parity Dec 15, 2021
038e89c
more clarity
joao-paulo-parity Dec 15, 2021
1c41c71
simplify
joao-paulo-parity Dec 15, 2021
636a745
validate diener patch targets
joao-paulo-parity Dec 15, 2021
75c8793
detect the dependent's companions automatically
joao-paulo-parity Dec 15, 2021
26271b2
fix
joao-paulo-parity Dec 15, 2021
d23121a
clone the dependent's master branch if it was not cloned as a companion
joao-paulo-parity Dec 15, 2021
cc3feef
simplify jq expression
joao-paulo-parity Dec 15, 2021
91f36b3
revert simplify
joao-paulo-parity Dec 15, 2021
d2da7e8
fixes
joao-paulo-parity Dec 15, 2021
f9c059f
fix
joao-paulo-parity Dec 15, 2021
f7fe682
fix
joao-paulo-parity Dec 15, 2021
f7742fa
support any companion through --target
joao-paulo-parity Dec 15, 2021
0bf3f55
simplify array expansion
joao-paulo-parity Dec 15, 2021
dc3e706
use --target since it is more flexible
joao-paulo-parity Dec 15, 2021
724e18b
docs
joao-paulo-parity Dec 15, 2021
44f1372
consistent logging style
joao-paulo-parity Dec 15, 2021
c0ae1c3
revert array expansion change
joao-paulo-parity Dec 15, 2021
1dab432
echo PIPELINE_SCRIPTS_TAG
joao-paulo-parity Dec 15, 2021
a02d7de
fix
joao-paulo-parity Dec 15, 2021
dd69fbb
newline for readability
joao-paulo-parity Dec 15, 2021
dae5933
merge master onto branches before checks
joao-paulo-parity Dec 17, 2021
c8d06b2
logging and docs
joao-paulo-parity Dec 17, 2021
fdb9617
prevent logging misinformation as the tag not used at the moment
joao-paulo-parity Dec 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
304 changes: 181 additions & 123 deletions check_dependent_project.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ check_dependent_project

This check ensures that this project's dependents do not suffer downstream breakages from new code
changes.

"

set -eu -o pipefail
Expand All @@ -39,19 +38,22 @@ github_api_token="$5"
update_crates_on_default_branch="$6"

this_repo_dir="$PWD"
companions_dir="$this_repo_dir/companions"
github_api="https://api.github.com"
org_github_prefix="https://github.com/$org"
org_crates_prefix="git+$org_github_prefix"

our_crates=()
our_crates_source="git+https://github.com/$org/$this_repo"
our_crates_source="$org_crates_prefix/$this_repo"
discover_our_crates() {
# workaround for early exits not being detected in command substitution
# https://unix.stackexchange.com/questions/541969/nested-command-substitution-does-not-stop-a-script-on-a-failure-even-if-e-and-s
local last_line

local found
while IFS= read -r crate; do
last_line="$crate"
# for avoiding duplicate entries
local found
for our_crate in "${our_crates[@]}"; do
if [ "$crate" == "$our_crate" ]; then
found=true
Expand All @@ -63,8 +65,8 @@ discover_our_crates() {
else
our_crates+=("$crate")
fi
# dependents with {"source": null} are the ones we own, hence the getpath($p)==null in the jq
# script below
# dependencies with {"source": null} are the ones in this project's workspace,
# hence the getpath($p)==null in the jq script below
done < <(cargo metadata --quiet --format-version=1 | jq -r '
. as $in |
paths |
Expand All @@ -77,10 +79,11 @@ discover_our_crates() {
fi
}

match_their_crates() {
dependent_companions=()
match_dependent_crates() {
local target_name="$1"
local crates_not_found=()
local found
dependent_companions=()

# workaround for early exits not being detected in command substitution
# https://unix.stackexchange.com/questions/541969/nested-command-substitution-does-not-stop-a-script-on-a-failure-even-if-e-and-s
Expand All @@ -101,7 +104,28 @@ match_their_crates() {
;;
source)
next="crate"

for comp in "${companions[@]}"; do
local companion_crate_source="$org_crates_prefix/$comp"
if [ "$line" == "$companion_crate_source" ] || [[ "$line" == "$companion_crate_source?"* ]]; then
# prevent duplicates in dependent_companions
local found
for dep_comp in "${dependent_companions[@]}"; do
if [ "$dep_comp" == "$comp" ]; then
found=true
break
fi
done
if [ "${found:-}" ]; then
unset found
else
dependent_companions+=("$comp")
fi
fi
done

if [ "$line" == "$our_crates_source" ] || [[ "$line" == "$our_crates_source?"* ]]; then
local found
for our_crate in "${our_crates[@]}"; do
if [ "$our_crate" == "$crate" ]; then
found=true
Expand Down Expand Up @@ -145,78 +169,156 @@ match_their_crates() {
if [ "${crates_not_found[@]}" ]; then
echo -e "Errors during crate matching\n"
printf "Failed to detect our crate \"%s\" referenced in $target_name\n" "${crates_not_found[@]}"
echo -e "\nNote: this error generally happens if you have deleted or renamed a crate and did not update it in $target_name. Consider opening a companion pull request on $target_name and referencing it in this pull request's description like:\n$target_name companion: [your companion PR here]"
echo -e "\nNote: this error generally happens if you have deleted or renamed a crate and did not update it in $target_name. Consider opening a companion pull request on $target_name and referencing it in this PR's description like:\n$target_name companion: [your companion PR here]"
die "Check failed"
fi
}

patch_and_check_dependent() {
match_their_crates "$(basename "$PWD")"

# Update the crates to the latest version.
#
# This is for example needed if there was a pr to Substrate that only required a Polkadot companion
# and Cumulus wasn't yet updated to use the latest commit of Polkadot.
for update in $update_crates_on_default_branch; do
cargo update -p $update
done

diener patch --crates-to-patch "$this_repo_dir" "$this_repo_diener_arg" --path "Cargo.toml"
eval "${COMPANION_CHECK_COMMAND:-cargo check --all-targets --workspace}"
}
companions=()
process_pr_description_line() {
local companion_expr="$1"
local source="$2"

process_companion_pr() {
# e.g. https://github.com/paritytech/polkadot/pull/123
# or polkadot#123
local companion_expr="$1"
if
[[ "$companion_expr" =~ ^https://github\.com/$org/([^/]+)/pull/([[:digit:]]+) ]] ||
[[ "$companion_expr" =~ ^$org/([^#]+)#([[:digit:]]+) ]] ||
[[ "$companion_expr" =~ ^([^#]+)#([[:digit:]]+) ]]; then
local companion_repo="${BASH_REMATCH[1]}"
local companion_pr_number="${BASH_REMATCH[2]}"
echo "Parsed companion_repo=$companion_repo and companion_pr_number=$companion_pr_number from $companion_expr (trying to match companion_repo=$dependent_repo)"
[[ "$companion_expr" =~ ^([^#]+)#([[:digit:]]+) ]]
then
local repo="${BASH_REMATCH[1]}"
local pr_number="${BASH_REMATCH[2]}"
echo "Parsed companion repo=$repo and pr_number=$pr_number in $companion_expr from $source"

if [ "$this_repo" == "$repo" ]; then
echo "Skipping $companion_expr as it refers to the repository where this script is currently running"
return
fi

# keep track of duplicated companion references not only to avoid useless
# work but also to avoid infinite mutual recursion when 2+ PRs reference
# each other
for comp in "${companions[@]}"; do
if [ "$comp" == "$repo" ]; then
echo "Skipping $companion_expr as the repository $repo has already been registered before"
return
fi
done
companions+=("$repo")

# Heuristic: assume the companion PR has a common merge ancestor with master
# in its last N commits.
local merge_ancestor_max_depth=100

# Clone the default branch of this companion's target repository (assumed to
# be named "master")
git clone \
--depth=$merge_ancestor_max_depth \
"https://github.com/$org/$repo.git" \
"$companions_dir/$repo"
pushd "$companions_dir/$repo" >/dev/null

# Clone the companion's branch
local ref="$(curl \
-sSL \
-H "Authorization: token $github_api_token" \
"$github_api/repos/$org/$repo/pulls/$pr_number" | \
jq -e -r ".head.ref"
)"
git fetch --depth=$merge_ancestor_max_depth origin "pull/$pr_number/head:$ref"
git checkout "$ref"

echo "
Attempting to merge $repo#$pr_number with master after fetching its last $merge_ancestor_max_depth commits.

If this step fails, either:

- $repo#$pr_number has conflicts with master

OR

- A common merge ancestor could not be found between master and the last $merge_ancestor_max_depth commits of $repo#$pr_number.

Both cases can be solved by merging master into $repo#$pr_number.
"
git show-ref origin/master
git merge origin/master \
--verbose \
--no-edit \
-m "Merge master of $repo into companion $repo#$pr_number"

popd >/dev/null

# collect also the companions of companions
process_pr_description "$repo" "$pr_number"
else
die "Companion PR description had invalid format or did not belong to organization $org: $companion_expr"
die "Companion in the PR description of $source had invalid format or did not belong to organization $org: $companion_expr"
fi
}

if [ "$companion_repo" != "$dependent_repo" ]; then
echo "companion repo $companion_repo doesn't match dependent repo $dependent_repo. Check that you pasted the pure link to the companion."
process_pr_description() {
local repo="$1"
local pr_number="$2"

if ! [[ "$pr_number" =~ ^[[:digit:]]+$ ]]; then
return
fi

was_companion_found=true
echo "Processing PR $repo#$pr_number"

read -d '\n' -r mergeable pr_head_ref pr_head_sha < <(curl \
local lines=()
while IFS= read -r line; do
lines+=("$line")
done < <(curl \
-sSL \
-H "Authorization: token $github_api_token" \
"$github_api/repos/$org/$companion_repo/pulls/$companion_pr_number" | \
jq -r "[
.mergeable // error(\"Companion $companion_expr is not mergeable\"),
.head.ref // error(\"Missing .head.ref from API data of $companion_expr\"),
.head.sha // error(\"Missing .head.sha from API data of $companion_expr\")
] | .[]"
# https://stackoverflow.com/questions/40547032/bash-read-returns-with-exit-code-1-even-though-it-runs-as-expected
# ignore the faulty exit code since read still is regardless still reading the values we want
) || :

local expected_mergeable=true
if [ "$mergeable" != "$expected_mergeable" ]; then
die "Github API says $companion_expr is not mergeable (got $mergeable, expected $expected_mergeable)"
"$github_api/repos/$org/$repo/pulls/$pr_number" | \
jq -e -r ".body"
)
# in case the PR has no body, jq should have printed "null" which effectively
# means lines will always be populated with something
if ! [ "$lines" ]; then
die "No lines were read for the description of PR $pr_number (some error probably occurred)"
fi

echo
echo "merging master into the pr..."
git clone --depth 100 "https://github.com/$org/$companion_repo.git"
pushd "$companion_repo" >/dev/null
git fetch origin "pull/$companion_pr_number/head:$pr_head_ref"
git checkout "$pr_head_sha"
git merge master --verbose --no-edit -m "master was merged into the pr by check_dependent_project.sh process_companion_pr()"
echo "done"
echo
for line in "${lines[@]}"; do
if [[ "$line" =~ [cC]ompanion:[[:space:]]*([^[:space:]]+) ]]; then
echo "Detected companion in the PR description of $repo#$pr_number: ${BASH_REMATCH[1]}"
process_pr_description_line "${BASH_REMATCH[1]}" "$repo#$pr_number"
fi
done
}

patch_and_check_dependent() {
local dependent="$1"
local dependent_repo_dir="$2"

pushd "$dependent_repo_dir" >/dev/null

match_dependent_crates "$dependent"

# Update the crates to the latest version. This is for example needed if there
# was a PR to Substrate which only required a Polkadot companion and Cumulus
# wasn't yet updated to use the latest commit of Polkadot.
for update in $update_crates_on_default_branch; do
cargo update -p "$update"
done

for comp in "${dependent_companions[@]}"; do
echo "Patching $comp companion into $dependent"
diener patch \
--target "$org_github_prefix/$comp" \
--crates-to-patch "$companions_dir/$comp" \
--path "Cargo.toml"
done

echo "Patching $this_repo into $dependent"
diener patch \
--target "$org_github_prefix/$this_repo" \
--crates-to-patch "$this_repo_dir" \
--path "Cargo.toml"

echo "running checks for the companion $companion_expr of $companion_repo"
patch_and_check_dependent
eval "${COMPANION_CHECK_COMMAND:-cargo check --all-targets --workspace}"

popd >/dev/null
}
Expand All @@ -227,77 +329,33 @@ main() {
git config --global user.email '<>'
git config --global pull.rebase false

echo
echo "merging master into the pr..."
# Merge master into our branch so that the compilation takes into account how the code is going to
# perform when the code for this pull request lands on the target branch (à la pre-merge pipelines).
# Note that the target branch might not actually be master, but we default to it in the assumption
# of the common case. This could be refined in the future.
git fetch origin +master:master
git fetch origin +$CI_COMMIT_REF_NAME:$CI_COMMIT_REF_NAME
git checkout $CI_COMMIT_REF_NAME
git merge master --verbose --no-edit -m "master was merged into the pr by check_dependent_project.sh main()"
echo "done"
echo
# Merge master into this branch so that we have a better expectation of the
# integration still working after this PR lands.
# Since master's HEAD is being merged here, at the start the dependency chain,
# the same has to be done for all the companions because they might have
# accompanying changes for the code being brought in.
git fetch --force origin master
git show-ref origin/master
echo "Merge master into $CI_COMMIT_REF_NAME"
git merge origin/master \
--verbose \
--no-edit \
-m "Merge master into $CI_COMMIT_REF_NAME"

discover_our_crates

if [[ "$CI_COMMIT_REF_NAME" =~ ^[[:digit:]]+$ ]]; then
echo "this is pull request number $CI_COMMIT_REF_NAME"

# workaround for early exits not being detected in command substitution
# https://unix.stackexchange.com/questions/541969/nested-command-substitution-does-not-stop-a-script-on-a-failure-even-if-e-and-s
local last_line

while IFS= read -r line; do
last_line="$line"

if [[ "$line" =~ [cC]ompanion:[[:space:]]*([^[:space:]]+) ]]; then
echo "Detected companion in PR description: ${BASH_REMATCH[1]}"
process_companion_pr "${BASH_REMATCH[1]}"
elif [[ "$line" =~ skip[^[:alnum:]]+([^[:space:]]+) ]]; then
# FIXME: This escape hatch should be removed at some point when the
# companion build system is able to deal with all edge cases, such as
# the one described in
# https://github.com/paritytech/pipeline-scripts/issues/3#issuecomment-947539791
if [[
"${BASH_REMATCH[1]}" = "$CI_JOB_NAME" ||
"${BASH_REMATCH[1]}" = "continuous-integration/gitlab-$CI_JOB_NAME"
]]; then
echo "Skipping $CI_JOB_NAME as specified in the PR description"
exit
fi
fi
done < <(curl \
-sSL \
-H "Authorization: token $GITHUB_TOKEN" \
"$github_api/repos/$org/$this_repo/pulls/$CI_COMMIT_REF_NAME" | \
jq -e -r ".body"
)
if [ -z "${last_line+_}" ]; then
die "No lines were read for the description of PR $companion_pr_number (some error probably occurred)"
fi
fi
# process_pr_description calls itself for each companion in the description on
# each detected companion PR, effectively considering all companion references
# on all PRs
process_pr_description "$this_repo" "$CI_COMMIT_REF_NAME"

if [ "${was_companion_found:-}" ]; then
exit
local dependent_repo_dir="$companions_dir/$dependent_repo"
if ! [ -e "$dependent_repo_dir" ]; then
echo "Cloning $dependent_repo directly as it was not detected as a companion"
dependent_repo_dir="$this_repo_dir/$dependent_repo"
git clone --depth=1 "https://github.com/$org/$dependent_repo.git" "$dependent_repo_dir"
fi

echo "running checks for the default branch of $dependent_repo"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just duplicated logic which was moved into patch_and_check_dependent


git clone --depth 1 "https://github.com/$org/$dependent_repo.git"
pushd "$dependent_repo" >/dev/null

# Update the crates to the latest version.
#
# This is for example needed if there was a pr to Substrate that only required a Polkadot companion
# and Cumulus wasn't yet updated to use the latest commit of Polkadot.
for update in $update_crates_on_default_branch; do
cargo update -p $update
done

patch_and_check_dependent

popd >/dev/null
patch_and_check_dependent "$dependent_repo" "$dependent_repo_dir"
}
main