diff --git a/Cargo.toml b/Cargo.toml index 4a0dde6330e..99b25cce2d3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -433,6 +433,9 @@ declare_interior_mutable_const = "warn" # We'd like to warn on lossy casts in the future, but lossless casts are the # easiest ones to convert over. cast_lossless = "warn" +# This lint catches format-style placeholders in string literals outside format +# macros, e.g. `.context("failed to read {path}")`. +literal_string_with_formatting_args = "warn" [workspace.dependencies] anyhow = "1.0" diff --git a/dev-tools/downloader/src/lib.rs b/dev-tools/downloader/src/lib.rs index bf210a988c2..5b7bce2a2ac 100644 --- a/dev-tools/downloader/src/lib.rs +++ b/dev-tools/downloader/src/lib.rs @@ -363,7 +363,10 @@ async fn get_values_from_file( } } if !keys.is_empty() { - bail!("Could not find keys: {:?}", keys.keys().collect::>(),); + bail!( + "Could not find keys {:?} in {path}", + keys.keys().collect::>(), + ); } Ok(values) } @@ -449,30 +452,21 @@ async fn unpack_gzip( task.await? } -async fn clickhouse_confirm_binary_works(binary: &Utf8Path) -> Result<()> { - let mut cmd = Command::new(binary); - cmd.args(["server", "--version"]); - - let output = - cmd.output().await.context(format!("Failed to run {binary}"))?; - if !output.status.success() { - let stderr = - String::from_utf8(output.stderr).unwrap_or_else(|_| String::new()); - bail!("{binary} failed: {} (stderr: {stderr})", output.status); - } - Ok(()) -} - -async fn cockroach_confirm_binary_works(binary: &Utf8Path) -> Result<()> { +async fn confirm_binary_works(binary: &Utf8Path, args: &[&str]) -> Result<()> { let mut cmd = Command::new(binary); - cmd.arg("version"); + cmd.args(args); - let output = - cmd.output().await.context(format!("Failed to run {binary}"))?; + let output = cmd + .output() + .await + .with_context(|| format!("Failed to run {binary}"))?; if !output.status.success() { - let stderr = - String::from_utf8(output.stderr).unwrap_or_else(|_| String::new()); - bail!("{binary} failed: {} (stderr: {stderr})", output.status); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + bail!( + "{binary} {args:?} failed: {} (stdout: {stdout}, stderr: {stderr})", + output.status + ); } Ok(()) } @@ -566,6 +560,18 @@ async fn download_file_and_verify( } impl Downloader<'_> { + async fn read_version_file(&self, filename: &str) -> Result { + let path = self.versions_dir.join(filename); + let contents = tokio::fs::read_to_string(&path) + .await + .with_context(|| format!("Failed to read version from {path}"))?; + let version = contents.trim(); + if version.is_empty() { + bail!("Version file {path} should not be empty"); + } + Ok(version.to_string()) + } + async fn download_cargo_hack(&self) -> Result<()> { let os = os_name()?; let arch = arch()?; @@ -580,11 +586,7 @@ impl Downloader<'_> { ) .await?; - let versions_path = self.versions_dir.join("cargo_hack_version"); - let version = tokio::fs::read_to_string(&versions_path) - .await - .context("Failed to read version from {versions_path}")?; - let version = version.trim(); + let version = self.read_version_file("cargo_hack_version").await?; let (platform, supported_arch) = match (os, arch) { (Os::Illumos, Arch::X86_64) => ("unknown-illumos", "x86_64"), @@ -632,11 +634,7 @@ impl Downloader<'_> { ) .await?; - let versions_path = self.versions_dir.join("clickhouse_version"); - let version = tokio::fs::read_to_string(&versions_path) - .await - .context("Failed to read version from {versions_path}")?; - let version = version.trim(); + let version = self.read_version_file("clickhouse_version").await?; const S3_BUCKET: &'static str = "https://oxide-clickhouse-build.s3.us-west-2.amazonaws.com"; @@ -668,7 +666,8 @@ impl Downloader<'_> { let clickhouse_binary = destination_dir.join("clickhouse"); info!(self.log, "Checking that binary works"); - clickhouse_confirm_binary_works(&clickhouse_binary).await?; + confirm_binary_works(&clickhouse_binary, &["server", "--version"]) + .await?; Ok(()) } @@ -725,10 +724,12 @@ impl Downloader<'_> { let binary_dir = destination_dir.join("bin"); tokio::fs::create_dir_all(&binary_dir).await?; let src = tarball_path.with_file_name("cockroach").join("cockroach"); - tokio::fs::copy(src, &cockroach_binary).await?; + tokio::fs::copy(&src, &cockroach_binary).await.with_context(|| { + format!("Failed to copy {src} to {cockroach_binary}") + })?; info!(self.log, "Checking that binary works"); - cockroach_confirm_binary_works(&cockroach_binary).await?; + confirm_binary_works(&cockroach_binary, &["version"]).await?; Ok(()) } diff --git a/dev-tools/omdb/src/bin/omdb/db.rs b/dev-tools/omdb/src/bin/omdb/db.rs index d2cdf2ddb56..55f03ebc748 100644 --- a/dev-tools/omdb/src/bin/omdb/db.rs +++ b/dev-tools/omdb/src/bin/omdb/db.rs @@ -4452,6 +4452,9 @@ async fn cmd_db_region_replacement_status( .await?; if let Some(repair_progress) = maybe_repair_progress { + // {wide_bar:.green}, etc. are indicatif template + // placeholders, not Rust format args. + #[expect(clippy::literal_string_with_formatting_args)] let bar = ProgressBar::with_draw_target( Some(repair_progress.total_items as u64), ProgressDrawTarget::stdout(), @@ -4552,6 +4555,9 @@ async fn cmd_db_region_replacement_info( .await?; if let Some(repair_progress) = maybe_repair_progress { + // {wide_bar:.green}, etc. are indicatif template + // placeholders, not Rust format args. + #[expect(clippy::literal_string_with_formatting_args)] let bar = ProgressBar::with_draw_target( Some(repair_progress.total_items as u64), ProgressDrawTarget::stdout(), diff --git a/dev-tools/repo-depot-standalone/src/main.rs b/dev-tools/repo-depot-standalone/src/main.rs index c1944fd1acf..90f510036c9 100644 --- a/dev-tools/repo-depot-standalone/src/main.rs +++ b/dev-tools/repo-depot-standalone/src/main.rs @@ -98,8 +98,9 @@ impl RepoDepotStandalone { .load_zip_path(repo_path.clone(), &log) .await .with_context(|| format!("load {:?}", repo_path))?; - ctx.load_repo(repo) - .context("loading artifacts from repository at {repo_path}")?; + ctx.load_repo(repo).with_context(|| { + format!("loading artifacts from repository at {repo_path}") + })?; info!(&log, "loaded Omicron TUF repository"; "path" => %repo_path); } diff --git a/flake.nix b/flake.nix index ecc9048146f..3305524fda8 100644 --- a/flake.nix +++ b/flake.nix @@ -103,7 +103,8 @@ prefix = "${upperName}=\""; in trivial.pipe shas [ - (lists.findFirst (strings.hasPrefix prefix) "") + (lists.findFirst (strings.hasPrefix prefix) + (throw "findSha: no '${upperName}' entry found in the provided checksums -- add it to the corresponding tools/*_checksums file")) (strings.removePrefix prefix) (strings.removeSuffix "\"") ]); diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 759245082c2..300859359d4 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -297,9 +297,9 @@ impl DataStore { .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?; match subnet { Some(subnet) => Ok(subnet), - None => Err(Error::internal_error( - "DB Error(bug): returned a null subnet for {rack_id}", - )), + None => Err(Error::internal_error(&format!( + "DB Error(bug): returned a null subnet for {rack_id}" + ))), } } diff --git a/nexus/db-queries/src/db/datastore/sled.rs b/nexus/db-queries/src/db/datastore/sled.rs index 5cd18a14e05..4ae78431c31 100644 --- a/nexus/db-queries/src/db/datastore/sled.rs +++ b/nexus/db-queries/src/db/datastore/sled.rs @@ -2847,10 +2847,10 @@ pub(in crate::db::datastore) mod test { all_groups: &AllGroups, ) { for our_group_name in &self.groups { - let (affinity, group_id) = all_groups - .id_by_name - .get(our_group_name) - .expect("Group not found: {our_group_name}"); + let (affinity, group_id) = + all_groups.id_by_name.get(our_group_name).unwrap_or_else( + || panic!("Group not found: {our_group_name}"), + ); match *affinity { Affinity::Positive => { add_instance_to_affinity_group( diff --git a/nexus/db-queries/src/db/queries/vpc_subnet.rs b/nexus/db-queries/src/db/queries/vpc_subnet.rs index b9013444d09..7c95411030d 100644 --- a/nexus/db-queries/src/db/queries/vpc_subnet.rs +++ b/nexus/db-queries/src/db/queries/vpc_subnet.rs @@ -398,8 +398,10 @@ mod test { other_ipv6_block, ); let err = db_datastore.vpc_create_subnet_raw(new_row).await.expect_err( - "Should not be able to insert VPC Subnet with \ - overlapping IPv4 range {overlapping_ipv4_block_longer}", + &format!( + "Should not be able to insert VPC Subnet with \ + overlapping IPv4 range {overlapping_ipv4_block_longer}" + ), ); assert_eq!( err, @@ -416,8 +418,10 @@ mod test { other_ipv6_block, ); let err = db_datastore.vpc_create_subnet_raw(new_row).await.expect_err( - "Should not be able to insert VPC Subnet with \ - overlapping IPv4 range {overlapping_ipv4_block_shorter}", + &format!( + "Should not be able to insert VPC Subnet with \ + overlapping IPv4 range {overlapping_ipv4_block_shorter}" + ), ); assert_eq!( err, @@ -434,8 +438,10 @@ mod test { overlapping_ipv6_block_longer, ); let err = db_datastore.vpc_create_subnet_raw(new_row).await.expect_err( - "Should not be able to insert VPC Subnet with \ - overlapping IPv6 range {overlapping_ipv6_block_longer}", + &format!( + "Should not be able to insert VPC Subnet with \ + overlapping IPv6 range {overlapping_ipv6_block_longer}" + ), ); assert_eq!( err, @@ -452,8 +458,10 @@ mod test { overlapping_ipv6_block_shorter, ); let err = db_datastore.vpc_create_subnet_raw(new_row).await.expect_err( - "Should not be able to insert VPC Subnet with \ - overlapping IPv6 range {overlapping_ipv6_block_shorter}", + &format!( + "Should not be able to insert VPC Subnet with \ + overlapping IPv6 range {overlapping_ipv6_block_shorter}" + ), ); assert_eq!( err, diff --git a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs index 913cc1c3ee6..87fe2e3ee81 100644 --- a/nexus/reconfigurator/planning/tests/integration_tests/planner.rs +++ b/nexus/reconfigurator/planning/tests/integration_tests/planner.rs @@ -4260,7 +4260,7 @@ fn test_update_boundary_ntp() { let config = &sled .last_reconciliation .as_ref() - .expect("Sled missing ledger? {sled:?}") + .unwrap_or_else(|| panic!("Sled missing ledger? {sled:?}")) .last_reconciled_config; let Some(zone_id) = diff --git a/nexus/src/app/sled.rs b/nexus/src/app/sled.rs index b14576af092..f24b4829516 100644 --- a/nexus/src/app/sled.rs +++ b/nexus/src/app/sled.rs @@ -371,9 +371,9 @@ impl super::Nexus { || existing_disk.serial != request.serial || existing_disk.model != request.model { - return Err(Error::internal_error( - "Invalid Physical Disk update (was: {existing_disk:?}, asking for {request:?})", - )); + return Err(Error::internal_error(&format!( + "Invalid Physical Disk update (was: {existing_disk:?}, asking for {request:?})" + ))); } return Ok(()); } diff --git a/nexus/test-utils/src/starter.rs b/nexus/test-utils/src/starter.rs index 4e9e0f3b774..60d04d2d3bf 100644 --- a/nexus/test-utils/src/starter.rs +++ b/nexus/test-utils/src/starter.rs @@ -1008,15 +1008,22 @@ impl<'a, N: NexusServer> ControlPlaneStarter<'a, N> { measurements: BTreeSet::new(), }) .await - .expect("Failed to configure sled agent {sled_id} with zones"); + .unwrap_or_else(|err| { + panic!( + "Failed to configure sled agent {sled_id} \ + with zones: {err:?}" + ) + }); client .write_network_bootstore_config(&early_network_config) .await - .expect( - "Failed to write early networking config \ - to bootstore on sled {sled_id}", - ); + .unwrap_or_else(|err| { + panic!( + "Failed to write early networking config \ + to bootstore on sled {sled_id}: {err:?}" + ) + }); } } diff --git a/oximeter/oxql-types/src/point.rs b/oximeter/oxql-types/src/point.rs index 04e2344f73b..aed6a3eac8c 100644 --- a/oximeter/oxql-types/src/point.rs +++ b/oximeter/oxql-types/src/point.rs @@ -726,9 +726,9 @@ impl Points { let mut new = Vec::with_capacity(doubles.len()); for maybe_double in doubles.iter().copied() { if let Some(d) = maybe_double { - let as_int = d - .to_i64() - .context("Cannot cast double {d} to i64")?; + let as_int = d.to_i64().with_context(|| { + format!("Cannot cast double {d} to i64") + })?; new.push(Some(as_int)); } else { new.push(None); @@ -753,9 +753,9 @@ impl Points { let mut new = Vec::with_capacity(strings.len()); for maybe_str in strings.iter() { if let Some(s) = maybe_str { - let as_int = s - .parse() - .context("Cannot cast string '{s}' to i64")?; + let as_int = s.parse().with_context(|| { + format!("Cannot cast string '{s}' to i64") + })?; new.push(Some(as_int)); } else { new.push(None); @@ -769,9 +769,12 @@ impl Points { let mut new = Vec::with_capacity(ints.len()); for maybe_int in ints.iter().copied() { if let Some(int) = maybe_int { - let as_double = int.to_f64().context( - "Cannot cast integer {int} as double", - )?; + let as_double = + int.to_f64().with_context(|| { + format!( + "Cannot cast integer {int} as double" + ) + })?; new.push(Some(as_double)); } else { new.push(None); @@ -801,9 +804,9 @@ impl Points { let mut new = Vec::with_capacity(strings.len()); for maybe_str in strings.iter() { if let Some(s) = maybe_str { - let as_double = s - .parse() - .context("Cannot cast string '{s}' to f64")?; + let as_double = s.parse().with_context(|| { + format!("Cannot cast string '{s}' to f64") + })?; new.push(Some(as_double)); } else { new.push(None); diff --git a/sled-agent/config-reconciler/src/debug_collector/file_archiver/filesystem.rs b/sled-agent/config-reconciler/src/debug_collector/file_archiver/filesystem.rs index 42deb80c01c..63d4f9955fe 100644 --- a/sled-agent/config-reconciler/src/debug_collector/file_archiver/filesystem.rs +++ b/sled-agent/config-reconciler/src/debug_collector/file_archiver/filesystem.rs @@ -67,7 +67,7 @@ impl FileLister for FilesystemLister { return vec![]; } else { return vec![Err( - anyhow!(error).context("readdir {path:?}") + anyhow!(error).context(format!("readdir {path:?}")) )]; } }