From 5c95a12c09d3acb6cc28944c09ca02ad8ea0c695 Mon Sep 17 00:00:00 2001 From: Schneems Date: Sat, 11 May 2024 12:45:52 -0400 Subject: [PATCH] Add explicit Distro/Architecture cache messages --- .../ruby/src/layers/bundle_install_layer.rs | 96 ++++++++++----- .../ruby/src/layers/ruby_install_layer.rs | 110 +++++++++++++----- buildpacks/ruby/src/main.rs | 17 ++- 3 files changed, 151 insertions(+), 72 deletions(-) diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs index 72f3c7ae..c387bb0d 100644 --- a/buildpacks/ruby/src/layers/bundle_install_layer.rs +++ b/buildpacks/ruby/src/layers/bundle_install_layer.rs @@ -51,7 +51,9 @@ pub(crate) struct BundleInstallLayerMetadataV1 { #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] pub(crate) struct BundleInstallLayerMetadataV2 { - pub(crate) target_id: TargetId, + pub(crate) distro_name: String, + pub(crate) distro_version: String, + pub(crate) cpu_architecture: String, pub(crate) ruby_version: ResolvedRubyVersion, pub(crate) force_bundle_install_key: String, @@ -71,6 +73,16 @@ pub(crate) struct BundleInstallLayerMetadataV2 { try_migrate_link!(BundleInstallLayerMetadataV1, BundleInstallLayerMetadataV2); pub(crate) type BundleInstallLayerMetadata = BundleInstallLayerMetadataV2; +impl BundleInstallLayerMetadataV2 { + pub(crate) fn target_id(&self) -> TargetId { + TargetId { + arch: self.cpu_architecture.clone(), + distro_name: self.distro_name.clone(), + distro_version: self.distro_version.clone(), + } + } +} + #[derive(thiserror::Error, Debug)] pub(crate) enum MigrateMetadataError { #[error("Could not migrate metadata {0}")] @@ -83,9 +95,13 @@ impl TryFrom for BundleInstallLayerMetadataV2 { type Error = MigrateMetadataError; fn try_from(v1: BundleInstallLayerMetadataV1) -> Result { + let target_id = + TargetId::from_stack(&v1.stack).map_err(MigrateMetadataError::UnsupportedStack)?; + Ok(Self { - target_id: TargetId::from_stack(&v1.stack) - .map_err(MigrateMetadataError::UnsupportedStack)?, + distro_name: target_id.distro_name.clone(), + distro_version: target_id.distro_version.clone(), + cpu_architecture: target_id.arch.clone(), ruby_version: v1.ruby_version, force_bundle_install_key: v1.force_bundle_install_key, digest: v1.digest, @@ -240,15 +256,34 @@ impl Layer for BundleInstallLayer<'_> { keep_and_run } - Changed::Target(_old, _now) => { - log_step(format!("Clearing cache {}", fmt::details("stack changed"))); + Changed::DistroName(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("distro name changed from {old} to {now}")) + )); + + clear_and_run + } + Changed::DistroVersion(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("distro version changed from {old} to {now}")) + )); + + clear_and_run + } + Changed::CpuArchitecture(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("cpu architecture changed from {old} to {now}")) + )); clear_and_run } - Changed::RubyVersion(_old, _now) => { + Changed::RubyVersion(old, now) => { log_step(format!( "Clearing cache {}", - fmt::details("ruby version changed") + fmt::details(format!("ruby version changed: {old} to {now}")) )); clear_and_run @@ -284,33 +319,30 @@ impl Layer for BundleInstallLayer<'_> { #[derive(Debug)] enum Changed { Nothing, - - /// The stack changed i.e. from `heroku-20` to `heroku-22` - /// When that happens we must invalidate native dependency gems - /// because they're compiled against system dependencies - /// i.e. - /// TODO: Only clear native dependencies instead of the whole cache - Target(TargetId, TargetId), // (old, now) - - /// Ruby version changed i.e. 3.0.2 to 3.1.2 - /// When that happens we must invalidate native dependency gems - /// because they're linked to a specific compiled version of Ruby. - /// TODO: Only clear native dependencies instead of the whole cache - RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion), // (old, now) + DistroName(String, String), + DistroVersion(String, String), + CpuArchitecture(String, String), + RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion), } // Compare the old metadata to current metadata to determine the state of the // cache. Based on that state, we can log and determine `ExistingLayerStrategy` fn cache_state(old: BundleInstallLayerMetadata, now: BundleInstallLayerMetadata) -> Changed { let BundleInstallLayerMetadata { - target_id, + distro_name, + distro_version, + cpu_architecture, ruby_version, force_bundle_install_key: _, digest: _, // digest state handled elsewhere } = now; // ensure all values are handled or we get a clippy warning - if old.target_id != target_id { - Changed::Target(old.target_id, target_id) + if old.distro_name != distro_name { + Changed::DistroName(old.distro_name, distro_name) + } else if old.distro_version != distro_version { + Changed::DistroVersion(old.distro_version, distro_version) + } else if old.cpu_architecture != cpu_architecture { + Changed::CpuArchitecture(old.cpu_architecture, cpu_architecture) } else if old.ruby_version != ruby_version { Changed::RubyVersion(old.ruby_version, ruby_version) } else { @@ -492,8 +524,11 @@ GEM_PATH=layer_path }; std::fs::write(&gemfile, "iamagemfile").unwrap(); + let target_id = TargetId::from_stack("heroku-24").unwrap(); let metadata = BundleInstallLayerMetadata { - target_id: TargetId::from_stack("heroku-24").unwrap(), + distro_name: target_id.distro_name, + distro_version: target_id.distro_version, + cpu_architecture: target_id.arch, ruby_version: ResolvedRubyVersion(String::from("3.1.3")), force_bundle_install_key: String::from("v1"), digest: MetadataDigest::new_env_files( @@ -507,13 +542,11 @@ GEM_PATH=layer_path let gemfile_path = gemfile.display(); let toml_string = format!( r#" -ruby_version = "3.1.3" -force_bundle_install_key = "v1" - -[target_id] -arch = "amd64" distro_name = "ubuntu" distro_version = "24.04" +cpu_architecture = "amd64" +ruby_version = "3.1.3" +force_bundle_install_key = "v1" [digest] platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3" @@ -581,8 +614,11 @@ platform_env = "c571543beaded525b7ee46ceb0b42c0fb7b9f6bfc3a211b3bbcfe6956b69ace3 .unwrap() .unwrap(); + let target_id = TargetId::from_stack(&metadata.stack).unwrap(); let expected = BundleInstallLayerMetadataV2 { - target_id: TargetId::from_stack(&metadata.stack).unwrap(), + distro_name: target_id.distro_name, + distro_version: target_id.distro_version, + cpu_architecture: target_id.arch, ruby_version: metadata.ruby_version, force_bundle_install_key: metadata.force_bundle_install_key, digest: metadata.digest, diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs index 037913dc..3350e4e0 100644 --- a/buildpacks/ruby/src/layers/ruby_install_layer.rs +++ b/buildpacks/ruby/src/layers/ruby_install_layer.rs @@ -47,9 +47,22 @@ pub(crate) struct RubyInstallLayerMetadataV1 { #[derive(Deserialize, Serialize, Debug, Clone, Eq, PartialEq)] pub(crate) struct RubyInstallLayerMetadataV2 { - pub(crate) target_id: TargetId, - pub(crate) version: ResolvedRubyVersion, + pub(crate) distro_name: String, + pub(crate) distro_version: String, + pub(crate) cpu_architecture: String, + pub(crate) ruby_version: ResolvedRubyVersion, +} + +impl RubyInstallLayerMetadataV2 { + pub(crate) fn target_id(&self) -> TargetId { + TargetId { + arch: self.cpu_architecture.clone(), + distro_name: self.distro_name.clone(), + distro_version: self.distro_version.clone(), + } + } } + try_migrate_link!(RubyInstallLayerMetadataV1, RubyInstallLayerMetadataV2); pub(crate) type RubyInstallLayerMetadata = RubyInstallLayerMetadataV2; @@ -63,10 +76,14 @@ impl TryFrom for RubyInstallLayerMetadataV2 { type Error = MetadataMigrateError; fn try_from(v1: RubyInstallLayerMetadataV1) -> Result { + let target_id = + TargetId::from_stack(&v1.stack).map_err(MetadataMigrateError::TargetIdError)?; + Ok(Self { - target_id: TargetId::from_stack(&v1.stack) - .map_err(MetadataMigrateError::TargetIdError)?, - version: v1.version, + distro_name: target_id.distro_name, + distro_version: target_id.distro_version, + cpu_architecture: target_id.arch, + ruby_version: v1.version, }) } } @@ -108,7 +125,7 @@ impl<'a> Layer for RubyInstallLayer<'a> { .map_err(RubyInstallError::CouldNotCreateDestinationFile) .map_err(RubyBuildpackError::RubyInstallError)?; - let url = download_url(&self.metadata.target_id, &self.metadata.version) + let url = download_url(&self.metadata.target_id(), &self.metadata.ruby_version) .map_err(RubyBuildpackError::RubyInstallError)?; download(url.as_ref(), tmp_ruby_tgz.path()) @@ -152,20 +169,39 @@ impl<'a> Layer for RubyInstallLayer<'a> { let now = self.metadata.clone(); match cache_state(old.clone(), now) { - Changed::Nothing(_version) => { + Changed::Nothing => { log_step("Using cached version"); Ok(ExistingLayerStrategy::Keep) } - Changed::Target(_old, _now) => { - log_step(format!("Clearing cache {}", fmt::details("OS changed"))); + Changed::CpuArchitecture(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("CPU architecture changed: {old} to {now}")) + )); + + Ok(ExistingLayerStrategy::Recreate) + } + Changed::DistroVersion(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("OS version changed: {old} to {now}")) + )); + + Ok(ExistingLayerStrategy::Recreate) + } + Changed::DistroName(old, now) => { + log_step(format!( + "Clearing cache {}", + fmt::details(format!("OS distribution changed: {old} to {now}")) + )); Ok(ExistingLayerStrategy::Recreate) } - Changed::RubyVersion(_old, _now) => { + Changed::RubyVersion(old, now) => { log_step(format!( "Clearing cache {}", - fmt::details("ruby version changed") + fmt::details(format!("Ruby version changed: {old} to {now}")) )); Ok(ExistingLayerStrategy::Recreate) @@ -175,21 +211,32 @@ impl<'a> Layer for RubyInstallLayer<'a> { } fn cache_state(old: RubyInstallLayerMetadata, now: RubyInstallLayerMetadata) -> Changed { - let RubyInstallLayerMetadata { target_id, version } = now; - - if old.target_id != target_id { - Changed::Target(old.target_id, target_id) - } else if old.version != version { - Changed::RubyVersion(old.version, version) + let RubyInstallLayerMetadata { + distro_name, + distro_version, + cpu_architecture, + ruby_version, + } = now; + + if old.distro_name != distro_name { + Changed::DistroName(old.distro_name, distro_name) + } else if old.distro_version != distro_version { + Changed::DistroVersion(old.distro_version, distro_version) + } else if old.cpu_architecture != cpu_architecture { + Changed::CpuArchitecture(old.cpu_architecture, cpu_architecture) + } else if old.ruby_version != ruby_version { + Changed::RubyVersion(old.ruby_version, ruby_version) } else { - Changed::Nothing(version) + Changed::Nothing } } #[derive(Debug)] enum Changed { - Nothing(ResolvedRubyVersion), - Target(TargetId, TargetId), + Nothing, + DistroName(String, String), + DistroVersion(String, String), + CpuArchitecture(String, String), RubyVersion(ResolvedRubyVersion, ResolvedRubyVersion), } @@ -276,22 +323,18 @@ mod tests { #[test] fn metadata_guard() { let metadata = RubyInstallLayerMetadata { - target_id: TargetId { - arch: String::from("amd64"), - distro_name: String::from("ubuntu"), - distro_version: String::from("22.04"), - }, - version: ResolvedRubyVersion(String::from("3.1.3")), + distro_name: String::from("ubuntu"), + distro_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#" -version = "3.1.3" - -[target_id] -arch = "amd64" distro_name = "ubuntu" distro_version = "22.04" +cpu_architecture = "amd64" +ruby_version = "3.1.3" "# .trim(); assert_eq!(expected, actual.trim()); @@ -317,9 +360,12 @@ version = "3.1.3" .unwrap() .unwrap(); + let target_id = TargetId::from_stack(&metadata.stack).unwrap(); let expected = RubyInstallLayerMetadataV2 { - target_id: TargetId::from_stack(&metadata.stack).expect("Valid stack"), - version: metadata.version, + distro_name: target_id.distro_name, + distro_version: target_id.distro_version, + cpu_architecture: target_id.arch, + ruby_version: metadata.version, }; assert_eq!(expected, deserialized); } diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs index 9c5de79d..fb6b6973 100644 --- a/buildpacks/ruby/src/main.rs +++ b/buildpacks/ruby/src/main.rs @@ -23,7 +23,6 @@ use libcnb::layer_env::Scope; use libcnb::Platform; use libcnb::{buildpack_main, Buildpack}; use std::io::stdout; -use target_id::TargetId; mod gem_list; mod layers; @@ -119,12 +118,6 @@ impl Buildpack for RubyBuildpack { let mut logger = BuildLog::new(stdout()).buildpack_name("Heroku Ruby Buildpack"); let warn_later = WarnGuard::new(stdout()); - let target_id = TargetId { - arch: context.target.arch.clone(), - distro_name: context.target.distro_name.clone(), - distro_version: context.target.distro_version.clone(), - }; - // ## Set default environment let (mut env, store) = crate::steps::default_env(&context, &context.platform.env().clone())?; @@ -178,8 +171,10 @@ impl Buildpack for RubyBuildpack { RubyInstallLayer { _in_section: section.as_ref(), metadata: RubyInstallLayerMetadata { - target_id: target_id.clone(), - version: ruby_version.clone(), + distro_name: context.target.distro_name.clone(), + distro_version: context.target.distro_version.clone(), + cpu_architecture: context.target.arch.clone(), + ruby_version: ruby_version.clone(), }, }, )?; @@ -219,7 +214,9 @@ impl Buildpack for RubyBuildpack { without: BundleWithout::new("development:test"), _section_log: section.as_ref(), metadata: BundleInstallLayerMetadata { - target_id: target_id.clone(), + distro_name: context.target.distro_name.clone(), + distro_version: context.target.distro_version.clone(), + cpu_architecture: context.target.arch.clone(), ruby_version: ruby_version.clone(), force_bundle_install_key: String::from( crate::layers::bundle_install_layer::FORCE_BUNDLE_INSTALL_CACHE_KEY,