[Stacked] Continue metadata migration (#322)

* Use metadata as single point of truth (SPOT) Previously we were using a metadata structure that was decoupled from the work being done (stored in the DOWNLOAD_URL constant). This created the possibility to update one without the other. * Style URLs * [Stacked] Fixes CI linting (#323) * Clippy recommended method * Allow deprecated Layer use * Clippy lint suggestions * Allow deprecated layer API usage * Allow deprecated layer API * Fewer allow(deprecation)-s
heroku · Sep 23, 2024 · fc2dfb7 · fc2dfb7
1 parent b4b1fcf
commit fc2dfb7
Show file tree

Hide file tree

Showing 10 changed files with 37 additions and 20 deletions.
diff --git a/buildpacks/ruby/src/layers/bundle_download_layer.rs b/buildpacks/ruby/src/layers/bundle_download_layer.rs
@@ -8,6 +8,7 @@ use commons::output::{
 use fun_run::{self, CommandWithName};
 use libcnb::build::BuildContext;
 use libcnb::data::layer_content_metadata::LayerTypes;
+#[allow(deprecated)]
 use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
 use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope};
 use libcnb::Env;
@@ -32,6 +33,7 @@ pub(crate) struct BundleDownloadLayer<'a> {
     pub(crate) _section_logger: &'a dyn SectionLogger,
 }
 
+#[allow(deprecated)]
 impl<'a> Layer for BundleDownloadLayer<'a> {
     type Buildpack = RubyBuildpack;
     type Metadata = BundleDownloadLayerMetadata;

diff --git a/buildpacks/ruby/src/layers/bundle_install_layer.rs b/buildpacks/ruby/src/layers/bundle_install_layer.rs
@@ -8,10 +8,11 @@ use commons::{
 };
 use fun_run::CommandWithName;
 use fun_run::{self, CmdError};
+#[allow(deprecated)]
+use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
 use libcnb::{
     build::BuildContext,
     data::layer_content_metadata::LayerTypes,
-    layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder},
     layer_env::{LayerEnv, ModificationBehavior, Scope},
     Env,
 };
@@ -149,6 +150,7 @@ fn update_state(old: &BundleInstallLayerMetadata, now: &BundleInstallLayerMetada
     }
 }
 
+#[allow(deprecated)]
 impl Layer for BundleInstallLayer<'_> {
     type Buildpack = RubyBuildpack;
     type Metadata = BundleInstallLayerMetadata;

diff --git a/buildpacks/ruby/src/layers/metrics_agent_install.rs b/buildpacks/ruby/src/layers/metrics_agent_install.rs
@@ -1,6 +1,6 @@
 use crate::{RubyBuildpack, RubyBuildpackError};
 use bullet_stream::state::SubBullet;
-use bullet_stream::Print;
+use bullet_stream::{style, Print};
 use flate2::read::GzDecoder;
 use libcnb::additional_buildpack_binary_path;
 use libcnb::data::layer_name;
@@ -29,7 +29,7 @@ const DOWNLOAD_URL: &str =
     "https://agentmon-releases.s3.us-east-1.amazonaws.com/agentmon-0.3.1-linux-amd64.tar.gz";
 const DOWNLOAD_SHA: &str = "f9bf9f33c949e15ffed77046ca38f8dae9307b6a0181c6af29a25dec46eb2dac";
 
-#[derive(Deserialize, Serialize, Debug, Clone)]
+#[derive(Deserialize, Serialize, Debug, Clone, PartialEq, Eq)]
 pub(crate) struct Metadata {
     download_url: String,
 }
