Clean up

buildpacks/ruby/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Update build logging style (https://github.com/heroku/buildpacks-ruby/pull/198)
+
 ## [2.1.0] - 2023-09-26
 
 ### Added

buildpacks/ruby/src/layers/bundle_download_layer.rs

@@ -65,12 +65,12 @@ impl<'a> Layer for BundleDownloadLayer<'a> {
         let short_name = fun_run::display(&mut cmd);
 
         // Arguments we don't need in the output
-        cmd.mut_cmd().args([
+        cmd.args([
             "--install-dir", // Directory where bundler's contents will live
             &layer_path.to_string_lossy(),
             "--bindir", // Directory where `bundle` executable lives
             &bin_dir.to_string_lossy(),
-            "--force",
+            "--force",       // Overwrite if it already exists
             "--no-document", // Don't install ri or rdoc documentation, which takes extra time
             "--env-shebang", // Start the `bundle` executable with `#! /usr/bin/env ruby`
         ]);

buildpacks/ruby/src/layers/bundle_install_layer.rs

@@ -134,17 +134,16 @@ impl Layer for BundleInstallLayer<'_> {
                 bundle_install(&env).map_err(RubyBuildpackError::BundleInstallCommandError)?;
             }
             UpdateState::Skip(checked) => {
-                let checked = SentenceList::new(&checked).join_str("or");
                 let bundle_install = fmt::value("bundle install");
-                let env_var = fmt::value(format!("{HEROKU_SKIP_BUNDLE_DIGEST}=1"));
 
                 log_step(format!(
-                    "Skipping {bundle_install} {}",
-                    fmt::details(format!("no changes found in {checked}"))
+                    "Skipping {bundle_install} (no changes found in {sources})",
+                    sources = SentenceList::new(&checked).join_str("or")
                 ));
 
                 log_step(format!(
-                    "{HELP} To force run {bundle_install} set {env_var}",
+                    "{HELP} To force run {bundle_install} set {}",
+                    fmt::value(format!("{HEROKU_SKIP_BUNDLE_DIGEST}=1"))
                 ));
             }
         }

buildpacks/ruby/src/layers/metrics_agent_install.rs

@@ -118,7 +118,7 @@ impl<'a> Layer for MetricsAgentInstall<'a> {
         LayerResultBuilder::new(Metadata {
             download_url: Some(DOWNLOAD_URL.to_string()),
         })
-        .exec_d_program("spawn agentmon", execd)
+        .exec_d_program("spawn_metrics_agent", execd)
         .build()
     }
 

buildpacks/ruby/src/main.rs

@@ -115,7 +115,6 @@ impl Buildpack for RubyBuildpack {
         };
 
         // ## Install executable ruby version
-
         (logger, env) = {
             let section = logger.section(&format!(
                 "Ruby version {} from {}",

commons/Cargo.toml

@@ -28,6 +28,7 @@ thiserror = "1"
 walkdir = "2"
 which_problem = "0.1"
 ascii_table = { version = "4.0.2", features = ["color_codes"] }
+const_format = "0.2.31"
 
 [dev-dependencies]
 indoc = "2.0.1"

commons/src/output/fmt.rs

@@ -1,12 +1,13 @@
 use crate::output::util::LinesWithEndings;
+use const_format::formatcp;
 
 /// Helpers for formatting and colorizing your output
 
 /// Decorated str for prefixing "Help:"
-pub const HELP: &str = "\x1B[0;36m! Help:\x1B[0m"; // "{IMPORTANT_COLOR}! HELP{RESET}"
+pub const HELP: &str = formatcp!("{IMPORTANT_COLOR}! HELP{RESET}");
 
 /// Decorated str for prefixing "Debug info:"
-pub const DEBUG_INFO: &str = "\x1B[0;36mDebug info:\x1B[0m"; // "{IMPORTANT_COLOR}Debug info:{RESET}"
+pub const DEBUG_INFO: &str = formatcp!("{IMPORTANT_COLOR}Debug info{RESET}");
 
 /// Decorate a URL for the build output
 #[must_use]
@@ -175,21 +176,6 @@ pub(crate) fn prepend_each_line(
         })
         .collect::<String>();
     lines
-
-    // if body.trim().is_empty() {
-    //     format!("{prepend}\n")
-    // } else {
-    //     body.split('\n')
-    //         .map(|section| {
-    //             if section.trim().is_empty() {
-    //                 prepend.to_string()
-    //             } else {
-    //                 format!("{prepend}{separator}{section}")
-    //             }
-    //         })
-    //         .collect::<Vec<String>>()
-    //         .join("\n")
-    // }
 }
 
 /// Colorizes a body while preserving existing color/reset combinations and clearing before newlines

commons/src/output/interface.rs

@@ -1,6 +1,13 @@
 use std::fmt::Debug;
 use std::io::Write;
 
+/// Consuming stateful logger interface
+///
+/// The log pattern used by `BuildLog` is a consuming state machine that is designed to minimize
+/// the amount of mistakes that can result in malformed build output.
+///
+/// The interface isn't stable and may need to change.
+
 pub trait Logger: Debug {
     fn buildpack_name(self, s: &str) -> Box<dyn StartedLogger>;
     fn without_buildpack_name(self) -> Box<dyn StartedLogger>;

commons/src/output/util.rs

@@ -19,6 +19,32 @@ where
     arc: Arc<Mutex<W>>,
 }
 
+impl<W> Clone for ReadYourWrite<W>
+where
+    W: Write + AsRef<[u8]> + Debug,
+{
+    fn clone(&self) -> Self {
+        Self {
+            arc: self.arc.clone(),
+        }
+    }
+}
+
+impl<W> Write for ReadYourWrite<W>
+where
+    W: Write + AsRef<[u8]> + Debug,
+{
+    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+        let mut writer = self.arc.lock().expect("Internal error");
+        writer.write(buf)
+    }
+
+    fn flush(&mut self) -> std::io::Result<()> {
+        let mut writer = self.arc.lock().expect("Internal error");
+        writer.flush()
+    }
+}
+
 impl<W> ReadYourWrite<W>
 where
     W: Write + AsRef<[u8]>,
@@ -75,32 +101,6 @@ where
     }
 }
 
