Move shared caching logic to public location, rename to DiffMigrateLayer

[schneems]

The goal of the shared layer logic in layers/shared was to iterate on an interface that uses magic_migrate and cache_diff to produce a declarative interface for layer caching. I'm happy with that interface and wanting to make that interface public so other buildpacks may use it if they choose.

Commits are small(ish) and in order. Details about the goals and functionality are in the module docs https://github.com/heroku/buildpacks-ruby/blob/schneems/cache_buddy/commons/src/layer/diff_migrate.rs.

[runesoerensen]

It seems the CacheBuddy struct and function implementations are mostly here to provide an interface. Perhaps it'd make sense to consider an approach that resembles the libcnb implementation closer in terms of both usage and naming, for instance by renaming CacheBuddy to CacheBuddyLayerDefinition (similar to the UncachedLayerDefinition and CachedLayerDefinition variants in libcnb) and use the same semantics for the build and launch types:

pub struct CacheBuddyLayerDefinition {
    /// Whether the layer is intended for build.
    pub build: bool,
    /// Whether the layer is intended for launch.
    pub launch: bool,
}

You could of course just use that in a function similar to the current CacheBuddy#layer (though the function name should probably be updated for readability, e.g. CacheBuddyLayerDefinition#create_layer or similar).

Alternatively, it might make sense to consider an interface that allows use of CacheBuddyLayerDefinition similar to how other definitions are used when creating layers. I'm not sure if there are more elegant solutions than defining and implementing a trait to achieve that, but here's one way to do that:

pub trait CacheBuddyLayer<B> {
    fn cache_buddy_layer<M>(
        &self,
        layer_name: LayerName,
        layer_definition: CacheBuddyLayerDefinition,
        metadata: &M,
    ) -> libcnb::Result<LayerRef<B, Meta<M>, Meta<M>>, B::Error>
    where
        B: libcnb::Buildpack,
        M: CacheDiff + TryMigrate + Serialize + Debug + Clone;
}

impl<B> CacheBuddyLayer<B> for BuildContext<B>
where
    B: libcnb::Buildpack,
{
    fn cache_buddy_layer<M>(
        &self,
        layer_name: LayerName,
        definition: CacheBuddyLayerDefinition,
        metadata: &M,
    ) -> libcnb::Result<LayerRef<B, Meta<M>, Meta<M>>, B::Error>
    where
        M: CacheDiff + TryMigrate + Serialize + Debug + Clone,
    {
        let layer_ref = self.cached_layer(
            layer_name,
            CachedLayerDefinition {
                build: definition.build,
                launch: definition.launch,
                invalid_metadata_action: &invalid_metadata_action,
                restored_layer_action: &|old: &M, _| restored_layer_action(old, metadata),
            },
        )?;
        layer_ref.write_metadata(metadata)?;
        Ok(layer_ref)
    }
}

This could then be used in for instance the Ruby layer like so:

let layer_ref = context.cache_buddy_layer(
    layer_name!("ruby"),
    CacheBuddyLayerDefinition {
        build: true,
        launch: true,
    },
    metadata,
)?;

The benefits compared to the current use (CacheBuddy::new().layer(layer_name!("bundler"), context, metadata)?;):

• Improved readability: For libcnb users, this interface closely resembles how they create layers. This also allows writing less, and more focused documentation. It's clear that the CacheBuddyLayerDefinition is equivalent to other layer definition structs. Similarly, the cache_buddy_layer implementation on BuildContext indicates that this function will do something similar to uncached_layer and cached_layer (i.e. create a layer with some added CacheBuddy magic).
• Clearer semantics: Assuming you don't plan to handle scenarios where None is a meaningful value, using bool for the build and launch fields avoids confusion compared to Option<bool>. None of the CacheBuddy callers in this repo set these values, and instantiate the struct with the CacheBuddy::new function. The layer function in turn defaults to true when these options are None, which might be a bit surprising.

[schneems]

It seems the CacheBuddy struct and function implementations are mostly here to provide an interface.

Exactly. That's the sticking point. Naming + an ergonomic interface. When creating/reading a cached layer I want the interface to:

• Provide a invalid_metadata_action
• Provide a restored_layer_action
• Write metadata by default (so it's not forgotten)

Based on your feedback. I experimented with trying to have a struct implement From<CachedLayerDefinition> but the lifetimes don't allow what I want (because &metadata needs to live somewhere. Even if I could do such a thing, it wouldn't give me the "write metadata" functionality by default.

Another alternative is shipping functions only. The current implementation makes <module>::invalid_metadata_action and <module>::restored_layer_action public, which gives me 2/3 of the things I want (again, missing the default metadata write).

Using a trait extension idea is an interesting option. I'm worried that there would be some confusion around associated function naming, in general they're a little awkward to work with. I don't want to end up in a situation where the compiler says something like "No such function awesome_cache_layer did you mean to call cache_layer" where it's guiding them to the default interface. It's not a strong objection, talking out loud.

As an alternative. I've experimented with a builder approach which allows us to be be maximally configurable while giving me all the defaults I want. Here's a sketch of an alternative interface using the builder pattern on a function with the bon crate #377. The downside is it's a little unusual of an interface if people haven't seen it before, but it's honestly very ergonomic to work with as it won't compile if you forget a required function.

[schneems]

Updated naming which is hopefully more clear. Added a tutorial. After exploring something more like an extremely flexible builder, I want to start with something highly opionated and only move to something more flexible if there's a specific reason for it.