@@ -63,22 +63,26 @@ pub(crate) fn handle_metrics_agent_layer(
     context: &libcnb::build::BuildContext<RubyBuildpack>,
     mut bullet: Print<SubBullet<Stdout>>,
 ) -> libcnb::Result<Print<SubBullet<Stdout>>, RubyBuildpackError> {
+    let metadata = Metadata {
+        download_url: DOWNLOAD_URL.to_string(),
+    };
+
     let layer_ref = context.cached_layer(
         layer_name!("metrics_agent"),
         CachedLayerDefinition {
             build: true,
             launch: true,
             invalid_metadata_action: &|_| InvalidMetadataAction::DeleteLayer,
-            restored_layer_action: &|metadata: &Metadata, _| {
-                if metadata.download_url == DOWNLOAD_URL {
+            restored_layer_action: &|old: &Metadata, _| {
+                if old == &metadata {
                     (
                         RestoredLayerAction::KeepLayer,
-                        metadata.download_url.clone(),
+                        style::url(old.download_url.clone()),
                     )
                 } else {
                     (
                         RestoredLayerAction::DeleteLayer,
-                        metadata.download_url.clone(),
+                        style::url(old.download_url.clone()),
                     )
                 }
             },
@@ -101,19 +105,20 @@ pub(crate) fn handle_metrics_agent_layer(
             }
             let bin_dir = layer_ref.path().join("bin");
 
-            let timer = bullet.start_timer(format!("Installing metrics agent from {DOWNLOAD_URL}"));
-            let agentmon =
-                install_agentmon(&bin_dir).map_err(RubyBuildpackError::MetricsAgentError)?;
+            let timer = bullet.start_timer(format!(
+                "Installing metrics agent from {url}",
+                url = style::url(&metadata.download_url)
+            ));
+            let agentmon = install_agentmon(&bin_dir, &metadata)
+                .map_err(RubyBuildpackError::MetricsAgentError)?;
             bullet = timer.done();
 
             bullet = bullet.sub_bullet("Writing scripts");
             let execd = write_execd_script(&agentmon, layer_ref.path().as_path())
                 .map_err(RubyBuildpackError::MetricsAgentError)?;
 
             layer_ref.write_exec_d_programs([("spawn_metrics_agent".to_string(), execd)])?;
-            layer_ref.write_metadata(Metadata {
-                download_url: DOWNLOAD_URL.to_string(),
-            })?;
+            layer_ref.write_metadata(metadata)?;
         }
     }
     Ok(bullet)
@@ -162,8 +167,8 @@ fn write_execd_script(
     Ok(execd)
 }
 
-fn install_agentmon(dir: &Path) -> Result<PathBuf, MetricsAgentInstallError> {
-    let agentmon = download_untar(DOWNLOAD_URL, dir).map(|()| dir.join("agentmon"))?;
+fn install_agentmon(dir: &Path, metadata: &Metadata) -> Result<PathBuf, MetricsAgentInstallError> {
+    let agentmon = download_untar(&metadata.download_url, dir).map(|()| dir.join("agentmon"))?;
 
     chmod_plus_x(&agentmon).map_err(MetricsAgentInstallError::PermissionError)?;
     Ok(agentmon)

diff --git a/buildpacks/ruby/src/layers/ruby_install_layer.rs b/buildpacks/ruby/src/layers/ruby_install_layer.rs
@@ -12,6 +12,7 @@ use commons::gemfile_lock::ResolvedRubyVersion;
 use flate2::read::GzDecoder;
 use libcnb::build::BuildContext;
 use libcnb::data::layer_content_metadata::LayerTypes;
+#[allow(deprecated)]
 use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
 use serde::{Deserialize, Deserializer, Serialize};
 use std::convert::Infallible;
@@ -92,6 +93,7 @@ impl TryFrom<RubyInstallLayerMetadataV1> for RubyInstallLayerMetadataV2 {
     }
 }
 
+#[allow(deprecated)]
 impl<'a> Layer for RubyInstallLayer<'a> {
     type Buildpack = RubyBuildpack;
     type Metadata = RubyInstallLayerMetadata;

diff --git a/buildpacks/ruby/src/main.rs b/buildpacks/ruby/src/main.rs
@@ -115,6 +115,7 @@ impl Buildpack for RubyBuildpack {
     }
 
     #[allow(clippy::too_many_lines)]
+    #[allow(deprecated)]
     fn build(&self, context: BuildContext<Self>) -> libcnb::Result<BuildResult, Self::Error> {
         let mut logger = BuildLog::new(stdout()).buildpack_name("Heroku Ruby Buildpack");
         let warn_later = WarnGuard::new(stdout());
@@ -131,15 +132,15 @@ impl Buildpack for RubyBuildpack {
         let bundler_version = gemfile_lock.resolve_bundler("2.4.5");
         let ruby_version = gemfile_lock.resolve_ruby("3.1.3");
 
-        let mut build_output = Print::new(stdout()).without_header();
+        let build_output = Print::new(stdout()).without_header();
         // ## Install metrics agent
-        build_output = {
+        _ = {
             let bullet = build_output.bullet("Metrics agent");
             if lockfile_contents.contains("barnes") {
                 layers::metrics_agent_install::handle_metrics_agent_layer(&context, bullet)?.done()
             } else {
                 bullet
-                    .sub_bullet(&format!(
+                    .sub_bullet(format!(
                         "Skipping install ({barnes} gem not found)",
                         barnes = style::value("barnes")
                     ))

diff --git a/buildpacks/ruby/src/steps/default_env.rs b/buildpacks/ruby/src/steps/default_env.rs
@@ -9,6 +9,7 @@ use libcnb::{
 use rand::Rng;
 
 // Set default environment values
+#[allow(deprecated)]
 pub(crate) fn default_env(
     context: &BuildContext<RubyBuildpack>,
     platform_env: &Env,

diff --git a/commons/src/cache/app_cache.rs b/commons/src/cache/app_cache.rs
@@ -236,6 +236,7 @@ pub enum PathState {
 /// # Errors
 ///
 /// - If the layer cannot be created
+#[allow(deprecated)]
 pub fn build<B: libcnb::Buildpack>(
     context: &BuildContext<B>,
     config: CacheConfig,

diff --git a/commons/src/cache/app_cache_collection.rs b/commons/src/cache/app_cache_collection.rs
@@ -34,15 +34,14 @@ impl<'a> AppCacheCollection<'a> {
         let caches = config
             .into_iter()
             .map(|config| {
-                AppCache::new_and_load(context, config).map(|store| {
+                AppCache::new_and_load(context, config).inspect(|store| {
                     let path = store.path().display();
 
                     log::log_step(match store.cache_state() {
                         CacheState::NewEmpty => format!("Creating cache for {path}"),
                         CacheState::ExistsEmpty => format!("Loading (empty) cache for {path}"),
                         CacheState::ExistsWithContents => format!("Loading cache for {path}"),
                     });
-                    store
                 })
             })
             .collect::<Result<Vec<AppCache>, CacheError>>()?;

diff --git a/commons/src/cache/in_app_dir_cache_layer.rs b/commons/src/cache/in_app_dir_cache_layer.rs
@@ -1,5 +1,6 @@
 use libcnb::build::BuildContext;
 use libcnb::data::layer_content_metadata::LayerTypes;
+#[allow(deprecated)]
 use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
 use libcnb::Buildpack;
 use serde::{Deserialize, Serialize};
@@ -41,6 +42,7 @@ impl<B> InAppDirCacheLayer<B> {
     }
 }
 
+#[allow(deprecated)]
 impl<B> Layer for InAppDirCacheLayer<B>
 where
     B: Buildpack,

diff --git a/commons/src/layer/configure_env_layer.rs b/commons/src/layer/configure_env_layer.rs
@@ -1,6 +1,7 @@
 use libcnb::build::BuildContext;
 use libcnb::data::layer_content_metadata::LayerTypes;
 use libcnb::generic::GenericMetadata;
+#[allow(deprecated)]
 use libcnb::layer::{Layer, LayerResult, LayerResultBuilder};
 use libcnb::layer_env::LayerEnv;
 use std::marker::PhantomData;
@@ -99,6 +100,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<B> Layer for ConfigureEnvLayer<B>
 where
     B: libcnb::Buildpack,