From c311b23834df9e057fa3937f1b50f602308bde5b Mon Sep 17 00:00:00 2001 From: Andre Popovitch Date: Wed, 19 Feb 2025 10:09:11 +0900 Subject: [PATCH] feat(release-runscript): Ask user if they want to retry if a script fails (#4007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This prevents a situation where they then need to go annoyingly restart the whole process Marked as draft for now to prevent accidental merging until the previous PR in this chain is merged. [← Previous PR](https://github.com/dfinity/ic/pull/4004) --- Cargo.lock | 1 + .../tools/release-runscript/BUILD.bazel | 2 + .../tools/release-runscript/Cargo.toml | 3 +- .../tools/release-runscript/src/commands.rs | 69 +++++++++++ .../tools/release-runscript/src/main.rs | 110 +++++++----------- 5 files changed, 117 insertions(+), 68 deletions(-) create mode 100644 rs/nervous_system/tools/release-runscript/src/commands.rs diff --git a/Cargo.lock b/Cargo.lock index eb2843d62a1..036c118a31e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -19144,6 +19144,7 @@ dependencies = [ "ic-nervous-system-common-test-keys", "ic-nns-common", "ic-nns-constants", + "itertools 0.12.1", "rgb", "serde", "serde_json", diff --git a/rs/nervous_system/tools/release-runscript/BUILD.bazel b/rs/nervous_system/tools/release-runscript/BUILD.bazel index bfcc104d17e..5b4b15fa9ec 100644 --- a/rs/nervous_system/tools/release-runscript/BUILD.bazel +++ b/rs/nervous_system/tools/release-runscript/BUILD.bazel @@ -17,6 +17,7 @@ DEPENDENCIES = [ "@crate_index//:colored", "@crate_index//:futures", "@crate_index//:ic-agent", + "@crate_index//:itertools", "@crate_index//:rgb", "@crate_index//:serde", "@crate_index//:serde_json", @@ -28,6 +29,7 @@ DEPENDENCIES = [ rust_binary( name = "release-runscript", srcs = [ + "src/commands.rs", "src/commit_switcher.rs", "src/main.rs", "src/utils.rs", diff --git a/rs/nervous_system/tools/release-runscript/Cargo.toml b/rs/nervous_system/tools/release-runscript/Cargo.toml index cb5ed3e6fb2..c969104d445 100644 --- a/rs/nervous_system/tools/release-runscript/Cargo.toml +++ b/rs/nervous_system/tools/release-runscript/Cargo.toml @@ -24,9 +24,10 @@ ic-nervous-system-clients = { path = "../../clients" } ic-nervous-system-common-test-keys = { path = "../../common/test_keys" } ic-nns-common = { path = "../../../nns/common" } ic-nns-constants = { path = "../../../nns/constants" } +itertools = { workspace = true } rgb = "0.8.37" serde = { workspace = true } serde_json = { workspace = true } tempfile = { workspace = true } tokio = { workspace = true } -url = { workspace = true } \ No newline at end of file +url = { workspace = true } diff --git a/rs/nervous_system/tools/release-runscript/src/commands.rs b/rs/nervous_system/tools/release-runscript/src/commands.rs new file mode 100644 index 00000000000..6f2998e13f2 --- /dev/null +++ b/rs/nervous_system/tools/release-runscript/src/commands.rs @@ -0,0 +1,69 @@ +use crate::utils::input_yes_or_no; +use anyhow::Result; +use itertools::Itertools; +use std::path::PathBuf; +use std::process::{Command, Stdio}; + +pub(crate) fn run_script( + script: PathBuf, + args: &[&str], + cwd: &PathBuf, +) -> Result { + loop { + let output = Command::new(&script).args(args).current_dir(cwd).output()?; + + if output.status.success() { + return Ok(output); + } else { + let command_str = format!( + "{} {}", + script.display(), + args.iter().map(|s| format!("\"{}\"", s)).join(" ") + ); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + eprintln!("Script failed: {}", stderr); + eprintln!("Failed to run command: {}", command_str); + if input_yes_or_no("Do you want to try again?", true)? { + continue; + } else { + return Err(anyhow::anyhow!("{}\n{}", stdout, stderr) + .context(format!("Failed to run command: {}", command_str))); + } + } + } +} + +pub(crate) fn run_script_in_current_process( + script: PathBuf, + args: &[&str], + cwd: &PathBuf, +) -> Result { + loop { + let output = Command::new(&script) + .args(args) + .current_dir(cwd) + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) + .output()?; + + if output.status.success() { + return Ok(output); + } else { + let command_str = format!( + "{} {}", + script.display(), + args.iter().map(|s| format!("\"{}\"", s)).join(" ") + ); + // we can't read stdout or stderr here because it's piped to the current process + eprintln!("Script failed :("); + eprintln!("Failed to run command: {}", command_str); + if input_yes_or_no("Do you want to try again?", true)? { + continue; + } else { + return Err(anyhow::anyhow!("Failed to run command: {}", command_str)); + } + } + } +} diff --git a/rs/nervous_system/tools/release-runscript/src/main.rs b/rs/nervous_system/tools/release-runscript/src/main.rs index 9149bc3ca45..8df2a4b9659 100644 --- a/rs/nervous_system/tools/release-runscript/src/main.rs +++ b/rs/nervous_system/tools/release-runscript/src/main.rs @@ -1,11 +1,13 @@ +mod commands; mod commit_switcher; mod utils; + use anyhow::{bail, Result}; use clap::{Parser, Subcommand}; use colored::*; +use commands::{run_script, run_script_in_current_process}; use commit_switcher::CommitSwitcher; use std::path::PathBuf; -use std::process::{Command, Stdio}; use url::Url; use utils::*; @@ -143,10 +145,7 @@ fn run_pick_commit() -> Result<()> { let cmd_path = ic.join("testnet/tools/nns-tools/cmd.sh"); // Run the command with the required argument. - let output = Command::new(cmd_path) - .arg("latest_commit_with_prebuilt_artifacts") - .current_dir(&ic) - .output()?; + let output = run_script(cmd_path, &["latest_commit_with_prebuilt_artifacts"], &ic)?; let commit = if output.status.success() { let commit = String::from_utf8_lossy(&output.stdout).trim().to_string(); @@ -295,20 +294,11 @@ fn run_create_proposal_texts(cmd: CreateProposalTexts) -> Result<()> { let script = ic.join("testnet/tools/nns-tools/prepare-nns-upgrade-proposal-text.sh"); // cycles minting requires an upgrade arg, usually '()' let output = if canister != "cycles-minting" { - Command::new(script) - .arg(canister) - .arg(&commit) - .current_dir(&ic) - .output() + run_script(script, &[canister, &commit], &ic) .expect("Failed to run NNS proposal text script") } else { let upgrade_arg = input_with_default("Upgrade arg for CMC?", "()")?; - Command::new(script) - .arg(canister) - .arg(&commit) - .arg(upgrade_arg) - .current_dir(&ic) - .output() + run_script(script, &[canister, &commit, &upgrade_arg], &ic) .expect("Failed to run NNS proposal text script") }; if !output.status.success() { @@ -332,12 +322,7 @@ fn run_create_proposal_texts(cmd: CreateProposalTexts) -> Result<()> { // The SNS script is expected to write directly to the file provided as an argument. let file_path = proposals_dir.join(format!("sns-{}.md", canister)); let file_path_str = file_path.to_str().expect("Invalid file path"); - let output = Command::new(script) - .arg(canister) - .arg(&commit) - .arg(file_path_str) - .current_dir(&ic) - .output() + let output = run_script(script, &[canister, &commit, file_path_str], &ic) .expect("Failed to run SNS proposal text script"); if !output.status.success() { bail!( @@ -365,7 +350,7 @@ fn run_create_proposal_texts(cmd: CreateProposalTexts) -> Result<()> { println!("The following proposals have TODOs. Please review them and remove the TODOs before submitting."); for proposal_text_path in &proposals_with_todos { println!(" - {}", proposal_text_path.display()); - let mut cmd = Command::new("code"); + let mut cmd = std::process::Command::new("code"); cmd.arg(proposal_text_path); cmd.current_dir(&ic); cmd.output() @@ -432,15 +417,15 @@ fn run_submit_proposals(cmd: SubmitProposals) -> Result<()> { for proposal_path in &nns_proposal_text_paths { println!("Submitting NNS proposal: {}", proposal_path.display()); let script = ic.join("testnet/tools/nns-tools/submit-mainnet-nns-upgrade-proposal.sh"); - let output = Command::new(script) - .arg(proposal_path.to_str().expect("Invalid proposal path")) - .arg(&neuron_id) - .current_dir(&ic) - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .output() - .expect("Failed to run submit-mainnet-nns-upgrade-proposal.sh"); + let output = run_script_in_current_process( + script, + &[ + proposal_path.to_str().expect("Invalid proposal path"), + &neuron_id, + ], + &ic, + ) + .expect("Failed to run submit-mainnet-nns-upgrade-proposal.sh"); if !output.status.success() { bail!( "Submission failed for {}: {}", @@ -457,15 +442,15 @@ fn run_submit_proposals(cmd: SubmitProposals) -> Result<()> { for proposal_path in &sns_proposal_text_paths { println!("Submitting SNS proposal: {}", proposal_path.display()); let script = ic.join("testnet/tools/nns-tools/submit-mainnet-publish-sns-wasm-proposal.sh"); - let output = Command::new(script) - .arg(proposal_path.to_str().expect("Invalid proposal path")) - .arg(&neuron_id) - .current_dir(&ic) - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()) - .output() - .expect("Failed to run submit-mainnet-publish-sns-wasm-proposal.sh"); + let output = run_script_in_current_process( + script, + &[ + proposal_path.to_str().expect("Invalid proposal path"), + &neuron_id, + ], + &ic, + ) + .expect("Failed to run submit-mainnet-publish-sns-wasm-proposal.sh"); if !output.status.success() { bail!( "Submission failed for {}: {}", @@ -528,18 +513,15 @@ fn run_create_forum_post(cmd: CreateForumPost) -> Result<()> { // --- Generate NNS forum post --- { let script = ic.join("testnet/tools/nns-tools/cmd.sh"); - let output = Command::new(script) - .arg("generate_forum_post_nns_upgrades") - .args(nns_proposal_text_paths) - .output() - .expect("Failed to run generate_forum_post_nns_upgrades"); + let mut args = vec!["generate_forum_post_nns_upgrades"]; + let path_strs: Vec<&str> = nns_proposal_text_paths + .iter() + .map(|p| p.to_str().unwrap()) + .collect(); + args.extend(path_strs.iter()); - if !output.status.success() { - bail!( - "Failed to generate NNS forum post: {}", - String::from_utf8_lossy(&output.stderr) - ); - } + let output = + run_script(script, &args, &ic).expect("Failed to run generate_forum_post_nns_upgrades"); copy(&output.stdout)?; @@ -574,18 +556,15 @@ fn run_create_forum_post(cmd: CreateForumPost) -> Result<()> { // --- Generate SNS forum post --- { let script = ic.join("testnet/tools/nns-tools/cmd.sh"); - let output = Command::new(script) - .arg("generate_forum_post_sns_wasm_publish") - .args(sns_proposal_text_paths) - .output() - .expect("Failed to run generate_forum_post_sns_wasm_publish"); + let mut args = vec!["generate_forum_post_sns_wasm_publish"]; + let path_strs: Vec<&str> = sns_proposal_text_paths + .iter() + .map(|p| p.to_str().unwrap()) + .collect(); + args.extend(path_strs.iter()); - if !output.status.success() { - bail!( - "Failed to generate SNS forum post: {}", - String::from_utf8_lossy(&output.stderr) - ); - } + let output = run_script(script, &args, &ic) + .expect("Failed to run generate_forum_post_sns_wasm_publish"); copy(&output.stdout)?; @@ -740,10 +719,7 @@ SNS: {}", for proposal_id in nns_proposal_ids.iter().chain(sns_proposal_ids.iter()) { println!("Updating changelog for proposal {}", proposal_id); let script = ic.join("testnet/tools/nns-tools/add-release-to-changelog.sh"); - let output = Command::new(script) - .arg(proposal_id) - .current_dir(&ic) - .output()?; + let output = run_script(script, &[proposal_id], &ic)?; if !output.status.success() { println!("{}", String::from_utf8_lossy(&output.stderr));