-impl<W> Clone for ReadYourWrite<W>
-where
-    W: Write + AsRef<[u8]> + Debug,
-{
-    fn clone(&self) -> Self {
-        Self {
-            arc: self.arc.clone(),
-        }
-    }
-}
-
-impl<W> Write for ReadYourWrite<W>
-where
-    W: Write + AsRef<[u8]> + Debug,
-{
-    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
-        let mut writer = self.arc.lock().expect("Internal error");
-        writer.write(buf)
-    }
-
-    fn flush(&mut self) -> std::io::Result<()> {
-        let mut writer = self.arc.lock().expect("Internal error");
-        writer.flush()
-    }
-}
-
 /// Iterator yielding every line in a string. The line includes newline character(s).
 ///
 /// <https://stackoverflow.com/a/40457615>

commons/src/output/warn_later.rs

@@ -29,7 +29,7 @@ thread_local!(static WARN_LATER: RefCell<Option<Vec<String>>> = RefCell::new(Non
 ///     let warn_later = WarnGuard::new(std::io::stdout());
 ///     // ...
 ///
-///     // Warnings will be emitted if the warn guard is dropped
+///     // Warnings will be emitted when the warn guard is dropped
 ///     drop(warn_later);
 /// // }
 /// ```
@@ -105,7 +105,7 @@ pub(crate) fn try_push(s: impl AsRef<str>) -> Result<(), WarnLaterError> {
 ///
 /// Should only ever be used within a `WarnGuard`.
 ///
-/// The state where the warnings are taken but a warn guard is still present
+/// The state where the warnings are taken, but a warn guard is still present
 /// can happen when more than one warn guard is created in the same thread
 fn force_push(s: impl AsRef<str>) {
     WARN_LATER.with(|cell| {