From bbb6d9cde9dc168179be84c441efccbc9cb62986 Mon Sep 17 00:00:00 2001 From: Richard Schneeman Date: Thu, 7 Sep 2023 14:08:54 -0500 Subject: [PATCH] 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. --- CHANGELOG.md | 5 +++++ examples/execd/src/layer.rs | 2 +- libcnb/src/build.rs | 2 +- libcnb/src/layer/handling.rs | 14 ++++++++------ libcnb/src/layer/public_interface.rs | 6 +++--- libcnb/src/layer/tests.rs | 16 ++++++++-------- .../readonly-layer-files/src/layer.rs | 4 ++-- test-buildpacks/sbom/src/test_layer.rs | 6 +++--- test-buildpacks/sbom/src/test_layer_2.rs | 4 ++-- 9 files changed, 33 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db3a9478..5e832bca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- `libcnb`: + - Change Layer interface from `&self` to `&mut self`. ([#669](https://github.com/heroku/libcnb.rs/pull/669)) + ## [0.17.0] - 2023-12-06 diff --git a/examples/execd/src/layer.rs b/examples/execd/src/layer.rs index 6c2fea3f..fbf4a124 100644 --- 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, _layer_path: &Path, ) -> Result, ::Error> { diff --git a/libcnb/src/build.rs b/libcnb/src/build.rs index 49785511..13d17fae 100644 --- a/libcnb/src/build.rs +++ b/libcnb/src/build.rs @@ -85,7 +85,7 @@ impl BuildContext { /// # } /// # /// fn create( - /// &self, + /// &mut self, /// context: &BuildContext, /// layer_path: &Path, /// ) -> Result, ::Error> { diff --git a/libcnb/src/layer/handling.rs b/libcnb/src/layer/handling.rs index 2a696b7d..0e0d9b71 100644 --- 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>( context: &BuildContext, layer_name: LayerName, - layer: L, + mut layer: L, ) -> Result, HandleLayerErrorOrBuildpackError> { 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>( 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>( fn handle_create_layer>( context: &BuildContext, layer_name: &LayerName, - layer: &L, + layer: &mut L, ) -> Result, HandleLayerErrorOrBuildpackError> { let layer_dir = context.layers_dir.join(layer_name.as_str()); @@ -138,7 +140,7 @@ fn handle_create_layer>( fn handle_update_layer>( context: &BuildContext, layer_data: &LayerData, - layer: &L, + layer: &mut L, ) -> Result, HandleLayerErrorOrBuildpackError> { let layer_result = layer .update(context, layer_data) diff --git a/libcnb/src/layer/public_interface.rs b/libcnb/src/layer/public_interface.rs index 658b1610..5f25e72d 100644 --- 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, layer_path: &Path, ) -> Result, ::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, layer_data: &LayerData, ) -> Result::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, layer_data: &LayerData, ) -> Result, ::Error> { diff --git a/libcnb/src/layer/tests.rs b/libcnb/src/layer/tests.rs index cbaa170f..81fa4489 100644 --- 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, layer_path: &Path, ) -> Result, ::Error> { @@ -79,7 +79,7 @@ impl Layer for TestLayer { } fn existing_layer_strategy( - &self, + &mut self, _context: &BuildContext, _layer_data: &LayerData, ) -> Result::Error> { @@ -87,7 +87,7 @@ impl Layer for TestLayer { } fn update( - &self, + &mut self, _context: &BuildContext, layer_data: &LayerData, ) -> Result, ::Error> { @@ -749,7 +749,7 @@ fn default_layer_method_implementations() { } fn create( - &self, + &mut self, _context: &BuildContext, _layer_path: &Path, ) -> Result, ::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, _layer_path: &Path, ) -> Result, ::Error> { @@ -848,7 +848,7 @@ fn layer_env_read_write() { } fn existing_layer_strategy( - &self, + &mut self, _context: &BuildContext, layer_data: &LayerData, ) -> Result::Error> { @@ -858,7 +858,7 @@ fn layer_env_read_write() { } fn update( - &self, + &mut self, _context: &BuildContext, layer_data: &LayerData, ) -> Result, ::Error> { diff --git a/test-buildpacks/readonly-layer-files/src/layer.rs b/test-buildpacks/readonly-layer-files/src/layer.rs index 0f6b631c..555ff54a 100644 --- 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, layer_path: &Path, ) -> Result, ::Error> { @@ -42,7 +42,7 @@ impl Layer for TestLayer { } fn existing_layer_strategy( - &self, + &mut self, _context: &BuildContext, _layer_data: &LayerData, ) -> Result::Error> { diff --git a/test-buildpacks/sbom/src/test_layer.rs b/test-buildpacks/sbom/src/test_layer.rs index 8ebdde2b..ce4154cb 100644 --- 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, _layer_path: &Path, ) -> Result, ::Error> { @@ -43,7 +43,7 @@ impl Layer for TestLayer { } fn existing_layer_strategy( - &self, + &mut self, _context: &BuildContext, _layer_data: &LayerData, ) -> Result::Error> { @@ -51,7 +51,7 @@ impl Layer for TestLayer { } fn update( - &self, + &mut self, _context: &BuildContext, _layer_data: &LayerData, ) -> Result, ::Error> { diff --git a/test-buildpacks/sbom/src/test_layer_2.rs b/test-buildpacks/sbom/src/test_layer_2.rs index 2ce8bc0f..bf244321 100644 --- a/test-buildpacks/sbom/src/test_layer_2.rs +++ b/test-buildpacks/sbom/src/test_layer_2.rs @@ -22,7 +22,7 @@ impl Layer for TestLayer2 { } fn create( - &self, + &mut self, _context: &BuildContext, _layer_path: &Path, ) -> Result, ::Error> { @@ -43,7 +43,7 @@ impl Layer for TestLayer2 { } fn existing_layer_strategy( - &self, + &mut self, _context: &BuildContext, _layer_data: &LayerData, ) -> Result::Error> {