Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

[CI] Add cumulus + beefy companions #9010

Closed
s3krit wants to merge 25 commits into from
Closed

[CI] Add cumulus + beefy companions #9010

Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
improve comments, test something neat
  • Loading branch information
@s3krit
s3krit committed Jun 28, 2021
commit c476a9d78703f5937fd3162e604575dfee35f304
6 changes: 3 additions & 3 deletions .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ check-polkadot-companion-status:
<<: *docker-env
<<: *test-refs-no-trigger
script:
- ./.maintain/gitlab/check_companion_build.sh "$COMPANION_ORG" "$COMPANION_REPO" "$COMPANION_BUILD"
- ./.maintain/gitlab/check_companion_build.sh "$COMPANION_ORG" "$COMPANION_REPO" "$COMPANION_BUILD" "$COMPANION_DEPENDENCY"
after_script:
- cd "$COMPANION_REPO" && git rev-parse --abbrev-ref HEAD
allow_failure: true
Expand All @@ -443,6 +443,7 @@ check-polkadot-companion-build:
variables:
COMPANION_ORG: paritytech
COMPANION_REPO: polkadot
COMPANION_DEPENDENCY: paritytech/grandpa-bridge-gadget
allow_failure: true

check-cumulus-companion-build:
Expand All @@ -451,8 +452,7 @@ check-cumulus-companion-build:
COMPANION_ORG: paritytech
COMPANION_REPO: cumulus
COMPANION_BUILD: "cargo check"
Copy link
Member

@bkchr bkchr Jun 29, 2021

Choose a reason for hiding this comment

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

Hmm this isn't really great :P I can understand why you are doing this, but honestly I don't know...

Overall it would be nice to have pre merge checks that runs the entire test suite of cumulus to make sure we don't break anything..

Copy link
Contributor

@TriplEight TriplEight Jun 29, 2021

Choose a reason for hiding this comment

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

You mean it's better to trigger the entire cumulus CI here? Is it worth doing on every commit? Maybe on some certain files/dirs changes at least?

Copy link
Member

@bkchr bkchr Jul 1, 2021

Choose a reason for hiding this comment

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

I mean for cumulus it is not that important at the moment. A cargo test --all --release would do it as well. However, even that takes some time. I would actually like to have this as a precommit check, but we don't have support for this :(

Copy link
Contributor

@TriplEight TriplEight Jul 1, 2021

Choose a reason for hiding this comment

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

yeah, we're kinda stuck between not being able to use the existing tools for pre-merge pipelines and the inability to use some kind of a pre-commit tool with github.com. This means we'll be looking into how to deal with it nicely, but currently, we're scaling vertically.

Copy link
Contributor

@TriplEight TriplEight Aug 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I suggest we could generalize these checks by actually triggering the CIs of the relevant repository. As we've been doing it with Simnet recently. This script passes overriding variables to the downstream CI and returns the status of all checks.
It will require some changes in the relevant repos, (with which @paritytech/ci will certainly help), but will be more future-proof.

Copy link
Contributor

@TriplEight TriplEight Aug 2, 2021

Choose a reason for hiding this comment

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

Now, this triggering script looks like this, it's a bit more specialized for Simnet.

Copy link
Member

@bkchr bkchr Jun 29, 2021

Choose a reason for hiding this comment

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

Suggested change
COMPANION_BUILD: "cargo check"
COMPANION_BUILD: "cargo check --benches --all --tests"

Copy link
Contributor

@gilescope gilescope Aug 6, 2021

Choose a reason for hiding this comment

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

How about cargo check --all-targets --workspace?

--all is deprecated in favour of --workspace https://doc.rust-lang.org/cargo/commands/cargo-check.html
--all-targets = --lib --bins --tests --benches --examples

script:
- ./.maintain/gitlab/check_companion_build.sh "$COMPANION_ORG" "$COMPANION_REPO" "$COMPANION_BUILD" paritytech/polkadot
COMPANION_DEPENDENCY: paritytech/polkadot
allow_failure: true

check-beefy-companion-build:
Expand Down
14 changes: 12 additions & 2 deletions .maintain/gitlab/check_companion_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,18 @@
#
# $ORGANISATION and $REPO are set using $1 and $2. You can also specify a custom
# build command with $3
# So invoke this script like:
# ./check_companion_build.sh paritytech polkadot 'cargo test --release'
# The 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

Expand Down