Skip to content
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.

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

Align omni-node and polkadot-parachain versions #7367

Merged

Conversation

iulianbarbu
Copy link
Contributor

@iulianbarbu iulianbarbu commented Jan 28, 2025

Description

Aligned polkadot-omni-node & polkadot-parachain versions. There is one NODE_VERSION constant, in polkadot-omni-node-lib, used by both binaries.

Closes #7276 .

Integration

Node operators will know what versions of polkadot-omni-node & polkadot-parachain they use since their versions will be kept in sync with the stable release polkadot SemVer version.

Review Notes

TODO:

  • update NODE_VERSION of polkadot-omni-node-lib when running branch off workflow

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu added the R0-silent Changes should not be mentioned in any release notes label Jan 28, 2025
@iulianbarbu iulianbarbu self-assigned this Jan 28, 2025
@skunert
Copy link
Contributor

skunert commented Jan 28, 2025

Not super sure how the NODE_VERSION is updated for polkadot binaries, but I think same should happen after we merge this, for each stable release. cc @EgorPopelyaev

Also not sure how this works, when I build polkadot-parachain locally it outputs polkadot-parachain 4.0.0-3e861941f2f as its version. If I download it from the releases page, it outputs polkadot-parachain 1.17.0-967989c5d94, so the correct version. Some adjustment seems to be in place here.

To me it makes sense to move the constant to the lib and call it a day. For sure polkadot-parachain and polkadot-omni-node should have the same version.

Copy link
Member

@ggwpez ggwpez left a comment

Choose a reason for hiding this comment

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

Something we recently talked about is that we need to include the stable release as part of the version number. So something like: 1.17-hash-stable2414 since otherwise node operators dont know to what release it aligns.

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu
Copy link
Contributor Author

iulianbarbu commented Jan 29, 2025

Something we recently talked about is that we need to include the stable release as part of the version number. So something like: 1.17-hash-stable2414 since otherwise node operators dont know to what release it aligns.

This would be useful, but I would do it in a follow up, where we can rethink how the baking of stableXXYY version gets into the building stage, alongside the polkadot SemVer version, automatically, and concatenates them. I think this is possible by setting env variables in the release workflows accordingly (considering we have some git tags representing the versions), and expand the env variables at compile time - baking them concatenated into the binary for usage with --version. I would also add the repository metadata to each crate Cargo.toml so that people can reference the repository & branch of a create easily when looking them up on crates.io.

Today we update the NODE_VERSION constant manually and commit it for polkadot binaries before building them. My suggestion with this PR is to do the same for polkadot-omni-node and polkadot-parachain as a short term solution. I would personally go after what I suggested earlier when I'll have more bandwidth (but might be picked earlier by someone with bandwidth that agrees with the suggestion and wants to do it). I think it is not that hard but still takes some time - we must search where these env variables make sense in the release pipeline, which binaries should use them and the testing of the whole release workflow and binaries. I can open an issue to track this. @ggwpez @EgorPopelyaev what do you think?

Also not sure how this works, when I build polkadot-parachain locally it outputs polkadot-parachain 4.0.0-3e861941f2f as its version. If I download it from the releases page, it outputs polkadot-parachain 1.17.0-967989c5d94, so the correct version. Some adjustment seems to be in place here.

@skunert , after an offline chat with Egor my understanding is that polkadot-parachain version is updated manually right before building the binary, in Cargo.toml, set to the stable release corresponding polkadot SemVer version (e.g. polkadot-v1.17.0 for stable2412). During build time (in build.rs), we set an env variable to the cargo package version + commit hash, and that gets baked into the binary, and used when calling --version. This wasn't the case for polkadot-omni-node (the part with the Cargo.toml update), for previous and first release, but @EgorPopelyaev mentioned that he can apply the same steps for polkadot-omni-node for next releases, and if we don't propose other ways of achieving this. The version set in Cargo.toml before building is also changed when the Plan.toml is generated, so the published crate for the binary has a different version than what's returned by --version. This can be very confusing, so I think using NODE_VERSION for both polkadot-parachain & polkadot-omni-node is a small improvement over the current way - we can see in code which is the "stable release" version of the crate, by going to the crate stable release branch, its tag, and checking the code. It is obviously terrible from a DX standpoint, but please see my suggestion above for how to make this better & automate it. In the future we could also append the CARGO_PKG_VERSION variable to the whole thing and make a full mapping that will uniquely identify binaries when calling --version (we already mention the stableXXYY version in the crates README, which appears on crates.io, but there is not a way to know that upfront, without some searching through the various versions of the binaries, on crates.io, and scanning through the READMEs).

