From d2fd0436053553c5995c0994af259e33e18d228d Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Fri, 13 Dec 2024 09:47:47 -0600 Subject: [PATCH] Adopt CacheDiff (crate) over MetadataDiff (internal) (#370) * Upgrade Rust version * Add CacheDiff to dependencies * Import and implement CacheDiff derive macro * Use CacheDiff implementation for MetadataDiff The CacheDiff is a public crate while MetadataDiff was a temporary experimentation platform internal to this buildpack * Update magic_migrate Due to https://github.com/schneems/magic_migrate/pull/7 * Introduce metadata v3 The idea is to centralize all OS distribution information into a single attribute. It's not yet used. * Use metadata v3 for bundle_install The coming move to CacheDiff prefers if one attribute relates to one difference reason. V2 introduced a distribution name and distribution value which we later determined looked better if it's combined to look like a single attribute to the user "OS Distribution." This move brings the attributes inline with how they're compared, now both name and value are evaluated and emited as a single unit. * Implement CacheDiff on bundle_install layer (Tests do not compile yet) * Update tests for CacheDiff change * Move OS distribution into a type * Implement v3 metadata with condensed OS distribution The coming move to CacheDiff prefers if one attribute relates to one difference reason. V2 introduced a distribution name and distribution value which we later determined looked better if it's combined to look like a single attribute to the user "OS Distribution." This move brings the attributes inline with how they're compared, now both name and value are evaluated and emitted as a single unit. * Fix tests due to V3 migration * Implement CacheDiff for ruby_install_layer Does not compile yet due to tests * Fix test compilation * Implement CacheDiff for shared tests * Update shared interfaces to use CacheDiff instead of MetadataDiff * Replace qualified path with use * Remove unused internal trait MetadataDiff * Use regex for integration test match * Use diff method directly In the refactor we had two `diff` methods, now there's only one so we can use it instead of having to reference the trait. This shaves off some lines of the total diff. * Format use statements --- Cargo.lock | 55 ++++++++- Cargo.toml | 2 +- buildpacks/ruby/Cargo.toml | 1 + .../ruby/src/layers/bundle_download_layer.rs | 20 +-- .../ruby/src/layers/bundle_install_layer.rs | 116 +++++++++--------- .../ruby/src/layers/ruby_install_layer.rs | 114 +++++++++-------- buildpacks/ruby/src/layers/shared.rs | 33 +++-- buildpacks/ruby/src/main.rs | 14 ++- buildpacks/ruby/src/target_id.rs | 15 +++ buildpacks/ruby/tests/integration_test.rs | 25 ++-- 10 files changed, 237 insertions(+), 158 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe75f4b8..14271ad3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -173,6 +173,28 @@ version = "1.7.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "428d9aa8fbc0670b7b8d6030a7fadd0f86151cae55e4dbbece15f3780a3dfaf3" +[[package]] +name = "cache_diff" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2afc054cd5e2f8a0fb0122db671c3731849b1fcc86ec664e1031834a0828b84b" +dependencies = [ + "bullet_stream", + "cache_diff_derive", +] + +[[package]] +name = "cache_diff_derive" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "273bbd860603cb240372b460f8dc2440528ba95593f944b549194eb8600285a8" +dependencies = [ + "proc-macro2", + "quote", + "strum", + "syn 2.0.90", +] + [[package]] name = "camino" version = "1.1.6" @@ -571,6 +593,7 @@ name = "heroku-ruby-buildpack" version = "0.0.0" dependencies = [ "bullet_stream", + "cache_diff", "clap", "commons", "flate2", @@ -945,9 +968,9 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c" [[package]] name = "magic_migrate" -version = "0.2.0" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c05b570dc24563cf1720263bbeeb78822c8d3ce75debe510ca6bae90dd0cccf" +checksum = "3995455690a60dcbb8688d0f0e84eee5322eb1cdd5e9a9bf0ac7b2aa56250291" dependencies = [ "serde", ] @@ -1307,6 +1330,12 @@ dependencies = [ "untrusted", ] +[[package]] +name = "rustversion" +version = "1.0.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e819f2bc632f285be6d7cd36e25940d45b2391dd6d9b939e79de557f7014248" + [[package]] name = "ryu" version = "1.0.17" @@ -1425,6 +1454,28 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strum" +version = "0.26.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fec0f0aef304996cf250b31b5a10dee7980c85da9d759361292b8bca5a18f06" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.26.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c6bee85a5a24955dc440386795aa378cd9cf82acd5f764469152d2270e581be" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn 2.0.90", +] + [[package]] name = "subtle" version = "2.5.0" diff --git a/Cargo.toml b/Cargo.toml index e787b059..b439a058 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["buildpacks/ruby", "commons"] [workspace.package] edition = "2021" -rust-version = "1.80" +rust-version = "1.82" [workspace.lints.rust] unreachable_pub = "warn" diff --git a/buildpacks/ruby/Cargo.toml b/buildpacks/ruby/Cargo.toml index 86953fa5..bb2739e5 100644 --- a/buildpacks/ruby/Cargo.toml +++ b/buildpacks/ruby/Cargo.toml @@ -30,6 +30,7 @@ ureq = { version = "2", default-features = false, features = ["tls"] } url = "2" magic_migrate = "0.2" toml = "0.8" +cache_diff = { version = "1.0.0", features = ["bullet_stream"] } [dev-dependencies] libcnb-test = "=0.26.1" diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs index 59ad73db..066a83a8 100644 --- a/buildpacks/ruby/src/layers/bundle_download_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_download_layer.rs @@ -4,11 +4,12 @@ //! //! 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, MetadataDiff}; +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 fun_run::{self, CommandWithName}; use libcnb::data::layer_name; @@ -57,22 +58,9 @@ try_migrate_deserializer_chain!( chain: [MetadataV1], ); -impl MetadataDiff for Metadata { - fn diff(&self, other: &Self) -> Vec { - let mut differences = Vec::new(); - if self.version != other.version { - differences.push(format!( - "Bundler version ({old} to {now})", - old = style::value(other.version.to_string()), - now = style::value(self.version.to_string()) - )); - } - differences - } -} - -#[derive(Deserialize, Serialize, Debug, Clone)] +#[derive(Deserialize, Serialize, Debug, Clone, CacheDiff)] pub(crate) struct MetadataV1 { + #[cache_diff(rename = "Bundler version")] pub(crate) version: ResolvedBundlerVersion, } diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 989482ba..6344f3c3 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -14,11 +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, MetadataDiff}; -use crate::target_id::{TargetId, TargetIdError}; +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::{ display::SentenceList, gemfile_lock::ResolvedRubyVersion, metadata_digest::MetadataDigest, }; @@ -116,51 +117,13 @@ pub(crate) fn handle( Ok((bullet, layer_ref.read_env()?)) } -pub(crate) type Metadata = MetadataV2; +pub(crate) type Metadata = MetadataV3; try_migrate_deserializer_chain!( - chain: [MetadataV1, MetadataV2], + chain: [MetadataV1, MetadataV2, MetadataV3], error: MetadataMigrateError, deserializer: toml::Deserializer::new, ); -impl MetadataDiff for Metadata { - fn diff(&self, old: &Self) -> Vec { - let mut differences = Vec::new(); - let Metadata { - distro_name, - distro_version, - cpu_architecture, - ruby_version, - force_bundle_install_key: _, - digest: _, - } = old; - - if ruby_version != &self.ruby_version { - differences.push(format!( - "Ruby version ({old} to {now})", - old = style::value(ruby_version.to_string()), - now = style::value(self.ruby_version.to_string()) - )); - } - if distro_name != &self.distro_name || distro_version != &self.distro_version { - differences.push(format!( - "Distribution ({old} to {now})", - old = style::value(format!("{distro_name} {distro_version}")), - now = style::value(format!("{} {}", self.distro_name, self.distro_version)) - )); - } - if cpu_architecture != &self.cpu_architecture { - differences.push(format!( - "CPU architecture ({old} to {now})", - old = style::value(cpu_architecture), - now = style::value(&self.cpu_architecture) - )); - } - - differences - } -} - #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] pub(crate) struct MetadataV1 { pub(crate) stack: String, @@ -176,6 +139,19 @@ pub(crate) struct MetadataV2 { pub(crate) cpu_architecture: String, pub(crate) ruby_version: ResolvedRubyVersion, pub(crate) force_bundle_install_key: String, + pub(crate) digest: MetadataDigest, // Must be last for serde to be happy https://github.com/toml-rs/toml-rs/issues/142 +} + +#[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] +pub(crate) struct MetadataV3 { + #[cache_diff(rename = "OS Distribution")] + pub(crate) os_distribution: OsDistribution, + #[cache_diff(rename = "CPU Architecture")] + pub(crate) cpu_architecture: String, + #[cache_diff(rename = "Ruby version")] + pub(crate) ruby_version: ResolvedRubyVersion, + #[cache_diff(ignore)] + pub(crate) force_bundle_install_key: String, /// A struct that holds the cryptographic hash of components that can /// affect the result of `bundle install`. When these values do not @@ -188,6 +164,7 @@ pub(crate) struct MetadataV2 { /// This value is cached with metadata, so changing the struct /// may cause metadata to be invalidated (and the cache cleared). /// + #[cache_diff(ignore)] pub(crate) digest: MetadataDigest, // Must be last for serde to be happy https://github.com/toml-rs/toml-rs/issues/142 } @@ -217,6 +194,23 @@ impl TryFrom for MetadataV2 { } } +impl TryFrom for MetadataV3 { + type Error = Infallible; + + fn try_from(v2: MetadataV2) -> Result { + Ok(Self { + os_distribution: OsDistribution { + name: v2.distro_name, + version: v2.distro_version, + }, + cpu_architecture: v2.cpu_architecture, + ruby_version: v2.ruby_version, + force_bundle_install_key: v2.force_bundle_install_key, + digest: v2.digest, + }) + } +} + #[derive(Debug)] enum InstallState { /// Holds message indicating the reason why we want to run 'bundle install' @@ -334,7 +328,7 @@ mod test { use super::*; use std::path::PathBuf; - /// `MetadataDiff` logic controls cache invalidation + /// `CacheDiff` logic controls cache invalidation /// When the vec is empty the cache is kept, otherwise it is invalidated #[test] fn metadata_diff_messages() { @@ -350,8 +344,10 @@ mod test { let old = Metadata { ruby_version: ResolvedRubyVersion("3.5.3".to_string()), - distro_name: "ubuntu".to_string(), - distro_version: "20.04".to_string(), + os_distribution: OsDistribution { + name: "ubuntu".to_string(), + version: "20.04".to_string(), + }, cpu_architecture: "amd64".to_string(), force_bundle_install_key: FORCE_BUNDLE_INSTALL_CACHE_KEY.to_string(), digest: MetadataDigest::new_env_files( @@ -364,13 +360,13 @@ mod test { let diff = Metadata { ruby_version: ResolvedRubyVersion("3.5.5".to_string()), - distro_name: old.distro_name.clone(), - distro_version: old.distro_version.clone(), + os_distribution: old.os_distribution.clone(), cpu_architecture: old.cpu_architecture.clone(), force_bundle_install_key: old.force_bundle_install_key.clone(), digest: old.digest.clone(), } .diff(&old); + assert_eq!( diff.iter().map(strip_ansi).collect::>(), vec!["Ruby version (`3.5.3` to `3.5.5`)".to_string()] @@ -378,8 +374,10 @@ mod test { let diff = Metadata { ruby_version: old.ruby_version.clone(), - distro_name: "alpine".to_string(), - distro_version: "3.20.0".to_string(), + os_distribution: OsDistribution { + name: "alpine".to_string(), + version: "3.20.0".to_string(), + }, cpu_architecture: old.cpu_architecture.clone(), force_bundle_install_key: old.force_bundle_install_key.clone(), digest: old.digest.clone(), @@ -388,21 +386,21 @@ mod test { assert_eq!( diff.iter().map(strip_ansi).collect::>(), - vec!["Distribution (`ubuntu 20.04` to `alpine 3.20.0`)".to_string()] + vec!["OS 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(), + os_distribution: old.os_distribution.clone(), cpu_architecture: "arm64".to_string(), force_bundle_install_key: old.force_bundle_install_key.clone(), digest: old.digest.clone(), } .diff(&old); + assert_eq!( diff.iter().map(strip_ansi).collect::>(), - vec!["CPU architecture (`amd64` to `arm64`)".to_string()] + vec!["CPU Architecture (`amd64` to `arm64`)".to_string()] ); } @@ -476,8 +474,10 @@ GEM_PATH=layer_path let target_id = TargetId::from_stack("heroku-22").unwrap(); let metadata = Metadata { - distro_name: target_id.distro_name, - distro_version: target_id.distro_version, + os_distribution: OsDistribution { + name: target_id.distro_name.clone(), + version: target_id.distro_version.clone(), + }, cpu_architecture: target_id.cpu_architecture, ruby_version: ResolvedRubyVersion(String::from("3.1.3")), force_bundle_install_key: String::from("v1"), @@ -492,12 +492,14 @@ GEM_PATH=layer_path let gemfile_path = gemfile.display(); let toml_string = format!( r#" -distro_name = "ubuntu" -distro_version = "22.04" cpu_architecture = "amd64" ruby_version = "3.1.3" force_bundle_install_key = "v1" +[os_distribution] +name = "ubuntu" +version = "22.04" + [digest] platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3" diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index e0444a67..8d72b6d0 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -11,13 +11,15 @@ //! //! When the Ruby version changes, invalidate and re-run. //! -use crate::layers::shared::{cached_layer_write_metadata, MetadataDiff}; +use crate::layers::shared::cached_layer_write_metadata; +use crate::target_id::OsDistribution; use crate::{ target_id::{TargetId, TargetIdError}, RubyBuildpack, RubyBuildpackError, }; use bullet_stream::state::SubBullet; -use bullet_stream::{style, Print}; +use bullet_stream::Print; +use cache_diff::CacheDiff; use commons::gemfile_lock::ResolvedRubyVersion; use flate2::read::GzDecoder; use libcnb::data::layer_name; @@ -86,20 +88,30 @@ pub(crate) struct MetadataV2 { pub(crate) cpu_architecture: String, pub(crate) ruby_version: ResolvedRubyVersion, } -pub(crate) type Metadata = MetadataV2; -impl MetadataV2 { +#[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq, CacheDiff)] +pub(crate) struct MetadataV3 { + #[cache_diff(rename = "OS Distribution")] + pub(crate) os_distribution: OsDistribution, + #[cache_diff(rename = "CPU architecture")] + pub(crate) cpu_architecture: String, + #[cache_diff(rename = "Ruby version")] + pub(crate) ruby_version: ResolvedRubyVersion, +} + +impl MetadataV3 { pub(crate) fn target_id(&self) -> TargetId { TargetId { cpu_architecture: self.cpu_architecture.clone(), - distro_name: self.distro_name.clone(), - distro_version: self.distro_version.clone(), + distro_name: self.os_distribution.name.clone(), + distro_version: self.os_distribution.version.clone(), } } } +pub(crate) type Metadata = MetadataV3; try_migrate_deserializer_chain!( - chain: [MetadataV1, MetadataV2], + chain: [MetadataV1, MetadataV2, MetadataV3], error: MetadataMigrateError, deserializer: toml::Deserializer::new, ); @@ -126,38 +138,18 @@ impl TryFrom for MetadataV2 { } } -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 || distro_version != &self.distro_version { - differences.push(format!( - "Distribution ({old} to {now})", - old = style::value(format!("{distro_name} {distro_version}")), - now = style::value(format!("{} {}", self.distro_name, self.distro_version)) - )); - } - if cpu_architecture != &self.cpu_architecture { - differences.push(format!( - "CPU architecture ({old} to {now})", - old = style::value(cpu_architecture), - now = style::value(&self.cpu_architecture) - )); - } +impl TryFrom for MetadataV3 { + type Error = MetadataMigrateError; - differences + fn try_from(v2: MetadataV2) -> Result { + Ok(Self { + os_distribution: OsDistribution { + name: v2.distro_name, + version: v2.distro_version, + }, + cpu_architecture: v2.cpu_architecture, + ruby_version: v2.ruby_version, + }) } } @@ -253,18 +245,22 @@ mod tests { #[test] fn metadata_guard() { let metadata = Metadata { - distro_name: String::from("ubuntu"), - distro_version: String::from("22.04"), + os_distribution: OsDistribution { + name: String::from("ubuntu"), + version: String::from("22.04"), + }, cpu_architecture: String::from("amd64"), ruby_version: ResolvedRubyVersion(String::from("3.1.3")), }; let actual = toml::to_string(&metadata).unwrap(); let expected = r#" -distro_name = "ubuntu" -distro_version = "22.04" cpu_architecture = "amd64" ruby_version = "3.1.3" + +[os_distribution] +name = "ubuntu" +version = "22.04" "# .trim(); assert_eq!(expected, actual.trim()); @@ -320,19 +316,24 @@ version = "3.1.3" 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(), + os_distribution: OsDistribution { + name: "ubuntu".to_string(), + 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(), + os_distribution: OsDistribution { + name: "ubuntu".to_string(), + version: "20.04".to_string(), + }, 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()] @@ -340,24 +341,29 @@ version = "3.1.3" let diff = Metadata { ruby_version: old.ruby_version.clone(), - distro_name: "alpine".to_string(), - distro_version: "3.20.0".to_string(), + os_distribution: OsDistribution { + name: "alpine".to_string(), + 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()] + vec!["OS 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(), + os_distribution: OsDistribution { + name: old.os_distribution.name.clone(), + version: old.os_distribution.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()] @@ -370,8 +376,10 @@ version = "3.1.3" let context = temp_build_context::(temp.path()); let old = Metadata { ruby_version: ResolvedRubyVersion("2.7.2".to_string()), - distro_name: "ubuntu".to_string(), - distro_version: "20.04".to_string(), + os_distribution: OsDistribution { + name: "ubuntu".to_string(), + version: "20.04".to_string(), + }, cpu_architecture: "x86_64".to_string(), }; let differences = old.diff(&old); diff --git a/buildpacks/ruby/src/layers/shared.rs b/buildpacks/ruby/src/layers/shared.rs index f7dcc3dc..f25ba19e 100644 --- a/buildpacks/ruby/src/layers/shared.rs +++ b/buildpacks/ruby/src/layers/shared.rs @@ -1,20 +1,25 @@ +use cache_diff::CacheDiff; use commons::display::SentenceList; 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; /// Default behavior for a cached layer, ensures new metadata is always written /// -/// The metadadata must implement `MetadataDiff` and `TryMigrate` in addition +/// 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: libcnb::data::layer::LayerName, + layer_name: LayerName, context: &BuildContext, metadata: &'_ M, ) -> libcnb::Result, Meta>, B::Error> where B: libcnb::Buildpack, - M: MetadataDiff + magic_migrate::TryMigrate + serde::ser::Serialize + std::fmt::Debug + Clone, - ::Error: std::fmt::Display, + M: CacheDiff + TryMigrate + Serialize + Debug + Clone, + ::Error: std::fmt::Display, { let layer_ref = context.cached_layer( layer_name, @@ -29,20 +34,13 @@ where Ok(layer_ref) } -/// 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 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: MetadataDiff + Clone, + M: CacheDiff + Clone, { let diff = now.diff(old); if diff.is_empty() { @@ -66,10 +64,10 @@ where /// 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: magic_migrate::TryMigrate + Clone, - S: serde::ser::Serialize + std::fmt::Debug, + M: TryMigrate + Clone, + S: Serialize + Debug, // TODO: Enforce Display + Debug in the library - ::Error: std::fmt::Display, + ::Error: std::fmt::Display, { let invalid = toml::to_string(invalid); match invalid { @@ -211,7 +209,7 @@ mod tests { struct TestMetadata { value: String, } - impl MetadataDiff for TestMetadata { + impl CacheDiff for TestMetadata { fn diff(&self, old: &Self) -> Vec { if self.value == old.value { vec![] @@ -228,8 +226,7 @@ mod tests { struct AlwaysNoDiff { value: String, } - - impl MetadataDiff for AlwaysNoDiff { + impl CacheDiff for AlwaysNoDiff { fn diff(&self, _: &Self) -> Vec { vec![] } diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 5ad3560a..b0190c35 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -31,6 +31,8 @@ use libcnb_test as _; use clap as _; +use crate::target_id::OsDistribution; + struct RubyBuildpack; #[derive(Debug, thiserror::Error)] @@ -149,8 +151,10 @@ impl Buildpack for RubyBuildpack { &context, bullet, &layers::ruby_install_layer::Metadata { - distro_name: context.target.distro_name.clone(), - distro_version: context.target.distro_version.clone(), + os_distribution: OsDistribution { + name: context.target.distro_name.clone(), + version: context.target.distro_version.clone(), + }, cpu_architecture: context.target.arch.clone(), ruby_version: ruby_version.clone(), }, @@ -186,8 +190,10 @@ impl Buildpack for RubyBuildpack { &env, bullet, &layers::bundle_install_layer::Metadata { - distro_name: context.target.distro_name.clone(), - distro_version: context.target.distro_version.clone(), + os_distribution: OsDistribution { + name: context.target.distro_name.clone(), + version: context.target.distro_version.clone(), + }, cpu_architecture: context.target.arch.clone(), ruby_version: ruby_version.clone(), force_bundle_install_key: String::from( diff --git a/buildpacks/ruby/src/target_id.rs b/buildpacks/ruby/src/target_id.rs index 42c07387..2002812f 100644 --- a/buildpacks/ruby/src/target_id.rs +++ b/buildpacks/ruby/src/target_id.rs @@ -1,3 +1,6 @@ +use serde::{Deserialize, Serialize}; +use std::fmt::{Display, Formatter}; + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct TargetId { pub(crate) distro_name: String, @@ -51,6 +54,18 @@ impl TargetId { } } +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub(crate) struct OsDistribution { + pub(crate) name: String, + pub(crate) version: String, +} + +impl Display for OsDistribution { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!(f, "{} {}", self.name, self.version) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/buildpacks/ruby/tests/integration_test.rs b/buildpacks/ruby/tests/integration_test.rs index 3a19fa2c..e9179a82 100644 --- a/buildpacks/ruby/tests/integration_test.rs +++ b/buildpacks/ruby/tests/integration_test.rs @@ -4,8 +4,8 @@ #![allow(clippy::unwrap_used)] use libcnb_test::{ - assert_contains, assert_empty, BuildConfig, BuildpackReference, ContainerConfig, - ContainerContext, TestRunner, + assert_contains, assert_contains_match, assert_empty, BuildConfig, BuildpackReference, + ContainerConfig, ContainerContext, TestRunner, }; use std::thread; use std::time::{Duration, Instant}; @@ -19,14 +19,15 @@ fn test_migrating_metadata() { // This test is a placeholder for when a change modifies metadata structures. // Remove the return and update the `buildpack-ruby` reference to the latest version. #![allow(unreachable_code)] - return; + // Test v4.0.2 compatible with v4.0.1 + // return; - let builder = "heroku/builder:22"; + let builder = "heroku/builder:24"; let app_dir = "tests/fixtures/default_ruby"; TestRunner::default().build( BuildConfig::new(builder, app_dir).buildpacks([BuildpackReference::Other( - "docker://docker.io/heroku/buildpack-ruby:2.1.2".to_string(), + "docker://docker.io/heroku/buildpack-ruby:4.0.1".to_string(), )]), |context| { println!("{}", context.pack_stdout); @@ -35,8 +36,18 @@ fn test_migrating_metadata() { |rebuild_context| { println!("{}", rebuild_context.pack_stdout); - assert_contains!(rebuild_context.pack_stdout, "Using cached Ruby version"); - assert_contains!(rebuild_context.pack_stdout, "Loading cached gems"); + assert_contains_match!( + rebuild_context.pack_stdout, + r"^- Ruby version[^\n]*\n - Using cache" + ); + assert_contains_match!( + rebuild_context.pack_stdout, + r"^- Bundler version[^\n]*\n - Using cache" + ); + assert_contains_match!( + rebuild_context.pack_stdout, + r"^- Bundle install gems[^\n]*\n - Using cache" + ); }, ); },