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

feat: automate compatible CFE upgrades #4149

Merged
merged 10 commits into from
Oct 26, 2023
Merged

Conversation

kylezs
Copy link
Contributor

@kylezs kylezs commented Oct 20, 2023

Pull Request

Closes: PRO-899

Checklist

Please conduct a thorough self-review before opening the PR.

  • I am confident that the code works.
  • I have updated documentation where appropriate.

Summary

Automates the process of upgrading from one commit to another - as long as the versions are compatible with the old CFE. Incompatible coming next 🚀

Pre requisite:

cargo install cargo-workspaces

For example: If you're on tag 0.10.0, you can run:

./commands/upgrade_network.ts 0.10.1

It'll do everything from compile the new runtime at the commit specified, writing the version bumps in the Cargo.toml files so that the version is correctly updated in the runtime storage to submitting the runtime upgrade to the network.

It does all this using git worktree so it doesn't override anything you might have in progress on your current working branch.

@linear
Copy link

linear bot commented Oct 20, 2023

@codecov
Copy link

codecov bot commented Oct 20, 2023

Codecov Report

Merging #4149 (8c45a61) into main (d34c3f4) will increase coverage by 0%.
Report is 13 commits behind head on main.
The diff coverage is n/a.

@@          Coverage Diff           @@
##            main   #4149    +/-   ##
======================================
  Coverage     72%     72%            
======================================
  Files        378     378            
  Lines      60906   61335   +429     
  Branches   60906   61335   +429     
======================================
+ Hits       43687   43997   +310     
- Misses     14955   15061   +106     
- Partials    2264    2277    +13     

see 59 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kylezs kylezs marked this pull request as ready for review October 20, 2023 13:05
@kylezs kylezs force-pushed the test/compatible-cfe-version-bump branch from 35f79b5 to b108da9 Compare October 20, 2023 13:15
export async function compileBinaries(type: 'runtime' | 'all', projectRoot: string) {
if (type === 'all') {
console.log('Building all the binaries...');
execSync(`cd ${projectRoot} cargo build --release`);
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use pushd instead of cd and then popd so that the current directory doesn't change for whoever runs the script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, didn't know this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried using it, doesn't seem to work with execSync

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, of course, it doesn't work because execSync would spawn different shells each time, and so the same commands don't share the same "directory stack". Could probably do something similar in JS by remembering pwd before cd, but not super important.

const fromTomlVersion = await readPackageTomlVersion(path.dirname(process.cwd()));
console.log("Version we're upgrading from: " + fromTomlVersion);

// abcd124 ensures there's no bracnh with the same name that will stop us from creating the workspace.
Copy link
Contributor

Choose a reason for hiding this comment

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

To which abcd124 is this referring to?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stale comment, removed.

console.log("Version we're upgrading from: " + fromTomlVersion);

// abcd124 ensures there's no bracnh with the same name that will stop us from creating the workspace.
// tmp/ is ignored in the bouncer .gitignore file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to ask the OS for a temporary directory? We do that with rust in other places, but not sure how to do that with JS. Looks like mktemp is a common GNU utility for that (seems to come by default with osx at least).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, I did think about doing something like this, but I liked keeping everything in this same file structure where possible rather than going out to /var/ etc, where the folder names change. Is a little simpler to reason about the file paths this way. And if wanted to add an option to not clean up the files when upgraded - it's nicer to have the prebuilt files in a pre-known directory for example.

@kylezs kylezs enabled auto-merge (squash) October 26, 2023 10:06
@kylezs kylezs disabled auto-merge October 26, 2023 10:07
@kylezs kylezs force-pushed the test/compatible-cfe-version-bump branch from 4130ea8 to 8c45a61 Compare October 26, 2023 11:05
@kylezs kylezs enabled auto-merge (squash) October 26, 2023 11:06
@kylezs kylezs merged commit 20e67c3 into main Oct 26, 2023
@kylezs kylezs deleted the test/compatible-cfe-version-bump branch October 26, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants