Change Layer interface from &self to &mut self

The primary driver for wanting to mutate a layer is a desire for stateful logging. I hit this in the consuming state-machine interface, and previously, Josh hit it in a logging interface designed to track indentation level. Upside: - Developers are now allowed to do whatever they want within a layer - It's more technically correct to require a single reference for these functions. With the prior interface, it would be theoretically possible to execute the same Layer in parallel (though unlikely) - We can now change things between function calls. Downside: - Mutable self means there's no guarantee something didn't change between function calls.
heroku · Jan 27, 2024 · 6a42587 · 6a42587
1 parent 2209e8f
commit 6a42587
Show file tree

Hide file tree

Showing 10 changed files with 210 additions and 26 deletions.
diff --git a/examples/execd/src/layer.rs b/examples/execd/src/layer.rs
@@ -21,7 +21,7 @@ impl Layer for ExecDLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/examples/ruby-sample/src/layers/bundler.rs b/examples/ruby-sample/src/layers/bundler.rs
@@ -0,0 +1,121 @@
+use crate::RubyBuildpack;
+use crate::{util, RubyBuildpackError};
+use libcnb::build::BuildContext;
+use libcnb::data::layer_content_metadata::LayerTypes;
+use libcnb::layer::{ExistingLayerStrategy, Layer, LayerData, LayerResult, LayerResultBuilder};
+use libcnb::Env;
+use serde::Deserialize;
+use serde::Serialize;
+use std::path::Path;
+use std::process::Command;
+
+#[derive(Deserialize, Serialize, Debug, Clone)]
+pub(crate) struct BundlerLayerMetadata {
+    gemfile_lock_checksum: String,
+}
+
+pub(crate) struct BundlerLayer {
+    pub ruby_env: Env,
+}
+
+impl Layer for BundlerLayer {
+    type Buildpack = RubyBuildpack;
+    type Metadata = BundlerLayerMetadata;
+
+    fn types(&self) -> LayerTypes {
+        LayerTypes {
+            build: true,
+            launch: true,
+            cache: true,
+        }
+    }
+
+    fn create(
+        &mut self,
+        context: &BuildContext<Self::Buildpack>,
+        layer_path: &Path,
+    ) -> Result<LayerResult<Self::Metadata>, RubyBuildpackError> {
+        println!("---> Installing bundler");
+
+        util::run_simple_command(
+            Command::new("gem")
+                .args(["install", "bundler", "--force"])
+                .envs(&self.ruby_env),
+            RubyBuildpackError::GemInstallBundlerCommandError,
+            RubyBuildpackError::GemInstallBundlerUnexpectedExitStatus,
+        )?;
+
+        println!("---> Installing gems");
+
+        util::run_simple_command(
+            Command::new("bundle")
+                .args([
+                    "install",
+                    "--path",
+                    layer_path.to_str().unwrap(),
+                    "--binstubs",
+                    layer_path.join("bin").to_str().unwrap(),
+                ])
+                .envs(&self.ruby_env),
+            RubyBuildpackError::BundleInstallCommandError,
+            RubyBuildpackError::BundleInstallUnexpectedExitStatus,
+        )?;
+
+        LayerResultBuilder::new(BundlerLayerMetadata {
+            gemfile_lock_checksum: util::sha256_checksum(context.app_dir.join("Gemfile.lock"))
+                .map_err(RubyBuildpackError::CouldNotGenerateChecksum)?,
+        })
+        .build()
+    }
+
+    fn existing_layer_strategy(
+        &mut self,
+        context: &BuildContext<Self::Buildpack>,
+        layer: &LayerData<Self::Metadata>,
+    ) -> Result<ExistingLayerStrategy, RubyBuildpackError> {
+        util::sha256_checksum(context.app_dir.join("Gemfile.lock"))
+            .map_err(RubyBuildpackError::CouldNotGenerateChecksum)
+            .map(|checksum| {
+                if checksum == layer.content_metadata.metadata.gemfile_lock_checksum {
+                    ExistingLayerStrategy::Keep
+                } else {
+                    ExistingLayerStrategy::Update
+                }
+            })
+    }
+
+    fn update(
+        &mut self,
+        context: &BuildContext<Self::Buildpack>,
+        layer: &LayerData<Self::Metadata>,
+    ) -> Result<LayerResult<Self::Metadata>, RubyBuildpackError> {
+        println!("---> Reusing gems");
+
+        util::run_simple_command(
+            Command::new("bundle")
+                .args(["config", "--local", "path", layer.path.to_str().unwrap()])
+                .envs(&self.ruby_env),
+            RubyBuildpackError::BundleConfigCommandError,
+            RubyBuildpackError::BundleConfigUnexpectedExitStatus,
+        )?;
+
+        util::run_simple_command(
+            Command::new("bundle")
+                .args([
+                    "config",
+                    "--local",
+                    "bin",
+                    layer.path.join("bin").as_path().to_str().unwrap(),
+                ])
+                .envs(&self.ruby_env),
+            RubyBuildpackError::BundleConfigCommandError,
+            RubyBuildpackError::BundleConfigUnexpectedExitStatus,
+        )?;
+
+        LayerResultBuilder::new(BundlerLayerMetadata {
+            gemfile_lock_checksum: util::sha256_checksum(context.app_dir.join("Gemfile.lock"))
+                .map_err(RubyBuildpackError::CouldNotGenerateChecksum)?,
+        })
+        .build()
+    }
+}
diff --git a/examples/ruby-sample/src/layers/ruby.rs b/examples/ruby-sample/src/layers/ruby.rs
@@ -0,0 +1,61 @@
+use crate::util;
+use crate::{RubyBuildpack, RubyBuildpackError};
+use libcnb::build::BuildContext;
+use libcnb::data::layer_content_metadata::LayerTypes;
+use libcnb::generic::GenericMetadata;
+use libcnb::layer::{Layer, LayerResult, LayerResultBuilder};
+use libcnb::layer_env::{LayerEnv, ModificationBehavior, Scope};
+use std::path::Path;
+use tempfile::NamedTempFile;
+
+pub(crate) struct RubyLayer;
+
+impl Layer for RubyLayer {
+    type Buildpack = RubyBuildpack;
+    type Metadata = GenericMetadata;
+
+    fn types(&self) -> LayerTypes {
+        LayerTypes {
+            build: true,
+            launch: true,
+            cache: false,
+        }
+    }
+
+    fn create(
+        &mut self,
+        context: &BuildContext<Self::Buildpack>,
+        layer_path: &Path,
+    ) -> Result<LayerResult<Self::Metadata>, RubyBuildpackError> {
+        println!("---> Download and extracting Ruby");
+
+        let ruby_tgz =
+            NamedTempFile::new().map_err(RubyBuildpackError::CouldNotCreateTemporaryFile)?;
+
+        util::download(
+            &context.buildpack_descriptor.metadata.ruby_url,
+            ruby_tgz.path(),
+        )
+        .map_err(RubyBuildpackError::RubyDownloadError)?;
+
+        util::untar(ruby_tgz.path(), layer_path).map_err(RubyBuildpackError::RubyUntarError)?;
+
+        LayerResultBuilder::new(GenericMetadata::default())
+            .env(
+                LayerEnv::new()
+                    .chainable_insert(
+                        Scope::All,
+                        ModificationBehavior::Prepend,
+                        "PATH",
+                        context.app_dir.join(".gem/ruby/2.6.6/bin"),
+                    )
+                    .chainable_insert(
+                        Scope::All,
+                        ModificationBehavior::Prepend,
+                        "LD_LIBRARY_PATH",
+                        layer_path,
+                    ),
+            )
+            .build()
+    }
+}
diff --git a/libcnb/src/build.rs b/libcnb/src/build.rs
@@ -85,7 +85,7 @@ impl<B: Buildpack + ?Sized> BuildContext<B> {
     /// #    }
     /// #
     ///     fn create(
-    ///         &self,
+    ///         &mut self,
     ///         context: &BuildContext<Self::Buildpack>,
     ///         layer_path: &Path,
     ///     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/libcnb/src/layer/handling.rs b/libcnb/src/layer/handling.rs
@@ -21,10 +21,10 @@ use std::path::{Path, PathBuf};
 pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_name: LayerName,
-    layer: L,
+    mut layer: L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     match read_layer(&context.layers_dir, &layer_name) {
-        Ok(None) => handle_create_layer(context, &layer_name, &layer),
+        Ok(None) => handle_create_layer(context, &layer_name, &mut layer),
         Ok(Some(layer_data)) => {
             let existing_layer_strategy = layer
                 .existing_layer_strategy(context, &layer_data)
@@ -33,9 +33,11 @@ pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
             match existing_layer_strategy {
                 ExistingLayerStrategy::Recreate => {
                     delete_layer(&context.layers_dir, &layer_name)?;
-                    handle_create_layer(context, &layer_name, &layer)
+                    handle_create_layer(context, &layer_name, &mut layer)
+                }
+                ExistingLayerStrategy::Update => {
+                    handle_update_layer(context, &layer_data, &mut layer)
                 }
-                ExistingLayerStrategy::Update => handle_update_layer(context, &layer_data, &layer),
                 ExistingLayerStrategy::Keep => {
                     // We need to rewrite the metadata even if we just want to keep the layer around
                     // since cached layers are restored without their types, causing the layer to be
@@ -106,7 +108,7 @@ pub(crate) fn handle_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
 fn handle_create_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_name: &LayerName,
-    layer: &L,
+    layer: &mut L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     let layer_dir = context.layers_dir.join(layer_name.as_str());
 
@@ -138,7 +140,7 @@ fn handle_create_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
 fn handle_update_layer<B: Buildpack + ?Sized, L: Layer<Buildpack = B>>(
     context: &BuildContext<B>,
     layer_data: &LayerData<L::Metadata>,
-    layer: &L,
+    layer: &mut L,
 ) -> Result<LayerData<L::Metadata>, HandleLayerErrorOrBuildpackError<B::Error>> {
     let layer_result = layer
         .update(context, layer_data)

diff --git a/libcnb/src/layer/public_interface.rs b/libcnb/src/layer/public_interface.rs
@@ -45,7 +45,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** write to any other location than `layer_path`.
     fn create(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error>;
@@ -74,7 +74,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** modify the file-system.
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
@@ -100,7 +100,7 @@ pub trait Layer {
     /// # Implementation Requirements
     /// Implementations **MUST NOT** write to any other location than `layer_path`.
     fn update(
-        &self,
+        &mut self,
         context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/libcnb/src/layer/tests.rs b/libcnb/src/layer/tests.rs
@@ -61,7 +61,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -79,15 +79,15 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
         Ok(self.existing_layer_strategy)
     }
 
     fn update(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -749,7 +749,7 @@ fn default_layer_method_implementations() {
         }
 
         fn create(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             _layer_path: &Path,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -770,7 +770,7 @@ fn default_layer_method_implementations() {
     let temp_dir = tempdir().unwrap();
     let context = build_context(&temp_dir);
     let layer_name = random_layer_name();
-    let simple_layer = SimpleLayer;
+    let mut simple_layer = SimpleLayer;
 
     let simple_layer_metadata = SimpleLayerMetadata {
         field_one: String::from("value one"),
@@ -838,7 +838,7 @@ fn layer_env_read_write() {
         }
 
         fn create(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             _layer_path: &Path,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -848,7 +848,7 @@ fn layer_env_read_write() {
         }
 
         fn existing_layer_strategy(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             layer_data: &LayerData<Self::Metadata>,
         ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
@@ -858,7 +858,7 @@ fn layer_env_read_write() {
         }
 
         fn update(
-            &self,
+            &mut self,
             _context: &BuildContext<Self::Buildpack>,
             layer_data: &LayerData<Self::Metadata>,
         ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {

diff --git a/test-buildpacks/readonly-layer-files/src/layer.rs b/test-buildpacks/readonly-layer-files/src/layer.rs
@@ -24,7 +24,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -42,7 +42,7 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {

diff --git a/test-buildpacks/sbom/src/test_layer.rs b/test-buildpacks/sbom/src/test_layer.rs
@@ -22,7 +22,7 @@ impl Layer for TestLayer {
     }
 
     fn create(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_path: &Path,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {
@@ -43,15 +43,15 @@ impl Layer for TestLayer {
     }
 
     fn existing_layer_strategy(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<ExistingLayerStrategy, <Self::Buildpack as Buildpack>::Error> {
         Ok(ExistingLayerStrategy::Update)
     }
 
     fn update(
-        &self,
+        &mut self,
         _context: &BuildContext<Self::Buildpack>,
         _layer_data: &LayerData<Self::Metadata>,
     ) -> Result<LayerResult<Self::Metadata>, <Self::Buildpack as Buildpack>::Error> {