From 06d12823129ec97d22200f28d5115e6768d1ab0d Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:08:10 -0600 Subject: [PATCH 01/34] Update magic_migrate version --- Cargo.lock | 4 ++-- buildpacks/ruby/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7c400303..34952b48 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -968,9 +968,9 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "magic_migrate" -version = "0.2.1" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3995455690a60dcbb8688d0f0e84eee5322eb1cdd5e9a9bf0ac7b2aa56250291" +checksum = "d6dbe7e54cb4d9e457cc23ef8e27ed148fbecf630c0b4675729310995bca2f04" dependencies = [ "serde", ] diff --git a/buildpacks/ruby/Cargo.toml b/buildpacks/ruby/Cargo.toml index de6b4563..98ee266d 100644 --- a/buildpacks/ruby/Cargo.toml +++ b/buildpacks/ruby/Cargo.toml @@ -28,7 +28,7 @@ tempfile = "3" thiserror = "2" ureq = { version = "2", default-features = false, features = ["tls"] } url = "2" -magic_migrate = "0.2" +magic_migrate = "1.0" toml = "0.8" cache_diff = { version = "1.0.0", features = ["bullet_stream"] } From 1379f9db2e4d590a42a2ac5c3a64f03eeca93939 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:10:02 -0600 Subject: [PATCH 02/34] Update generic constraints The `magic_migrate` library now requires errors to be Display + Debug, so this extra constraint is no longer needed on these interfaces. --- buildpacks/ruby/src/layers/shared.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index f25ba19e..d37fad2d 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -19,7 +19,6 @@ pub(crate) fn cached_layer_write_metadata( where B: libcnb::Buildpack, M: CacheDiff + TryMigrate + Serialize + Debug + Clone, - ::Error: std::fmt::Display, { let layer_ref = context.cached_layer( layer_name, @@ -66,8 +65,6 @@ pub(crate) fn invalid_metadata_action(invalid: &S) -> (InvalidMetadataActi where M: TryMigrate + Clone, S: Serialize + Debug, - // TODO: Enforce Display + Debug in the library - ::Error: std::fmt::Display, { let invalid = toml::to_string(invalid); match invalid { From 8e5134b4e378db41960cb3f6bf731f85d174bc2b Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:39:11 -0600 Subject: [PATCH 03/34] Make deprecation more specific --- commons/src/layer.rs | 9 +++++++++ commons/src/lib.rs | 4 ---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/commons/src/layer.rs b/commons/src/layer.rs index 48d5a73b..881fbfbc 100644 --- a/commons/src/layer.rs +++ b/commons/src/layer.rs @@ -1,5 +1,14 @@ mod configure_env_layer; mod default_env_layer; +#[deprecated( + since = "0.0.0", + note = "Use the struct layer API in the latest libcnb.rs instead" +)] pub use self::configure_env_layer::ConfigureEnvLayer; + +#[deprecated( + since = "0.0.0", + note = "Use the struct layer API in the latest libcnb.rs instead" +)] pub use self::default_env_layer::DefaultEnvLayer; diff --git a/commons/src/lib.rs b/commons/src/lib.rs index 27499b7a..27dc68ce 100644 --- a/commons/src/lib.rs +++ b/commons/src/lib.rs @@ -2,10 +2,6 @@ pub mod cache; pub mod display; pub mod gem_version; pub mod gemfile_lock; -#[deprecated( - since = "0.0.0", - note = "Use the struct layer API in the latest libcnb.rs instead" -)] pub mod layer; pub mod metadata_digest; pub mod output; From c78ccfd6f613aefff6fc1185e3f9fda6d1ff136f Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:39:29 -0600 Subject: [PATCH 04/34] Make Meta struct public --- buildpacks/ruby/src/layers/shared.rs | 41 +--------------------------- commons/src/layer.rs | 1 + commons/src/layer/cache_buddy.rs | 39 ++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 commons/src/layer/cache_buddy.rs diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index d37fad2d..f36bd547 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,5 +1,6 @@ use cache_diff::CacheDiff; use commons::display::SentenceList; +pub(crate) use commons::layer::cache_buddy::Meta; use libcnb::build::BuildContext; use libcnb::data::layer::LayerName; use libcnb::layer::{CachedLayerDefinition, InvalidMetadataAction, LayerRef, RestoredLayerAction}; @@ -96,46 +97,6 @@ where } } -/// 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 { diff --git a/commons/src/layer.rs b/commons/src/layer.rs index 881fbfbc..d28d672b 100644 --- a/commons/src/layer.rs +++ b/commons/src/layer.rs @@ -1,3 +1,4 @@ +pub mod cache_buddy; mod configure_env_layer; mod default_env_layer; diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs new file mode 100644 index 00000000..2606d5d0 --- /dev/null +++ b/commons/src/layer/cache_buddy.rs @@ -0,0 +1,39 @@ +/// 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 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", + } + } +} From 72614928eaf02a7f48cf0668847ae1a15e7e9fe6 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 14:54:51 -0600 Subject: [PATCH 05/34] Make invalid_metadata_action public --- Cargo.lock | 1 + buildpacks/ruby/src/layers/shared.rs | 45 ++-------------------------- commons/Cargo.toml | 2 ++ commons/src/layer/cache_buddy.rs | 45 ++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 34952b48..5f2454c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -302,6 +302,7 @@ dependencies = [ "libcnb", "libcnb-test", "libherokubuildpack", + "magic_migrate", "pretty_assertions", "regex", "serde", diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index f36bd547..b96c6b79 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,9 +1,10 @@ use cache_diff::CacheDiff; use commons::display::SentenceList; +use commons::layer::cache_buddy::invalid_metadata_action; pub(crate) use commons::layer::cache_buddy::Meta; use libcnb::build::BuildContext; use libcnb::data::layer::LayerName; -use libcnb::layer::{CachedLayerDefinition, InvalidMetadataAction, LayerRef, RestoredLayerAction}; +use libcnb::layer::{CachedLayerDefinition, LayerRef, RestoredLayerAction}; use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; @@ -57,46 +58,6 @@ where } } -/// Standardizes formatting for invalid metadata behavior -/// -/// 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, Meta) -where - M: TryMigrate + Clone, - S: Serialize + Debug, -{ - let invalid = toml::to_string(invalid); - match invalid { - Ok(toml) => match M::try_from_str_migrations(&toml) { - Some(Ok(migrated)) => ( - InvalidMetadataAction::ReplaceMetadata(migrated.clone()), - Meta::Data(migrated), - ), - Some(Err(error)) => ( - InvalidMetadataAction::DeleteLayer, - Meta::Message(format!( - "Clearing cache due to metadata migration error: {error}" - )), - ), - None => ( - InvalidMetadataAction::DeleteLayer, - Meta::Message(format!( - "Clearing cache due to invalid metadata ({toml})", - toml = toml.trim() - )), - ), - }, - Err(error) => ( - InvalidMetadataAction::DeleteLayer, - Meta::Message(format!( - "Clearing cache due to invalid metadata serialization error: {error}" - )), - ), - } -} - /// Removes ANSI control characters from a string #[cfg(test)] pub(crate) fn strip_ansi(input: impl AsRef) -> String { @@ -156,7 +117,7 @@ mod tests { use crate::RubyBuildpack; use core::panic; use libcnb::data::layer_name; - use libcnb::layer::{EmptyLayerCause, LayerState}; + use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState}; use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; use serde::Deserializer; use std::convert::Infallible; diff --git a/commons/Cargo.toml b/commons/Cargo.toml index 6e070f82..7fcf38ff 100644 --- a/commons/Cargo.toml +++ b/commons/Cargo.toml @@ -27,6 +27,8 @@ tempfile = "3" thiserror = "2" walkdir = "2" filetime = "0.2" +magic_migrate = "1.0" +toml = "0.8" [dev-dependencies] filetime = "0.2" diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 2606d5d0..e4954512 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -1,3 +1,48 @@ +use libcnb::layer::InvalidMetadataAction; +use magic_migrate::TryMigrate; +use serde::ser::Serialize; +use std::fmt::Debug; + +/// Standardizes formatting for invalid metadata behavior +/// +/// 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 fn invalid_metadata_action(invalid: &S) -> (InvalidMetadataAction, Meta) +where + M: TryMigrate + Clone, + S: Serialize + Debug, +{ + let invalid = toml::to_string(invalid); + match invalid { + Ok(toml) => match M::try_from_str_migrations(&toml) { + Some(Ok(migrated)) => ( + InvalidMetadataAction::ReplaceMetadata(migrated.clone()), + Meta::Data(migrated), + ), + Some(Err(error)) => ( + InvalidMetadataAction::DeleteLayer, + Meta::Message(format!( + "Clearing cache due to metadata migration error: {error}" + )), + ), + None => ( + InvalidMetadataAction::DeleteLayer, + Meta::Message(format!( + "Clearing cache due to invalid metadata ({toml})", + toml = toml.trim() + )), + ), + }, + Err(error) => ( + InvalidMetadataAction::DeleteLayer, + 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 a581bdbc75ad82c450896b2aed7cce0fa926802a Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:41:59 -0600 Subject: [PATCH 06/34] Make restored_layer_action public --- Cargo.lock | 1 + buildpacks/ruby/src/layers/shared.rs | 30 +++------------------------- commons/Cargo.toml | 1 + commons/src/layer/cache_buddy.rs | 28 +++++++++++++++++++++++++- 4 files changed, 32 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5f2454c3..9e759eb3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -291,6 +291,7 @@ name = "commons" version = "0.0.0" dependencies = [ "byte-unit", + "cache_diff", "const_format", "fancy-regex", "filetime", diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index b96c6b79..f16450cd 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,10 +1,9 @@ use cache_diff::CacheDiff; -use commons::display::SentenceList; -use commons::layer::cache_buddy::invalid_metadata_action; pub(crate) use commons::layer::cache_buddy::Meta; +use commons::layer::cache_buddy::{invalid_metadata_action, restored_layer_action}; use libcnb::build::BuildContext; use libcnb::data::layer::LayerName; -use libcnb::layer::{CachedLayerDefinition, LayerRef, RestoredLayerAction}; +use libcnb::layer::{CachedLayerDefinition, LayerRef}; use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; @@ -35,29 +34,6 @@ where Ok(layer_ref) } -/// Standardizes formatting for layer cache clearing behavior -/// -/// If the diff is empty, there are no changes and the layer is kept and the old data is returned -/// If the diff is not empty, the layer is deleted and the changes are listed -pub(crate) fn restored_layer_action(old: &M, now: &M) -> (RestoredLayerAction, Meta) -where - M: CacheDiff + Clone, -{ - let diff = now.diff(old); - if diff.is_empty() { - (RestoredLayerAction::KeepLayer, Meta::Data(old.clone())) - } else { - ( - RestoredLayerAction::DeleteLayer, - Meta::Message(format!( - "Clearing cache due to {changes}: {differences}", - changes = if diff.len() > 1 { "changes" } else { "change" }, - differences = SentenceList::new(&diff) - )), - ) - } -} - /// Removes ANSI control characters from a string #[cfg(test)] pub(crate) fn strip_ansi(input: impl AsRef) -> String { @@ -117,7 +93,7 @@ mod tests { use crate::RubyBuildpack; use core::panic; use libcnb::data::layer_name; - use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState}; + use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; use serde::Deserializer; use std::convert::Infallible; diff --git a/commons/Cargo.toml b/commons/Cargo.toml index 7fcf38ff..686797b9 100644 --- a/commons/Cargo.toml +++ b/commons/Cargo.toml @@ -29,6 +29,7 @@ walkdir = "2" filetime = "0.2" magic_migrate = "1.0" toml = "0.8" +cache_diff = "1.0" [dev-dependencies] filetime = "0.2" diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index e4954512..4394d53f 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -1,8 +1,34 @@ -use libcnb::layer::InvalidMetadataAction; +use cache_diff::CacheDiff; +use libcnb::layer::{InvalidMetadataAction, RestoredLayerAction}; use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; +use crate::display::SentenceList; + +/// Standardizes formatting for layer cache clearing behavior +/// +/// If the diff is empty, there are no changes and the layer is kept and the old data is returned +/// If the diff is not empty, the layer is deleted and the changes are listed +pub fn restored_layer_action(old: &M, now: &M) -> (RestoredLayerAction, Meta) +where + M: CacheDiff + Clone, +{ + let diff = now.diff(old); + if diff.is_empty() { + (RestoredLayerAction::KeepLayer, Meta::Data(old.clone())) + } else { + ( + RestoredLayerAction::DeleteLayer, + Meta::Message(format!( + "Clearing cache due to {changes}: {differences}", + changes = if diff.len() > 1 { "changes" } else { "change" }, + differences = SentenceList::new(&diff) + )), + ) + } +} + /// Standardizes formatting for invalid metadata behavior /// /// If the metadata can be migrated, it is replaced with the migrated version From a9c2982f214f993507d72e3f7c8d554e9f89bad0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:51:25 -0600 Subject: [PATCH 07/34] Make cached_layer_write_metadata public --- buildpacks/ruby/src/layers/shared.rs | 41 ++++------------------------ commons/src/layer/cache_buddy.rs | 31 +++++++++++++++++++-- 2 files changed, 34 insertions(+), 38 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index f16450cd..267ebbea 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,38 +1,5 @@ -use cache_diff::CacheDiff; +pub(crate) use commons::layer::cache_buddy::cached_layer_write_metadata; pub(crate) use commons::layer::cache_buddy::Meta; -use commons::layer::cache_buddy::{invalid_metadata_action, restored_layer_action}; -use libcnb::build::BuildContext; -use libcnb::data::layer::LayerName; -use libcnb::layer::{CachedLayerDefinition, LayerRef}; -use magic_migrate::TryMigrate; -use serde::ser::Serialize; -use std::fmt::Debug; - -/// Default behavior for a cached layer, ensures new metadata is always written -/// -/// The metadadata must implement `CacheDiff` and `TryMigrate` in addition -/// to the typical `Serialize` and `Debug` traits -pub(crate) fn cached_layer_write_metadata( - layer_name: LayerName, - context: &BuildContext, - metadata: &'_ M, -) -> libcnb::Result, Meta>, B::Error> -where - B: libcnb::Buildpack, - M: CacheDiff + TryMigrate + Serialize + Debug + Clone, -{ - let layer_ref = context.cached_layer( - layer_name, - CachedLayerDefinition { - build: true, - launch: true, - invalid_metadata_action: &invalid_metadata_action, - restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), - }, - )?; - layer_ref.write_metadata(metadata)?; - Ok(layer_ref) -} /// Removes ANSI control characters from a string #[cfg(test)] @@ -47,7 +14,7 @@ pub(crate) fn strip_ansi(input: impl AsRef) -> String { #[cfg(test)] pub(crate) fn temp_build_context( from_dir: impl AsRef, -) -> BuildContext { +) -> libcnb::build::BuildContext { let base_dir = from_dir.as_ref().to_path_buf(); let layers_dir = base_dir.join("layers"); let app_dir = base_dir.join("app_dir"); @@ -75,7 +42,7 @@ pub(crate) fn temp_build_context( }; let store = None; - BuildContext { + libcnb::build::BuildContext { layers_dir, app_dir, buildpack_dir, @@ -91,6 +58,8 @@ pub(crate) fn temp_build_context( mod tests { use super::*; use crate::RubyBuildpack; + use cache_diff::CacheDiff; + use commons::layer::cache_buddy::{invalid_metadata_action, restored_layer_action}; use core::panic; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 4394d53f..537dbbe4 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -1,10 +1,37 @@ +use crate::display::SentenceList; use cache_diff::CacheDiff; -use libcnb::layer::{InvalidMetadataAction, RestoredLayerAction}; +use libcnb::build::BuildContext; +use libcnb::data::layer::LayerName; +use libcnb::layer::{CachedLayerDefinition, InvalidMetadataAction, LayerRef, RestoredLayerAction}; use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; -use crate::display::SentenceList; +/// Default behavior for a cached layer, ensures new metadata is always written +/// +/// The metadadata must implement `CacheDiff` and `TryMigrate` in addition +/// to the typical `Serialize` and `Debug` traits +pub fn cached_layer_write_metadata( + layer_name: LayerName, + context: &BuildContext, + metadata: &'_ M, +) -> libcnb::Result, Meta>, B::Error> +where + B: libcnb::Buildpack, + M: CacheDiff + TryMigrate + Serialize + Debug + Clone, +{ + let layer_ref = context.cached_layer( + layer_name, + CachedLayerDefinition { + build: true, + launch: true, + invalid_metadata_action: &invalid_metadata_action, + restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), + }, + )?; + layer_ref.write_metadata(metadata)?; + Ok(layer_ref) +} /// Standardizes formatting for layer cache clearing behavior /// From bdcc00e6eaf30d640d4454dab9d0b161595b15b7 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 11:52:53 -0600 Subject: [PATCH 08/34] Remove strip_ansi helper method --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 2 +- buildpacks/ruby/src/layers/bundle_install_layer.rs | 2 +- buildpacks/ruby/src/layers/ruby_install_layer.rs | 4 ++-- buildpacks/ruby/src/layers/shared.rs | 7 ------- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 1dedcb90..203c2810 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -123,7 +123,7 @@ fn download_bundler( #[cfg(test)] mod test { use super::*; - use crate::layers::shared::strip_ansi; + use bullet_stream::strip_ansi; #[test] fn test_metadata_diff() { diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 90d42723..d0ac73b5 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -319,7 +319,7 @@ fn display_name(cmd: &mut Command, env: &Env) -> String { #[cfg(test)] mod test { - use crate::layers::shared::strip_ansi; + use bullet_stream::strip_ansi; use super::*; use std::path::PathBuf; diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index ea98109b..3ce7d26a 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -237,9 +237,9 @@ pub(crate) enum RubyInstallError { #[cfg(test)] mod tests { - use crate::layers::shared::{strip_ansi, temp_build_context}; - use super::*; + use crate::layers::shared::temp_build_context; + use bullet_stream::strip_ansi; /// If this test fails due to a change you'll need to /// implement `TryMigrate` for the new layer data and add diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 267ebbea..deaae508 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,13 +1,6 @@ pub(crate) use commons::layer::cache_buddy::cached_layer_write_metadata; pub(crate) use commons::layer::cache_buddy::Meta; -/// Removes ANSI control characters from a string -#[cfg(test)] -pub(crate) fn strip_ansi(input: impl AsRef) -> String { - let re = regex::Regex::new(r"\x1b\[[0-9;]*[a-zA-Z]").expect("Clippy checked"); - re.replace_all(input.as_ref(), "").to_string() -} - /// Takes in a directory and returns a minimal build context for use in testing shared caching behavior /// /// Intented only for use with this buildpack, but meant to be used by multiple layers to assert caching behavior. From 53b2f8020c941745459a0dd64037a08d81385d16 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:03:22 -0600 Subject: [PATCH 09/34] Move test --- buildpacks/ruby/src/layers/shared.rs | 28 -------------- commons/src/layer/cache_buddy.rs | 56 ++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 28 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index deaae508..b63e492f 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -77,34 +77,6 @@ mod tests { } migrate_toml_chain! {TestMetadata} - #[test] - fn test_restored_layer_action_returns_old_data() { - #[derive(Debug, Clone)] - struct AlwaysNoDiff { - value: String, - } - impl CacheDiff for AlwaysNoDiff { - fn diff(&self, _: &Self) -> Vec { - vec![] - } - } - - let old = AlwaysNoDiff { - value: "old".to_string(), - }; - let now = AlwaysNoDiff { - value: "now".to_string(), - }; - - let result = restored_layer_action(&old, &now); - match result { - (RestoredLayerAction::KeepLayer, Meta::Data(data)) => { - assert_eq!(data.value, "old"); - } - _ => panic!("Expected to keep layer"), - } - } - #[test] fn test_cached_layer_write_metadata_restored_layer_action() { let temp = tempfile::tempdir().unwrap(); diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 537dbbe4..f682352d 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -135,3 +135,59 @@ impl AsRef for Meta { } } } + +#[cfg(test)] +mod tests { + use super::*; + use cache_diff::CacheDiff; + use core::panic; + use libcnb::data::layer_name; + use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; + use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; + use serde::Deserializer; + use std::convert::Infallible; + /// Struct for asserting the behavior of `cached_layer_write_metadata` + #[derive(Debug, serde::Serialize, serde::Deserialize, Clone)] + #[serde(deny_unknown_fields)] + struct TestMetadata { + value: String, + } + impl CacheDiff for TestMetadata { + fn diff(&self, old: &Self) -> Vec { + if self.value == old.value { + vec![] + } else { + vec![format!("value ({} to {})", old.value, self.value)] + } + } + } + migrate_toml_chain! {TestMetadata} + + #[test] + fn test_restored_layer_action_returns_old_data() { + #[derive(Debug, Clone)] + struct AlwaysNoDiff { + value: String, + } + impl CacheDiff for AlwaysNoDiff { + fn diff(&self, _: &Self) -> Vec { + vec![] + } + } + + let old = AlwaysNoDiff { + value: "old".to_string(), + }; + let now = AlwaysNoDiff { + value: "now".to_string(), + }; + + let result = restored_layer_action(&old, &now); + match result { + (RestoredLayerAction::KeepLayer, Meta::Data(data)) => { + assert_eq!(data.value, "old"); + } + _ => panic!("Expected to keep layer"), + } + } +} From 7c48d5bbb2a50af72292c6b941d717fdfb3d467a Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:04:45 -0600 Subject: [PATCH 10/34] Move test --- buildpacks/ruby/src/layers/shared.rs | 77 ---------------------------- commons/src/layer/cache_buddy.rs | 77 ++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 77 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index b63e492f..a2a0dd9a 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -133,81 +133,4 @@ mod tests { "Clearing cache due to change: value (hello to world)" ); } - - /// Struct for asserting the behavior of `invalid_metadata_action` - #[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, Clone)] - #[serde(deny_unknown_fields)] - struct PersonV2 { - name: String, - updated_at: String, - } - // First define how to map from one struct to another - impl TryFrom for PersonV2 { - type Error = NotRichard; - fn try_from(value: PersonV1) -> Result { - if &value.name == "Schneems" { - Ok(PersonV2 { - name: value.name.clone(), - updated_at: "unknown".to_string(), - }) - } else { - Err(NotRichard { - name: value.name.clone(), - }) - } - } - } - #[derive(Debug, Eq, PartialEq)] - struct NotRichard { - name: String, - } - impl From for PersonMigrationError { - fn from(value: NotRichard) -> Self { - PersonMigrationError::NotRichard(value) - } - } - #[derive(Debug, Eq, PartialEq, thiserror::Error)] - enum PersonMigrationError { - #[error("Not Richard")] - NotRichard(NotRichard), - } - try_migrate_deserializer_chain!( - deserializer: toml::Deserializer::new, - error: PersonMigrationError, - chain: [PersonV1, PersonV2], - ); - - #[test] - fn test_invalid_metadata_action() { - let (action, message) = invalid_metadata_action::(&PersonV1 { - name: "schneems".to_string(), - }); - assert!(matches!(action, InvalidMetadataAction::ReplaceMetadata(_))); - 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.as_ref(), - "Clearing cache due to metadata migration error: Not Richard" - ); - - let (action, message) = invalid_metadata_action::(&TestMetadata { - value: "world".to_string(), - }); - assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); - assert_eq!( - 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/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index f682352d..9e784114 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -190,4 +190,81 @@ mod tests { _ => panic!("Expected to keep layer"), } } + + /// Struct for asserting the behavior of `invalid_metadata_action` + #[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, Clone)] + #[serde(deny_unknown_fields)] + struct PersonV2 { + name: String, + updated_at: String, + } + // First define how to map from one struct to another + impl TryFrom for PersonV2 { + type Error = NotRichard; + fn try_from(value: PersonV1) -> Result { + if &value.name == "Schneems" { + Ok(PersonV2 { + name: value.name.clone(), + updated_at: "unknown".to_string(), + }) + } else { + Err(NotRichard { + name: value.name.clone(), + }) + } + } + } + #[derive(Debug, Eq, PartialEq)] + struct NotRichard { + name: String, + } + impl From for PersonMigrationError { + fn from(value: NotRichard) -> Self { + PersonMigrationError::NotRichard(value) + } + } + #[derive(Debug, Eq, PartialEq, thiserror::Error)] + enum PersonMigrationError { + #[error("Not Richard")] + NotRichard(NotRichard), + } + try_migrate_deserializer_chain!( + deserializer: toml::Deserializer::new, + error: PersonMigrationError, + chain: [PersonV1, PersonV2], + ); + + #[test] + fn test_invalid_metadata_action() { + let (action, message) = invalid_metadata_action::(&PersonV1 { + name: "schneems".to_string(), + }); + assert!(matches!(action, InvalidMetadataAction::ReplaceMetadata(_))); + 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.as_ref(), + "Clearing cache due to metadata migration error: Not Richard" + ); + + let (action, message) = invalid_metadata_action::(&TestMetadata { + value: "world".to_string(), + }); + assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); + assert_eq!( + 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}" + } } From 09b7533914f73d9894ebc862af5053e995d3ebad Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:09:56 -0600 Subject: [PATCH 11/34] Add testing helper to cache_buddy --- commons/src/layer/cache_buddy.rs | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 9e784114..1b2981b4 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -136,6 +136,50 @@ impl AsRef for Meta { } } +/// Takes in a directory and returns a minimal build context for use in testing caching behavior +#[cfg(test)] +fn temp_build_context( + from_dir: impl AsRef, + buildpack_toml_string: &str, +) -> libcnb::build::BuildContext { + let base_dir = from_dir.as_ref().to_path_buf(); + let layers_dir = base_dir.join("layers"); + let app_dir = base_dir.join("app_dir"); + let platform_dir = base_dir.join("platform_dir"); + let buildpack_dir = base_dir.join("buildpack_dir"); + for dir in [&app_dir, &layers_dir, &buildpack_dir, &platform_dir] { + std::fs::create_dir_all(dir).unwrap(); + } + + let target = libcnb::Target { + os: String::new(), + arch: String::new(), + arch_variant: None, + distro_name: String::new(), + distro_version: String::new(), + }; + let platform = + <::Platform as libcnb::Platform>::from_path(&platform_dir).unwrap(); + let buildpack_descriptor: libcnb::data::buildpack::ComponentBuildpackDescriptor< + ::Metadata, + > = toml::from_str(buildpack_toml_string).unwrap(); + let buildpack_plan = libcnb::data::buildpack_plan::BuildpackPlan { + entries: Vec::::new(), + }; + let store = None; + + libcnb::build::BuildContext { + layers_dir, + app_dir, + buildpack_dir, + target, + platform, + buildpack_plan, + buildpack_descriptor, + store, + } +} + #[cfg(test)] mod tests { use super::*; From e563c488813ac170e23faee293c2ff804af2d1b0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:14:22 -0600 Subject: [PATCH 12/34] Move test --- buildpacks/ruby/src/layers/shared.rs | 57 ------------------- commons/src/layer/cache_buddy.rs | 83 ++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 57 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index a2a0dd9a..252036d9 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -76,61 +76,4 @@ mod tests { } } migrate_toml_chain! {TestMetadata} - - #[test] - fn test_cached_layer_write_metadata_restored_layer_action() { - let temp = tempfile::tempdir().unwrap(); - let context = temp_build_context::(temp.path()); - - // First write - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); - assert!(matches!( - result.state, - LayerState::Empty { - cause: EmptyLayerCause::NewlyCreated - } - )); - - // Second write, preserve the contents - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); - let LayerState::Restored { cause } = &result.state else { - panic!("Expected restored layer") - }; - assert_eq!(cause.as_ref(), "Using cache"); - - // Third write, change the data - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "world".to_string(), - }, - ) - .unwrap(); - - let LayerState::Empty { - cause: EmptyLayerCause::RestoredLayerAction { cause }, - } = &result.state - else { - panic!("Expected empty layer with restored layer action"); - }; - assert_eq!( - cause.as_ref(), - "Clearing cache due to change: value (hello to world)" - ); - } } diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 1b2981b4..fdb55014 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -186,6 +186,7 @@ mod tests { use cache_diff::CacheDiff; use core::panic; use libcnb::data::layer_name; + use libcnb::generic::{GenericMetadata, GenericPlatform}; use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; use serde::Deserializer; @@ -207,6 +208,88 @@ mod tests { } migrate_toml_chain! {TestMetadata} + struct FakeBuildpack; + + impl libcnb::Buildpack for FakeBuildpack { + type Platform = GenericPlatform; + type Metadata = GenericMetadata; + type Error = Infallible; + + fn detect( + &self, + _context: libcnb::detect::DetectContext, + ) -> libcnb::Result { + todo!() + } + + fn build( + &self, + _context: BuildContext, + ) -> libcnb::Result { + todo!() + } + } + + #[test] + fn test_cached_layer_write_metadata_restored_layer_action() { + let temp = tempfile::tempdir().unwrap(); + let context = temp_build_context::( + temp.path(), + include_str!("../../../buildpacks/ruby/buildpack.toml"), + ); + + // First write + let result = cached_layer_write_metadata( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); + assert!(matches!( + result.state, + LayerState::Empty { + cause: EmptyLayerCause::NewlyCreated + } + )); + + // Second write, preserve the contents + let result = cached_layer_write_metadata( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); + let LayerState::Restored { cause } = &result.state else { + panic!("Expected restored layer") + }; + assert_eq!(cause.as_ref(), "Using cache"); + + // Third write, change the data + let result = cached_layer_write_metadata( + layer_name!("testing"), + &context, + &TestMetadata { + value: "world".to_string(), + }, + ) + .unwrap(); + + let LayerState::Empty { + cause: EmptyLayerCause::RestoredLayerAction { cause }, + } = &result.state + else { + panic!("Expected empty layer with restored layer action"); + }; + assert_eq!( + cause.as_ref(), + "Clearing cache due to change: value (hello to world)" + ); + } + #[test] fn test_restored_layer_action_returns_old_data() { #[derive(Debug, Clone)] From 7569ffc5de9006be5c1d05e054d32fe193d41f24 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:14:55 -0600 Subject: [PATCH 13/34] Remove unused test module --- buildpacks/ruby/src/layers/shared.rs | 31 ---------------------------- 1 file changed, 31 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 252036d9..68ff7f19 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -46,34 +46,3 @@ pub(crate) fn temp_build_context( store, } } - -#[cfg(test)] -mod tests { - use super::*; - use crate::RubyBuildpack; - use cache_diff::CacheDiff; - use commons::layer::cache_buddy::{invalid_metadata_action, restored_layer_action}; - use core::panic; - use libcnb::data::layer_name; - use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; - use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; - use serde::Deserializer; - use std::convert::Infallible; - - /// Struct for asserting the behavior of `cached_layer_write_metadata` - #[derive(Debug, serde::Serialize, serde::Deserialize, Clone)] - #[serde(deny_unknown_fields)] - struct TestMetadata { - value: String, - } - impl CacheDiff for TestMetadata { - fn diff(&self, old: &Self) -> Vec { - if self.value == old.value { - vec![] - } else { - vec![format!("value ({} to {})", old.value, self.value)] - } - } - } - migrate_toml_chain! {TestMetadata} -} From 588755ad863b79e7eabe5c19efcaddf8d0837f3d Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:16:42 -0600 Subject: [PATCH 14/34] Update use statements --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 2 +- buildpacks/ruby/src/layers/bundle_install_layer.rs | 2 +- buildpacks/ruby/src/layers/ruby_install_layer.rs | 2 +- buildpacks/ruby/src/layers/shared.rs | 3 --- 4 files changed, 3 insertions(+), 6 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 203c2810..19ea3d68 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -4,13 +4,13 @@ //! //! Installs a copy of `bundler` to the `` with a bundler executable in //! `/bin`. Must run before [`crate.steps.bundle_install`]. -use crate::layers::shared::cached_layer_write_metadata; use crate::RubyBuildpack; use crate::RubyBuildpackError; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedBundlerVersion; +use commons::layer::cache_buddy::cached_layer_write_metadata; use fun_run::{self, CommandWithName}; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index d0ac73b5..baf06a4c 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -14,12 +14,12 @@ //! 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}; use crate::target_id::{OsDistribution, TargetId, TargetIdError}; use crate::{BundleWithout, RubyBuildpack, RubyBuildpackError}; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; +use commons::layer::cache_buddy::{cached_layer_write_metadata, Meta}; use commons::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 3ce7d26a..85be72f6 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,7 +11,6 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::cached_layer_write_metadata; use crate::target_id::OsDistribution; use crate::{ target_id::{TargetId, TargetIdError}, @@ -21,6 +20,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::Print; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedRubyVersion; +use commons::layer::cache_buddy::cached_layer_write_metadata; use flate2::read::GzDecoder; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 68ff7f19..773828c4 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,6 +1,3 @@ -pub(crate) use commons::layer::cache_buddy::cached_layer_write_metadata; -pub(crate) use commons::layer::cache_buddy::Meta; - /// Takes in a directory and returns a minimal build context for use in testing shared caching behavior /// /// Intented only for use with this buildpack, but meant to be used by multiple layers to assert caching behavior. From 18720847ad0e59d0717fff08751550c4e3a9be88 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:34:24 -0600 Subject: [PATCH 15/34] Introduce CacheBuddy struct It's basically the same logic as `cached_layer_write_metadata` but with configurable build/launch logic --- commons/src/layer/cache_buddy.rs | 71 +++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index fdb55014..8db43c2f 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -7,10 +7,68 @@ use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; +/// Handle caching behavior for a layer +/// +/// Guarantees that new metadata is always written (prevents accidentally reading one struct type and +/// writing a different one). It also provides a standard interface to define caching behavior via +/// the `CacheDiff` and `TryMigrate` traits: +/// +/// - The `TryMigrate` trait is for handling invalid metadata: +/// When old metadata from cache is invalid, we try to load it into a known older version and then migrate it +/// to the latest via `TryMigrate`. If that fails, the layer is deleted and the error is returned. If it +/// succeeds, then the logic in `CacheDiff` below is applied. +/// +/// The `CacheDiff` trait defines cache invalidation behavior when metadata is valid: +/// When a `CacheDiff::diff` is empty, the layer is kept and the old data is returned. Otherwise, +/// the layer is deleted and the changes are returned. +/// +#[derive(Default, Debug, Clone, Eq, PartialEq)] +pub struct CacheBuddy { + pub build: Option, + pub launch: Option, +} + +impl CacheBuddy { + #[must_use] + pub fn new() -> Self { + Self::default() + } + + /// # Errors + /// + /// Returns an error if libcnb cannot read or write the metadata. + pub fn layer( + self, + layer_name: LayerName, + context: &BuildContext, + metadata: &M, + ) -> libcnb::Result, Meta>, B::Error> + where + B: libcnb::Buildpack, + M: CacheDiff + TryMigrate + Serialize + Debug + Clone, + { + let layer_ref = context.cached_layer( + layer_name, + CachedLayerDefinition { + build: self.build.unwrap_or(true), + launch: self.launch.unwrap_or(true), + invalid_metadata_action: &invalid_metadata_action, + restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), + }, + )?; + layer_ref.write_metadata(metadata)?; + Ok(layer_ref) + } +} + /// Default behavior for a cached layer, ensures new metadata is always written /// /// The metadadata must implement `CacheDiff` and `TryMigrate` in addition /// to the typical `Serialize` and `Debug` traits +/// +/// # Errors +/// +/// Returns an error if libcnb cannot read or write the metadata. pub fn cached_layer_write_metadata( layer_name: LayerName, context: &BuildContext, @@ -20,23 +78,14 @@ where B: libcnb::Buildpack, M: CacheDiff + TryMigrate + Serialize + Debug + Clone, { - let layer_ref = context.cached_layer( - layer_name, - CachedLayerDefinition { - build: true, - launch: true, - invalid_metadata_action: &invalid_metadata_action, - restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), - }, - )?; - layer_ref.write_metadata(metadata)?; - Ok(layer_ref) + CacheBuddy::new().layer(layer_name, context, metadata) } /// Standardizes formatting for layer cache clearing behavior /// /// If the diff is empty, there are no changes and the layer is kept and the old data is returned /// If the diff is not empty, the layer is deleted and the changes are listed +/// pub fn restored_layer_action(old: &M, now: &M) -> (RestoredLayerAction, Meta) where M: CacheDiff + Clone, From 0d07a7b8e1cb884abb7a3296a6a4e8602d0eb9b6 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:34:38 -0600 Subject: [PATCH 16/34] Switch layer to CacheBuddy --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 85be72f6..976fd629 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -20,7 +20,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::Print; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedRubyVersion; -use commons::layer::cache_buddy::cached_layer_write_metadata; +use commons::layer::cache_buddy::CacheBuddy; use flate2::read::GzDecoder; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; @@ -39,7 +39,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, metadata)?; + let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); @@ -388,8 +388,12 @@ version = "3.1.3" let differences = old.diff(&old); assert_eq!(differences, Vec::::new()); - cached_layer_write_metadata(layer_name!("ruby"), &context, &old).unwrap(); - let result = cached_layer_write_metadata(layer_name!("ruby"), &context, &old).unwrap(); + CacheBuddy::new() + .layer(layer_name!("ruby"), &context, &old) + .unwrap(); + let result = CacheBuddy::new() + .layer(layer_name!("ruby"), &context, &old) + .unwrap(); let actual = result.state; assert!(matches!(actual, LayerState::Restored { .. })); @@ -400,7 +404,9 @@ version = "3.1.3" let differences = now.diff(&old); assert_eq!(differences.len(), 1); - let result = cached_layer_write_metadata(layer_name!("ruby"), &context, &now).unwrap(); + let result = CacheBuddy::new() + .layer(layer_name!("ruby"), &context, &now) + .unwrap(); assert!(matches!( result.state, LayerState::Empty { From 70a82d7b592c24219b59d8653b680327dabeff6c Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:37:03 -0600 Subject: [PATCH 17/34] Switch layer to CacheBuddy --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 19ea3d68..4fb0105d 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -10,7 +10,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedBundlerVersion; -use commons::layer::cache_buddy::cached_layer_write_metadata; +use commons::layer::cache_buddy::CacheBuddy; use fun_run::{self, CommandWithName}; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; @@ -29,7 +29,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = cached_layer_write_metadata(layer_name!("bundler"), context, metadata)?; + let layer_ref = CacheBuddy::new().layer(layer_name!("bundler"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); From f784b5a3e96db2959aaee6ace5ddb424f724ada4 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:37:36 -0600 Subject: [PATCH 18/34] Switch layer to CacheBuddy --- buildpacks/ruby/src/layers/bundle_install_layer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index baf06a4c..1d505d31 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -19,7 +19,7 @@ use crate::{BundleWithout, RubyBuildpack, RubyBuildpackError}; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; -use commons::layer::cache_buddy::{cached_layer_write_metadata, Meta}; +use commons::layer::cache_buddy::{CacheBuddy, Meta}; use commons::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; @@ -52,7 +52,7 @@ pub(crate) fn handle( metadata: &Metadata, without: &BundleWithout, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = cached_layer_write_metadata(layer_name!("gems"), context, metadata)?; + let layer_ref = CacheBuddy::new().layer(layer_name!("gems"), context, metadata)?; let install_state = match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); From c3321e3ea815cc57698229c936983f5e7035b3ec Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:37:59 -0600 Subject: [PATCH 19/34] Remove unused function --- commons/src/layer/cache_buddy.rs | 75 ++++++++++++-------------------- 1 file changed, 29 insertions(+), 46 deletions(-) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 8db43c2f..6f1624ce 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -61,26 +61,6 @@ impl CacheBuddy { } } -/// Default behavior for a cached layer, ensures new metadata is always written -/// -/// The metadadata must implement `CacheDiff` and `TryMigrate` in addition -/// to the typical `Serialize` and `Debug` traits -/// -/// # Errors -/// -/// Returns an error if libcnb cannot read or write the metadata. -pub fn cached_layer_write_metadata( - layer_name: LayerName, - context: &BuildContext, - metadata: &'_ M, -) -> libcnb::Result, Meta>, B::Error> -where - B: libcnb::Buildpack, - M: CacheDiff + TryMigrate + Serialize + Debug + Clone, -{ - CacheBuddy::new().layer(layer_name, context, metadata) -} - /// Standardizes formatting for layer cache clearing behavior /// /// If the diff is empty, there are no changes and the layer is kept and the old data is returned @@ -240,7 +220,7 @@ mod tests { use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; use serde::Deserializer; use std::convert::Infallible; - /// Struct for asserting the behavior of `cached_layer_write_metadata` + /// Struct for asserting the behavior of `CacheBuddy` #[derive(Debug, serde::Serialize, serde::Deserialize, Clone)] #[serde(deny_unknown_fields)] struct TestMetadata { @@ -280,7 +260,7 @@ mod tests { } #[test] - fn test_cached_layer_write_metadata_restored_layer_action() { + fn test_cache_buddy() { let temp = tempfile::tempdir().unwrap(); let context = temp_build_context::( temp.path(), @@ -288,14 +268,15 @@ mod tests { ); // First write - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy::new() + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); assert!(matches!( result.state, LayerState::Empty { @@ -304,28 +285,30 @@ mod tests { )); // Second write, preserve the contents - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy::new() + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); let LayerState::Restored { cause } = &result.state else { panic!("Expected restored layer") }; assert_eq!(cause.as_ref(), "Using cache"); // Third write, change the data - let result = cached_layer_write_metadata( - layer_name!("testing"), - &context, - &TestMetadata { - value: "world".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy::new() + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "world".to_string(), + }, + ) + .unwrap(); let LayerState::Empty { cause: EmptyLayerCause::RestoredLayerAction { cause }, From 563cd2766ea01924734455658662f1ceb0240612 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 12:56:15 -0600 Subject: [PATCH 20/34] Docs --- commons/src/layer/cache_buddy.rs | 52 ++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 3 deletions(-) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 6f1624ce..57c74cfb 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -1,3 +1,45 @@ +//! Your layer cache invalidation pal +//! +//! Cache invalidation is one of the "famously" difficult problems in computer science. This module +//! provides a clean, yet opinonated interface for handling cache invalidation and migrating invalid +//! metadata. +//! +//! - Declarative interface for defining cache invalidation behavior (via [`CacheDiff`]) +//! - Declarative interface for defining invalid metadata migration behavior (via [`TryMigrate`]) +//! - Prevent accidentally reading one struct type and writing a different one +//! +//! The primary interface is [`CacheBuddy`]. +//! +//! ## Cache invalidation logic ([`CacheDiff`]) +//! +//! The `CacheDiff` derive macro from `cache_diff` allows you to tell `CacheBuddy` which fields in your +//! metadata struct act as cache keys and how to compare them. If a difference is reported, the cache +//! is cleared. +//! +//! Importantly, when the cache is cleared, a clear message stating why the cache was cleared is returned +//! in a user readable format. +//! +//! ## Invalid metadata migration ([`TryMigrate`]) +//! +//! If previously serialized metadata cannot be deserialized into the current struct then usually the +//! only thing a buildpack can do is discard the cache. However, that may involve needing to re-do an +//! expensive operation such as re-compiling native libraries. Buildpack authors should feel free to +//! refactor and update their metadata structs without fear of busting the cache. Users should not +//! have to suffer slower builds due to internal only buildpack changes. +//! +//! The `TryMigrate` trait from `magic_migrate` allows buildpack authors to define how to migrate an +//! older struct into a newer one. If the migration fails, the cache is cleared and the reason is returned. +//! If the migration succeeds, then the regular logic in `CacheDiff` is applied. +//! +//! ## Read your write, or (read) why you can't ([`Meta`]) +//! +//! If non-cache data is stored in the Metadata, then your buildpack may want to read that data back. +//! When the cache is not cleared then the old metadata is returned. This allows you to read your write. +//! +//! A buildpack cache should never be cleared without explaining why to a user via printing to the +//! build output. If the cache is cleared for any reason, then a user readable message is returned. This message should +//! be printed to the buildpack user so they can understand what caused the cache to clear. +//! use crate::display::SentenceList; use cache_diff::CacheDiff; use libcnb::build::BuildContext; @@ -11,14 +53,14 @@ use std::fmt::Debug; /// /// Guarantees that new metadata is always written (prevents accidentally reading one struct type and /// writing a different one). It also provides a standard interface to define caching behavior via -/// the `CacheDiff` and `TryMigrate` traits: +/// the [`CacheDiff`] and [`TryMigrate`] traits: /// -/// - The `TryMigrate` trait is for handling invalid metadata: +/// - The [`TryMigrate`] trait is for handling invalid metadata: /// When old metadata from cache is invalid, we try to load it into a known older version and then migrate it /// to the latest via `TryMigrate`. If that fails, the layer is deleted and the error is returned. If it /// succeeds, then the logic in `CacheDiff` below is applied. /// -/// The `CacheDiff` trait defines cache invalidation behavior when metadata is valid: +/// The [`CacheDiff`] trait defines cache invalidation behavior when metadata is valid: /// When a `CacheDiff::diff` is empty, the layer is kept and the old data is returned. Otherwise, /// the layer is deleted and the changes are returned. /// @@ -34,6 +76,10 @@ impl CacheBuddy { Self::default() } + /// Writes metadata to a layer and returns a layer reference with info about prior cache state + /// + /// See the struct documentation for more information. + /// /// # Errors /// /// Returns an error if libcnb cannot read or write the metadata. From 9246ab99132f16e77ad836bcf9067818e0e5b462 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 14:22:53 -0600 Subject: [PATCH 21/34] More docs --- commons/src/layer/cache_buddy.rs | 10 ++- .../src/layer/fixtures/cache_buddy_example.md | 83 +++++++++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 commons/src/layer/fixtures/cache_buddy_example.md diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 57c74cfb..3880da45 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -40,6 +40,8 @@ //! build output. If the cache is cleared for any reason, then a user readable message is returned. This message should //! be printed to the buildpack user so they can understand what caused the cache to clear. //! +#![doc = include_str!("./fixtures/cache_buddy_example.md")] + use crate::display::SentenceList; use cache_diff::CacheDiff; use libcnb::build::BuildContext; @@ -64,6 +66,7 @@ use std::fmt::Debug; /// When a `CacheDiff::diff` is empty, the layer is kept and the old data is returned. Otherwise, /// the layer is deleted and the changes are returned. /// +#[doc = include_str!("./fixtures/cache_buddy_example.md")] #[derive(Default, Debug, Clone, Eq, PartialEq)] pub struct CacheBuddy { pub build: Option, @@ -212,6 +215,12 @@ impl AsRef for Meta { } /// Takes in a directory and returns a minimal build context for use in testing caching behavior +/// +/// Public for use in doc tests, but the interface is not stable. +/// +/// # Panics +/// +/// - If a context cannot be created #[cfg(test)] fn temp_build_context( from_dir: impl AsRef, @@ -284,7 +293,6 @@ mod tests { migrate_toml_chain! {TestMetadata} struct FakeBuildpack; - impl libcnb::Buildpack for FakeBuildpack { type Platform = GenericPlatform; type Metadata = GenericMetadata; diff --git a/commons/src/layer/fixtures/cache_buddy_example.md b/commons/src/layer/fixtures/cache_buddy_example.md new file mode 100644 index 00000000..328c8650 --- /dev/null +++ b/commons/src/layer/fixtures/cache_buddy_example.md @@ -0,0 +1,83 @@ +# Example + +``` +use commons::layer::cache_buddy::CacheBuddy; +use commons::layer::cache_buddy::Meta; +use cache_diff::CacheDiff; +use magic_migrate::TryMigrate; +use serde::Deserializer; + +use libcnb::layer::{LayerState, EmptyLayerCause}; +use libcnb::data::layer_name; + +# #[derive(Debug, serde::Serialize, serde::Deserialize, Clone, cache_diff::CacheDiff)] +# #[serde(deny_unknown_fields)] +# struct TestMetadata { +# value: String, +# } +# use magic_migrate::Migrate; +# magic_migrate::migrate_toml_chain!(TestMetadata); +# +# struct FakeBuildpack; +# +# impl libcnb::Buildpack for FakeBuildpack { +# type Platform = libcnb::generic::GenericPlatform; +# type Metadata = libcnb::generic::GenericMetadata; +# type Error = std::convert::Infallible; +# +# fn detect( +# &self, +# _context: libcnb::detect::DetectContext, +# ) -> libcnb::Result { +# todo!() +# } +# +# fn build( +# &self, +# _context: libcnb::build::BuildContext, +# ) -> libcnb::Result { +# todo!() +# } +# } +# fn install_ruby(path: &std::path::Path) { +# todo!(); +# } +# +# pub(crate) fn call( +# context: &libcnb::build::BuildContext, +# ) -> libcnb::Result<(), ::Error> { +# let metadata_owned = TestMetadata { value: "Hello".to_string() }; +# let metadata = &metadata_owned; +let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?; +match &layer_ref.state { + // CacheDiff reported no difference, cache was kept + LayerState::Restored { cause } => { + println!(" - {cause}"); // States that the cache is being used + match cause { + Meta::Data(old) => { + // Inspect or use the old metadata from cache here if you like! + assert!(metadata.diff(old).is_empty()); + }, + Meta::Message(_) => unreachable!("Will only ever contain metadata when the cache is retained") + } + } + LayerState::Empty { cause } => { + match cause { + // Nothing in old cache + EmptyLayerCause::NewlyCreated => {} + // Problem restoring old cache (`TryMigrate` could not migrate the old metadata) + EmptyLayerCause::InvalidMetadataAction { cause } + // Difference with old cache + | EmptyLayerCause::RestoredLayerAction { cause } => { + // Report why the cache was cleared + println!(" - {cause}"); + } + } + println!(" - Installing"); + install_ruby(&layer_ref.path()); + println!(" - Done"); + } +} +# Ok(()) +# } +``` From 45c63f3f58a2d7b7513b6fcc68e77e1d574d98d2 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 14:29:18 -0600 Subject: [PATCH 22/34] Clarify intermediate struct docs --- commons/src/layer/cache_buddy.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 3880da45..a3dfdbe6 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -174,9 +174,9 @@ where } } -/// Either contains metadata or a message describing the state +/// Either contains (old) metadata or a message describing the state /// -/// Why: The `CachedLayerDefinition` allows returning information about the cache 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 @@ -189,11 +189,10 @@ where /// to inspect the previous metadata they can match on the enum and extract /// what they need. /// +/// When produced using functions in this module: +/// /// - 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 enum Meta { Message(String), Data(M), From f3c0822fe7612225fbb432be675a57d9157bb19d Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 14:31:12 -0600 Subject: [PATCH 23/34] Fix changelog reference The dir is `layer` singular, not `layers` plural. --- commons/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commons/CHANGELOG.md b/commons/CHANGELOG.md index 25be13bd..ab31fc0e 100644 --- a/commons/CHANGELOG.md +++ b/commons/CHANGELOG.md @@ -4,7 +4,7 @@ ## Changed -- Deprecate `layers` including `layers::ConfigureEnvLayer` and `layers::DefaultEnvLayer` (https://github.com/heroku/buildpacks-ruby/pull/345) +- Deprecate `layer` including `layer::ConfigureEnvLayer` and `layer::DefaultEnvLayer` (https://github.com/heroku/buildpacks-ruby/pull/345) - Remove `AppCacheCollection` (https://github.com/heroku/buildpacks-ruby/pull/345) - Deprecate `output` module in favor of the `bullet_stream` crate (https://github.com/heroku/buildpacks-ruby/pull/345) From 580ca5b69e5a20e2ce20f09f5bd9f2742968f0f2 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 18 Dec 2024 14:33:26 -0600 Subject: [PATCH 24/34] Update changelog --- commons/CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/commons/CHANGELOG.md b/commons/CHANGELOG.md index ab31fc0e..557410b9 100644 --- a/commons/CHANGELOG.md +++ b/commons/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog for commons features +### Added + +- Introduced `layer::cache_buddy::CacheBuddy` for public cache use (https://github.com/heroku/buildpacks-ruby/pull/376) + +### Changed + +- The `layer` module is no longer deprecated, only `layer::ConfigureEnvLayer` and `layer::DefaultEnvLayer` (https://github.com/heroku/buildpacks-ruby/pull/376) + ## 2024-11-11 ## Changed From f5e8ab8e5586acd607b915f1d69975b8aad4e02f Mon Sep 17 00:00:00 2001 From: Schneems Date: Thu, 19 Dec 2024 09:53:30 -0600 Subject: [PATCH 25/34] Remove unused `since` versions --- commons/src/layer.rs | 10 ++-------- commons/src/output/build_log.rs | 2 +- commons/src/output/fmt.rs | 12 ++++++------ commons/src/output/section_log.rs | 14 +++++++------- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/commons/src/layer.rs b/commons/src/layer.rs index d28d672b..5edf280c 100644 --- a/commons/src/layer.rs +++ b/commons/src/layer.rs @@ -2,14 +2,8 @@ pub mod cache_buddy; mod configure_env_layer; mod default_env_layer; -#[deprecated( - since = "0.0.0", - note = "Use the struct layer API in the latest libcnb.rs instead" -)] +#[deprecated(note = "Use the struct layer API in the latest libcnb.rs instead")] pub use self::configure_env_layer::ConfigureEnvLayer; -#[deprecated( - since = "0.0.0", - note = "Use the struct layer API in the latest libcnb.rs instead" -)] +#[deprecated(note = "Use the struct layer API in the latest libcnb.rs instead")] pub use self::default_env_layer::DefaultEnvLayer; diff --git a/commons/src/output/build_log.rs b/commons/src/output/build_log.rs index d22e9a4e..888d0920 100644 --- a/commons/src/output/build_log.rs +++ b/commons/src/output/build_log.rs @@ -32,7 +32,7 @@ use std::time::{Duration, Instant}; /// To log inside of a layer see [`section_log`]. #[derive(Debug)] -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub struct BuildLog { pub(crate) io: W, pub(crate) data: BuildData, diff --git a/commons/src/output/fmt.rs b/commons/src/output/fmt.rs index f5f27037..3f73cd9d 100644 --- a/commons/src/output/fmt.rs +++ b/commons/src/output/fmt.rs @@ -4,30 +4,30 @@ use const_format::formatcp; use std::fmt::Write; /// Decorated str for prefixing "Help:" -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub const HELP: &str = formatcp!("{IMPORTANT_COLOR}! HELP{RESET}"); /// Decorated str for prefixing "Debug info:" -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub const DEBUG_INFO: &str = formatcp!("{IMPORTANT_COLOR}Debug info{RESET}"); /// Decorate a URL for the build output #[must_use] -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn url(contents: impl AsRef) -> String { colorize(URL_COLOR, contents) } /// Decorate the name of a command being run i.e. `bundle install` #[must_use] -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn command(contents: impl AsRef) -> String { value(colorize(COMMAND_COLOR, contents.as_ref())) } /// Decorate an important value i.e. `2.3.4` #[must_use] -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn value(contents: impl AsRef) -> String { let contents = colorize(VALUE_COLOR, contents.as_ref()); format!("`{contents}`") @@ -35,7 +35,7 @@ pub fn value(contents: impl AsRef) -> String { /// Decorate additional information at the end of a line #[must_use] -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn details(contents: impl AsRef) -> String { let contents = contents.as_ref(); format!("({contents})") diff --git a/commons/src/output/section_log.rs b/commons/src/output/section_log.rs index 44883426..0ecdfc13 100644 --- a/commons/src/output/section_log.rs +++ b/commons/src/output/section_log.rs @@ -45,7 +45,7 @@ use std::marker::PhantomData; /// /// log_step("Clearing cache (ruby version changed)"); /// ``` -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_step(s: impl AsRef) { logger().step(s.as_ref()); } @@ -62,7 +62,7 @@ pub fn log_step(s: impl AsRef) { /// ``` /// /// Timing information will be output at the end of the step. -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_step_timed(s: impl AsRef, f: impl FnOnce() -> T) -> T { let timer = logger().step_timed(s.as_ref()); let out = f(); @@ -70,7 +70,7 @@ pub fn log_step_timed(s: impl AsRef, f: impl FnOnce() -> T) -> T { out } -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_step_stream( s: impl AsRef, f: impl FnOnce(&mut Box) -> T, @@ -82,25 +82,25 @@ pub fn log_step_stream( } /// Print an error block to the output -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_error(s: impl AsRef) { logger().announce().error(s.as_ref()); } /// Print an warning block to the output -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_warning(s: impl AsRef) { logger().announce().warning(s.as_ref()); } /// Print an warning block to the output at a later time -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_warning_later(s: impl AsRef) { logger().announce().warn_later(s.as_ref()); } /// Print an important block to the output -#[deprecated(since = "0.0.0", note = "Use `bullet_stream` instead")] +#[deprecated(note = "Use `bullet_stream` instead")] pub fn log_important(s: impl AsRef) { logger().announce().important(s.as_ref()); } From aa76d601ac0a4daade442f359a4ed0b21a244855 Mon Sep 17 00:00:00 2001 From: Schneems Date: Thu, 19 Dec 2024 15:37:19 -0600 Subject: [PATCH 26/34] Move test helper into mod tests --- commons/src/layer/cache_buddy.rs | 98 ++++++++++++++++---------------- 1 file changed, 48 insertions(+), 50 deletions(-) diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index a3dfdbe6..e04efa49 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -213,56 +213,6 @@ impl AsRef for Meta { } } -/// Takes in a directory and returns a minimal build context for use in testing caching behavior -/// -/// Public for use in doc tests, but the interface is not stable. -/// -/// # Panics -/// -/// - If a context cannot be created -#[cfg(test)] -fn temp_build_context( - from_dir: impl AsRef, - buildpack_toml_string: &str, -) -> libcnb::build::BuildContext { - let base_dir = from_dir.as_ref().to_path_buf(); - let layers_dir = base_dir.join("layers"); - let app_dir = base_dir.join("app_dir"); - let platform_dir = base_dir.join("platform_dir"); - let buildpack_dir = base_dir.join("buildpack_dir"); - for dir in [&app_dir, &layers_dir, &buildpack_dir, &platform_dir] { - std::fs::create_dir_all(dir).unwrap(); - } - - let target = libcnb::Target { - os: String::new(), - arch: String::new(), - arch_variant: None, - distro_name: String::new(), - distro_version: String::new(), - }; - let platform = - <::Platform as libcnb::Platform>::from_path(&platform_dir).unwrap(); - let buildpack_descriptor: libcnb::data::buildpack::ComponentBuildpackDescriptor< - ::Metadata, - > = toml::from_str(buildpack_toml_string).unwrap(); - let buildpack_plan = libcnb::data::buildpack_plan::BuildpackPlan { - entries: Vec::::new(), - }; - let store = None; - - libcnb::build::BuildContext { - layers_dir, - app_dir, - buildpack_dir, - target, - platform, - buildpack_plan, - buildpack_descriptor, - store, - } -} - #[cfg(test)] mod tests { use super::*; @@ -479,4 +429,52 @@ mod tests { ); // Unable to produce this error at will: "Clearing cache due to invalid metadata serialization error: {error}" } + + /// Takes in a directory and returns a minimal build context for use in testing caching behavior + /// + /// # Panics + /// + /// - If a context cannot be created + fn temp_build_context( + from_dir: impl AsRef, + buildpack_toml_string: &str, + ) -> libcnb::build::BuildContext { + let base_dir = from_dir.as_ref().to_path_buf(); + let layers_dir = base_dir.join("layers"); + let app_dir = base_dir.join("app_dir"); + let platform_dir = base_dir.join("platform_dir"); + let buildpack_dir = base_dir.join("buildpack_dir"); + for dir in [&app_dir, &layers_dir, &buildpack_dir, &platform_dir] { + std::fs::create_dir_all(dir).unwrap(); + } + + let target = libcnb::Target { + os: String::new(), + arch: String::new(), + arch_variant: None, + distro_name: String::new(), + distro_version: String::new(), + }; + let platform = + <::Platform as libcnb::Platform>::from_path(&platform_dir) + .unwrap(); + let buildpack_descriptor: libcnb::data::buildpack::ComponentBuildpackDescriptor< + ::Metadata, + > = toml::from_str(buildpack_toml_string).unwrap(); + let buildpack_plan = libcnb::data::buildpack_plan::BuildpackPlan { + entries: Vec::::new(), + }; + let store = None; + + libcnb::build::BuildContext { + layers_dir, + app_dir, + buildpack_dir, + target, + platform, + buildpack_plan, + buildpack_descriptor, + store, + } + } } From 193ff02efaf4fe78c0b5aaa29e315e2ad3b754f5 Mon Sep 17 00:00:00 2001 From: Schneems Date: Thu, 19 Dec 2024 15:42:13 -0600 Subject: [PATCH 27/34] Require explicit values These map 1:1 with `CachedLayerDefinition`. The extra indirection of supporting default values isn't buying us much. --- .../ruby/src/layers/bundle_download_layer.rs | 6 +- .../ruby/src/layers/bundle_install_layer.rs | 6 +- .../ruby/src/layers/ruby_install_layer.rs | 33 +++++--- commons/src/layer/cache_buddy.rs | 78 ++++++++++--------- .../src/layer/fixtures/cache_buddy_example.md | 6 +- 5 files changed, 79 insertions(+), 50 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 4fb0105d..8fe8cce3 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -29,7 +29,11 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy::new().layer(layer_name!("bundler"), context, metadata)?; + let layer_ref = CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("bundler"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 1d505d31..320302ee 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -52,7 +52,11 @@ pub(crate) fn handle( metadata: &Metadata, without: &BundleWithout, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy::new().layer(layer_name!("gems"), context, metadata)?; + let layer_ref = CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("gems"), context, metadata)?; let install_state = match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 976fd629..99f551fa 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -39,7 +39,11 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?; + let layer_ref = CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); @@ -388,12 +392,18 @@ version = "3.1.3" let differences = old.diff(&old); assert_eq!(differences, Vec::::new()); - CacheBuddy::new() - .layer(layer_name!("ruby"), &context, &old) - .unwrap(); - let result = CacheBuddy::new() - .layer(layer_name!("ruby"), &context, &old) - .unwrap(); + CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("ruby"), &context, &old) + .unwrap(); + let result = CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("ruby"), &context, &old) + .unwrap(); let actual = result.state; assert!(matches!(actual, LayerState::Restored { .. })); @@ -404,9 +414,12 @@ version = "3.1.3" let differences = now.diff(&old); assert_eq!(differences.len(), 1); - let result = CacheBuddy::new() - .layer(layer_name!("ruby"), &context, &now) - .unwrap(); + let result = CacheBuddy { + build: true, + launch: true, + } + .layer(layer_name!("ruby"), &context, &now) + .unwrap(); assert!(matches!( result.state, LayerState::Empty { diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index e04efa49..be0d3144 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -67,18 +67,13 @@ use std::fmt::Debug; /// the layer is deleted and the changes are returned. /// #[doc = include_str!("./fixtures/cache_buddy_example.md")] -#[derive(Default, Debug, Clone, Eq, PartialEq)] +#[derive(Debug, Clone, Eq, PartialEq)] pub struct CacheBuddy { - pub build: Option, - pub launch: Option, + pub build: bool, + pub launch: bool, } impl CacheBuddy { - #[must_use] - pub fn new() -> Self { - Self::default() - } - /// Writes metadata to a layer and returns a layer reference with info about prior cache state /// /// See the struct documentation for more information. @@ -99,8 +94,8 @@ impl CacheBuddy { let layer_ref = context.cached_layer( layer_name, CachedLayerDefinition { - build: self.build.unwrap_or(true), - launch: self.launch.unwrap_or(true), + build: self.build, + launch: self.launch, invalid_metadata_action: &invalid_metadata_action, restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), }, @@ -271,15 +266,18 @@ mod tests { ); // First write - let result = CacheBuddy::new() - .layer( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy { + build: true, + launch: true, + } + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); assert!(matches!( result.state, LayerState::Empty { @@ -288,30 +286,36 @@ mod tests { )); // Second write, preserve the contents - let result = CacheBuddy::new() - .layer( - layer_name!("testing"), - &context, - &TestMetadata { - value: "hello".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy { + build: true, + launch: true, + } + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "hello".to_string(), + }, + ) + .unwrap(); let LayerState::Restored { cause } = &result.state else { panic!("Expected restored layer") }; assert_eq!(cause.as_ref(), "Using cache"); // Third write, change the data - let result = CacheBuddy::new() - .layer( - layer_name!("testing"), - &context, - &TestMetadata { - value: "world".to_string(), - }, - ) - .unwrap(); + let result = CacheBuddy { + build: true, + launch: true, + } + .layer( + layer_name!("testing"), + &context, + &TestMetadata { + value: "world".to_string(), + }, + ) + .unwrap(); let LayerState::Empty { cause: EmptyLayerCause::RestoredLayerAction { cause }, diff --git a/commons/src/layer/fixtures/cache_buddy_example.md b/commons/src/layer/fixtures/cache_buddy_example.md index 328c8650..9f679684 100644 --- a/commons/src/layer/fixtures/cache_buddy_example.md +++ b/commons/src/layer/fixtures/cache_buddy_example.md @@ -48,7 +48,11 @@ use libcnb::data::layer_name; # ) -> libcnb::Result<(), ::Error> { # let metadata_owned = TestMetadata { value: "Hello".to_string() }; # let metadata = &metadata_owned; -let layer_ref = CacheBuddy::new().layer(layer_name!("ruby"), context, metadata)?; +let layer_ref = CacheBuddy { + build: true, + launch: true, +} +.layer(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { // CacheDiff reported no difference, cache was kept LayerState::Restored { cause } => { From c006c7c470b995a6b492469a889b934ddde8f714 Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 09:32:40 -0600 Subject: [PATCH 28/34] Use `cached_layer` to match API with BuildContext::cached_layer --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 2 +- buildpacks/ruby/src/layers/bundle_install_layer.rs | 2 +- buildpacks/ruby/src/layers/ruby_install_layer.rs | 8 ++++---- commons/src/layer/cache_buddy.rs | 8 ++++---- commons/src/layer/fixtures/cache_buddy_example.md | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 8fe8cce3..e9245d7e 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -33,7 +33,7 @@ pub(crate) fn handle( build: true, launch: true, } - .layer(layer_name!("bundler"), context, metadata)?; + .cached_layer(layer_name!("bundler"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 320302ee..498ff06b 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -56,7 +56,7 @@ pub(crate) fn handle( build: true, launch: true, } - .layer(layer_name!("gems"), context, metadata)?; + .cached_layer(layer_name!("gems"), context, metadata)?; let install_state = match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 99f551fa..f8800e3d 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -43,7 +43,7 @@ pub(crate) fn handle( build: true, launch: true, } - .layer(layer_name!("ruby"), context, metadata)?; + .cached_layer(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause } => { bullet = bullet.sub_bullet(cause); @@ -396,13 +396,13 @@ version = "3.1.3" build: true, launch: true, } - .layer(layer_name!("ruby"), &context, &old) + .cached_layer(layer_name!("ruby"), &context, &old) .unwrap(); let result = CacheBuddy { build: true, launch: true, } - .layer(layer_name!("ruby"), &context, &old) + .cached_layer(layer_name!("ruby"), &context, &old) .unwrap(); let actual = result.state; assert!(matches!(actual, LayerState::Restored { .. })); @@ -418,7 +418,7 @@ version = "3.1.3" build: true, launch: true, } - .layer(layer_name!("ruby"), &context, &now) + .cached_layer(layer_name!("ruby"), &context, &now) .unwrap(); assert!(matches!( result.state, diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index be0d3144..3b6bf441 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -81,7 +81,7 @@ impl CacheBuddy { /// # Errors /// /// Returns an error if libcnb cannot read or write the metadata. - pub fn layer( + pub fn cached_layer( self, layer_name: LayerName, context: &BuildContext, @@ -270,7 +270,7 @@ mod tests { build: true, launch: true, } - .layer( + .cached_layer( layer_name!("testing"), &context, &TestMetadata { @@ -290,7 +290,7 @@ mod tests { build: true, launch: true, } - .layer( + .cached_layer( layer_name!("testing"), &context, &TestMetadata { @@ -308,7 +308,7 @@ mod tests { build: true, launch: true, } - .layer( + .cached_layer( layer_name!("testing"), &context, &TestMetadata { diff --git a/commons/src/layer/fixtures/cache_buddy_example.md b/commons/src/layer/fixtures/cache_buddy_example.md index 9f679684..4faa510f 100644 --- a/commons/src/layer/fixtures/cache_buddy_example.md +++ b/commons/src/layer/fixtures/cache_buddy_example.md @@ -52,7 +52,7 @@ let layer_ref = CacheBuddy { build: true, launch: true, } -.layer(layer_name!("ruby"), context, metadata)?; +.cached_layer(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { // CacheDiff reported no difference, cache was kept LayerState::Restored { cause } => { From 99cbd2c795fc9f564e1d3b21bd27c86a6e846e53 Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 09:40:14 -0600 Subject: [PATCH 29/34] Use more informative name --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 4 ++-- buildpacks/ruby/src/layers/bundle_install_layer.rs | 4 ++-- buildpacks/ruby/src/layers/ruby_install_layer.rs | 10 +++++----- commons/src/layer/cache_buddy.rs | 12 +++++++----- commons/src/layer/fixtures/cache_buddy_example.md | 4 ++-- 5 files changed, 18 insertions(+), 16 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index e9245d7e..222c727d 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -10,7 +10,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedBundlerVersion; -use commons::layer::cache_buddy::CacheBuddy; +use commons::layer::cache_buddy::DiffMigrateLayer; use fun_run::{self, CommandWithName}; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; @@ -29,7 +29,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy { + let layer_ref = DiffMigrateLayer { build: true, launch: true, } diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 498ff06b..36900fdd 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -19,7 +19,7 @@ use crate::{BundleWithout, RubyBuildpack, RubyBuildpackError}; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; -use commons::layer::cache_buddy::{CacheBuddy, Meta}; +use commons::layer::cache_buddy::{DiffMigrateLayer, Meta}; use commons::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; @@ -52,7 +52,7 @@ pub(crate) fn handle( metadata: &Metadata, without: &BundleWithout, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy { + let layer_ref = DiffMigrateLayer { build: true, launch: true, } diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index f8800e3d..26ef29dc 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -20,7 +20,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::Print; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedRubyVersion; -use commons::layer::cache_buddy::CacheBuddy; +use commons::layer::cache_buddy::DiffMigrateLayer; use flate2::read::GzDecoder; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; @@ -39,7 +39,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = CacheBuddy { + let layer_ref = DiffMigrateLayer { build: true, launch: true, } @@ -392,13 +392,13 @@ version = "3.1.3" let differences = old.diff(&old); assert_eq!(differences, Vec::::new()); - CacheBuddy { + DiffMigrateLayer { build: true, launch: true, } .cached_layer(layer_name!("ruby"), &context, &old) .unwrap(); - let result = CacheBuddy { + let result = DiffMigrateLayer { build: true, launch: true, } @@ -414,7 +414,7 @@ version = "3.1.3" let differences = now.diff(&old); assert_eq!(differences.len(), 1); - let result = CacheBuddy { + let result = DiffMigrateLayer { build: true, launch: true, } diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/cache_buddy.rs index 3b6bf441..fff599e9 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/cache_buddy.rs @@ -68,12 +68,14 @@ use std::fmt::Debug; /// #[doc = include_str!("./fixtures/cache_buddy_example.md")] #[derive(Debug, Clone, Eq, PartialEq)] -pub struct CacheBuddy { +pub struct DiffMigrateLayer { + /// Whether the layer is intended for build. pub build: bool, + /// Whether the layer is intended for launch. pub launch: bool, } -impl CacheBuddy { +impl DiffMigrateLayer { /// Writes metadata to a layer and returns a layer reference with info about prior cache state /// /// See the struct documentation for more information. @@ -266,7 +268,7 @@ mod tests { ); // First write - let result = CacheBuddy { + let result = DiffMigrateLayer { build: true, launch: true, } @@ -286,7 +288,7 @@ mod tests { )); // Second write, preserve the contents - let result = CacheBuddy { + let result = DiffMigrateLayer { build: true, launch: true, } @@ -304,7 +306,7 @@ mod tests { assert_eq!(cause.as_ref(), "Using cache"); // Third write, change the data - let result = CacheBuddy { + let result = DiffMigrateLayer { build: true, launch: true, } diff --git a/commons/src/layer/fixtures/cache_buddy_example.md b/commons/src/layer/fixtures/cache_buddy_example.md index 4faa510f..35282eb4 100644 --- a/commons/src/layer/fixtures/cache_buddy_example.md +++ b/commons/src/layer/fixtures/cache_buddy_example.md @@ -1,7 +1,7 @@ # Example ``` -use commons::layer::cache_buddy::CacheBuddy; +use commons::layer::cache_buddy::DiffMigrateLayer; use commons::layer::cache_buddy::Meta; use cache_diff::CacheDiff; use magic_migrate::TryMigrate; @@ -48,7 +48,7 @@ use libcnb::data::layer_name; # ) -> libcnb::Result<(), ::Error> { # let metadata_owned = TestMetadata { value: "Hello".to_string() }; # let metadata = &metadata_owned; -let layer_ref = CacheBuddy { +let layer_ref = DiffMigrateLayer { build: true, launch: true, } From 0b186fc1e7d6acf758d016ab9416551caeef50ef Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 09:43:34 -0600 Subject: [PATCH 30/34] Rename module --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 2 +- buildpacks/ruby/src/layers/bundle_install_layer.rs | 2 +- buildpacks/ruby/src/layers/ruby_install_layer.rs | 2 +- commons/src/layer.rs | 2 +- commons/src/layer/{cache_buddy.rs => diff_migrate.rs} | 6 +++--- .../{cache_buddy_example.md => diff_migrate_example.md} | 3 +-- 6 files changed, 8 insertions(+), 9 deletions(-) rename commons/src/layer/{cache_buddy.rs => diff_migrate.rs} (99%) rename commons/src/layer/fixtures/{cache_buddy_example.md => diff_migrate_example.md} (96%) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 222c727d..bfcc77a6 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -10,7 +10,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedBundlerVersion; -use commons::layer::cache_buddy::DiffMigrateLayer; +use commons::layer::diff_migrate::DiffMigrateLayer; use fun_run::{self, CommandWithName}; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 36900fdd..c53663a2 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -19,7 +19,7 @@ use crate::{BundleWithout, RubyBuildpack, RubyBuildpackError}; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use cache_diff::CacheDiff; -use commons::layer::cache_buddy::{DiffMigrateLayer, Meta}; +use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; use commons::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 26ef29dc..64515f98 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -20,7 +20,7 @@ use bullet_stream::state::SubBullet; use bullet_stream::Print; use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedRubyVersion; -use commons::layer::cache_buddy::DiffMigrateLayer; +use commons::layer::diff_migrate::DiffMigrateLayer; use flate2::read::GzDecoder; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; diff --git a/commons/src/layer.rs b/commons/src/layer.rs index 5edf280c..4ec3cade 100644 --- a/commons/src/layer.rs +++ b/commons/src/layer.rs @@ -1,6 +1,6 @@ -pub mod cache_buddy; mod configure_env_layer; mod default_env_layer; +pub mod diff_migrate; #[deprecated(note = "Use the struct layer API in the latest libcnb.rs instead")] pub use self::configure_env_layer::ConfigureEnvLayer; diff --git a/commons/src/layer/cache_buddy.rs b/commons/src/layer/diff_migrate.rs similarity index 99% rename from commons/src/layer/cache_buddy.rs rename to commons/src/layer/diff_migrate.rs index fff599e9..a5183172 100644 --- a/commons/src/layer/cache_buddy.rs +++ b/commons/src/layer/diff_migrate.rs @@ -40,7 +40,7 @@ //! build output. If the cache is cleared for any reason, then a user readable message is returned. This message should //! be printed to the buildpack user so they can understand what caused the cache to clear. //! -#![doc = include_str!("./fixtures/cache_buddy_example.md")] +#![doc = include_str!("./fixtures/diff_migrate_example.md")] use crate::display::SentenceList; use cache_diff::CacheDiff; @@ -66,7 +66,7 @@ use std::fmt::Debug; /// When a `CacheDiff::diff` is empty, the layer is kept and the old data is returned. Otherwise, /// the layer is deleted and the changes are returned. /// -#[doc = include_str!("./fixtures/cache_buddy_example.md")] +#[doc = include_str!("./fixtures/diff_migrate_example.md")] #[derive(Debug, Clone, Eq, PartialEq)] pub struct DiffMigrateLayer { /// Whether the layer is intended for build. @@ -260,7 +260,7 @@ mod tests { } #[test] - fn test_cache_buddy() { + fn test_diff_migrate() { let temp = tempfile::tempdir().unwrap(); let context = temp_build_context::( temp.path(), diff --git a/commons/src/layer/fixtures/cache_buddy_example.md b/commons/src/layer/fixtures/diff_migrate_example.md similarity index 96% rename from commons/src/layer/fixtures/cache_buddy_example.md rename to commons/src/layer/fixtures/diff_migrate_example.md index 35282eb4..afecae4c 100644 --- a/commons/src/layer/fixtures/cache_buddy_example.md +++ b/commons/src/layer/fixtures/diff_migrate_example.md @@ -1,8 +1,7 @@ # Example ``` -use commons::layer::cache_buddy::DiffMigrateLayer; -use commons::layer::cache_buddy::Meta; +use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; use cache_diff::CacheDiff; use magic_migrate::TryMigrate; use serde::Deserializer; From da814e91c14fc4902e7da6d87b139bde9f7656ab Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 09:47:30 -0600 Subject: [PATCH 31/34] Update magic_migrate --- Cargo.lock | 4 ++-- commons/Cargo.toml | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9e759eb3..9ee78415 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -970,9 +970,9 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "magic_migrate" -version = "1.0.0" +version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6dbe7e54cb4d9e457cc23ef8e27ed148fbecf630c0b4675729310995bca2f04" +checksum = "5a173b20fd1f2e55ecc1fe680c88e039f9962dcf84b2531009bcdf6f2d589186" dependencies = [ "serde", ] diff --git a/commons/Cargo.toml b/commons/Cargo.toml index 686797b9..c363b919 100644 --- a/commons/Cargo.toml +++ b/commons/Cargo.toml @@ -27,7 +27,7 @@ tempfile = "3" thiserror = "2" walkdir = "2" filetime = "0.2" -magic_migrate = "1.0" +magic_migrate = "1.0.1" toml = "0.8" cache_diff = "1.0" From 91202e92a237f70fe0b333e0b232556d894295b5 Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 09:47:54 -0600 Subject: [PATCH 32/34] Remove unused imports --- buildpacks/ruby/src/layers/bundle_download_layer.rs | 3 +-- buildpacks/ruby/src/layers/bundle_install_layer.rs | 5 ++--- buildpacks/ruby/src/layers/ruby_install_layer.rs | 3 +-- commons/src/layer/diff_migrate.rs | 1 - commons/src/layer/fixtures/diff_migrate_example.md | 1 - 5 files changed, 4 insertions(+), 9 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index bfcc77a6..ea3e91b5 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -17,8 +17,7 @@ use libcnb::layer::{EmptyLayerCause, LayerState}; use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope}; use libcnb::Env; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; -use serde::{Deserialize, Deserializer, Serialize}; -use std::convert::Infallible; +use serde::{Deserialize, Serialize}; use std::io::Stdout; use std::path::Path; use std::process::Command; diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index c53663a2..17f955ad 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -31,8 +31,7 @@ use libcnb::{ Env, }; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; -use serde::{Deserialize, Deserializer, Serialize}; -use std::convert::Infallible; +use serde::{Deserialize, Serialize}; use std::io::Stdout; use std::{path::Path, process::Command}; @@ -202,7 +201,7 @@ impl TryFrom for MetadataV2 { } impl TryFrom for MetadataV3 { - type Error = Infallible; + type Error = std::convert::Infallible; fn try_from(v2: MetadataV2) -> Result { Ok(Self { diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 64515f98..aad8b1f5 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -26,8 +26,7 @@ use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; use libcnb::layer_env::LayerEnv; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; -use serde::{Deserialize, Deserializer, Serialize}; -use std::convert::Infallible; +use serde::{Deserialize, Serialize}; use std::io::{self, Stdout}; use std::path::Path; use tar::Archive; diff --git a/commons/src/layer/diff_migrate.rs b/commons/src/layer/diff_migrate.rs index a5183172..6ec9c7a3 100644 --- a/commons/src/layer/diff_migrate.rs +++ b/commons/src/layer/diff_migrate.rs @@ -219,7 +219,6 @@ mod tests { use libcnb::generic::{GenericMetadata, GenericPlatform}; use libcnb::layer::{EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction}; use magic_migrate::{migrate_toml_chain, try_migrate_deserializer_chain, Migrate, TryMigrate}; - use serde::Deserializer; use std::convert::Infallible; /// Struct for asserting the behavior of `CacheBuddy` #[derive(Debug, serde::Serialize, serde::Deserialize, Clone)] diff --git a/commons/src/layer/fixtures/diff_migrate_example.md b/commons/src/layer/fixtures/diff_migrate_example.md index afecae4c..6d49ca11 100644 --- a/commons/src/layer/fixtures/diff_migrate_example.md +++ b/commons/src/layer/fixtures/diff_migrate_example.md @@ -4,7 +4,6 @@ use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; use cache_diff::CacheDiff; use magic_migrate::TryMigrate; -use serde::Deserializer; use libcnb::layer::{LayerState, EmptyLayerCause}; use libcnb::data::layer_name; From ae49cf08c7f2594d239814ebe3792b94c68ccfc0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Fri, 20 Dec 2024 19:42:16 -0600 Subject: [PATCH 33/34] Update docs and add a tutorial --- Cargo.lock | 1 + commons/CHANGELOG.md | 2 +- commons/Cargo.toml | 1 + commons/src/layer/diff_migrate.rs | 43 +- .../layer/fixtures/diff_migrate_example.md | 85 ---- .../fixtures/metadata_migration_example.md | 404 ++++++++++++++++++ 6 files changed, 440 insertions(+), 96 deletions(-) delete mode 100644 commons/src/layer/fixtures/diff_migrate_example.md create mode 100644 commons/src/layer/fixtures/metadata_migration_example.md diff --git a/Cargo.lock b/Cargo.lock index 9ee78415..1597fc38 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -290,6 +290,7 @@ checksum = "f46ad14479a25103f283c0f10005961cf086d8dc42205bb44c46ac563475dca6" name = "commons" version = "0.0.0" dependencies = [ + "bullet_stream", "byte-unit", "cache_diff", "const_format", diff --git a/commons/CHANGELOG.md b/commons/CHANGELOG.md index 557410b9..dfa3109e 100644 --- a/commons/CHANGELOG.md +++ b/commons/CHANGELOG.md @@ -2,7 +2,7 @@ ### Added -- Introduced `layer::cache_buddy::CacheBuddy` for public cache use (https://github.com/heroku/buildpacks-ruby/pull/376) +- Introduced `layer::diff_migrate` and `DiffMigrateLayer` for public cache use (https://github.com/heroku/buildpacks-ruby/pull/376) ### Changed diff --git a/commons/Cargo.toml b/commons/Cargo.toml index c363b919..bbfb0d44 100644 --- a/commons/Cargo.toml +++ b/commons/Cargo.toml @@ -37,3 +37,4 @@ indoc = "2" libcnb-test = "=0.26.1" pretty_assertions = "1" toml = "0.8" +bullet_stream = "0.3.0" diff --git a/commons/src/layer/diff_migrate.rs b/commons/src/layer/diff_migrate.rs index 6ec9c7a3..4476f6fd 100644 --- a/commons/src/layer/diff_migrate.rs +++ b/commons/src/layer/diff_migrate.rs @@ -1,25 +1,25 @@ -//! Your layer cache invalidation pal +//! Declarative Layer Cache invalidation logic. //! //! Cache invalidation is one of the "famously" difficult problems in computer science. This module //! provides a clean, yet opinonated interface for handling cache invalidation and migrating invalid //! metadata. //! -//! - Declarative interface for defining cache invalidation behavior (via [`CacheDiff`]) -//! - Declarative interface for defining invalid metadata migration behavior (via [`TryMigrate`]) +//! - Declarative interface for defining cache invalidation behavior (via [`cache_diff::CacheDiff`]) +//! - Declarative interface for defining invalid metadata migration behavior (via [`magic_migrate::TryMigrate`]) //! - Prevent accidentally reading one struct type and writing a different one //! -//! The primary interface is [`CacheBuddy`]. +//! The primary interface is [`DiffMigrateLayer`]. //! -//! ## Cache invalidation logic ([`CacheDiff`]) +//! ## Cache invalidation logic ([`cache_diff::CacheDiff`]) //! -//! The `CacheDiff` derive macro from `cache_diff` allows you to tell `CacheBuddy` which fields in your +//! The `CacheDiff` derive macro from `cache_diff` allows you to tell [`DiffMigrateLayer`] which fields in your //! metadata struct act as cache keys and how to compare them. If a difference is reported, the cache //! is cleared. //! //! Importantly, when the cache is cleared, a clear message stating why the cache was cleared is returned //! in a user readable format. //! -//! ## Invalid metadata migration ([`TryMigrate`]) +//! ## Invalid metadata migration ([`magic_migrate::TryMigrate`]) //! //! If previously serialized metadata cannot be deserialized into the current struct then usually the //! only thing a buildpack can do is discard the cache. However, that may involve needing to re-do an @@ -40,7 +40,7 @@ //! build output. If the cache is cleared for any reason, then a user readable message is returned. This message should //! be printed to the buildpack user so they can understand what caused the cache to clear. //! -#![doc = include_str!("./fixtures/diff_migrate_example.md")] +#![doc = include_str!("fixtures/metadata_migration_example.md")] use crate::display::SentenceList; use cache_diff::CacheDiff; @@ -51,7 +51,30 @@ use magic_migrate::TryMigrate; use serde::ser::Serialize; use std::fmt::Debug; -/// Handle caching behavior for a layer +#[cfg(test)] +use bullet_stream as _; + +/// Creates a cached layer, potentially re-using a previously cached version with default invalidation and migration logic. +/// +/// Like [`BuildContext::cached_layer`], this allows Buildpack code to create a cached layer and get +/// back a reference to the layer directory on disk. Intricacies of the CNB spec are automatically handled +/// such as the maintenance of TOML files. +/// +/// In addition it provides default behavior for cache invalidation, automatic invalid metadata migration, +/// as well as ensuring that the latest metadata is set on the layer. +/// +/// Uses [`BuildContext::cached_layer`] with declarative traits [`CacheDiff`] for invalidation and [`TryMigrate`] +/// for migration logic. +/// The behavior here can be manually assembled using the provided struct [`Meta`] and functions: +/// +/// - [`invalid_metadata_action`] +/// - [`restored_layer_action`] +/// +/// In addition to default behavior it also ensures that the metadata is updated. +/// +/// The return is a [`LayerRef`] as if you had manually assembled your own [`BuildContext::cached_layer`] +/// call. This allows users to be flexible in how and when the layer is modified and to abstract layer +/// creation away if necessary. /// /// Guarantees that new metadata is always written (prevents accidentally reading one struct type and /// writing a different one). It also provides a standard interface to define caching behavior via @@ -66,7 +89,7 @@ use std::fmt::Debug; /// When a `CacheDiff::diff` is empty, the layer is kept and the old data is returned. Otherwise, /// the layer is deleted and the changes are returned. /// -#[doc = include_str!("./fixtures/diff_migrate_example.md")] +/// **TUTORIAL:** In the [`diff_migrate`] module docs #[derive(Debug, Clone, Eq, PartialEq)] pub struct DiffMigrateLayer { /// Whether the layer is intended for build. diff --git a/commons/src/layer/fixtures/diff_migrate_example.md b/commons/src/layer/fixtures/diff_migrate_example.md deleted file mode 100644 index 6d49ca11..00000000 --- a/commons/src/layer/fixtures/diff_migrate_example.md +++ /dev/null @@ -1,85 +0,0 @@ -# Example - -``` -use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; -use cache_diff::CacheDiff; -use magic_migrate::TryMigrate; - -use libcnb::layer::{LayerState, EmptyLayerCause}; -use libcnb::data::layer_name; - -# #[derive(Debug, serde::Serialize, serde::Deserialize, Clone, cache_diff::CacheDiff)] -# #[serde(deny_unknown_fields)] -# struct TestMetadata { -# value: String, -# } -# use magic_migrate::Migrate; -# magic_migrate::migrate_toml_chain!(TestMetadata); -# -# struct FakeBuildpack; -# -# impl libcnb::Buildpack for FakeBuildpack { -# type Platform = libcnb::generic::GenericPlatform; -# type Metadata = libcnb::generic::GenericMetadata; -# type Error = std::convert::Infallible; -# -# fn detect( -# &self, -# _context: libcnb::detect::DetectContext, -# ) -> libcnb::Result { -# todo!() -# } -# -# fn build( -# &self, -# _context: libcnb::build::BuildContext, -# ) -> libcnb::Result { -# todo!() -# } -# } -# fn install_ruby(path: &std::path::Path) { -# todo!(); -# } -# -# pub(crate) fn call( -# context: &libcnb::build::BuildContext, -# ) -> libcnb::Result<(), ::Error> { -# let metadata_owned = TestMetadata { value: "Hello".to_string() }; -# let metadata = &metadata_owned; -let layer_ref = DiffMigrateLayer { - build: true, - launch: true, -} -.cached_layer(layer_name!("ruby"), context, metadata)?; -match &layer_ref.state { - // CacheDiff reported no difference, cache was kept - LayerState::Restored { cause } => { - println!(" - {cause}"); // States that the cache is being used - match cause { - Meta::Data(old) => { - // Inspect or use the old metadata from cache here if you like! - assert!(metadata.diff(old).is_empty()); - }, - Meta::Message(_) => unreachable!("Will only ever contain metadata when the cache is retained") - } - } - LayerState::Empty { cause } => { - match cause { - // Nothing in old cache - EmptyLayerCause::NewlyCreated => {} - // Problem restoring old cache (`TryMigrate` could not migrate the old metadata) - EmptyLayerCause::InvalidMetadataAction { cause } - // Difference with old cache - | EmptyLayerCause::RestoredLayerAction { cause } => { - // Report why the cache was cleared - println!(" - {cause}"); - } - } - println!(" - Installing"); - install_ruby(&layer_ref.path()); - println!(" - Done"); - } -} -# Ok(()) -# } -``` diff --git a/commons/src/layer/fixtures/metadata_migration_example.md b/commons/src/layer/fixtures/metadata_migration_example.md new file mode 100644 index 00000000..24315314 --- /dev/null +++ b/commons/src/layer/fixtures/metadata_migration_example.md @@ -0,0 +1,404 @@ + ## Setup DiffMigrateLayer for new layer Metadata + +Starting from scratch, add dependencies: + +```term +$ cargo add cache_diff --features bullet_stream +$ cargo add magic_migrate toml serde bullet_stream +$ cargo add commons --git https://github.com/heroku/buildpacks-ruby --branch main +``` + +In a layer file, define a metadata struct: + +```rust +use cache_diff::CacheDiff; +use serde::{Deserialize, Serialize}; + + #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] + #[serde(deny_unknown_fields)] +pub(crate) struct MetadataV1 { + #[cache_diff(rename = "Ruby version")] + pub(crate) version: String, +} + +pub(crate) type Metadata = MetadataV1; +``` + +This code: + +- Allows the struct to be [`serde::Serialize`]/[`serde::Deserialize`] as toml +- Sets some convenient traits [`Debug`], [`Clone`], [`Eq`], [`PartialEq`] +- Defines how the metadata is used to invalidate the cache with the [`CacheDiff`] derive +- Sets a convienece type alias for the latest Metadata + +In this code if the `version` field changes then the cache will be invalidated. + +Now tell it how to migrate invalid metadata: + + +```rust +use magic_migrate::TryMigrate; +// ... +# use cache_diff::CacheDiff; +# use serde::{Deserialize, Serialize}; +# +# #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] +# #[serde(deny_unknown_fields)] +# pub(crate) struct MetadataV1 { +# #[cache_diff(rename = "Ruby version")] +# pub(crate) version: String, +# } +# +# pub(crate) type Metadata = MetadataV1; + + #[derive(Debug)] +pub(crate) enum MigrationError {} + +impl std::fmt::Display for MigrationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + todo!() + } +} + +magic_migrate::try_migrate_toml_chain!( + error: MigrationError, + chain: [MetadataV1] +); +``` + +This code: + +- Defines an error type (so we can populate it when we need to add a failable migration) +- The error type needs to be `Display` and `Debug` +- Uses the `magic_migrate::try_migrate_toml_chain` macro to tell our code how it can migrate from one type to the next. + This will implement `TryMigrate` on every struct in the `chain` argument. In this case there's only one metadata value, + but we will implement this behavior now so it's easy to extend later. + + +At this point we've implemented `CacheDiff` and `TryMigrate` on our metadata, so we can define a layer: + +```rust +use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; + +use bullet_stream::{Print, state::SubBullet}; +use std::io::Stdout; + +use libcnb::layer::{LayerState, EmptyLayerCause}; +use libcnb::data::layer_name; +use libcnb::Buildpack; +use libcnb::build::BuildContext; +use libcnb::layer_env::LayerEnv; + +// ... +# use magic_migrate::TryMigrate; +# use cache_diff::CacheDiff; +# use serde::{Deserialize, Serialize}; +# +# #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] +# #[serde(deny_unknown_fields)] +# pub(crate) struct MetadataV1 { +# #[cache_diff(rename = "Ruby version")] +# pub(crate) version: String, +# } +# +# pub(crate) type Metadata = MetadataV1; +# +# #[derive(Debug)] +# pub(crate) enum MigrationError {} +# +# impl std::fmt::Display for MigrationError { +# fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +# todo!() +# } +# } +# +# magic_migrate::try_migrate_toml_chain!( +# error: MigrationError, +# chain: [MetadataV1] +# ); + +fn install_ruby(version: &str, path: &std::path::Path) { + todo!() +} + +pub(crate) fn call( + context: &BuildContext, + mut bullet: Print>, + metadata: &Metadata, +) -> libcnb::Result<(Print>, LayerEnv), ::Error> +where + B: Buildpack +{ + let layer_ref = DiffMigrateLayer { + build: true, + launch: true, + } + .cached_layer(layer_name!("ruby"), context, metadata)?; + match &layer_ref.state { + LayerState::Restored { cause } => { + bullet = bullet.sub_bullet(cause); + } + LayerState::Empty { cause } => { + match cause { + EmptyLayerCause::NewlyCreated => {} + EmptyLayerCause::InvalidMetadataAction { cause } + | EmptyLayerCause::RestoredLayerAction { cause } => { + bullet = bullet.sub_bullet(cause); + } + } + let timer = bullet.start_timer("Installing"); + install_ruby(&metadata.version, &layer_ref.path()); + bullet = timer.done(); + } + } + Ok((bullet, layer_ref.read_env()?)) +} +``` + +The signature + +- Defines a `call` function that: + - Takes a build context. In your code you'll want to replace the generic with a concrete buildpack type. + - Takes a bullet_stream printer for maximal printing consistency + - A `Metadata` struct constructed externally + +The logic of the function uses [`DiffMigrateLayer`] to create a layer that is both available at build and launch time. It creates a layer named "ruby" and passes in our metadata. When this executes it will: + +- Create the layer if it doesn't exist yet +- Invalidate the cache if the `version` attribute changed and return a `Meta::Message` explaining why +- Keep the cache if the version did not change and return the old `Meta::Data` (useful if not every attribute is used as a cache key) +- Migrate any old metadata (not applicable yet) +- Write the new metadata to the layer + +The return value is a `LayerRef` which we are using in a match statement. If the cache was restored it will emit that information to the buildpack user. If it was invalidated (if the version changed) it will emit that. When the layer is empty for any reason it will "install ruby" with a timer printed to stdout. + +A successful run of this function returns a tuple with `bullet_stream::Print>` which can be used to continue streaming and a `LayerEnv` which can be used to pass on any environment varible modifications from this layer (if any are added in the future). + +## Add a Metadata migration + +Over time, you might realize that your Metadata didn't accurately reflect your correct domain. For example, you might realize that OS distribution and version number are important and when they change, the cache needs to be cleared. If you simply added these fields to `MetadataV1` you would trigger invalid metadata which has to be handled. So instead we can add whatever fields we want to a new struct named `MetadataV2` and tell our program how to migrate from one to the other. + +> This might seem like overkill, but consider we might not stop at just these two versions we could have a V3 or v4 etc. Even trivial modifications, such as renaming an existing field could accidentally trigger this invalid metadata. In isolation, it's easy to migrate from one version to the other, but there's no guarantee that buildpack users will deploy at a regular cadence. We need to handle the situation where we're on V5 of metadata and users need to upgrade V1 and v4 at the same time. + +Let's add that new metadata now: + +```rust +use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; +use magic_migrate::TryMigrate; +use cache_diff::CacheDiff; +use serde::{Deserialize, Serialize}; + +# #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] +# #[serde(deny_unknown_fields)] +# pub(crate) struct MetadataV1 { +# pub(crate) version: String, +# } +# +# +# #[derive(Debug)] +# pub(crate) enum MigrationError {} +# +# impl std::fmt::Display for MigrationError { +# fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +# todo!() +# } +# } +# + #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] + #[serde(deny_unknown_fields)] +pub(crate) struct MetadataV2 { + #[cache_diff(rename = "Ruby version")] + pub(crate) version: String, + + #[cache_diff(rename = "OS distribution")] + pub(crate) distro: String +} + +fn get_distro_from_current_os() -> String { + // Just pretend, ok + todo!(); +} + +impl TryFrom for MetadataV2 { + type Error = MigrationError; + + fn try_from(old: MetadataV1) -> Result { + Ok(Self { + version: old.version, + distro: get_distro_from_current_os() + }) + } +} + +pub(crate) type Metadata = MetadataV2; + +magic_migrate::try_migrate_toml_chain!( + error: MigrationError, + chain: [MetadataV1, MetadataV2] +); +``` + +Here we added: + +- A new struct `MetadataV2` with a new field `distro` that `V1` does not have. +- Updated the `type Metadata = MetadataV2` to `V2` +- Added `MetadataV2` to the end of our migration chain. + +Now when our layer logic is called, it will first try to deserialize the contents into `MetadataV2` if it can it will return that and continue on to the cache invalidation logic. If not, it will try to deserialize the old toml into `MetadataV1`. If it can, then it will and then migrate from `MetadataV1` to `MetadataV2` using the `TryFrom` and `TryMigrate` traits. + +## Handle migration errors + +The logic so far doesn't need an error state, but what if we did? What if we realized we wanted to add another field for CPU architecture, and we also know that only versions greater than 2 support ARM? Let's add that logic and find out: + +```rust +use commons::layer::diff_migrate::{DiffMigrateLayer, Meta}; +use magic_migrate::TryMigrate; +use cache_diff::CacheDiff; +use serde::{Deserialize, Serialize}; + +# #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] +# #[serde(deny_unknown_fields)] +# pub(crate) struct MetadataV1 { +# pub(crate) version: String, +# } +# +# + #[derive(Debug)] +pub(crate) enum MigrationError { + InvalidVersionArch { + version: String, + arch: String, + } +} +# +# impl std::fmt::Display for MigrationError { +# fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +# todo!() +# } +# } +# +# #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] +# #[serde(deny_unknown_fields)] +# pub(crate) struct MetadataV2 { +# pub(crate) version: String, +# pub(crate) distro: String +# } +# +# fn get_distro_from_current_os() -> String { +# // Just pretend, ok +# todo!(); +# } +# +# impl TryFrom for MetadataV2 { +# type Error = MigrationError; +# +# fn try_from(old: MetadataV1) -> Result { +# Ok(Self { +# version: old.version, +# distro: get_distro_from_current_os() +# }) +# } +# } +# + + #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] + #[serde(deny_unknown_fields)] +pub(crate) struct MetadataV3 { + #[cache_diff(rename = "Ruby version")] + pub(crate) version: String, + + #[cache_diff(rename = "OS distribution")] + pub(crate) distro: String, + + #[cache_diff(rename = "CPU architecture")] + pub(crate) arch: String +} + +# fn get_arch_from_current_cpu() -> String { todo!(); } + +impl TryFrom for MetadataV3 { + type Error = MigrationError; + + fn try_from(old: MetadataV2) -> Result { + let distro = get_distro_from_current_os(); + let arch = get_arch_from_current_cpu(); + if old.version.starts_with("1.") && &arch == "arm64" { + Err( + MigrationError::InvalidVersionArch { + version: old.version, + arch: arch + } + ) + } else { + Ok(Self { + version: old.version, + distro: old.distro, + arch: arch + }) + } + } +} + +pub(crate) type Metadata = MetadataV3; + +magic_migrate::try_migrate_toml_chain!( + error: MigrationError, + chain: [MetadataV1, MetadataV2, MetadataV3] +); +``` + +What did we do? We added: + +- A new `MetadataV3` with a new field `Arch` +- A new error variant to our `MigrationError` named `InvalidVersionArch`. +- A new `TryFrom` to `MetadataV3` that fails if we try to re-use version 1.x on an `arm64` CPU (an arbitrary specification made for this example). + +Then we: + +- Updated the `type Metadata = MetadataV3` to `V3` +- Added `MetadataV3` to the end of our migration chain. + +Now when metadata is loaded it will go down the chain in reverse, it will try to load `V3` if it fails go to `V2` if it fails go to `V1`. If a match is successful it will reverse the process, migrating from `V1` to `V2` to `V3` etc. If our error condition is hit where someone is using version 1.x with an ARM CPU then an that will halt the migration process and trigger clearing the cache. + +## Recap + +The two traits `CacheDiff` and `TryMigrate` are relatively simple, but combined, give the program enough information to make previously tedious or complicated logic the default. + +## Q&A + +- Q: Wait, do I have to support metadata schemas (structs) forever? +- A: No. You can drop old structs whenver you feel it's necessary or invalidate the metadata at any time you like. The key with making your metadata migrate-able is that you don't HAVE to invalidate with every change. It makes it easier to ship the behavior that's best for you and your users. + +- Q: Can I use default logic without having to implement both traits? It seems odd to add a `TryMigrate` trait for a scenario where we might never need one. +- A: If you put in work adopting this migration pattern and never need it, it's one crate, one trait, and one struct. Not that much work. But a co-worker or contributor new to buildpacks needs to modify it, or a future tired you needs to modify it...it's easier to extend an existing pattern than remember the esoteric rules and edge cases of what will and won't serialize into a struct. + +- Q: You used `Metadata` as a type alias for use outside of the module. If you have multiple modules wouldn't they all have the same import? Shouldn't you namespace them somehow? +- A: Having to remember a naming convention for metadata in various layer modules is needless creativity. Instead of importing the struct, import the module and use that as a namespace, for example: + +```text +use ruby_layer; +use bundler_layer; + +//... + +ruby_layer::Metadata { + //... +} + +bundler_layer::Metadata { + //... +} +``` + +When you rev your metadata version, you'll need to add or modify any attributes that changed, but your imports and struct names won't need to change. Any use in type signatures doesn't need to be refactored. + +- Q: What bad habbits did you use here for the sake of making the example easier that I should avoid? +- A: Having all of your metadata fields be strings will not yield a strongly typed program. It will be "stringly" typed instead. Best practice would be to make purpose-built structs or if you must use strings, use the [New Type pattern](https://doc.rust-lang.org/rust-by-example/generics/new_types.html). + +- Q: Any other tips? +- A: Sure! + - Make sure to `#[serde(deny_unknown_fields)]` on your metadata structs + - Don't use overly flexible types such as `Option` unless you really have to. Metadata can be loaded wither with or without that attribute which might not be exactly what you want when you're deserializing old metadata. + - For layers that need to execute commands (such as `bundle install`), you can [use the `fun_run` crate](https://docs.rs/fun_run/0.2.0/fun_run/) which helps clearly print what's happening and gives lots of information when things fail. + - Beware if v1 and v3 have the same named attributes, but different semantics rust will happily serialize the values stored from v1 into v3 and you'll never get an error or warning and your `TryFrom` code won't fire. This is also a problem when not using the `TryMigrate` pattern, so stay on the lookout. + - For extremly important cache invalidation logic, add unit tests. From b99f60a6f50b332b188672cff4fb74e0e8d811c7 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sat, 21 Dec 2024 08:44:34 -0600 Subject: [PATCH 34/34] Update cache_diff --- Cargo.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1597fc38..f49eb386 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -175,9 +175,9 @@ checksum = "428d9aa8fbc0670b7b8d6030a7fadd0f86151cae55e4dbbece15f3780a3dfaf3" [[package]] name = "cache_diff" -version = "1.0.0" +version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2afc054cd5e2f8a0fb0122db671c3731849b1fcc86ec664e1031834a0828b84b" +checksum = "84b58ba1160088b5306b19fa156e9be5736043243ee6197346bf8ecd55e5366c" dependencies = [ "bullet_stream", "cache_diff_derive",