From af5331afa65a8102eeec2edb04ce15351097a288 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 18:46:29 -0500 Subject: [PATCH 01/14] Print all changes on cache change The `Changed` struct limits us to a single reason for a change. By using a vec we can list all changes. There will be some redundancy in the output if multiple layers are keyed to the same values (such as distro name or distro version). That might be okay, or we could try storing common information in its own layer up front, and printing that information once (the specific values) and then later we could say "distro version changed" rather than "distro verision changed (22.04 to 24.04)". --- .../ruby/src/layers/ruby_install_layer.rs | 117 ++++++++---------- 1 file changed, 53 insertions(+), 64 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 023a8828..a16c460d 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -1,3 +1,4 @@ +use commons::display::SentenceList; use commons::output::{ fmt::{self}, section_log::{log_step, log_step_timed, SectionLogger}, @@ -159,76 +160,64 @@ impl<'a> Layer for RubyInstallLayer<'a> { let old = &layer_data.content_metadata.metadata; let now = self.metadata.clone(); - match cache_state(old.clone(), now) { - Changed::Nothing => { - log_step("Using cached Ruby version"); - - Ok(ExistingLayerStrategy::Keep) - } - Changed::CpuArchitecture(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("CPU architecture changed: {old} to {now}")) - )); - - Ok(ExistingLayerStrategy::Recreate) - } - Changed::DistroVersion(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("distro version changed: {old} to {now}")) - )); - - Ok(ExistingLayerStrategy::Recreate) - } - Changed::DistroName(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("distro name changed: {old} to {now}")) - )); - - Ok(ExistingLayerStrategy::Recreate) - } - Changed::RubyVersion(old, now) => { - log_step(format!( - "Clearing cache {}", - fmt::details(format!("Ruby version changed: {old} to {now}")) - )); - - Ok(ExistingLayerStrategy::Recreate) - } + if let Some(differences) = metadata_diff(old, &now) { + log_step(format!( + "Clearing cache {}", + SentenceList::new(&differences) + )); + Ok(ExistingLayerStrategy::Recreate) + } else { + log_step("Using cached Ruby version"); + Ok(ExistingLayerStrategy::Keep) } } } -fn cache_state(old: RubyInstallLayerMetadata, now: RubyInstallLayerMetadata) -> Changed { - let RubyInstallLayerMetadata { - distro_name, - distro_version, - cpu_architecture, - ruby_version, - } = now; - - 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) +fn metadata_diff( + old: &RubyInstallLayerMetadata, + metadata: &RubyInstallLayerMetadata, +) -> Option> { + if old == metadata { + None } else { - Changed::Nothing - } -} + let mut differences = Vec::new(); + let RubyInstallLayerMetadata { + distro_name, + distro_version, + cpu_architecture, + ruby_version, + } = old; + if ruby_version != &metadata.ruby_version { + differences.push(format!( + "Ruby version changed: ({ruby_version} to {})", + metadata.ruby_version + )); + } + if distro_name != &metadata.distro_name { + differences.push(format!( + "distro name changed: ({distro_name} to {})", + metadata.distro_name + )); + } + if distro_version != &metadata.distro_version { + differences.push(format!( + "distro version changed ({distro_version} to {})", + metadata.distro_version + )); + } + if cpu_architecture != &metadata.cpu_architecture { + differences.push(format!( + "CPU architecture changed ({cpu_architecture} to {})", + metadata.cpu_architecture + )); + } -#[derive(Debug)] -enum Changed { - Nothing, - DistroName(String, String), - DistroVersion(String, String), - CpuArchitecture(String, String), - RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion), + if differences.is_empty() { + None + } else { + Some(differences) + } + } } fn download_url( From 3976f735a883389282a7809f6bace3bf4853b486 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 18:51:02 -0500 Subject: [PATCH 02/14] Format cache changed values --- .../ruby/src/layers/ruby_install_layer.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index a16c460d..54919f18 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -189,26 +189,30 @@ fn metadata_diff( } = old; if ruby_version != &metadata.ruby_version { differences.push(format!( - "Ruby version changed: ({ruby_version} to {})", - metadata.ruby_version + "Ruby version changed: ({old} to {now})", + old = fmt::value(ruby_version.to_string()), + now = fmt::value(metadata.ruby_version.to_string()) )); } if distro_name != &metadata.distro_name { differences.push(format!( - "distro name changed: ({distro_name} to {})", - metadata.distro_name + "distro name changed: ({old} to {now})", + old = fmt::value(distro_name), + now = fmt::value(&metadata.distro_name) )); } if distro_version != &metadata.distro_version { differences.push(format!( - "distro version changed ({distro_version} to {})", - metadata.distro_version + "distro version changed ({old} to {now})", + old = fmt::value(distro_version), + now = fmt::value(&metadata.distro_version) )); } if cpu_architecture != &metadata.cpu_architecture { differences.push(format!( - "CPU architecture changed ({cpu_architecture} to {})", - metadata.cpu_architecture + "CPU architecture changed ({old} to {now})", + old = fmt::value(cpu_architecture), + now = fmt::value(&metadata.cpu_architecture) )); } From 590cd99919dab5a0f5fe7c16f389e94a92b03fe7 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 18:53:33 -0500 Subject: [PATCH 03/14] Rename RubyInstallLayerMetadata to Metadata --- .../ruby/src/layers/ruby_install_layer.rs | 38 +++++++++---------- buildpacks/ruby/src/main.rs | 4 +- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 54919f18..c3b4b19b 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -38,24 +38,25 @@ use url::Url; /// pub(crate) struct RubyInstallLayer<'a> { pub(crate) _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer - pub(crate) metadata: RubyInstallLayerMetadata, + pub(crate) metadata: Metadata, } #[derive(Deserialize, Serialize, Debug, Clone)] -pub(crate) struct RubyInstallLayerMetadataV1 { +pub(crate) struct MetadataV1 { pub(crate) stack: String, pub(crate) version: ResolvedRubyVersion, } #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] -pub(crate) struct RubyInstallLayerMetadataV2 { +pub(crate) struct MetadataV2 { pub(crate) distro_name: String, pub(crate) distro_version: String, pub(crate) cpu_architecture: String, pub(crate) ruby_version: ResolvedRubyVersion, } +pub(crate) type Metadata = MetadataV2; -impl RubyInstallLayerMetadataV2 { +impl MetadataV2 { pub(crate) fn target_id(&self) -> TargetId { TargetId { cpu_architecture: self.cpu_architecture.clone(), @@ -66,11 +67,10 @@ impl RubyInstallLayerMetadataV2 { } try_migrate_deserializer_chain!( - chain: [RubyInstallLayerMetadataV1, RubyInstallLayerMetadataV2], + chain: [MetadataV1, MetadataV2], error: MetadataMigrateError, deserializer: toml::Deserializer::new, ); -pub(crate) type RubyInstallLayerMetadata = RubyInstallLayerMetadataV2; #[derive(thiserror::Error, Debug)] pub(crate) enum MetadataMigrateError { @@ -78,10 +78,10 @@ pub(crate) enum MetadataMigrateError { TargetIdError(TargetIdError), } -impl TryFrom for RubyInstallLayerMetadataV2 { +impl TryFrom for MetadataV2 { type Error = MetadataMigrateError; - fn try_from(v1: RubyInstallLayerMetadataV1) -> Result { + fn try_from(v1: MetadataV1) -> Result { let target_id = TargetId::from_stack(&v1.stack).map_err(MetadataMigrateError::TargetIdError)?; @@ -97,7 +97,7 @@ impl TryFrom for RubyInstallLayerMetadataV2 { #[allow(deprecated)] impl<'a> Layer for RubyInstallLayer<'a> { type Buildpack = RubyBuildpack; - type Metadata = RubyInstallLayerMetadata; + type Metadata = Metadata; fn types(&self) -> LayerTypes { LayerTypes { @@ -173,15 +173,12 @@ impl<'a> Layer for RubyInstallLayer<'a> { } } -fn metadata_diff( - old: &RubyInstallLayerMetadata, - metadata: &RubyInstallLayerMetadata, -) -> Option> { +fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option> { if old == metadata { None } else { let mut differences = Vec::new(); - let RubyInstallLayerMetadata { + let Metadata { distro_name, distro_version, cpu_architecture, @@ -313,7 +310,7 @@ mod tests { /// be built from the previous version. #[test] fn metadata_guard() { - let metadata = RubyInstallLayerMetadata { + let metadata = Metadata { distro_name: String::from("ubuntu"), distro_version: String::from("22.04"), cpu_architecture: String::from("amd64"), @@ -333,7 +330,7 @@ ruby_version = "3.1.3" #[test] fn metadata_migrate_v1_to_v2() { - let metadata = RubyInstallLayerMetadataV1 { + let metadata = MetadataV1 { stack: String::from("heroku-22"), version: ResolvedRubyVersion(String::from("3.1.3")), }; @@ -346,13 +343,12 @@ version = "3.1.3" .trim(); assert_eq!(expected, actual.trim()); - let deserialized: RubyInstallLayerMetadataV2 = - RubyInstallLayerMetadataV2::try_from_str_migrations(&actual) - .unwrap() - .unwrap(); + let deserialized: MetadataV2 = MetadataV2::try_from_str_migrations(&actual) + .unwrap() + .unwrap(); let target_id = TargetId::from_stack(&metadata.stack).unwrap(); - let expected = RubyInstallLayerMetadataV2 { + 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/main.rs b/buildpacks/ruby/src/main.rs index ce6dac44..7ac69175 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -12,7 +12,7 @@ use layers::{ bundle_download_layer::{BundleDownloadLayer, BundleDownloadLayerMetadata}, bundle_install_layer::{BundleInstallLayer, BundleInstallLayerMetadata}, metrics_agent_install::MetricsAgentInstallError, - ruby_install_layer::{RubyInstallError, RubyInstallLayer, RubyInstallLayerMetadata}, + ruby_install_layer::{Metadata, RubyInstallError, RubyInstallLayer}, }; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; use libcnb::data::build_plan::BuildPlanBuilder; @@ -160,7 +160,7 @@ impl Buildpack for RubyBuildpack { layer_name!("ruby"), RubyInstallLayer { _in_section: section.as_ref(), - metadata: RubyInstallLayerMetadata { + metadata: layers::ruby_install_layer::Metadata { distro_name: context.target.distro_name.clone(), distro_version: context.target.distro_version.clone(), cpu_architecture: context.target.arch.clone(), From 35e6258ac63af7ba0dc5a907d8a2e727ff119218 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 19:13:57 -0500 Subject: [PATCH 04/14] Replace Ruby layer trait with layer struct API --- .../ruby/src/layers/ruby_install_layer.rs | 195 +++++++++--------- buildpacks/ruby/src/main.rs | 28 ++- 2 files changed, 107 insertions(+), 116 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index c3b4b19b..789003fd 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -1,8 +1,26 @@ +//! # Install Ruby version +//! +//! ## Layer dir +//! +//! The compiled Ruby tgz file is downloaded to a temporary directory and exported to ``. +//! The tgz already contains a `bin/` directory with a `ruby` executable file. +//! +//! This layer relies on the CNB lifecycle to add `/bin` to the PATH. +//! +//! ## Cache invalidation +//! +//! When the Ruby version changes, invalidate and re-run. +//! use commons::display::SentenceList; use commons::output::{ fmt::{self}, - section_log::{log_step, log_step_timed, SectionLogger}, + section_log::{log_step, log_step_timed}, }; +use libcnb::data::layer_name; +use libcnb::layer::{ + CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, +}; +use libcnb::layer_env::LayerEnv; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; use crate::{ @@ -11,10 +29,6 @@ use crate::{ }; use commons::gemfile_lock::ResolvedRubyVersion; use flate2::read::GzDecoder; -use libcnb::build::BuildContext; -use libcnb::data::layer_content_metadata::LayerTypes; -#[allow(deprecated)] -use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder}; use serde::{Deserialize, Deserializer, Serialize}; use std::convert::Infallible; use std::io; @@ -23,22 +37,82 @@ use tar::Archive; use tempfile::NamedTempFile; use url::Url; -/// # Install Ruby version -/// -/// ## Layer dir -/// -/// The compiled Ruby tgz file is downloaded to a temporary directory and exported to ``. -/// The tgz already contains a `bin/` directory with a `ruby` executable file. -/// -/// This layer relies on the CNB lifecycle to add `/bin` to the PATH. -/// -/// ## Cache invalidation -/// -/// When the Ruby version changes, invalidate and re-run. -/// -pub(crate) struct RubyInstallLayer<'a> { - pub(crate) _in_section: &'a dyn SectionLogger, // force the layer to be called within a Section logging context, not necessary but it's safer - pub(crate) metadata: Metadata, +pub(crate) fn handle( + context: &libcnb::build::BuildContext, + metadata: Metadata, +) -> libcnb::Result { + // TODO: Replace when implementing bullet_stream. + let layer_ref = context.cached_layer( + layer_name!("ruby"), + CachedLayerDefinition { + build: true, + launch: true, + invalid_metadata_action: &|old| match Metadata::try_from_str_migrations( + &toml::to_string(old).expect("TOML deserialization of GenericMetadata"), + ) { + Some(Ok(migrated)) => ( + InvalidMetadataAction::ReplaceMetadata(migrated), + "replaced metadata".to_string(), + ), + Some(Err(error)) => ( + InvalidMetadataAction::DeleteLayer, + format!("metadata migration error {error}"), + ), + None => ( + InvalidMetadataAction::DeleteLayer, + "invalid metadata".to_string(), + ), + }, + restored_layer_action: &|old: &Metadata, _| { + if let Some(diff) = metadata_diff(old, &metadata) { + ( + RestoredLayerAction::DeleteLayer, + SentenceList::new(&diff).to_string(), + ) + } else { + ( + RestoredLayerAction::KeepLayer, + "using cached version".to_string(), + ) + } + }, + }, + )?; + match &layer_ref.state { + LayerState::Restored { cause: _ } => { + log_step("Using cached Ruby version"); + } + LayerState::Empty { cause } => { + match cause { + EmptyLayerCause::NewlyCreated => {} + EmptyLayerCause::InvalidMetadataAction { cause } + | EmptyLayerCause::RestoredLayerAction { cause } => { + log_step(format!("Clearing cache {cause}")); + } + } + install_ruby(&metadata, &layer_ref.path())?; + } + } + layer_ref.write_metadata(metadata)?; + layer_ref.read_env() +} + +fn install_ruby(metadata: &Metadata, layer_path: &Path) -> Result<(), RubyBuildpackError> { + log_step_timed("Installing", || { + let tmp_ruby_tgz = NamedTempFile::new() + .map_err(RubyInstallError::CouldNotCreateDestinationFile) + .map_err(RubyBuildpackError::RubyInstallError)?; + + let url = download_url(&metadata.target_id(), &metadata.ruby_version) + .map_err(RubyBuildpackError::RubyInstallError)?; + + download(url.as_ref(), tmp_ruby_tgz.path()) + .map_err(RubyBuildpackError::RubyInstallError)?; + + untar(tmp_ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyInstallError)?; + + Ok(()) + }) } #[derive(Deserialize, Serialize, Debug, Clone)] @@ -94,85 +168,6 @@ impl TryFrom for MetadataV2 { } } -#[allow(deprecated)] -impl<'a> Layer for RubyInstallLayer<'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> { - log_step_timed("Installing", || { - let tmp_ruby_tgz = NamedTempFile::new() - .map_err(RubyInstallError::CouldNotCreateDestinationFile) - .map_err(RubyBuildpackError::RubyInstallError)?; - - let url = download_url(&self.metadata.target_id(), &self.metadata.ruby_version) - .map_err(RubyBuildpackError::RubyInstallError)?; - - download(url.as_ref(), tmp_ruby_tgz.path()) - .map_err(RubyBuildpackError::RubyInstallError)?; - - untar(tmp_ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyInstallError)?; - - LayerResultBuilder::new(self.metadata.clone()).build() - }) - } - - 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) - } - } - } - - fn existing_layer_strategy( - &mut self, - _context: &BuildContext, - layer_data: &LayerData, - ) -> Result { - let old = &layer_data.content_metadata.metadata; - let now = self.metadata.clone(); - - if let Some(differences) = metadata_diff(old, &now) { - log_step(format!( - "Clearing cache {}", - SentenceList::new(&differences) - )); - Ok(ExistingLayerStrategy::Recreate) - } else { - log_step("Using cached Ruby version"); - Ok(ExistingLayerStrategy::Keep) - } - } -} - fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option> { if old == metadata { None diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 7ac69175..79a5b97c 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -12,7 +12,7 @@ use layers::{ bundle_download_layer::{BundleDownloadLayer, BundleDownloadLayerMetadata}, bundle_install_layer::{BundleInstallLayer, BundleInstallLayerMetadata}, metrics_agent_install::MetricsAgentInstallError, - ruby_install_layer::{Metadata, RubyInstallError, RubyInstallLayer}, + ruby_install_layer::RubyInstallError, }; use libcnb::build::{BuildContext, BuildResult, BuildResultBuilder}; use libcnb::data::build_plan::BuildPlanBuilder; @@ -155,21 +155,17 @@ impl Buildpack for RubyBuildpack { fmt::value(ruby_version.to_string()), fmt::value(gemfile_lock.ruby_source()) )); - let ruby_layer = context // - .handle_layer( - layer_name!("ruby"), - RubyInstallLayer { - _in_section: section.as_ref(), - metadata: layers::ruby_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(), - }, - }, - )?; - let env = ruby_layer.env.apply(Scope::Build, &env); - (section.end_section(), env) + let layer_env = layers::ruby_install_layer::handle( + &context, + layers::ruby_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(), + }, + )?; + + (section.end_section(), layer_env.apply(Scope::Build, &env)) }; // ## Setup bundler From 0038b56dc52a1ad7ac2a054365079254eda9849d Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 19:18:06 -0500 Subject: [PATCH 05/14] Update RubyInstallLayer signature to accept and return bullet stream Changing the logging will be it's own commit. --- .../ruby/src/layers/ruby_install_layer.rs | 9 ++++++--- buildpacks/ruby/src/main.rs | 17 +++++++++-------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 789003fd..0f379570 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,6 +11,8 @@ //! //! When the Ruby version changes, invalidate and re-run. //! +use bullet_stream::state::SubBullet; +use bullet_stream::Print; use commons::display::SentenceList; use commons::output::{ fmt::{self}, @@ -31,7 +33,7 @@ use commons::gemfile_lock::ResolvedRubyVersion; use flate2::read::GzDecoder; use serde::{Deserialize, Deserializer, Serialize}; use std::convert::Infallible; -use std::io; +use std::io::{self, Stdout}; use std::path::Path; use tar::Archive; use tempfile::NamedTempFile; @@ -39,8 +41,9 @@ use url::Url; pub(crate) fn handle( context: &libcnb::build::BuildContext, + mut bullet: Print>, metadata: Metadata, -) -> libcnb::Result { +) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { // TODO: Replace when implementing bullet_stream. let layer_ref = context.cached_layer( layer_name!("ruby"), @@ -94,7 +97,7 @@ pub(crate) fn handle( } } layer_ref.write_metadata(metadata)?; - layer_ref.read_env() + Ok((bullet, layer_ref.read_env()?)) } fn install_ruby(metadata: &Metadata, layer_path: &Path) -> Result<(), RubyBuildpackError> { diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 79a5b97c..141e33e8 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -132,9 +132,9 @@ 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 build_output = Print::new(stdout()).without_header(); + let mut build_output = Print::new(stdout()).without_header(); // ## Install metrics agent - _ = { + build_output = { let bullet = build_output.bullet("Metrics agent"); if lockfile_contents.contains("barnes") { layers::metrics_agent_install::handle_metrics_agent_layer(&context, bullet)?.done() @@ -149,14 +149,15 @@ impl Buildpack for RubyBuildpack { }; // ## Install executable ruby version - (logger, env) = { - let section = logger.section(&format!( + (_, env) = { + let bullet = build_output.bullet(format!( "Ruby version {} from {}", - fmt::value(ruby_version.to_string()), - fmt::value(gemfile_lock.ruby_source()) + style::value(ruby_version.to_string()), + style::value(gemfile_lock.ruby_source()) )); - let layer_env = layers::ruby_install_layer::handle( + let (bullet, layer_env) = layers::ruby_install_layer::handle( &context, + bullet, layers::ruby_install_layer::Metadata { distro_name: context.target.distro_name.clone(), distro_version: context.target.distro_version.clone(), @@ -165,7 +166,7 @@ impl Buildpack for RubyBuildpack { }, )?; - (section.end_section(), layer_env.apply(Scope::Build, &env)) + (bullet.done(), layer_env.apply(Scope::Build, &env)) }; // ## Setup bundler From 27c5ac10fde93bcf987e86004974993691ca2d9b Mon Sep 17 00:00:00 2001 From: Schneems Date: Sun, 22 Sep 2024 19:23:34 -0500 Subject: [PATCH 06/14] Switch Ruby Install build output to bullet stream --- .../ruby/src/layers/ruby_install_layer.rs | 48 ++++++++----------- 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 0f379570..0d36d692 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -12,12 +12,8 @@ //! When the Ruby version changes, invalidate and re-run. //! use bullet_stream::state::SubBullet; -use bullet_stream::Print; +use bullet_stream::{style, Print}; use commons::display::SentenceList; -use commons::output::{ - fmt::{self}, - section_log::{log_step, log_step_timed}, -}; use libcnb::data::layer_name; use libcnb::layer::{ CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, @@ -44,7 +40,6 @@ pub(crate) fn handle( mut bullet: Print>, metadata: Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - // TODO: Replace when implementing bullet_stream. let layer_ref = context.cached_layer( layer_name!("ruby"), CachedLayerDefinition { @@ -83,17 +78,19 @@ pub(crate) fn handle( )?; match &layer_ref.state { LayerState::Restored { cause: _ } => { - log_step("Using cached Ruby version"); + bullet = bullet.sub_bullet("Using cached Ruby version"); } LayerState::Empty { cause } => { match cause { EmptyLayerCause::NewlyCreated => {} EmptyLayerCause::InvalidMetadataAction { cause } | EmptyLayerCause::RestoredLayerAction { cause } => { - log_step(format!("Clearing cache {cause}")); + bullet = bullet.sub_bullet(format!("Clearing cache {cause}")); } } + let timer = bullet.start_timer("Installing"); install_ruby(&metadata, &layer_ref.path())?; + bullet = timer.done(); } } layer_ref.write_metadata(metadata)?; @@ -101,21 +98,18 @@ pub(crate) fn handle( } fn install_ruby(metadata: &Metadata, layer_path: &Path) -> Result<(), RubyBuildpackError> { - log_step_timed("Installing", || { - let tmp_ruby_tgz = NamedTempFile::new() - .map_err(RubyInstallError::CouldNotCreateDestinationFile) - .map_err(RubyBuildpackError::RubyInstallError)?; + let tmp_ruby_tgz = NamedTempFile::new() + .map_err(RubyInstallError::CouldNotCreateDestinationFile) + .map_err(RubyBuildpackError::RubyInstallError)?; - let url = download_url(&metadata.target_id(), &metadata.ruby_version) - .map_err(RubyBuildpackError::RubyInstallError)?; + let url = download_url(&metadata.target_id(), &metadata.ruby_version) + .map_err(RubyBuildpackError::RubyInstallError)?; - download(url.as_ref(), tmp_ruby_tgz.path()) - .map_err(RubyBuildpackError::RubyInstallError)?; + download(url.as_ref(), tmp_ruby_tgz.path()).map_err(RubyBuildpackError::RubyInstallError)?; - untar(tmp_ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyInstallError)?; + untar(tmp_ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyInstallError)?; - Ok(()) - }) + Ok(()) } #[derive(Deserialize, Serialize, Debug, Clone)] @@ -185,29 +179,29 @@ fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option> { if ruby_version != &metadata.ruby_version { differences.push(format!( "Ruby version changed: ({old} to {now})", - old = fmt::value(ruby_version.to_string()), - now = fmt::value(metadata.ruby_version.to_string()) + old = style::value(ruby_version.to_string()), + now = style::value(metadata.ruby_version.to_string()) )); } if distro_name != &metadata.distro_name { differences.push(format!( "distro name changed: ({old} to {now})", - old = fmt::value(distro_name), - now = fmt::value(&metadata.distro_name) + old = style::value(distro_name), + now = style::value(&metadata.distro_name) )); } if distro_version != &metadata.distro_version { differences.push(format!( "distro version changed ({old} to {now})", - old = fmt::value(distro_version), - now = fmt::value(&metadata.distro_version) + old = style::value(distro_version), + now = style::value(&metadata.distro_version) )); } if cpu_architecture != &metadata.cpu_architecture { differences.push(format!( "CPU architecture changed ({old} to {now})", - old = fmt::value(cpu_architecture), - now = fmt::value(&metadata.cpu_architecture) + old = style::value(cpu_architecture), + now = style::value(&metadata.cpu_architecture) )); } From 2de1c3eaecb4aa49a6d996185156940191ed10bf Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 14:48:35 -0500 Subject: [PATCH 07/14] Reduce repetition in change output --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 0d36d692..150c0edc 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -65,7 +65,7 @@ pub(crate) fn handle( if let Some(diff) = metadata_diff(old, &metadata) { ( RestoredLayerAction::DeleteLayer, - SentenceList::new(&diff).to_string(), + format!("due to change(s): {}", SentenceList::new(&diff)), ) } else { ( @@ -178,28 +178,28 @@ fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option> { } = old; if ruby_version != &metadata.ruby_version { differences.push(format!( - "Ruby version changed: ({old} to {now})", + "Ruby version ({old} to {now})", old = style::value(ruby_version.to_string()), now = style::value(metadata.ruby_version.to_string()) )); } if distro_name != &metadata.distro_name { differences.push(format!( - "distro name changed: ({old} to {now})", + "distro name ({old} to {now})", old = style::value(distro_name), now = style::value(&metadata.distro_name) )); } if distro_version != &metadata.distro_version { differences.push(format!( - "distro version changed ({old} to {now})", + "distro version ({old} to {now})", old = style::value(distro_version), now = style::value(&metadata.distro_version) )); } if cpu_architecture != &metadata.cpu_architecture { differences.push(format!( - "CPU architecture changed ({old} to {now})", + "CPU architecture ({old} to {now})", old = style::value(cpu_architecture), now = style::value(&metadata.cpu_architecture) )); From 7110b136993e7400f21312d9b5f1d2275ef434a8 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 14:57:47 -0500 Subject: [PATCH 08/14] Remove redundant logic The explicit check if the old and new metadata structs were equal isn't needed since I'm already comparing each individual element and have logic to return `None` if they're the same. --- .../ruby/src/layers/ruby_install_layer.rs | 80 +++++++++---------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 150c0edc..3e08f1c2 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -166,50 +166,46 @@ impl TryFrom for MetadataV2 { } fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option> { - if old == metadata { + let mut differences = Vec::new(); + let Metadata { + distro_name, + distro_version, + cpu_architecture, + ruby_version, + } = old; + if ruby_version != &metadata.ruby_version { + differences.push(format!( + "Ruby version ({old} to {now})", + old = style::value(ruby_version.to_string()), + now = style::value(metadata.ruby_version.to_string()) + )); + } + if distro_name != &metadata.distro_name { + differences.push(format!( + "distro name ({old} to {now})", + old = style::value(distro_name), + now = style::value(&metadata.distro_name) + )); + } + if distro_version != &metadata.distro_version { + differences.push(format!( + "distro version ({old} to {now})", + old = style::value(distro_version), + now = style::value(&metadata.distro_version) + )); + } + if cpu_architecture != &metadata.cpu_architecture { + differences.push(format!( + "CPU architecture ({old} to {now})", + old = style::value(cpu_architecture), + now = style::value(&metadata.cpu_architecture) + )); + } + + if differences.is_empty() { None } else { - let mut differences = Vec::new(); - let Metadata { - distro_name, - distro_version, - cpu_architecture, - ruby_version, - } = old; - if ruby_version != &metadata.ruby_version { - differences.push(format!( - "Ruby version ({old} to {now})", - old = style::value(ruby_version.to_string()), - now = style::value(metadata.ruby_version.to_string()) - )); - } - if distro_name != &metadata.distro_name { - differences.push(format!( - "distro name ({old} to {now})", - old = style::value(distro_name), - now = style::value(&metadata.distro_name) - )); - } - if distro_version != &metadata.distro_version { - differences.push(format!( - "distro version ({old} to {now})", - old = style::value(distro_version), - now = style::value(&metadata.distro_version) - )); - } - if cpu_architecture != &metadata.cpu_architecture { - differences.push(format!( - "CPU architecture ({old} to {now})", - old = style::value(cpu_architecture), - now = style::value(&metadata.cpu_architecture) - )); - } - - if differences.is_empty() { - None - } else { - Some(differences) - } + Some(differences) } } From 0c0f3571a96f4a1c77f985709813a7881736fe12 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 15:29:49 -0500 Subject: [PATCH 09/14] Move struct docs to module docs --- 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..cab65ab3 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`. 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 85952e9555252f8e137c72c877774a158e525175 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 15:32:21 -0500 Subject: [PATCH 10/14] Rename metadata struct --- 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 cab65ab3..1a0aee3b 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 141e33e8..6bf41b46 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 fea5d41f7ddca5d943fbd44d84c50f56fb8efa5a Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 15:44:35 -0500 Subject: [PATCH 11/14] Replace metadata difference enum with Option --- .../ruby/src/layers/bundle_download_layer.rs | 42 +++++++------------ 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 1a0aee3b..619113e7 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -112,38 +112,28 @@ impl<'a> Layer for BundleDownloadLayer<'a> { 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) - } + if let Some(cause) = metadata_diff(old, &self.metadata) { + log_step(format!("Clearing cache due to change: {cause}")); + + Ok(ExistingLayerStrategy::Recreate) + } else { + log_step("Using cached version"); + Ok(ExistingLayerStrategy::Keep) } } } -// [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 +fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option { + let Metadata { version } = old; - if old.version == version { - State::NothingChanged(version) + if version == &metadata.version { + None } else { - State::BundlerVersionChanged(old.version, version) + Some(format!( + "Bundler version ({old} to {now})", + old = fmt::value(version.to_string()), + now = fmt::value(metadata.version.to_string()) + )) } } From c186298abd0d76ead5afa762c2a72cbf0efe92cb Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 16:21:46 -0500 Subject: [PATCH 12/14] Refactor command arguments Prefer format!(path.display) over `to_string_lossy`. --- .../ruby/src/layers/bundle_download_layer.rs | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 619113e7..97f21796 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -55,24 +55,19 @@ impl<'a> Layer for BundleDownloadLayer<'a> { 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); + cmd.args(["install", "bundler"]); + cmd.args(["--version", &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 + // Directory where bundler's contents will live + cmd.args(["--install-dir", &format!("{}", layer_path.display())]); + // Directory where `bundle` executable lives + cmd.args(["--bindir", &format!("{}", bin_dir.display())]); 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` From ce9fe1b9ebbcd7546b623d1bd2dc8221c5bc71a0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 16:17:13 -0500 Subject: [PATCH 13/14] Migrate from the Layer trait to the Struct API --- .../ruby/src/layers/bundle_download_layer.rs | 191 +++++++++--------- buildpacks/ruby/src/main.rs | 17 +- 2 files changed, 102 insertions(+), 106 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 97f21796..43071952 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -9,113 +9,114 @@ use crate::RubyBuildpackError; 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::{ + CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, +}; use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libcnb::Env; use serde::{Deserialize, Serialize}; -use std::path::Path; use std::process::Command; -#[derive(Deserialize, Serialize, Debug, Clone)] -pub(crate) struct Metadata { - pub(crate) version: ResolvedBundlerVersion, -} - -pub(crate) struct BundleDownloadLayer<'a> { - pub(crate) env: Env, - pub(crate) metadata: Metadata, - pub(crate) _section_logger: &'a dyn SectionLogger, -} - -#[allow(deprecated)] -impl<'a> Layer for BundleDownloadLayer<'a> { - type Buildpack = RubyBuildpack; - type Metadata = Metadata; - - fn types(&self) -> LayerTypes { - LayerTypes { +pub(crate) fn handle( + context: &libcnb::build::BuildContext, + env: &Env, + metadata: Metadata, +) -> libcnb::Result { + // TODO switch logging to bullet stream + let layer_ref = context.cached_layer( + layer_name!("bundler"), + CachedLayerDefinition { build: true, launch: true, - cache: true, - } - } - - fn create( - &mut self, - _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"]); - cmd.args(["--version", &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); - - // Directory where bundler's contents will live - cmd.args(["--install-dir", &format!("{}", layer_path.display())]); - // Directory where `bundle` executable lives - cmd.args(["--bindir", &format!("{}", bin_dir.display())]); - cmd.args([ - "--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)?; - - 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, + invalid_metadata_action: &|_| { + ( + InvalidMetadataAction::DeleteLayer, + "invalid metadata".to_string(), + ) + }, + restored_layer_action: &|old: &Metadata, _| { + if let Some(cause) = metadata_diff(old, &metadata) { + (RestoredLayerAction::DeleteLayer, cause) + } else { + ( + RestoredLayerAction::KeepLayer, + "using cached version".to_string(), ) - .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, - ), - ) - .build() - } - - fn existing_layer_strategy( - &mut self, - _context: &BuildContext, - layer_data: &LayerData, - ) -> Result { - let old = &layer_data.content_metadata.metadata; - if let Some(cause) = metadata_diff(old, &self.metadata) { - log_step(format!("Clearing cache due to change: {cause}")); - - Ok(ExistingLayerStrategy::Recreate) - } else { + } + }, + }, + )?; + + let bin_dir = layer_ref.path().join("bin"); + let gem_path = layer_ref.path(); + match &layer_ref.state { + LayerState::Restored { cause: _ } => { log_step("Using cached version"); - Ok(ExistingLayerStrategy::Keep) + } + LayerState::Empty { cause } => { + match cause { + EmptyLayerCause::NewlyCreated => {} + EmptyLayerCause::InvalidMetadataAction { cause } + | EmptyLayerCause::RestoredLayerAction { cause } => { + log_step(format!("Clearing cache {cause}")); + } + } + + let mut cmd = Command::new("gem"); + cmd.args(["install", "bundler"]); + cmd.args(["--version", &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); + + // Directory where bundler's contents will live + cmd.args(["--install-dir", &format!("{}", layer_ref.path().display())]); + // Directory where `bundle` executable lives + cmd.args(["--bindir", &format!("{}", bin_dir.display())]); + cmd.args([ + "--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)?; } } + layer_ref.write_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, + ), + )?; + layer_ref.write_metadata(metadata)?; + layer_ref.read_env() +} + +#[derive(Deserialize, Serialize, Debug, Clone)] +pub(crate) struct Metadata { + pub(crate) version: ResolvedBundlerVersion, } fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option { @@ -137,7 +138,7 @@ mod test { use super::*; /// If this test fails due to a change you'll need to implement - /// `migrate_incompatible_metadata` for the Layer trait + /// `invalid_metadata_action` #[test] fn metadata_guard() { let metadata = Metadata { diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 6bf41b46..7d58a8c4 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 e69311f690013039ccc82e99ba468f4804357244 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 23 Sep 2024 16:51:55 -0500 Subject: [PATCH 14/14] Update Bundle download logging to bullet stream --- .../ruby/src/layers/bundle_download_layer.rs | 30 ++++++++++--------- buildpacks/ruby/src/main.rs | 22 +++++++------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 43071952..7d4edc89 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -6,11 +6,9 @@ //! `/bin`. use crate::RubyBuildpack; use crate::RubyBuildpackError; +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::{ @@ -19,13 +17,15 @@ use libcnb::layer::{ use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libcnb::Env; use serde::{Deserialize, Serialize}; +use std::io::Stdout; use std::process::Command; pub(crate) fn handle( context: &libcnb::build::BuildContext, + mut bullet: Print>, env: &Env, metadata: Metadata, -) -> libcnb::Result { +) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { // TODO switch logging to bullet stream let layer_ref = context.cached_layer( layer_name!("bundler"), @@ -55,14 +55,14 @@ pub(crate) fn handle( let gem_path = layer_ref.path(); match &layer_ref.state { LayerState::Restored { cause: _ } => { - log_step("Using cached version"); + bullet = bullet.sub_bullet("Using cached version"); } LayerState::Empty { cause } => { match cause { EmptyLayerCause::NewlyCreated => {} EmptyLayerCause::InvalidMetadataAction { cause } | EmptyLayerCause::RestoredLayerAction { cause } => { - log_step(format!("Clearing cache {cause}")); + bullet = bullet.sub_bullet(format!("Clearing cache {cause}")); } } @@ -85,12 +85,13 @@ pub(crate) fn handle( "--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| { + 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)?; + .map_err(RubyBuildpackError::GemInstallBundlerCommandError)?; + bullet = timer.done(); } } layer_ref.write_env( @@ -111,7 +112,8 @@ pub(crate) fn handle( ), )?; layer_ref.write_metadata(metadata)?; - layer_ref.read_env() + + Ok((bullet, layer_ref.read_env()?)) } #[derive(Deserialize, Serialize, Debug, Clone)] @@ -127,8 +129,8 @@ fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Option { } else { Some(format!( "Bundler version ({old} to {now})", - old = fmt::value(version.to_string()), - now = fmt::value(metadata.version.to_string()) + old = style::value(version.to_string()), + now = style::value(metadata.version.to_string()) )) } } diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 7d58a8c4..55ae271c 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,21 @@ impl Buildpack for RubyBuildpack { }; // ## Setup bundler - (logger, env) = { - let section = logger.section(&format!( - "Bundler version {} from {}", - fmt::value(bundler_version.to_string()), - fmt::value(gemfile_lock.bundler_source()) - )); - let layer_env = layers::bundle_download_layer::handle( + (_, env) = { + let (bullet, layer_env) = layers::bundle_download_layer::handle( &context, + build_output.bullet(format!( + "Bundler version {} from {}", + style::value(bundler_version.to_string()), + style::value(gemfile_lock.bundler_source()) + )), &env, 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