search

heroku / libcnb.rs

A framework for writing Cloud Native Buildpacks in Rust
BSD 3-Clause "New" or "Revised" License
34 stars 6 forks source link

Refactor low-level layer operations #786

Closed Malax closed 5 months ago

Malax commented 6 months ago

This PR refactors the internal functions in layer/handling.rs in preparation of a new Layer API PR. The only public (and strictly speaking breaking) change are the new error enums for writing SBOM and exec.d.

The initial PR that also exposed these low-level building blocked proved to be more controversial than I expected so this PR changed to only refactor the internal bits.

Old PR description

This PR is the first phase for a new high-level Layer API. It exposes the low-level building blocks needed to work with CNB layers. It is not the new layer API.

Why do we need this when we're aiming for a new high-level API?

First of all, that new high-level API needs access to these operations. They were originally written as helpers for the Layer trait API and and needed some adjustments to make them usable in other contexts. I mainly improved error handing and naming. This work needed to be done in any case and this PR front loads these changes in a separate PR for easier review. :)

Secondly, regardless of how great the new high-level API will be, there will always be fringe use-cases where low-level building blocks are needed. Right now, users of libcnb.rs are out of luck if they ever hit these uses-cases. After this PR, we have an escape hatch when a higher-level abstraction (either the current or new one) doesn't work.

Notes for reviewers

Start with the BuildContext changes in libcnb/src/build.rs. This is the newly exposed API for users. Everything else is just supporting changes.

I've broken up the steps I took in separate commits. It might be helpful to refer to those to see how things changed in atomic steps from the original.

Malax commented 6 months ago

There's no way to write an environment variable that's exposed right now. I would like one to be added.

https://docs.rs/libcnb/latest/libcnb/layer_env/struct.LayerEnv.html#method.write_to_layer_dir

Sboms and execd can be written but not read (so not updated). I would like a way to read these values.

See comment here: https://github.com/heroku/libcnb.rs/pull/786#discussion_r1505736236

Malax commented 6 months ago

@schneems, @edmorley since the exposure of the low-level building blocks seems much more controversial than I expected, those changes have been removed from this PR. It's now only refactoring the internal functions and slightly changes WriteLayerError.

I still think we want to do this eventually (and some of it is needed for the new Layer API), but I want the internal changes merged ASAP to be able to open the actual new Layer API PR.

I will re-request review from you both. :)