Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bff60a7
add cumulus companions
s3krit Jun 3, 2021
deaeab7
remove references to any specific repo in check_companion_build.sh
s3krit Jun 3, 2021
21c3d80
naughty naughty very naughty (revert me)
s3krit Jun 3, 2021
98030be
add custom build string option
s3krit Jun 3, 2021
4b68a08
add beefy
s3krit Jun 3, 2021
5f05ce8
Revert "naughty naughty very naughty (revert me)"
s3krit Jun 3, 2021
07747bc
out with the old
s3krit Jun 3, 2021
00a925b
add anchors
s3krit Jun 7, 2021
3b90256
move check-companion-build to test stage
s3krit Jun 7, 2021
cd97118
ugh...
s3krit Jun 7, 2021
98f83ce
fix
s3krit Jun 7, 2021
a9427dd
Merge branch 'master' of https://github.com/paritytech/substrate into…
s3krit Jun 10, 2021
359820c
dynamically patch crates as needed
s3krit Jun 10, 2021
3002789
Merge branch 'master' into mp-cumulus-companions
s3krit Jun 28, 2021
8c5363d
Merge remote-tracking branch 'origin/master' into mp-cumulus-companions
s3krit Jun 28, 2021
dc8d301
fix reviews
s3krit Jun 28, 2021
c90db6c
first attempt at deeper dependencies
s3krit Jun 28, 2021
7df4a60
include lib.sh
s3krit Jun 28, 2021
1b8c9d8
Merge remote-tracking branch 'origin/master' into mp-cumulus-companions
s3krit Jun 28, 2021
8181635
patch things correctly
s3krit Jun 28, 2021
c476a9d
improve comments, test something neat
s3krit Jun 28, 2021
f5f920d
Apply suggestions from code review
s3krit Jun 29, 2021
46988ba
update comments
s3krit Jun 29, 2021
c96f997
Merge branch 'master' into mp-cumulus-companions
s3krit Aug 6, 2021
a7ee4a3
use jq for parsing PRs
s3krit Aug 6, 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
36 changes: 29 additions & 7 deletions .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -446,19 +446,41 @@ check-polkadot-companion-status:
script:
- ./.maintain/gitlab/check_polkadot_companion_status.sh

check-polkadot-companion-build:
stage: build
.check-companion-build: &check-companion-build
stage: test
<<: *docker-env
<<: *test-refs-no-trigger
needs:
- job: test-linux-stable-int
artifacts: false
script:
- ./.maintain/gitlab/check_polkadot_companion_build.sh
- ./.maintain/gitlab/check_companion_build.sh "$COMPANION_ORG" "$COMPANION_REPO" "$COMPANION_BUILD" "$COMPANION_DEPENDENCY"
after_script:
- cd polkadot && git rev-parse --abbrev-ref HEAD
- cd "$COMPANION_REPO" && git rev-parse --abbrev-ref HEAD
allow_failure: true

check-polkadot-companion-build:
<<: *check-companion-build
variables:
COMPANION_ORG: paritytech
COMPANION_REPO: polkadot
COMPANION_DEPENDENCY: paritytech/grandpa-bridge-gadget
allow_failure: true

check-cumulus-companion-build:
<<: *check-companion-build
variables:
COMPANION_ORG: paritytech
COMPANION_REPO: cumulus
COMPANION_BUILD: "cargo check --all-targets --workspace"
COMPANION_DEPENDENCY: paritytech/polkadot
allow_failure: true

check-beefy-companion-build:
<<: *check-companion-build
variables:
COMPANION_ORG: paritytech
COMPANION_REPO: grandpa-bridge-gadget
COMPANION_BUILD: "cargo check --all-targets --workspace"
allow_failure: true

test-browser-node:
stage: build
<<: *docker-env
Expand Down
42 changes: 33 additions & 9 deletions .maintain/common/lib.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@

api_base="https://api.github.com/repos"


set_github_token(){
# These will exist if the function is called in Gitlab.
# If the function's called in Github, we should have GITHUB_ACCESS_TOKEN set
# already.
if [ -n "$GITHUB_RELEASE_TOKEN" ]; then
GITHUB_TOKEN="$GITHUB_RELEASE_TOKEN"
elif [ -n "$GITHUB_PR_TOKEN" ]; then
GITHUB_TOKEN="$GITHUB_PR_TOKEN"
fi
}

