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

ci: use srtool-actions to build runtimes #3423

Merged
merged 2 commits into from
Jul 15, 2021
Merged

Conversation

chevdor
Copy link
Contributor

@chevdor chevdor commented Jul 6, 2021

  • build westend as well
  • switch to using srtool-actions@v0.3.0 which defaults to the paritytech/srtool docker image
  • add compressed runtimes to the artifacts

I have tested at https://github.com/chevdor/polkadot/actions/runs/1007250044

image

The compressed runtime is added to the -runtime artifacts.

image

@chevdor chevdor added the B0-silent Changes should not be mentioned in any release notes label Jul 6, 2021
@chevdor chevdor changed the title Use srtool-actions to build runtimes ci: use srtool-actions to build runtimes Jul 6, 2021
@chevdor chevdor added the C1-low PR touches the given topic and has a low impact on builders. label Jul 7, 2021
@chevdor chevdor requested review from s3krit and TriplEight July 7, 2021 08:08
@chevdor chevdor marked this pull request as ready for review July 7, 2021 08:08
@chevdor chevdor added the A0-please_review Pull request needs code review. label Jul 7, 2021
Copy link
Contributor

@s3krit s3krit left a comment

Choose a reason for hiding this comment

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

Lgtm. So long as the runtime srtool output json file hasn't changed format and is still reachable by publish_draft_release.rb, everything should stll Just Work™ (the job failed on your test run due to you currently having no releases). I'll verify that in my polkadot fork I use for testing release stuff then give this an approval. Cheers!

Comment on lines +57 to +59
path: |
${{ steps.srtool_build.outputs.wasm }}
${{ steps.srtool_build.outputs.wasm_compressed }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, what happens when you use two lines for the path here?

Copy link
Contributor

Choose a reason for hiding this comment

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

that are the names of two artifacts from id: srtool_build job

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The artifact ${{ matrix.runtime }}-runtime is an id. Listing several files or a pattern means you end up with several files in this artifact.

Copy link
Contributor

@TriplEight TriplEight left a comment

Choose a reason for hiding this comment

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

every time I see CI installs something I'm tempted to write an image for this.

Comment on lines +57 to +59
path: |
${{ steps.srtool_build.outputs.wasm }}
${{ steps.srtool_build.outputs.wasm_compressed }}
Copy link
Contributor

Choose a reason for hiding this comment

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

that are the names of two artifacts from id: srtool_build job

Comment on lines +72 to +85
- name: Set up Ruby 2.7
uses: actions/setup-ruby@v1
with:
ruby-version: 2.7
- name: Download srtool json output
uses: actions/download-artifact@v2
- name: Generate release text
env:
RUSTC_STABLE: ${{ needs.get-rust-versions.outputs.rustc-stable }}
RUSTC_NIGHTLY: ${{ needs.get-rust-versions.outputs.rustc-nightly }}
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
gem install changelogerator git toml
ruby $GITHUB_WORKSPACE/polkadot/scripts/github/generate_release_text.rb | tee release_text.md
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks huge

Copy link
Contributor Author

@chevdor chevdor Jul 13, 2021

Choose a reason for hiding this comment

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

The PR did not change that content (despite the diff). It looks like the identation changed though.
I did run some formatting.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then PR is good, but in general, it's something that would need some love.

@chevdor
Copy link
Contributor Author

chevdor commented Jul 13, 2021

the job failed on your test run due to you currently having no releases

I also don't have the Matrix keys/tokens so I did not expect that part to work.

@TriplEight TriplEight self-requested a review July 13, 2021 16:25
Copy link
Contributor

@s3krit s3krit left a comment

Choose a reason for hiding this comment

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

Cool, verified on my end with a fork that the release generation stuff works as expected

@chevdor
Copy link
Contributor Author

chevdor commented Jul 15, 2021

bot merge

@ghost
Copy link

ghost commented Jul 15, 2021

Error: When trying to meet the "Project Owners" approval requirements: this pull request does not belong to a project defined in Process.json.

Approval by "Project Owners" is only attempted if other means defined in the criteria for merge are not satisfied first.

The following errors might have affected the outcome of this attempt:

  • 'Batch: Codebase Restructure' does not match any projects in polkadot's Process.json

@s3krit s3krit merged commit 4c50905 into paritytech:master Jul 15, 2021
ordian added a commit that referenced this pull request Jul 20, 2021
* master:
  Update secp256k1 and remove unrequired usage (#3502)
  Bump libc from 0.2.91 to 0.2.98 (#3496)
  Bump slotmap from 1.0.2 to 1.0.5 (#3495)
  Gossip rebroadcast rate limiter (#3494)
  dependabot: ignore another git dep (#3493)
  add rustfmt toml (#3491)
  Disputes runtime (#2947)
  Bump async-process from 1.0.1 to 1.1.0 (#3122)
  remove the kubernetes helm chart (#3483)
  added pallet-proxy in rococo feature dependencies (#3486)
  Update BEEFY+MMR integration. (#3480)
  more verbose asserts (#3476)
  ci: use srtool-actions to build runtimes (#3423)
  overseer gen minor chore fixes (#3479)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants