From 87ed0a871caad96829b70263fc443348eaf7a6f3 Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 21:34:46 -0400 Subject: [PATCH 1/4] lib: Write command results to stdout via io::Write println!() panics if stdout is closed (e.g. a broken pipe when piping into head), whereas writeln!() lets us propagate the error normally. These call sites are the actual output of a subcommand (digests, JSON, tables, GC stats), so an explicit stdout writer is the right thing anyway. Prep for denying clippy::print_stdout in crates/lib, see https://github.com/bootc-dev/bootc/pull/2449#discussion_r4072799684 Generated-by: AI --- crates/lib/src/cli.rs | 36 ++++++++++++++++++++++-------------- crates/lib/src/image.rs | 4 +++- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index bc11e07e2d..00dc677564 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -4,7 +4,7 @@ use std::ffi::{CString, OsStr, OsString}; use std::fs::File; -use std::io::{BufWriter, Seek, SeekFrom}; +use std::io::{BufWriter, Seek, SeekFrom, Write}; use std::os::fd::AsFd; use std::os::unix::process::CommandExt; use std::process::Command; @@ -2174,7 +2174,7 @@ async fn run_from_opt(opt: Opt) -> Result { write_dumpfile_to.as_deref(), ) .await?; - println!("{digest}"); + writeln!(std::io::stdout(), "{digest}")?; Ok(()) } ContainerOpts::ComputeComposefsDigestFromStorage { @@ -2227,7 +2227,7 @@ async fn run_from_opt(opt: Opt) -> Result { .context("Populating fs")?; fs.transform_for_boot(&repo).context("Preparing for boot")?; let id = fs.compute_image_id(repo.erofs_version()); - println!("{}", id.to_hex()); + writeln!(std::io::stdout(), "{}", id.to_hex())?; if let Some(path) = write_dumpfile_to.as_deref() { let mut w = File::create(path) @@ -2449,7 +2449,7 @@ async fn run_from_opt(opt: Opt) -> Result { std::fs::File::open(&path).with_context(|| format!("Reading {path}"))?; let digest: fsverity::Sha256HashValue = fsverity::measure_verity(&fd)?; let digest = digest.to_hex(); - println!("{digest}"); + writeln!(std::io::stdout(), "{digest}")?; Ok(()) } FsverityOpts::Enable { path } => { @@ -2524,14 +2524,18 @@ async fn run_from_opt(opt: Opt) -> Result { let loopback = crate::blockdev::LoopbackDevice::new(temp_path) .context("Failed to create loopback device")?; - println!("Created loopback device: {}", loopback.path()); + writeln!( + std::io::stdout(), + "Created loopback device: {}", + loopback.path() + )?; // Close the device to test cleanup loopback .close() .context("Failed to close loopback device")?; - println!("Successfully closed loopback device"); + writeln!(std::io::stdout(), "Successfully closed loopback device")?; Ok(()) } #[cfg(feature = "rhsm")] @@ -2541,7 +2545,7 @@ async fn run_from_opt(opt: Opt) -> Result { use clap::CommandFactory; let cmd = Opt::command(); let json = crate::cli_json::dump_cli_json(&cmd)?; - println!("{}", json); + writeln!(std::io::stdout(), "{json}")?; Ok(()) } InternalsOpts::DirDiff { @@ -2624,20 +2628,23 @@ async fn run_from_opt(opt: Opt) -> Result { ) .await?; + let mut stdout = std::io::stdout().lock(); if dry_run { - println!("Dry run (no files deleted)"); + writeln!(stdout, "Dry run (no files deleted)")?; } - println!( + writeln!( + stdout, "Objects: {} removed ({} bytes)", gc_result.objects_removed, gc_result.objects_bytes - ); + )?; if gc_result.images_pruned > 0 || gc_result.streams_pruned > 0 { - println!( + writeln!( + stdout, "Pruned symlinks: {} images, {} streams", gc_result.images_pruned, gc_result.streams_pruned - ); + )?; } if assert_no_op { @@ -2666,8 +2673,9 @@ async fn run_from_opt(opt: Opt) -> Result { crate::blockdev::list_dev_by_dir(&dir)? } }; - serde_json::to_writer_pretty(std::io::stdout().lock(), &dev)?; - println!(); + let mut stdout = std::io::stdout().lock(); + serde_json::to_writer_pretty(&mut stdout, &dev)?; + writeln!(stdout)?; Ok(()) } InternalsOpts::Uki(uki_opts) => match uki_opts { diff --git a/crates/lib/src/image.rs b/crates/lib/src/image.rs index 2221db4300..8fdaa7af4a 100644 --- a/crates/lib/src/image.rs +++ b/crates/lib/src/image.rs @@ -13,6 +13,8 @@ //! for environments like ext4 where podman access to the OS image matters //! more than disk efficiency. +use std::io::Write; + use anyhow::{Context, Result, bail}; use bootc_utils::CommandRunExt; use cap_std_ext::cap_std::{self, fs::Dir}; @@ -175,7 +177,7 @@ pub(crate) async fn list_entrypoint( table.add_row([image.image, image.image_type.to_string()]); } - println!("{table}"); + writeln!(std::io::stdout(), "{table}")?; } ImageListFormat::Json => { let mut stdout = std::io::stdout(); From 5fd2fcf08363d8a1410e18c5b580b35fbde22840 Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 21:35:17 -0400 Subject: [PATCH 2/4] lib/fsck: Write passing checks to the provided output fsck() already takes an explicit writer for its errors, but the "ok: " lines bypassed it and went straight to stdout. The only caller passes stdout, so the output is unchanged; this just makes the function honor its own API. Prep for denying clippy::print_stdout in crates/lib, see https://github.com/bootc-dev/bootc/pull/2449#discussion_r4072799684 Generated-by: AI --- crates/lib/src/fsck.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/lib/src/fsck.rs b/crates/lib/src/fsck.rs index 25e95e33cf..9fa03e3bc8 100644 --- a/crates/lib/src/fsck.rs +++ b/crates/lib/src/fsck.rs @@ -281,7 +281,7 @@ pub(crate) async fn fsck(storage: &Storage, mut output: impl std::io::Write) -> }; match r { Ok(Ok(())) => { - println!("ok: {name}"); + writeln!(output, "ok: {name}")?; } Ok(Err(e)) => { errors = true; From cc6445f727d9cf50b7b596c6c075e746e95eb170 Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 23:09:08 -0400 Subject: [PATCH 3/4] lib: Route CLI status messages through cli_status!/cli_warn! Subcommand implementations such as install, upgrade, switch and rollback print status lines for a human at a terminal ("Installing bootloader via bootupd", "Installation complete!", ...). We want to deny direct println!/eprintln! in the library, but exempting each of these functions with #[expect] would also exempt everything else in them, and several are very large. Instead, put the one exemption on a pair of small helpers and use them at these call sites, so a new stray println!() anywhere else, including inside those functions, still trips the lint. Output is unchanged: cli_status!() writes a line to stdout and cli_warn!() to stderr, just as the calls they replace did. Prep for denying clippy::print_stdout in crates/lib, see https://github.com/bootc-dev/bootc/pull/2449#discussion_r4072799684 Generated-by: AI --- crates/lib/src/bootc_composefs/rollback.rs | 6 +-- crates/lib/src/bootc_composefs/soft_reboot.rs | 8 ++-- crates/lib/src/bootc_composefs/state.rs | 6 +-- crates/lib/src/bootc_composefs/switch.rs | 4 +- crates/lib/src/bootc_composefs/update.rs | 10 ++--- crates/lib/src/bootloader.rs | 8 ++-- crates/lib/src/boundimage.rs | 2 +- crates/lib/src/cli.rs | 30 +++++++------- crates/lib/src/cli_output.rs | 40 +++++++++++++++++++ crates/lib/src/deploy.rs | 26 ++++++------ crates/lib/src/image.rs | 8 ++-- crates/lib/src/install.rs | 20 +++++----- crates/lib/src/install/baseline.rs | 16 ++++---- crates/lib/src/install/completion.rs | 2 +- crates/lib/src/install/config.rs | 2 +- crates/lib/src/install/osconfig.rs | 2 +- crates/lib/src/lib.rs | 4 ++ crates/lib/src/progress_jsonl.rs | 4 +- crates/lib/src/task.rs | 4 +- 19 files changed, 124 insertions(+), 78 deletions(-) create mode 100644 crates/lib/src/cli_output.rs diff --git a/crates/lib/src/bootc_composefs/rollback.rs b/crates/lib/src/bootc_composefs/rollback.rs index 307545463d..a14c23e5fa 100644 --- a/crates/lib/src/bootc_composefs/rollback.rs +++ b/crates/lib/src/bootc_composefs/rollback.rs @@ -237,7 +237,7 @@ pub(crate) async fn composefs_rollback( let reverting = new_spec.boot_order == BootOrder::Default; if reverting { - println!("notice: Reverting queued rollback state"); + cli_status!("notice: Reverting queued rollback state"); } let rollback_status = host @@ -269,9 +269,9 @@ pub(crate) async fn composefs_rollback( } if reverting { - println!("Next boot: current deployment"); + cli_status!("Next boot: current deployment"); } else { - println!("Next boot: rollback deployment"); + cli_status!("Next boot: rollback deployment"); } Ok(()) diff --git a/crates/lib/src/bootc_composefs/soft_reboot.rs b/crates/lib/src/bootc_composefs/soft_reboot.rs index 393ae84f64..dae10c7fbd 100644 --- a/crates/lib/src/bootc_composefs/soft_reboot.rs +++ b/crates/lib/src/bootc_composefs/soft_reboot.rs @@ -40,7 +40,7 @@ pub(crate) fn reset_soft_reboot() -> Result<()> { let Some(nextroot) = nextroot else { tracing::debug!("Nextroot does not exist"); - println!("No deployment staged for soft rebooting"); + cli_status!("No deployment staged for soft rebooting"); return Ok(()); }; @@ -50,13 +50,13 @@ pub(crate) fn reset_soft_reboot() -> Result<()> { if !nextroot_mounted { tracing::debug!("Nextroot is not a mountpoint"); - println!("No deployment staged for soft rebooting"); + cli_status!("No deployment staged for soft rebooting"); return Ok(()); } unmount(NEXTROOT, UnmountFlags::DETACH).context("Unmounting nextroot")?; - println!("Cleared soft reboot queued state"); + cli_status!("Cleared soft reboot queued state"); Ok(()) } @@ -133,7 +133,7 @@ pub(crate) async fn prepare_soft_reboot_composefs( setup_root(args)?; - println!("Soft reboot setup complete"); + cli_status!("Soft reboot setup complete"); if reboot { // Replacing the current process should be fine as we restart userspace anyway diff --git a/crates/lib/src/bootc_composefs/state.rs b/crates/lib/src/bootc_composefs/state.rs index 9f15d31fca..6830515466 100644 --- a/crates/lib/src/bootc_composefs/state.rs +++ b/crates/lib/src/bootc_composefs/state.rs @@ -331,7 +331,7 @@ pub(crate) async fn write_composefs_state( pub(crate) fn composefs_usr_overlay(access_mode: FilesystemOverlayAccessMode) -> Result<()> { let status = get_composefs_usr_overlay_status()?; if status.is_some() { - println!("An overlayfs is already mounted on /usr"); + cli_status!("An overlayfs is already mounted on /usr"); return Ok(()); } @@ -345,8 +345,8 @@ pub(crate) fn composefs_usr_overlay(access_mode: FilesystemOverlayAccessMode) -> let overlay_fd = overlay_transient(usr.as_fd(), "transient", mount_attr_flags)?; mount_at_wrapper(overlay_fd, &usr, ".").context("Attaching /usr overlay")?; - println!("A {} overlayfs is now mounted on /usr", access_mode); - println!("All changes there will be discarded on reboot."); + cli_status!("A {} overlayfs is now mounted on /usr", access_mode); + cli_status!("All changes there will be discarded on reboot."); Ok(()) } diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 0cb0b9ebea..1d124159ad 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -49,7 +49,7 @@ pub(crate) async fn switch_composefs( }; if new_spec == host.spec { - println!("Image specification is unchanged."); + cli_status!("Image specification is unchanged."); if opts.apply && host.status.staged.is_some() { crate::reboot::reboot()?; } @@ -106,7 +106,7 @@ pub(crate) async fn switch_composefs( match action { UpdateAction::Skip => { - println!("No changes in image: {target_imgref:#}"); + cli_status!("No changes in image: {target_imgref:#}"); return Ok(()); } diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 45e4c5ad28..29165b5aed 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -423,8 +423,8 @@ pub(crate) async fn apply_upgrade_from_downloaded( // Staged deployment exists, but it will be finalized if !staged.download_only { - println!("Staged deployment is present and not in download only mode."); - println!("Use `bootc update --apply` to apply the update."); + cli_status!("Staged deployment is present and not in download only mode."); + cli_status!("Use `bootc update --apply` to apply the update."); return Ok(()); } @@ -541,7 +541,7 @@ pub(crate) async fn upgrade_composefs( return crate::reboot::reboot(); } - println!("Update already staged. To apply update run `bootc update --apply`"); + cli_status!("Update already staged. To apply update run `bootc update --apply`"); return Ok(()); } @@ -566,7 +566,7 @@ pub(crate) async fn upgrade_composefs( match action { UpdateAction::Skip => { - println!("No changes in staged image: {booted_imgref:#}"); + cli_status!("No changes in staged image: {booted_imgref:#}"); return Ok(()); } @@ -598,7 +598,7 @@ pub(crate) async fn upgrade_composefs( match action { UpdateAction::Skip => { - println!("No changes in: {booted_imgref:#}"); + cli_status!("No changes in: {booted_imgref:#}"); return Ok(()); } diff --git a/crates/lib/src/bootloader.rs b/crates/lib/src/bootloader.rs index 8b44c27b18..8f924360be 100644 --- a/crates/lib/src/bootloader.rs +++ b/crates/lib/src/bootloader.rs @@ -167,7 +167,7 @@ pub(crate) fn install_via_bootupd( "/" }; - println!("Installing bootloader via bootupd"); + cli_status!("Installing bootloader via bootupd"); // Build the bootupctl arguments let mut bootupd_args: Vec<&str> = vec!["backend", "install"]; @@ -269,7 +269,7 @@ pub(crate) fn install_systemd_boot( configopts: &crate::install::InstallConfigOpts, autoenroll: Option, ) -> Result<()> { - println!("Installing bootloader via systemd-boot"); + cli_status!("Installing bootloader via systemd-boot"); // We use the --root of the mounted target root, so we have the right /etc/os-release. let root_path = prepared_root @@ -347,7 +347,7 @@ pub(crate) fn install_systemd_boot( } dir.copy(filename, &keys_dir, filename) .with_context(|| format!("Copying secure boot key {filename:?}"))?; - println!( + cli_status!( "Wrote Secure Boot key: {}/{}", keys_path.display(), filename.as_str() @@ -452,7 +452,7 @@ pub(crate) fn install_via_zipl(device: &bootc_blockdev::Device, boot_uuid: &str) let ramdisk = boot_dir.join(initrd).canonicalize_utf8()?; // Execute the zipl command to install bootloader - println!("Running zipl on {device_path}"); + cli_status!("Running zipl on {device_path}"); Command::new("zipl") .args(["--target", boot_dir.as_str()]) .args(["--image", image.as_str()]) diff --git a/crates/lib/src/boundimage.rs b/crates/lib/src/boundimage.rs index 31b4e5fb3c..44999f4df1 100644 --- a/crates/lib/src/boundimage.rs +++ b/crates/lib/src/boundimage.rs @@ -196,7 +196,7 @@ pub(crate) async fn pull_images_impl( .await?; } - println!("Bound images stored: {n}"); + cli_status!("Bound images stored: {n}"); Ok(()) } diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 00dc677564..9f952835a7 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1232,7 +1232,7 @@ fn handle_staged_soft_reboot( /// Perform a soft reboot for a staged deployment #[context("Soft reboot staged deployment")] fn soft_reboot_staged(sysroot: &SysrootLock) -> Result<()> { - println!("Staged deployment is soft-reboot capable, preparing for soft-reboot..."); + cli_status!("Staged deployment is soft-reboot capable, preparing for soft-reboot..."); let deployments_list = sysroot.deployments(); let staged_deployment = deployments_list @@ -1247,7 +1247,7 @@ fn soft_reboot_staged(sysroot: &SysrootLock) -> Result<()> { /// Perform a soft reboot for a rollback deployment #[context("Soft reboot rollback deployment")] fn soft_reboot_rollback(booted_ostree: &BootedOstree<'_>) -> Result<()> { - println!("Rollback deployment is soft-reboot capable, preparing for soft-reboot..."); + cli_status!("Rollback deployment is soft-reboot capable, preparing for soft-reboot..."); let deployments_list = booted_ostree.sysroot.deployments(); let target_deployment = deployments_list @@ -1304,9 +1304,9 @@ async fn apply_from_downloaded_ostree( if staged_deployment.is_finalization_locked() { crate::boundimage::pull_bound_images(storage, &staged_deployment).await?; ostree.change_finalization(&staged_deployment)?; - println!("Staged deployment will now be applied on reboot"); + cli_status!("Staged deployment will now be applied on reboot"); } else { - println!("Staged deployment is already set to apply on reboot"); + cli_status!("Staged deployment is already set to apply on reboot"); } handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &host)?; @@ -1395,15 +1395,15 @@ async fn upgrade( .await?; match imp.prepare().await? { PrepareResult::AlreadyPresent(_) => { - println!("No changes in: {ostree_imgref:#}"); + cli_status!("No changes in: {ostree_imgref:#}"); } PrepareResult::Ready(r) => { crate::deploy::check_bootc_label(&r.config); - println!("Update available for: {ostree_imgref:#}"); + cli_status!("Update available for: {ostree_imgref:#}"); if let Some(version) = r.version() { - println!(" Version: {version}"); + cli_status!(" Version: {version}"); } - println!(" Digest: {}", r.manifest_digest); + cli_status!(" Digest: {}", r.manifest_digest); changed = true; if let Some(previous_image) = booted_image.as_ref() { let diff = @@ -1457,7 +1457,7 @@ async fn upgrade( // --download-only: set download-only mode if !staged.is_finalization_locked() { storage.get_ostree()?.change_finalization(&staged)?; - println!("Image downloaded, but will not be applied on reboot"); + cli_status!("Image downloaded, but will not be applied on reboot"); download_only_changed = true; } } else if !opts.check { @@ -1465,7 +1465,7 @@ async fn upgrade( // (skip if --check, which is read-only) if staged.is_finalization_locked() { storage.get_ostree()?.change_finalization(&staged)?; - println!("Staged deployment will now be applied on reboot"); + cli_status!("Staged deployment will now be applied on reboot"); download_only_changed = true; } } @@ -1474,7 +1474,7 @@ async fn upgrade( } if !download_only_changed { - println!("Staged update present, not changed"); + cli_status!("Staged update present, not changed"); } handle_staged_soft_reboot(booted_ostree, opts.soft_reboot, &host)?; @@ -1482,7 +1482,7 @@ async fn upgrade( crate::reboot::reboot()?; } } else if booted_unchanged { - println!("No update available.") + cli_status!("No update available.") } else { let stateroot = booted_ostree.stateroot(); let from = MergeState::from_stateroot(storage, &stateroot)?; @@ -1578,7 +1578,7 @@ async fn switch_ostree( }; if new_spec == host.spec { - println!("Image specification is unchanged."); + cli_status!("Image specification is unchanged."); if opts.apply && host.status.staged.is_some() { crate::reboot::reboot()?; } @@ -1700,7 +1700,7 @@ async fn switch(opts: SwitchOpts) -> Result<()> { }) .await?? }; - println!("Updated {deployid} to pull from {target}"); + cli_status!("Updated {deployid} to pull from {target}"); return Ok(()); } let storage = &get_storage().await?; @@ -1772,7 +1772,7 @@ async fn edit_ostree( }; if new_host.spec == host.spec { - println!("Edit cancelled, no changes made."); + cli_status!("Edit cancelled, no changes made."); return Ok(()); } host.spec.verify_transition(&new_host.spec)?; diff --git a/crates/lib/src/cli_output.rs b/crates/lib/src/cli_output.rs new file mode 100644 index 0000000000..746aecf625 --- /dev/null +++ b/crates/lib/src/cli_output.rs @@ -0,0 +1,40 @@ +//! Human-facing status output for CLI subcommands. +//! +//! Subcommand implementations (install, upgrade, switch, ...) print +//! progress and status lines meant for a person at a terminal. Route those +//! through `cli_status!` and `cli_warn!` rather than calling +//! `println!`/`eprintln!` directly, so that direct printing elsewhere in the +//! library stands out. +//! +//! Output that is the actual result of a command (JSON, digests, tables) +//! should instead be written to an explicit `impl Write`, and diagnostics +//! should use `tracing`. + +use std::fmt::Arguments; + +/// Print a human-readable status line to stdout. +macro_rules! cli_status { + ($($arg:tt)*) => { + $crate::cli_output::status(format_args!($($arg)*)) + }; +} + +/// Print a human-readable warning line to stderr, regardless of the +/// `tracing` log level. +macro_rules! cli_warn { + ($($arg:tt)*) => { + $crate::cli_output::warn(format_args!($($arg)*)) + }; +} + +/// Implementation of `cli_status!`. +#[expect(clippy::print_stdout, reason = "this is the CLI status output helper")] +pub(crate) fn status(args: Arguments<'_>) { + println!("{args}"); +} + +/// Implementation of `cli_warn!`. +#[expect(clippy::print_stderr, reason = "this is the CLI warning output helper")] +pub(crate) fn warn(args: Arguments<'_>) { + eprintln!("{args}"); +} diff --git a/crates/lib/src/deploy.rs b/crates/lib/src/deploy.rs index 4ef2b471ab..a07de58809 100644 --- a/crates/lib/src/deploy.rs +++ b/crates/lib/src/deploy.rs @@ -561,7 +561,7 @@ pub(crate) async fn prepare_for_pull( } let prep = match imp.prepare().await? { PrepareResult::AlreadyPresent(c) => { - println!("No changes in {imgref:#} => {}", c.manifest_digest); + cli_status!("No changes in {imgref:#} => {}", c.manifest_digest); return Ok(PreparedPullResult::AlreadyPresent(Box::new((*c).into()))); } PrepareResult::Ready(p) => p, @@ -658,7 +658,7 @@ pub(crate) async fn prepare_for_pull_unified( } let prep = match imp.prepare().await? { PrepareResult::AlreadyPresent(c) => { - println!("No changes in {imgref:#} => {}", c.manifest_digest); + cli_status!("No changes in {imgref:#} => {}", c.manifest_digest); return Ok(PreparedPullResult::AlreadyPresent(Box::new((*c).into()))); } PrepareResult::Ready(p) => p, @@ -993,9 +993,11 @@ pub(crate) async fn cleanup(sysroot: &Storage) -> Result<()> { ostree_container::deploy::prune(locked_sysroot).context("Pruning images")?; if !pruned.is_empty() { let size = glib::format_size(pruned.objsize); - println!( + cli_status!( "Pruned images: {} (layers: {}, objsize: {})", - pruned.n_images, pruned.n_layers, size + pruned.n_images, + pruned.n_layers, + size ); } else { tracing::debug!("Nothing to prune"); @@ -1276,15 +1278,15 @@ pub(crate) async fn stage( crate::deploy::cleanup(sysroot).await?; if !lock_finalization { - println!("Queued for next boot: {:#}", spec.image); + cli_status!("Queued for next boot: {:#}", spec.image); } else { - println!("Staged but not queued for next boot: {:#}", spec.image); + cli_status!("Staged but not queued for next boot: {:#}", spec.image); } if let Some(version) = image.version.as_deref() { - println!(" Version: {version}"); + cli_status!(" Version: {version}"); } - println!(" Digest: {}", image.manifest_digest); + cli_status!(" Digest: {}", image.manifest_digest); subtask.completed = true; subtasks.push(subtask.clone()); @@ -1341,7 +1343,7 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> { let reverting = new_spec.boot_order == BootOrder::Default; if reverting { - println!("notice: Reverting queued rollback state"); + cli_status!("notice: Reverting queued rollback state"); } let rollback_status = host .status @@ -1386,9 +1388,9 @@ pub(crate) async fn rollback(sysroot: &Storage) -> Result<()> { .sysroot .write_deployments(&new_deployments, gio::Cancellable::NONE)?; if reverting { - println!("Next boot: current deployment"); + cli_status!("Next boot: current deployment"); } else { - println!("Next boot: rollback deployment"); + cli_status!("Next boot: rollback deployment"); } write_reboot_required(rollback_image.manifest_digest.as_ref())?; @@ -1550,7 +1552,7 @@ pub(crate) fn fixup_etc_fstab(root: &Dir) -> Result<()> { }) .context("Replacing /etc/fstab")?; - println!("Updated /etc/fstab to add `ro` for `/`"); + cli_status!("Updated /etc/fstab to add `ro` for `/`"); Ok(()) } diff --git a/crates/lib/src/image.rs b/crates/lib/src/image.rs index 8fdaa7af4a..29e95fb42c 100644 --- a/crates/lib/src/image.rs +++ b/crates/lib/src/image.rs @@ -250,10 +250,10 @@ pub(crate) async fn push_entrypoint( let mut opts = ostree_ext::container::store::ExportToOCIOpts::default(); opts.progress_to_stdout = true; - println!("Copying local image {source} to {target} ..."); + cli_status!("Copying local image {source} to {target} ..."); let r = ostree_ext::container::store::export(repo, &source, &target, Some(opts)).await?; - println!("Pushed: {target} {r}"); + cli_status!("Pushed: {target} {r}"); Ok(()) } @@ -323,7 +323,7 @@ async fn set_unified_composefs( // Check if the image is already in bootc storage let img_transport = imgref.to_transport_image()?; if imgstore.exists(&img_transport).await? { - println!("Image {} is already in bootc storage.", imgref.image); + cli_status!("Image {} is already in bootc storage.", imgref.image); tracing::info!( message_id = SET_UNIFIED_CFS_JOURNAL_ID, bootc.status = "already_unified", @@ -373,7 +373,7 @@ async fn set_unified_composefs( bootc.status = "set_unified_complete", "Unified storage set. Future upgrade/switch will use zero-copy path automatically.", ); - println!("Unified storage enabled for {}.", imgref.image); + cli_status!("Unified storage enabled for {}.", imgref.image); Ok(()) } diff --git a/crates/lib/src/install.rs b/crates/lib/src/install.rs index 38866768f6..682cc6f7d0 100644 --- a/crates/lib/src/install.rs +++ b/crates/lib/src/install.rs @@ -944,7 +944,7 @@ async fn initialize_ostree_root(state: &State, root_setup: &RootSetup) -> Result .cwd(rootfs_dir)? .run()?; } else { - println!("Reusing extant ostree layout"); + cli_status!("Reusing extant ostree layout"); let path = ".".into(); let _ = crate::utils::open_dir_remount_rw(rootfs_dir, path) @@ -1445,7 +1445,7 @@ pub(crate) fn reexecute_self_for_selinux_if_needed( let host_selinux = crate::lsm::host_selinux_enabled()?; tracing::debug!("Target has SELinux, host={host_selinux}"); let r = if override_disable_selinux { - println!("notice: Target has SELinux enabled, overriding to disable"); + cli_status!("notice: Target has SELinux enabled, overriding to disable"); SELinuxFinalState::ForceTargetDisabled } else if host_selinux { // /sys/fs/selinuxfs is not normally mounted, so we do that now. @@ -1764,9 +1764,9 @@ async fn prepare_install( let selinux_state = reexecute_self_for_selinux_if_needed(&source, config_opts.disable_selinux)?; tracing::debug!("SELinux state: {selinux_state:?}"); - println!("Installing image: {:#}", &target_imgref); + cli_status!("Installing image: {:#}", &target_imgref); if let Some(digest) = source.digest.as_deref() { - println!("Digest: {digest}"); + cli_status!("Digest: {digest}"); } let root_filesystem = target_fs @@ -1891,7 +1891,7 @@ impl PostFetchState { } } }; - println!("Bootloader: {detected_bootloader}"); + cli_status!("Bootloader: {detected_bootloader}"); let r = Self { detected_bootloader, }; @@ -2204,7 +2204,7 @@ async fn install_to_filesystem_impl( } fn installation_complete() { - println!("Installation complete!"); + cli_status!("Installation complete!"); } /// Implementation of the `bootc install to-disk` CLI command. @@ -2512,12 +2512,12 @@ fn warn_on_host_root(rootfs_fd: &Dir) -> Result<()> { } let dashes = "----------------------------"; let timeout = Duration::from_secs(DELAY_SECONDS); - eprintln!("{dashes}"); + cli_warn!("{dashes}"); crate::utils::medium_visibility_warning( "WARNING: This operation will OVERWRITE THE BOOTED HOST ROOT FILESYSTEM and is NOT REVERSIBLE.", ); - eprintln!("Waiting {timeout:?} to continue; interrupt (Control-C) to cancel."); - eprintln!("{dashes}"); + cli_warn!("Waiting {timeout:?} to continue; interrupt (Control-C) to cancel."); + cli_warn!("{dashes}"); let bar = indicatif::ProgressBar::new_spinner(); bar.enable_steady_tick(Duration::from_millis(100)); @@ -2657,7 +2657,7 @@ pub(crate) async fn install_to_filesystem( match fsopts.replace { Some(ReplaceMode::Wipe) => { let rootfs_fd = rootfs_fd.try_clone()?; - println!("Wiping contents of root"); + cli_status!("Wiping contents of root"); tokio::task::spawn_blocking(move || remove_all_in_dir_no_xdev(&rootfs_fd, true)) .await??; } diff --git a/crates/lib/src/install/baseline.rs b/crates/lib/src/install/baseline.rs index 0db968b20d..aacecc21e8 100644 --- a/crates/lib/src/install/baseline.rs +++ b/crates/lib/src/install/baseline.rs @@ -168,7 +168,7 @@ fn mkfs<'a>( } pub(crate) fn wipefs(dev: &Utf8Path) -> Result<()> { - println!("Wiping device {dev}"); + cli_status!("Wiping device {dev}"); Command::new("wipefs") .args(["-a", dev.as_str()]) .run_inherited_with_cmd_context() @@ -219,10 +219,10 @@ pub(crate) fn install_create_rootfs( let dev = &opts.device; for child in device.children.iter().flatten() { let child = child.path(); - println!("Wiping {child}"); + cli_status!("Wiping {child}"); wipefs(Utf8Path::new(&child))?; } - println!("Wiping {dev}"); + cli_status!("Wiping {dev}"); wipefs(dev)?; } else if device.has_children() { anyhow::bail!( @@ -252,11 +252,11 @@ pub(crate) fn install_create_rootfs( let serial = device.serial.as_deref().unwrap_or(""); let model = device.model.as_deref().unwrap_or(""); let discoverable = use_discoverable_partitions(state); - println!("Block setup: {block_setup}"); - println!(" Size: {}", device.size); - println!(" Serial: {serial}"); - println!(" Model: {model}"); - println!( + cli_status!("Block setup: {block_setup}"); + cli_status!(" Size: {}", device.size); + cli_status!(" Serial: {serial}"); + cli_status!(" Model: {model}"); + cli_status!( " Partitions: {}", if discoverable { "Discoverable" } else { "UUID" } ); diff --git a/crates/lib/src/install/completion.rs b/crates/lib/src/install/completion.rs index a999f25035..c13742387c 100644 --- a/crates/lib/src/install/completion.rs +++ b/crates/lib/src/install/completion.rs @@ -221,7 +221,7 @@ pub(crate) async fn run_from_anaconda(rootfs: &Dir) -> Result<()> { .try_exists(RUN_BOOTC_INSTALL_RECONCILED) .context("Querying reconciliation")? { - println!("Reconciliation already completed."); + cli_status!("Reconciliation already completed."); return Ok(()); } diff --git a/crates/lib/src/install/config.rs b/crates/lib/src/install/config.rs index 7f123e45f2..4318fbba77 100644 --- a/crates/lib/src/install/config.rs +++ b/crates/lib/src/install/config.rs @@ -305,7 +305,7 @@ pub(crate) fn load_config() -> Result> { }) .with_context(|| format!("Parsing {path:?}"))?; for key in unused { - eprintln!("warning: {path:?}: Unknown key {key}"); + cli_warn!("warning: {path:?}: Unknown key {key}"); } if let Some(config) = config.as_mut() { if let Some(install) = c.install { diff --git a/crates/lib/src/install/osconfig.rs b/crates/lib/src/install/osconfig.rs index 6c06f9e26a..791d29938c 100644 --- a/crates/lib/src/install/osconfig.rs +++ b/crates/lib/src/install/osconfig.rs @@ -46,7 +46,7 @@ pub(crate) fn inject_root_ssh_authorized_keys( |w| w.write_all(tmpfiles_content.as_bytes()).map_err(Into::into), )?; - println!("Injected: {ETC_TMPFILES}/{ROOT_SSH_TMPFILE}"); + cli_status!("Injected: {ETC_TMPFILES}/{ROOT_SSH_TMPFILE}"); Ok(()) } diff --git a/crates/lib/src/lib.rs b/crates/lib/src/lib.rs index d9eccc0c81..70288a64aa 100644 --- a/crates/lib/src/lib.rs +++ b/crates/lib/src/lib.rs @@ -63,6 +63,10 @@ //! - [`linux-kernel-cmdline`](../linux_kernel_cmdline/index.html) - Cmdline parsing //! - [`etc-merge`](../etc_merge/index.html) - `/etc` three-way merge +// Declared first so its macros are in scope for all other modules. +#[macro_use] +mod cli_output; + mod bootc_composefs; pub(crate) mod bootc_kargs; mod bootloader; diff --git a/crates/lib/src/progress_jsonl.rs b/crates/lib/src/progress_jsonl.rs index 4b9341d812..4b60f84531 100644 --- a/crates/lib/src/progress_jsonl.rs +++ b/crates/lib/src/progress_jsonl.rs @@ -248,7 +248,7 @@ impl ProgressWriter { /// Send an event. pub(crate) async fn send(&self, event: Event<'_>) { if let Err(e) = self.send_impl(event, true).await { - eprintln!("Failed to write to jsonl: {e}"); + cli_warn!("Failed to write to jsonl: {e}"); // Stop writing to fd but let process continue // SAFETY: Propagating panics from the mutex here is intentional let _ = self.inner.lock().await.take(); @@ -258,7 +258,7 @@ impl ProgressWriter { /// Send an event that can be dropped. pub(crate) async fn send_lossy(&self, event: Event<'_>) { if let Err(e) = self.send_impl(event, false).await { - eprintln!("Failed to write to jsonl: {e}"); + cli_warn!("Failed to write to jsonl: {e}"); // Stop writing to fd but let process continue // SAFETY: Propagating panics from the mutex here is intentional let _ = self.inner.lock().await.take(); diff --git a/crates/lib/src/task.rs b/crates/lib/src/task.rs index 8af8196f7e..9572c189b6 100644 --- a/crates/lib/src/task.rs +++ b/crates/lib/src/task.rs @@ -97,11 +97,11 @@ impl Task { match self.verbosity { CmdVerbosity::Quiet => {} CmdVerbosity::Description => { - println!("{}", self.description); + cli_status!("{}", self.description); } CmdVerbosity::Verbose => { // Output the description first - println!("{}", self.description); + cli_status!("{}", self.description); // Lock stdout so we buffer let mut stdout = std::io::stdout().lock(); From 83201bd287b89226d56c756826994f7b02f89229 Mon Sep 17 00:00:00 2001 From: cgwalters-bot Date: Tue, 22 Sep 2026 21:37:23 -0400 Subject: [PATCH 4/4] lib: Deny clippy::print_stdout and print_stderr Library code writing directly to stdout/stderr is easy to add and hard to notice in review, and it corrupts output for callers that expect something machine-readable (or that want progress routed through ProgressWriter). This came up in https://github.com/bootc-dev/bootc/pull/2449#discussion_r4072799684 where a new println!() slipped into the composefs pull path. Deny both lints at the crate root so new call sites fail clippy. They are restriction lints, so the -A clippy::all in the Makefile validate target doesn't touch them and CI enforces the deny as is. Human-facing status output already goes through cli_status!() and cli_warn!(). The few remaining direct prints get a targeted #[expect] with a reason: global_init() runs before tracing is set up, journal_send() reports that the journal itself is failing, and the spinner has a plain-text fallback when there is no tty. Using expect rather than allow means the attribute gets flagged once it is no longer needed. Tests are exempted via allow-print-in-tests in clippy.toml. Generated-by: AI --- clippy.toml | 3 +++ crates/lib/src/cli.rs | 1 + crates/lib/src/journal.rs | 4 ++++ crates/lib/src/lib.rs | 6 ++++++ crates/lib/src/utils.rs | 4 ++++ 5 files changed, 18 insertions(+) diff --git a/clippy.toml b/clippy.toml index c4b8878697..43b38df21f 100644 --- a/clippy.toml +++ b/clippy.toml @@ -2,3 +2,6 @@ disallowed-methods = [ # https://github.com/rust-lang/rust-clippy/issues/13434#issuecomment-2484292914 { path = "str::len", reason = "use .as_bytes().len() instead" } ] + +# Printing from tests is fine; see the print_stdout deny in crates/lib +allow-print-in-tests = true diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 9f952835a7..a8422be807 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -1876,6 +1876,7 @@ fn join_host_ipc_namespace() -> Result<()> { /// Perform process global initialization. This should be called as early as possible /// in the standard `main` function. #[allow(unsafe_code)] +#[expect(clippy::print_stderr, reason = "runs before tracing is initialized")] pub fn global_init() -> Result<()> { join_host_ipc_namespace()?; // In some cases we re-exec with a temporary binary, diff --git a/crates/lib/src/journal.rs b/crates/lib/src/journal.rs index bed50ec62f..11271b522b 100644 --- a/crates/lib/src/journal.rs +++ b/crates/lib/src/journal.rs @@ -9,6 +9,10 @@ static EMITTED_JOURNAL_ERROR: AtomicBool = AtomicBool::new(false); /// Wrapper for structured logging which is an explicit no-op /// when systemd is not in use (e.g. in a container). +#[expect( + clippy::print_stderr, + reason = "the journal is failing, and tracing to stderr is filtered by default" +)] pub(crate) fn journal_send( priority: libsystemd::logging::Priority, msg: &str, diff --git a/crates/lib/src/lib.rs b/crates/lib/src/lib.rs index 70288a64aa..2624bb20af 100644 --- a/crates/lib/src/lib.rs +++ b/crates/lib/src/lib.rs @@ -1,6 +1,12 @@ // The internals docs are built with --document-private-items, so allow // linking to private items from module documentation. #![allow(rustdoc::private_intra_doc_links)] +// Library code should not write to stdout/stderr behind the caller's back: +// that can corrupt machine-readable output. Use `tracing` for diagnostics, +// `ProgressWriter` for progress, or write to an explicit `impl Write`. +// Human-facing status lines from CLI subcommands go through the +// `cli_status!`/`cli_warn!` macros in `cli_output`. +#![deny(clippy::print_stdout, clippy::print_stderr)] //! # Bootable container tool //! diff --git a/crates/lib/src/utils.rs b/crates/lib/src/utils.rs index 544415e0f0..7dbfd5af04 100644 --- a/crates/lib/src/utils.rs +++ b/crates/lib/src/utils.rs @@ -181,6 +181,10 @@ pub(crate) fn medium_visibility_warning(s: &str) { /// with an automatic spinner to show that we're not blocked. /// Note that generally the called function should not output /// anything to stderr as this will interfere with the spinner. +#[expect( + clippy::print_stderr, + reason = "fallback when the progress bar is hidden (no tty)" +)] pub(crate) async fn async_task_with_spinner(msg: &str, f: F) -> T where F: Future,