Check umbrella version

[Wolfenheimm]

Description

This PR fixes the issue where check-umbrella fails unless the proper umbrella crate version is hardcoded into the workflow. The solution adds the version detection logic to properly determine the umbrella crate version by:

1. Using cargo metadata & jq to extract the version, specifying umbrella/cargo.toml
2. Falling back to using version 0.1.0 if metadata fails

Fixes #8029

Integration

No integration changes are required for downstream projects.
The workflow will now always use the version from cargo metadata if available, or fallback to 0.1.0 if not.
No environment variables or external configuration changes are needed.

Review Notes

• Modified version extraction logic in check-umbrella job to dynamically determine the correct version

Checklist

• My PR includes a detailed description as outlined in the "Description" and its two subsections above.
• My PR follows the labeling requirements of this project (at minimum one label for T required)
• I have made corresponding changes to the documentation (if applicable)
• I have added tests that prove my fix is effective or that my feature works (if applicable)

[iulianbarbu]

Thanks for taking a crack at this! I like the cargo metadata based approach, and IMO we can simplify the changes. LMK what you think. For testing these changes against a stable branch, you can open another draft PR against stable2503/stable2412, and ping me to allow the CI to run. :)

[Wolfenheimm]

Thanks for taking a crack at this! I like the cargo metadata based approach, and IMO we can simplify the changes. LMK what you think. For testing these changes against a stable branch, you can open another draft PR against stable2503/stable2412, and ping me to allow the CI to run. :)

LGTM, let me whip up those fixes as soon as I can, should be ready in a few hours.

[iulianbarbu]

Hey @Wolfenheimm ! Sorry for my lack of clarity. The PR as opened here is what we want. The suggestion to open another draft against stable2503 was just related to testing that the workflow works there as well. It is good that you opened one more draft there, but once we're clear that the job works as expected, we can close that one (I'll do it myself).

The process we currently use is that we do PRs against master first and once merged we backport to the relevant stable releases branches. We have an automation where we can label the merged PRs and they'll be automatically picked by a bot and a new PR opened against the relevant stable release branch. You don't have to worry about merging these changes into the stable release branches for now (it will be my job after we merge this PR), but we can still check if the job works correctly there in a dedicated PR in parallel.

[Wolfenheimm]

Hey @Wolfenheimm ! Sorry for my lack of clarity. The PR as opened here is what we want. The suggestion to open another draft against stable2503 was just related to testing that the workflow works there as well. It is good that you opened one more draft there, but once we're clear that the job works as expected, we can close that one (I'll do it myself).

The process we currently use is that we do PRs against master first and once merged we backport to the relevant stable releases branches. We have an automation where we can label the merged PRs and they'll be automatically picked by a bot and a new PR opened against the relevant stable release branch. You don't have to worry about merging these changes into the stable release branches for now (it will be my job after we merge this PR), but we can still check if the job works correctly there in a dedicated PR in parallel.

Haha whoops on my part as well. Here to help, no worries!

[Wolfenheimm]

@iulianbarbu - New PR on #8268 - I thought Master -> stable would play nice but I had to make a new branch based on stable2503 and cherry pick my commits to it 😢

[iulianbarbu]

Looks great! Will be good to update the PR description as well before merging.

[github-actions]

Review required! Latest push from author must always be reviewed

[ggwpez]

It would also be possibly to modify the generate-umbrella.py script and read the version from the Cargo.toml?!

[iulianbarbu]

It would also be possibly to modify the generate-umbrella.py script and read the version from the Cargo.toml?!

Actually yes! Thinking outside of the box 👀 . It should be less changes. I would support changing to that if @Wolfenheimm is up for it, but either way, I would be happy to merge this as is and maybe do the python equivalent whenever we feel like. TBH it is something I would rather throw in there in any reasonable form and just forget about, but I agree it is not the best solution.

[Wolfenheimm]

It would also be possibly to modify the generate-umbrella.py script and read the version from the Cargo.toml?!

Actually yes! Thinking outside of the box 👀 . It should be less changes. I would support changing to that if @Wolfenheimm is up for it, but either way, I would be happy to merge this as is and maybe do the python equivalent whenever we feel like. TBH it is something I would rather throw in there in any reasonable form and just forget about, but I agree it is not the best solution.

