From 0ee879198224414bbef1729ec7cd14310f7f5002 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 25 Sep 2024 10:05:34 -0500 Subject: [PATCH 01/19] Let rust-analyzer reorder use statements There was a newline preventing rust-analyzer from ordering the `use` statements. I removed it, and saved and this is the outcome. --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 7f9c0851..1dd397b1 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,22 +11,21 @@ //! //! When the Ruby version changes, invalidate and re-run. //! +use crate::{ + target_id::{TargetId, TargetIdError}, + RubyBuildpack, RubyBuildpackError, +}; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; use commons::display::SentenceList; +use commons::gemfile_lock::ResolvedRubyVersion; +use flate2::read::GzDecoder; use libcnb::data::layer_name; use libcnb::layer::{ CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, }; use libcnb::layer_env::LayerEnv; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; - -use crate::{ - target_id::{TargetId, TargetIdError}, - RubyBuildpack, RubyBuildpackError, -}; -use commons::gemfile_lock::ResolvedRubyVersion; -use flate2::read::GzDecoder; use serde::{Deserialize, Deserializer, Serialize}; use std::convert::Infallible; use std::io::{self, Stdout}; From e615538ef9dd624853d1976e9a4fb95c30788347 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 12:55:53 -0500 Subject: [PATCH 02/19] Move metadata diff logic into a shared trait --- buildpacks/ruby/src/layers.rs | 1 + .../ruby/src/layers/ruby_install_layer.rs | 79 ++++++++++--------- buildpacks/ruby/src/layers/shared.rs | 6 ++ 3 files changed, 48 insertions(+), 38 deletions(-) create mode 100644 buildpacks/ruby/src/layers/shared.rs diff --git a/buildpacks/ruby/src/layers.rs b/buildpacks/ruby/src/layers.rs index 4c00f500..1037d54a 100644 --- a/buildpacks/ruby/src/layers.rs +++ b/buildpacks/ruby/src/layers.rs @@ -2,3 +2,4 @@ pub(crate) mod bundle_download_layer; pub(crate) mod bundle_install_layer; pub(crate) mod metrics_agent_install; pub(crate) mod ruby_install_layer; +mod shared; diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 1dd397b1..5d3eaa1a 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,6 +11,7 @@ //! //! When the Ruby version changes, invalidate and re-run. //! +use crate::layers::shared::MetadataDiff; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, @@ -61,7 +62,7 @@ pub(crate) fn handle( ), }, restored_layer_action: &|old: &Metadata, _| { - let diff = metadata_diff(old, &metadata); + let diff = metadata.diff(old); if diff.is_empty() { ( RestoredLayerAction::KeepLayer, @@ -169,44 +170,46 @@ impl TryFrom for MetadataV2 { } } -fn metadata_diff(old: &Metadata, metadata: &Metadata) -> Vec { - let mut differences = Vec::new(); - let Metadata { - distro_name, - distro_version, - cpu_architecture, - ruby_version, - } = old; - if ruby_version != &metadata.ruby_version { - differences.push(format!( - "Ruby version ({old} to {now})", - old = style::value(ruby_version.to_string()), - now = style::value(metadata.ruby_version.to_string()) - )); - } - if distro_name != &metadata.distro_name { - differences.push(format!( - "distro name ({old} to {now})", - old = style::value(distro_name), - now = style::value(&metadata.distro_name) - )); - } - if distro_version != &metadata.distro_version { - differences.push(format!( - "distro version ({old} to {now})", - old = style::value(distro_version), - now = style::value(&metadata.distro_version) - )); - } - if cpu_architecture != &metadata.cpu_architecture { - differences.push(format!( - "CPU architecture ({old} to {now})", - old = style::value(cpu_architecture), - now = style::value(&metadata.cpu_architecture) - )); - } +impl MetadataDiff for Metadata { + fn diff(&self, old: &Self) -> Vec { + let mut differences = Vec::new(); + let Metadata { + distro_name, + distro_version, + cpu_architecture, + ruby_version, + } = old; + if ruby_version != &self.ruby_version { + differences.push(format!( + "Ruby version ({old} to {now})", + old = style::value(ruby_version.to_string()), + now = style::value(self.ruby_version.to_string()) + )); + } + if distro_name != &self.distro_name { + differences.push(format!( + "distro name ({old} to {now})", + old = style::value(distro_name), + now = style::value(&self.distro_name) + )); + } + if distro_version != &self.distro_version { + differences.push(format!( + "distro version ({old} to {now})", + old = style::value(distro_version), + now = style::value(&self.distro_version) + )); + } + if cpu_architecture != &self.cpu_architecture { + differences.push(format!( + "CPU architecture ({old} to {now})", + old = style::value(cpu_architecture), + now = style::value(&self.cpu_architecture) + )); + } - differences + differences + } } fn download_url( diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs new file mode 100644 index 00000000..069a4498 --- /dev/null +++ b/buildpacks/ruby/src/layers/shared.rs @@ -0,0 +1,6 @@ +/// Given another metadata object, returns a list of differences between the two +/// +/// If no differences, return an empty list +pub(crate) trait MetadataDiff { + fn diff(&self, old: &Self) -> Vec; +} From c2d655b27dd64b21dbb7187b1c374b01f8507aba Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 18:44:50 -0500 Subject: [PATCH 03/19] Extract invalid_metadata_action to shared module --- .../ruby/src/layers/ruby_install_layer.rs | 19 ++--------- buildpacks/ruby/src/layers/shared.rs | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 5d3eaa1a..74d75e68 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,7 +11,7 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::MetadataDiff; +use crate::layers::shared::{invalid_metadata_action, MetadataDiff}; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, @@ -45,22 +45,7 @@ pub(crate) fn handle( CachedLayerDefinition { build: true, launch: true, - invalid_metadata_action: &|old| match Metadata::try_from_str_migrations( - &toml::to_string(old).expect("TOML deserialization of GenericMetadata"), - ) { - Some(Ok(migrated)) => ( - InvalidMetadataAction::ReplaceMetadata(migrated), - "replaced metadata".to_string(), - ), - Some(Err(error)) => ( - InvalidMetadataAction::DeleteLayer, - format!("metadata migration error {error}"), - ), - None => ( - InvalidMetadataAction::DeleteLayer, - "invalid metadata".to_string(), - ), - }, + invalid_metadata_action: &invalid_metadata_action, restored_layer_action: &|old: &Metadata, _| { let diff = metadata.diff(old); if diff.is_empty() { diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 069a4498..59384d15 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,6 +1,38 @@ +use libcnb::layer::InvalidMetadataAction; + /// Given another metadata object, returns a list of differences between the two /// /// If no differences, return an empty list pub(crate) trait MetadataDiff { fn diff(&self, old: &Self) -> Vec; } + +/// 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, String) +where + T: magic_migrate::TryMigrate, + S: serde::ser::Serialize + std::fmt::Debug, + // TODO: Enforce Display + Debug in the library + ::Error: std::fmt::Display, +{ + match T::try_from_str_migrations( + &toml::to_string(invalid).expect("TOML deserialization of GenericMetadata"), + ) { + Some(Ok(migrated)) => ( + InvalidMetadataAction::ReplaceMetadata(migrated), + "Replaced metadata".to_string(), + ), + Some(Err(error)) => ( + InvalidMetadataAction::DeleteLayer, + format!("Clearing cache due to metadata migration error: {error}"), + ), + None => ( + InvalidMetadataAction::DeleteLayer, + format!("Clearing cache due to invalid metadata ({invalid:?})"), + ), + } +} From a5f0491b7b081c962fd57a365fe9e0864f6184f7 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 18:46:56 -0500 Subject: [PATCH 04/19] Extract restored_layer_action to shared module --- .../ruby/src/layers/ruby_install_layer.rs | 21 ++------------- buildpacks/ruby/src/layers/shared.rs | 26 ++++++++++++++++++- 2 files changed, 27 insertions(+), 20 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 74d75e68..764813e7 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,7 +11,7 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::{invalid_metadata_action, MetadataDiff}; +use crate::layers::shared::{invalid_metadata_action, restored_layer_action, MetadataDiff}; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, @@ -46,24 +46,7 @@ pub(crate) fn handle( build: true, launch: true, invalid_metadata_action: &invalid_metadata_action, - restored_layer_action: &|old: &Metadata, _| { - let diff = metadata.diff(old); - if diff.is_empty() { - ( - RestoredLayerAction::KeepLayer, - "using cached version".to_string(), - ) - } else { - ( - RestoredLayerAction::DeleteLayer, - format!( - "due to {changes}: {differences}", - changes = if diff.len() > 1 { "changes" } else { "change" }, - differences = SentenceList::new(&diff) - ), - ) - } - }, + restored_layer_action: &|old: &Metadata, _| restored_layer_action(old, &metadata), }, )?; match &layer_ref.state { diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 59384d15..1c22514f 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,4 +1,5 @@ -use libcnb::layer::InvalidMetadataAction; +use commons::display::SentenceList; +use libcnb::layer::{InvalidMetadataAction, RestoredLayerAction}; /// Given another metadata object, returns a list of differences between the two /// @@ -7,6 +8,29 @@ pub(crate) trait MetadataDiff { fn diff(&self, old: &Self) -> Vec; } +/// Standardizes formatting for layer cache clearing behavior +/// +/// If the diff is empty, there are no changes and the layer is kept +/// If the diff is not empty, the layer is deleted and the changes are listed +pub(crate) fn restored_layer_action(old: &T, now: &T) -> (RestoredLayerAction, String) +where + T: MetadataDiff, +{ + let diff = now.diff(old); + if diff.is_empty() { + (RestoredLayerAction::KeepLayer, "Using cache".to_string()) + } else { + ( + RestoredLayerAction::DeleteLayer, + 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 fa86da7b891c1c954dfe6a5e35191cd51d45c9a8 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 18:48:11 -0500 Subject: [PATCH 05/19] Bundle default cache behavior into shared module --- .../ruby/src/layers/ruby_install_layer.rs | 14 +++------- buildpacks/ruby/src/layers/shared.rs | 28 ++++++++++++++++++- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 764813e7..8a78ba87 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,7 +11,9 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::{invalid_metadata_action, restored_layer_action, MetadataDiff}; +use crate::layers::shared::{ + cached_layer_ref, invalid_metadata_action, restored_layer_action, MetadataDiff, +}; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, @@ -40,15 +42,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = context.cached_layer( - layer_name!("ruby"), - CachedLayerDefinition { - build: true, - launch: true, - invalid_metadata_action: &invalid_metadata_action, - restored_layer_action: &|old: &Metadata, _| restored_layer_action(old, &metadata), - }, - )?; + let layer_ref = cached_layer_ref(layer_name!("ruby"), context, &metadata)?; match &layer_ref.state { LayerState::Restored { cause: _ } => { bullet = bullet.sub_bullet("Using cached Ruby version"); diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 1c22514f..c9bae7bc 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,5 +1,31 @@ use commons::display::SentenceList; -use libcnb::layer::{InvalidMetadataAction, RestoredLayerAction}; +use libcnb::build::BuildContext; +use libcnb::layer::{CachedLayerDefinition, InvalidMetadataAction, LayerRef, RestoredLayerAction}; + +/// Default behavior for a cached layer +/// +/// The metadadata must implement `MetadataDiff` and `TryMigrate` in addition +/// to the typical `Serialize` and `Debug` traits +pub(crate) fn cached_layer_ref( + layer_name: libcnb::data::layer::LayerName, + context: &BuildContext, + metadata: &'_ M, +) -> libcnb::Result, B::Error> +where + B: libcnb::Buildpack, + M: MetadataDiff + magic_migrate::TryMigrate + serde::ser::Serialize + std::fmt::Debug, + ::Error: std::fmt::Display, +{ + 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), + }, + ) +} /// Given another metadata object, returns a list of differences between the two /// From da25bea06134e6059ab2cc4953607daf134ede1e Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 12:59:57 -0500 Subject: [PATCH 06/19] Ensure we don't forget to write new metdata --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 5 ++--- buildpacks/ruby/src/layers/shared.rs | 10 ++++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 8a78ba87..f025d308 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -12,7 +12,7 @@ //! When the Ruby version changes, invalidate and re-run. //! use crate::layers::shared::{ - cached_layer_ref, invalid_metadata_action, restored_layer_action, MetadataDiff, + cached_layer_write_metadata, invalid_metadata_action, restored_layer_action, MetadataDiff, }; use crate::{ target_id::{TargetId, TargetIdError}, @@ -42,7 +42,7 @@ pub(crate) fn handle( mut bullet: Print>, metadata: Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = cached_layer_ref(layer_name!("ruby"), context, &metadata)?; + let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, &metadata)?; match &layer_ref.state { LayerState::Restored { cause: _ } => { bullet = bullet.sub_bullet("Using cached Ruby version"); @@ -60,7 +60,6 @@ pub(crate) fn handle( bullet = timer.done(); } } - layer_ref.write_metadata(metadata)?; Ok((bullet, layer_ref.read_env()?)) } diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index c9bae7bc..c45202e0 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -2,11 +2,11 @@ use commons::display::SentenceList; use libcnb::build::BuildContext; use libcnb::layer::{CachedLayerDefinition, InvalidMetadataAction, LayerRef, RestoredLayerAction}; -/// Default behavior for a cached layer +/// Default behavior for a cached layer, ensures new metadata is always written /// /// The metadadata must implement `MetadataDiff` and `TryMigrate` in addition /// to the typical `Serialize` and `Debug` traits -pub(crate) fn cached_layer_ref( +pub(crate) fn cached_layer_write_metadata( layer_name: libcnb::data::layer::LayerName, context: &BuildContext, metadata: &'_ M, @@ -16,7 +16,7 @@ where M: MetadataDiff + magic_migrate::TryMigrate + serde::ser::Serialize + std::fmt::Debug, ::Error: std::fmt::Display, { - context.cached_layer( + let layer_ref = context.cached_layer( layer_name, CachedLayerDefinition { build: true, @@ -24,7 +24,9 @@ where invalid_metadata_action: &invalid_metadata_action, restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata), }, - ) + )?; + layer_ref.write_metadata(metadata)?; + Ok(layer_ref) } /// Given another metadata object, returns a list of differences between the two From 70653aa18e559f8a1d3a0a5dfdea903adb4aedaf Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 13:01:28 -0500 Subject: [PATCH 07/19] Apply clippy suggestion --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 6 +++--- buildpacks/ruby/src/main.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index f025d308..ca49fdec 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -40,9 +40,9 @@ use url::Url; pub(crate) fn handle( context: &libcnb::build::BuildContext, mut bullet: Print>, - metadata: Metadata, + metadata: &Metadata, ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { - let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, &metadata)?; + let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { LayerState::Restored { cause: _ } => { bullet = bullet.sub_bullet("Using cached Ruby version"); @@ -56,7 +56,7 @@ pub(crate) fn handle( } } let timer = bullet.start_timer("Installing"); - install_ruby(&metadata, &layer_ref.path())?; + install_ruby(metadata, &layer_ref.path())?; bullet = timer.done(); } } diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 141e33e8..fcd1e8f3 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -158,7 +158,7 @@ impl Buildpack for RubyBuildpack { let (bullet, layer_env) = layers::ruby_install_layer::handle( &context, bullet, - layers::ruby_install_layer::Metadata { + &layers::ruby_install_layer::Metadata { distro_name: context.target.distro_name.clone(), distro_version: context.target.distro_version.clone(), cpu_architecture: context.target.arch.clone(), From e5fc7f22b2b49b0c8fd155a8385eb293fd56c339 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 13:12:24 -0500 Subject: [PATCH 08/19] Add a test for metadata differences --- .../ruby/src/layers/ruby_install_layer.rs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index ca49fdec..6ccfdac5 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -325,4 +325,31 @@ version = "3.1.3" "https://heroku-buildpack-ruby.s3.us-east-1.amazonaws.com/heroku-22/ruby-2.7.4.tgz", ); } + + #[test] + fn test_ruby_version_difference() { + let old = Metadata { + ruby_version: ResolvedRubyVersion("2.7.2".to_string()), + distro_name: "ubuntu".to_string(), + distro_version: "20.04".to_string(), + cpu_architecture: "x86_64".to_string(), + }; + let differences = old.diff(&old); + assert_eq!(differences, Vec::::new()); + + let now = Metadata { + ruby_version: ResolvedRubyVersion("3.0.0".to_string()), + ..old.clone() + }; + + let differences = now.diff(&old); + assert_eq!( + differences, + vec![format!( + "Ruby version ({old} to {now})", + old = style::value("2.7.2"), + now = style::value("3.0.0") + )] + ); + } } From da456fb2662dc2eec4026bbc50b390333bdc84e0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 13:13:19 -0500 Subject: [PATCH 09/19] Handle case where metadata is invalid It might be safe to `expect` in the specific case, but not in the general one. We can return a nicely formatted string with information if it ever happens. --- buildpacks/ruby/src/layers/shared.rs | 31 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index c45202e0..1f457889 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -71,20 +71,25 @@ where // TODO: Enforce Display + Debug in the library ::Error: std::fmt::Display, { - match T::try_from_str_migrations( - &toml::to_string(invalid).expect("TOML deserialization of GenericMetadata"), - ) { - Some(Ok(migrated)) => ( - InvalidMetadataAction::ReplaceMetadata(migrated), - "Replaced metadata".to_string(), - ), - Some(Err(error)) => ( - InvalidMetadataAction::DeleteLayer, - format!("Clearing cache due to metadata migration error: {error}"), - ), - None => ( + let invalid = toml::to_string(invalid); + match invalid { + Ok(toml) => match T::try_from_str_migrations(&toml) { + Some(Ok(migrated)) => ( + InvalidMetadataAction::ReplaceMetadata(migrated), + "Replaced metadata".to_string(), + ), + Some(Err(error)) => ( + InvalidMetadataAction::DeleteLayer, + format!("Clearing cache due to metadata migration error: {error}"), + ), + None => ( + InvalidMetadataAction::DeleteLayer, + format!("Clearing cache due to invalid metadata ({toml:?})"), + ), + }, + Err(error) => ( InvalidMetadataAction::DeleteLayer, - format!("Clearing cache due to invalid metadata ({invalid:?})"), + format!("Clearing cache due to invalid metadata serialization error: {error}"), ), } } From 17808d314f2229c14715bd5ca13df5143a181347 Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 13:15:04 -0500 Subject: [PATCH 10/19] Assert desired caching behavior Instead of asserting implementation (that diffing a metadata object returns a vec entry), assert the behavior we want (that it clears the cache). --- .../ruby/src/layers/ruby_install_layer.rs | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 6ccfdac5..c8002b5e 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -327,7 +327,7 @@ version = "3.1.3" } #[test] - fn test_ruby_version_difference() { + fn test_ruby_version_difference_clears_cache() { let old = Metadata { ruby_version: ResolvedRubyVersion("2.7.2".to_string()), distro_name: "ubuntu".to_string(), @@ -335,6 +335,12 @@ version = "3.1.3" cpu_architecture: "x86_64".to_string(), }; let differences = old.diff(&old); + let actual = restored_layer_action(&old, &old); + assert!(matches!( + actual, + (libcnb::layer::RestoredLayerAction::KeepLayer, _) + )); + assert_eq!(differences, Vec::::new()); let now = Metadata { @@ -342,14 +348,10 @@ version = "3.1.3" ..old.clone() }; - let differences = now.diff(&old); - assert_eq!( - differences, - vec![format!( - "Ruby version ({old} to {now})", - old = style::value("2.7.2"), - now = style::value("3.0.0") - )] - ); + let actual = restored_layer_action(&old, &now); + assert!(matches!( + actual, + (libcnb::layer::RestoredLayerAction::DeleteLayer, _) + )); } } From 7fea8029902f529aa70af37aea35c46b3eba910b Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 13:18:24 -0500 Subject: [PATCH 11/19] Clippy --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index c8002b5e..eafda51d 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,22 +11,17 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::{ - cached_layer_write_metadata, invalid_metadata_action, restored_layer_action, MetadataDiff, -}; +use crate::layers::shared::{cached_layer_write_metadata, MetadataDiff}; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, }; use bullet_stream::state::SubBullet; use bullet_stream::{style, Print}; -use commons::display::SentenceList; use commons::gemfile_lock::ResolvedRubyVersion; use flate2::read::GzDecoder; use libcnb::data::layer_name; -use libcnb::layer::{ - CachedLayerDefinition, EmptyLayerCause, InvalidMetadataAction, LayerState, RestoredLayerAction, -}; +use libcnb::layer::{EmptyLayerCause, LayerState}; use libcnb::layer_env::LayerEnv; use magic_migrate::{try_migrate_deserializer_chain, TryMigrate}; use serde::{Deserialize, Deserializer, Serialize}; @@ -254,6 +249,8 @@ pub(crate) enum RubyInstallError { #[cfg(test)] mod tests { + use crate::layers::shared::restored_layer_action; + use super::*; /// If this test fails due to a change you'll need to From 707e15e6f7f0c1ada613925eb6e0470c233ef8bd Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 24 Sep 2024 20:18:21 -0500 Subject: [PATCH 12/19] Use standardized output --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index eafda51d..339fab4b 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -39,15 +39,15 @@ pub(crate) fn handle( ) -> libcnb::Result<(Print>, LayerEnv), RubyBuildpackError> { let layer_ref = cached_layer_write_metadata(layer_name!("ruby"), context, metadata)?; match &layer_ref.state { - LayerState::Restored { cause: _ } => { - bullet = bullet.sub_bullet("Using cached Ruby version"); + 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(format!("Clearing cache {cause}")); + bullet = bullet.sub_bullet(cause); } } let timer = bullet.start_timer("Installing"); From 0a61c9021b96f17848d94955b68aded085e20625 Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 30 Sep 2024 13:46:57 -0500 Subject: [PATCH 13/19] Add tests to shared layer cache logic To fully exercise all caching logic in the shared folder, I'm introducing a helper to create a temporary `BuildContext` that can be used for exercising caching logic in test. This also introduces tests for both `restored_layer_action` states as called through `cached_layer_write_metadata`. Which should address this comment https://github.com/heroku/buildpacks-ruby/pull/327#discussion_r1775410807. --- buildpacks/ruby/src/layers/shared.rs | 129 +++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 1f457889..d4198635 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -93,3 +93,132 @@ where ), } } + +/// 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. +#[cfg(test)] +pub(crate) fn temp_build_context( + from_dir: impl AsRef, +) -> 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 buildpack_toml_string = include_str!("../../buildpack.toml"); + 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; + + BuildContext { + layers_dir, + app_dir, + buildpack_dir, + target, + platform, + buildpack_plan, + buildpack_descriptor, + store, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::RubyBuildpack; + use libcnb::data::layer_name; + use libcnb::layer::{EmptyLayerCause, LayerState}; + use magic_migrate::{migrate_toml_chain, Migrate}; + use serde::Deserializer; + + /// Struct for asserting the behavior of `cached_layer_write_metadata` + #[derive(Debug, serde::Serialize, serde::Deserialize)] + struct TestMetadata { + value: String, + } + impl MetadataDiff 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_cached_layer_write_metadata() { + 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, "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, + "Clearing cache due to change: value (hello to world)" + ); + } +} From 76d301ccc42cdba97046b8b73933ad3860c2acaa Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 30 Sep 2024 15:31:23 -0500 Subject: [PATCH 14/19] Assert the behavior of invalid_metadata_action --- buildpacks/ruby/src/layers/shared.rs | 89 +++++++++++++++++++++++++++- 1 file changed, 86 insertions(+), 3 deletions(-) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index d4198635..28071859 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -84,7 +84,10 @@ where ), None => ( InvalidMetadataAction::DeleteLayer, - format!("Clearing cache due to invalid metadata ({toml:?})"), + format!( + "Clearing cache due to invalid metadata ({toml})", + toml = toml.trim() + ), ), }, Err(error) => ( @@ -144,10 +147,12 @@ pub(crate) fn temp_build_context( mod tests { use super::*; use crate::RubyBuildpack; + use core::panic; use libcnb::data::layer_name; use libcnb::layer::{EmptyLayerCause, LayerState}; - use magic_migrate::{migrate_toml_chain, Migrate}; + 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)] @@ -166,7 +171,7 @@ mod tests { migrate_toml_chain! {TestMetadata} #[test] - fn test_cached_layer_write_metadata() { + fn test_cached_layer_write_metadata_restored_layer_action() { let temp = tempfile::tempdir().unwrap(); let context = temp_build_context::(temp.path()); @@ -221,4 +226,82 @@ 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)] + #[serde(deny_unknown_fields)] + struct PersonV1 { + name: String, + } + /// Struct for asserting the behavior of `invalid_metadata_action` + #[derive(serde::Deserialize, serde::Serialize, Debug)] + #[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, "Replaced metadata".to_string()); + + let (action, message) = invalid_metadata_action::(&PersonV1 { + name: "not_richard".to_string(), + }); + assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); + assert_eq!( + message, + "Clearing cache due to metadata migration error: Not Richard".to_string() + ); + + let (action, message) = invalid_metadata_action::(&TestMetadata { + value: "world".to_string(), + }); + assert!(matches!(action, InvalidMetadataAction::DeleteLayer)); + assert_eq!( + message, + "Clearing cache due to invalid metadata (value = \"world\")".to_string() + ); + + // Unable to produce this error at will: "Clearing cache due to invalid metadata serialization error: {error}" + } } From b556b9aa6dfced95f07ca84ee60cee03f6fccd5f Mon Sep 17 00:00:00 2001 From: Schneems Date: Mon, 30 Sep 2024 15:40:56 -0500 Subject: [PATCH 15/19] Exercise same interface used in file In the ruby_install_layer we're exercising `cached_layer_write_metadata` but we were testing `restored_layer_action` which is called currently but might not be in the future via refactoring. This change asserts the behavior we want while using the exact same interface we are currently using. --- .../ruby/src/layers/ruby_install_layer.rs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 339fab4b..36b35082 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -249,7 +249,7 @@ pub(crate) enum RubyInstallError { #[cfg(test)] mod tests { - use crate::layers::shared::restored_layer_action; + use crate::layers::shared::temp_build_context; use super::*; @@ -325,6 +325,8 @@ version = "3.1.3" #[test] fn test_ruby_version_difference_clears_cache() { + let temp = tempfile::tempdir().unwrap(); + let context = temp_build_context::(temp.path()); let old = Metadata { ruby_version: ResolvedRubyVersion("2.7.2".to_string()), distro_name: "ubuntu".to_string(), @@ -332,23 +334,26 @@ version = "3.1.3" cpu_architecture: "x86_64".to_string(), }; let differences = old.diff(&old); - let actual = restored_layer_action(&old, &old); - assert!(matches!( - actual, - (libcnb::layer::RestoredLayerAction::KeepLayer, _) - )); - 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(); + let actual = result.state; + assert!(matches!(actual, LayerState::Restored { .. })); + let now = Metadata { ruby_version: ResolvedRubyVersion("3.0.0".to_string()), ..old.clone() }; + let differences = now.diff(&old); + assert_eq!(differences.len(), 1); - let actual = restored_layer_action(&old, &now); + let result = cached_layer_write_metadata(layer_name!("ruby"), &context, &now).unwrap(); assert!(matches!( - actual, - (libcnb::layer::RestoredLayerAction::DeleteLayer, _) + result.state, + LayerState::Empty { + cause: EmptyLayerCause::RestoredLayerAction { .. } + } )); } } From 695a8ebc93b57600544229b9bf93d27b90270ead Mon Sep 17 00:00:00 2001 From: Schneems Date: Tue, 1 Oct 2024 19:17:45 -0500 Subject: [PATCH 16/19] Combine distro name and version in changed output --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 36b35082..bef7f8c6 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -142,18 +142,10 @@ impl MetadataDiff for Metadata { now = style::value(self.ruby_version.to_string()) )); } - if distro_name != &self.distro_name { + if distro_name != &self.distro_name || distro_version != &self.distro_version { differences.push(format!( - "distro name ({old} to {now})", - old = style::value(distro_name), - now = style::value(&self.distro_name) - )); - } - if distro_version != &self.distro_version { - differences.push(format!( - "distro version ({old} to {now})", - old = style::value(distro_version), - now = style::value(&self.distro_version) + "Distribution ({} {} to {} {})", + distro_name, distro_version, self.distro_name, self.distro_version )); } if cpu_architecture != &self.cpu_architecture { From 6b253e8846a2f3239dfed838fbf27ad224aa0de0 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 11:04:29 -0500 Subject: [PATCH 17/19] Add helper for removing ANSI codes for testing --- buildpacks/ruby/src/layers/shared.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index 28071859..48152aed 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -97,6 +97,13 @@ where } } +/// 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 38e46e202ba8d22eef7fb2e7239cf0710a34ff9f Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 11:05:02 -0500 Subject: [PATCH 18/19] Format distribution values as backticks with color --- buildpacks/ruby/src/layers/ruby_install_layer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index bef7f8c6..6194ff33 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -144,8 +144,9 @@ impl MetadataDiff for Metadata { } if distro_name != &self.distro_name || distro_version != &self.distro_version { differences.push(format!( - "Distribution ({} {} to {} {})", - distro_name, distro_version, self.distro_name, self.distro_version + "Distribution ({old} to {now})", + old = style::value(format!("{distro_name} {distro_version}")), + now = style::value(format!("{} {}", self.distro_name, self.distro_version)) )); } if cpu_architecture != &self.cpu_architecture { From a01ceac410bec53ede29d9277932d87110e0fec3 Mon Sep 17 00:00:00 2001 From: Schneems Date: Wed, 2 Oct 2024 11:07:28 -0500 Subject: [PATCH 19/19] Assert all difference output - Ensures each of these cases where the vec is not empty will clear the cache. - Makes it easier for reviewers to see the output as a user would see it. --- .../ruby/src/layers/ruby_install_layer.rs | 50 ++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 6194ff33..e0444a67 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -242,7 +242,7 @@ pub(crate) enum RubyInstallError { #[cfg(test)] mod tests { - use crate::layers::shared::temp_build_context; + use crate::layers::shared::{strip_ansi, temp_build_context}; use super::*; @@ -316,6 +316,54 @@ version = "3.1.3" ); } + #[test] + fn metadata_diff_messages() { + let old = Metadata { + ruby_version: ResolvedRubyVersion("3.5.3".to_string()), + distro_name: "ubuntu".to_string(), + distro_version: "20.04".to_string(), + cpu_architecture: "amd64".to_string(), + }; + assert_eq!(old.diff(&old), Vec::::new()); + + let diff = Metadata { + ruby_version: ResolvedRubyVersion("3.5.5".to_string()), + distro_name: old.distro_name.clone(), + distro_version: old.distro_version.clone(), + cpu_architecture: old.cpu_architecture.clone(), + } + .diff(&old); + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["Ruby version (`3.5.3` to `3.5.5`)".to_string()] + ); + + let diff = Metadata { + ruby_version: old.ruby_version.clone(), + distro_name: "alpine".to_string(), + distro_version: "3.20.0".to_string(), + cpu_architecture: old.cpu_architecture.clone(), + } + .diff(&old); + + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["Distribution (`ubuntu 20.04` to `alpine 3.20.0`)".to_string()] + ); + + let diff = Metadata { + ruby_version: old.ruby_version.clone(), + distro_name: old.distro_name.clone(), + distro_version: old.distro_version.clone(), + cpu_architecture: "arm64".to_string(), + } + .diff(&old); + assert_eq!( + diff.iter().map(strip_ansi).collect::>(), + vec!["CPU architecture (`amd64` to `arm64`)".to_string()] + ); + } + #[test] fn test_ruby_version_difference_clears_cache() { let temp = tempfile::tempdir().unwrap();