@iulianbarbu iulianbarbu requested a review from ggwpez January 30, 2025 10:35
@iulianbarbu iulianbarbu changed the title aligned omni-node and polkadot-parachains version Align omni-node and polkadot-parachains version Jan 30, 2025
@iulianbarbu iulianbarbu changed the title Align omni-node and polkadot-parachains version Align omni-node and polkadot-parachain versions Jan 30, 2025
Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

I will leave the details to those who know better, but all in all sharing my appreciation of this work!

All in all, for the parachain product, having clear versioning is a great direction, and frankly a long hanging fruit in terms of DX.

The following should have proper versions, aligned with the main polkadot-sdk releases:

  • polkadot, polkadot-parachian, polkadot-omni-node, polkadot-omni-node-lib
  • polkadot-sdk, polkadot-sdk-frame umbrella crates
  • 3 main templates
  • (might be missing more items, in general all major crates that we foresee developers interact with while building a parachain should be part of the list)

cc @re-gius @seemantaggarwal

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
@iulianbarbu iulianbarbu requested review from a team as code owners February 3, 2025 09:14
@iulianbarbu
Copy link
Contributor Author

@ggwpez @kianenigma I opened #7431 to track the requests for adding stableXXYY-M to the version returned by node binaries (polkadot* & polkadot-omni-node/polkadot-parachain, LMK if we should do it in other places too), and also to double check/ensure the versions are the same for the components mentioned here: #7367 (review).

Copy link
Contributor

@skunert skunert left a comment

Choose a reason for hiding this comment

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

LGTM

@iulianbarbu iulianbarbu enabled auto-merge February 4, 2025 09:00
@iulianbarbu iulianbarbu added this pull request to the merge queue Feb 4, 2025
Merged via the queue into paritytech:master with commit 3fb7c8c Feb 4, 2025
204 of 207 checks passed
@iulianbarbu iulianbarbu deleted the ib-align-omni-node-version branch February 4, 2025 10:03
@iulianbarbu iulianbarbu added the A4-needs-backport Pull request must be backported to all maintained releases. label Feb 6, 2025
@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2407:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7367-to-stable2407
git worktree add --checkout .worktree/backport-7367-to-stable2407 backport-7367-to-stable2407
cd .worktree/backport-7367-to-stable2407
git reset --hard HEAD^
git cherry-pick -x 3fb7c8c6d68cbc40134a4fc8f0c8b4de38ec1388
git push --force-with-lease

@paritytech-cmd-bot-polkadot-sdk

Created backport PR for stable2409:

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-7367-to-stable2409
git worktree add --checkout .worktree/backport-7367-to-stable2409 backport-7367-to-stable2409
cd .worktree/backport-7367-to-stable2409
git reset --hard HEAD^
git cherry-pick -x 3fb7c8c6d68cbc40134a4fc8f0c8b4de38ec1388
git push --force-with-lease

github-actions bot pushed a commit that referenced this pull request Feb 6, 2025
# Description

Aligned `polkadot-omni-node` & `polkadot-parachain` versions. There is
one `NODE_VERSION` constant, in `polkadot-omni-node-lib`, used by both
binaries.

Closes #7276 .

## Integration

Node operators will know what versions of `polkadot-omni-node` &
`polkadot-parachain` they use since their versions will be kept in sync
with the stable release `polkadot` SemVer version.

## Review Notes

TODO:
- [x] update NODE_VERSION of `polkadot-omni-node-lib` when running
branch off workflow

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
(cherry picked from commit 3fb7c8c)
@paritytech-cmd-bot-polkadot-sdk

Successfully created backport PR for stable2412:

EgorPopelyaev pushed a commit that referenced this pull request Feb 7, 2025
Backport #7367 into `stable2412` from iulianbarbu.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

---------

Signed-off-by: Iulian Barbu <iulian.barbu@parity.io>
Co-authored-by: Iulian Barbu <14218860+iulianbarbu@users.noreply.github.com>
Co-authored-by: Iulian Barbu <iulian.barbu@parity.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A4-needs-backport Pull request must be backported to all maintained releases. R0-silent Changes should not be mentioned in any release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

align omni-node crate version with polkadot release
5 participants