The umbrella script is originally meant to take in a version flag, right now we hard-code in the version for every version we create. By that same logic, why weren't we hard coding the version inside each script?

Also, fetching jq does not even need to be done, the mechanism is there in-case it isn't (in-case we are scared of bloat):

I'm willing to do the change inside the python file, let me know 👍

[iulianbarbu]

By that same logic, why weren't we hard coding the version inside each script?

Yeh, I think @ggwpez suggests a different code path where instead of generating the umbrella crate's Cargo.toml by giving it a version, we rely on the fact the crate already exists and pick its version from the Cargo.toml. Same same but different 😅 . You can argue it helps whenever we want to run the script locally while not being concerned with the version (since otherwise we'd either need to run the bash commands you do, or look into the umbrella/Cargo.toml to check the version, and use it). We might run this locally whenever we do changes which should be propagated to polkadot-sdk, and need to commit them.

The new aspect would be a new requirement, but I don't find it necessary. I think we would solve a non-problem though, since for most of times we run the script on master where polkadot-sdk version is set to 0.1.0. Another argument is that it might be a better practice to keep the scripting to a single language and not mix them, but I am fine keeping a certain API for the python script, and feed into it anything is needed through bash.

@ggwpez , do you see other benefits of implementing this in the python script? IMO it doesn't matter that much, and we should be good to merge - unless you request changes for some reason.

Otherwise, sorry for my overindexing in advance. Just trying to make sure we don't miss anything.

[Wolfenheimm]

By that same logic, why weren't we hard coding the version inside each script?

Yeh, I think @ggwpez suggests a different code path where instead of generating the umbrella crate's Cargo.toml by giving it a version, we rely on the fact the crate already exists and pick its version from the Cargo.toml. Same same but different 😅 . You can argue it helps whenever we want to run the script locally while not being concerned with the version (since otherwise we'd either need to run the bash commands you do, or look into the umbrella/Cargo.toml to check the version, and use it). We might run this locally whenever we do changes which should be propagated to polkadot-sdk, and need to commit them.

The new aspect would be a new requirement, but I don't find it necessary. I think we would solve a non-problem though, since for most of times we run the script on master where polkadot-sdk version is set to 0.1.0. Another argument is that it might be a better practice to keep the scripting to a single language and not mix them, but I am fine keeping a certain API for the python script, and feed into it anything is needed through bash.

@ggwpez , do you see other benefits of implementing this in the python script? IMO it doesn't matter that much, and we should be good to merge - unless you request changes for some reason.

Otherwise, sorry for my overindexing in advance. Just trying to make sure we don't miss anything.

[Wolfenheimm]

Sorry for closing this, hard accident, dropped my phone and button mashed 😓

[ggwpez]

do you see other benefits of implementing this in the python script? IMO it doesn't matter that much, and we should be good to merge - unless you request changes for some reason.

Yea I dont really care if it works, just wanted to point it out.

[paritytech-release-backport-bot]

Created backport PR for stable2412:

• [stable2412] Backport #8250 #8342 with remaining conflicts!

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

git fetch origin backport-8250-to-stable2412
git worktree add --checkout .worktree/backport-8250-to-stable2412 backport-8250-to-stable2412
cd .worktree/backport-8250-to-stable2412
git reset --hard HEAD^
git cherry-pick -x c854f7c6b7291154072d8e931eddb426f98217fc
git push --force-with-lease

[paritytech-release-backport-bot]

Git push to origin failed for stable2412 with exitcode 1

[paritytech-release-backport-bot]

Created backport PR for stable2503:

• [stable2503] Backport #8250 #8343 with remaining conflicts!

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

git fetch origin backport-8250-to-stable2503
git worktree add --checkout .worktree/backport-8250-to-stable2503 backport-8250-to-stable2503
cd .worktree/backport-8250-to-stable2503
git reset --hard HEAD^
git cherry-pick -x c854f7c6b7291154072d8e931eddb426f98217fc
git push --force-with-lease

[paritytech-release-backport-bot]

Git push to origin failed for stable2503 with exitcode 1