# Function to take 2 git tags/commits and get any lines from commit messages
# that contain something that looks like a PR reference: e.g., (#1234)
sanitised_git_logs(){
Expand Down Expand Up @@ -66,15 +78,7 @@ has_label(){
repo="$1"
pr_id="$2"
label="$3"

# These will exist if the function is called in Gitlab.
# If the function's called in Github, we should have GITHUB_ACCESS_TOKEN set
# already.
if [ -n "$GITHUB_RELEASE_TOKEN" ]; then
GITHUB_TOKEN="$GITHUB_RELEASE_TOKEN"
elif [ -n "$GITHUB_PR_TOKEN" ]; then
GITHUB_TOKEN="$GITHUB_PR_TOKEN"
fi
set_github_token

out=$(curl -H "Authorization: token $GITHUB_TOKEN" -s "$api_base/$repo/pulls/$pr_id")
[ -n "$(echo "$out" | tr -d '\r\n' | jq ".labels | .[] | select(.name==\"$label\")")" ]
Expand Down Expand Up @@ -115,3 +119,23 @@ has_runtime_changes() {
return 1
fi
}

# Given an origin repo & PR,and a repository, will check if the given PR has a
# companion PR in the target repo
get_companion() {
origin_repo="$1"
pr_num="$2"
companion_repo="$3"
set_github_token
github_header="Authorization: token ${GITHUB_TOKEN}"
# get the last reference to a pr in the target repo
pr_body=$(
curl -sSL -H "${github_header}" "${api_base}/$origin_repo/pulls/$pr_num" | \
jq -r '.body'
)

echo "${pr_body}" | sed -n -r \
Copy link
Contributor

Choose a reason for hiding this comment

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

Does sed account for newlines in the JSON string?

This probably never happens but you might have a comment like this:

Something something companion

Bla bla another pull request: https://github.com/foo/pull/1

As I understand it, since you're not matching [Cc]ompanion: [regex] all in the same line, that comment would be detected although it should not be.

Using echo -e will output the JSON newlines as actual newlines so you'll be able to match it line-by-line.

-e "s;^.*[Cc]ompanion.*$companion_repo#([0-9]+).*$;\1;p" \
-e "s;^.*[Cc]ompanion.*https://github.com/$companion_repo/pull/([0-9]+).*$;\1;p" \
| tail -n 1
}
123 changes: 123 additions & 0 deletions .maintain/gitlab/check_companion_build.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
#!/usr/bin/env bash
#
# check if a pr is compatible with companion pr or master if not available
#
# to override one that was just mentioned mark companion pr in the body of the
# pr like
#
# $REPO companion: $ORGANISATION/$REPO#567
#
# $ORGANISATION and $REPO are set using $1 and $2. You can also specify a custom
# build command with $3
# Every argument after $3 is for specifying *additional* dependencies this
# project has that depend on substrate, which might also have companion PRs.

# Example: Cumulus relies on both substrate and polkadot. If this substrate PR
# requires a companion build on polkadot, when we are testing that cumulus builds
# with this commit of substrate, we *also* need to clone & patch polkadot, and tell
# cumulus to use this cloned+patched version, else the build is guaranteed to fail
# (since it doesn't have the changes to polkadot that were required in the polkadot
# companion PR)

# So invoke this script like (arguments in [] indicate optional arguments)
# ./check_companion_build.sh paritytech cumulus ['cargo test --release'] [paritytech/polkadot]

set -e

# Include the common functions library
#shellcheck source=../common/lib.sh
. "$(dirname "${0}")/../common/lib.sh"

ORGANISATION=$1
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Apparently "Organisation" is valid spelling in British English but IMO it'd look better ORGANIZATION

REPO=$2
BUILDSTRING=${3:-cargo test --workspace --release}
DEPS=("${@:4}")
SUBSTRATE_DIR="$(pwd)"

boldprint () { printf "|\n| \033[1m%s\033[0m\n|\n" "${@}"; }
boldcat () { printf "|\n"; while read -r l; do printf "| \033[1m%s\033[0m\n" "${l}"; done; printf "|\n" ; }

boldcat <<-EOT
check_${REPO}_companion_build
==============================
this job checks if there is a string in the description of the pr like
$REPO companion: $ORGANISATION/$REPO#567
it will then run cargo check from this ${REPO}'s branch with substrate code
from this pull request. otherwise, it will uses master instead
EOT

# Set the user name and email to make merging work
git config --global user.name 'CI system'
git config --global user.email '<>'

# Merge master into our branch before building to make sure we don't miss
# any commits that are required.
git fetch --depth 100 origin
git merge origin/master
Comment on lines +63 to +64
Copy link
Contributor

Choose a reason for hiding this comment

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

can it just be git pull origin master? I don't quite get the depth numbers but fetching a single branch is better than doing fetch origin (which fetches everything)


# Clone the current master branch into a local directory
# NOTE: we need to pull enough commits to be able to find a common
# ancestor for successfully performing merges below.
git clone --depth 20 "https://github.com/${ORGANISATION}/${REPO}.git"

cd "$REPO"

# either it's a pull request then check for a companion otherwise use
# master
# shellcheck disable=SC2003
if expr match "${CI_COMMIT_REF_NAME}" '^[0-9]\+$' >/dev/null
then
boldprint "this is pull request no ${CI_COMMIT_REF_NAME}"
pr_companion="$(get_companion "paritytech/substrate" "$CI_COMMIT_REF_NAME" "$ORGANISATION/$REPO")"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I get why "paritytech/substrate" is hardcoded for now (because this script is only used in this repository at the moment?) but it'd be good to have this as another variable for clarity's sake

if [ "$pr_companion" ]
then
boldprint "companion pr specified/detected: #${pr_companion}"
git fetch origin "refs/pull/${pr_companion}/head:pr/${pr_companion}"
git checkout "pr/${pr_companion}"
git merge origin/master
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be merging whatever branch the PR is currently targetting instead of master (although PRs usually target master). Perhaps it doesn't need to be handled at the moment, but a FIXME note about this detail would be welcome.

else
boldprint "no companion branch found - building ${REPO}:master"
fi

# If this repo has any additional dependencies, we should check whether they
# are mentioned as companions as well. If they are, we patch this repo to
# use that companion build as well. See example at top of this script
# Note: Will only work with repos supported by diener
declare -A diener_commands
diener_commands=()
diener_commands["paritytech/polkadot"]='--polkadot'
diener_commands["paritytech/substrate"]='--substrate'
diener_commands["paritytech/grandpa-bridge-gadget"]='--beefy'

for dep in "${DEPS[@]}"; do
Copy link
Contributor

Choose a reason for hiding this comment

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

If https://github.com/paritytech/substrate/pull/9010/files#r660727553 is taken care of, you could try to get rid of this $DEPS and just loop through all companion lines in the description; and if some repository is not supported (might be typo) then just fail the script.

dep_companion="$(get_companion "paritytech/substrate" "$CI_COMMIT_REF_NAME" "$dep")"
Copy link
Contributor

Choose a reason for hiding this comment

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

since you're already fetching the description once in the lines above, it would be good to reuse it instead of making more API requests

if [ "$dep_companion" ]; then
echo "Companion PR found for $dep, need to patch $REPO to use that"
git clone --depth 20 "https://github.com/$dep.git" "$dep"
git -C "$dep" fetch origin "refs/pull/${dep_companion}/head:pr/${dep_companion}"
git -C "$dep" checkout "pr/${dep_companion}"
git -C "$dep" merge origin/master
Copy link
Contributor

Choose a reason for hiding this comment

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

# Tell this dependency to use the version of substrate in this PR
diener patch --crates-to-patch "$SUBSTRATE_DIR" --substrate --path "$dep/Cargo.toml"
Copy link
Contributor

@joao-paulo-parity joao-paulo-parity Jun 29, 2021

Choose a reason for hiding this comment

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

SUBSTRATE_DIR and --substrate will have a rework to be dynamic once this script is made more general, right?

# then we tell this repository to point at our locally cloned dependency
diener patch --crates-to-patch "$dep" "${diener_commands[$dep]}" --path "Cargo.toml"
fi

done

else
boldprint "this is not a pull request - building ${REPO}:master"
fi

# Test pr or master branch with this Substrate commit.
diener patch --crates-to-patch ".." --substrate --path "Cargo.toml"
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing this ".." to "$SUBSTRATE_DIR" makes the script less confusing to follow. It makes sense because you're doing cd at the start but it's not immediately obvious which is the current directory when this line is reached.


eval "$BUILDSTRING"
99 changes: 0 additions & 99 deletions .maintain/gitlab/check_polkadot_companion_build.sh

This file was deleted.