Add build options to the srtool build step #4956
Conversation
EgorPopelyaev
added
the
R0-silent
.github/workflows/release-srtool.yml
Outdated
| run: | | ||
| BUILD_OPTIONS="" | ||
| if [[ ${{ matrix.chain }} == 'asset-hub-rococo' || ${{ matrix.chain }} == 'asset-hub-westend' || ${{ matrix.chain }} == 'rococo' || ${{ matrix.chain }} == 'westend' ]]; then | ||
| BUILD_OPTIONS="${{ inputs.build_opts }}" |
There was a problem hiding this comment.
Conceptually it seems a bit odd that it ignores the build opts for some runtimes. Maybe you can add a METADATA_HASH_ENABLED flag or something?
There was a problem hiding this comment.
We should add on-chain-release-build to every runtime and use this one here.
There was a problem hiding this comment.
This was the initial plan, the reason why I went this way was, that it includes this feature sp-api/disable-logging that turns off the logging, that is on the other hand needed for devops guys cc: @PierreBesson. After discussion with Pierre, that was kinda compromised solution🙈 to not build two types of runtimes (with on chain feature and with logging). If there is another to do it, I'll be happy to use it:). Otherwise, I can also add additional step to build those runtimes with logs for devops.
There was a problem hiding this comment.
We are also running logging nodes for main net, because we have logging disabled there. However, we can also just remove the "disable-logging" feature there.
There was a problem hiding this comment.
I've tested the solution with two separate jobs, building runtimes with and without logging (having on-chain-release-build option). It looks fine. The only thing now is that people-rococo and poepele-westend seems not have this feature. Whom could I contact to ask to add this option? Is it Joe?
There was a problem hiding this comment.
You can just copy the feature declaration from the other runtimes.
There was a problem hiding this comment.
And then we should remove the disable-logging everywhere.
There was a problem hiding this comment.
Basti, I've added the on-chain-release-build where it was missing, should the disable-logging deletion be done here as well or as a separate task? I'm not sure if it is worth to mention it in the rel notes or not.
|
I think we should generally re-evaluate if we still need srtool at all, since we should not in theory. |
Yep, I agree with that, we can do it step by step. After the switch to the new process, we can start to re-think what is needed for the current setup and what can be replaced |
|
| uses: "./.github/workflows/release-srtool.yml" | ||
| with: | ||
| excluded_runtimes: "substrate-test bp cumulus-test kitchensink minimal-template parachain-template penpal polkadot-test seedling shell frame-try sp solochain-template" | ||
| build_opts: "" |
There was a problem hiding this comment.
This doesn't enable the on-chain-release-build feature. This means that metadata hash will not be generated at compile time. This is bad 🙈
There was a problem hiding this comment.
Yes, this one was done only for the case to have the logging enabled. In theory, this step should go away if the disable-logging feature will be deleted from the on-chain-release-build as we discussed above. If it is ok, I can delete it as part of this PR, and then we will have only the one build with the on-chain-release-build feature activated.
…nto ep-add-build-opts-to-release
…rPopelyaev/polkadot-sdk into ep-add-build-opts-to-release
…nto ep-add-build-opts-to-release
…rPopelyaev/polkadot-sdk into ep-add-build-opts-to-release
Command failed ❌Run by @EgorPopelyaev forCommand PrDoc failed. See logs here.
|
…nto ep-add-build-opts-to-release
|
@bkchr Basti, could you please have a look at the deletion of the |
…nto ep-add-build-opts-to-release
3cbefaf
* master: (36 commits) Bump the ci_dependencies group across 1 directory with 2 updates (#5401) Remove deprecated calls in cumulus-parachain-system (#5439) Make the PR template a default for new PRs (#5462) Only log the propagating transactions when they are not empty (#5424) [CI] Fix SemVer check base commit (#5361) Sync status refactoring (#5450) Add build options to the srtool build step (#4956) `MaybeConsideration` extension trait for `Consideration` (#5384) Skip slot before creating inherent data providers during major sync (#5344) Add symlinks for code of conduct and contribution guidelines (#5447) pallet-collator-selection: correctly register weight in `new_session` (#5430) Derive `Clone` on `EncodableOpaqueLeaf` (#5442) Moving `Find FAIL-CI` check to GHA (#5377) Remove panic, as proof is invalid. (#5427) Reactive syncing metrics (#5410) [bridges] Prune messages from confirmation tx body, not from the on_idle (#5006) Change the chain to Rococo in the parachain template Zombienet config (#5279) Improve the appearance of crates on `crates.io` (#5243) Add initial version of `pallet_revive` (#5293) Update OpenZeppelin template documentation (#5398) ...
This PR adds possibility to set BUILD_OPTIONS to the "Srtool Build" step in the release pipeline while building runtimes.
Colses: https://github.com/paritytech/release-engineering/issues/213