Use bullet stream's built in stream_cmd and time_cmd for consistent output (#393)

* Update bullet_stream and dependencies

* Fix compilation

Error now returns W so both arms have to match return type of `()`

* Use consistent interface

* Use consistent interface

* Use consistent interface

* Use consistent interface

* Use consistent interface

* Use consistent interface

Author: schneems

Cargo.lock

@@ -3,6 +3,8 @@ resolver = "2"
 members = ["buildpacks/ruby", "commons"]
 
 [workspace.dependencies]
+bullet_stream = { version = ">=0.7,<1.0", features = ["fun_run"] }
+fun_run = { version = ">=0.5,<1.0", features = ["which_problem"] }
 cache_diff = { version = "1.1", features = ["bullet_stream"] }
 
 [workspace.package]
@@ -20,3 +22,4 @@ panic_in_result_fn = "warn"
 pedantic = { level = "warn", priority = -1 }
 unwrap_used = "warn"
 module_name_repetitions = "allow"
+

Cargo.toml

@@ -7,12 +7,13 @@ rust-version.workspace = true
 workspace = true
 
 [dependencies]
-bullet_stream = "0.3.0"
+bullet_stream.workspace = true
+fun_run.workspace = true
+cache_diff.workspace = true
 clap = { version = "4", default-features = false, features = ["derive", "error-context", "help", "std", "usage"] }
 commons = { path = "../../commons" }
 flate2 = { version = "1", default-features = false, features = ["zlib"] }
 fs-err = "3"
-fun_run = { version = "0.2", features = ["which_problem"] }
 glob = "0.3"
 indoc = "2"
 # libcnb has a much bigger impact on buildpack behaviour than any other dependencies,
@@ -30,7 +31,6 @@ ureq = { version = "2", default-features = false, features = ["tls"] }
 url = "2"
 magic_migrate = "1.0"
 toml = "0.8"
-cache_diff.workspace = true
 
 [dev-dependencies]
 libcnb-test = "=0.26.1"

buildpacks/ruby/Cargo.toml

@@ -1,7 +1,7 @@
-use bullet_stream::{state::SubBullet, style, Print};
+use bullet_stream::{state::SubBullet, Print};
 use commons::gem_version::GemVersion;
 use core::str::FromStr;
-use fun_run::{CmdError, CommandWithName};
+use fun_run::CmdError;
 use regex::Regex;
 use std::collections::HashMap;
 use std::ffi::OsStr;
@@ -22,7 +22,7 @@ pub(crate) struct GemList {
 ///
 /// Errors if the command `bundle list` is unsuccessful.
 pub(crate) fn bundle_list<T, K, V>(
-    bullet: Print<SubBullet<Stdout>>,
+    mut bullet: Print<SubBullet<Stdout>>,
     envs: T,
 ) -> Result<(Print<SubBullet<Stdout>>, GemList), CmdError>
 where
@@ -33,12 +33,11 @@ where
     let mut cmd = Command::new("bundle");
     cmd.arg("list").env_clear().envs(envs);
 
-    let timer = bullet.start_timer(format!("Running {}", style::command(cmd.name())));
-    let gem_list = cmd
-        .named_output()
+    let gem_list = bullet
+        .time_cmd(&mut cmd)
         .map(|output| output.stdout_lossy())
         .and_then(|output| GemList::from_str(&output))?;
-    Ok((timer.done(), gem_list))
+    Ok((bullet, gem_list))
 }
 
 /// Converts the output of `$ gem list` into a data structure that can be inspected and compared

buildpacks/ruby/src/gem_list.rs

@@ -7,7 +7,7 @@
 use crate::RubyBuildpack;
 use crate::RubyBuildpackError;
 use bullet_stream::state::SubBullet;
-use bullet_stream::{style, Print};
+use bullet_stream::Print;
 use cache_diff::CacheDiff;
 use commons::gemfile_lock::ResolvedBundlerVersion;
 use commons::layer::diff_migrate::DiffMigrateLayer;
@@ -80,7 +80,7 @@ pub(crate) struct MetadataV1 {
 }
 
 fn download_bundler(
-    bullet: Print<SubBullet<Stdout>>,
+    mut bullet: Print<SubBullet<Stdout>>,
     env: &Env,
     metadata: &Metadata,
     gem_path: &Path,
@@ -103,13 +103,12 @@ fn download_bundler(
         "--env-shebang", // Start the `bundle` executable with `#! /usr/bin/env ruby`
     ]);
 
-    let timer = bullet.start_timer(format!("Running {}", style::command(short_name)));
-
-    cmd.named_output()
+    bullet
+        .time_cmd(&mut cmd.named(short_name))
         .map_err(|error| fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned()))
         .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?;
 
-    Ok(timer.done())
+    Ok(bullet)
 }
 
 #[cfg(test)]

buildpacks/ruby/src/layers/bundle_download_layer.rs

@@ -90,12 +90,8 @@ pub(crate) fn handle(
             cmd.args(["install"])
                 .env_clear() // Current process env vars already merged into env
                 .envs(&env);
-            let mut cmd = cmd.named_fn(|cmd| display_name(cmd, &env));
             bullet
-                .stream_with(
-                    format!("Running {}", style::command(cmd.name())),
-                    |stdout, stderr| cmd.stream_output(stdout, stderr),
-                )
+                .stream_cmd(&mut cmd.named_fn(|cmd| display_name(cmd, &env)))
                 .map_err(|error| {
                     fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned())
                 })

buildpacks/ruby/src/layers/bundle_install_layer.rs

@@ -1,9 +1,6 @@
-use bullet_stream::{
-    state::SubBullet,
-    {style, Print},
-};
+use bullet_stream::{state::SubBullet, Print};
 use core::str::FromStr;
