Skip to content

Commit

Permalink
unblock release pipeline (#1065)
Browse files Browse the repository at this point in the history
Release pipeline is [currently
failing](https://github.com/NomicFoundation/slang/actions/runs/10281343571/job/28451407526)
due to a tiny bug in `Command::run()`, where it returns a `Result<()>`
even though it is panicking on error. This PR fixes that, and fixes all
callsites to not expect a result.

Just a mechanical find/replace. The only functional change (the bug fix)
is in `crates/infra/cli/src/toolchains/mkdocs/mod.rs` to use
`Command::evaluate()` instead to check for the sub-process result.
  • Loading branch information
OmarTawfik authored Aug 7, 2024
1 parent c86b86e commit cb36a4b
Show file tree
Hide file tree
Showing 27 changed files with 172 additions and 159 deletions.
18 changes: 10 additions & 8 deletions crates/infra/cli/src/commands/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,15 @@ impl OrderedCommand for CheckCommand {
match self {
CheckCommand::Cargo => check_cargo(),
CheckCommand::Rustdoc => check_rustdoc(),
CheckCommand::Npm => check_npm(),
CheckCommand::Npm => check_npm()?,
CheckCommand::Mkdocs => check_mkdocs(),
}
};

Ok(())
}
}

fn check_cargo() -> Result<()> {
fn check_cargo() {
// 'cargo clippy' will run both 'cargo check', and 'clippy' lints:
Command::new("cargo")
.arg("clippy")
Expand All @@ -55,18 +57,18 @@ fn check_cargo() -> Result<()> {
.flag("--all-targets")
.flag("--no-deps")
.add_build_rustflags()
.run()
.run();
}

fn check_rustdoc() -> Result<()> {
fn check_rustdoc() {
Command::new("cargo")
.arg("doc")
.flag("--workspace")
.flag("--all-features")
.flag("--no-deps")
.flag("--document-private-items")
.add_build_rustflags()
.run()
.run();
}

fn check_npm() -> Result<()> {
Expand All @@ -77,6 +79,6 @@ fn check_npm() -> Result<()> {
Ok(())
}

fn check_mkdocs() -> Result<()> {
Mkdocs::check()
fn check_mkdocs() {
Mkdocs::check();
}
6 changes: 1 addition & 5 deletions crates/infra/cli/src/commands/completions/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use clap::{CommandFactory, Parser};
use clap_complete::Generator;

Expand All @@ -11,13 +10,10 @@ pub struct CompletionController {
}

impl CompletionController {
#[allow(clippy::unnecessary_wraps)] // for consistency with other commands
pub fn execute(&self) -> Result<()> {
pub fn execute(&self) {
let mut command = crate::commands::Cli::command();
command.build(); // Required to generate completions

self.shell.generate(&command, &mut std::io::stdout());

Ok(())
}
}
46 changes: 28 additions & 18 deletions crates/infra/cli/src/commands/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,33 @@ impl OrderedCommand for LintCommand {
match self {
LintCommand::Cspell => run_cspell(),
LintCommand::Prettier => run_prettier(),
LintCommand::MarkdownLinkCheck => run_markdown_link_check(),
LintCommand::MarkdownLint => run_markdown_lint(),
LintCommand::MarkdownLinkCheck => run_markdown_link_check()?,
LintCommand::MarkdownLint => run_markdown_lint()?,
LintCommand::Rustfmt => run_rustfmt(),
LintCommand::Shellcheck => run_shellcheck(),
LintCommand::Shellcheck => run_shellcheck()?,
LintCommand::Tsc => run_tsc(),
LintCommand::Yamllint => run_yamllint(),
}
LintCommand::Yamllint => run_yamllint()?,
};

Ok(())
}
}

fn run_cspell() -> Result<()> {
fn run_cspell() {
Command::new("cspell")
.arg("lint")
.flag("--show-context")
.flag("--show-suggestions")
.flag("--dot")
.flag("--gitignore")
.run()
.run();
}

fn run_prettier() -> Result<()> {
fn run_prettier() {
if GitHub::is_running_in_ci() {
Command::new("prettier").property("--check", ".").run()
Command::new("prettier").property("--check", ".").run();
} else {
Command::new("prettier").property("--write", ".").run()
Command::new("prettier").property("--write", ".").run();
}
}

Expand All @@ -82,9 +84,11 @@ fn run_markdown_link_check() -> Result<()> {

let markdown_files = FileWalker::from_repo_root().find(["**/*.md"])?;

return Command::new("markdown-link-check")
Command::new("markdown-link-check")
.property("--config", config_file.unwrap_str())
.run_xargs(markdown_files);

Ok(())
}

fn run_markdown_lint() -> Result<()> {
Expand All @@ -99,10 +103,12 @@ fn run_markdown_lint() -> Result<()> {
command = command.flag("--fix");
}

command.run_xargs(markdown_files)
command.run_xargs(markdown_files);

Ok(())
}

fn run_rustfmt() -> Result<()> {
fn run_rustfmt() {
let mut command = Command::new("cargo-fmt")
.arg(format!("+{}", env!("RUST_NIGHTLY_VERSION")))
.flag("--all")
Expand All @@ -112,7 +118,7 @@ fn run_rustfmt() -> Result<()> {
command = command.flag("--check");
}

command.run()
command.run();
}

fn run_shellcheck() -> Result<()> {
Expand All @@ -123,13 +129,15 @@ fn run_shellcheck() -> Result<()> {
path
});

Command::new("shellcheck").run_xargs(bash_files)
Command::new("shellcheck").run_xargs(bash_files);

Ok(())
}

fn run_tsc() -> Result<()> {
fn run_tsc() {
let root_project = Path::repo_path("tsconfig.json");

return Command::new("tsc")
Command::new("tsc")
.property("--build", root_project.unwrap_str())
.run();
}
Expand All @@ -144,8 +152,10 @@ fn run_yamllint() -> Result<()> {
path
});

return PipEnv::run("yamllint")
PipEnv::run("yamllint")
.flag("--strict")
.property("--config-file", config_file.unwrap_str())
.run_xargs(yaml_files);

Ok(())
}
20 changes: 11 additions & 9 deletions crates/infra/cli/src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,16 +71,18 @@ impl Cli {
impl AppCommand {
pub fn execute(&self) -> Result<()> {
match self {
AppCommand::Setup(controller) => controller.execute(),
AppCommand::Check(controller) => controller.execute(),
AppCommand::Test(controller) => controller.execute(),
AppCommand::Lint(controller) => controller.execute(),
AppCommand::Ci(controller) => controller.execute(),
AppCommand::Setup(controller) => controller.execute()?,
AppCommand::Check(controller) => controller.execute()?,
AppCommand::Test(controller) => controller.execute()?,
AppCommand::Lint(controller) => controller.execute()?,
AppCommand::Ci(controller) => controller.execute()?,
AppCommand::Run(controller) => controller.execute(),
AppCommand::Watch(controller) => controller.execute(),
AppCommand::Perf(controller) => controller.execute(),
AppCommand::Publish(controller) => controller.execute(),
AppCommand::Watch(controller) => controller.execute()?,
AppCommand::Perf(controller) => controller.execute()?,
AppCommand::Publish(controller) => controller.execute()?,
AppCommand::Completions(controller) => controller.execute(),
}
};

Ok(())
}
}
15 changes: 7 additions & 8 deletions crates/infra/cli/src/commands/perf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl PerfController {
// Bencher supports multiple languages/frameworks: https://bencher.dev/docs/explanation/adapters/
// We currently only have one benchmark suite (Rust/iai), but we can add more here in the future.

run_iai_bench("solidity_testing_perf", "iai")?;
run_iai_bench("solidity_testing_perf", "iai");
}
};

Expand Down Expand Up @@ -62,10 +62,11 @@ fn install_perf_tools() -> Result<()> {
Ok(())
}

fn run_iai_bench(package_name: &str, bench_name: &str) -> Result<()> {
if std::env::var("BENCHER_API_TOKEN").is_err() {
bail!("BENCHER_API_TOKEN is not set. Please set it to your Bencher API token: https://bencher.dev/console");
}
fn run_iai_bench(package_name: &str, bench_name: &str) {
assert!(
std::env::var("BENCHER_API_TOKEN").is_ok(),
"BENCHER_API_TOKEN is not set. Please set it to your Bencher API token: https://bencher.dev/console",
);

let cargo_command = format!("cargo bench --package {package_name} --bench {bench_name}");

Expand All @@ -81,7 +82,7 @@ fn run_iai_bench(package_name: &str, bench_name: &str) -> Result<()> {
.property("--adapter", "rust_iai_callgrind")
.property("--testbed", testbed)
.arg(cargo_command)
.run()?;
.run();

let reports_dir = Path::repo_path("target/iai")
.join(package_name)
Expand All @@ -97,6 +98,4 @@ Reports/Logs: {reports_dir:?}
- DHAT traces (dhat.*.out) can be viewed using the [dhat/dh_view.html] tool from the Valgrind release [https://valgrind.org/downloads/].
");

Ok(())
}
12 changes: 6 additions & 6 deletions crates/infra/cli/src/commands/publish/cargo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl CargoController {
changeset.commit_changes()?;

for crate_name in &changed_crates {
run_cargo_publish(crate_name, self.dry_run)?;
run_cargo_publish(crate_name, self.dry_run);
}

changeset.revert_changes()?;
Expand Down Expand Up @@ -109,11 +109,11 @@ fn strip_publish_markers(cargo_toml_path: &Path) -> Result<bool> {

fn update_cargo_lock(changeset: &mut TemporaryChangeset) -> Result<()> {
let cargo_lock_path = Path::repo_path("Cargo.lock");
let old_contents = std::fs::read_to_string(&cargo_lock_path)?;
let old_contents = cargo_lock_path.read_to_string()?;

Command::new("cargo").arg("check").run()?;
Command::new("cargo").arg("check").run();

let new_contents = std::fs::read_to_string(&cargo_lock_path)?;
let new_contents = cargo_lock_path.read_to_string()?;

if new_contents != old_contents {
changeset.expect_change(&cargo_lock_path);
Expand All @@ -122,7 +122,7 @@ fn update_cargo_lock(changeset: &mut TemporaryChangeset) -> Result<()> {
Ok(())
}

fn run_cargo_publish(crate_name: &str, dry_run: DryRun) -> Result<()> {
fn run_cargo_publish(crate_name: &str, dry_run: DryRun) {
let mut command = Command::new("cargo")
.arg("publish")
.property("--package", crate_name)
Expand All @@ -132,5 +132,5 @@ fn run_cargo_publish(crate_name: &str, dry_run: DryRun) -> Result<()> {
command = command.flag("--dry-run");
}

command.run()
command.run();
}
10 changes: 5 additions & 5 deletions crates/infra/cli/src/commands/publish/changesets/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl ChangesetsController {
// 2) Update the CHANGELOG.md file for the NPM package.
// 3) Bump the version in its package.json accordingly.

Command::new("changeset").arg("version").run()?;
Command::new("changeset").arg("version").run();

let updated_version = NapiConfig::local_version(&package_dir)?;
println!("Updated version: {updated_version}");
Expand All @@ -56,14 +56,14 @@ impl ChangesetsController {
let package_dir = resolver.main_package_dir();
Command::new("prettier")
.property("--write", package_dir.unwrap_str())
.run()?;
.run();

// Update NPM lock file:

Command::new("npm")
.arg("install")
.flag("--package-lock-only")
.run()?;
.run();

// Update Cargo workspace:

Expand All @@ -75,7 +75,7 @@ impl ChangesetsController {
Command::new("cargo")
.arg("update")
.flag("--workspace")
.run()?;
.run();

// Update other CHANGELOG files:

Expand All @@ -92,7 +92,7 @@ impl ChangesetsController {
.args(["stash", "push"])
.flag("--include-untracked")
.property("--message", "applied changesets")
.run()?;
.run();

println!();
println!("Source files are now updated with the new version, and stored in a 'git stash'.");
Expand Down
6 changes: 4 additions & 2 deletions crates/infra/cli/src/commands/publish/mkdocs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ impl MkdocsController {
pub fn execute(&self) -> Result<()> {
match self.target {
PublishTarget::MainBranch => Mkdocs::publish_main_branch(self.dry_run),
PublishTarget::LatestRelease => Mkdocs::publish_latest_release(self.dry_run),
}
PublishTarget::LatestRelease => Mkdocs::publish_latest_release(self.dry_run)?,
};

Ok(())
}
}
4 changes: 3 additions & 1 deletion crates/infra/cli/src/commands/publish/npm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,7 @@ fn publish_package(
command = command.flag("--dry-run");
}

command.run()
command.run();

Ok(())
}
5 changes: 2 additions & 3 deletions crates/infra/cli/src/commands/run/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use clap::{Parser, ValueEnum};
use infra_utils::commands::Command;
use infra_utils::terminal::Terminal;
Expand Down Expand Up @@ -33,7 +32,7 @@ enum BinaryName {
}

impl RunController {
pub fn execute(&self) -> Result<()> {
pub fn execute(&self) {
let bin = self.bin.clap_name();

Terminal::step(format!("run {bin}"));
Expand All @@ -48,6 +47,6 @@ impl RunController {
.property("--bin", bin)
.arg("--")
.args(&self.args)
.run()
.run();
}
}
Loading

0 comments on commit cb36a4b

Please sign in to comment.