From a5a7c6d19c0974b7d9cdac406dca61f82f94220d Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 14:08:07 -0500 Subject: [PATCH 1/9] Move struct doc to module doc --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index bfe2470d..ee9daa9e 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -1,3 +1,9 @@ +//! # Install the bundler gem +//! +//! ## Layer dir: Install bundler to disk +//! +//! Installs a copy of `bundler` to the `` with a bundler executable in +//! `/bin`. Must run before [`crate.steps.bundle_install`]. use crate::RubyBuildpack; use crate::RubyBuildpackError; use commons::gemfile_lock::ResolvedBundlerVersion; @@ -21,12 +27,6 @@ pub(crate) struct BundleDownloadLayerMetadata { pub(crate) version: ResolvedBundlerVersion, } -/// # Install the bundler gem -/// -/// ## Layer dir: Install bundler to disk -/// -/// Installs a copy of `bundler` to the `` with a bundler executable in -/// `/bin`. Must run before [`crate.steps.bundle_install`]. pub(crate) struct BundleDownloadLayer<'a> { pub(crate) env: Env, pub(crate) metadata: BundleDownloadLayerMetadata, From e87a779217da397f18a07ced5d2c75cc5ccd811c Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 14:09:11 -0500 Subject: [PATCH 2/9] Rename metadata to Metadata --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 12 ++++++------ buildpacks/ruby/src/main.rs | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index ee9daa9e..86b09c4b 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -23,20 +23,20 @@ use std::path::Path; use std::process::Command; #[derive(Deserialize, Serialize, Debug, Clone)] -pub(crate) struct BundleDownloadLayerMetadata { +pub(crate) struct Metadata { pub(crate) version: ResolvedBundlerVersion, } pub(crate) struct BundleDownloadLayer<'a> { pub(crate) env: Env, - pub(crate) metadata: BundleDownloadLayerMetadata, + pub(crate) metadata: Metadata, pub(crate) _section_logger: &'a dyn SectionLogger, } #[allow(deprecated)] impl<'a> Layer for BundleDownloadLayer<'a> { type Buildpack = RubyBuildpack; - type Metadata = BundleDownloadLayerMetadata; + type Metadata = Metadata; fn types(&self) -> LayerTypes { LayerTypes { @@ -137,8 +137,8 @@ enum State { BundlerVersionChanged(ResolvedBundlerVersion, ResolvedBundlerVersion), } -fn cache_state(old: BundleDownloadLayerMetadata, now: BundleDownloadLayerMetadata) -> State { - let BundleDownloadLayerMetadata { version } = now; // Ensure all properties are checked +fn cache_state(old: Metadata, now: Metadata) -> State { + let Metadata { version } = now; // Ensure all properties are checked if old.version == version { State::NothingChanged(version) @@ -155,7 +155,7 @@ mod test { /// `migrate_incompatible_metadata` for the Layer trait #[test] fn metadata_guard() { - let metadata = BundleDownloadLayerMetadata { + let metadata = Metadata { version: ResolvedBundlerVersion(String::from("2.3.6")), }; diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index fcd1e8f3..2872d2ba 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -9,7 +9,7 @@ use core::str::FromStr; use fs_err::PathExt; use fun_run::CmdError; use layers::{ - bundle_download_layer::{BundleDownloadLayer, BundleDownloadLayerMetadata}, + bundle_download_layer::BundleDownloadLayer, bundle_install_layer::{BundleInstallLayer, BundleInstallLayerMetadata}, metrics_agent_install::MetricsAgentInstallError, ruby_install_layer::RubyInstallError, @@ -180,7 +180,7 @@ impl Buildpack for RubyBuildpack { layer_name!("bundler"), BundleDownloadLayer { env: env.clone(), - metadata: BundleDownloadLayerMetadata { + metadata: layers::bundle_download_layer::Metadata { version: bundler_version, }, _section_logger: section.as_ref(), From 28e42b9da94f36e03da68e5e9f07153c892aee58 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 14:17:13 -0500 Subject: [PATCH 3/9] Implement TryMigrate for bundle_download_layer::Metadata --- .../ruby/src/layers/bundle_download_layer.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 86b09c4b..eda20353 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -18,15 +18,29 @@ use libcnb::data::layer_content_metadata::LayerTypes; use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder}; use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libcnb::Env; -use serde::{Deserialize, Serialize}; +use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; +use serde::{Deserialize, Deserializer, Serialize}; +use std::convert::Infallible; use std::path::Path; use std::process::Command; +pub(crate) type Metadata = MetadataV1; +try_migrate_deserializer_chain!( + deserializer: toml::Deserializer::new, + error: MetadataError, + chain: [MetadataV1], +); + #[derive(Deserialize, Serialize, Debug, Clone)] -pub(crate) struct Metadata { +pub(crate) struct MetadataV1 { pub(crate) version: ResolvedBundlerVersion, } +#[derive(Debug, thiserror::Error)] +pub(crate) enum MetadataError { + // Update if migrating between a metadata version can error +} + pub(crate) struct BundleDownloadLayer<'a> { pub(crate) env: Env, pub(crate) metadata: Metadata, From 28d038be5e9e1a244b3f7b110c8d1a8380967fac Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 14:22:54 -0500 Subject: [PATCH 4/9] Implement MetadatDiff for bundler download layer --- .../ruby/src/layers/bundle_download_layer.rs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index eda20353..c8972a9f 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -4,8 +4,10 @@ //! //! Installs a copy of `bundler` to the `` with a bundler executable in //! `/bin`. Must run before [`crate.steps.bundle_install`]. +use crate::layers::shared::MetadataDiff; use crate::RubyBuildpack; use crate::RubyBuildpackError; +use bullet_stream::style; use commons::gemfile_lock::ResolvedBundlerVersion; use commons::output::{ fmt, @@ -31,6 +33,20 @@ try_migrate_deserializer_chain!( chain: [MetadataV1], ); +impl MetadataDiff for Metadata { + fn diff(&self, other: &Self) -> Vec { + let mut differences = Vec::new(); + if self.version != other.version { + differences.push(format!( + "Bundler version ({old} to {now})", + old = style::value(other.version.to_string()), + now = style::value(self.version.to_string()) + )); + } + differences + } +} + #[derive(Deserialize, Serialize, Debug, Clone)] pub(crate) struct MetadataV1 { pub(crate) version: ResolvedBundlerVersion, @@ -164,6 +180,24 @@ fn cache_state(old: Metadata, now: Metadata) -> State { #[cfg(test)] mod test { use super::*; + use crate::layers::shared::strip_ansi; + + #[test] + fn test_metadata_diff() { + let old = Metadata { + version: ResolvedBundlerVersion("2.3.5".to_string()), + }; + assert!(old.diff(&old).is_empty()); + + let diff = Metadata { + version: ResolvedBundlerVersion("2.3.6".to_string()), + } + .diff(&old); + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["Bundler version (`2.3.5` to `2.3.6`)"] + ); + } /// If this test fails due to a change you'll need to implement /// `migrate_incompatible_metadata` for the Layer trait From 7b580a14547b0ca8cb15736a961c6b410aa2f1ea Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 17:00:10 -0500 Subject: [PATCH 5/9] Extract layer logic to function --- .../ruby/src/layers/bundle_download_layer.rs | 110 ++++++++++-------- 1 file changed, 60 insertions(+), 50 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index c8972a9f..e1c68ec3 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -63,6 +63,64 @@ pub(crate) struct BundleDownloadLayer<'a> { pub(crate) _section_logger: &'a dyn SectionLogger, } +fn download_bundler( + env: &Env, + metadata: &Metadata, + path: &Path, +) -> Result { + let bin_dir = path.join("bin"); + let gem_path = path; + + let mut cmd = Command::new("gem"); + cmd.args([ + "install", + "bundler", + "--version", // Specify exact version to install + &metadata.version.to_string(), + ]) + .env_clear() + .envs(env); + + // Format `gem install --version ` without other content for display + let short_name = fun_run::display(&mut cmd); + + // Arguments we don't need in the output + cmd.args([ + "--install-dir", // Directory where bundler's contents will live + &gem_path.to_string_lossy(), + "--bindir", // Directory where `bundle` executable lives + &bin_dir.to_string_lossy(), + "--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` + ]); + + log_step_timed(format!("Running {}", fmt::command(short_name)), || { + cmd.named_output().map_err(|error| { + fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned()) + }) + }) + .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?; + + let layer_env = LayerEnv::new() + .chainable_insert(Scope::All, ModificationBehavior::Delimiter, "PATH", ":") + .chainable_insert( + Scope::All, + ModificationBehavior::Prepend, + "PATH", // Ensure this path comes before default bundler that ships with ruby, don't rely on the lifecycle + bin_dir, + ) + .chainable_insert(Scope::All, ModificationBehavior::Delimiter, "GEM_PATH", ":") + .chainable_insert( + Scope::All, + ModificationBehavior::Prepend, + "GEM_PATH", // Bundler is a gem too, allow it to be required + gem_path, + ); + + Ok(layer_env) +} + #[allow(deprecated)] impl<'a> Layer for BundleDownloadLayer<'a> { type Buildpack = RubyBuildpack; @@ -81,58 +139,10 @@ impl<'a> Layer for BundleDownloadLayer<'a> { _context: &BuildContext, layer_path: &Path, ) -> Result, RubyBuildpackError> { - let bin_dir = layer_path.join("bin"); - let gem_path = layer_path; - - let mut cmd = Command::new("gem"); - cmd.args([ - "install", - "bundler", - "--version", // Specify exact version to install - &self.metadata.version.to_string(), - ]) - .env_clear() - .envs(&self.env); - - // Format `gem install --version ` without other content for display - let short_name = fun_run::display(&mut cmd); - - // Arguments we don't need in the output - 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", // 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` - ]); - - log_step_timed(format!("Running {}", fmt::command(short_name)), || { - cmd.named_output().map_err(|error| { - fun_run::map_which_problem(error, cmd.mut_cmd(), self.env.get("PATH").cloned()) - }) - }) - .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?; + let layer_env = download_bundler(&self.env, &self.metadata, layer_path)?; LayerResultBuilder::new(self.metadata.clone()) - .env( - LayerEnv::new() - .chainable_insert(Scope::All, ModificationBehavior::Delimiter, "PATH", ":") - .chainable_insert( - Scope::All, - ModificationBehavior::Prepend, - "PATH", // Ensure this path comes before default bundler that ships with ruby, don't rely on the lifecycle - bin_dir, - ) - .chainable_insert(Scope::All, ModificationBehavior::Delimiter, "GEM_PATH", ":") - .chainable_insert( - Scope::All, - ModificationBehavior::Prepend, - "GEM_PATH", // Bundler is a gem too, allow it to be required - gem_path, - ), - ) + .env(layer_env) .build() } From 5c8d6599e100bf51be0d7823989ace3fe508542a Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 17:06:13 -0500 Subject: [PATCH 6/9] Refactor Command argument formatting --- .../ruby/src/layers/bundle_download_layer.rs | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index e1c68ec3..b076d81c 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -72,24 +72,16 @@ fn download_bundler( let gem_path = path; let mut cmd = Command::new("gem"); + cmd.args(["install", "bundler"]); + cmd.args(["--version", &metadata.version.to_string()]) // Specify exact version to install + .env_clear() + .envs(env); + + let short_name = fun_run::display(&mut cmd); // Format `gem install --version ` without other content for display + + cmd.args(["--install-dir", &format!("{}", gem_path.display())]); // Directory where bundler's contents will live + cmd.args(["--bindir", &format!("{}", bin_dir.display())]); // Directory where `bundle` executable lives cmd.args([ - "install", - "bundler", - "--version", // Specify exact version to install - &metadata.version.to_string(), - ]) - .env_clear() - .envs(env); - - // Format `gem install --version ` without other content for display - let short_name = fun_run::display(&mut cmd); - - // Arguments we don't need in the output - cmd.args([ - "--install-dir", // Directory where bundler's contents will live - &gem_path.to_string_lossy(), - "--bindir", // Directory where `bundle` executable lives - &bin_dir.to_string_lossy(), "--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` From 6080b50cb4c4727757c270a2036a974099789e42 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 17:13:35 -0500 Subject: [PATCH 7/9] Replace layer trait with struct API --- .../ruby/src/layers/bundle_download_layer.rs | 107 +++++------------- buildpacks/ruby/src/main.rs | 17 +-- 2 files changed, 35 insertions(+), 89 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index b076d81c..615803a1 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -4,20 +4,18 @@ //! //! Installs a copy of `bundler` to the `` with a bundler executable in //! `/bin`. Must run before [`crate.steps.bundle_install`]. -use crate::layers::shared::MetadataDiff; +use crate::layers::shared::{cached_layer_write_metadata, MetadataDiff}; use crate::RubyBuildpack; use crate::RubyBuildpackError; use bullet_stream::style; use commons::gemfile_lock::ResolvedBundlerVersion; use commons::output::{ fmt, - section_log::{log_step, log_step_timed, SectionLogger}, + section_log::{log_step, log_step_timed}, }; use fun_run::{self, CommandWithName}; -use libcnb::build::BuildContext; -use libcnb::data::layer_content_metadata::LayerTypes; -#[allow(deprecated)] -use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder}; +use libcnb::data::layer_name; +use libcnb::layer::{EmptyLayerCause, LayerState}; use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libcnb::Env; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; @@ -26,6 +24,31 @@ use std::convert::Infallible; use std::path::Path; use std::process::Command; +pub(crate) fn handle( + context: &libcnb::build::BuildContext, + env: &Env, + metadata: &Metadata, +) -> libcnb::Result { + let layer_ref = cached_layer_write_metadata(layer_name!("bundler"), context, metadata)?; + match &layer_ref.state { + LayerState::Restored { cause } => { + log_step(cause); + } + LayerState::Empty { cause } => { + match cause { + EmptyLayerCause::NewlyCreated => {} + EmptyLayerCause::InvalidMetadataAction { cause } + | EmptyLayerCause::RestoredLayerAction { cause } => { + log_step(cause); + } + } + let layer_env = download_bundler(env, metadata, &layer_ref.path())?; + layer_ref.write_env(&layer_env)?; + } + } + Ok(layer_ref.read_env()?) +} + pub(crate) type Metadata = MetadataV1; try_migrate_deserializer_chain!( deserializer: toml::Deserializer::new, @@ -57,12 +80,6 @@ pub(crate) enum MetadataError { // Update if migrating between a metadata version can error } -pub(crate) struct BundleDownloadLayer<'a> { - pub(crate) env: Env, - pub(crate) metadata: Metadata, - pub(crate) _section_logger: &'a dyn SectionLogger, -} - fn download_bundler( env: &Env, metadata: &Metadata, @@ -113,72 +130,6 @@ fn download_bundler( Ok(layer_env) } -#[allow(deprecated)] -impl<'a> Layer for BundleDownloadLayer<'a> { - type Buildpack = RubyBuildpack; - type Metadata = Metadata; - - fn types(&self) -> LayerTypes { - LayerTypes { - build: true, - launch: true, - cache: true, - } - } - - fn create( - &mut self, - _context: &BuildContext, - layer_path: &Path, - ) -> Result, RubyBuildpackError> { - let layer_env = download_bundler(&self.env, &self.metadata, layer_path)?; - - LayerResultBuilder::new(self.metadata.clone()) - .env(layer_env) - .build() - } - - fn existing_layer_strategy( - &mut self, - _context: &BuildContext, - layer_data: &LayerData, - ) -> Result { - let old = &layer_data.content_metadata.metadata; - let now = self.metadata.clone(); - match cache_state(old.clone(), now) { - State::NothingChanged(_version) => { - log_step("Using cached version"); - - Ok(ExistingLayerStrategy::Keep) - } - State::BundlerVersionChanged(_old, _now) => { - log_step(format!( - "Clearing cache {}", - fmt::details("bundler version changed") - )); - - Ok(ExistingLayerStrategy::Recreate) - } - } - } -} - -// [derive(Debug)] -enum State { - NothingChanged(ResolvedBundlerVersion), - BundlerVersionChanged(ResolvedBundlerVersion, ResolvedBundlerVersion), -} - -fn cache_state(old: Metadata, now: Metadata) -> State { - let Metadata { version } = now; // Ensure all properties are checked - - if old.version == version { - State::NothingChanged(version) - } else { - State::BundlerVersionChanged(old.version, version) - } -} - #[cfg(test)] mod test { use super::*; diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 2872d2ba..8c129643 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -9,7 +9,6 @@ use core::str::FromStr; use fs_err::PathExt; use fun_run::CmdError; use layers::{ - bundle_download_layer::BundleDownloadLayer, bundle_install_layer::{BundleInstallLayer, BundleInstallLayerMetadata}, metrics_agent_install::MetricsAgentInstallError, ruby_install_layer::RubyInstallError, @@ -176,19 +175,15 @@ impl Buildpack for RubyBuildpack { fmt::value(bundler_version.to_string()), fmt::value(gemfile_lock.bundler_source()) )); - let download_bundler_layer = context.handle_layer( - layer_name!("bundler"), - BundleDownloadLayer { - env: env.clone(), - metadata: layers::bundle_download_layer::Metadata { - version: bundler_version, - }, - _section_logger: section.as_ref(), + let layer_env = layers::bundle_download_layer::handle( + &context, + &env, + &layers::bundle_download_layer::Metadata { + version: bundler_version, }, )?; - let env = download_bundler_layer.env.apply(Scope::Build, &env); - (section.end_section(), env) + (section.end_section(), layer_env.apply(Scope::Build, &env)) }; // ## Bundle install From f23beb92760d3ce70a69a329cf96f3297ddeaff2 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 17:22:49 -0500 Subject: [PATCH 8/9] Update logging to bullet_stream --- .../ruby/src/layers/bundle_download_layer.rs | 37 ++++++++++--------- buildpacks/ruby/src/main.rs | 19 +++++----- 2 files changed, 29 insertions(+), 27 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 615803a1..59ad73db 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -7,12 +7,9 @@ use crate::layers::shared::{cached_layer_write_metadata, MetadataDiff}; use crate::RubyBuildpack; use crate::RubyBuildpackError; -use bullet_stream::style; +use bullet_stream::state::SubBullet; +use bullet_stream::{style, Print}; use commons::gemfile_lock::ResolvedBundlerVersion; -use commons::output::{ - fmt, - section_log::{log_step, log_step_timed}, -}; use fun_run::{self, CommandWithName}; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; @@ -21,32 +18,36 @@ use libcnb::Env; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; use serde::{Deserialize, Deserializer, Serialize}; use std::convert::Infallible; +use std::io::Stdout; use std::path::Path; use std::process::Command; pub(crate) fn handle( context: &libcnb::build::BuildContext, env: &Env, + mut bullet: Print>, metadata: &Metadata, -) -> libcnb::Result { +) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { let layer_ref = cached_layer_write_metadata(layer_name!("bundler"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { - log_step(cause); + bullet = bullet.sub_bullet(cause); + Ok((bullet, layer_ref.read_env()?)) } LayerState::Empty { cause } => { match cause { EmptyLayerCause::NewlyCreated => {} EmptyLayerCause::InvalidMetadataAction { cause } | EmptyLayerCause::RestoredLayerAction { cause } => { - log_step(cause); + bullet = bullet.sub_bullet(cause); } } - let layer_env = download_bundler(env, metadata, &layer_ref.path())?; + let (bullet, layer_env) = download_bundler(bullet, env, metadata, &layer_ref.path())?; layer_ref.write_env(&layer_env)?; + + Ok((bullet, layer_ref.read_env()?)) } } - Ok(layer_ref.read_env()?) } pub(crate) type Metadata = MetadataV1; @@ -81,10 +82,11 @@ pub(crate) enum MetadataError { } fn download_bundler( + bullet: Print>, env: &Env, metadata: &Metadata, path: &Path, -) -> Result { +) -> Result<(Print>, LayerEnv), RubyBuildpackError> { let bin_dir = path.join("bin"); let gem_path = path; @@ -104,12 +106,11 @@ fn download_bundler( "--env-shebang", // Start the `bundle` executable with `#! /usr/bin/env ruby` ]); - log_step_timed(format!("Running {}", fmt::command(short_name)), || { - cmd.named_output().map_err(|error| { - fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned()) - }) - }) - .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?; + let timer = bullet.start_timer(format!("Running {}", style::command(short_name))); + + cmd.named_output() + .map_err(|error| fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned())) + .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?; let layer_env = LayerEnv::new() .chainable_insert(Scope::All, ModificationBehavior::Delimiter, "PATH", ":") @@ -127,7 +128,7 @@ fn download_bundler( gem_path, ); - Ok(layer_env) + Ok((timer.done(), layer_env)) } #[cfg(test)] diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 8c129643..ca481d47 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -2,9 +2,9 @@ use bullet_stream::{style, Print}; use commons::cache::CacheError; use commons::gemfile_lock::GemfileLock; use commons::metadata_digest::MetadataDigest; -use commons::output::warn_later::WarnGuard; #[allow(clippy::wildcard_imports)] -use commons::output::{build_log::*, fmt}; +use commons::output::build_log::*; +use commons::output::warn_later::WarnGuard; use core::str::FromStr; use fs_err::PathExt; use fun_run::CmdError; @@ -148,7 +148,7 @@ impl Buildpack for RubyBuildpack { }; // ## Install executable ruby version - (_, env) = { + (build_output, env) = { let bullet = build_output.bullet(format!( "Ruby version {} from {}", style::value(ruby_version.to_string()), @@ -169,21 +169,22 @@ impl Buildpack for RubyBuildpack { }; // ## Setup bundler - (logger, env) = { - let section = logger.section(&format!( + (_, env) = { + let bullet = build_output.bullet(format!( "Bundler version {} from {}", - fmt::value(bundler_version.to_string()), - fmt::value(gemfile_lock.bundler_source()) + style::value(bundler_version.to_string()), + style::value(gemfile_lock.bundler_source()) )); - let layer_env = layers::bundle_download_layer::handle( + let (bullet, layer_env) = layers::bundle_download_layer::handle( &context, &env, + bullet, &layers::bundle_download_layer::Metadata { version: bundler_version, }, )?; - (section.end_section(), layer_env.apply(Scope::Build, &env)) + (bullet.done(), layer_env.apply(Scope::Build, &env)) }; // ## Bundle install From a05fb5c15ec776f77fc0f8d51b24deeb9bb5ee72 Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Mon, 7 Oct 2024 16:35:44 -0500 Subject: [PATCH 9/9] [stacked] Struct API and bullet stream for Bundle install layer (#332) * Introduce meta struct for holding layer ref results * Rename generic to match M for metadata style * Rename metadata generic * Use Meta to return old cached data Meta acts like a String for purposes of printing, but can optionally contain old metadata if the programmer needs it. This is useful for the "bundle install" layer which stores information in it's cache that is not used for cache invalidation but rather to determine whether or not it can skip running `bundle install`. * Rename structs * Move docs to module * Update docs * Move metadata type alias to top of file * Update const name and add docs * Add docs * Implement MetadataDiff This diff will eventually control caching logic * Test caching logic * Refactor bundle install command invocation * Group imports * Stub handle function with no logic * Build install state from old cache data * Rename UpdateState to InstallState * Call bundle install from handle function * Replace layer with struct API call * Rename function * Remove unused imports * Move buildpack logging to the top of main * Remove import * Move logic out of function * Replace commons logging with bullet stream * Clippy --- .../ruby/src/layers/bundle_install_layer.rs | 538 ++++++++---------- buildpacks/ruby/src/layers/shared.rs | 99 +++- buildpacks/ruby/src/main.rs | 71 ++- 3 files changed, 357 insertions(+), 351 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 2f8c01b1..989482ba 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -1,48 +1,168 @@ +//! Installs Ruby gems (libraries) via `bundle install` +//! +//! Creates the cache where gems live. We want 'bundle install' +//! to execute on every build (as opposed to only when the cache is empty). +//! +//! As a small performance optimization, it will not run if the `Gemfile.lock`, +//! `Gemfile`, or user provided "platform" environment variable have not changed. +//! User applications can opt out of this behavior by setting the environment +//! variable `HEROKU_SKIP_BUNDLE_DIGEST=1`. That would be useful if the application's +//! `Gemfile` sources logic or data from another file that is unknown to the buildpack. +//! +//! Gems can be plain Ruby code which are OS, Architecture, and Ruby version independent. +//! They can also be native extensions that use Ruby's C API or contain libraries that +//! must be compiled and will then be invoked via FFI. These native extensions are +//! OS, Architecture, and Ruby version dependent. Due to this, when one of these changes +//! we must clear the cache and re-run `bundle install`. +use crate::layers::shared::{cached_layer_write_metadata, Meta, MetadataDiff}; +use crate::target_id::{TargetId, TargetIdError}; use crate::{BundleWithout, RubyBuildpack, RubyBuildpackError}; -use commons::output::{ - fmt::{self, HELP}, - section_log::{log_step, log_step_stream, SectionLogger}, -}; +use bullet_stream::state::SubBullet; +use bullet_stream::{style, Print}; use commons::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; -use fun_run::CommandWithName; -use fun_run::{self, CmdError}; -#[allow(deprecated)] -use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder}; +use fun_run::{self, CommandWithName}; +use libcnb::data::layer_name; +use libcnb::layer::{EmptyLayerCause, LayerState}; use libcnb::{ - build::BuildContext, - data::layer_content_metadata::LayerTypes, layer_env::{LayerEnv, ModificationBehavior, Scope}, Env, }; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; use serde::{Deserialize, Deserializer, Serialize}; use std::convert::Infallible; +use std::io::Stdout; use std::{path::Path, process::Command}; -use crate::target_id::{TargetId, TargetIdError}; - -const HEROKU_SKIP_BUNDLE_DIGEST: &str = "HEROKU_SKIP_BUNDLE_DIGEST"; +/// When this environment variable is set, the `bundle install` command will always +/// run regardless of whether the `Gemfile`, `Gemfile.lock`, or platform environment +/// variables have changed. +const SKIP_DIGEST_ENV_KEY: &str = "HEROKU_SKIP_BUNDLE_DIGEST"; +/// A failsafe, if a programmer made a mistake in the caching logic, rev-ing this +/// key will force a re-run of `bundle install` to ensure the cache is correct +/// on the next build. pub(crate) const FORCE_BUNDLE_INSTALL_CACHE_KEY: &str = "v1"; -/// Mostly runs 'bundle install' -/// -/// Creates the cache where gems live. We want 'bundle install' -/// to execute on every build (as opposed to only when the cache is empty) -/// -/// To help achieve this the logic inside of `BundleInstallLayer::update` and -/// `BundleInstallLayer::create` are the same. -#[derive(Debug)] -pub(crate) struct BundleInstallLayer<'a> { - pub(crate) env: Env, - pub(crate) without: BundleWithout, - pub(crate) _section_log: &'a dyn SectionLogger, - pub(crate) metadata: BundleInstallLayerMetadata, +pub(crate) fn handle( + context: &libcnb::build::BuildContext, + env: &Env, + mut bullet: Print>, + metadata: &Metadata, + without: &BundleWithout, +) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { + let layer_ref = cached_layer_write_metadata(layer_name!("gems"), context, metadata)?; + let install_state = match &layer_ref.state { + LayerState::Restored { cause } => { + bullet = bullet.sub_bullet(cause); + match cause { + Meta::Data(old) => install_state(old, metadata), + Meta::Message(_) => InstallState::Run(String::new()), + } + } + LayerState::Empty { cause } => match cause { + EmptyLayerCause::NewlyCreated => InstallState::Run(String::new()), + EmptyLayerCause::InvalidMetadataAction { cause } + | EmptyLayerCause::RestoredLayerAction { cause } => { + bullet = bullet.sub_bullet(cause); + InstallState::Run(String::new()) + } + }, + }; + + let env = { + let layer_env = layer_env(&layer_ref.path(), &context.app_dir, without); + layer_ref.write_env(&layer_env)?; + layer_env.apply(Scope::Build, env) + }; + + match install_state { + InstallState::Run(reason) => { + if !reason.is_empty() { + bullet = bullet.sub_bullet(reason); + } + + let mut cmd = Command::new("bundle"); + 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), + ) + .map_err(|error| { + fun_run::map_which_problem(error, cmd.mut_cmd(), env.get("PATH").cloned()) + }) + .map_err(RubyBuildpackError::BundleInstallCommandError)?; + } + InstallState::Skip(checked) => { + let bundle_install = style::value("bundle install"); + let help = style::important("HELP"); + + bullet = bullet + .sub_bullet(format!( + "Skipping {bundle_install} (no changes found in {sources})", + sources = SentenceList::new(&checked).join_str("or") + )) + .sub_bullet(format!( + "{help} To force run {bundle_install} set {}", + style::value(format!("{SKIP_DIGEST_ENV_KEY}=1")) + )); + } + } + + Ok((bullet, layer_ref.read_env()?)) +} + +pub(crate) type Metadata = MetadataV2; +try_migrate_deserializer_chain!( + chain: [MetadataV1, MetadataV2], + error: MetadataMigrateError, + deserializer: toml::Deserializer::new, +); + +impl MetadataDiff for Metadata { + fn diff(&self, old: &Self) -> Vec { + let mut differences = Vec::new(); + let Metadata { + distro_name, + distro_version, + cpu_architecture, + ruby_version, + force_bundle_install_key: _, + digest: _, + } = old; + + if ruby_version != &self.ruby_version { + differences.push(format!( + "Ruby version ({old} to {now})", + old = style::value(ruby_version.to_string()), + now = style::value(self.ruby_version.to_string()) + )); + } + if distro_name != &self.distro_name || distro_version != &self.distro_version { + differences.push(format!( + "Distribution ({old} to {now})", + old = style::value(format!("{distro_name} {distro_version}")), + now = style::value(format!("{} {}", self.distro_name, self.distro_version)) + )); + } + if cpu_architecture != &self.cpu_architecture { + differences.push(format!( + "CPU architecture ({old} to {now})", + old = style::value(cpu_architecture), + now = style::value(&self.cpu_architecture) + )); + } + + differences + } } #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] -pub(crate) struct BundleInstallLayerMetadataV1 { +pub(crate) struct MetadataV1 { pub(crate) stack: String, pub(crate) ruby_version: ResolvedRubyVersion, pub(crate) force_bundle_install_key: String, @@ -50,7 +170,7 @@ pub(crate) struct BundleInstallLayerMetadataV1 { } #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] -pub(crate) struct BundleInstallLayerMetadataV2 { +pub(crate) struct MetadataV2 { pub(crate) distro_name: String, pub(crate) distro_version: String, pub(crate) cpu_architecture: String, @@ -71,13 +191,6 @@ pub(crate) struct BundleInstallLayerMetadataV2 { pub(crate) digest: MetadataDigest, // Must be last for serde to be happy https://github.com/toml-rs/toml-rs/issues/142 } -try_migrate_deserializer_chain!( - chain: [BundleInstallLayerMetadataV1, BundleInstallLayerMetadataV2], - error: MetadataMigrateError, - deserializer: toml::Deserializer::new, -); -pub(crate) type BundleInstallLayerMetadata = BundleInstallLayerMetadataV2; - #[derive(thiserror::Error, Debug)] pub(crate) enum MetadataMigrateError { #[error("Could not migrate metadata {0}")] @@ -86,10 +199,10 @@ pub(crate) enum MetadataMigrateError { // CNB spec moved from the concept of "stacks" (i.e. "heroku-22" which represented an OS and system dependencies) to finer // grained "target" which includes the OS, OS version, and architecture. This function converts the old stack id to the new target id. -impl TryFrom for BundleInstallLayerMetadataV2 { +impl TryFrom for MetadataV2 { type Error = MetadataMigrateError; - fn try_from(v1: BundleInstallLayerMetadataV1) -> Result { + fn try_from(v1: MetadataV1) -> Result { let target_id = TargetId::from_stack(&v1.stack).map_err(MetadataMigrateError::UnsupportedStack)?; @@ -104,21 +217,8 @@ impl TryFrom for BundleInstallLayerMetadataV2 { } } -impl<'a> BundleInstallLayer<'a> { - #[allow(clippy::unnecessary_wraps)] - fn build_layer_env( - &self, - context: &BuildContext, - layer_path: &Path, - ) -> Result { - let out = layer_env(layer_path, &context.app_dir, &self.without); - - Ok(out) - } -} - #[derive(Debug)] -enum UpdateState { +enum InstallState { /// Holds message indicating the reason why we want to run 'bundle install' Run(String), @@ -128,206 +228,24 @@ enum UpdateState { /// Determines if 'bundle install' should execute on a given call to `BundleInstallLatyer::update` /// -/// -fn update_state(old: &BundleInstallLayerMetadata, now: &BundleInstallLayerMetadata) -> UpdateState { - let forced_env = std::env::var_os(HEROKU_SKIP_BUNDLE_DIGEST); +fn install_state(old: &Metadata, now: &Metadata) -> InstallState { + let forced_env = std::env::var_os(SKIP_DIGEST_ENV_KEY); let old_key = &old.force_bundle_install_key; let now_key = &now.force_bundle_install_key; if old_key != now_key { - UpdateState::Run(format!( + InstallState::Run(format!( "buildpack author triggered internal change {old_key} to {now_key}" )) } else if let Some(value) = forced_env { let value = value.to_string_lossy(); - UpdateState::Run(format!("found {HEROKU_SKIP_BUNDLE_DIGEST}={value}")) + InstallState::Run(format!("found {SKIP_DIGEST_ENV_KEY}={value}")) } else if let Some(changed) = now.digest.changed(&old.digest) { - UpdateState::Run(format!("{changed}")) + InstallState::Run(format!("{changed}")) } else { let checked = now.digest.checked_list(); - UpdateState::Skip(checked) - } -} - -#[allow(deprecated)] -impl Layer for BundleInstallLayer<'_> { - type Buildpack = RubyBuildpack; - type Metadata = BundleInstallLayerMetadata; - - fn types(&self) -> LayerTypes { - LayerTypes { - build: true, - launch: true, - cache: true, - } - } - - /// Runs with gems cache from last execution - fn update( - &mut self, - context: &BuildContext, - layer_data: &LayerData, - ) -> Result, RubyBuildpackError> { - let metadata = self.metadata.clone(); - let layer_env = self.build_layer_env(context, &layer_data.path)?; - let env = layer_env.apply(Scope::Build, &self.env); - - match update_state(&layer_data.content_metadata.metadata, &metadata) { - UpdateState::Run(reason) => { - log_step(reason); - - bundle_install(&env).map_err(RubyBuildpackError::BundleInstallCommandError)?; - } - UpdateState::Skip(checked) => { - let bundle_install = fmt::value("bundle install"); - - log_step(format!( - "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 {}", - fmt::value(format!("{HEROKU_SKIP_BUNDLE_DIGEST}=1")) - )); - } - } - - LayerResultBuilder::new(metadata).env(layer_env).build() - } - - /// Runs when with empty cache - fn create( - &mut self, - context: &BuildContext, - layer_path: &Path, - ) -> Result, RubyBuildpackError> { - let layer_env = self.build_layer_env(context, layer_path)?; - let env = layer_env.apply(Scope::Build, &self.env); - - bundle_install(&env).map_err(RubyBuildpackError::BundleInstallCommandError)?; - - LayerResultBuilder::new(self.metadata.clone()) - .env(layer_env) - .build() - } - - /// When there is a cache determines if we will run: - /// - update (keep cache and bundle install) - /// - recreate (destroy cache and bundle instal) - /// - /// CAUTION: We should Should never Keep, this will prevent env vars - /// if a coder updates env vars they won't be set unless update or - /// create is run. - fn existing_layer_strategy( - &mut self, - _context: &BuildContext, - layer_data: &LayerData, - ) -> Result { - let old = &layer_data.content_metadata.metadata; - let now = self.metadata.clone(); - - let clear_and_run = Ok(ExistingLayerStrategy::Recreate); - let keep_and_run = Ok(ExistingLayerStrategy::Update); - - match cache_state(old.clone(), now) { - Changed::Nothing => { - log_step("Loading cached gems"); - - keep_and_run - } - Changed::DistroName(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("distro name changed: {old} to {now}")) - )); - - clear_and_run - } - Changed::DistroVersion(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("distro version changed: {old} to {now}")) - )); - - clear_and_run - } - Changed::CpuArchitecture(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("cpu architecture changed: {old} to {now}")) - )); - - clear_and_run - } - Changed::RubyVersion(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("Ruby version changed: {old} to {now}")) - )); - - clear_and_run - } - } - } - - fn migrate_incompatible_metadata( - &mut self, - _context: &BuildContext, - metadata: &libcnb::generic::GenericMetadata, - ) -> Result< - libcnb::layer::MetadataMigration, - ::Error, - > { - match Self::Metadata::try_from_str_migrations( - &toml::to_string(&metadata).expect("TOML deserialization of GenericMetadata"), - ) { - Some(Ok(metadata)) => Ok(libcnb::layer::MetadataMigration::ReplaceMetadata(metadata)), - Some(Err(e)) => { - log_step(format!("Clearing cache (metadata migration error {e})")); - Ok(libcnb::layer::MetadataMigration::RecreateLayer) - } - None => { - log_step("Clearing cache (invalid metadata)"); - Ok(libcnb::layer::MetadataMigration::RecreateLayer) - } - } - } -} - -/// The possible states of the cache values, used for determining `ExistingLayerStrategy` -#[derive(Debug)] -enum Changed { - Nothing, - DistroName(String, String), - DistroVersion(String, String), - CpuArchitecture(String, String), - RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion), -} - -// Compare the old metadata to current metadata to determine the state of the -// cache. Based on that state, we can log and determine `ExistingLayerStrategy` -fn cache_state(old: BundleInstallLayerMetadata, now: BundleInstallLayerMetadata) -> Changed { - let BundleInstallLayerMetadata { - distro_name, - distro_version, - cpu_architecture, - ruby_version, - force_bundle_install_key: _, - digest: _, // digest state handled elsewhere - } = now; // ensure all values are handled or we get a clippy warning - - if old.distro_name != distro_name { - Changed::DistroName(old.distro_name, distro_name) - } else if old.distro_version != distro_version { - Changed::DistroVersion(old.distro_version, distro_version) - } else if old.cpu_architecture != cpu_architecture { - Changed::CpuArchitecture(old.cpu_architecture, cpu_architecture) - } else if old.ruby_version != ruby_version { - Changed::RubyVersion(old.ruby_version, ruby_version) - } else { - Changed::Nothing + InstallState::Skip(checked) } } @@ -385,44 +303,21 @@ fn layer_env(layer_path: &Path, app_dir: &Path, without_default: &BundleWithout) layer_env } -/// Sets the needed environment variables to configure bundler and uses them -/// to execute the `bundle install` command. The results are streamed to stdout/stderr. -/// -/// # Errors -/// -/// When the 'bundle install' command fails this function returns an error. -/// -fn bundle_install(env: &Env) -> Result<(), CmdError> { - let path_env = env.get("PATH").cloned(); - let display_with_env = |cmd: &'_ mut Command| { - fun_run::display_with_env_keys( - cmd, - env, - [ - "BUNDLE_BIN", - "BUNDLE_CLEAN", - "BUNDLE_DEPLOYMENT", - "BUNDLE_GEMFILE", - "BUNDLE_PATH", - "BUNDLE_WITHOUT", - ], - ) - }; - - // ## Run `$ bundle install` - let mut cmd = Command::new("bundle"); - cmd.env_clear() // Current process env vars already merged into env - .args(["install"]) - .envs(env); - - let mut cmd = cmd.named_fn(display_with_env); - - log_step_stream(format!("Running {}", fmt::command(cmd.name())), |stream| { - cmd.stream_output(stream.io(), stream.io()) - }) - .map_err(|error| fun_run::map_which_problem(error, cmd.mut_cmd(), path_env))?; - - Ok(()) +/// Displays the `bundle install` command with `BUNDLE_` environment variables +/// that we use to configure bundler. +fn display_name(cmd: &mut Command, env: &Env) -> String { + fun_run::display_with_env_keys( + cmd, + env, + [ + "BUNDLE_BIN", + "BUNDLE_CLEAN", + "BUNDLE_DEPLOYMENT", + "BUNDLE_GEMFILE", + "BUNDLE_PATH", + "BUNDLE_WITHOUT", + ], + ) } #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, Default)] @@ -434,9 +329,83 @@ pub(crate) struct BundleDigest { #[cfg(test)] mod test { + use crate::layers::shared::strip_ansi; + use super::*; use std::path::PathBuf; + /// `MetadataDiff` logic controls cache invalidation + /// When the vec is empty the cache is kept, otherwise it is invalidated + #[test] + fn metadata_diff_messages() { + let tmpdir = tempfile::tempdir().unwrap(); + let app_path = tmpdir.path().to_path_buf(); + let gemfile = app_path.join("Gemfile"); + let env = Env::new(); + let context = FakeContext { + platform: FakePlatform { env }, + app_path, + }; + std::fs::write(&gemfile, "iamagemfile").unwrap(); + + let old = Metadata { + ruby_version: ResolvedRubyVersion("3.5.3".to_string()), + distro_name: "ubuntu".to_string(), + distro_version: "20.04".to_string(), + cpu_architecture: "amd64".to_string(), + force_bundle_install_key: FORCE_BUNDLE_INSTALL_CACHE_KEY.to_string(), + digest: MetadataDigest::new_env_files( + &context.platform, + &[&context.app_path.join("Gemfile")], + ) + .unwrap(), + }; + assert_eq!(old.diff(&old), Vec::::new()); + + let diff = Metadata { + ruby_version: ResolvedRubyVersion("3.5.5".to_string()), + distro_name: old.distro_name.clone(), + distro_version: old.distro_version.clone(), + cpu_architecture: old.cpu_architecture.clone(), + force_bundle_install_key: old.force_bundle_install_key.clone(), + digest: old.digest.clone(), + } + .diff(&old); + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["Ruby version (`3.5.3` to `3.5.5`)".to_string()] + ); + + let diff = Metadata { + ruby_version: old.ruby_version.clone(), + distro_name: "alpine".to_string(), + distro_version: "3.20.0".to_string(), + cpu_architecture: old.cpu_architecture.clone(), + force_bundle_install_key: old.force_bundle_install_key.clone(), + digest: old.digest.clone(), + } + .diff(&old); + + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["Distribution (`ubuntu 20.04` to `alpine 3.20.0`)".to_string()] + ); + + let diff = Metadata { + ruby_version: old.ruby_version.clone(), + distro_name: old.distro_name.clone(), + distro_version: old.distro_version.clone(), + cpu_architecture: "arm64".to_string(), + force_bundle_install_key: old.force_bundle_install_key.clone(), + digest: old.digest.clone(), + } + .diff(&old); + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["CPU architecture (`amd64` to `arm64`)".to_string()] + ); + } + #[cfg(test)] #[derive(Default, Clone)] struct FakeContext { @@ -506,7 +475,7 @@ GEM_PATH=layer_path std::fs::write(&gemfile, "iamagemfile").unwrap(); let target_id = TargetId::from_stack("heroku-22").unwrap(); - let metadata = BundleInstallLayerMetadata { + let metadata = Metadata { distro_name: target_id.distro_name, distro_version: target_id.distro_version, cpu_architecture: target_id.cpu_architecture, @@ -540,7 +509,7 @@ platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3 .to_string(); assert_eq!(toml_string, actual.trim()); - let deserialized: BundleInstallLayerMetadata = toml::from_str(&toml_string).unwrap(); + let deserialized: Metadata = toml::from_str(&toml_string).unwrap(); assert_eq!(metadata, deserialized); } @@ -560,7 +529,7 @@ platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3 }; std::fs::write(&gemfile, "iamagemfile").unwrap(); - let metadata = BundleInstallLayerMetadataV1 { + let metadata = MetadataV1 { stack: String::from("heroku-22"), ruby_version: ResolvedRubyVersion(String::from("3.1.3")), force_bundle_install_key: String::from("v1"), @@ -590,13 +559,12 @@ platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3 .to_string(); assert_eq!(toml_string, actual.trim()); - let deserialized: BundleInstallLayerMetadataV2 = - BundleInstallLayerMetadataV2::try_from_str_migrations(&toml_string) - .unwrap() - .unwrap(); + let deserialized: MetadataV2 = MetadataV2::try_from_str_migrations(&toml_string) + .unwrap() + .unwrap(); let target_id = TargetId::from_stack(&metadata.stack).unwrap(); - let expected = BundleInstallLayerMetadataV2 { + let expected = MetadataV2 { distro_name: target_id.distro_name, distro_version: target_id.distro_version, cpu_architecture: target_id.cpu_architecture, diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 48152aed..cbb2af4f 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -10,10 +10,10 @@ pub(crate) fn cached_layer_write_metadata( layer_name: libcnb::data::layer::LayerName, context: &BuildContext, metadata: &'_ M, -) -> libcnb::Result, B::Error> +) -> libcnb::Result, Meta>, B::Error> where B: libcnb::Buildpack, - M: MetadataDiff + magic_migrate::TryMigrate + serde::ser::Serialize + std::fmt::Debug, + M: MetadataDiff + magic_migrate::TryMigrate + serde::ser::Serialize + std::fmt::Debug + Clone, ::Error: std::fmt::Display, { let layer_ref = context.cached_layer( @@ -40,21 +40,21 @@ pub(crate) trait MetadataDiff { /// /// If the diff is empty, there are no changes and the layer is kept /// If the diff is not empty, the layer is deleted and the changes are listed -pub(crate) fn restored_layer_action(old: &T, now: &T) -> (RestoredLayerAction, String) +pub(crate) fn restored_layer_action(old: &M, now: &M) -> (RestoredLayerAction, Meta) where - T: MetadataDiff, + M: MetadataDiff + Clone, { let diff = now.diff(old); if diff.is_empty() { - (RestoredLayerAction::KeepLayer, "Using cache".to_string()) + (RestoredLayerAction::KeepLayer, Meta::Data(now.clone())) } else { ( RestoredLayerAction::DeleteLayer, - format!( + Meta::Message(format!( "Clearing cache due to {changes}: {differences}", changes = if diff.len() > 1 { "changes" } else { "change" }, differences = SentenceList::new(&diff) - ), + )), ) } } @@ -64,39 +64,83 @@ where /// If the metadata can be migrated, it is replaced with the migrated version /// If an error occurs, the layer is deleted and the error displayed /// If no migration is possible, the layer is deleted and the invalid metadata is displayed -pub(crate) fn invalid_metadata_action(invalid: &S) -> (InvalidMetadataAction, String) +pub(crate) fn invalid_metadata_action(invalid: &S) -> (InvalidMetadataAction, Meta) where - T: magic_migrate::TryMigrate, + M: magic_migrate::TryMigrate + Clone, S: serde::ser::Serialize + std::fmt::Debug, // TODO: Enforce Display + Debug in the library - ::Error: std::fmt::Display, + ::Error: std::fmt::Display, { let invalid = toml::to_string(invalid); match invalid { - Ok(toml) => match T::try_from_str_migrations(&toml) { + Ok(toml) => match M::try_from_str_migrations(&toml) { Some(Ok(migrated)) => ( - InvalidMetadataAction::ReplaceMetadata(migrated), - "Replaced metadata".to_string(), + InvalidMetadataAction::ReplaceMetadata(migrated.clone()), + Meta::Data(migrated), ), Some(Err(error)) => ( InvalidMetadataAction::DeleteLayer, - format!("Clearing cache due to metadata migration error: {error}"), + Meta::Message(format!( + "Clearing cache due to metadata migration error: {error}" + )), ), None => ( InvalidMetadataAction::DeleteLayer, - format!( + Meta::Message(format!( "Clearing cache due to invalid metadata ({toml})", toml = toml.trim() - ), + )), ), }, Err(error) => ( InvalidMetadataAction::DeleteLayer, - format!("Clearing cache due to invalid metadata serialization error: {error}"), + Meta::Message(format!( + "Clearing cache due to invalid metadata serialization error: {error}" + )), ), } } +/// Either contains metadata or a message describing the state +/// +/// Why: The `CachedLayerDefinition` allows returning information about the cache state +/// from either `invalid_metadata_action` or `restored_layer_action` functions. +/// +/// Because the function returns only a single type, that type must be the same for +/// all possible cache conditions (cleared or retained). Therefore, the type must be +/// able to represent information about the cache state when it's cleared or not. +/// +/// This struct implements `Display` and `AsRef` so if the end user only +/// wants to advertise the cache state, they can do so by passing the whole struct +/// to `format!` or `println!` without any further maniuplation. If they need +/// to inspect the previous metadata they can match on the enum and extract +/// what they need. +/// +/// - Will only ever contain metadata when the cache is retained. +/// - Will contain a message when the cache is cleared, describing why it was cleared. +/// It is also allowable to return a message when the cache is retained, and the +/// message describes the state of the cache. (i.e. because a message is returned +/// does not guarantee the cache was cleared). +pub(crate) enum Meta { + Message(String), + Data(M), +} + +impl std::fmt::Display for Meta { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.as_ref()) + } +} + +impl AsRef for Meta { + fn as_ref(&self) -> &str { + match self { + Meta::Message(s) => s.as_str(), + Meta::Data(_) => "Using cache", + } + } +} + /// Removes ANSI control characters from a string #[cfg(test)] pub(crate) fn strip_ansi(input: impl AsRef) -> String { @@ -162,7 +206,7 @@ mod tests { use std::convert::Infallible; /// Struct for asserting the behavior of `cached_layer_write_metadata` - #[derive(Debug, serde::Serialize, serde::Deserialize)] + #[derive(Debug, serde::Serialize, serde::Deserialize, Clone)] struct TestMetadata { value: String, } @@ -210,7 +254,7 @@ mod tests { let LayerState::Restored { cause } = &result.state else { panic!("Expected restored layer") }; - assert_eq!(cause, "Using cache"); + assert_eq!(cause.as_ref(), "Using cache"); // Third write, change the data let result = cached_layer_write_metadata( @@ -229,19 +273,19 @@ mod tests { panic!("Expected empty layer with restored layer action"); }; assert_eq!( - cause, + cause.as_ref(), "Clearing cache due to change: value (hello to world)" ); } /// Struct for asserting the behavior of `invalid_metadata_action` - #[derive(serde::Deserialize, serde::Serialize, Debug)] + #[derive(serde::Deserialize, serde::Serialize, Debug, Clone)] #[serde(deny_unknown_fields)] struct PersonV1 { name: String, } /// Struct for asserting the behavior of `invalid_metadata_action` - #[derive(serde::Deserialize, serde::Serialize, Debug)] + #[derive(serde::Deserialize, serde::Serialize, Debug, Clone)] #[serde(deny_unknown_fields)] struct PersonV2 { name: String, @@ -289,15 +333,15 @@ mod tests { name: "schneems".to_string(), }); assert!(matches!(action, InvalidMetadataAction::ReplaceMetadata(_))); - assert_eq!(message, "Replaced metadata".to_string()); + assert_eq!(message.as_ref(), "Using cache"); let (action, message) = invalid_metadata_action::(&PersonV1 { name: "not_richard".to_string(), }); assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); assert_eq!( - message, - "Clearing cache due to metadata migration error: Not Richard".to_string() + message.as_ref(), + "Clearing cache due to metadata migration error: Not Richard" ); let (action, message) = invalid_metadata_action::(&TestMetadata { @@ -305,10 +349,9 @@ mod tests { }); assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); assert_eq!( - message, - "Clearing cache due to invalid metadata (value = \"world\")".to_string() + message.as_ref(), + "Clearing cache due to invalid metadata (value = \"world\")" ); - // Unable to produce this error at will: "Clearing cache due to invalid metadata serialization error: {error}" } } diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index ca481d47..a20ce457 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -9,14 +9,11 @@ use core::str::FromStr; use fs_err::PathExt; use fun_run::CmdError; use layers::{ - bundle_install_layer::{BundleInstallLayer, BundleInstallLayerMetadata}, - metrics_agent_install::MetricsAgentInstallError, - ruby_install_layer::RubyInstallError, + metrics_agent_install::MetricsAgentInstallError, ruby_install_layer::RubyInstallError, }; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; use libcnb::data::build_plan::BuildPlanBuilder; use libcnb::data::launch::LaunchBuilder; -use libcnb::data::layer_name; use libcnb::detect::{DetectContext, DetectResult, DetectResultBuilder}; use libcnb::generic::{GenericMetadata, GenericPlatform}; use libcnb::layer_env::Scope; @@ -116,7 +113,8 @@ impl Buildpack for RubyBuildpack { #[allow(clippy::too_many_lines)] #[allow(deprecated)] fn build(&self, context: BuildContext) -> libcnb::Result { - let mut logger = BuildLog::new(stdout()).buildpack_name("Heroku Ruby Buildpack"); + let mut build_output = Print::new(stdout()).h2("Heroku Ruby Buildpack"); + let logger = BuildLog::new(stdout()).without_buildpack_name(); let warn_later = WarnGuard::new(stdout()); // ## Set default environment @@ -131,7 +129,6 @@ impl Buildpack for RubyBuildpack { let bundler_version = gemfile_lock.resolve_bundler("2.4.5"); let ruby_version = gemfile_lock.resolve_ruby("3.1.3"); - let mut build_output = Print::new(stdout()).without_header(); // ## Install metrics agent build_output = { let bullet = build_output.bullet("Metrics agent"); @@ -169,7 +166,7 @@ impl Buildpack for RubyBuildpack { }; // ## Setup bundler - (_, env) = { + (build_output, env) = { let bullet = build_output.bullet(format!( "Bundler version {} from {}", style::value(bundler_version.to_string()), @@ -188,39 +185,37 @@ impl Buildpack for RubyBuildpack { }; // ## Bundle install - (logger, env) = { - let section = logger.section("Bundle install"); - let bundle_install_layer = context.handle_layer( - layer_name!("gems"), - BundleInstallLayer { - env: env.clone(), - without: BundleWithout::new("development:test"), - _section_log: section.as_ref(), - metadata: BundleInstallLayerMetadata { - distro_name: context.target.distro_name.clone(), - distro_version: context.target.distro_version.clone(), - cpu_architecture: context.target.arch.clone(), - ruby_version: ruby_version.clone(), - force_bundle_install_key: String::from( - crate::layers::bundle_install_layer::FORCE_BUNDLE_INSTALL_CACHE_KEY, - ), - digest: MetadataDigest::new_env_files( - &context.platform, - &[ - &context.app_dir.join("Gemfile"), - &context.app_dir.join("Gemfile.lock"), - ], - ) - .map_err(|error| match error { - commons::metadata_digest::DigestError::CannotReadFile(path, error) => { - RubyBuildpackError::BundleInstallDigestError(path, error) - } - })?, - }, + (_, env) = { + let bullet = build_output.bullet("Bundle install gems"); + let (bullet, layer_env) = layers::bundle_install_layer::handle( + &context, + &env, + bullet, + &layers::bundle_install_layer::Metadata { + distro_name: context.target.distro_name.clone(), + distro_version: context.target.distro_version.clone(), + cpu_architecture: context.target.arch.clone(), + ruby_version: ruby_version.clone(), + force_bundle_install_key: String::from( + crate::layers::bundle_install_layer::FORCE_BUNDLE_INSTALL_CACHE_KEY, + ), + digest: MetadataDigest::new_env_files( + &context.platform, + &[ + &context.app_dir.join("Gemfile"), + &context.app_dir.join("Gemfile.lock"), + ], + ) + .map_err(|error| match error { + commons::metadata_digest::DigestError::CannotReadFile(path, error) => { + RubyBuildpackError::BundleInstallDigestError(path, error) + } + })?, }, + &BundleWithout::new("development:test"), )?; - let env = bundle_install_layer.env.apply(Scope::Build, &env); - (section.end_section(), env) + + (bullet.done(), layer_env.apply(Scope::Build, &env)) }; // ## Detect gems