-use fun_run::{CmdError, CommandWithName};
+use fun_run::CmdError;
 use std::io::Stdout;
 use std::{ffi::OsStr, process::Command};
 
@@ -25,15 +22,14 @@ pub(crate) struct RakeDetect {
 ///
 /// Will return `Err` if `bundle exec rake -p` command cannot be invoked by the operating system.
 pub(crate) fn call<T: IntoIterator<Item = (K, V)>, K: AsRef<OsStr>, V: AsRef<OsStr>>(
-    bullet: Print<SubBullet<Stdout>>,
+    mut bullet: Print<SubBullet<Stdout>>,
     envs: T,
     error_on_failure: bool,
 ) -> Result<(Print<SubBullet<Stdout>>, RakeDetect), CmdError> {
     let mut cmd = Command::new("rake");
     cmd.args(["-P", "--trace"]).env_clear().envs(envs);
 
-    let timer = bullet.start_timer(format!("Running {}", style::command(cmd.name())));
-    let output = cmd.named_output().or_else(|error| {
+    let output = bullet.time_cmd(cmd).or_else(|error| {
         if error_on_failure {
             Err(error)
         } else {
@@ -45,7 +41,7 @@ pub(crate) fn call<T: IntoIterator<Item = (K, V)>, K: AsRef<OsStr>, V: AsRef<OsS
         }
     })?;
 
-    RakeDetect::from_str(&output.stdout_lossy()).map(|rake_detect| (timer.done(), rake_detect))
+    RakeDetect::from_str(&output.stdout_lossy()).map(|rake_detect| (bullet, rake_detect))
 }
 
 impl RakeDetect {

buildpacks/ruby/src/rake_task_detect.rs

@@ -4,7 +4,6 @@ use crate::RubyBuildpackError;
 use bullet_stream::state::SubBullet;
 use bullet_stream::{style, Print};
 use commons::cache::{mib, AppCache, CacheConfig, CacheError, CacheState, KeepPath, PathState};
-use fun_run::{self, CommandWithName};
 use libcnb::build::BuildContext;
 use libcnb::Env;
 use std::io::Stdout;
@@ -39,10 +38,7 @@ pub(crate) fn rake_assets_install(
                 .envs(env);
 
             bullet
-                .stream_with(
-                    format!("Running {}", style::command(cmd.name())),
-                    |stdout, stderr| cmd.stream_output(stdout, stderr),
-                )
+                .stream_cmd(&mut cmd)
                 .map_err(|error| {
                     fun_run::map_which_problem(error, &mut cmd, env.get("PATH").cloned())
                 })
@@ -85,10 +81,7 @@ pub(crate) fn rake_assets_install(
                 .envs(env);
 
             bullet
-                .stream_with(
-                    format!("Running {}", style::command(cmd.name())),
-                    |stdout, stderr| cmd.stream_output(stdout, stderr),
-                )
+                .stream_cmd(&mut cmd)
                 .map_err(|error| {
                     fun_run::map_which_problem(error, &mut cmd, env.get("PATH").cloned())
                 })

buildpacks/ruby/src/steps/rake_assets_install.rs

@@ -1,6 +1,6 @@
 use crate::{DetectError, RubyBuildpackError};
 use bullet_stream::{state::Bullet, state::SubBullet, style, Print};
-use fun_run::{CmdError, CommandWithName};
+use fun_run::CmdError;
 use indoc::formatdoc;
 use std::io::Stdout;
 use std::process::Command;
@@ -11,7 +11,7 @@ pub(crate) fn on_error(err: libcnb::Error<RubyBuildpackError>) {
     let debug_info = style::important(DEBUG_INFO_STR);
     match cause(err) {
         Cause::OurError(error) => log_our_error(output, error),
-        Cause::FrameworkError(error) =>
+        Cause::FrameworkError(error) => {
             output
             .bullet(&debug_info)
             .sub_bullet(error.to_string())
@@ -28,7 +28,8 @@ pub(crate) fn on_error(err: libcnb::Error<RubyBuildpackError>) {
                 If the issue persists, please try to reproduce the behavior locally using the `pack`
                 CLI. If you can reproduce the behavior locally and believe you've found a bug in the
                 buildpack or the framework please open an issue on the buildpack's GitHub repository.
-            "}),
+            "});
+        }
     };
 }
 
@@ -340,11 +341,7 @@ fn replace_app_path_with_relative(contents: impl AsRef<str>) -> String {
 }
 
 fn debug_cmd(mut log: Print<SubBullet<Stdout>>, command: &mut Command) -> Print<Bullet<Stdout>> {
-    let result = log.stream_with(
-        format!("Running debug command {}", style::command(command.name())),
-        |stdout, stderr| command.stream_output(stdout, stderr),
-    );
-    match result {
+    match log.stream_cmd(command) {
         Ok(_) => log.done(),
         Err(e) => log.sub_bullet(e.to_string()).done(),
     }

buildpacks/ruby/src/user_errors.rs

@@ -7,6 +7,7 @@ rust-version.workspace = true
 workspace = true
 
 [dependencies]
+cache_diff.workspace = true
 byte-unit = "5"
 # TODO: Consolidate on either the regex crate or the fancy-regex crate, since this repo currently uses both.
 fancy-regex = "0.14"
@@ -25,9 +26,8 @@ walkdir = "2"
 filetime = "0.2"
 magic_migrate = "1.0.1"
 toml = "0.8"
-cache_diff.workspace = true
 
 [dev-dependencies]
+bullet_stream.workspace = true
 filetime = "0.2"
 toml = "0.8"
-bullet_stream = "0.3.0"