From f4966b5ef9ad2f74a844a0e624921e6f492007ba Mon Sep 17 00:00:00 2001 From: Nando Vieira Date: Fri, 18 Sep 2026 11:41:33 -0700 Subject: [PATCH] Stream image-pull progress for container builds. --- .../src/commands/container/shared.rs | 59 ++++++++++++++++++- .../src/commands/contract/build/container.rs | 30 ++-------- 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/cmd/soroban-cli/src/commands/container/shared.rs b/cmd/soroban-cli/src/commands/container/shared.rs index 3bbd073918..cecdbb2117 100644 --- a/cmd/soroban-cli/src/commands/container/shared.rs +++ b/cmd/soroban-cli/src/commands/container/shared.rs @@ -1,6 +1,8 @@ use core::fmt; +use std::process::Stdio; use clap::ValueEnum; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, BufReader}; use tokio::process::Command; use crate::print::Print; @@ -20,6 +22,9 @@ pub enum Error { program: String, source: std::io::Error, }, + + #[error("could not pull image {image}: {stderr}")] + PullImageFailed { image: String, stderr: String }, } /// Container runtime to shell out to. @@ -261,6 +266,58 @@ impl Args { }; cmd } + + /// Pull `image`, streaming the engine's high-level status lines ("Pulling + /// from", "Digest", "Status") through `print`. Per-layer progress written to + /// stderr is captured rather than shown and surfaced only when the pull + /// fails, as `PullImageFailed`, so a failed pull can report the engine's own + /// error. A missing engine binary surfaces via `io_error` as `NotFound`. + pub(crate) async fn pull_image(&self, image: &str, print: &Print) -> Result<(), Error> { + let mut child = self + .pull_command(image) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| self.io_error(e))?; + + let stdout = child.stdout.take(); + let stderr = child.stderr.take(); + + let stream_stdout = async { + if let Some(stdout) = stdout { + let mut lines = BufReader::new(stdout).lines(); + while let Ok(Some(line)) = lines.next_line().await { + if line.contains("Pulling from") + || line.contains("Digest") + || line.contains("Status") + { + print.infoln(line); + } + } + } + }; + + let capture_stderr = async { + let mut buf = String::new(); + if let Some(mut stderr) = stderr { + let _ = stderr.read_to_string(&mut buf).await; + } + buf + }; + + // Drain both pipes concurrently so a full stderr buffer can't deadlock + // the child while we're reading stdout. + let ((), stderr) = tokio::join!(stream_stdout, capture_stderr); + + if child.wait().await.map_err(|e| self.io_error(e))?.success() { + Ok(()) + } else { + Err(Error::PullImageFailed { + image: image.to_string(), + stderr: stderr.trim().to_string(), + }) + } + } } /// Resource limits for commands that *run* a container (e.g. `container start`). @@ -498,7 +555,7 @@ mod test { let not_found = std::io::Error::from(std::io::ErrorKind::NotFound); match args(None, Some(Engine::AppleContainer)).io_error(not_found) { Error::NotFound { program, .. } => assert_eq!(program, "container"), - Error::Command { .. } => panic!("expected NotFound, got Command"), + other => panic!("expected NotFound, got {other:?}"), } } diff --git a/cmd/soroban-cli/src/commands/contract/build/container.rs b/cmd/soroban-cli/src/commands/contract/build/container.rs index c6299e9985..50526e52ce 100644 --- a/cmd/soroban-cli/src/commands/contract/build/container.rs +++ b/cmd/soroban-cli/src/commands/contract/build/container.rs @@ -45,9 +45,6 @@ pub enum Error { #[error(transparent)] Engine(#[from] shared::Error), - #[error("could not pull image {image}")] - PullImageFailed { image: String }, - #[error( "could not determine the image's default Rust toolchain via `rustup default`; \ the image must provide rustup so the build toolchain can be pinned" @@ -95,7 +92,10 @@ pub async fn run( // `pull` up front to refresh a moving tag to its newest image. Nothing is // pulled when only printing the command, since nothing runs. if !print_only && cmd.pull { - pull_image(&docker, image, print).await?; + docker + .pull_image(image, print) + .await + .map_err(Error::Engine)?; } // Gather everything we need to know about the image in one throwaway @@ -318,28 +318,6 @@ fn forwarded_build_args( args } -async fn pull_image(docker: &shared::Args, image: &str, print: &Print) -> Result<(), Error> { - print.infoln(format!("Pulling image {image}")); - let (stdout, stderr) = if print.quiet { - (Stdio::null(), Stdio::null()) - } else { - (Stdio::inherit(), Stdio::inherit()) - }; - let status = docker - .pull_command(image) - .stdout(stdout) - .stderr(stderr) - .status() - .await - .map_err(|e| docker.io_error(e))?; - if !status.success() { - return Err(Error::PullImageFailed { - image: image.to_string(), - }); - } - Ok(()) -} - /// Run `cmd` in a throwaway `docker run --rm` container (optionally overriding /// the entrypoint) and return its captured stdout. stderr and the exit status /// are ignored — every probe treats a missing subcommand or unexpected output as