Skip to content

Commit

Permalink
Change Layer interface from &self to &mut self
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
schneems committed Jan 27, 2024
1 parent 2209e8f commit 18aeaaa
Show file tree
Hide file tree
Showing 11 changed files with 215 additions and 26 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion examples/execd/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
121 changes: 121 additions & 0 deletions examples/ruby-sample/src/layers/bundler.rs
Original file line number Diff line number Diff line change
@@ -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()
}
}
61 changes: 61 additions & 0 deletions examples/ruby-sample/src/layers/ruby.rs
Original file line number Diff line number Diff line change
@@ -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()
}
}
2 changes: 1 addition & 1 deletion libcnb/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
14 changes: 8 additions & 6 deletions libcnb/src/layer/handling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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());

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions libcnb/src/layer/public_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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> {
Expand All @@ -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> {
Expand Down
16 changes: 8 additions & 8 deletions libcnb/src/layer/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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> {
Expand Down Expand Up @@ -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> {
Expand All @@ -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"),
Expand Down Expand Up @@ -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> {
Expand All @@ -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> {
Expand All @@ -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> {
Expand Down
4 changes: 2 additions & 2 deletions test-buildpacks/readonly-layer-files/src/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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> {
Expand Down
Loading

0 comments on commit 18aeaaa

Please sign in to comment.